Message-ID: <CAJ2a_DdZHFTHiRu5+ZENAwUq1Cor-jVoE9qdhb2x5uSej-MaRA@mail.gmail.com>
Date:   Fri, 28 Jul 2023 15:23:56 +0200
From:   Christian Göttsche <cgzones@...glemail.com>
To:     Ondrej Mosnacek <omosnace@...hat.com>
Cc:     Stephen Smalley <stephen.smalley.work@...il.com>,
        Michael Ellerman <mpe@...erman.id.au>,
        Paul Moore <paul@...l-moore.com>, selinux@...r.kernel.org,
        LKML <linux-kernel@...r.kernel.org>,
        linuxppc-dev@...ts.ozlabs.org, linux-next@...r.kernel.org
Subject: Re: Login broken with old userspace (was Re: [PATCH v2] selinux:
 introduce an initial SID for early boot processes)

On Fri, 28 Jul 2023 at 15:14, Ondrej Mosnacek <omosnace@...hat.com> wrote:
>
> On Fri, Jul 28, 2023 at 1:52 PM Stephen Smalley
> <stephen.smalley.work@...il.com> wrote:
> >
> > On Fri, Jul 28, 2023 at 7:36 AM Ondrej Mosnacek <omosnace@...hat.com> wrote:
> > >
> > > On Fri, Jul 28, 2023 at 4:12 AM Michael Ellerman <mpe@...erman.id.au> wrote:
> > > >
> > > > Ondrej Mosnacek <omosnace@...hat.com> writes:
> > > > > 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.
> > > >
> > > > Hi,
> > > >
> > > > This commit breaks login for me when booting linux-next kernels with old
> > > > userspace, specifically Ubuntu 16.04 on ppc64le. 18.04 is OK.
> > > >
> > > > The symptom is that login never accepts the root password, it just
> > > > always says "Login incorrect".
> > > >
> > > > Bisect points to this commit.
> > > >
> > > > Reverting this commit on top of next-20230726, fixes the problem
> > > > (ie. login works again).
> > > >
> > > > Booting with selinux=0 also fixes the problem.
> > > >
> > > > Is this expected? The change log below suggests backward compatibility
> > > > was considered, is 16.04 just too old?
> > >
> > > Hi Michael,
> > >
> > > I can reproduce it on Fedora 38 when I boot with SELINUX=disabled in
> > > /etc/selinux/config (+ a kernel including that commit), so it likely
> > > isn't caused by the userspace being old. Can you check what you have
> > > in /etc/selinux/config (or if it exists at all)?
> > >
> > > We have deprecated and removed the "runtime disable" functionality in
> > > SELinux recently [1], which was used to implement "disabling" SELinux
> > > via the /etc/selinux/config file, so now the situation (selinux=0 +
> > > SELINUX=disabled in /etc/selinux/config) leads to a state where
> > > SELinux is enabled, but no policy is loaded (and no enforcement is
> > > done). Such a state mostly behaves as if SElinux was truly disabled
> > > (via kernel command line), but there are some subtle differences and I
> > > believe we don't officially support it (Paul might clarify). With
> > > latest kernels it is recommended to either disable SELinux via the
> > > kernel command line (or Kconfig[2]) or to boot it in Enforcing or
> > > Permissive mode with a valid/usable policy installed.
> > >
> > > So I wonder if Ubuntu ships by default with the bad configuration or
> > > if it's just a result of using the custom-built linux-next kernel (or
> > > some changes on your part). If Ubuntu's stock kernel is configured to
> > > boot with SELinux enabled by default, they should also by default ship
> > > a usable policy and SELINUX=permissive/enforcing in
> > > /etc/selinux/config (or configure the kernel[2] or bootloader to boot
> > > with SELinux disabled by default). (Although if they ship a pre-[1]
> > > kernel, they may continue to rely on the runtime disable
> > > functionality, but it means people will have to be careful when
> > > booting newer or custom kernels.)
> > >
> > > That said, I'd like to get to the bottom of why the commit causes the
> > > login to fail and fix it somehow. I presume something in PAM chokes on
> > > the fact that userspace tasks now have "init" instead of "kernel" as
> > > the pre-policy-load security context, but so far I haven't been able
> > > to pinpoint the problem. I'll keep digging...
> > >
> > > [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f22f9aaf6c3d92ebd5ad9e67acc03afebaaeb289
> > > [2] via CONFIG_LSM (or CONFIG_SECURITY_SELINUX_BOOTPARAM_VALUE on older kernels)
> >
> > Prior to selinux userspace commit
> > 685f4aeeadc0b60f3770404d4f149610d656e3c8 ("libselinux:
> > is_selinux_enabled(): drop no-policy-loaded test.") libselinux was
> > checking the result of reading /proc/self/attr/current to see if it
> > returned the "kernel" string as a means of detecting a system with
> > SELinux enabled but no policy loaded, and treated that as if SELinux
> > were disabled. Hence, this does break old userspace. Not sure though
> > why you'd see the same behavior with modern libselinux.
>
> Hm... now I tried booting the stock Fedora kernel (without the early
> boot initial SID commit) and I got the same failure to login as with
> the new kernel. So if Ubuntu 16.04 ships with pre-685f4aeeadc0
> libselinux (quite possible), then it seems that the scenario with
> terminal login + SELinux enabled + policy not loaded only works with
> pre-685f4aeeadc0 libselinux and pre-5b0eea835d4e kernel, the other
> combinations are broken. With pre-685f4aeeadc0 libselinux +
> post-5b0eea835d4e kernel it is expected as you say (and probably
> inevitable barring some hack on the kernel side), but it's not clear
> why also only updating libselinux seems to break it... /sys/fs/selinux
> is not mounted in my scenario, so there must be something else coming
> into play.
>
>
> --
> Ondrej Mosnacek
> Senior Software Engineer, Linux Security - SELinux kernel
> Red Hat, Inc.
>

Completely untested:

diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index 2c5be06fbada..1ed275bd4551 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;

+                       /*
+                        * Hide the context split of kernel threads and
+                        * userspace threads from userspace before the first
+                        * policy is loaded.  Userspace, e.g. libselinux prior
+                        * to v2.6 or systemd, depends on the context being
+                        * "kernel".
+                        */
+                       if (sid == SECINITSID_INIT)
+                               sid = SECINITSID_KERNEL;
+
+                       s = initial_sid_to_string[sid];
                        if (!s)
                                return -EINVAL;
                        *scontext_len = strlen(s) + 1;

Powered by blists - more mailing lists