| Message ID | 20200220175250.10795-1-kpsingh@chromium.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | MAC and Audit policy using eBPF (KRSI) | expand |
On 2/20/2020 9:52 AM, KP Singh wrote: > From: KP Singh <kpsingh@google.com> 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 <kpsingh@google.com> > > 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 <kpsingh@google.com> >> 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 <kpsingh@google.com> > >> 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 <kpsingh@google.com> >>>> 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 <kpsingh@google.com> > >>> # 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"?
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 <kpsingh@google.com> >>>>> # 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. :)
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: greg@idfusion.net ------------------------------------------------------------------------------ "We have to grow some roots before we can even think about having any blossoms." -- Terrance George Wieland Resurrection.
From: KP Singh <kpsingh@google.com> # 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/20200117212825.11755-1-kpsingh@chromium.org/ * 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/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 (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