Re: [PATCH v3] selinux: introduce an initial SID for early boot processes

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Nov 21, 2023 at 3:01 PM Stephen Smalley
<stephen.smalley.work@xxxxxxxxx> wrote:
>
> On Tue, Nov 14, 2023 at 10:51 AM Ondrej Mosnacek <omosnace@xxxxxxxxxx> wrote:
> >
> > Currently, SELinux doesn't allow distinguishing between kernel threads
> > and userspace processes that are started before the policy is first
> > loaded - both get the label corresponding to the kernel SID. The only
> > way a process that persists from early boot can get a meaningful label
> > is by doing a voluntary dyntransition or re-executing itself.
> >
> > Reusing the kernel label for userspace processes is problematic for
> > several reasons:
> > 1. The kernel is considered to be a privileged domain and generally
> >    needs to have a wide range of permissions allowed to work correctly,
> >    which prevents the policy writer from effectively hardening against
> >    early boot processes that might remain running unintentionally after
> >    the policy is loaded (they represent a potential extra attack surface
> >    that should be mitigated).
> > 2. Despite the kernel being treated as a privileged domain, the policy
> >    writer may want to impose certain special limitations on kernel
> >    threads that may conflict with the requirements of intentional early
> >    boot processes. For example, it is a good hardening practice to limit
> >    what executables the kernel can execute as usermode helpers and to
> >    confine the resulting usermode helper processes. However, a
> >    (legitimate) process surviving from early boot may need to execute a
> >    different set of executables.
> > 3. As currently implemented, overlayfs remembers the security context of
> >    the process that created an overlayfs mount and uses it to bound
> >    subsequent operations on files using this context. If an overlayfs
> >    mount is created before the SELinux policy is loaded, these "mounter"
> >    checks are made against the kernel context, which may clash with
> >    restrictions on the kernel domain (see 2.).
> >
> > To resolve this, introduce a new initial SID (reusing the slot of the
> > former "init" initial SID) that will be assigned to any userspace
> > process started before the policy is first loaded. This is easy to do,
> > as we can simply label any process that goes through the
> > bprm_creds_for_exec LSM hook with the new init-SID instead of
> > propagating the kernel SID from the parent.
> >
> > To provide backwards compatibility for existing policies that are
> > unaware of this new semantic of the "init" initial SID, introduce a new
> > policy capability "userspace_initial_context" and set the "init" SID to
> > the same context as the "kernel" SID unless this capability is set by
> > the policy.
> >
> > Another small backwards compatibility measure is needed in
> > security_sid_to_context_core() for before the initial SELinux policy
> > load - see the code comment for explanation.
> >
> > Signed-off-by: Ondrej Mosnacek <omosnace@xxxxxxxxxx>
> > ---
>
>
> > diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
> > index 1eeffc66ea7d7..344c598fc1e74 100644
> > --- a/security/selinux/ss/services.c
> > +++ b/security/selinux/ss/services.c
> > @@ -1322,8 +1322,19 @@ static int security_sid_to_context_core(u32 sid, char **scontext,
> >         if (!selinux_initialized()) {
> >                 if (sid <= SECINITSID_NUM) {
> >                         char *scontextp;
> > -                       const char *s = initial_sid_to_string[sid];
> > +                       const char *s;
> >
> > +                       /*
> > +                        * Before the policy is loaded, translate
> > +                        * SECINITSID_INIT to "kernel", because systemd and
> > +                        * libselinux < 2.6 take getcon_raw() != "kernel" to
>
> Don't you mean getcon_raw() == "kernel"?
> The old test for SELinux-disabled was to check whether policy was not
> loaded by checking that we get "kernel" when reading
> /proc/thread-self/attr/current.

You're right, I misread the systemd code (which I used as reference
for the comment; didn't bother to look at the old libsepol code). I
also typo'd "that" into "than"... The comment should say "[...] take
getcon_raw() is non-null and not "kernel" to mean that a policy is
already loaded." or similar.

Paul, do you want me to resubmit the patch?

>
> Other than that,
> Reviewed-by: Stephen Smalley <stephen.smalley.work@xxxxxxxxx>
>
> And I did test the SELINUX=disabled case.
>
> > +                        * mean than SELinux is not enabled as the major LSM
> > +                        * and thus returning "init" would make them misbehave.
> > +                        */
> > +                       if (sid == SECINITSID_INIT)
> > +                               sid = SECINITSID_KERNEL;
> > +
> > +                       s = initial_sid_to_string[sid];
> >                         if (!s)
> >                                 return -EINVAL;
> >                         *scontext_len = strlen(s) + 1;
> > --
> > 2.41.0
> >
>

-- 
Ondrej Mosnacek
Senior Software Engineer, Linux Security - SELinux kernel
Red Hat, Inc.





[Index of Archives]     [Selinux Refpolicy]     [Linux SGX]     [Fedora Users]     [Fedora Desktop]     [Yosemite Photos]     [Yosemite Camping]     [Yosemite Campsites]     [KDE Users]     [Gnome Users]

  Powered by Linux