On Thu, Oct 31, 2019 at 10:01 AM Stephen Smalley <sds@xxxxxxxxxxxxx> wrote: > On 10/31/19 5:59 AM, Paul Moore wrote: > > On Wed, Oct 30, 2019 at 9:16 AM Stephen Smalley <sds@xxxxxxxxxxxxx> wrote: > >> Add SELinux access control hooks for lockdown integrity and > >> confidentiality. This effectively mimics the current implementation of > >> lockdown (caveat noted below). If lockdown is enabled alongside SELinux, > >> then the lockdown access control will take precedence over the SELinux > >> lockdown implementation. > >> > >> Note that this SELinux implementation allows the integrity and > >> confidentiality reasons to be controlled independently from one another. > >> Thus, in an SELinux policy, one could allow integrity operations while > >> blocking confidentiality operations. ... > > I don't have any objections to adding a hook to control access to the > > lockdown functionality (I think it's a good idea), but I am a little > > nervous about the granularity of the control. Sticking with just an > > integrity and a confidentiality permission seems okay, but I worry > > about adding additional permissions until we have a better idea of how > > the lockdown functionality is adopted by developers and we see how the > > lockdown_reason evolves. > > Ok, so let's discuss what if anything else is needed for a final non-RFC > version of this patch. > > One thing that I wondered about was whether we ought to include the > reason information in the audit record as supplemental data via a new > LSM_AUDIT_DATA_* type for help in policy debugging / development. > However, the lockdown_reasons[] string array is presently private to the > lockdown module so we would have to export that or replicate it for > creating a string representation of the reason if we were to do so. > That would expose the reasons in terms of audit data but not as a basis > for the permission check. Note that the lockdown module logs these > reason strings via pr_notice() when it denies access, so it appears that > exposing the strings as part of audit data would not introduce any extra > kernel stable ABI guarantees? That is an interesting question: do we consider dmesg output to be part of the stable kernel API? My hunch would be "no", as I've seen things change quite a bit there over the years, but IANL (I Am Not Linus). However, that said, logging a reason string via audit seems like a good idea (especially since there is presently a many-to-one mapping between reasons and the SELinux permission). Further, while the audit field name is part of the kernel API, the value is much more open. > I also wasn't sure about the pr_warn() above. If we reach it, it is > effectively a kernel bug. We could mirror what the lockdown module does > in lockdown_is_locked_down(), i.e. use WARN() instead. Of course, the > SELinux hook won't even be reached in this case if the lockdown module > is enabled, but the lockdown module could be disabled so I guess we need > to check it too. Since this seems security relevant, I wonder if we should be using SELINUX_ERR? > If we take the lockdown class with just integrity and confidentiality > permissions now and later introduce finer granularity, we'll presumably > need a policy capability to select whether the coarse-grained or > fine-grained permissions are used. True, although I'm not overly worried about the need to use policy capabilities; I'm more worried about jumping into too fine a granularity before we see how this will be used. -- paul moore www.paul-moore.com
