From: KP Singh <[email protected]>
# v3 -> v4
https://lkml.org/lkml/2020/1/23/515
* Moved away from allocating a separate security_hook_heads and adding a
new special case for arch_prepare_bpf_trampoline to using BPF fexit
trampolines called from the right place in the LSM hook and toggled by
static keys based on the discussion in:
https://lore.kernel.org/bpf/CAG48ez25mW+_oCxgCtbiGMX07g_ph79UOJa07h=o_6B6+Q-u5g@mail.gmail.com/
* Since the code does not deal with security_hook_heads anymore, it goes
from "being a BPF LSM" to "BPF program attachment to LSM hooks".
* Added a new test case which ensures that the BPF programs' return value
is reflected by the LSM hook.
# v2 -> v3 does not change the overall design and has some minor fixes:
https://lkml.org/lkml/2020/1/15/843
* LSM_ORDER_LAST is introduced to represent the behaviour of the BPF LSM
* Fixed the inadvertent clobbering of the LSM Hook error codes
* Added GPL license requirement to the commit log
* The lsm_hook_idx is now the more conventional 0-based index
* Some changes were split into a separate patch ("Load btf_vmlinux only
once per object")
https://lore.kernel.org/bpf/[email protected]/
* Addressed Andrii's feedback on the BTF implementation
* Documentation update for using generated vmlinux.h to simplify
programs
* Rebase
# Changes since v1:
https://lkml.org/lkml/2019/12/20/641
* Eliminate the requirement to maintain LSM hooks separately in
security/bpf/hooks.h Use BPF trampolines to dynamically allocate
security hooks
* Drop the use of securityfs as bpftool provides the required
introspection capabilities. Update the tests to use the bpf_skeleton
and global variables
* Use O_CLOEXEC anonymous fds to represent BPF attachment in line with
the other BPF programs with the possibility to use bpf program pinning
in the future to provide "permanent attachment".
* Drop the logic based on prog names for handling re-attachment.
* Drop bpf_lsm_event_output from this series and send it as a separate
patch.
# Motivation
Google does analysis of rich runtime security data 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 allows BPF programs to be attached to LSM hooks 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 patchset introduces a new eBPF (https://docs.cilium.io/en/v1.6/bpf/)
program type BPF_PROG_TYPE_LSM which can only be attached to LSM hooks.
Attachment requires CAP_SYS_ADMIN for loading eBPF programs and
CAP_MAC_ADMIN for modifying MAC policies.
The eBPF programs are attached to nop functions (bpf_lsm_<name>) added
in the LSM hooks (when CONFIG_BPF_LSM) and executed after all the
statically defined hooks (i.e. the ones declared by static LSMs (eg,
SELinux, AppArmor, Smack etc) allow the action. This also ensures that
statically defined LSM hooks retain the behaviour of "being read-only
after init", i.e. __lsm_ro_after_init and do not increase the attack
surface.
The branch into this nop function is guarded with a static key (jump
label) and is only taken when a BPF program is attached to the LSM hook.
eg. for bprm_check_security:
int bpf_lsm_bprm_check_security(struct linux_binprm *bprm)
{
return 0;
}
DEFINE_STATIC_KEY_FALSE(bpf_lsm_key_bprm_check_security)
// Run all static hooks for bprm_check_security and set RC
if (static_key_unlikely(&bpf_lsm_key_bprm_check_security) {
if (RC == 0)
bpf_lsm_bprm_check_security(bprm);
}
Upon attachment, a BPF fexit trampoline is attached to the nop function
and the static key for the LSM hook is enabled. The trampoline has code
to handle the conversion from the signature of the hook to the BPF
context and allows the JIT'ed BPF program to be called as a C function
with the same arguments as the LSM hooks. If the attached 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 or to global eBPF variables or maps and can be
further processed in user-space.
# 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:
// Only declare the structure and fields intended to be used
// in the program
struct vm_area_struct {
unsigned long vm_start;
} __attribute__((preserve_access_index));
// Declare the eBPF program mprotect_audit which attaches to
// to the file_mprotect LSM hook and accepts three arguments.
SEC("lsm/file_mprotect")
int BPF_PROG(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 BTF type information is also used by the BPF
verifier to validate memory accesses within the BPF program and also
prevents arbitrary writes to the kernel memory.
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/[email protected]/).
# 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 (not-ready for review) 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 (8):
bpf: Introduce BPF_PROG_TYPE_LSM
security: Refactor declaration of LSM hooks
bpf: lsm: provide attachment points for BPF LSM programs
bpf: lsm: Add support for enabling/disabling BPF hooks
bpf: lsm: Implement attach, detach and execution
tools/libbpf: Add support for BPF_PROG_TYPE_LSM
bpf: lsm: Add selftests for BPF_PROG_TYPE_LSM
bpf: lsm: Add Documentation
Documentation/bpf/bpf_lsm.rst | 147 +++++
Documentation/bpf/index.rst | 1 +
MAINTAINERS | 1 +
arch/x86/net/bpf_jit_comp.c | 21 +-
include/linux/bpf.h | 7 +
include/linux/bpf_lsm.h | 66 ++
include/linux/bpf_types.h | 4 +
include/linux/lsm_hook_names.h | 353 ++++++++++
include/linux/lsm_hooks.h | 622 +-----------------
include/uapi/linux/bpf.h | 2 +
init/Kconfig | 11 +
kernel/bpf/Makefile | 1 +
kernel/bpf/bpf_lsm.c | 88 +++
kernel/bpf/btf.c | 3 +-
kernel/bpf/syscall.c | 47 +-
kernel/bpf/trampoline.c | 24 +-
kernel/bpf/verifier.c | 19 +-
kernel/trace/bpf_trace.c | 12 +-
security/security.c | 35 +
tools/include/uapi/linux/bpf.h | 2 +
tools/lib/bpf/bpf.c | 3 +-
tools/lib/bpf/libbpf.c | 46 +-
tools/lib/bpf/libbpf.h | 4 +
tools/lib/bpf/libbpf.map | 3 +
tools/lib/bpf/libbpf_probes.c | 1 +
tools/testing/selftests/bpf/lsm_helpers.h | 19 +
.../selftests/bpf/prog_tests/lsm_mprotect.c | 96 +++
.../selftests/bpf/progs/lsm_mprotect_audit.c | 48 ++
.../selftests/bpf/progs/lsm_mprotect_mac.c | 53 ++
29 files changed, 1085 insertions(+), 654 deletions(-)
create mode 100644 Documentation/bpf/bpf_lsm.rst
create mode 100644 include/linux/bpf_lsm.h
create mode 100644 include/linux/lsm_hook_names.h
create mode 100644 kernel/bpf/bpf_lsm.c
create mode 100644 tools/testing/selftests/bpf/lsm_helpers.h
create mode 100644 tools/testing/selftests/bpf/prog_tests/lsm_mprotect.c
create mode 100644 tools/testing/selftests/bpf/progs/lsm_mprotect_audit.c
create mode 100644 tools/testing/selftests/bpf/progs/lsm_mprotect_mac.c
--
2.20.1
On 2/20/2020 9:52 AM, KP Singh wrote:
> From: KP Singh <[email protected]>
Again, apologies for the CC list trimming.
>
> # v3 -> v4
>
> https://lkml.org/lkml/2020/1/23/515
>
> * Moved away from allocating a separate security_hook_heads and adding a
> new special case for arch_prepare_bpf_trampoline to using BPF fexit
> trampolines called from the right place in the LSM hook and toggled by
> static keys based on the discussion in:
>
> https://lore.kernel.org/bpf/CAG48ez25mW+_oCxgCtbiGMX07g_ph79UOJa07h=o_6B6+Q-u5g@mail.gmail.com/
>
> * Since the code does not deal with security_hook_heads anymore, it goes
> from "being a BPF LSM" to "BPF program attachment to LSM hooks".
I've finally been able to review the entire patch set.
I can't imagine how it can make sense to add this much
complexity to the LSM infrastructure in support of this
feature. There is macro magic going on that is going to
break, and soon. You are introducing dependencies on BPF
into the infrastructure, and that's unnecessary and most
likely harmful.
Would you please drop the excessive optimization? I understand
that there's been a lot of discussion and debate about it,
but this implementation is out of control, disruptive, and
dangerous to the code around it.
On 21-Feb 11:19, Casey Schaufler wrote:
> On 2/20/2020 9:52 AM, KP Singh wrote:
> > From: KP Singh <[email protected]>
>
> Again, apologies for the CC list trimming.
>
> >
> > # v3 -> v4
> >
> > https://lkml.org/lkml/2020/1/23/515
> >
> > * Moved away from allocating a separate security_hook_heads and adding a
> > new special case for arch_prepare_bpf_trampoline to using BPF fexit
> > trampolines called from the right place in the LSM hook and toggled by
> > static keys based on the discussion in:
> >
> > https://lore.kernel.org/bpf/CAG48ez25mW+_oCxgCtbiGMX07g_ph79UOJa07h=o_6B6+Q-u5g@mail.gmail.com/
> >
> > * Since the code does not deal with security_hook_heads anymore, it goes
> > from "being a BPF LSM" to "BPF program attachment to LSM hooks".
>
> I've finally been able to review the entire patch set.
> I can't imagine how it can make sense to add this much
> complexity to the LSM infrastructure in support of this
> feature. There is macro magic going on that is going to
> break, and soon. You are introducing dependencies on BPF
> into the infrastructure, and that's unnecessary and most
> likely harmful.
We will be happy to document each of the macros in detail. Do note a
few things here:
* There is really nothing magical about them though, the LSM hooks are
collectively declared in lsm_hook_names.h and are used to delcare
the security_list_options and security_hook_heads for the LSM
framework (this was previously maitained in two different places):
For BPF, they declare:
* bpf_lsm_<name> attachment points and their prototypes.
* A static key (bpf_lsm_key_<name>) to enable and disable these
hooks with a function to set its value i.e.
(bpf_lsm_<name>_set_enabled).
* We have kept the BPF related macros out of security/.
* All the BPF calls in the LSM infrastructure are guarded by
CONFIG_BPF_LSM (there are only two main calls though, i.e.
call_int_hook, call_void_hook).
Honestly, the macros aren't any more complicated than
call_int_progs/call_void_progs.
- KP
>
> Would you please drop the excessive optimization? I understand
> that there's been a lot of discussion and debate about it,
> but this implementation is out of control, disruptive, and
> dangerous to the code around it.
>
>
On 2/21/2020 11:41 AM, KP Singh wrote:
> On 21-Feb 11:19, Casey Schaufler wrote:
>> On 2/20/2020 9:52 AM, KP Singh wrote:
>>> From: KP Singh <[email protected]>
>> Again, apologies for the CC list trimming.
>>
>>> # v3 -> v4
>>>
>>> https://lkml.org/lkml/2020/1/23/515
>>>
>>> * Moved away from allocating a separate security_hook_heads and adding a
>>> new special case for arch_prepare_bpf_trampoline to using BPF fexit
>>> trampolines called from the right place in the LSM hook and toggled by
>>> static keys based on the discussion in:
>>>
>>> https://lore.kernel.org/bpf/CAG48ez25mW+_oCxgCtbiGMX07g_ph79UOJa07h=o_6B6+Q-u5g@mail.gmail.com/
>>>
>>> * Since the code does not deal with security_hook_heads anymore, it goes
>>> from "being a BPF LSM" to "BPF program attachment to LSM hooks".
>> I've finally been able to review the entire patch set.
>> I can't imagine how it can make sense to add this much
>> complexity to the LSM infrastructure in support of this
>> feature. There is macro magic going on that is going to
>> break, and soon. You are introducing dependencies on BPF
>> into the infrastructure, and that's unnecessary and most
>> likely harmful.
> We will be happy to document each of the macros in detail. Do note a
> few things here:
>
> * There is really nothing magical about them though,
+#define LSM_HOOK_void(NAME, ...) \
+ noinline void bpf_lsm_##NAME(__VA_ARGS__) {}
+
+#include <linux/lsm_hook_names.h>
+#undef LSM_HOOK
I haven't seen anything this ... novel ... in a very long time.
I see why you want to do this, but you're tying the two sets
of code together unnaturally. When (not if) the two sets diverge
you're going to be introducing another clever way to deal with
the special case.
It's not that I don't understand what you're doing. It's that
I don't like what you're doing. Explanation doesn't make me like
it better.
> the LSM hooks are
> collectively declared in lsm_hook_names.h and are used to delcare
> the security_list_options and security_hook_heads for the LSM
> framework (this was previously maitained in two different places):
>
> For BPF, they declare:
>
> * bpf_lsm_<name> attachment points and their prototypes.
> * A static key (bpf_lsm_key_<name>) to enable and disable these
> hooks with a function to set its value i.e.
> (bpf_lsm_<name>_set_enabled).
>
> * We have kept the BPF related macros out of security/.
> * All the BPF calls in the LSM infrastructure are guarded by
> CONFIG_BPF_LSM (there are only two main calls though, i.e.
> call_int_hook, call_void_hook).
>
> Honestly, the macros aren't any more complicated than
> call_int_progs/call_void_progs.
>
> - KP
>
>> Would you please drop the excessive optimization? I understand
>> that there's been a lot of discussion and debate about it,
>> but this implementation is out of control, disruptive, and
>> dangerous to the code around it.
>>
>>
Thanks Casey,
I appreciate your quick responses!
On 21-Feb 14:31, Casey Schaufler wrote:
> On 2/21/2020 11:41 AM, KP Singh wrote:
> > On 21-Feb 11:19, Casey Schaufler wrote:
> >> On 2/20/2020 9:52 AM, KP Singh wrote:
> >>> From: KP Singh <[email protected]>
> >> Again, apologies for the CC list trimming.
> >>
> >>> # v3 -> v4
> >>>
> >>> https://lkml.org/lkml/2020/1/23/515
> >>>
> >>> * Moved away from allocating a separate security_hook_heads and adding a
> >>> new special case for arch_prepare_bpf_trampoline to using BPF fexit
> >>> trampolines called from the right place in the LSM hook and toggled by
> >>> static keys based on the discussion in:
> >>>
> >>> https://lore.kernel.org/bpf/CAG48ez25mW+_oCxgCtbiGMX07g_ph79UOJa07h=o_6B6+Q-u5g@mail.gmail.com/
> >>>
> >>> * Since the code does not deal with security_hook_heads anymore, it goes
> >>> from "being a BPF LSM" to "BPF program attachment to LSM hooks".
[...]
> >> likely harmful.
> > We will be happy to document each of the macros in detail. Do note a
> > few things here:
> >
> > * There is really nothing magical about them though,
>
>
> +#define LSM_HOOK_void(NAME, ...) \
> + noinline void bpf_lsm_##NAME(__VA_ARGS__) {}
> +
> +#include <linux/lsm_hook_names.h>
> +#undef LSM_HOOK
>
> I haven't seen anything this ... novel ... in a very long time.
This is not "novel", it's a fairly common pattern followed in tracing:
For example, the TRACE_INCLUDE macro which is used for tracepoints:
? include/trace/define_trace.h
and used in:
? * include/trace/bpf_probe.h
? ? https://github.com/torvalds/linux/blob/master/include/trace/bpf_probe.h#L110
? * include/trace/perf.h
? ? https://github.com/torvalds/linux/blob/master/include/trace/perf.h#L90
? * include/trace/trace_events.h
https://github.com/torvalds/linux/blob/master/include/trace/trace_events.h#L402
> I see why you want to do this, but you're tying the two sets
> of code together unnaturally. When (not if) the two sets diverge
> you're going to be introducing another clever way to deal with
I don't fully understand what "two sets diverge means" here. All the
BPF headers need is the name, return type and the args. This is the
same information which is needed by the call_{int, void}_hooks and the
LSM declarataions (i.e. security_hook_heads and
security_list_options).
> the special case.
>
> It's not that I don't understand what you're doing. It's that
> I don't like what you're doing. Explanation doesn't make me like
> it better.
As I have previously said, we will be happy to (and have already)
updated our approach based on the consensus we arrive at here. The
best outcome would be to not sacrifice performance as the LSM hooks
are called from various performance critical code-paths.
It would be great to know the maintainers' (BPF and Security)
perspective on this as well.
- KP
>
> > the LSM hooks are
> > collectively declared in lsm_hook_names.h and are used to delcare
> > the security_list_options and security_hook_heads for the LSM
> > framework (this was previously maitained in two different places):
> >
> > For BPF, they declare:
> >
> > * bpf_lsm_<name> attachment points and their prototypes.
> > * A static key (bpf_lsm_key_<name>) to enable and disable these
> > hooks with a function to set its value i.e.
> > (bpf_lsm_<name>_set_enabled).
> >
> > * We have kept the BPF related macros out of security/.
> > * All the BPF calls in the LSM infrastructure are guarded by
> > CONFIG_BPF_LSM (there are only two main calls though, i.e.
> > call_int_hook, call_void_hook).
> >
> > Honestly, the macros aren't any more complicated than
> > call_int_progs/call_void_progs.
> >
> > - KP
> >
> >> Would you please drop the excessive optimization? I understand
> >> that there's been a lot of discussion and debate about it,
> >> but this implementation is out of control, disruptive, and
> >> dangerous to the code around it.
> >>
> >>
>
On 2/21/2020 3:09 PM, KP Singh wrote:
> Thanks Casey,
>
> I appreciate your quick responses!
>
> On 21-Feb 14:31, Casey Schaufler wrote:
>> On 2/21/2020 11:41 AM, KP Singh wrote:
>>> On 21-Feb 11:19, Casey Schaufler wrote:
>>>> On 2/20/2020 9:52 AM, KP Singh wrote:
>>>>> From: KP Singh <[email protected]>
>>>> Again, apologies for the CC list trimming.
>>>>
>>>>> # v3 -> v4
>>>>>
>>>>> https://lkml.org/lkml/2020/1/23/515
>>>>>
>>>>> * Moved away from allocating a separate security_hook_heads and adding a
>>>>> new special case for arch_prepare_bpf_trampoline to using BPF fexit
>>>>> trampolines called from the right place in the LSM hook and toggled by
>>>>> static keys based on the discussion in:
>>>>>
>>>>> https://lore.kernel.org/bpf/CAG48ez25mW+_oCxgCtbiGMX07g_ph79UOJa07h=o_6B6+Q-u5g@mail.gmail.com/
>>>>>
>>>>> * Since the code does not deal with security_hook_heads anymore, it goes
>>>>> from "being a BPF LSM" to "BPF program attachment to LSM hooks".
> [...]
>
>>>> likely harmful.
>>> We will be happy to document each of the macros in detail. Do note a
>>> few things here:
>>>
>>> * There is really nothing magical about them though,
>>
>> +#define LSM_HOOK_void(NAME, ...) \
>> + noinline void bpf_lsm_##NAME(__VA_ARGS__) {}
>> +
>> +#include <linux/lsm_hook_names.h>
>> +#undef LSM_HOOK
>>
>> I haven't seen anything this ... novel ... in a very long time.
> This is not "novel", it's a fairly common pattern followed in tracing:
>
> For example, the TRACE_INCLUDE macro which is used for tracepoints:
>
> include/trace/define_trace.h
>
> and used in:
>
> * include/trace/bpf_probe.h
>
> https://github.com/torvalds/linux/blob/master/include/trace/bpf_probe.h#L110
>
> * include/trace/perf.h
>
> https://github.com/torvalds/linux/blob/master/include/trace/perf.h#L90
>
> * include/trace/trace_events.h
>
> https://github.com/torvalds/linux/blob/master/include/trace/trace_events.h#L402
I can't say I care for that, either, and it's a simpler case.
>> I see why you want to do this, but you're tying the two sets
>> of code together unnaturally. When (not if) the two sets diverge
>> you're going to be introducing another clever way to deal with
> I don't fully understand what "two sets diverge means" here. All the
> BPF headers need is the name, return type and the args. This is the
> same information which is needed by the call_{int, void}_hooks and the
> LSM declarataions (i.e. security_hook_heads and
> security_list_options).
As you've noticed, not all the interfaces can use call_{int,void}_hooks.
If you've been following the stacking efforts, you'll see that increasing.
At some point I anticipate a BPF hook that needs different information
than the LSM hook. That's been discussed, too. Asserting that it will
never happen does not make me comfortable.
>> the special case.
>>
>> It's not that I don't understand what you're doing. It's that
>> I don't like what you're doing. Explanation doesn't make me like
>> it better.
> As I have previously said, we will be happy to (and have already)
> updated our approach based on the consensus we arrive at here.
Not to put too fine a point on it, but I have raised the same
objection - that you should use the infrastructure as it is -
each time. I do not see consensus, I see you plowing ahead with
the direction you've chosen in spite of the significant objection.
> The
> best outcome would be to not sacrifice performance as the LSM hooks
> are called from various performance critical code-paths.
Then help me tune the infrastructure to be better in those cases.
> It would be great to know the maintainers' (BPF and Security)
> perspective on this as well.
Many eyes and all that, but the BPF maintainers haven't been working
with the LSM infrastructure and won't be familiar with its quirks.
On Fri, Feb 21, 2020 at 02:31:18PM -0800, Casey Schaufler wrote:
> On 2/21/2020 11:41 AM, KP Singh wrote:
> > On 21-Feb 11:19, Casey Schaufler wrote:
> >> On 2/20/2020 9:52 AM, KP Singh wrote:
> >>> From: KP Singh <[email protected]>
> >>> # v3 -> v4
> >>>
> >>> https://lkml.org/lkml/2020/1/23/515
> >>>
> >>> * Moved away from allocating a separate security_hook_heads and adding a
> >>> new special case for arch_prepare_bpf_trampoline to using BPF fexit
> >>> trampolines called from the right place in the LSM hook and toggled by
> >>> static keys based on the discussion in:
> >>>
> >>> https://lore.kernel.org/bpf/CAG48ez25mW+_oCxgCtbiGMX07g_ph79UOJa07h=o_6B6+Q-u5g@mail.gmail.com/
> >>>
> >>> * Since the code does not deal with security_hook_heads anymore, it goes
> >>> from "being a BPF LSM" to "BPF program attachment to LSM hooks".
> >> I've finally been able to review the entire patch set.
> >> I can't imagine how it can make sense to add this much
> >> complexity to the LSM infrastructure in support of this
> >> feature. There is macro magic going on that is going to
> >> break, and soon. You are introducing dependencies on BPF
> >> into the infrastructure, and that's unnecessary and most
> >> likely harmful.
> > We will be happy to document each of the macros in detail. Do note a
> > few things here:
> >
> > * There is really nothing magical about them though,
>
>
> +#define LSM_HOOK_void(NAME, ...) \
> + noinline void bpf_lsm_##NAME(__VA_ARGS__) {}
> +
> +#include <linux/lsm_hook_names.h>
> +#undef LSM_HOOK
>
> I haven't seen anything this ... novel ... in a very long time.
> I see why you want to do this, but you're tying the two sets
> of code together unnaturally. When (not if) the two sets diverge
> you're going to be introducing another clever way to deal with
> the special case.
I really like this approach: it actually _simplifies_ the LSM piece in
that there is no need to keep the union and the hook lists in sync any
more: they're defined once now. (There were already 2 lists, and this
collapses the list into 1 place for all 3 users.) It's very visible in
the diffstat too (~300 lines removed):
include/linux/lsm_hook_names.h | 353 +++++++++++++++++++
include/linux/lsm_hooks.h | 622 +--------------------------------
2 files changed, 359 insertions(+), 616 deletions(-)
Also, there is no need to worry about divergence: the BPF will always
track the exposed LSM. Backward compat is (AIUI) explicitly a
non-feature.
I don't see why anything here is "harmful"?
--
Kees Cook
On 2/21/2020 4:22 PM, Kees Cook wrote:
> On Fri, Feb 21, 2020 at 02:31:18PM -0800, Casey Schaufler wrote:
>> On 2/21/2020 11:41 AM, KP Singh wrote:
>>> On 21-Feb 11:19, Casey Schaufler wrote:
>>>> On 2/20/2020 9:52 AM, KP Singh wrote:
>>>>> From: KP Singh <[email protected]>
>>>>> # v3 -> v4
>>>>>
>>>>> https://lkml.org/lkml/2020/1/23/515
>>>>>
>>>>> * Moved away from allocating a separate security_hook_heads and adding a
>>>>> new special case for arch_prepare_bpf_trampoline to using BPF fexit
>>>>> trampolines called from the right place in the LSM hook and toggled by
>>>>> static keys based on the discussion in:
>>>>>
>>>>> https://lore.kernel.org/bpf/CAG48ez25mW+_oCxgCtbiGMX07g_ph79UOJa07h=o_6B6+Q-u5g@mail.gmail.com/
>>>>>
>>>>> * Since the code does not deal with security_hook_heads anymore, it goes
>>>>> from "being a BPF LSM" to "BPF program attachment to LSM hooks".
>>>> I've finally been able to review the entire patch set.
>>>> I can't imagine how it can make sense to add this much
>>>> complexity to the LSM infrastructure in support of this
>>>> feature. There is macro magic going on that is going to
>>>> break, and soon. You are introducing dependencies on BPF
>>>> into the infrastructure, and that's unnecessary and most
>>>> likely harmful.
>>> We will be happy to document each of the macros in detail. Do note a
>>> few things here:
>>>
>>> * There is really nothing magical about them though,
>>
>> +#define LSM_HOOK_void(NAME, ...) \
>> + noinline void bpf_lsm_##NAME(__VA_ARGS__) {}
>> +
>> +#include <linux/lsm_hook_names.h>
>> +#undef LSM_HOOK
>>
>> I haven't seen anything this ... novel ... in a very long time.
>> I see why you want to do this, but you're tying the two sets
>> of code together unnaturally. When (not if) the two sets diverge
>> you're going to be introducing another clever way to deal with
>> the special case.
> I really like this approach: it actually _simplifies_ the LSM piece in
> that there is no need to keep the union and the hook lists in sync any
> more: they're defined once now. (There were already 2 lists, and this
> collapses the list into 1 place for all 3 users.) It's very visible in
> the diffstat too (~300 lines removed):
Erk. Too many smart people like this. I still don't, but it's possible
that I could learn to.
>
> include/linux/lsm_hook_names.h | 353 +++++++++++++++++++
> include/linux/lsm_hooks.h | 622 +--------------------------------
> 2 files changed, 359 insertions(+), 616 deletions(-)
>
> Also, there is no need to worry about divergence: the BPF will always
> track the exposed LSM. Backward compat is (AIUI) explicitly a
> non-feature.
As written you're correct, it can't diverge. My concern is about
what happens when someone decides that they want the BPF and hook
to be different. I fear there will be a hideous solution.
> I don't see why anything here is "harmful"?
Injecting large chucks of code via an #include does nothing
for readability. I've seen it fail disastrously many times,
usually after the original author has moved on and entrusted
the code to someone who missed some of the nuance.
I'll drop objection to this bit, but still object to making
BPF special in the infrastructure. It doesn't need to be and
it is exactly the kind of additional complexity we need to
avoid.
On Fri, Feb 21, 2020 at 05:04:38PM -0800, Casey Schaufler wrote:
> On 2/21/2020 4:22 PM, Kees Cook wrote:
> > I really like this approach: it actually _simplifies_ the LSM piece in
> > that there is no need to keep the union and the hook lists in sync any
> > more: they're defined once now. (There were already 2 lists, and this
> > collapses the list into 1 place for all 3 users.) It's very visible in
> > the diffstat too (~300 lines removed):
>
> Erk. Too many smart people like this. I still don't, but it's possible
> that I could learn to.
Well, I admit that I am, perhaps, overly infatuatied with "fancy" macros,
but in cases like this where we're operating on a list of stuff and doing
the same thing over and over but with different elements, I've found
this is actually much nicer way to do it. (E.g. I did something like
this in drivers/misc/lkdtm/core.c to avoid endless typing, and Mimi did
something similar in include/linux/fs.h for keeping kernel_read_file_id
and kernel_read_file_str automatically in sync.) KP's macros are more
extensive, but I think it's a clever to avoid going crazy as LSM hooks
evolve.
> > Also, there is no need to worry about divergence: the BPF will always
> > track the exposed LSM. Backward compat is (AIUI) explicitly a
> > non-feature.
>
> As written you're correct, it can't diverge. My concern is about
> what happens when someone decides that they want the BPF and hook
> to be different. I fear there will be a hideous solution.
This is related to some of the discussion at the last Maintainer's
Summit and tracepoints: i.e. the exposure of what is basically kernel
internals to a userspace system. The conclusion there (which, I think,
has been extended strongly into BPF) is that things that produce BPF are
accepted to be strongly tied to kernel version, so if a hook changes, so
much the userspace side. This appears to be proven out in the existing
BPF world, which gives me some evidence that this claim (close tie to
kernel version) isn't an empty promise.
> > I don't see why anything here is "harmful"?
>
> Injecting large chucks of code via an #include does nothing
> for readability. I've seen it fail disastrously many times,
> usually after the original author has moved on and entrusted
> the code to someone who missed some of the nuance.
I totally agree about wanting to avoid reduced readability. In this case,
I actually think readability is improved since the macro "implementation"
are right above each #include. And then looking at the resulting included
header, all the metadata is visible in one place. But I agree: it is
"unusual", but I think on the sum it's an improvement. (But I share some
of the frustration of the kernel being filled with weird preprocessor
insanity. I will never get back the weeks I spent on trying to improve
the min/max macros.... *sob*)
> I'll drop objection to this bit, but still object to making
> BPF special in the infrastructure. It doesn't need to be and
> it is exactly the kind of additional complexity we need to
> avoid.
You mean 3/8's RUN_BPF_LSM_*_PROGS() additions to the call_*_hook()s?
I'll go comment on that thread directly instead of splitting the
discussion. :)
--
Kees Cook
On Thu, Feb 20, 2020 at 06:52:42PM +0100, KP Singh wrote:
Good morning, I hope the week is going well for everyone.
Apologies for being somewhat late with these comments, I've been
recovering from travel.
> # Motivation
>
> Google does analysis of rich runtime security data 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 allows BPF programs to be attached to LSM hooks 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.
On the remote possibility that our practical experiences are relevant
to this, I thought I would pitch these comments in, since I see that
LWN is covering the issues and sensitivities surrounding BPF based
'intelligent' LSM hooks, if I can take the liberty of referring to
them as that.
We namespaced a modified version of the Linux IMA implementation in
order to provide a mechanism for deterministic system modeling, in
order to support autonomously self defensive platforms for
IOT/INED/SCADA type applications. Big picture, the objective was to
provide 'dynamic intelligence' for LSM decisions, presumably an
objective similar to the KRSI initiative.
Our IMA implementation, if you can still call it that, pushes
actor/subject interaction identities up into an SGX enclave that runs
a modeling engine that makes decisions on whether or not a process is
engaging in activity inconsistent with a behavioral map defined by the
platform or container developer. If the behavior is extra-dimensional
(untrusted), the enclave, via an OCALL, sets the value of a 'bad
actor' variable in the task control structure that is used to indicate
that the context of execution has questionable trust status.
We paired this with a very simple LSM that has each hook check a bit
position in the bad actor variable/bitfield to determine whether or
not the hook should operate on the requested action. Separate LSM
infrastructure is provided that specifies whether or not the behavior
should be EPERM'ed or logged. An LSM using this infrastructure also
has the ability, if triggered by the trust status of the context of
execution, to make further assessments based on what information is
supplied via the hook itself.
Our field experience and testing has suggested that this architecture
has considerable utility.
In this model, numerous and disparate sections of the kernel can have
input into the trust status of a context of execution. This
methodology would seem to be consistent with having multiple eBPF tap
points in the kernel that can make decisions on what they perceive to
be security relevant issues and if and how the behavior should be
acted upon by the LSM.
At the LSM level the costs are minimal, essentially a conditional
check for non-zero status. Performance costs will be with the eBPF
code installed at introspection points. At the end of the
day. security costs money, if no one is willing to pay the bill we
simply won't have secure systems, the fundamental tenant of the
inherent economic barrier to security.
Food for thought if anyone is interested.
Best wishes for a productive remainder of the week.
Dr. Greg
As always,
Dr. Greg Wettstein, Ph.D, Worker
IDfusion, LLC SGX secured infrastructure and
4206 N. 19th Ave. autonomously self-defensive platforms.
Fargo, ND 58102
PH: 701-281-1686 EMAIL: [email protected]
------------------------------------------------------------------------------
"We have to grow some roots before we can even think about having
any blossoms."
-- Terrance George Wieland
Resurrection.