[bpf-next,v1,00/13] MAC and Audit policy using eBPF (KRSI)
mbox series

Message ID 20191220154208.15895-1-kpsingh@chromium.org
Headers show
Series
  • MAC and Audit policy using eBPF (KRSI)
Related show

Message

KP Singh Dec. 20, 2019, 3:41 p.m. UTC
From: KP Singh <kpsingh@google.com>

This patch series is a continuation of the KRSI RFC
(https://lore.kernel.org/bpf/20190910115527.5235-1-kpsingh@chromium.org/)

# Motivation

Google does rich analysis of runtime security data collected from
internal Linux deployments (corporate devices and servers) to detect and
thwart threats in real-time. Currently, this is done in custom kernel
modules but we would like to replace this with something that's upstream
and useful to others.

The current kernel infrastructure for providing telemetry (Audit, Perf
etc.) is disjoint from access enforcement (i.e. LSMs).  Augmenting the
information provided by audit requires kernel changes to audit, its
policy language and user-space components. Furthermore, building a MAC
policy based on the newly added telemetry data requires changes to
various LSMs and their respective policy languages.

This patchset proposes a new stackable and privileged LSM which allows
the LSM hooks to be implemented using eBPF. This facilitates a unified
and dynamic (not requiring re-compilation of the kernel) audit and MAC
policy.

# Why an LSM?

Linux Security Modules target security behaviours rather than the
kernel's API. For example, it's easy to miss out a newly added system
call for executing processes (eg. execve, execveat etc.) but the LSM
framework ensures that all process executions trigger the relevant hooks
irrespective of how the process was executed.

Allowing users to implement LSM hooks at runtime also benefits the LSM
eco-system by enabling a quick feedback loop from the security community
about the kind of behaviours that the LSM Framework should be targeting.

# How does it work?

The LSM introduces a new eBPF (https://docs.cilium.io/en/v1.6/bpf/)
program type, BPF_PROG_TYPE_LSM, which can only be attached to a LSM
hook.  All LSM hooks are exposed as files in securityfs. Attachment
requires CAP_SYS_ADMIN for loading eBPF programs and CAP_MAC_ADMIN for
modifying MAC policies.

The eBPF programs are passed the same arguments as the LSM hooks and
executed in the body of the hook. If any of the eBPF programs returns an
error (like ENOPERM), the behaviour represented by the hook is denied.

Audit logs can be written using a format chosen by the eBPF program to
the perf events buffer and can be further processed in user-space.

# Limitations of RFC v1

In the previous design
(https://lore.kernel.org/bpf/20190910115527.5235-1-kpsingh@chromium.org/),
the BPF programs received a context which could be queried to retrieve
specific pieces of information using specific helpers.

For example, a program that attaches to the file_mprotect LSM hook and
queries the VMA region could have had the following context:

// Special context for the hook.
struct bpf_mprotect_ctx {
	struct vm_area_struct *vma;
};

and accessed the fields using a hypothetical helper
"bpf_mprotect_vma_get_start:

SEC("lsm/file_mprotect")
int mprotect_audit(bpf_mprotect_ctx *ctx)
{
	unsigned long vm_start = bpf_mprotect_vma_get_start(ctx);
	return 0;
}

or directly read them from the context by updating the verifier to allow
accessing the fields:

int mprotect_audit(bpf_mprotect_ctx *ctx)
{
	unsigned long vm_start = ctx->vma->vm_start;
	return 0;
}

As we prototyped policies based on this design, we realized that this
approach is not general enough. Adding helpers or verifier code for all
usages would imply a high maintenance cost while severely restricting
the instrumentation capabilities which is the key value add of our
eBPF-based LSM.

Feedback from the BPF maintainers at Linux Plumbers also pushed us
towards the following, more general, approach.

# BTF Based Design

The current design uses BTF
(https://facebookmicrosites.github.io/bpf/blog/2018/11/14/btf-enhancement.html,
https://lwn.net/Articles/803258/) which allows verifiable read-only
structure accesses by field names rather than fixed offsets. This allows
accessing the hook parameters using a dynamically created context which
provides a certain degree of ABI stability:

/* Clang builtin to handle field accesses. */
#define _(P) (__builtin_preserve_access_index(P))

// Only declare the structure and fields intended to be used
// in the program
struct vm_area_struct {
	unsigned long vm_start;
};

// Declare the eBPF program mprotect_audit which attaches to
// to the file_mprotect LSM hook and accepts three arguments.
BPF_TRACE_3("lsm/file_mprotect", mprotect_audit,
	    struct vm_area_struct *, vma,
	    unsigned long, reqprot, unsigned long, prot
{
	unsigned long vm_start = _(vma->vm_start);
	return 0;
}

By relocating field offsets, BTF makes a large portion of kernel data
structures readily accessible across kernel versions without requiring a
large corpus of BPF helper functions and requiring recompilation with
every kernel version. The limitations of BTF compatibility are described
in BPF Co-Re (http://vger.kernel.org/bpfconf2019_talks/bpf-core.pdf,
i.e. field renames, #defines and changes to the signature of LSM hooks).

This design imposes that the MAC policy (eBPF programs) be updated when
the inspected kernel structures change outside of BTF compatibility
guarantees. In practice, this is only required when a structure field
used by a current policy is removed (or renamed) or when the used LSM
hooks change. We expect the maintenance cost of these changes to be
acceptable as compared to the previous design
(https://lore.kernel.org/bpf/20190910115527.5235-1-kpsingh@chromium.org/).

# Distinction from Landlock

We believe there exist two distinct use-cases with distinct set of users:

* Unprivileged processes voluntarily relinquishing privileges with the
  primary users being software developers.

* Flexible privileged (CAP_MAC_ADMIN, CAP_SYS_ADMIN) MAC and Audit with
  the primary users being system policy admins.

These use-cases imply different APIs and trade-offs:

* The unprivileged use case requires defining more stable and custom APIs
  (through opaque contexts and precise helpers).

* Privileged Audit and MAC requires deeper introspection of the kernel
  data structures to maximise the flexibility that can be achieved without
  kernel modification.

Landlock has demonstrated filesystem sandboxes and now Ptrace access
control in its patches which are excellent use cases for an unprivileged
process voluntarily relinquishing privileges.

However, Landlock has expanded its original goal, "towards unprivileged
sandboxing", to being a "low-level framework to build
access-control/audit systems" (https://landlock.io). We feel that the
design and implementation are still driven by the constraints and
trade-offs of the former use-case, and do not provide a satisfactory
solution to the latter.

We also believe that our approach, direct access to common kernel data
structures as with BTF, is inappropriate for unprivileged processes and
probably not a good option for Landlock.

In conclusion, we feel that the design for a privileged LSM and
unprivileged LSM are mutually exclusive and that one cannot be built
"on-top-of" the other. Doing so would limit the capabilities of what can
be done for an LSM that provides flexible audit and MAC capabilities or
provide in-appropriate access to kernel internals to an unprivileged
process.

Furthermore, the Landlock design supports its historical use-case only
when unprivileged eBPF is allowed. This is something that warrants
discussion before an unprivileged LSM that uses eBPF is upstreamed.

# Why not tracepoints or kprobes?

In order to do MAC with tracepoints or kprobes, we would need to
override the return value of the security hook. This is not possible
with tracepoints or call-site kprobes.

Attaching to the return boundary (kretprobe) implies that BPF programs
would always get called after all the other LSM hooks are called and
clobber the pre-existing LSM semantics.

Enforcing MAC policy with an actual LSM helps leverage the verified
semantics of the framework.

# Usage Examples

A simple example and some documentation is included in the patchset.

In order to better illustrate the capabilities of the framework some
more advanced prototype code has also been published separately:

* Logging execution events (including environment variables and arguments):
https://github.com/sinkap/linux-krsi/blob/patch/v1/examples/samples/bpf/lsm_audit_env.c
* Detecting deletion of running executables:
https://github.com/sinkap/linux-krsi/blob/patch/v1/examples/samples/bpf/lsm_detect_exec_unlink.c
* Detection of writes to /proc/<pid>/mem:
https://github.com/sinkap/linux-krsi/blob/patch/v1/examples/samples/bpf/lsm_audit_env.c

We have updated Google's internal telemetry infrastructure and have
started deploying this LSM on our Linux Workstations. This gives us more
confidence in the real-world applications of such a system.

KP Singh (13):
  bpf: Refactor BPF_EVENT context macros to its own header.
  bpf: lsm: Add a skeleton and config options
  bpf: lsm: Introduce types for eBPF based LSM
  bpf: lsm: Allow btf_id based attachment for LSM hooks
  tools/libbpf: Add support in libbpf for BPF_PROG_TYPE_LSM
  bpf: lsm: Init Hooks and create files in securityfs
  bpf: lsm: Implement attach, detach and execution.
  bpf: lsm: Show attached program names in hook read handler.
  bpf: lsm: Add a helper function bpf_lsm_event_output
  bpf: lsm: Handle attachment of the same program
  tools/libbpf: Add bpf_program__attach_lsm
  bpf: lsm: Add selftests for BPF_PROG_TYPE_LSM
  bpf: lsm: Add Documentation

 Documentation/security/bpf.rst                |  164 +++
 Documentation/security/index.rst              |    1 +
 MAINTAINERS                                   |   11 +
 include/linux/bpf_event.h                     |   78 ++
 include/linux/bpf_lsm.h                       |   25 +
 include/linux/bpf_types.h                     |    4 +
 include/trace/bpf_probe.h                     |   30 +-
 include/uapi/linux/bpf.h                      |   12 +-
 kernel/bpf/syscall.c                          |   10 +
 kernel/bpf/verifier.c                         |   84 +-
 kernel/trace/bpf_trace.c                      |   24 +-
 security/Kconfig                              |   11 +-
 security/Makefile                             |    2 +
 security/bpf/Kconfig                          |   25 +
 security/bpf/Makefile                         |    7 +
 security/bpf/include/bpf_lsm.h                |   63 +
 security/bpf/include/fs.h                     |   23 +
 security/bpf/include/hooks.h                  | 1015 +++++++++++++++++
 security/bpf/lsm.c                            |  160 +++
 security/bpf/lsm_fs.c                         |  176 +++
 security/bpf/ops.c                            |  224 ++++
 tools/include/uapi/linux/bpf.h                |   12 +-
 tools/lib/bpf/bpf.c                           |    2 +-
 tools/lib/bpf/bpf.h                           |    6 +
 tools/lib/bpf/libbpf.c                        |  163 ++-
 tools/lib/bpf/libbpf.h                        |    4 +
 tools/lib/bpf/libbpf.map                      |    7 +
 tools/lib/bpf/libbpf_probes.c                 |    1 +
 .../bpf/prog_tests/lsm_mprotect_audit.c       |  129 +++
 .../selftests/bpf/progs/lsm_mprotect_audit.c  |   58 +
 30 files changed, 2451 insertions(+), 80 deletions(-)
 create mode 100644 Documentation/security/bpf.rst
 create mode 100644 include/linux/bpf_event.h
 create mode 100644 include/linux/bpf_lsm.h
 create mode 100644 security/bpf/Kconfig
 create mode 100644 security/bpf/Makefile
 create mode 100644 security/bpf/include/bpf_lsm.h
 create mode 100644 security/bpf/include/fs.h
 create mode 100644 security/bpf/include/hooks.h
 create mode 100644 security/bpf/lsm.c
 create mode 100644 security/bpf/lsm_fs.c
 create mode 100644 security/bpf/ops.c
 create mode 100644 tools/testing/selftests/bpf/prog_tests/lsm_mprotect_audit.c
 create mode 100644 tools/testing/selftests/bpf/progs/lsm_mprotect_audit.c

Comments

Casey Schaufler Dec. 20, 2019, 5:17 p.m. UTC | #1
On 12/20/2019 7:41 AM, KP Singh wrote:
> From: KP Singh <kpsingh@google.com>
>
> This patch series is a continuation of the KRSI RFC
> (https://lore.kernel.org/bpf/20190910115527.5235-1-kpsingh@chromium.org/)
>
> # Motivation
>
> Google does rich analysis of runtime security data collected from
> internal Linux deployments (corporate devices and servers) to detect and
> thwart threats in real-time. Currently, this is done in custom kernel
> modules but we would like to replace this with something that's upstream
> and useful to others.
>
> The current kernel infrastructure for providing telemetry (Audit, Perf
> etc.) is disjoint from access enforcement (i.e. LSMs).  Augmenting the
> information provided by audit requires kernel changes to audit, its
> policy language and user-space components. Furthermore, building a MAC
> policy based on the newly added telemetry data requires changes to
> various LSMs and their respective policy languages.
>
> This patchset proposes a new stackable and privileged LSM which allows
> the LSM hooks to be implemented using eBPF. This facilitates a unified
> and dynamic (not requiring re-compilation of the kernel) audit and MAC
> policy.
>
> # Why an LSM?
>
> Linux Security Modules target security behaviours rather than the
> kernel's API. For example, it's easy to miss out a newly added system
> call for executing processes (eg. execve, execveat etc.) but the LSM
> framework ensures that all process executions trigger the relevant hooks
> irrespective of how the process was executed.
>
> Allowing users to implement LSM hooks at runtime also benefits the LSM
> eco-system by enabling a quick feedback loop from the security community
> about the kind of behaviours that the LSM Framework should be targeting.
>
> # How does it work?
>
> The LSM introduces a new eBPF (https://docs.cilium.io/en/v1.6/bpf/)
> program type, BPF_PROG_TYPE_LSM, which can only be attached to a LSM
> hook.  All LSM hooks are exposed as files in securityfs. Attachment
> requires CAP_SYS_ADMIN for loading eBPF programs and CAP_MAC_ADMIN for
> modifying MAC policies.
>
> The eBPF programs are passed the same arguments as the LSM hooks and
> executed in the body of the hook.

This effectively exposes the LSM hooks as external APIs.
It would mean that we can't change or delete them. That
would be bad.


>  If any of the eBPF programs returns an
> error (like ENOPERM), the behaviour represented by the hook is denied.
>
> Audit logs can be written using a format chosen by the eBPF program to
> the perf events buffer and can be further processed in user-space.
>
> # Limitations of RFC v1
>
> In the previous design
> (https://lore.kernel.org/bpf/20190910115527.5235-1-kpsingh@chromium.org/),
> the BPF programs received a context which could be queried to retrieve
> specific pieces of information using specific helpers.
>
> For example, a program that attaches to the file_mprotect LSM hook and
> queries the VMA region could have had the following context:
>
> // Special context for the hook.
> struct bpf_mprotect_ctx {
> 	struct vm_area_struct *vma;
> };
>
> and accessed the fields using a hypothetical helper
> "bpf_mprotect_vma_get_start:
>
> SEC("lsm/file_mprotect")
> int mprotect_audit(bpf_mprotect_ctx *ctx)
> {
> 	unsigned long vm_start = bpf_mprotect_vma_get_start(ctx);
> 	return 0;
> }
>
> or directly read them from the context by updating the verifier to allow
> accessing the fields:
>
> int mprotect_audit(bpf_mprotect_ctx *ctx)
> {
> 	unsigned long vm_start = ctx->vma->vm_start;
> 	return 0;
> }
>
> As we prototyped policies based on this design, we realized that this
> approach is not general enough. Adding helpers or verifier code for all
> usages would imply a high maintenance cost while severely restricting
> the instrumentation capabilities which is the key value add of our
> eBPF-based LSM.
>
> Feedback from the BPF maintainers at Linux Plumbers also pushed us
> towards the following, more general, approach.
>
> # BTF Based Design
>
> The current design uses BTF
> (https://facebookmicrosites.github.io/bpf/blog/2018/11/14/btf-enhancement.html,
> https://lwn.net/Articles/803258/) which allows verifiable read-only
> structure accesses by field names rather than fixed offsets. This allows
> accessing the hook parameters using a dynamically created context which
> provides a certain degree of ABI stability:
>
> /* Clang builtin to handle field accesses. */
> #define _(P) (__builtin_preserve_access_index(P))
>
> // Only declare the structure and fields intended to be used
> // in the program
> struct vm_area_struct {
> 	unsigned long vm_start;
> };
>
> // Declare the eBPF program mprotect_audit which attaches to
> // to the file_mprotect LSM hook and accepts three arguments.
> BPF_TRACE_3("lsm/file_mprotect", mprotect_audit,
> 	    struct vm_area_struct *, vma,
> 	    unsigned long, reqprot, unsigned long, prot
> {
> 	unsigned long vm_start = _(vma->vm_start);
> 	return 0;
> }
>
> By relocating field offsets, BTF makes a large portion of kernel data
> structures readily accessible across kernel versions without requiring a
> large corpus of BPF helper functions and requiring recompilation with
> every kernel version. The limitations of BTF compatibility are described
> in BPF Co-Re (http://vger.kernel.org/bpfconf2019_talks/bpf-core.pdf,
> i.e. field renames, #defines and changes to the signature of LSM hooks).
>
> This design imposes that the MAC policy (eBPF programs) be updated when
> the inspected kernel structures change outside of BTF compatibility
> guarantees. In practice, this is only required when a structure field
> used by a current policy is removed (or renamed) or when the used LSM
> hooks change. We expect the maintenance cost of these changes to be
> acceptable as compared to the previous design
> (https://lore.kernel.org/bpf/20190910115527.5235-1-kpsingh@chromium.org/).
>
> # Distinction from Landlock
>
> We believe there exist two distinct use-cases with distinct set of users:
>
> * Unprivileged processes voluntarily relinquishing privileges with the
>   primary users being software developers.
>
> * Flexible privileged (CAP_MAC_ADMIN, CAP_SYS_ADMIN) MAC and Audit with
>   the primary users being system policy admins.
>
> These use-cases imply different APIs and trade-offs:
>
> * The unprivileged use case requires defining more stable and custom APIs
>   (through opaque contexts and precise helpers).
>
> * Privileged Audit and MAC requires deeper introspection of the kernel
>   data structures to maximise the flexibility that can be achieved without
>   kernel modification.
>
> Landlock has demonstrated filesystem sandboxes and now Ptrace access
> control in its patches which are excellent use cases for an unprivileged
> process voluntarily relinquishing privileges.
>
> However, Landlock has expanded its original goal, "towards unprivileged
> sandboxing", to being a "low-level framework to build
> access-control/audit systems" (https://landlock.io). We feel that the
> design and implementation are still driven by the constraints and
> trade-offs of the former use-case, and do not provide a satisfactory
> solution to the latter.
>
> We also believe that our approach, direct access to common kernel data
> structures as with BTF, is inappropriate for unprivileged processes and
> probably not a good option for Landlock.
>
> In conclusion, we feel that the design for a privileged LSM and
> unprivileged LSM are mutually exclusive and that one cannot be built
> "on-top-of" the other. Doing so would limit the capabilities of what can
> be done for an LSM that provides flexible audit and MAC capabilities or
> provide in-appropriate access to kernel internals to an unprivileged
> process.
>
> Furthermore, the Landlock design supports its historical use-case only
> when unprivileged eBPF is allowed. This is something that warrants
> discussion before an unprivileged LSM that uses eBPF is upstreamed.
>
> # Why not tracepoints or kprobes?
>
> In order to do MAC with tracepoints or kprobes, we would need to
> override the return value of the security hook. This is not possible
> with tracepoints or call-site kprobes.
>
> Attaching to the return boundary (kretprobe) implies that BPF programs
> would always get called after all the other LSM hooks are called and
> clobber the pre-existing LSM semantics.
>
> Enforcing MAC policy with an actual LSM helps leverage the verified
> semantics of the framework.
>
> # Usage Examples
>
> A simple example and some documentation is included in the patchset.
>
> In order to better illustrate the capabilities of the framework some
> more advanced prototype code has also been published separately:
>
> * Logging execution events (including environment variables and arguments):
> https://github.com/sinkap/linux-krsi/blob/patch/v1/examples/samples/bpf/lsm_audit_env.c
> * Detecting deletion of running executables:
> https://github.com/sinkap/linux-krsi/blob/patch/v1/examples/samples/bpf/lsm_detect_exec_unlink.c
> * Detection of writes to /proc/<pid>/mem:
> https://github.com/sinkap/linux-krsi/blob/patch/v1/examples/samples/bpf/lsm_audit_env.c
>
> We have updated Google's internal telemetry infrastructure and have
> started deploying this LSM on our Linux Workstations. This gives us more
> confidence in the real-world applications of such a system.
>
> KP Singh (13):
>   bpf: Refactor BPF_EVENT context macros to its own header.
>   bpf: lsm: Add a skeleton and config options
>   bpf: lsm: Introduce types for eBPF based LSM
>   bpf: lsm: Allow btf_id based attachment for LSM hooks
>   tools/libbpf: Add support in libbpf for BPF_PROG_TYPE_LSM
>   bpf: lsm: Init Hooks and create files in securityfs
>   bpf: lsm: Implement attach, detach and execution.
>   bpf: lsm: Show attached program names in hook read handler.
>   bpf: lsm: Add a helper function bpf_lsm_event_output
>   bpf: lsm: Handle attachment of the same program
>   tools/libbpf: Add bpf_program__attach_lsm
>   bpf: lsm: Add selftests for BPF_PROG_TYPE_LSM
>   bpf: lsm: Add Documentation
>
>  Documentation/security/bpf.rst                |  164 +++
>  Documentation/security/index.rst              |    1 +
>  MAINTAINERS                                   |   11 +
>  include/linux/bpf_event.h                     |   78 ++
>  include/linux/bpf_lsm.h                       |   25 +
>  include/linux/bpf_types.h                     |    4 +
>  include/trace/bpf_probe.h                     |   30 +-
>  include/uapi/linux/bpf.h                      |   12 +-
>  kernel/bpf/syscall.c                          |   10 +
>  kernel/bpf/verifier.c                         |   84 +-
>  kernel/trace/bpf_trace.c                      |   24 +-
>  security/Kconfig                              |   11 +-
>  security/Makefile                             |    2 +
>  security/bpf/Kconfig                          |   25 +
>  security/bpf/Makefile                         |    7 +
>  security/bpf/include/bpf_lsm.h                |   63 +
>  security/bpf/include/fs.h                     |   23 +
>  security/bpf/include/hooks.h                  | 1015 +++++++++++++++++
>  security/bpf/lsm.c                            |  160 +++
>  security/bpf/lsm_fs.c                         |  176 +++
>  security/bpf/ops.c                            |  224 ++++
>  tools/include/uapi/linux/bpf.h                |   12 +-
>  tools/lib/bpf/bpf.c                           |    2 +-
>  tools/lib/bpf/bpf.h                           |    6 +
>  tools/lib/bpf/libbpf.c                        |  163 ++-
>  tools/lib/bpf/libbpf.h                        |    4 +
>  tools/lib/bpf/libbpf.map                      |    7 +
>  tools/lib/bpf/libbpf_probes.c                 |    1 +
>  .../bpf/prog_tests/lsm_mprotect_audit.c       |  129 +++
>  .../selftests/bpf/progs/lsm_mprotect_audit.c  |   58 +
>  30 files changed, 2451 insertions(+), 80 deletions(-)
>  create mode 100644 Documentation/security/bpf.rst
>  create mode 100644 include/linux/bpf_event.h
>  create mode 100644 include/linux/bpf_lsm.h
>  create mode 100644 security/bpf/Kconfig
>  create mode 100644 security/bpf/Makefile
>  create mode 100644 security/bpf/include/bpf_lsm.h
>  create mode 100644 security/bpf/include/fs.h
>  create mode 100644 security/bpf/include/hooks.h
>  create mode 100644 security/bpf/lsm.c
>  create mode 100644 security/bpf/lsm_fs.c
>  create mode 100644 security/bpf/ops.c
>  create mode 100644 tools/testing/selftests/bpf/prog_tests/lsm_mprotect_audit.c
>  create mode 100644 tools/testing/selftests/bpf/progs/lsm_mprotect_audit.c
>
KP Singh Dec. 20, 2019, 5:38 p.m. UTC | #2
Hi Casey,

Thanks for taking a look!

On Fri, Dec 20, 2019 at 6:17 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>
> On 12/20/2019 7:41 AM, KP Singh wrote:
> > From: KP Singh <kpsingh@google.com>
> >
> > This patch series is a continuation of the KRSI RFC
> > (https://lore.kernel.org/bpf/20190910115527.5235-1-kpsingh@chromium.org/)
> >
> > # Motivation
> >
> > Google does rich analysis of runtime security data collected from
> > internal Linux deployments (corporate devices and servers) to detect and
> > thwart threats in real-time. Currently, this is done in custom kernel
> > modules but we would like to replace this with something that's upstream
> > and useful to others.
> >
> > The current kernel infrastructure for providing telemetry (Audit, Perf
> > etc.) is disjoint from access enforcement (i.e. LSMs).  Augmenting the
> > information provided by audit requires kernel changes to audit, its
> > policy language and user-space components. Furthermore, building a MAC
> > policy based on the newly added telemetry data requires changes to
> > various LSMs and their respective policy languages.
> >
> > This patchset proposes a new stackable and privileged LSM which allows
> > the LSM hooks to be implemented using eBPF. This facilitates a unified
> > and dynamic (not requiring re-compilation of the kernel) audit and MAC
> > policy.
> >
> > # Why an LSM?
> >
> > Linux Security Modules target security behaviours rather than the
> > kernel's API. For example, it's easy to miss out a newly added system
> > call for executing processes (eg. execve, execveat etc.) but the LSM
> > framework ensures that all process executions trigger the relevant hooks
> > irrespective of how the process was executed.
> >
> > Allowing users to implement LSM hooks at runtime also benefits the LSM
> > eco-system by enabling a quick feedback loop from the security community
> > about the kind of behaviours that the LSM Framework should be targeting.
> >
> > # How does it work?
> >
> > The LSM introduces a new eBPF (https://docs.cilium.io/en/v1.6/bpf/)
> > program type, BPF_PROG_TYPE_LSM, which can only be attached to a LSM
> > hook.  All LSM hooks are exposed as files in securityfs. Attachment
> > requires CAP_SYS_ADMIN for loading eBPF programs and CAP_MAC_ADMIN for
> > modifying MAC policies.
> >
> > The eBPF programs are passed the same arguments as the LSM hooks and
> > executed in the body of the hook.
>
> This effectively exposes the LSM hooks as external APIs.
> It would mean that we can't change or delete them. That
> would be bad.

Perhaps this should have been clearer, we *do not* want to make LSM hooks
a stable API and expect the eBPF programs to adapt when such changes occur.

Based on our comparison with the previous approach, this still ends up
being a better trade-off (w.r.t. maintenance) when compared to adding
specific helpers or verifier logic for  each new hook or field that
needs to be exposed.

- KP

>
>
> >  If any of the eBPF programs returns an
> > error (like ENOPERM), the behaviour represented by the hook is denied.
> >
> > Audit logs can be written using a format chosen by the eBPF program to
> > the perf events buffer and can be further processed in user-space.
> >
> > # Limitations of RFC v1
> >
> > In the previous design
> > (https://lore.kernel.org/bpf/20190910115527.5235-1-kpsingh@chromium.org/),
> > the BPF programs received a context which could be queried to retrieve
> > specific pieces of information using specific helpers.
> >
> > For example, a program that attaches to the file_mprotect LSM hook and
> > queries the VMA region could have had the following context:
> >
> > // Special context for the hook.
> > struct bpf_mprotect_ctx {
> >       struct vm_area_struct *vma;
> > };
> >
> > and accessed the fields using a hypothetical helper
> > "bpf_mprotect_vma_get_start:
> >
> > SEC("lsm/file_mprotect")
> > int mprotect_audit(bpf_mprotect_ctx *ctx)
> > {
> >       unsigned long vm_start = bpf_mprotect_vma_get_start(ctx);
> >       return 0;
> > }
> >
> > or directly read them from the context by updating the verifier to allow
> > accessing the fields:
> >
> > int mprotect_audit(bpf_mprotect_ctx *ctx)
> > {
> >       unsigned long vm_start = ctx->vma->vm_start;
> >       return 0;
> > }
> >
> > As we prototyped policies based on this design, we realized that this
> > approach is not general enough. Adding helpers or verifier code for all
> > usages would imply a high maintenance cost while severely restricting
> > the instrumentation capabilities which is the key value add of our
> > eBPF-based LSM.
> >
> > Feedback from the BPF maintainers at Linux Plumbers also pushed us
> > towards the following, more general, approach.
> >
> > # BTF Based Design
> >
> > The current design uses BTF
> > (https://facebookmicrosites.github.io/bpf/blog/2018/11/14/btf-enhancement.html,
> > https://lwn.net/Articles/803258/) which allows verifiable read-only
> > structure accesses by field names rather than fixed offsets. This allows
> > accessing the hook parameters using a dynamically created context which
> > provides a certain degree of ABI stability:
> >
> > /* Clang builtin to handle field accesses. */
> > #define _(P) (__builtin_preserve_access_index(P))
> >
> > // Only declare the structure and fields intended to be used
> > // in the program
> > struct vm_area_struct {
> >       unsigned long vm_start;
> > };
> >
> > // Declare the eBPF program mprotect_audit which attaches to
> > // to the file_mprotect LSM hook and accepts three arguments.
> > BPF_TRACE_3("lsm/file_mprotect", mprotect_audit,
> >           struct vm_area_struct *, vma,
> >           unsigned long, reqprot, unsigned long, prot
> > {
> >       unsigned long vm_start = _(vma->vm_start);
> >       return 0;
> > }
> >
> > By relocating field offsets, BTF makes a large portion of kernel data
> > structures readily accessible across kernel versions without requiring a
> > large corpus of BPF helper functions and requiring recompilation with
> > every kernel version. The limitations of BTF compatibility are described
> > in BPF Co-Re (http://vger.kernel.org/bpfconf2019_talks/bpf-core.pdf,
> > i.e. field renames, #defines and changes to the signature of LSM hooks).
> >
> > This design imposes that the MAC policy (eBPF programs) be updated when
> > the inspected kernel structures change outside of BTF compatibility
> > guarantees. In practice, this is only required when a structure field
> > used by a current policy is removed (or renamed) or when the used LSM
> > hooks change. We expect the maintenance cost of these changes to be
> > acceptable as compared to the previous design
> > (https://lore.kernel.org/bpf/20190910115527.5235-1-kpsingh@chromium.org/).
> >
> > # Distinction from Landlock
> >
> > We believe there exist two distinct use-cases with distinct set of users:
> >
> > * Unprivileged processes voluntarily relinquishing privileges with the
> >   primary users being software developers.
> >
> > * Flexible privileged (CAP_MAC_ADMIN, CAP_SYS_ADMIN) MAC and Audit with
> >   the primary users being system policy admins.
> >
> > These use-cases imply different APIs and trade-offs:
> >
> > * The unprivileged use case requires defining more stable and custom APIs
> >   (through opaque contexts and precise helpers).
> >
> > * Privileged Audit and MAC requires deeper introspection of the kernel
> >   data structures to maximise the flexibility that can be achieved without
> >   kernel modification.
> >
> > Landlock has demonstrated filesystem sandboxes and now Ptrace access
> > control in its patches which are excellent use cases for an unprivileged
> > process voluntarily relinquishing privileges.
> >
> > However, Landlock has expanded its original goal, "towards unprivileged
> > sandboxing", to being a "low-level framework to build
> > access-control/audit systems" (https://landlock.io). We feel that the
> > design and implementation are still driven by the constraints and
> > trade-offs of the former use-case, and do not provide a satisfactory
> > solution to the latter.
> >
> > We also believe that our approach, direct access to common kernel data
> > structures as with BTF, is inappropriate for unprivileged processes and
> > probably not a good option for Landlock.
> >
> > In conclusion, we feel that the design for a privileged LSM and
> > unprivileged LSM are mutually exclusive and that one cannot be built
> > "on-top-of" the other. Doing so would limit the capabilities of what can
> > be done for an LSM that provides flexible audit and MAC capabilities or
> > provide in-appropriate access to kernel internals to an unprivileged
> > process.
> >
> > Furthermore, the Landlock design supports its historical use-case only
> > when unprivileged eBPF is allowed. This is something that warrants
> > discussion before an unprivileged LSM that uses eBPF is upstreamed.
> >
> > # Why not tracepoints or kprobes?
> >
> > In order to do MAC with tracepoints or kprobes, we would need to
> > override the return value of the security hook. This is not possible
> > with tracepoints or call-site kprobes.
> >
> > Attaching to the return boundary (kretprobe) implies that BPF programs
> > would always get called after all the other LSM hooks are called and
> > clobber the pre-existing LSM semantics.
> >
> > Enforcing MAC policy with an actual LSM helps leverage the verified
> > semantics of the framework.
> >
> > # Usage Examples
> >
> > A simple example and some documentation is included in the patchset.
> >
> > In order to better illustrate the capabilities of the framework some
> > more advanced prototype code has also been published separately:
> >
> > * Logging execution events (including environment variables and arguments):
> > https://github.com/sinkap/linux-krsi/blob/patch/v1/examples/samples/bpf/lsm_audit_env.c
> > * Detecting deletion of running executables:
> > https://github.com/sinkap/linux-krsi/blob/patch/v1/examples/samples/bpf/lsm_detect_exec_unlink.c
> > * Detection of writes to /proc/<pid>/mem:
> > https://github.com/sinkap/linux-krsi/blob/patch/v1/examples/samples/bpf/lsm_audit_env.c
> >
> > We have updated Google's internal telemetry infrastructure and have
> > started deploying this LSM on our Linux Workstations. This gives us more
> > confidence in the real-world applications of such a system.
> >
> > KP Singh (13):
> >   bpf: Refactor BPF_EVENT context macros to its own header.
> >   bpf: lsm: Add a skeleton and config options
> >   bpf: lsm: Introduce types for eBPF based LSM
> >   bpf: lsm: Allow btf_id based attachment for LSM hooks
> >   tools/libbpf: Add support in libbpf for BPF_PROG_TYPE_LSM
> >   bpf: lsm: Init Hooks and create files in securityfs
> >   bpf: lsm: Implement attach, detach and execution.
> >   bpf: lsm: Show attached program names in hook read handler.
> >   bpf: lsm: Add a helper function bpf_lsm_event_output
> >   bpf: lsm: Handle attachment of the same program
> >   tools/libbpf: Add bpf_program__attach_lsm
> >   bpf: lsm: Add selftests for BPF_PROG_TYPE_LSM
> >   bpf: lsm: Add Documentation
> >
> >  Documentation/security/bpf.rst                |  164 +++
> >  Documentation/security/index.rst              |    1 +
> >  MAINTAINERS                                   |   11 +
> >  include/linux/bpf_event.h                     |   78 ++
> >  include/linux/bpf_lsm.h                       |   25 +
> >  include/linux/bpf_types.h                     |    4 +
> >  include/trace/bpf_probe.h                     |   30 +-
> >  include/uapi/linux/bpf.h                      |   12 +-
> >  kernel/bpf/syscall.c                          |   10 +
> >  kernel/bpf/verifier.c                         |   84 +-
> >  kernel/trace/bpf_trace.c                      |   24 +-
> >  security/Kconfig                              |   11 +-
> >  security/Makefile                             |    2 +
> >  security/bpf/Kconfig                          |   25 +
> >  security/bpf/Makefile                         |    7 +
> >  security/bpf/include/bpf_lsm.h                |   63 +
> >  security/bpf/include/fs.h                     |   23 +
> >  security/bpf/include/hooks.h                  | 1015 +++++++++++++++++
> >  security/bpf/lsm.c                            |  160 +++
> >  security/bpf/lsm_fs.c                         |  176 +++
> >  security/bpf/ops.c                            |  224 ++++
> >  tools/include/uapi/linux/bpf.h                |   12 +-
> >  tools/lib/bpf/bpf.c                           |    2 +-
> >  tools/lib/bpf/bpf.h                           |    6 +
> >  tools/lib/bpf/libbpf.c                        |  163 ++-
> >  tools/lib/bpf/libbpf.h                        |    4 +
> >  tools/lib/bpf/libbpf.map                      |    7 +
> >  tools/lib/bpf/libbpf_probes.c                 |    1 +
> >  .../bpf/prog_tests/lsm_mprotect_audit.c       |  129 +++
> >  .../selftests/bpf/progs/lsm_mprotect_audit.c  |   58 +
> >  30 files changed, 2451 insertions(+), 80 deletions(-)
> >  create mode 100644 Documentation/security/bpf.rst
> >  create mode 100644 include/linux/bpf_event.h
> >  create mode 100644 include/linux/bpf_lsm.h
> >  create mode 100644 security/bpf/Kconfig
> >  create mode 100644 security/bpf/Makefile
> >  create mode 100644 security/bpf/include/bpf_lsm.h
> >  create mode 100644 security/bpf/include/fs.h
> >  create mode 100644 security/bpf/include/hooks.h
> >  create mode 100644 security/bpf/lsm.c
> >  create mode 100644 security/bpf/lsm_fs.c
> >  create mode 100644 security/bpf/ops.c
> >  create mode 100644 tools/testing/selftests/bpf/prog_tests/lsm_mprotect_audit.c
> >  create mode 100644 tools/testing/selftests/bpf/progs/lsm_mprotect_audit.c
> >
>
Mickaël Salaün Dec. 20, 2019, 10:46 p.m. UTC | #3
On 20/12/2019 16:41, KP Singh wrote:
> From: KP Singh <kpsingh@google.com>
> 
> This patch series is a continuation of the KRSI RFC
> (https://lore.kernel.org/bpf/20190910115527.5235-1-kpsingh@chromium.org/)
> 
> # Motivation
> 
> Google does rich analysis of runtime security data collected from
> internal Linux deployments (corporate devices and servers) to detect and
> thwart threats in real-time. Currently, this is done in custom kernel
> modules but we would like to replace this with something that's upstream
> and useful to others.
> 
> The current kernel infrastructure for providing telemetry (Audit, Perf
> etc.) is disjoint from access enforcement (i.e. LSMs).  Augmenting the
> information provided by audit requires kernel changes to audit, its
> policy language and user-space components. Furthermore, building a MAC
> policy based on the newly added telemetry data requires changes to
> various LSMs and their respective policy languages.
> 
> This patchset proposes a new stackable and privileged LSM which allows
> the LSM hooks to be implemented using eBPF. This facilitates a unified
> and dynamic (not requiring re-compilation of the kernel) audit and MAC
> policy.
> 
> # Why an LSM?
> 
> Linux Security Modules target security behaviours rather than the
> kernel's API. For example, it's easy to miss out a newly added system
> call for executing processes (eg. execve, execveat etc.) but the LSM
> framework ensures that all process executions trigger the relevant hooks
> irrespective of how the process was executed.
> 
> Allowing users to implement LSM hooks at runtime also benefits the LSM
> eco-system by enabling a quick feedback loop from the security community
> about the kind of behaviours that the LSM Framework should be targeting.
> 
> # How does it work?
> 
> The LSM introduces a new eBPF (https://docs.cilium.io/en/v1.6/bpf/)
> program type, BPF_PROG_TYPE_LSM, which can only be attached to a LSM
> hook.  All LSM hooks are exposed as files in securityfs. Attachment
> requires CAP_SYS_ADMIN for loading eBPF programs and CAP_MAC_ADMIN for
> modifying MAC policies.
> 
> The eBPF programs are passed the same arguments as the LSM hooks and
> executed in the body of the hook. If any of the eBPF programs returns an
> error (like ENOPERM), the behaviour represented by the hook is denied.
> 
> Audit logs can be written using a format chosen by the eBPF program to
> the perf events buffer and can be further processed in user-space.
> 
> # Limitations of RFC v1
> 
> In the previous design
> (https://lore.kernel.org/bpf/20190910115527.5235-1-kpsingh@chromium.org/),
> the BPF programs received a context which could be queried to retrieve
> specific pieces of information using specific helpers.
> 
> For example, a program that attaches to the file_mprotect LSM hook and
> queries the VMA region could have had the following context:
> 
> // Special context for the hook.
> struct bpf_mprotect_ctx {
> 	struct vm_area_struct *vma;
> };
> 
> and accessed the fields using a hypothetical helper
> "bpf_mprotect_vma_get_start:
> 
> SEC("lsm/file_mprotect")
> int mprotect_audit(bpf_mprotect_ctx *ctx)
> {
> 	unsigned long vm_start = bpf_mprotect_vma_get_start(ctx);
> 	return 0;
> }
> 
> or directly read them from the context by updating the verifier to allow
> accessing the fields:
> 
> int mprotect_audit(bpf_mprotect_ctx *ctx)
> {
> 	unsigned long vm_start = ctx->vma->vm_start;
> 	return 0;
> }
> 
> As we prototyped policies based on this design, we realized that this
> approach is not general enough. Adding helpers or verifier code for all
> usages would imply a high maintenance cost while severely restricting
> the instrumentation capabilities which is the key value add of our
> eBPF-based LSM.
> 
> Feedback from the BPF maintainers at Linux Plumbers also pushed us
> towards the following, more general, approach.
> 
> # BTF Based Design
> 
> The current design uses BTF
> (https://facebookmicrosites.github.io/bpf/blog/2018/11/14/btf-enhancement.html,
> https://lwn.net/Articles/803258/) which allows verifiable read-only
> structure accesses by field names rather than fixed offsets. This allows
> accessing the hook parameters using a dynamically created context which
> provides a certain degree of ABI stability:
> 
> /* Clang builtin to handle field accesses. */
> #define _(P) (__builtin_preserve_access_index(P))
> 
> // Only declare the structure and fields intended to be used
> // in the program
> struct vm_area_struct {
> 	unsigned long vm_start;
> };
> 
> // Declare the eBPF program mprotect_audit which attaches to
> // to the file_mprotect LSM hook and accepts three arguments.
> BPF_TRACE_3("lsm/file_mprotect", mprotect_audit,
> 	    struct vm_area_struct *, vma,
> 	    unsigned long, reqprot, unsigned long, prot
> {
> 	unsigned long vm_start = _(vma->vm_start);
> 	return 0;
> }
> 
> By relocating field offsets, BTF makes a large portion of kernel data
> structures readily accessible across kernel versions without requiring a
> large corpus of BPF helper functions and requiring recompilation with
> every kernel version. The limitations of BTF compatibility are described
> in BPF Co-Re (http://vger.kernel.org/bpfconf2019_talks/bpf-core.pdf,
> i.e. field renames, #defines and changes to the signature of LSM hooks).
> 
> This design imposes that the MAC policy (eBPF programs) be updated when
> the inspected kernel structures change outside of BTF compatibility
> guarantees. In practice, this is only required when a structure field
> used by a current policy is removed (or renamed) or when the used LSM
> hooks change. We expect the maintenance cost of these changes to be
> acceptable as compared to the previous design
> (https://lore.kernel.org/bpf/20190910115527.5235-1-kpsingh@chromium.org/).
> 
> # Distinction from Landlock
> 
> We believe there exist two distinct use-cases with distinct set of users:
> 
> * Unprivileged processes voluntarily relinquishing privileges with the
>    primary users being software developers.
> 
> * Flexible privileged (CAP_MAC_ADMIN, CAP_SYS_ADMIN) MAC and Audit with
>    the primary users being system policy admins.
> 
> These use-cases imply different APIs and trade-offs:
> 
> * The unprivileged use case requires defining more stable and custom APIs
>    (through opaque contexts and precise helpers).
> 
> * Privileged Audit and MAC requires deeper introspection of the kernel
>    data structures to maximise the flexibility that can be achieved without
>    kernel modification.
> 
> Landlock has demonstrated filesystem sandboxes and now Ptrace access
> control in its patches which are excellent use cases for an unprivileged
> process voluntarily relinquishing privileges.
> 
> However, Landlock has expanded its original goal, "towards unprivileged
> sandboxing", to being a "low-level framework to build
> access-control/audit systems" (https://landlock.io). We feel that the
> design and implementation are still driven by the constraints and
> trade-offs of the former use-case, and do not provide a satisfactory
> solution to the latter.
> 
> We also believe that our approach, direct access to common kernel data
> structures as with BTF, is inappropriate for unprivileged processes and
> probably not a good option for Landlock.
> 
> In conclusion, we feel that the design for a privileged LSM and
> unprivileged LSM are mutually exclusive and that one cannot be built
> "on-top-of" the other. Doing so would limit the capabilities of what can
> be done for an LSM that provides flexible audit and MAC capabilities or
> provide in-appropriate access to kernel internals to an unprivileged
> process.

I don't think that the design for a privileged LSM and an unprivileged 
LSM are necessarily mutually exclusive, however I do agree that the 
design of an *introspection* LSM like this version of KRSI, and an 
unprivileged LSM are mutually exclusive.

> 
> Furthermore, the Landlock design supports its historical use-case only
> when unprivileged eBPF is allowed. This is something that warrants
> discussion before an unprivileged LSM that uses eBPF is upstreamed.

Because of seccomp-bpf, on which the first version of Landlock was based 
on, I then used eBPF as a way to define a security policy which could be 
updated on the fly (thanks to eBPF maps) and evolves over time. The main 
goal of Landlock was and still is to bring sandboxing features to all 
users, which means to have an unprivileged access-control. However, I've 
reach a similar conclusion about eBPF for unprivileged access-control, 
but for slightly different reasons.

eBPF is very powerful and I proved with Landlock that it is possible to 
implement an access-control with it. However, a programmatic 
access-control does not fit well with unprivileged principles (i.e. 
innocuous composability). First, it can be used for side-channel attacks 
against (other) access-controls. Second, composability of eBPF programs 
imply to execute a stack of programs, which does not scale well. Third, 
as shown by multiple attacks, it is quite challenging to safely expose 
eBPF to malicious processes.

I'm working on a version of Landlock without eBPF, but still with the 
initial sought properties: safe unprivileged composability, modularity, 
and dynamic update. I'll send this version soon.

I hope that the work and experience from Landlock to bring eBPF to LSM 
will continue to be used through KRSI. Landlock will now focus on the 
unprivileged sandboxing part, without eBPF. Stay tuned!


> 
> # Why not tracepoints or kprobes?
> 
> In order to do MAC with tracepoints or kprobes, we would need to
> override the return value of the security hook. This is not possible
> with tracepoints or call-site kprobes.
> 
> Attaching to the return boundary (kretprobe) implies that BPF programs
> would always get called after all the other LSM hooks are called and
> clobber the pre-existing LSM semantics.
> 
> Enforcing MAC policy with an actual LSM helps leverage the verified
> semantics of the framework.
> 
> # Usage Examples
> 
> A simple example and some documentation is included in the patchset.
> 
> In order to better illustrate the capabilities of the framework some
> more advanced prototype code has also been published separately:
> 
> * Logging execution events (including environment variables and arguments):
> https://github.com/sinkap/linux-krsi/blob/patch/v1/examples/samples/bpf/lsm_audit_env.c
> * Detecting deletion of running executables:
> https://github.com/sinkap/linux-krsi/blob/patch/v1/examples/samples/bpf/lsm_detect_exec_unlink.c
> * Detection of writes to /proc/<pid>/mem:
> https://github.com/sinkap/linux-krsi/blob/patch/v1/examples/samples/bpf/lsm_audit_env.c
> 
> We have updated Google's internal telemetry infrastructure and have
> started deploying this LSM on our Linux Workstations. This gives us more
> confidence in the real-world applications of such a system.
> 
> KP Singh (13):
>    bpf: Refactor BPF_EVENT context macros to its own header.
>    bpf: lsm: Add a skeleton and config options
>    bpf: lsm: Introduce types for eBPF based LSM
>    bpf: lsm: Allow btf_id based attachment for LSM hooks
>    tools/libbpf: Add support in libbpf for BPF_PROG_TYPE_LSM
>    bpf: lsm: Init Hooks and create files in securityfs
>    bpf: lsm: Implement attach, detach and execution.
>    bpf: lsm: Show attached program names in hook read handler.
>    bpf: lsm: Add a helper function bpf_lsm_event_output
>    bpf: lsm: Handle attachment of the same program
>    tools/libbpf: Add bpf_program__attach_lsm
>    bpf: lsm: Add selftests for BPF_PROG_TYPE_LSM
>    bpf: lsm: Add Documentation
> 
>   Documentation/security/bpf.rst                |  164 +++
>   Documentation/security/index.rst              |    1 +
>   MAINTAINERS                                   |   11 +
>   include/linux/bpf_event.h                     |   78 ++
>   include/linux/bpf_lsm.h                       |   25 +
>   include/linux/bpf_types.h                     |    4 +
>   include/trace/bpf_probe.h                     |   30 +-
>   include/uapi/linux/bpf.h                      |   12 +-
>   kernel/bpf/syscall.c                          |   10 +
>   kernel/bpf/verifier.c                         |   84 +-
>   kernel/trace/bpf_trace.c                      |   24 +-
>   security/Kconfig                              |   11 +-
>   security/Makefile                             |    2 +
>   security/bpf/Kconfig                          |   25 +
>   security/bpf/Makefile                         |    7 +
>   security/bpf/include/bpf_lsm.h                |   63 +
>   security/bpf/include/fs.h                     |   23 +
>   security/bpf/include/hooks.h                  | 1015 +++++++++++++++++
>   security/bpf/lsm.c                            |  160 +++
>   security/bpf/lsm_fs.c                         |  176 +++
>   security/bpf/ops.c                            |  224 ++++
>   tools/include/uapi/linux/bpf.h                |   12 +-
>   tools/lib/bpf/bpf.c                           |    2 +-
>   tools/lib/bpf/bpf.h                           |    6 +
>   tools/lib/bpf/libbpf.c                        |  163 ++-
>   tools/lib/bpf/libbpf.h                        |    4 +
>   tools/lib/bpf/libbpf.map                      |    7 +
>   tools/lib/bpf/libbpf_probes.c                 |    1 +
>   .../bpf/prog_tests/lsm_mprotect_audit.c       |  129 +++
>   .../selftests/bpf/progs/lsm_mprotect_audit.c  |   58 +
>   30 files changed, 2451 insertions(+), 80 deletions(-)
>   create mode 100644 Documentation/security/bpf.rst
>   create mode 100644 include/linux/bpf_event.h
>   create mode 100644 include/linux/bpf_lsm.h
>   create mode 100644 security/bpf/Kconfig
>   create mode 100644 security/bpf/Makefile
>   create mode 100644 security/bpf/include/bpf_lsm.h
>   create mode 100644 security/bpf/include/fs.h
>   create mode 100644 security/bpf/include/hooks.h
>   create mode 100644 security/bpf/lsm.c
>   create mode 100644 security/bpf/lsm_fs.c
>   create mode 100644 security/bpf/ops.c
>   create mode 100644 tools/testing/selftests/bpf/prog_tests/lsm_mprotect_audit.c
>   create mode 100644 tools/testing/selftests/bpf/progs/lsm_mprotect_audit.c
>
Alexei Starovoitov Dec. 22, 2019, 1:27 a.m. UTC | #4
On Fri, Dec 20, 2019 at 04:41:55PM +0100, KP Singh wrote:
> // Declare the eBPF program mprotect_audit which attaches to
> // to the file_mprotect LSM hook and accepts three arguments.
> BPF_TRACE_3("lsm/file_mprotect", mprotect_audit,
> 	    struct vm_area_struct *, vma,
> 	    unsigned long, reqprot, unsigned long, prot
> {
> 	unsigned long vm_start = _(vma->vm_start);
> 	return 0;
> }

I think the only sore point of the patchset is:
security/bpf/include/hooks.h   | 1015 ++++++++++++++++++++++++++++++++
With bpf trampoline this type of 'kernel types -> bpf types' converters
are no longer necessary. Please take a look at tcp-congestion-control patchset:
https://patchwork.ozlabs.org/cover/1214417/
Instead of doing similar thing (like your patch 1 plus patch 6) it's using
trampoline to provide bpf based congestion control callbacks into tcp stack.
The same trampoline-based mechanism can be reused by bpf_lsm.
Then all manual work of doing BPF_LSM_HOOK(...) for every hook won't be
necessary. It will also prove the point that attaching BPF to raw LSM hooks
doesn't freeze them into stable abi.
The programs can keep the same syntax as in your examples:
BPF_TRACE_3("lsm/file_mprotect", mprotect_audit,
libbpf will find file_mprotect's btf_id in kernel vmlinux and pass it to
the kernel for attaching. Just like fentry/fexit bpf progs are doing
and just like bpf-based cc is doing as well.

> In order to better illustrate the capabilities of the framework some
> more advanced prototype code has also been published separately:
> 
> * Logging execution events (including environment variables and arguments):
> https://github.com/sinkap/linux-krsi/blob/patch/v1/examples/samples/bpf/lsm_audit_env.c
> * Detecting deletion of running executables:
> https://github.com/sinkap/linux-krsi/blob/patch/v1/examples/samples/bpf/lsm_detect_exec_unlink.c
> * Detection of writes to /proc/<pid>/mem:
> https://github.com/sinkap/linux-krsi/blob/patch/v1/examples/samples/bpf/lsm_audit_env.c

Thank you for sharing these examples. That definitely helps to see more
complete picture. I noticed that the examples are using the pattern:
  u32 map_id = 0;
  env = bpf_map_lookup_elem(&env_map, &map_id);
Essentially they're global variables. libbpf already supports them.
bpf prog can use them as:
  struct env_value env;
  int bpf_prog(..)
  {
    env.name... env.value..
  }
That will make progs a bit easier to read and faster too.
Accesing global vars from user space is also trivial with skeleton work:
  lsm_audit_env_skel->bss->env.name... env.value.
Both bpf side and user side can access globals as normal C variables.

There is a small issue in the patches 8 and 10.
bpf program names are not unique and bpf-lsm should not require them to be different.
bpf_attr->prog_name is also short at 16 bytes. It's for introspection only.
Longer program names are supplied via btf's func_info.
It feels that:
cat /sys/kernel/security/bpf/process_execution
env_dumper__v2
is reinventing the wheel. bpftool is the main introspection tool.
It can print progs attached to perf, cgroup, networking. I think it's better to
stay consistent and do the same with bpf-lsm.

Another issue is in proposed attaching method:
hook_fd = open("/sys/kernel/security/bpf/process_execution");
sys_bpf(attach, prog_fd, hook_fd);
With bpf tracing we moved to FD-based attaching, because permanent attaching is
problematic in production. We're going to provide FD-based api to attach to
networking as well, because xdp/tc/cgroup prog attaching suffers from the same
production issues. Mainly with permanent attaching there is no ownership of
attachment. Everything is global and permanent. It's not clear what
process/script suppose to detach/cleanup. I suggest bpf-lsm use FD-based
attaching from the beginning. Take a look at raw_tp/tp_btf/fentry/fexit style
of attaching. All of them return FD which represents what libbpf calls
'bpf_link' concept. Once refcnt of that FD goes to zero that link (attachment)
is destroyed and program is detached _by the kernel_. To make such links
permanent the application can pin them in bpffs. The pinning patches haven't
landed yet, but the concept of the link is quite powerful and much more
production friendly than permanent attaching.
bpf-lsm will still be able to attach multiple progs to the same hook and
see what is attached via bpftool.

The rest looks good. Thank you for working on it.
Andrii Nakryiko Dec. 24, 2019, 6:51 a.m. UTC | #5
On Fri, Dec 20, 2019 at 7:42 AM KP Singh <kpsingh@chromium.org> wrote:
>
> From: KP Singh <kpsingh@google.com>
>
> This patch series is a continuation of the KRSI RFC
> (https://lore.kernel.org/bpf/20190910115527.5235-1-kpsingh@chromium.org/)
>

[...]

> # Usage Examples
>
> A simple example and some documentation is included in the patchset.
>
> In order to better illustrate the capabilities of the framework some
> more advanced prototype code has also been published separately:
>
> * Logging execution events (including environment variables and arguments):
> https://github.com/sinkap/linux-krsi/blob/patch/v1/examples/samples/bpf/lsm_audit_env.c
> * Detecting deletion of running executables:
> https://github.com/sinkap/linux-krsi/blob/patch/v1/examples/samples/bpf/lsm_detect_exec_unlink.c
> * Detection of writes to /proc/<pid>/mem:
> https://github.com/sinkap/linux-krsi/blob/patch/v1/examples/samples/bpf/lsm_audit_env.c

Are you planning on submitting these examples for inclusion into
samples/bpf or selftests/bpf? It would be great to have more examples
and we can review and suggest nicer ways to go about writing them
(e.g., BPF skeleton and global data Alexei mentioned earlier).

>
> We have updated Google's internal telemetry infrastructure and have
> started deploying this LSM on our Linux Workstations. This gives us more
> confidence in the real-world applications of such a system.
>
> KP Singh (13):
>   bpf: Refactor BPF_EVENT context macros to its own header.
>   bpf: lsm: Add a skeleton and config options
>   bpf: lsm: Introduce types for eBPF based LSM
>   bpf: lsm: Allow btf_id based attachment for LSM hooks
>   tools/libbpf: Add support in libbpf for BPF_PROG_TYPE_LSM
>   bpf: lsm: Init Hooks and create files in securityfs
>   bpf: lsm: Implement attach, detach and execution.
>   bpf: lsm: Show attached program names in hook read handler.
>   bpf: lsm: Add a helper function bpf_lsm_event_output
>   bpf: lsm: Handle attachment of the same program
>   tools/libbpf: Add bpf_program__attach_lsm
>   bpf: lsm: Add selftests for BPF_PROG_TYPE_LSM
>   bpf: lsm: Add Documentation
>

[...]
KP Singh Dec. 30, 2019, 2:58 p.m. UTC | #6
On 21-Dez 17:27, Alexei Starovoitov wrote:
> On Fri, Dec 20, 2019 at 04:41:55PM +0100, KP Singh wrote:
> > // Declare the eBPF program mprotect_audit which attaches to
> > // to the file_mprotect LSM hook and accepts three arguments.
> > BPF_TRACE_3("lsm/file_mprotect", mprotect_audit,
> > 	    struct vm_area_struct *, vma,
> > 	    unsigned long, reqprot, unsigned long, prot
> > {
> > 	unsigned long vm_start = _(vma->vm_start);
> > 	return 0;
> > }
> 

Hi Alexei,

Thanks for the feedback. This is really helpful!

> I think the only sore point of the patchset is:
> security/bpf/include/hooks.h   | 1015 ++++++++++++++++++++++++++++++++
> With bpf trampoline this type of 'kernel types -> bpf types' converters
> are no longer necessary. Please take a look at tcp-congestion-control patchset:
> https://patchwork.ozlabs.org/cover/1214417/
> Instead of doing similar thing (like your patch 1 plus patch 6) it's using
> trampoline to provide bpf based congestion control callbacks into tcp stack.
> The same trampoline-based mechanism can be reused by bpf_lsm.
> Then all manual work of doing BPF_LSM_HOOK(...) for every hook won't be
> necessary. It will also prove the point that attaching BPF to raw LSM hooks
> doesn't freeze them into stable abi.

Really cool!

I looked into how BPF trampolines are being used in tracing and the
new STRUCT_OPS patchset and was able protoype
(https://github.com/sinkap/linux-krsi/tree/patch/v1/trampoline_prototype,
not ready for review yet) which:

* Gets rid of security/bpf/include/hooks.h and all of the static
  macro magic essentially making the LSM ~truly instrumentable~ at
  runtime.
* Gets rid of the generation of any new types as we already have
  all the BTF information in the kernel in the following two types:

struct security_hook_heads {
        .
        .
        struct hlist_head file_mprotect;   <- Append the callback at this offset
        .
        .
};

and

union security_list_options {
	int (*file_mprotect)(struct vm_area_struct *vma, unsigned long reqprot,
				unsigned long prot);
};

Which is the same type as the typedef that's currently being generated
, i.e. lsm_btf_file_mprotect

In the current prototype, libbpf converts the name of the hook into an
offset into the security_hook_heads and the verifier does the
following when a program is loaded:

* Verifies the offset and the type at the offset (struct hlist_head).
* Resolves the func_proto (by looking up the type in
  security_list_options) and updates prog->aux with the name and
  func_proto which are then verified similar to raw_tp programs with
  btf_ctx_access.

On attachment:

* A trampoline is created and appended to the security_hook_heads
  for the BPF LSM.
* An anonymous FD is returned and the attachment is conditional on the
  references to FD (as suggested and similar to fentry/fexit tracing
  programs).

This implies that the BPF programs are "the LSM hook" as opposed to
being executed inside a statically defined hook body which requires
mutable LSM hooks for which I was able to re-use some of ideas in
Sargun's patch:

https://lore.kernel.org/lkml/20180408065916.GA2832@ircssh-2.c.rugged-nimbus-611.internal/

to maintain a separate security_hook_heads struct for dynamically
added LSM hooks by the BPF LSM which are executed after all the
statically defined hooks.

> Longer program names are supplied via btf's func_info.
> It feels that:
> cat /sys/kernel/security/bpf/process_execution
> env_dumper__v2
> is reinventing the wheel. bpftool is the main introspection tool.
> It can print progs attached to perf, cgroup, networking. I think it's better to
> stay consistent and do the same with bpf-lsm.

I agree, based on the new feedback, I don't think we need securityFS
attachment points anymore. I was able to get rid of it completely.

> 
> Another issue is in proposed attaching method:
> hook_fd = open("/sys/kernel/security/bpf/process_execution");
> sys_bpf(attach, prog_fd, hook_fd);
> With bpf tracing we moved to FD-based attaching, because permanent attaching is
> problematic in production. We're going to provide FD-based api to attach to
> networking as well, because xdp/tc/cgroup prog attaching suffers from the same
> production issues. Mainly with permanent attaching there is no ownership of
> attachment. Everything is global and permanent. It's not clear what
> process/script suppose to detach/cleanup. I suggest bpf-lsm use FD-based
> attaching from the beginning. Take a look at raw_tp/tp_btf/fentry/fexit style
> of attaching. All of them return FD which represents what libbpf calls
> 'bpf_link' concept. Once refcnt of that FD goes to zero that link (attachment)
> is destroyed and program is detached _by the kernel_. To make such links
> permanent the application can pin them in bpffs. The pinning patches haven't
> landed yet, but the concept of the link is quite powerful and much more
> production friendly than permanent attaching.

I like this. This also means we don't immediately need the handling of
duplicate names so I dropped that bit of the patch as well and updated
the attachment to use this mechanism.

> bpf-lsm will still be able to attach multiple progs to the same hook and
> see what is attached via bpftool.
> 
> The rest looks good. Thank you for working on it.

There are some choices we need to make here from an API perspective:

* Should we "repurpose" attr->attach_btf_id and use it as an offset
  into security_hook_heads or add a new attribute
  (e.g lsm_hook_offset) for the offset or use name of the LSM hook
  (e.g. lsm_hook_name).
* Since we don't have the files in securityFS, the attachment does not
  have a target_fd. Should we add a new type of BPF command?
  e.g. LSM_HOOK_OPEN?

I will clean up the prototype, incorporate some of the other feedback
received, and send a v2.

Wishing everyone a very Happy New Year!

- KP
KP Singh Dec. 30, 2019, 3:04 p.m. UTC | #7
On 23-Dec 22:51, Andrii Nakryiko wrote:
> On Fri, Dec 20, 2019 at 7:42 AM KP Singh <kpsingh@chromium.org> wrote:
> >
> > From: KP Singh <kpsingh@google.com>
> >
> > This patch series is a continuation of the KRSI RFC
> > (https://lore.kernel.org/bpf/20190910115527.5235-1-kpsingh@chromium.org/)
> >
> 
> [...]
> 
> > # Usage Examples
> >
> > A simple example and some documentation is included in the patchset.
> >
> > In order to better illustrate the capabilities of the framework some
> > more advanced prototype code has also been published separately:
> >
> > * Logging execution events (including environment variables and arguments):
> > https://github.com/sinkap/linux-krsi/blob/patch/v1/examples/samples/bpf/lsm_audit_env.c
> > * Detecting deletion of running executables:
> > https://github.com/sinkap/linux-krsi/blob/patch/v1/examples/samples/bpf/lsm_detect_exec_unlink.c
> > * Detection of writes to /proc/<pid>/mem:
> > https://github.com/sinkap/linux-krsi/blob/patch/v1/examples/samples/bpf/lsm_audit_env.c
> 
> Are you planning on submitting these examples for inclusion into
> samples/bpf or selftests/bpf? It would be great to have more examples
> and we can review and suggest nicer ways to go about writing them
> (e.g., BPF skeleton and global data Alexei mentioned earlier).

Eventually, yes and in selftest/bpf.

But these examples depend on using security blobs and some non-atomic
calls in the BPF helpers which are not handled as a part of the
initial patch-set.

Once we have the initial framework finalized, I will update the
examples and the helpers they are based on and send these separate
patch-sets on the list for review.

- KP

> 
> >
> > We have updated Google's internal telemetry infrastructure and have
> > started deploying this LSM on our Linux Workstations. This gives us more
> > confidence in the real-world applications of such a system.
> >
> > KP Singh (13):
> >   bpf: Refactor BPF_EVENT context macros to its own header.
> >   bpf: lsm: Add a skeleton and config options
> >   bpf: lsm: Introduce types for eBPF based LSM
> >   bpf: lsm: Allow btf_id based attachment for LSM hooks
> >   tools/libbpf: Add support in libbpf for BPF_PROG_TYPE_LSM
> >   bpf: lsm: Init Hooks and create files in securityfs
> >   bpf: lsm: Implement attach, detach and execution.
> >   bpf: lsm: Show attached program names in hook read handler.
> >   bpf: lsm: Add a helper function bpf_lsm_event_output
> >   bpf: lsm: Handle attachment of the same program
> >   tools/libbpf: Add bpf_program__attach_lsm
> >   bpf: lsm: Add selftests for BPF_PROG_TYPE_LSM
> >   bpf: lsm: Add Documentation
> >
> 
> [...]
Andrii Nakryiko Dec. 30, 2019, 6:58 p.m. UTC | #8
On Mon, Dec 30, 2019 at 7:04 AM KP Singh <kpsingh@chromium.org> wrote:
>
> On 23-Dec 22:51, Andrii Nakryiko wrote:
> > On Fri, Dec 20, 2019 at 7:42 AM KP Singh <kpsingh@chromium.org> wrote:
> > >
> > > From: KP Singh <kpsingh@google.com>
> > >
> > > This patch series is a continuation of the KRSI RFC
> > > (https://lore.kernel.org/bpf/20190910115527.5235-1-kpsingh@chromium.org/)
> > >
> >
> > [...]
> >
> > > # Usage Examples
> > >
> > > A simple example and some documentation is included in the patchset.
> > >
> > > In order to better illustrate the capabilities of the framework some
> > > more advanced prototype code has also been published separately:
> > >
> > > * Logging execution events (including environment variables and arguments):
> > > https://github.com/sinkap/linux-krsi/blob/patch/v1/examples/samples/bpf/lsm_audit_env.c
> > > * Detecting deletion of running executables:
> > > https://github.com/sinkap/linux-krsi/blob/patch/v1/examples/samples/bpf/lsm_detect_exec_unlink.c
> > > * Detection of writes to /proc/<pid>/mem:
> > > https://github.com/sinkap/linux-krsi/blob/patch/v1/examples/samples/bpf/lsm_audit_env.c
> >
> > Are you planning on submitting these examples for inclusion into
> > samples/bpf or selftests/bpf? It would be great to have more examples
> > and we can review and suggest nicer ways to go about writing them
> > (e.g., BPF skeleton and global data Alexei mentioned earlier).
>
> Eventually, yes and in selftest/bpf.
>
> But these examples depend on using security blobs and some non-atomic
> calls in the BPF helpers which are not handled as a part of the
> initial patch-set.
>
> Once we have the initial framework finalized, I will update the
> examples and the helpers they are based on and send these separate
> patch-sets on the list for review.

Great! The reason I was asking is that once they are in selftests, it
would be nice to switch them to use all the latest BPF usability
improvements to make code cleaner and have it as another good example
of modern BPF program. Like use BTF-defined maps, BPF skeleton,
vmlinux.h, etc. We can go over this when the time comes, though :)


>
> - KP
>
> >
> > >
> > > We have updated Google's internal telemetry infrastructure and have
> > > started deploying this LSM on our Linux Workstations. This gives us more
> > > confidence in the real-world applications of such a system.
> > >
> > > KP Singh (13):
> > >   bpf: Refactor BPF_EVENT context macros to its own header.
> > >   bpf: lsm: Add a skeleton and config options
> > >   bpf: lsm: Introduce types for eBPF based LSM
> > >   bpf: lsm: Allow btf_id based attachment for LSM hooks
> > >   tools/libbpf: Add support in libbpf for BPF_PROG_TYPE_LSM
> > >   bpf: lsm: Init Hooks and create files in securityfs
> > >   bpf: lsm: Implement attach, detach and execution.
> > >   bpf: lsm: Show attached program names in hook read handler.
> > >   bpf: lsm: Add a helper function bpf_lsm_event_output
> > >   bpf: lsm: Handle attachment of the same program
> > >   tools/libbpf: Add bpf_program__attach_lsm
> > >   bpf: lsm: Add selftests for BPF_PROG_TYPE_LSM
> > >   bpf: lsm: Add Documentation
> > >
> >
> > [...]
Andrii Nakryiko Dec. 30, 2019, 7:14 p.m. UTC | #9
On Mon, Dec 30, 2019 at 6:59 AM KP Singh <kpsingh@chromium.org> wrote:
>
> On 21-Dez 17:27, Alexei Starovoitov wrote:
> > On Fri, Dec 20, 2019 at 04:41:55PM +0100, KP Singh wrote:
> > > // Declare the eBPF program mprotect_audit which attaches to
> > > // to the file_mprotect LSM hook and accepts three arguments.
> > > BPF_TRACE_3("lsm/file_mprotect", mprotect_audit,
> > >         struct vm_area_struct *, vma,
> > >         unsigned long, reqprot, unsigned long, prot
> > > {
> > >     unsigned long vm_start = _(vma->vm_start);
> > >     return 0;
> > > }
> >
>
> Hi Alexei,
>
> Thanks for the feedback. This is really helpful!
>
> > I think the only sore point of the patchset is:
> > security/bpf/include/hooks.h   | 1015 ++++++++++++++++++++++++++++++++
> > With bpf trampoline this type of 'kernel types -> bpf types' converters
> > are no longer necessary. Please take a look at tcp-congestion-control patchset:
> > https://patchwork.ozlabs.org/cover/1214417/
> > Instead of doing similar thing (like your patch 1 plus patch 6) it's using
> > trampoline to provide bpf based congestion control callbacks into tcp stack.
> > The same trampoline-based mechanism can be reused by bpf_lsm.
> > Then all manual work of doing BPF_LSM_HOOK(...) for every hook won't be
> > necessary. It will also prove the point that attaching BPF to raw LSM hooks
> > doesn't freeze them into stable abi.
>
> Really cool!
>
> I looked into how BPF trampolines are being used in tracing and the
> new STRUCT_OPS patchset and was able protoype
> (https://github.com/sinkap/linux-krsi/tree/patch/v1/trampoline_prototype,
> not ready for review yet) which:
>
> * Gets rid of security/bpf/include/hooks.h and all of the static
>   macro magic essentially making the LSM ~truly instrumentable~ at
>   runtime.
> * Gets rid of the generation of any new types as we already have
>   all the BTF information in the kernel in the following two types:
>
> struct security_hook_heads {
>         .
>         .
>         struct hlist_head file_mprotect;   <- Append the callback at this offset
>         .
>         .
> };
>
> and
>
> union security_list_options {
>         int (*file_mprotect)(struct vm_area_struct *vma, unsigned long reqprot,
>                                 unsigned long prot);
> };
>
> Which is the same type as the typedef that's currently being generated
> , i.e. lsm_btf_file_mprotect
>
> In the current prototype, libbpf converts the name of the hook into an
> offset into the security_hook_heads and the verifier does the
> following when a program is loaded:
>
> * Verifies the offset and the type at the offset (struct hlist_head).
> * Resolves the func_proto (by looking up the type in
>   security_list_options) and updates prog->aux with the name and
>   func_proto which are then verified similar to raw_tp programs with
>   btf_ctx_access.
>
> On attachment:
>
> * A trampoline is created and appended to the security_hook_heads
>   for the BPF LSM.
> * An anonymous FD is returned and the attachment is conditional on the
>   references to FD (as suggested and similar to fentry/fexit tracing
>   programs).
>
> This implies that the BPF programs are "the LSM hook" as opposed to
> being executed inside a statically defined hook body which requires
> mutable LSM hooks for which I was able to re-use some of ideas in
> Sargun's patch:
>
> https://lore.kernel.org/lkml/20180408065916.GA2832@ircssh-2.c.rugged-nimbus-611.internal/
>
> to maintain a separate security_hook_heads struct for dynamically
> added LSM hooks by the BPF LSM which are executed after all the
> statically defined hooks.
>
> > Longer program names are supplied via btf's func_info.
> > It feels that:
> > cat /sys/kernel/security/bpf/process_execution
> > env_dumper__v2
> > is reinventing the wheel. bpftool is the main introspection tool.
> > It can print progs attached to perf, cgroup, networking. I think it's better to
> > stay consistent and do the same with bpf-lsm.
>
> I agree, based on the new feedback, I don't think we need securityFS
> attachment points anymore. I was able to get rid of it completely.
>
> >
> > Another issue is in proposed attaching method:
> > hook_fd = open("/sys/kernel/security/bpf/process_execution");
> > sys_bpf(attach, prog_fd, hook_fd);
> > With bpf tracing we moved to FD-based attaching, because permanent attaching is
> > problematic in production. We're going to provide FD-based api to attach to
> > networking as well, because xdp/tc/cgroup prog attaching suffers from the same
> > production issues. Mainly with permanent attaching there is no ownership of
> > attachment. Everything is global and permanent. It's not clear what
> > process/script suppose to detach/cleanup. I suggest bpf-lsm use FD-based
> > attaching from the beginning. Take a look at raw_tp/tp_btf/fentry/fexit style
> > of attaching. All of them return FD which represents what libbpf calls
> > 'bpf_link' concept. Once refcnt of that FD goes to zero that link (attachment)
> > is destroyed and program is detached _by the kernel_. To make such links
> > permanent the application can pin them in bpffs. The pinning patches haven't
> > landed yet, but the concept of the link is quite powerful and much more
> > production friendly than permanent attaching.
>
> I like this. This also means we don't immediately need the handling of
> duplicate names so I dropped that bit of the patch as well and updated
> the attachment to use this mechanism.
>
> > bpf-lsm will still be able to attach multiple progs to the same hook and
> > see what is attached via bpftool.
> >
> > The rest looks good. Thank you for working on it.
>
> There are some choices we need to make here from an API perspective:
>
> * Should we "repurpose" attr->attach_btf_id and use it as an offset
>   into security_hook_heads or add a new attribute
>   (e.g lsm_hook_offset) for the offset or use name of the LSM hook
>   (e.g. lsm_hook_name).

I think setting this to member index inside union
security_list_options will be better? Or member index inside struct
security_hook_heads. Seems like kernel will have to "join" those two
anyways, right (one for type info, another for trampoline)? Offset is
less convenient either way.

> * Since we don't have the files in securityFS, the attachment does not
>   have a target_fd. Should we add a new type of BPF command?
>   e.g. LSM_HOOK_OPEN?

Semantics of LSM program seems closer to fentry/fexit/raw_tp, so maybe
instead use BPF_RAW_TRACEPOINT_OPEN command? On libbpf side it's all
going to be abstracted behind bpf_program__attach() anyways.

>
> I will clean up the prototype, incorporate some of the other feedback
> received, and send a v2.
>
> Wishing everyone a very Happy New Year!

Thanks, you too!

>
> - KP
>
Kees Cook Dec. 30, 2019, 7:15 p.m. UTC | #10
On Fri, Dec 20, 2019 at 06:38:45PM +0100, KP Singh wrote:
> Hi Casey,
> 
> Thanks for taking a look!
> 
> On Fri, Dec 20, 2019 at 6:17 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
> >
> > On 12/20/2019 7:41 AM, KP Singh wrote:
> > > From: KP Singh <kpsingh@google.com>
> > >
> > > This patch series is a continuation of the KRSI RFC
> > > (https://lore.kernel.org/bpf/20190910115527.5235-1-kpsingh@chromium.org/)
> > >
> > > # Motivation
> > >
> > > Google does rich analysis of runtime security data collected from
> > > internal Linux deployments (corporate devices and servers) to detect and
> > > thwart threats in real-time. Currently, this is done in custom kernel
> > > modules but we would like to replace this with something that's upstream
> > > and useful to others.
> > >
> > > The current kernel infrastructure for providing telemetry (Audit, Perf
> > > etc.) is disjoint from access enforcement (i.e. LSMs).  Augmenting the
> > > information provided by audit requires kernel changes to audit, its
> > > policy language and user-space components. Furthermore, building a MAC
> > > policy based on the newly added telemetry data requires changes to
> > > various LSMs and their respective policy languages.
> > >
> > > This patchset proposes a new stackable and privileged LSM which allows
> > > the LSM hooks to be implemented using eBPF. This facilitates a unified
> > > and dynamic (not requiring re-compilation of the kernel) audit and MAC
> > > policy.
> > >
> > > # Why an LSM?
> > >
> > > Linux Security Modules target security behaviours rather than the
> > > kernel's API. For example, it's easy to miss out a newly added system
> > > call for executing processes (eg. execve, execveat etc.) but the LSM
> > > framework ensures that all process executions trigger the relevant hooks
> > > irrespective of how the process was executed.
> > >
> > > Allowing users to implement LSM hooks at runtime also benefits the LSM
> > > eco-system by enabling a quick feedback loop from the security community
> > > about the kind of behaviours that the LSM Framework should be targeting.
> > >
> > > # How does it work?
> > >
> > > The LSM introduces a new eBPF (https://docs.cilium.io/en/v1.6/bpf/)
> > > program type, BPF_PROG_TYPE_LSM, which can only be attached to a LSM
> > > hook.  All LSM hooks are exposed as files in securityfs. Attachment
> > > requires CAP_SYS_ADMIN for loading eBPF programs and CAP_MAC_ADMIN for
> > > modifying MAC policies.
> > >
> > > The eBPF programs are passed the same arguments as the LSM hooks and
> > > executed in the body of the hook.
> >
> > This effectively exposes the LSM hooks as external APIs.
> > It would mean that we can't change or delete them. That
> > would be bad.
> 
> Perhaps this should have been clearer, we *do not* want to make LSM hooks
> a stable API and expect the eBPF programs to adapt when such changes occur.
> 
> Based on our comparison with the previous approach, this still ends up
> being a better trade-off (w.r.t. maintenance) when compared to adding
> specific helpers or verifier logic for  each new hook or field that
> needs to be exposed.

Given the discussion around tracing and stable ABI at the last kernel
summit, Linus's mandate is mainly around "every day users" and not
around these system-builder-sensitive cases where everyone has a strong
expectation to rebuild their policy when the kernel changes. i.e. it's
not "powertop", which was Linus's example of "and then everyone running
Fedora breaks".

So, while I know we've tried in the past to follow the letter of the
law, it seems Linus really expects this only to be followed when it will
have "real world" impact on unsuspecting end users.

Obviously James Morris has the final say here, but as I understand it,
it is fine to expose these here for the same reasons it's fine to expose
the (ever changing) tracepoints and BPF hooks.

-Kees
Kees Cook Dec. 30, 2019, 7:30 p.m. UTC | #11
On Fri, Dec 20, 2019 at 11:46:47PM +0100, Mickaël Salaün wrote:
> I'm working on a version of Landlock without eBPF, but still with the
> initial sought properties: safe unprivileged composability, modularity, and
> dynamic update. I'll send this version soon.
> 
> I hope that the work and experience from Landlock to bring eBPF to LSM will
> continue to be used through KRSI. Landlock will now focus on the
> unprivileged sandboxing part, without eBPF. Stay tuned!

Will it end up looking at all like pledge? I'm still struggling to come
up with a sensible pledge-like design on top of seccomp, especially
given the need to have it very closely tied to the running libc...
Mickaël Salaün Dec. 31, 2019, 12:11 p.m. UTC | #12
On 30/12/2019 20:30, Kees Cook wrote:
> On Fri, Dec 20, 2019 at 11:46:47PM +0100, Mickaël Salaün wrote:
>> I'm working on a version of Landlock without eBPF, but still with the
>> initial sought properties: safe unprivileged composability, modularity, and
>> dynamic update. I'll send this version soon.
>>
>> I hope that the work and experience from Landlock to bring eBPF to LSM will
>> continue to be used through KRSI. Landlock will now focus on the
>> unprivileged sandboxing part, without eBPF. Stay tuned!
> 
> Will it end up looking at all like pledge? I'm still struggling to come
> up with a sensible pledge-like design on top of seccomp, especially
> given the need to have it very closely tied to the running libc...
> 

Yes, it's similar to Pledge/Unveil but with fine-grained control (and a
more flexible design). And because it is not tied to syscall, there is
no similar issues than with seccomp and libc. In fact, there is no more
relationship with seccomp neither. The version I'm working on is similar
in principle to the patch series v10 [1], without the usage complexity
brought by eBPF, but with a more polished file-based access-control. The
demo from LSS 2018 [2] gives an overview of the possibilities.

[1] https://lore.kernel.org/lkml/20190721213116.23476-1-mic@digikod.net/
[2] https://landlock.io/talks/2018-08-27_landlock-lss_demo-1-web.mkv
Stephen Smalley Jan. 8, 2020, 3:25 p.m. UTC | #13
On 12/30/19 2:15 PM, Kees Cook wrote:
> On Fri, Dec 20, 2019 at 06:38:45PM +0100, KP Singh wrote:
>> Hi Casey,
>>
>> Thanks for taking a look!
>>
>> On Fri, Dec 20, 2019 at 6:17 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>>>
>>> On 12/20/2019 7:41 AM, KP Singh wrote:
>>>> From: KP Singh <kpsingh@google.com>
>>>>
>>>> This patch series is a continuation of the KRSI RFC
>>>> (https://lore.kernel.org/bpf/20190910115527.5235-1-kpsingh@chromium.org/)
>>>>
>>>> # Motivation
>>>>
>>>> Google does rich analysis of runtime security data collected from
>>>> internal Linux deployments (corporate devices and servers) to detect and
>>>> thwart threats in real-time. Currently, this is done in custom kernel
>>>> modules but we would like to replace this with something that's upstream
>>>> and useful to others.
>>>>
>>>> The current kernel infrastructure for providing telemetry (Audit, Perf
>>>> etc.) is disjoint from access enforcement (i.e. LSMs).  Augmenting the
>>>> information provided by audit requires kernel changes to audit, its
>>>> policy language and user-space components. Furthermore, building a MAC
>>>> policy based on the newly added telemetry data requires changes to
>>>> various LSMs and their respective policy languages.
>>>>
>>>> This patchset proposes a new stackable and privileged LSM which allows
>>>> the LSM hooks to be implemented using eBPF. This facilitates a unified
>>>> and dynamic (not requiring re-compilation of the kernel) audit and MAC
>>>> policy.
>>>>
>>>> # Why an LSM?
>>>>
>>>> Linux Security Modules target security behaviours rather than the
>>>> kernel's API. For example, it's easy to miss out a newly added system
>>>> call for executing processes (eg. execve, execveat etc.) but the LSM
>>>> framework ensures that all process executions trigger the relevant hooks
>>>> irrespective of how the process was executed.
>>>>
>>>> Allowing users to implement LSM hooks at runtime also benefits the LSM
>>>> eco-system by enabling a quick feedback loop from the security community
>>>> about the kind of behaviours that the LSM Framework should be targeting.
>>>>
>>>> # How does it work?
>>>>
>>>> The LSM introduces a new eBPF (https://docs.cilium.io/en/v1.6/bpf/)
>>>> program type, BPF_PROG_TYPE_LSM, which can only be attached to a LSM
>>>> hook.  All LSM hooks are exposed as files in securityfs. Attachment
>>>> requires CAP_SYS_ADMIN for loading eBPF programs and CAP_MAC_ADMIN for
>>>> modifying MAC policies.
>>>>
>>>> The eBPF programs are passed the same arguments as the LSM hooks and
>>>> executed in the body of the hook.
>>>
>>> This effectively exposes the LSM hooks as external APIs.
>>> It would mean that we can't change or delete them. That
>>> would be bad.
>>
>> Perhaps this should have been clearer, we *do not* want to make LSM hooks
>> a stable API and expect the eBPF programs to adapt when such changes occur.
>>
>> Based on our comparison with the previous approach, this still ends up
>> being a better trade-off (w.r.t. maintenance) when compared to adding
>> specific helpers or verifier logic for  each new hook or field that
>> needs to be exposed.
> 
> Given the discussion around tracing and stable ABI at the last kernel
> summit, Linus's mandate is mainly around "every day users" and not
> around these system-builder-sensitive cases where everyone has a strong
> expectation to rebuild their policy when the kernel changes. i.e. it's
> not "powertop", which was Linus's example of "and then everyone running
> Fedora breaks".
> 
> So, while I know we've tried in the past to follow the letter of the
> law, it seems Linus really expects this only to be followed when it will
> have "real world" impact on unsuspecting end users.
> 
> Obviously James Morris has the final say here, but as I understand it,
> it is fine to expose these here for the same reasons it's fine to expose
> the (ever changing) tracepoints and BPF hooks.

This appears to impose a very different standard to this eBPF-based LSM 
than has been applied to the existing LSMs, e.g. we are required to 
preserve SELinux policy compatibility all the way back to Linux 2.6.0 
such that new kernel with old policy does not break userspace.  If that 
standard isn't being applied to the eBPF-based LSM, it seems to inhibit 
its use in major Linux distros, since otherwise users will in fact start 
experiencing breakage on the first such incompatible change.  Not 
arguing for or against, just trying to make sure I understand correctly...
James Morris Jan. 8, 2020, 6:27 p.m. UTC | #14
On Mon, 30 Dec 2019, Kees Cook wrote:

> 
> Given the discussion around tracing and stable ABI at the last kernel
> summit, Linus's mandate is mainly around "every day users" and not
> around these system-builder-sensitive cases where everyone has a strong
> expectation to rebuild their policy when the kernel changes. i.e. it's
> not "powertop", which was Linus's example of "and then everyone running
> Fedora breaks".
> 
> So, while I know we've tried in the past to follow the letter of the
> law, it seems Linus really expects this only to be followed when it will
> have "real world" impact on unsuspecting end users.
> 
> Obviously James Morris has the final say here, but as I understand it,
> it is fine to expose these here for the same reasons it's fine to expose
> the (ever changing) tracepoints and BPF hooks.

Agreed. This API should be seen in the same light as tracing / debugging, 
and it should not be exposed by users directly to general purpose 
applications.
James Morris Jan. 8, 2020, 6:58 p.m. UTC | #15
On Wed, 8 Jan 2020, Stephen Smalley wrote:

> This appears to impose a very different standard to this eBPF-based LSM than
> has been applied to the existing LSMs, e.g. we are required to preserve
> SELinux policy compatibility all the way back to Linux 2.6.0 such that new
> kernel with old policy does not break userspace.  If that standard isn't being
> applied to the eBPF-based LSM, it seems to inhibit its use in major Linux
> distros, since otherwise users will in fact start experiencing breakage on the
> first such incompatible change.  Not arguing for or against, just trying to
> make sure I understand correctly...

A different standard would be applied here vs. a standard LSM like 
SELinux, which are retrofitted access control systems.

I see KRSI as being more of a debugging / analytical API, rather than an 
access control system. You could of course build such a system with KRSI 
but it would need to provide a layer of abstraction for general purpose 
users.

So yes this would be a special case, as its real value is in being a 
special case, i.e. dynamic security telemetry.
Stephen Smalley Jan. 8, 2020, 7:33 p.m. UTC | #16
On 1/8/20 1:58 PM, James Morris wrote:
> On Wed, 8 Jan 2020, Stephen Smalley wrote:
> 
>> This appears to impose a very different standard to this eBPF-based LSM than
>> has been applied to the existing LSMs, e.g. we are required to preserve
>> SELinux policy compatibility all the way back to Linux 2.6.0 such that new
>> kernel with old policy does not break userspace.  If that standard isn't being
>> applied to the eBPF-based LSM, it seems to inhibit its use in major Linux
>> distros, since otherwise users will in fact start experiencing breakage on the
>> first such incompatible change.  Not arguing for or against, just trying to
>> make sure I understand correctly...
> 
> A different standard would be applied here vs. a standard LSM like
> SELinux, which are retrofitted access control systems.
> 
> I see KRSI as being more of a debugging / analytical API, rather than an
> access control system. You could of course build such a system with KRSI
> but it would need to provide a layer of abstraction for general purpose
> users.
> 
> So yes this would be a special case, as its real value is in being a
> special case, i.e. dynamic security telemetry.

The cover letter subject line and the Kconfig help text refer to it as a 
BPF-based "MAC and Audit policy".  It has an enforce config option that 
enables the bpf programs to deny access, providing access control. IIRC, 
in the earlier discussion threads, the BPF maintainers suggested that 
Smack and other LSMs could be entirely re-implemented via it in the 
future, and that such an implementation would be more optimal.

Again, not arguing for or against, but wondering if people fully 
understand the implications.  If it ends up being useful, people will 
build access control systems with it, and it directly exposes a lot of 
kernel internals to userspace.  There was a lot of concern originally 
about the LSM hook interface becoming a stable ABI and/or about it being 
misused.  Exposing that interface along with every kernel data structure 
exposed through it to userspace seems like a major leap.  Even if the 
mainline kernel doesn't worry about any kind of stable interface 
guarantees for it, the distros might be forced to provide some kABI 
guarantees for it to appease ISVs and users...
James Morris Jan. 9, 2020, 6:11 p.m. UTC | #17
On Wed, 8 Jan 2020, Stephen Smalley wrote:

> The cover letter subject line and the Kconfig help text refer to it as a
> BPF-based "MAC and Audit policy".  It has an enforce config option that
> enables the bpf programs to deny access, providing access control. IIRC, in
> the earlier discussion threads, the BPF maintainers suggested that Smack and
> other LSMs could be entirely re-implemented via it in the future, and that
> such an implementation would be more optimal.

In this case, the eBPF code is similar to a kernel module, rather than a 
loadable policy file.  It's a loadable mechanism, rather than a policy, in 
my view.

This would be similar to the difference between iptables rules and 
loadable eBPF networking code.  I'd be interested to know how the 
eBPF networking scenarios are handled wrt kernel ABI.


> Again, not arguing for or against, but wondering if people fully understand
> the implications.  If it ends up being useful, people will build access
> control systems with it, and it directly exposes a lot of kernel internals to
> userspace.  There was a lot of concern originally about the LSM hook interface
> becoming a stable ABI and/or about it being misused.  Exposing that interface
> along with every kernel data structure exposed through it to userspace seems
> like a major leap.

Agreed this is a leap, although I'm not sure I'd characterize it as 
exposure to userspace -- it allows dynamic extension of the LSM API from 
userland, but the code is executed in the kernel.

KP: One thing I'd like to understand better is the attack surface 
introduced by this.  IIUC, the BTF fields are read only, so the eBPF code 
should not be able to modify any LSM parameters, correct?


>  Even if the mainline kernel doesn't worry about any kind
> of stable interface guarantees for it, the distros might be forced to provide
> some kABI guarantees for it to appease ISVs and users...

How is this handled currently for other eBPF use-cases?
Greg KH Jan. 9, 2020, 6:23 p.m. UTC | #18
On Fri, Jan 10, 2020 at 05:11:38AM +1100, James Morris wrote:
> On Wed, 8 Jan 2020, Stephen Smalley wrote:
> 
> > The cover letter subject line and the Kconfig help text refer to it as a
> > BPF-based "MAC and Audit policy".  It has an enforce config option that
> > enables the bpf programs to deny access, providing access control. IIRC, in
> > the earlier discussion threads, the BPF maintainers suggested that Smack and
> > other LSMs could be entirely re-implemented via it in the future, and that
> > such an implementation would be more optimal.
> 
> In this case, the eBPF code is similar to a kernel module, rather than a 
> loadable policy file.  It's a loadable mechanism, rather than a policy, in 
> my view.
> 
> This would be similar to the difference between iptables rules and 
> loadable eBPF networking code.  I'd be interested to know how the 
> eBPF networking scenarios are handled wrt kernel ABI.

I already know of some people who pre-compile ebpf programs based on a
number of "supported" kernel versions and then load the needed one at
runtime.

Messy, yes, but you are right, ebpf code is much more similiar to a
kernel module than userspace code at the moment.

thanks,

greg k-h
Stephen Smalley Jan. 9, 2020, 6:58 p.m. UTC | #19
On 1/9/20 1:11 PM, James Morris wrote:
> On Wed, 8 Jan 2020, Stephen Smalley wrote:
> 
>> The cover letter subject line and the Kconfig help text refer to it as a
>> BPF-based "MAC and Audit policy".  It has an enforce config option that
>> enables the bpf programs to deny access, providing access control. IIRC, in
>> the earlier discussion threads, the BPF maintainers suggested that Smack and
>> other LSMs could be entirely re-implemented via it in the future, and that
>> such an implementation would be more optimal.
> 
> In this case, the eBPF code is similar to a kernel module, rather than a
> loadable policy file.  It's a loadable mechanism, rather than a policy, in
> my view.

I thought you frowned on dynamically loadable LSMs for both security and 
correctness reasons? And a traditional security module would necessarily 
fall under GPL; is the eBPF code required to be likewise?  If not, KRSI 
is a gateway for proprietary LSMs...

> This would be similar to the difference between iptables rules and
> loadable eBPF networking code.  I'd be interested to know how the
> eBPF networking scenarios are handled wrt kernel ABI.
> 
> 
>> Again, not arguing for or against, but wondering if people fully understand
>> the implications.  If it ends up being useful, people will build access
>> control systems with it, and it directly exposes a lot of kernel internals to
>> userspace.  There was a lot of concern originally about the LSM hook interface
>> becoming a stable ABI and/or about it being misused.  Exposing that interface
>> along with every kernel data structure exposed through it to userspace seems
>> like a major leap.
> 
> Agreed this is a leap, although I'm not sure I'd characterize it as
> exposure to userspace -- it allows dynamic extension of the LSM API from
> userland, but the code is executed in the kernel.
> 
> KP: One thing I'd like to understand better is the attack surface
> introduced by this.  IIUC, the BTF fields are read only, so the eBPF code
> should not be able to modify any LSM parameters, correct?
> 
> 
>>   Even if the mainline kernel doesn't worry about any kind
>> of stable interface guarantees for it, the distros might be forced to provide
>> some kABI guarantees for it to appease ISVs and users...
> 
> How is this handled currently for other eBPF use-cases?
>
James Morris Jan. 9, 2020, 7:07 p.m. UTC | #20
On Thu, 9 Jan 2020, Stephen Smalley wrote:

> On 1/9/20 1:11 PM, James Morris wrote:
> > On Wed, 8 Jan 2020, Stephen Smalley wrote:
> > 
> > > The cover letter subject line and the Kconfig help text refer to it as a
> > > BPF-based "MAC and Audit policy".  It has an enforce config option that
> > > enables the bpf programs to deny access, providing access control. IIRC,
> > > in
> > > the earlier discussion threads, the BPF maintainers suggested that Smack
> > > and
> > > other LSMs could be entirely re-implemented via it in the future, and that
> > > such an implementation would be more optimal.
> > 
> > In this case, the eBPF code is similar to a kernel module, rather than a
> > loadable policy file.  It's a loadable mechanism, rather than a policy, in
> > my view.
> 
> I thought you frowned on dynamically loadable LSMs for both security and
> correctness reasons?

Evaluating the security impact of this is the next step. My understanding 
is that eBPF via BTF is constrained to read only access to hook 
parameters, and that its behavior would be entirely restrictive.

I'd like to understand the security impact more fully, though.  Can the 
eBPF code make arbitrary writes to the kernel, or read anything other than 
the correctly bounded LSM hook parameters?

> And a traditional security module would necessarily fall
> under GPL; is the eBPF code required to be likewise?  If not, KRSI is a
> gateway for proprietary LSMs...

Right, we do not want this to be a GPL bypass.

If these issues can be resolved, this may be a "safe" way to support 
loadable LSM applications.

Again, I'd be interested in knowing how this is is handled in the 
networking stack (keeping in mind that LSM is a much more invasive API, 
and may not be directly comparable).
KP Singh Jan. 9, 2020, 7:11 p.m. UTC | #21
On 10-Jan 05:11, James Morris wrote:
> On Wed, 8 Jan 2020, Stephen Smalley wrote:
> 
> > The cover letter subject line and the Kconfig help text refer to it as a
> > BPF-based "MAC and Audit policy".  It has an enforce config option that
> > enables the bpf programs to deny access, providing access control. IIRC, in
> > the earlier discussion threads, the BPF maintainers suggested that Smack and
> > other LSMs could be entirely re-implemented via it in the future, and that
> > such an implementation would be more optimal.
> 
> In this case, the eBPF code is similar to a kernel module, rather than a 
> loadable policy file.  It's a loadable mechanism, rather than a policy, in 
> my view.
> 
> This would be similar to the difference between iptables rules and 
> loadable eBPF networking code.  I'd be interested to know how the 
> eBPF networking scenarios are handled wrt kernel ABI.
> 
> 
> > Again, not arguing for or against, but wondering if people fully understand
> > the implications.  If it ends up being useful, people will build access
> > control systems with it, and it directly exposes a lot of kernel internals to
> > userspace.  There was a lot of concern originally about the LSM hook interface
> > becoming a stable ABI and/or about it being misused.  Exposing that interface
> > along with every kernel data structure exposed through it to userspace seems
> > like a major leap.
> 
> Agreed this is a leap, although I'm not sure I'd characterize it as 
> exposure to userspace -- it allows dynamic extension of the LSM API from 
> userland, but the code is executed in the kernel.
> 
> KP: One thing I'd like to understand better is the attack surface 
> introduced by this.  IIUC, the BTF fields are read only, so the eBPF code 
> should not be able to modify any LSM parameters, correct?
> 

That's correct, the verifier does not allow writes to BTF types:

from kernel/bpf/verifier.c:

        case PTR_TO_BTF_ID:
	if (type == BPF_WRITE) {
	        verbose(env, "Writes through BTF pointers are not allowed\n");
		return -EINVAL;
	}

We can also add additional checks on top of those added by the
verifier using the verifier_ops that each BPF program type can define. 

- KP

> 
> >  Even if the mainline kernel doesn't worry about any kind
> > of stable interface guarantees for it, the distros might be forced to provide
> > some kABI guarantees for it to appease ISVs and users...
> 
> How is this handled currently for other eBPF use-cases?
> 
> -- 
> James Morris
> <jmorris@namei.org>
>
KP Singh Jan. 9, 2020, 7:43 p.m. UTC | #22
On 10-Jan 06:07, James Morris wrote:
> On Thu, 9 Jan 2020, Stephen Smalley wrote:
> 
> > On 1/9/20 1:11 PM, James Morris wrote:
> > > On Wed, 8 Jan 2020, Stephen Smalley wrote:
> > > 
> > > > The cover letter subject line and the Kconfig help text refer to it as a
> > > > BPF-based "MAC and Audit policy".  It has an enforce config option that
> > > > enables the bpf programs to deny access, providing access control. IIRC,
> > > > in
> > > > the earlier discussion threads, the BPF maintainers suggested that Smack
> > > > and
> > > > other LSMs could be entirely re-implemented via it in the future, and that
> > > > such an implementation would be more optimal.
> > > 
> > > In this case, the eBPF code is similar to a kernel module, rather than a
> > > loadable policy file.  It's a loadable mechanism, rather than a policy, in
> > > my view.
> > 
> > I thought you frowned on dynamically loadable LSMs for both security and
> > correctness reasons?

Based on the feedback from the lists we've updated the design for v2.

In v2, LSM hook callbacks are allocated dynamically using BPF
trampolines, appended to a separate security_hook_heads and run
only after the statically allocated hooks.

The security_hook_heads for all the other LSMs (SELinux, AppArmor etc)
still remains __lsm_ro_after_init and cannot be modified. We are still
working on v2 (not ready for review yet) but the general idea can be
seen here:

  https://github.com/sinkap/linux-krsi/blob/patch/v1/trampoline_prototype/security/bpf/lsm.c

> 
> Evaluating the security impact of this is the next step. My understanding 
> is that eBPF via BTF is constrained to read only access to hook 
> parameters, and that its behavior would be entirely restrictive.
> 
> I'd like to understand the security impact more fully, though.  Can the 
> eBPF code make arbitrary writes to the kernel, or read anything other than 
> the correctly bounded LSM hook parameters?
> 

As mentioned, the BPF verifier does not allow writes to BTF types.

> > And a traditional security module would necessarily fall
> > under GPL; is the eBPF code required to be likewise?  If not, KRSI is a
> > gateway for proprietary LSMs...
> 
> Right, we do not want this to be a GPL bypass.

This is not intended to be a GPL bypass and the BPF verifier checks
for license compatibility of the loaded program with GPL.

- KP

> 
> If these issues can be resolved, this may be a "safe" way to support 
> loadable LSM applications.
> 
> Again, I'd be interested in knowing how this is is handled in the 
> networking stack (keeping in mind that LSM is a much more invasive API, 
> and may not be directly comparable).
> 
> -- 
> James Morris
> <jmorris@namei.org>
>
Stephen Smalley Jan. 9, 2020, 7:47 p.m. UTC | #23
On 1/9/20 2:43 PM, KP Singh wrote:
> On 10-Jan 06:07, James Morris wrote:
>> On Thu, 9 Jan 2020, Stephen Smalley wrote:
>>
>>> On 1/9/20 1:11 PM, James Morris wrote:
>>>> On Wed, 8 Jan 2020, Stephen Smalley wrote:
>>>>
>>>>> The cover letter subject line and the Kconfig help text refer to it as a
>>>>> BPF-based "MAC and Audit policy".  It has an enforce config option that
>>>>> enables the bpf programs to deny access, providing access control. IIRC,
>>>>> in
>>>>> the earlier discussion threads, the BPF maintainers suggested that Smack
>>>>> and
>>>>> other LSMs could be entirely re-implemented via it in the future, and that
>>>>> such an implementation would be more optimal.
>>>>
>>>> In this case, the eBPF code is similar to a kernel module, rather than a
>>>> loadable policy file.  It's a loadable mechanism, rather than a policy, in
>>>> my view.
>>>
>>> I thought you frowned on dynamically loadable LSMs for both security and
>>> correctness reasons?
> 
> Based on the feedback from the lists we've updated the design for v2.
> 
> In v2, LSM hook callbacks are allocated dynamically using BPF
> trampolines, appended to a separate security_hook_heads and run
> only after the statically allocated hooks.
> 
> The security_hook_heads for all the other LSMs (SELinux, AppArmor etc)
> still remains __lsm_ro_after_init and cannot be modified. We are still
> working on v2 (not ready for review yet) but the general idea can be
> seen here:
> 
>    https://github.com/sinkap/linux-krsi/blob/patch/v1/trampoline_prototype/security/bpf/lsm.c
> 
>>
>> Evaluating the security impact of this is the next step. My understanding
>> is that eBPF via BTF is constrained to read only access to hook
>> parameters, and that its behavior would be entirely restrictive.
>>
>> I'd like to understand the security impact more fully, though.  Can the
>> eBPF code make arbitrary writes to the kernel, or read anything other than
>> the correctly bounded LSM hook parameters?
>>
> 
> As mentioned, the BPF verifier does not allow writes to BTF types.
> 
>>> And a traditional security module would necessarily fall
>>> under GPL; is the eBPF code required to be likewise?  If not, KRSI is a
>>> gateway for proprietary LSMs...
>>
>> Right, we do not want this to be a GPL bypass.
> 
> This is not intended to be a GPL bypass and the BPF verifier checks
> for license compatibility of the loaded program with GPL.

IIUC, it checks that the program is GPL compatible if it uses a function 
marked GPL-only.  But what specifically is marked GPL-only that is 
required for eBPF programs using KRSI?

> 
> - KP
> 
>>
>> If these issues can be resolved, this may be a "safe" way to support
>> loadable LSM applications.
>>
>> Again, I'd be interested in knowing how this is is handled in the
>> networking stack (keeping in mind that LSM is a much more invasive API,
>> and may not be directly comparable).
>>
>> -- 
>> James Morris
>> <jmorris@namei.org>
>>
KP Singh Jan. 10, 2020, 3:27 p.m. UTC | #24
On 09-Jan 14:47, Stephen Smalley wrote:
> On 1/9/20 2:43 PM, KP Singh wrote:
> > On 10-Jan 06:07, James Morris wrote:
> > > On Thu, 9 Jan 2020, Stephen Smalley wrote:
> > > 
> > > > On 1/9/20 1:11 PM, James Morris wrote:
> > > > > On Wed, 8 Jan 2020, Stephen Smalley wrote:
> > > > > 
> > > > > > The cover letter subject line and the Kconfig help text refer to it as a
> > > > > > BPF-based "MAC and Audit policy".  It has an enforce config option that
> > > > > > enables the bpf programs to deny access, providing access control. IIRC,
> > > > > > in
> > > > > > the earlier discussion threads, the BPF maintainers suggested that Smack
> > > > > > and
> > > > > > other LSMs could be entirely re-implemented via it in the future, and that
> > > > > > such an implementation would be more optimal.
> > > > > 
> > > > > In this case, the eBPF code is similar to a kernel module, rather than a
> > > > > loadable policy file.  It's a loadable mechanism, rather than a policy, in
> > > > > my view.
> > > > 
> > > > I thought you frowned on dynamically loadable LSMs for both security and
> > > > correctness reasons?
> > 
> > Based on the feedback from the lists we've updated the design for v2.
> > 
> > In v2, LSM hook callbacks are allocated dynamically using BPF
> > trampolines, appended to a separate security_hook_heads and run
> > only after the statically allocated hooks.
> > 
> > The security_hook_heads for all the other LSMs (SELinux, AppArmor etc)
> > still remains __lsm_ro_after_init and cannot be modified. We are still
> > working on v2 (not ready for review yet) but the general idea can be
> > seen here:
> > 
> >    https://github.com/sinkap/linux-krsi/blob/patch/v1/trampoline_prototype/security/bpf/lsm.c
> > 
> > > 
> > > Evaluating the security impact of this is the next step. My understanding
> > > is that eBPF via BTF is constrained to read only access to hook
> > > parameters, and that its behavior would be entirely restrictive.
> > > 
> > > I'd like to understand the security impact more fully, though.  Can the
> > > eBPF code make arbitrary writes to the kernel, or read anything other than
> > > the correctly bounded LSM hook parameters?
> > > 
> > 
> > As mentioned, the BPF verifier does not allow writes to BTF types.
> > 
> > > > And a traditional security module would necessarily fall
> > > > under GPL; is the eBPF code required to be likewise?  If not, KRSI is a
> > > > gateway for proprietary LSMs...
> > > 
> > > Right, we do not want this to be a GPL bypass.
> > 
> > This is not intended to be a GPL bypass and the BPF verifier checks
> > for license compatibility of the loaded program with GPL.
> 
> IIUC, it checks that the program is GPL compatible if it uses a function
> marked GPL-only.  But what specifically is marked GPL-only that is required
> for eBPF programs using KRSI?

Good point! If no-one objects, I can add it to the BPF_PROG_TYPE_LSM
specific verification for the v2 of the patch-set which would require
all BPF-LSM programs to be GPL.

- KP

> 
> > 
> > - KP
> > 
> > > 
> > > If these issues can be resolved, this may be a "safe" way to support
> > > loadable LSM applications.
> > > 
> > > Again, I'd be interested in knowing how this is is handled in the
> > > networking stack (keeping in mind that LSM is a much more invasive API,
> > > and may not be directly comparable).
> > > 
> > > -- 
> > > James Morris
> > > <jmorris@namei.org>
> > > 
>
James Morris Jan. 10, 2020, 5:48 p.m. UTC | #25
On Fri, 10 Jan 2020, KP Singh wrote:

> 
> Good point! If no-one objects, I can add it to the BPF_PROG_TYPE_LSM
> specific verification for the v2 of the patch-set which would require
> all BPF-LSM programs to be GPL.

Sounds good to me.
Alexei Starovoitov Jan. 10, 2020, 5:53 p.m. UTC | #26
On Fri, Jan 10, 2020 at 04:27:58PM +0100, KP Singh wrote:
> On 09-Jan 14:47, Stephen Smalley wrote:
> > On 1/9/20 2:43 PM, KP Singh wrote:
> > > On 10-Jan 06:07, James Morris wrote:
> > > > On Thu, 9 Jan 2020, Stephen Smalley wrote:
> > > > 
> > > > > On 1/9/20 1:11 PM, James Morris wrote:
> > > > > > On Wed, 8 Jan 2020, Stephen Smalley wrote:
> > > > > > 
> > > > > > > The cover letter subject line and the Kconfig help text refer to it as a
> > > > > > > BPF-based "MAC and Audit policy".  It has an enforce config option that
> > > > > > > enables the bpf programs to deny access, providing access control. IIRC,
> > > > > > > in
> > > > > > > the earlier discussion threads, the BPF maintainers suggested that Smack
> > > > > > > and
> > > > > > > other LSMs could be entirely re-implemented via it in the future, and that
> > > > > > > such an implementation would be more optimal.
> > > > > > 
> > > > > > In this case, the eBPF code is similar to a kernel module, rather than a
> > > > > > loadable policy file.  It's a loadable mechanism, rather than a policy, in
> > > > > > my view.
> > > > > 
> > > > > I thought you frowned on dynamically loadable LSMs for both security and
> > > > > correctness reasons?
> > > 
> > > Based on the feedback from the lists we've updated the design for v2.
> > > 
> > > In v2, LSM hook callbacks are allocated dynamically using BPF
> > > trampolines, appended to a separate security_hook_heads and run
> > > only after the statically allocated hooks.
> > > 
> > > The security_hook_heads for all the other LSMs (SELinux, AppArmor etc)
> > > still remains __lsm_ro_after_init and cannot be modified. We are still
> > > working on v2 (not ready for review yet) but the general idea can be
> > > seen here:
> > > 
> > >    https://github.com/sinkap/linux-krsi/blob/patch/v1/trampoline_prototype/security/bpf/lsm.c
> > > 
> > > > 
> > > > Evaluating the security impact of this is the next step. My understanding
> > > > is that eBPF via BTF is constrained to read only access to hook
> > > > parameters, and that its behavior would be entirely restrictive.
> > > > 
> > > > I'd like to understand the security impact more fully, though.  Can the
> > > > eBPF code make arbitrary writes to the kernel, or read anything other than
> > > > the correctly bounded LSM hook parameters?
> > > > 
> > > 
> > > As mentioned, the BPF verifier does not allow writes to BTF types.
> > > 
> > > > > And a traditional security module would necessarily fall
> > > > > under GPL; is the eBPF code required to be likewise?  If not, KRSI is a
> > > > > gateway for proprietary LSMs...
> > > > 
> > > > Right, we do not want this to be a GPL bypass.
> > > 
> > > This is not intended to be a GPL bypass and the BPF verifier checks
> > > for license compatibility of the loaded program with GPL.
> > 
> > IIUC, it checks that the program is GPL compatible if it uses a function
> > marked GPL-only.  But what specifically is marked GPL-only that is required
> > for eBPF programs using KRSI?
> 
> Good point! If no-one objects, I can add it to the BPF_PROG_TYPE_LSM
> specific verification for the v2 of the patch-set which would require
> all BPF-LSM programs to be GPL.

I don't think it's a good idea to enforce license on the program.
The kernel doesn't do it for modules.
For years all of BPF tracing progs were GPL because they have to use
GPL-ed helpers to do anything meaningful.
So for KRSI just make sure that all helpers are GPL-ed as well.
Stephen Smalley Jan. 14, 2020, 4:54 p.m. UTC | #27
On 1/10/20 12:53 PM, Alexei Starovoitov wrote:
> On Fri, Jan 10, 2020 at 04:27:58PM +0100, KP Singh wrote:
>> On 09-Jan 14:47, Stephen Smalley wrote:
>>> On 1/9/20 2:43 PM, KP Singh wrote:
>>>> On 10-Jan 06:07, James Morris wrote:
>>>>> On Thu, 9 Jan 2020, Stephen Smalley wrote:
>>>>>
>>>>>> On 1/9/20 1:11 PM, James Morris wrote:
>>>>>>> On Wed, 8 Jan 2020, Stephen Smalley wrote:
>>>>>>>
>>>>>>>> The cover letter subject line and the Kconfig help text refer to it as a
>>>>>>>> BPF-based "MAC and Audit policy".  It has an enforce config option that
>>>>>>>> enables the bpf programs to deny access, providing access control. IIRC,
>>>>>>>> in
>>>>>>>> the earlier discussion threads, the BPF maintainers suggested that Smack
>>>>>>>> and
>>>>>>>> other LSMs could be entirely re-implemented via it in the future, and that
>>>>>>>> such an implementation would be more optimal.
>>>>>>>
>>>>>>> In this case, the eBPF code is similar to a kernel module, rather than a
>>>>>>> loadable policy file.  It's a loadable mechanism, rather than a policy, in
>>>>>>> my view.
>>>>>>
>>>>>> I thought you frowned on dynamically loadable LSMs for both security and
>>>>>> correctness reasons?
>>>>
>>>> Based on the feedback from the lists we've updated the design for v2.
>>>>
>>>> In v2, LSM hook callbacks are allocated dynamically using BPF
>>>> trampolines, appended to a separate security_hook_heads and run
>>>> only after the statically allocated hooks.
>>>>
>>>> The security_hook_heads for all the other LSMs (SELinux, AppArmor etc)
>>>> still remains __lsm_ro_after_init and cannot be modified. We are still
>>>> working on v2 (not ready for review yet) but the general idea can be
>>>> seen here:
>>>>
>>>>     https://github.com/sinkap/linux-krsi/blob/patch/v1/trampoline_prototype/security/bpf/lsm.c
>>>>
>>>>>
>>>>> Evaluating the security impact of this is the next step. My understanding
>>>>> is that eBPF via BTF is constrained to read only access to hook
>>>>> parameters, and that its behavior would be entirely restrictive.
>>>>>
>>>>> I'd like to understand the security impact more fully, though.  Can the
>>>>> eBPF code make arbitrary writes to the kernel, or read anything other than
>>>>> the correctly bounded LSM hook parameters?
>>>>>
>>>>
>>>> As mentioned, the BPF verifier does not allow writes to BTF types.
>>>>
>>>>>> And a traditional security module would necessarily fall
>>>>>> under GPL; is the eBPF code required to be likewise?  If not, KRSI is a
>>>>>> gateway for proprietary LSMs...
>>>>>
>>>>> Right, we do not want this to be a GPL bypass.
>>>>
>>>> This is not intended to be a GPL bypass and the BPF verifier checks
>>>> for license compatibility of the loaded program with GPL.
>>>
>>> IIUC, it checks that the program is GPL compatible if it uses a function
>>> marked GPL-only.  But what specifically is marked GPL-only that is required
>>> for eBPF programs using KRSI?
>>
>> Good point! If no-one objects, I can add it to the BPF_PROG_TYPE_LSM
>> specific verification for the v2 of the patch-set which would require
>> all BPF-LSM programs to be GPL.
> 
> I don't think it's a good idea to enforce license on the program.
> The kernel doesn't do it for modules.
> For years all of BPF tracing progs were GPL because they have to use
> GPL-ed helpers to do anything meaningful.
> So for KRSI just make sure that all helpers are GPL-ed as well.

IIUC, the example eBPF code included in this patch series showed a 
program that used a GPL-only helper for the purpose of reporting event 
output to userspace. But it could have just as easily omitted the use of 
that helper and still implemented its own arbitrary access control model 
on the LSM hooks to which it attached.  It seems like the question is 
whether the kernel developers are ok with exposing the entire LSM hook 
interface and all the associated data structures to non-GPLd code, 
irrespective of what helpers it may or may not use.
Stephen Smalley Jan. 14, 2020, 5:42 p.m. UTC | #28
On 1/14/20 11:54 AM, Stephen Smalley wrote:
> On 1/10/20 12:53 PM, Alexei Starovoitov wrote:
>> On Fri, Jan 10, 2020 at 04:27:58PM +0100, KP Singh wrote:
>>> On 09-Jan 14:47, Stephen Smalley wrote:
>>>> On 1/9/20 2:43 PM, KP Singh wrote:
>>>>> On 10-Jan 06:07, James Morris wrote:
>>>>>> On Thu, 9 Jan 2020, Stephen Smalley wrote:
>>>>>>
>>>>>>> On 1/9/20 1:11 PM, James Morris wrote:
>>>>>>>> On Wed, 8 Jan 2020, Stephen Smalley wrote:
>>>>>>>>
>>>>>>>>> The cover letter subject line and the Kconfig help text refer 
>>>>>>>>> to it as a
>>>>>>>>> BPF-based "MAC and Audit policy".  It has an enforce config 
>>>>>>>>> option that
>>>>>>>>> enables the bpf programs to deny access, providing access 
>>>>>>>>> control. IIRC,
>>>>>>>>> in
>>>>>>>>> the earlier discussion threads, the BPF maintainers suggested 
>>>>>>>>> that Smack
>>>>>>>>> and
>>>>>>>>> other LSMs could be entirely re-implemented via it in the 
>>>>>>>>> future, and that
>>>>>>>>> such an implementation would be more optimal.
>>>>>>>>
>>>>>>>> In this case, the eBPF code is similar to a kernel module, 
>>>>>>>> rather than a
>>>>>>>> loadable policy file.  It's a loadable mechanism, rather than a 
>>>>>>>> policy, in
>>>>>>>> my view.
>>>>>>>
>>>>>>> I thought you frowned on dynamically loadable LSMs for both 
>>>>>>> security and
>>>>>>> correctness reasons?
>>>>>
>>>>> Based on the feedback from the lists we've updated the design for v2.
>>>>>
>>>>> In v2, LSM hook callbacks are allocated dynamically using BPF
>>>>> trampolines, appended to a separate security_hook_heads and run
>>>>> only after the statically allocated hooks.
>>>>>
>>>>> The security_hook_heads for all the other LSMs (SELinux, AppArmor etc)
>>>>> still remains __lsm_ro_after_init and cannot be modified. We are still
>>>>> working on v2 (not ready for review yet) but the general idea can be
>>>>> seen here:
>>>>>
>>>>>     
>>>>> https://github.com/sinkap/linux-krsi/blob/patch/v1/trampoline_prototype/security/bpf/lsm.c 
>>>>>
>>>>>
>>>>>>
>>>>>> Evaluating the security impact of this is the next step. My 
>>>>>> understanding
>>>>>> is that eBPF via BTF is constrained to read only access to hook
>>>>>> parameters, and that its behavior would be entirely restrictive.
>>>>>>
>>>>>> I'd like to understand the security impact more fully, though.  
>>>>>> Can the
>>>>>> eBPF code make arbitrary writes to the kernel, or read anything 
>>>>>> other than
>>>>>> the correctly bounded LSM hook parameters?
>>>>>>
>>>>>
>>>>> As mentioned, the BPF verifier does not allow writes to BTF types.
>>>>>
>>>>>>> And a traditional security module would necessarily fall
>>>>>>> under GPL; is the eBPF code required to be likewise?  If not, 
>>>>>>> KRSI is a
>>>>>>> gateway for proprietary LSMs...
>>>>>>
>>>>>> Right, we do not want this to be a GPL bypass.
>>>>>
>>>>> This is not intended to be a GPL bypass and the BPF verifier checks
>>>>> for license compatibility of the loaded program with GPL.
>>>>
>>>> IIUC, it checks that the program is GPL compatible if it uses a 
>>>> function
>>>> marked GPL-only.  But what specifically is marked GPL-only that is 
>>>> required
>>>> for eBPF programs using KRSI?
>>>
>>> Good point! If no-one objects, I can add it to the BPF_PROG_TYPE_LSM
>>> specific verification for the v2 of the patch-set which would require
>>> all BPF-LSM programs to be GPL.
>>
>> I don't think it's a good idea to enforce license on the program.
>> The kernel doesn't do it for modules.
>> For years all of BPF tracing progs were GPL because they have to use
>> GPL-ed helpers to do anything meaningful.
>> So for KRSI just make sure that all helpers are GPL-ed as well.
> 
> IIUC, the example eBPF code included in this patch series showed a 
> program that used a GPL-only helper for the purpose of reporting event 
> output to userspace. But it could have just as easily omitted the use of 
> that helper and still implemented its own arbitrary access control model 
> on the LSM hooks to which it attached.  It seems like the question is 
> whether the kernel developers are ok with exposing the entire LSM hook 
> interface and all the associated data structures to non-GPLd code, 
> irrespective of what helpers it may or may not use.

Also, to be clear, while kernel modules aren't necessarily GPL, prior to 
this patch series, all Linux security modules were necessarily GPLd in 
order to use the LSM interface.  So allowing non-GPL eBPF-based LSMs 
would be a change.
Alexei Starovoitov Jan. 15, 2020, 2:48 a.m. UTC | #29
On Tue, Jan 14, 2020 at 12:42:22PM -0500, Stephen Smalley wrote:
> On 1/14/20 11:54 AM, Stephen Smalley wrote:
> > On 1/10/20 12:53 PM, Alexei Starovoitov wrote:
> > > On Fri, Jan 10, 2020 at 04:27:58PM +0100, KP Singh wrote:
> > > > On 09-Jan 14:47, Stephen Smalley wrote:
> > > > > On 1/9/20 2:43 PM, KP Singh wrote:
> > > > > > On 10-Jan 06:07, James Morris wrote:
> > > > > > > On Thu, 9 Jan 2020, Stephen Smalley wrote:
> > > > > > > 
> > > > > > > > On 1/9/20 1:11 PM, James Morris wrote:
> > > > > > > > > On Wed, 8 Jan 2020, Stephen Smalley wrote:
> > > > > > > > > 
> > > > > > > > > > The cover letter subject line and the
> > > > > > > > > > Kconfig help text refer to it as a
> > > > > > > > > > BPF-based "MAC and Audit policy".  It
> > > > > > > > > > has an enforce config option that
> > > > > > > > > > enables the bpf programs to deny access,
> > > > > > > > > > providing access control. IIRC,
> > > > > > > > > > in
> > > > > > > > > > the earlier discussion threads, the BPF
> > > > > > > > > > maintainers suggested that Smack
> > > > > > > > > > and
> > > > > > > > > > other LSMs could be entirely
> > > > > > > > > > re-implemented via it in the future, and
> > > > > > > > > > that
> > > > > > > > > > such an implementation would be more optimal.
> > > > > > > > > 
> > > > > > > > > In this case, the eBPF code is similar to a
> > > > > > > > > kernel module, rather than a
> > > > > > > > > loadable policy file.  It's a loadable
> > > > > > > > > mechanism, rather than a policy, in
> > > > > > > > > my view.
> > > > > > > > 
> > > > > > > > I thought you frowned on dynamically loadable
> > > > > > > > LSMs for both security and
> > > > > > > > correctness reasons?
> > > > > > 
> > > > > > Based on the feedback from the lists we've updated the design for v2.
> > > > > > 
> > > > > > In v2, LSM hook callbacks are allocated dynamically using BPF
> > > > > > trampolines, appended to a separate security_hook_heads and run
> > > > > > only after the statically allocated hooks.
> > > > > > 
> > > > > > The security_hook_heads for all the other LSMs (SELinux, AppArmor etc)
> > > > > > still remains __lsm_ro_after_init and cannot be modified. We are still
> > > > > > working on v2 (not ready for review yet) but the general idea can be
> > > > > > seen here:
> > > > > > 
> > > > > >      https://github.com/sinkap/linux-krsi/blob/patch/v1/trampoline_prototype/security/bpf/lsm.c
> > > > > > 
> > > > > > 
> > > > > > > 
> > > > > > > Evaluating the security impact of this is the next
> > > > > > > step. My understanding
> > > > > > > is that eBPF via BTF is constrained to read only access to hook
> > > > > > > parameters, and that its behavior would be entirely restrictive.
> > > > > > > 
> > > > > > > I'd like to understand the security impact more
> > > > > > > fully, though.  Can the
> > > > > > > eBPF code make arbitrary writes to the kernel, or
> > > > > > > read anything other than
> > > > > > > the correctly bounded LSM hook parameters?
> > > > > > > 
> > > > > > 
> > > > > > As mentioned, the BPF verifier does not allow writes to BTF types.
> > > > > > 
> > > > > > > > And a traditional security module would necessarily fall
> > > > > > > > under GPL; is the eBPF code required to be
> > > > > > > > likewise?  If not, KRSI is a
> > > > > > > > gateway for proprietary LSMs...
> > > > > > > 
> > > > > > > Right, we do not want this to be a GPL bypass.
> > > > > > 
> > > > > > This is not intended to be a GPL bypass and the BPF verifier checks
> > > > > > for license compatibility of the loaded program with GPL.
> > > > > 
> > > > > IIUC, it checks that the program is GPL compatible if it
> > > > > uses a function
> > > > > marked GPL-only.  But what specifically is marked GPL-only
> > > > > that is required
> > > > > for eBPF programs using KRSI?
> > > > 
> > > > Good point! If no-one objects, I can add it to the BPF_PROG_TYPE_LSM
> > > > specific verification for the v2 of the patch-set which would require
> > > > all BPF-LSM programs to be GPL.
> > > 
> > > I don't think it's a good idea to enforce license on the program.
> > > The kernel doesn't do it for modules.
> > > For years all of BPF tracing progs were GPL because they have to use
> > > GPL-ed helpers to do anything meaningful.
> > > So for KRSI just make sure that all helpers are GPL-ed as well.
> > 
> > IIUC, the example eBPF code included in this patch series showed a
> > program that used a GPL-only helper for the purpose of reporting event
> > output to userspace. But it could have just as easily omitted the use of
> > that helper and still implemented its own arbitrary access control model
> > on the LSM hooks to which it attached.  It seems like the question is
> > whether the kernel developers are ok with exposing the entire LSM hook
> > interface and all the associated data structures to non-GPLd code,
> > irrespective of what helpers it may or may not use.
> 
> Also, to be clear, while kernel modules aren't necessarily GPL, prior to
> this patch series, all Linux security modules were necessarily GPLd in order
> to use the LSM interface. 

Because they use securityfs_create_file() GPL-ed api, right?
but not because module license is enforced.

> So allowing non-GPL eBPF-based LSMs would be a
> change.

I don't see it this way. seccomp progs technically unlicensed. Yet they can
disallow any syscall. Primitive KRSI progs like
int bpf-prog(void*) { return REJECT; }
would be able to do selectively disable a syscall with an overhead acceptable
in production systems (unlike seccomp). I want this use case to be available to
people. It's a bait, because to do real progs people would need to GPL them.
Key helpers bpf_perf_event_output, bpf_ktime_get_ns, bpf_trace_printk are all
GPL-ed. It may look that most networking helpers are not-GPL, but real life is
different. To debug programs bpf_trace_printk() is necessary. To have
communication with user space bpf_perf_event_output() is necssary. To measure
anything or implement timestamps bpf_ktime_get_ns() is necessary. So today all
meaninful bpf programs are GPL. Those that are not GPL probably exist, but
they're toy programs. Hence I have zero concerns about GPL bypass coming from
tracing, networking, and, in the future, KRSI progs too.
Stephen Smalley Jan. 15, 2020, 1:59 p.m. UTC | #30
On 1/14/20 9:48 PM, Alexei Starovoitov wrote:
> On Tue, Jan 14, 2020 at 12:42:22PM -0500, Stephen Smalley wrote:
>> On 1/14/20 11:54 AM, Stephen Smalley wrote:
>>> On 1/10/20 12:53 PM, Alexei Starovoitov wrote:
>>>> On Fri, Jan 10, 2020 at 04:27:58PM +0100, KP Singh wrote:
>>>>> On 09-Jan 14:47, Stephen Smalley wrote:
>>>>>> On 1/9/20 2:43 PM, KP Singh wrote:
>>>>>>> On 10-Jan 06:07, James Morris wrote:
>>>>>>>> On Thu, 9 Jan 2020, Stephen Smalley wrote:
>>>>>>>>
>>>>>>>>> On 1/9/20 1:11 PM, James Morris wrote:
>>>>>>>>>> On Wed, 8 Jan 2020, Stephen Smalley wrote:
>>>>>>>>>>
>>>>>>>>>>> The cover letter subject line and the
>>>>>>>>>>> Kconfig help text refer to it as a
>>>>>>>>>>> BPF-based "MAC and Audit policy".  It
>>>>>>>>>>> has an enforce config option that
>>>>>>>>>>> enables the bpf programs to deny access,
>>>>>>>>>>> providing access control. IIRC,
>>>>>>>>>>> in
>>>>>>>>>>> the earlier discussion threads, the BPF
>>>>>>>>>>> maintainers suggested that Smack
>>>>>>>>>>> and
>>>>>>>>>>> other LSMs could be entirely
>>>>>>>>>>> re-implemented via it in the future, and
>>>>>>>>>>> that
>>>>>>>>>>> such an implementation would be more optimal.
>>>>>>>>>>
>>>>>>>>>> In this case, the eBPF code is similar to a
>>>>>>>>>> kernel module, rather than a
>>>>>>>>>> loadable policy file.  It's a loadable
>>>>>>>>>> mechanism, rather than a policy, in
>>>>>>>>>> my view.
>>>>>>>>>
>>>>>>>>> I thought you frowned on dynamically loadable
>>>>>>>>> LSMs for both security and
>>>>>>>>> correctness reasons?
>>>>>>>
>>>>>>> Based on the feedback from the lists we've updated the design for v2.
>>>>>>>
>>>>>>> In v2, LSM hook callbacks are allocated dynamically using BPF
>>>>>>> trampolines, appended to a separate security_hook_heads and run
>>>>>>> only after the statically allocated hooks.
>>>>>>>
>>>>>>> The security_hook_heads for all the other LSMs (SELinux, AppArmor etc)
>>>>>>> still remains __lsm_ro_after_init and cannot be modified. We are still
>>>>>>> working on v2 (not ready for review yet) but the general idea can be
>>>>>>> seen here:
>>>>>>>
>>>>>>>       https://github.com/sinkap/linux-krsi/blob/patch/v1/trampoline_prototype/security/bpf/lsm.c
>>>>>>>
>>>>>>>
>>>>>>>>
>>>>>>>> Evaluating the security impact of this is the next
>>>>>>>> step. My understanding
>>>>>>>> is that eBPF via BTF is constrained to read only access to hook
>>>>>>>> parameters, and that its behavior would be entirely restrictive.
>>>>>>>>
>>>>>>>> I'd like to understand the security impact more
>>>>>>>> fully, though.  Can the
>>>>>>>> eBPF code make arbitrary writes to the kernel, or
>>>>>>>> read anything other than
>>>>>>>> the correctly bounded LSM hook parameters?
>>>>>>>>
>>>>>>>
>>>>>>> As mentioned, the BPF verifier does not allow writes to BTF types.
>>>>>>>
>>>>>>>>> And a traditional security module would necessarily fall
>>>>>>>>> under GPL; is the eBPF code required to be
>>>>>>>>> likewise?  If not, KRSI is a
>>>>>>>>> gateway for proprietary LSMs...
>>>>>>>>
>>>>>>>> Right, we do not want this to be a GPL bypass.
>>>>>>>
>>>>>>> This is not intended to be a GPL bypass and the BPF verifier checks
>>>>>>> for license compatibility of the loaded program with GPL.
>>>>>>
>>>>>> IIUC, it checks that the program is GPL compatible if it
>>>>>> uses a function
>>>>>> marked GPL-only.  But what specifically is marked GPL-only
>>>>>> that is required
>>>>>> for eBPF programs using KRSI?
>>>>>
>>>>> Good point! If no-one objects, I can add it to the BPF_PROG_TYPE_LSM
>>>>> specific verification for the v2 of the patch-set which would require
>>>>> all BPF-LSM programs to be GPL.
>>>>
>>>> I don't think it's a good idea to enforce license on the program.
>>>> The kernel doesn't do it for modules.
>>>> For years all of BPF tracing progs were GPL because they have to use
>>>> GPL-ed helpers to do anything meaningful.
>>>> So for KRSI just make sure that all helpers are GPL-ed as well.
>>>
>>> IIUC, the example eBPF code included in this patch series showed a
>>> program that used a GPL-only helper for the purpose of reporting event
>>> output to userspace. But it could have just as easily omitted the use of
>>> that helper and still implemented its own arbitrary access control model
>>> on the LSM hooks to which it attached.  It seems like the question is
>>> whether the kernel developers are ok with exposing the entire LSM hook
>>> interface and all the associated data structures to non-GPLd code,
>>> irrespective of what helpers it may or may not use.
>>
>> Also, to be clear, while kernel modules aren't necessarily GPL, prior to
>> this patch series, all Linux security modules were necessarily GPLd in order
>> to use the LSM interface.
> 
> Because they use securityfs_create_file() GPL-ed api, right?
> but not because module license is enforced.

No, securityfs was a later addition and is not required by all LSMs 
either.  Originally LSMs had to register their hooks via 
register_security(), which was intentionally EXPORT_SYMBOL_GPL() to 
avoid exposing the LSM interface to non-GPLd modules because there were 
significant concerns with doing so when LSM was first merged.  Then in 
20510f2f4e2dabb0ff6c13901807627ec9452f98 ("security: Convert LSM into a 
static interface"), the ability for loadable modules to use 
register_security() at all was removed, limiting its use to built-in 
modules.  In commit b1d9e6b0646d0e5ee5d9050bd236b6c65d66faef ("LSM: 
Switch to lists of hooks"), register_security() was replaced by 
security_add_hooks(), but this was likewise not exported for use by 
modules and could only be used by built-in code.  The bpf LSM is 
providing a shim that allows eBPF code to attach to these hooks that 
would otherwise not be exposed to non-GPLd code, so if the bpf LSM does 
not require the eBPF programs to also be GPLd, then that is a change 
from current practice.

>> So allowing non-GPL eBPF-based LSMs would be a
>> change.
> 
> I don't see it this way. seccomp progs technically unlicensed. Yet they can
> disallow any syscall. Primitive KRSI progs like
> int bpf-prog(void*) { return REJECT; }
> would be able to do selectively disable a syscall with an overhead acceptable
> in production systems (unlike seccomp). I want this use case to be available to
> people. It's a bait, because to do real progs people would need to GPL them.
> Key helpers bpf_perf_event_output, bpf_ktime_get_ns, bpf_trace_printk are all
> GPL-ed. It may look that most networking helpers are not-GPL, but real life is
> different. To debug programs bpf_trace_printk() is necessary. To have
> communication with user space bpf_perf_event_output() is necssary. To measure
> anything or implement timestamps bpf_ktime_get_ns() is necessary. So today all
> meaninful bpf programs are GPL. Those that are not GPL probably exist, but
> they're toy programs. Hence I have zero concerns about GPL bypass coming from
> tracing, networking, and, in the future, KRSI progs too.

You have more confidence than I do about that.  I would anticipate 
developers of out-of-tree LSMs latching onto this bpf LSM and using it 
to avoid GPL.  I don't see that any of those helpers are truly needed to 
implement an access control model.
Greg KH Jan. 15, 2020, 2:09 p.m. UTC | #31
On Wed, Jan 15, 2020 at 08:59:08AM -0500, Stephen Smalley wrote:
> On 1/14/20 9:48 PM, Alexei Starovoitov wrote:
> > On Tue, Jan 14, 2020 at 12:42:22PM -0500, Stephen Smalley wrote:
> > > On 1/14/20 11:54 AM, Stephen Smalley wrote:
> > > > On 1/10/20 12:53 PM, Alexei Starovoitov wrote:
> > > > > On Fri, Jan 10, 2020 at 04:27:58PM +0100, KP Singh wrote:
> > > > > > On 09-Jan 14:47, Stephen Smalley wrote:
> > > > > > > On 1/9/20 2:43 PM, KP Singh wrote:
> > > > > > > > On 10-Jan 06:07, James Morris wrote:
> > > > > > > > > On Thu, 9 Jan 2020, Stephen Smalley wrote:
> > > > > > > > > 
> > > > > > > > > > On 1/9/20 1:11 PM, James Morris wrote:
> > > > > > > > > > > On Wed, 8 Jan 2020, Stephen Smalley wrote:
> > > > > > > > > > > 
> > > > > > > > > > > > The cover letter subject line and the
> > > > > > > > > > > > Kconfig help text refer to it as a
> > > > > > > > > > > > BPF-based "MAC and Audit policy".  It
> > > > > > > > > > > > has an enforce config option that
> > > > > > > > > > > > enables the bpf programs to deny access,
> > > > > > > > > > > > providing access control. IIRC,
> > > > > > > > > > > > in
> > > > > > > > > > > > the earlier discussion threads, the BPF
> > > > > > > > > > > > maintainers suggested that Smack
> > > > > > > > > > > > and
> > > > > > > > > > > > other LSMs could be entirely
> > > > > > > > > > > > re-implemented via it in the future, and
> > > > > > > > > > > > that
> > > > > > > > > > > > such an implementation would be more optimal.
> > > > > > > > > > > 
> > > > > > > > > > > In this case, the eBPF code is similar to a
> > > > > > > > > > > kernel module, rather than a
> > > > > > > > > > > loadable policy file.  It's a loadable
> > > > > > > > > > > mechanism, rather than a policy, in
> > > > > > > > > > > my view.
> > > > > > > > > > 
> > > > > > > > > > I thought you frowned on dynamically loadable
> > > > > > > > > > LSMs for both security and
> > > > > > > > > > correctness reasons?
> > > > > > > > 
> > > > > > > > Based on the feedback from the lists we've updated the design for v2.
> > > > > > > > 
> > > > > > > > In v2, LSM hook callbacks are allocated dynamically using BPF
> > > > > > > > trampolines, appended to a separate security_hook_heads and run
> > > > > > > > only after the statically allocated hooks.
> > > > > > > > 
> > > > > > > > The security_hook_heads for all the other LSMs (SELinux, AppArmor etc)
> > > > > > > > still remains __lsm_ro_after_init and cannot be modified. We are still
> > > > > > > > working on v2 (not ready for review yet) but the general idea can be
> > > > > > > > seen here:
> > > > > > > > 
> > > > > > > >       https://github.com/sinkap/linux-krsi/blob/patch/v1/trampoline_prototype/security/bpf/lsm.c
> > > > > > > > 
> > > > > > > > 
> > > > > > > > > 
> > > > > > > > > Evaluating the security impact of this is the next
> > > > > > > > > step. My understanding
> > > > > > > > > is that eBPF via BTF is constrained to read only access to hook
> > > > > > > > > parameters, and that its behavior would be entirely restrictive.
> > > > > > > > > 
> > > > > > > > > I'd like to understand the security impact more
> > > > > > > > > fully, though.  Can the
> > > > > > > > > eBPF code make arbitrary writes to the kernel, or
> > > > > > > > > read anything other than
> > > > > > > > > the correctly bounded LSM hook parameters?
> > > > > > > > > 
> > > > > > > > 
> > > > > > > > As mentioned, the BPF verifier does not allow writes to BTF types.
> > > > > > > > 
> > > > > > > > > > And a traditional security module would necessarily fall
> > > > > > > > > > under GPL; is the eBPF code required to be
> > > > > > > > > > likewise?  If not, KRSI is a
> > > > > > > > > > gateway for proprietary LSMs...
> > > > > > > > > 
> > > > > > > > > Right, we do not want this to be a GPL bypass.
> > > > > > > > 
> > > > > > > > This is not intended to be a GPL bypass and the BPF verifier checks
> > > > > > > > for license compatibility of the loaded program with GPL.
> > > > > > > 
> > > > > > > IIUC, it checks that the program is GPL compatible if it
> > > > > > > uses a function
> > > > > > > marked GPL-only.  But what specifically is marked GPL-only
> > > > > > > that is required
> > > > > > > for eBPF programs using KRSI?
> > > > > > 
> > > > > > Good point! If no-one objects, I can add it to the BPF_PROG_TYPE_LSM
> > > > > > specific verification for the v2 of the patch-set which would require
> > > > > > all BPF-LSM programs to be GPL.
> > > > > 
> > > > > I don't think it's a good idea to enforce license on the program.
> > > > > The kernel doesn't do it for modules.
> > > > > For years all of BPF tracing progs were GPL because they have to use
> > > > > GPL-ed helpers to do anything meaningful.
> > > > > So for KRSI just make sure that all helpers are GPL-ed as well.
> > > > 
> > > > IIUC, the example eBPF code included in this patch series showed a
> > > > program that used a GPL-only helper for the purpose of reporting event
> > > > output to userspace. But it could have just as easily omitted the use of
> > > > that helper and still implemented its own arbitrary access control model
> > > > on the LSM hooks to which it attached.  It seems like the question is
> > > > whether the kernel developers are ok with exposing the entire LSM hook
> > > > interface and all the associated data structures to non-GPLd code,
> > > > irrespective of what helpers it may or may not use.
> > > 
> > > Also, to be clear, while kernel modules aren't necessarily GPL, prior to
> > > this patch series, all Linux security modules were necessarily GPLd in order
> > > to use the LSM interface.
> > 
> > Because they use securityfs_create_file() GPL-ed api, right?
> > but not because module license is enforced.
> 
> No, securityfs was a later addition and is not required by all LSMs either.
> Originally LSMs had to register their hooks via register_security(), which
> was intentionally EXPORT_SYMBOL_GPL() to avoid exposing the LSM interface to
> non-GPLd modules because there were significant concerns with doing so when
> LSM was first merged.  Then in 20510f2f4e2dabb0ff6c13901807627ec9452f98
> ("security: Convert LSM into a static interface"), the ability for loadable
> modules to use register_security() at all was removed, limiting its use to
> built-in modules.  In commit b1d9e6b0646d0e5ee5d9050bd236b6c65d66faef ("LSM:
> Switch to lists of hooks"), register_security() was replaced by
> security_add_hooks(), but this was likewise not exported for use by modules
> and could only be used by built-in code.  The bpf LSM is providing a shim
> that allows eBPF code to attach to these hooks that would otherwise not be
> exposed to non-GPLd code, so if the bpf LSM does not require the eBPF
> programs to also be GPLd, then that is a change from current practice.
> 
> > > So allowing non-GPL eBPF-based LSMs would be a
> > > change.
> > 
> > I don't see it this way. seccomp progs technically unlicensed. Yet they can
> > disallow any syscall. Primitive KRSI progs like
> > int bpf-prog(void*) { return REJECT; }
> > would be able to do selectively disable a syscall with an overhead acceptable
> > in production systems (unlike seccomp). I want this use case to be available to
> > people. It's a bait, because to do real progs people would need to GPL them.
> > Key helpers bpf_perf_event_output, bpf_ktime_get_ns, bpf_trace_printk are all
> > GPL-ed. It may look that most networking helpers are not-GPL, but real life is
> > different. To debug programs bpf_trace_printk() is necessary. To have
> > communication with user space bpf_perf_event_output() is necssary. To measure
> > anything or implement timestamps bpf_ktime_get_ns() is necessary. So today all
> > meaninful bpf programs are GPL. Those that are not GPL probably exist, but
> > they're toy programs. Hence I have zero concerns about GPL bypass coming from
> > tracing, networking, and, in the future, KRSI progs too.
> 
> You have more confidence than I do about that.  I would anticipate
> developers of out-of-tree LSMs latching onto this bpf LSM and using it to
> avoid GPL.  I don't see that any of those helpers are truly needed to
> implement an access control model.

Yeah, I'm with Stephen here, this should be explicitly marked for
GPL-only bpf code to prevent anyone from trying to route around the LSM
apis we have today.  We have enough problem with companies trying to do
that as-is, let's not give them any other ways to abuse our license.

thanks,

greg k-h
Alexei Starovoitov Jan. 15, 2020, 10:23 p.m. UTC | #32
On Wed, Jan 15, 2020 at 03:09:53PM +0100, Greg Kroah-Hartman wrote:
> On Wed, Jan 15, 2020 at 08:59:08AM -0500, Stephen Smalley wrote:
> > On 1/14/20 9:48 PM, Alexei Starovoitov wrote:
> > > On Tue, Jan 14, 2020 at 12:42:22PM -0500, Stephen Smalley wrote:
> > > > On 1/14/20 11:54 AM, Stephen Smalley wrote:
> > > > > On 1/10/20 12:53 PM, Alexei Starovoitov wrote:
> > > > > > On Fri, Jan 10, 2020 at 04:27:58PM +0100, KP Singh wrote:
> > > > > > > On 09-Jan 14:47, Stephen Smalley wrote:
> > > > > > > > On 1/9/20 2:43 PM, KP Singh wrote:
> > > > > > > > > On 10-Jan 06:07, James Morris wrote:
> > > > > > > > > > On Thu, 9 Jan 2020, Stephen Smalley wrote:
> > > > > > > > > > 
> > > > > > > > > > > On 1/9/20 1:11 PM, James Morris wrote:
> > > > > > > > > > > > On Wed, 8 Jan 2020, Stephen Smalley wrote:
> > > > > > > > > > > > 
> > > > > > > > > > > > > The cover letter subject line and the
> > > > > > > > > > > > > Kconfig help text refer to it as a
> > > > > > > > > > > > > BPF-based "MAC and Audit policy".  It
> > > > > > > > > > > > > has an enforce config option that
> > > > > > > > > > > > > enables the bpf programs to deny access,
> > > > > > > > > > > > > providing access control. IIRC,
> > > > > > > > > > > > > in
> > > > > > > > > > > > > the earlier discussion threads, the BPF
> > > > > > > > > > > > > maintainers suggested that Smack
> > > > > > > > > > > > > and
> > > > > > > > > > > > > other LSMs could be entirely
> > > > > > > > > > > > > re-implemented via it in the future, and
> > > > > > > > > > > > > that
> > > > > > > > > > > > > such an implementation would be more optimal.
> > > > > > > > > > > > 
> > > > > > > > > > > > In this case, the eBPF code is similar to a
> > > > > > > > > > > > kernel module, rather than a
> > > > > > > > > > > > loadable policy file.  It's a loadable
> > > > > > > > > > > > mechanism, rather than a policy, in
> > > > > > > > > > > > my view.
> > > > > > > > > > > 
> > > > > > > > > > > I thought you frowned on dynamically loadable
> > > > > > > > > > > LSMs for both security and
> > > > > > > > > > > correctness reasons?
> > > > > > > > > 
> > > > > > > > > Based on the feedback from the lists we've updated the design for v2.
> > > > > > > > > 
> > > > > > > > > In v2, LSM hook callbacks are allocated dynamically using BPF
> > > > > > > > > trampolines, appended to a separate security_hook_heads and run
> > > > > > > > > only after the statically allocated hooks.
> > > > > > > > > 
> > > > > > > > > The security_hook_heads for all the other LSMs (SELinux, AppArmor etc)
> > > > > > > > > still remains __lsm_ro_after_init and cannot be modified. We are still
> > > > > > > > > working on v2 (not ready for review yet) but the general idea can be
> > > > > > > > > seen here:
> > > > > > > > > 
> > > > > > > > >       https://github.com/sinkap/linux-krsi/blob/patch/v1/trampoline_prototype/security/bpf/lsm.c
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > Evaluating the security impact of this is the next
> > > > > > > > > > step. My understanding
> > > > > > > > > > is that eBPF via BTF is constrained to read only access to hook
> > > > > > > > > > parameters, and that its behavior would be entirely restrictive.
> > > > > > > > > > 
> > > > > > > > > > I'd like to understand the security impact more
> > > > > > > > > > fully, though.  Can the
> > > > > > > > > > eBPF code make arbitrary writes to the kernel, or
> > > > > > > > > > read anything other than
> > > > > > > > > > the correctly bounded LSM hook parameters?
> > > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > As mentioned, the BPF verifier does not allow writes to BTF types.
> > > > > > > > > 
> > > > > > > > > > > And a traditional security module would necessarily fall
> > > > > > > > > > > under GPL; is the eBPF code required to be
> > > > > > > > > > > likewise?  If not, KRSI is a
> > > > > > > > > > > gateway for proprietary LSMs...
> > > > > > > > > > 
> > > > > > > > > > Right, we do not want this to be a GPL bypass.
> > > > > > > > > 
> > > > > > > > > This is not intended to be a GPL bypass and the BPF verifier checks
> > > > > > > > > for license compatibility of the loaded program with GPL.
> > > > > > > > 
> > > > > > > > IIUC, it checks that the program is GPL compatible if it
> > > > > > > > uses a function
> > > > > > > > marked GPL-only.  But what specifically is marked GPL-only
> > > > > > > > that is required
> > > > > > > > for eBPF programs using KRSI?
> > > > > > > 
> > > > > > > Good point! If no-one objects, I can add it to the BPF_PROG_TYPE_LSM
> > > > > > > specific verification for the v2 of the patch-set which would require
> > > > > > > all BPF-LSM programs to be GPL.
> > > > > > 
> > > > > > I don't think it's a good idea to enforce license on the program.
> > > > > > The kernel doesn't do it for modules.
> > > > > > For years all of BPF tracing progs were GPL because they have to use
> > > > > > GPL-ed helpers to do anything meaningful.
> > > > > > So for KRSI just make sure that all helpers are GPL-ed as well.
> > > > > 
> > > > > IIUC, the example eBPF code included in this patch series showed a
> > > > > program that used a GPL-only helper for the purpose of reporting event
> > > > > output to userspace. But it could have just as easily omitted the use of
> > > > > that helper and still implemented its own arbitrary access control model
> > > > > on the LSM hooks to which it attached.  It seems like the question is
> > > > > whether the kernel developers are ok with exposing the entire LSM hook
> > > > > interface and all the associated data structures to non-GPLd code,
> > > > > irrespective of what helpers it may or may not use.
> > > > 
> > > > Also, to be clear, while kernel modules aren't necessarily GPL, prior to
> > > > this patch series, all Linux security modules were necessarily GPLd in order
> > > > to use the LSM interface.
> > > 
> > > Because they use securityfs_create_file() GPL-ed api, right?
> > > but not because module license is enforced.
> > 
> > No, securityfs was a later addition and is not required by all LSMs either.
> > Originally LSMs had to register their hooks via register_security(), which
> > was intentionally EXPORT_SYMBOL_GPL() to avoid exposing the LSM interface to
> > non-GPLd modules because there were significant concerns with doing so when
> > LSM was first merged.  Then in 20510f2f4e2dabb0ff6c13901807627ec9452f98
> > ("security: Convert LSM into a static interface"), the ability for loadable
> > modules to use register_security() at all was removed, limiting its use to
> > built-in modules.  In commit b1d9e6b0646d0e5ee5d9050bd236b6c65d66faef ("LSM:
> > Switch to lists of hooks"), register_security() was replaced by
> > security_add_hooks(), but this was likewise not exported for use by modules
> > and could only be used by built-in code.  The bpf LSM is providing a shim
> > that allows eBPF code to attach to these hooks that would otherwise not be
> > exposed to non-GPLd code, so if the bpf LSM does not require the eBPF
> > programs to also be GPLd, then that is a change from current practice.
> > 
> > > > So allowing non-GPL eBPF-based LSMs would be a
> > > > change.
> > > 
> > > I don't see it this way. seccomp progs technically unlicensed. Yet they can
> > > disallow any syscall. Primitive KRSI progs like
> > > int bpf-prog(void*) { return REJECT; }
> > > would be able to do selectively disable a syscall with an overhead acceptable
> > > in production systems (unlike seccomp). I want this use case to be available to
> > > people. It's a bait, because to do real progs people would need to GPL them.
> > > Key helpers bpf_perf_event_output, bpf_ktime_get_ns, bpf_trace_printk are all
> > > GPL-ed. It may look that most networking helpers are not-GPL, but real life is
> > > different. To debug programs bpf_trace_printk() is necessary. To have
> > > communication with user space bpf_perf_event_output() is necssary. To measure
> > > anything or implement timestamps bpf_ktime_get_ns() is necessary. So today all
> > > meaninful bpf programs are GPL. Those that are not GPL probably exist, but
> > > they're toy programs. Hence I have zero concerns about GPL bypass coming from
> > > tracing, networking, and, in the future, KRSI progs too.
> > 
> > You have more confidence than I do about that.  I would anticipate
> > developers of out-of-tree LSMs latching onto this bpf LSM and using it to
> > avoid GPL.  I don't see that any of those helpers are truly needed to
> > implement an access control model.
> 
> Yeah, I'm with Stephen here, this should be explicitly marked for
> GPL-only bpf code to prevent anyone from trying to route around the LSM
> apis we have today.  We have enough problem with companies trying to do
> that as-is, let's not give them any other ways to abuse our license.

Fine. Let's do per prog type check. We can undo it later when this early
concerns prove to be overblown.