Re: [RFC] Turn lockdown into an LSM

[Stephen Smalley]

On 5/22/19 1:08 PM, Andy Lutomirski wrote:
> On Wed, May 22, 2019 at 9:49 AM Matthew Garrett <mjg59@xxxxxxxxxx> wrote:
> > On Tue, May 21, 2019 at 7:40 PM James Morris <jmorris@xxxxxxxxx> wrote:
> > > An LSM could also potentially implement its own policy for the hook.
> >
> > That was my plan. Right now the hook just gets an ASCII description of
> > the reason for the lockdown - that seems suboptimal for cases like
> > SELinux. What information would you want? My initial thinking was to
> > just have a stable enum of lockdown reasons that's in the UAPI headers
> > and then let other LSM tooling consume that, but I haven't spent
> > enough time with the internals of SELinux to know if there'd be a more
> > attractive solution.
>
> I may be in the minority here, but I see this issue as a significant
> downside of making lockdown more flexible.  If we stick with just
> "this may violate integrity" and "this may violate confidentiality",
> then the ABI surface is nice and narrow.  If we start having a big
> uapi list of things that might qualify for lockdown, we need to worry
> about compatibility issues.
>
> This isn't purely theoretical.  Lockdown has some interesting
> interactions with eBPF.  I don't want to be in a situation where v1 of
> lockdown has a few eBPF hooks, but a later update improves the eBPF vs
> lockdown interaction so that you can do more with eBPF on a locked
> down kernel.  But now any such change has to worry about breaking the
> lockdown LSM ABI.

I think we could keep the enum itself private to the kernel, and 
internally each security module could map the values to whatever 
permissions it wants to define.  SELinux itself has the ability to 
add/remove/re-arrange its permission definitions without breaking 
userspace/policy; they are dynamically mapped at policy load time. If a 
given permission doesn't exist in the loaded policy, there is a policy 
flag to control whether the policy should be rejected, or the permission 
should always be denied, or the permission should always be allowed. 
There is also support for an extensible set of "policy capabilities" to 
control whether new/changed permission checking logic for a given 
permission/operation should be enabled for that policy or if the kernel 
should continue using the older logic.

> And I still think it would be nice to have some credible use case for
> a more fine grained policy than just the tri-state.  Having a lockdown
> policy of "may not violate kernel confidentiality except using
> kprobes" may be convenient, but it's also basically worthless, since
> kernel confidentiality is gone.

I would expect that distros and users would have to make pragmatic 
tradeoffs in this area, and it would be nice if they could have a choice 
beyond all-or-nothing.  Do all of the interfaces being locked down 
expose the same degree of confidentiality or integrity risk?

> All this being said, I do see one big benefit for LSM integration:
> SELinux or another LSM could allow certain privileged tasks to bypass
> lockdown.

That seems to violate the intent of lockdown as I understood it, and 
turns security_is_locked_down() into a finer-grained capable() call.
Also, if I understand correctly, this could only be done if one were to 
disable the lockdown module in the lsm list, since the security 
framework will return non-zero (i.e. the operation is locked down) if 
any module that implements the hook returns non-zero; LSM is 
"restrictive". At that point, SELinux or the other LSM would be the sole 
arbiter of lockdown decisions.  SELinux or the other LSM also wouldn't 
have access to the kernel_locked_down level unless that was exported in 
some manner from the lockdown module.  Not sure how to compose these.

  This seems fine, except that there's potential nastiness
> where current->cred isn't actually a valid thing to look at in the
> current context.
>
> So I guess my proposal is: use LSM, but make the hook very coarse
> grained: int security_violate_confidentiality(const struct cred *) and
> int security_violate_integrity(const struct cred *).

Not sure one can construct a useful policy at that granularity.