Re: [RFC PATCH] selinux: add SELinux hooks for lockdown integrity and confidentiality

[Paul Moore <paul@paul-moore.com>]

On Wed, Oct 30, 2019 at 9:16 AM Stephen Smalley <sds@tycho.nsa.gov> 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.
>
> (original patch authored by an intern who wishes to remain anonymous;
> I am signing off on his behalf)
>
> Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
> ---
>  security/selinux/hooks.c            | 22 ++++++++++++++++++++++
>  security/selinux/include/classmap.h |  2 ++
>  2 files changed, 24 insertions(+)
>
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 36e531b91df2..6722c6b4ae74 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -91,6 +91,7 @@
>  #include <uapi/linux/mount.h>
>  #include <linux/fsnotify.h>
>  #include <linux/fanotify.h>
> +#include <linux/security.h>
>
>  #include "avc.h"
>  #include "objsec.h"
> @@ -6799,6 +6800,25 @@ static void selinux_bpf_prog_free(struct bpf_prog_aux *aux)
>  }
>  #endif
>
> +static int selinux_lockdown(enum lockdown_reason what)
> +{
> +       u32 sid = current_sid();
> +
> +       if (what <= LOCKDOWN_INTEGRITY_MAX)
> +               return avc_has_perm(&selinux_state,
> +                               sid, sid,
> +                               SECCLASS_LOCKDOWN, LOCKDOWN__INTEGRITY, NULL);
> +       else if (what <= LOCKDOWN_CONFIDENTIALITY_MAX)
> +               return avc_has_perm(&selinux_state,
> +                               sid, sid,
> +                               SECCLASS_LOCKDOWN, LOCKDOWN__CONFIDENTIALITY,
> +                               NULL);
> +
> +       /* invalid reason */
> +       pr_warn("SELinux: invalid lockdown reason\n");
> +       return -EPERM;
> +}

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.

-- 
paul moore
www.paul-moore.com