Re: [PATCH 23/27] bpf: Restrict kernel image access functions when the kernel is locked down

Re: [PATCH 23/27] bpf: Restrict kernel image access functions when the kernel is locked down

[Andy Lutomirski]

On Mon, Mar 25, 2019 at 4:42 PM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> On Mon, 25 Mar 2019 15:09:50 -0700
> Matthew Garrett <matthewgarrett@google.com> wrote:
>
> > From: David Howells <dhowells@redhat.com>
> >
> > There are some bpf functions can be used to read kernel memory:
> > bpf_probe_read, bpf_probe_write_user and bpf_trace_printk.  These allow
> > private keys in kernel memory (e.g. the hibernation image signing key) to
> > be read by an eBPF program and kernel memory to be altered without
> > restriction.
> >
> > Completely prohibit the use of BPF when the kernel is locked down.
> >
> > Suggested-by: Alexei Starovoitov <alexei.starovoitov@gmail.com>
> > Signed-off-by: David Howells <dhowells@redhat.com>
> > cc: netdev@vger.kernel.org
> > cc: Chun-Yi Lee <jlee@suse.com>
> > cc: Alexei Starovoitov <alexei.starovoitov@gmail.com>
> > Cc: Daniel Borkmann <daniel@iogearbox.net>
> > Signed-off-by: Matthew Garrett <matthewgarrett@google.com>
>
> Wouldn't this mean that Seccomp won't work in locked down mode?

I wasn't cc'd on this series, nor was linux-api, so it's awkward to review.

A while back, I suggested an approach to actually make this stuff
mergeable: submit a patch series that adds lockdown mode, enables it
by command line option (and maybe sysctl) *only* and has either no
effect or only a token effect.  Then we can add actual features to
lockdown mode one at a time and review them separately.

And I'm going to complain loudly unless two things change about this
whole thing:

1. Lockdown mode becomes three states, not a boolean.  The states are:
no lockdown, best-effort-to-protect-kernel-integrity, and
best-effort-to-protect-kernel-secrecy-and-integrity.  And this BPF
mess illustrates why: most users will really strongly object to
turning off BPF when they actually just want to protect kernel
integrity.  And as far as I know, things like Secure Boot policy will
mostly care about integrity, not secrecy, and tracing and such should
work on a normal locked-down kernel.  So I think we need this knob.

2. All the proponents of this series, and the documentation, needs to
document that it's best effort.  There will always be security bugs,
and there will always be things we miss.


--Andy

Re: [PATCH 23/27] bpf: Restrict kernel image access functions when the kernel is locked down

[James Morris]

On Mon, 25 Mar 2019, Andy Lutomirski wrote:

> A while back, I suggested an approach to actually make this stuff
> mergeable: submit a patch series that adds lockdown mode, enables it
> by command line option (and maybe sysctl) *only* and has either no
> effect or only a token effect.  Then we can add actual features to
> lockdown mode one at a time and review them separately.

This makes sense to me.

> 
> And I'm going to complain loudly unless two things change about this
> whole thing:
> 
> 1. Lockdown mode becomes three states, not a boolean.  The states are:
> no lockdown, best-effort-to-protect-kernel-integrity, and
> best-effort-to-protect-kernel-secrecy-and-integrity.  And this BPF
> mess illustrates why: most users will really strongly object to
> turning off BPF when they actually just want to protect kernel
> integrity.  And as far as I know, things like Secure Boot policy will
> mostly care about integrity, not secrecy, and tracing and such should
> work on a normal locked-down kernel.  So I think we need this knob.

Another approach would be to make this entirely policy based:

- Assign an ID to each lockdown point
- Implement a policy mechanism where each ID is mapped to 0 or 1
- Allow this policy to be specified statically or dynamically

So, 

	kernel_is_locked_down("ioperm")

becomes

	kernel_is_locked_down(LOCKDOWN_IOPERM)

and this function checks e.g.

	if (lockdown_polcy[id]) {
		fail or warn;
        }

Thoughts?


> 2. All the proponents of this series, and the documentation, needs to
> document that it's best effort.  There will always be security bugs,
> and there will always be things we miss.

Right.  Maintaining this feature will be an ongoing effort, and if its not 
actively maintained, it will bitrot and become useless.


-- 
James Morris
<jmorris@namei.org>

Re: [PATCH 23/27] bpf: Restrict kernel image access functions when the kernel is locked down

[Andy Lutomirski]

On Tue, Mar 26, 2019 at 11:57 AM James Morris <jmorris@namei.org> wrote:
>
> On Mon, 25 Mar 2019, Andy Lutomirski wrote:
>
> > A while back, I suggested an approach to actually make this stuff
> > mergeable: submit a patch series that adds lockdown mode, enables it
> > by command line option (and maybe sysctl) *only* and has either no
> > effect or only a token effect.  Then we can add actual features to
> > lockdown mode one at a time and review them separately.
>
> This makes sense to me.
>
> >
> > And I'm going to complain loudly unless two things change about this
> > whole thing:
> >
> > 1. Lockdown mode becomes three states, not a boolean.  The states are:
> > no lockdown, best-effort-to-protect-kernel-integrity, and
> > best-effort-to-protect-kernel-secrecy-and-integrity.  And this BPF
> > mess illustrates why: most users will really strongly object to
> > turning off BPF when they actually just want to protect kernel
> > integrity.  And as far as I know, things like Secure Boot policy will
> > mostly care about integrity, not secrecy, and tracing and such should
> > work on a normal locked-down kernel.  So I think we need this knob.
>
> Another approach would be to make this entirely policy based:
>
> - Assign an ID to each lockdown point
> - Implement a policy mechanism where each ID is mapped to 0 or 1
> - Allow this policy to be specified statically or dynamically
>
> So,
>
>         kernel_is_locked_down("ioperm")
>
> becomes
>
>         kernel_is_locked_down(LOCKDOWN_IOPERM)
>
> and this function checks e.g.
>
>         if (lockdown_polcy[id]) {
>                 fail or warn;
>         }
>
> Thoughts?

I'm concerned that this gives too much useless flexibility to
administrators and user code in general.  If you can break kernel
integrity, you can break kernel integrity -- it shouldn't really
matter *how* you break it.

--Andy

Re: [PATCH 23/27] bpf: Restrict kernel image access functions when the kernel is locked down

[James Morris]

On Tue, 26 Mar 2019, Andy Lutomirski wrote:

> >
> >         kernel_is_locked_down("ioperm")
> >
> > becomes
> >
> >         kernel_is_locked_down(LOCKDOWN_IOPERM)
> >
> > and this function checks e.g.
> >
> >         if (lockdown_polcy[id]) {
> >                 fail or warn;
> >         }
> >
> > Thoughts?
> 
> I'm concerned that this gives too much useless flexibility to
> administrators and user code in general.  If you can break kernel
> integrity, you can break kernel integrity -- it shouldn't really
> matter *how* you break it.

OTOH, this seems like a combination of mechanism and policy. The 3 modes 
are a help here, but I wonder if they may be too coarse grained still, 
e.g. if someone wants to allow a specific mechanism according to their own 
threat model and mitigations.

Secure boot gives you some assurance of the static state of the system at 
boot time, and lockdown is certainly useful (with or without secure boot), 
but it's not a complete solution to runtime kernel integrity protection by 
any stretch of the imagination.  I'm concerned about it being perceived as 
such.

I'm not sure how to think about it architecturally and how it fits as such 
in the mainline kernel.


-- 
James Morris
<jmorris@namei.org>

Re: [PATCH 23/27] bpf: Restrict kernel image access functions when the kernel is locked down

[Matthew Garrett]

On Wed, Mar 27, 2019 at 8:15 PM James Morris <jmorris@namei.org> wrote:
> OTOH, this seems like a combination of mechanism and policy. The 3 modes
> are a help here, but I wonder if they may be too coarse grained still,
> e.g. if someone wants to allow a specific mechanism according to their own
> threat model and mitigations.

In general the interfaces blocked by these patches could also be
blocked with an LSM, and I'd guess that people with more fine-grained
requirements would probably take that approach.

> Secure boot gives you some assurance of the static state of the system at
> boot time, and lockdown is certainly useful (with or without secure boot),
> but it's not a complete solution to runtime kernel integrity protection by
> any stretch of the imagination.  I'm concerned about it being perceived as
> such.

What do you think the functionality gaps are in terms of ensuring
kernel integrity (other than kernel flaws that allow the restrictions
to be bypassed)?

Re: [PATCH 23/27] bpf: Restrict kernel image access functions when the kernel is locked down

[James Morris]

On Thu, 28 Mar 2019, Matthew Garrett wrote:

> On Wed, Mar 27, 2019 at 8:15 PM James Morris <jmorris@namei.org> wrote:
> > OTOH, this seems like a combination of mechanism and policy. The 3 modes
> > are a help here, but I wonder if they may be too coarse grained still,
> > e.g. if someone wants to allow a specific mechanism according to their own
> > threat model and mitigations.
> 
> In general the interfaces blocked by these patches could also be
> blocked with an LSM, and I'd guess that people with more fine-grained
> requirements would probably take that approach.

So... I have to ask, why not use LSM for this in the first place?

Either with an existing module or perhaps a lockdown LSM?

> 
> > Secure boot gives you some assurance of the static state of the system at
> > boot time, and lockdown is certainly useful (with or without secure boot),
> > but it's not a complete solution to runtime kernel integrity protection by
> > any stretch of the imagination.  I'm concerned about it being perceived as
> > such.
> 
> What do you think the functionality gaps are in terms of ensuring
> kernel integrity (other than kernel flaws that allow the restrictions
> to be bypassed)?

I don't know of any non-flaw gaps.

-- 
James Morris
<jmorris@namei.org>

Re: [PATCH 23/27] bpf: Restrict kernel image access functions when the kernel is locked down

[Matthew Garrett]

On Thu, Mar 28, 2019 at 12:23 PM James Morris <jmorris@namei.org> wrote:
>
> On Thu, 28 Mar 2019, Matthew Garrett wrote:
>
> > On Wed, Mar 27, 2019 at 8:15 PM James Morris <jmorris@namei.org> wrote:
> > > OTOH, this seems like a combination of mechanism and policy. The 3 modes
> > > are a help here, but I wonder if they may be too coarse grained still,
> > > e.g. if someone wants to allow a specific mechanism according to their own
> > > threat model and mitigations.
> >
> > In general the interfaces blocked by these patches could also be
> > blocked with an LSM, and I'd guess that people with more fine-grained
> > requirements would probably take that approach.
>
> So... I have to ask, why not use LSM for this in the first place?
>
> Either with an existing module or perhaps a lockdown LSM?

Some of it isn't really achievable that way - for instance, enforcing
module or kexec signatures. We have other mechanisms that can be used
to enable that which could be done at the more fine-grained level, but
a design goal was to make it possible to automatically enable a full
set of integrity protections under specified circumstances.

Re: [PATCH 23/27] bpf: Restrict kernel image access functions when the kernel is locked down

[Matthew Garrett]

On Tue, Mar 26, 2019 at 11:57 AM James Morris <jmorris@namei.org> wrote:
> - Assign an ID to each lockdown point
> - Implement a policy mechanism where each ID is mapped to 0 or 1
> - Allow this policy to be specified statically or dynamically

One of the problems with this approach is what the default behaviour
should be when a new feature is added. If an admin fails to notice
that there's now a new policy element, they run the risk of kernel
integrity being compromised via the new feature even if the rest of
the kernel is locked down.