| Message ID | 20230620131223.431281-1-omosnace@redhat.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Paul Moore |
| Headers | show |
| Series | [v2] selinux: introduce an initial SID for early boot processes | expand |
On Tue, Jun 20, 2023 at 9:12 AM Ondrej Mosnacek <omosnace@redhat.com> 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. > > Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com> > --- > > v2: apply Paul's style suggestions > > security/selinux/hooks.c | 28 +++++++++++++++++++ > .../selinux/include/initial_sid_to_string.h | 2 +- > security/selinux/include/policycap.h | 1 + > security/selinux/include/policycap_names.h | 3 +- > security/selinux/include/security.h | 6 ++++ > security/selinux/ss/policydb.c | 27 ++++++++++++++++++ > 6 files changed, 65 insertions(+), 2 deletions(-) Merged into selinux/next, thanks!
Ondrej Mosnacek <omosnace@redhat.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? cheers > 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. > > Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com> > --- > > v2: apply Paul's style suggestions > > security/selinux/hooks.c | 28 +++++++++++++++++++ > .../selinux/include/initial_sid_to_string.h | 2 +- > security/selinux/include/policycap.h | 1 + > security/selinux/include/policycap_names.h | 3 +- > security/selinux/include/security.h | 6 ++++ > security/selinux/ss/policydb.c | 27 ++++++++++++++++++ > 6 files changed, 65 insertions(+), 2 deletions(-) > > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > index 99ded60a6b911..83d71433e23e9 100644 > --- a/security/selinux/hooks.c > +++ b/security/selinux/hooks.c > @@ -2264,6 +2264,19 @@ static int selinux_bprm_creds_for_exec(struct linux_binprm *bprm) > new_tsec->keycreate_sid = 0; > new_tsec->sockcreate_sid = 0; > > + /* > + * Before policy is loaded, label any task outside kernel space > + * as SECINITSID_INIT, so that any userspace tasks surviving from > + * early boot end up with a label different from SECINITSID_KERNEL > + * (if the policy chooses to set SECINITSID_INIT != SECINITSID_KERNEL). > + */ > + if (!selinux_initialized()) { > + new_tsec->sid = SECINITSID_INIT; > + /* also clear the exec_sid just in case */ > + new_tsec->exec_sid = 0; > + return 0; > + } > + > if (old_tsec->exec_sid) { > new_tsec->sid = old_tsec->exec_sid; > /* Reset exec SID on execve. */ > @@ -4480,6 +4493,21 @@ static int sock_has_perm(struct sock *sk, u32 perms) > if (sksec->sid == SECINITSID_KERNEL) > return 0; > > + /* > + * Before POLICYDB_CAP_USERSPACE_INITIAL_CONTEXT, sockets that > + * inherited the kernel context from early boot used to be skipped > + * here, so preserve that behavior unless the capability is set. > + * > + * By setting the capability the policy signals that it is ready > + * for this quirk to be fixed. Note that sockets created by a kernel > + * thread or a usermode helper executed without a transition will > + * still be skipped in this check regardless of the policycap > + * setting. > + */ > + if (!selinux_policycap_userspace_initial_context() && > + sksec->sid == SECINITSID_INIT) > + return 0; > + > ad.type = LSM_AUDIT_DATA_NET; > ad.u.net = &net; > ad.u.net->sk = sk; > diff --git a/security/selinux/include/initial_sid_to_string.h b/security/selinux/include/initial_sid_to_string.h > index 60820517aa438..6d450669e9c68 100644 > --- a/security/selinux/include/initial_sid_to_string.h > +++ b/security/selinux/include/initial_sid_to_string.h > @@ -7,7 +7,7 @@ static const char *const initial_sid_to_string[] = { > NULL, > "file", > NULL, > - NULL, > + "init", > "any_socket", > "port", > "netif", > diff --git a/security/selinux/include/policycap.h b/security/selinux/include/policycap.h > index f35d3458e71de..c7373e6effe5d 100644 > --- a/security/selinux/include/policycap.h > +++ b/security/selinux/include/policycap.h > @@ -12,6 +12,7 @@ enum { > POLICYDB_CAP_NNP_NOSUID_TRANSITION, > POLICYDB_CAP_GENFS_SECLABEL_SYMLINKS, > POLICYDB_CAP_IOCTL_SKIP_CLOEXEC, > + POLICYDB_CAP_USERSPACE_INITIAL_CONTEXT, > __POLICYDB_CAP_MAX > }; > #define POLICYDB_CAP_MAX (__POLICYDB_CAP_MAX - 1) > diff --git a/security/selinux/include/policycap_names.h b/security/selinux/include/policycap_names.h > index 2a87fc3702b81..28e4c9ee23997 100644 > --- a/security/selinux/include/policycap_names.h > +++ b/security/selinux/include/policycap_names.h > @@ -13,7 +13,8 @@ const char *const selinux_policycap_names[__POLICYDB_CAP_MAX] = { > "cgroup_seclabel", > "nnp_nosuid_transition", > "genfs_seclabel_symlinks", > - "ioctl_skip_cloexec" > + "ioctl_skip_cloexec", > + "userspace_initial_context", > }; > > #endif /* _SELINUX_POLICYCAP_NAMES_H_ */ > diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h > index 8746fafeb7789..c08b8b58439c9 100644 > --- a/security/selinux/include/security.h > +++ b/security/selinux/include/security.h > @@ -201,6 +201,12 @@ static inline bool selinux_policycap_ioctl_skip_cloexec(void) > return READ_ONCE(state->policycap[POLICYDB_CAP_IOCTL_SKIP_CLOEXEC]); > } > > +static inline bool selinux_policycap_userspace_initial_context(void) > +{ > + return READ_ONCE( > + selinux_state.policycap[POLICYDB_CAP_USERSPACE_INITIAL_CONTEXT]); > +} > + > struct selinux_policy_convert_data; > > struct selinux_load_state { > diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c > index 97c0074f9312a..c5465a0b8055a 100644 > --- a/security/selinux/ss/policydb.c > +++ b/security/selinux/ss/policydb.c > @@ -863,6 +863,8 @@ void policydb_destroy(struct policydb *p) > int policydb_load_isids(struct policydb *p, struct sidtab *s) > { > struct ocontext *head, *c; > + bool isid_init_supported = ebitmap_get_bit(&p->policycaps, > + POLICYDB_CAP_USERSPACE_INITIAL_CONTEXT); > int rc; > > rc = sidtab_init(s); > @@ -886,6 +888,13 @@ int policydb_load_isids(struct policydb *p, struct sidtab *s) > if (!name) > continue; > > + /* > + * Also ignore SECINITSID_INIT if the policy doesn't declare > + * support for it > + */ > + if (sid == SECINITSID_INIT && !isid_init_supported) > + continue; > + > rc = sidtab_set_initial(s, sid, &c->context[0]); > if (rc) { > pr_err("SELinux: unable to load initial SID %s.\n", > @@ -893,6 +902,24 @@ int policydb_load_isids(struct policydb *p, struct sidtab *s) > sidtab_destroy(s); > return rc; > } > + > + /* > + * If the policy doesn't support the "userspace_initial_context" > + * capability, set SECINITSID_INIT to the same context as > + * SECINITSID_KERNEL. This ensures the same behavior as before > + * the reintroduction of SECINITSID_INIT, where all tasks > + * started before policy load would initially get the context > + * corresponding to SECINITSID_KERNEL. > + */ > + if (sid == SECINITSID_KERNEL && !isid_init_supported) { > + rc = sidtab_set_initial(s, SECINITSID_INIT, &c->context[0]); > + if (rc) { > + pr_err("SELinux: unable to load initial SID %s.\n", > + name); > + sidtab_destroy(s); > + return rc; > + } > + } > } > return 0; > } > -- > 2.41.0
On Fri, Jul 28, 2023 at 4:12 AM Michael Ellerman <mpe@ellerman.id.au> wrote: > > Ondrej Mosnacek <omosnace@redhat.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)
On Fri, Jul 28, 2023 at 7:36 AM Ondrej Mosnacek <omosnace@redhat.com> wrote: > > On Fri, Jul 28, 2023 at 4:12 AM Michael Ellerman <mpe@ellerman.id.au> wrote: > > > > Ondrej Mosnacek <omosnace@redhat.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.
On Fri, Jul 28, 2023 at 1:52 PM Stephen Smalley <stephen.smalley.work@gmail.com> wrote: > > On Fri, Jul 28, 2023 at 7:36 AM Ondrej Mosnacek <omosnace@redhat.com> wrote: > > > > On Fri, Jul 28, 2023 at 4:12 AM Michael Ellerman <mpe@ellerman.id.au> wrote: > > > > > > Ondrej Mosnacek <omosnace@redhat.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.
On Fri, 28 Jul 2023 at 15:14, Ondrej Mosnacek <omosnace@redhat.com> wrote: > > On Fri, Jul 28, 2023 at 1:52 PM Stephen Smalley > <stephen.smalley.work@gmail.com> wrote: > > > > On Fri, Jul 28, 2023 at 7:36 AM Ondrej Mosnacek <omosnace@redhat.com> wrote: > > > > > > On Fri, Jul 28, 2023 at 4:12 AM Michael Ellerman <mpe@ellerman.id.au> wrote: > > > > > > > > Ondrej Mosnacek <omosnace@redhat.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;
On Fri, Jul 28, 2023 at 9:24 AM Christian Göttsche <cgzones@googlemail.com> wrote: > > On Fri, 28 Jul 2023 at 15:14, Ondrej Mosnacek <omosnace@redhat.com> wrote: > > > > On Fri, Jul 28, 2023 at 1:52 PM Stephen Smalley > > <stephen.smalley.work@gmail.com> wrote: > > > > > > On Fri, Jul 28, 2023 at 7:36 AM Ondrej Mosnacek <omosnace@redhat.com> wrote: > > > > > > > > On Fri, Jul 28, 2023 at 4:12 AM Michael Ellerman <mpe@ellerman.id.au> wrote: > > > > > > > > > > Ondrej Mosnacek <omosnace@redhat.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; I think I'd rather see something that does the following: 1. Convert all direct access of @initial_sid_to_string to calls to security_get_initial_sid_context(). I think we can leave all the stuff under scripts/ as-is, but I didn't think about it that hard, so some additional thought would be required here. 2. Modify security_get_initial_sid_context() to so something similar to what Christian proposed, e.g. translate INIT to KERNEL, but do so only when POLICYDB_CAP_USERSPACE_INITIAL_CONTEXT is not enabled. I believe this should cover both the uninitialized and old policy case. It might be easier if both were done as a single patch, as those that want the userspace isid patch will likely want this as a fix, but if it becomes to ugly I have no problem splitting #1 and #2 into different patches. What do folks think? Am I missing something?
Ondrej Mosnacek <omosnace@redhat.com> writes: > On Fri, Jul 28, 2023 at 4:12 AM Michael Ellerman <mpe@ellerman.id.au> wrote: >> >> Ondrej Mosnacek <omosnace@redhat.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)? Not sure if you still need it, but /etc/selinux/config doesn't exist in the 16.04 image. cheers
On Fri, Jul 28, 2023 at 5:12 PM Paul Moore <paul@paul-moore.com> wrote: > > On Fri, Jul 28, 2023 at 9:24 AM Christian Göttsche > <cgzones@googlemail.com> wrote: > > > > On Fri, 28 Jul 2023 at 15:14, Ondrej Mosnacek <omosnace@redhat.com> wrote: > > > > > > On Fri, Jul 28, 2023 at 1:52 PM Stephen Smalley > > > <stephen.smalley.work@gmail.com> wrote: > > > > > > > > On Fri, Jul 28, 2023 at 7:36 AM Ondrej Mosnacek <omosnace@redhat.com> wrote: > > > > > > > > > > On Fri, Jul 28, 2023 at 4:12 AM Michael Ellerman <mpe@ellerman.id.au> wrote: > > > > > > > > > > > > Ondrej Mosnacek <omosnace@redhat.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; > > I think I'd rather see something that does the following: > > 1. Convert all direct access of @initial_sid_to_string to calls to > security_get_initial_sid_context(). I think we can leave all the > stuff under scripts/ as-is, but I didn't think about it that hard, so > some additional thought would be required here. What should we then do with the reverse translation in security_context_to_sid_core()? It seems it is currently possible for a process to e.g. change its SID to another initial SID before the policy is loaded - would we let it to set itself to INIT and yet still return back KERNEL afterwards? > 2. Modify security_get_initial_sid_context() to so something similar > to what Christian proposed, e.g. translate INIT to KERNEL, but do so > only when POLICYDB_CAP_USERSPACE_INITIAL_CONTEXT is not enabled. I > believe this should cover both the uninitialized and old policy case. You don't know whether the policycap is enabled or not until the policy is loaded and at that point it doesn't matter because then you already have a full context assigned to the SID. So we could only translate INIT to KERNEL unconditionally, but it feels wrong to lie to userspace like that (plus there is the reverse translation issue above)... OTOH, I don't know if we have another choice given the "no regressions" rule... > It might be easier if both were done as a single patch, as those that > want the userspace isid patch will likely want this as a fix, but if > it becomes to ugly I have no problem splitting #1 and #2 into > different patches. > > What do folks think? Am I missing something? > > -- > paul-moore.com >
On Tue, Aug 1, 2023 at 9:24 AM Ondrej Mosnacek <omosnace@redhat.com> wrote: > On Fri, Jul 28, 2023 at 5:12 PM Paul Moore <paul@paul-moore.com> wrote: > > > > On Fri, Jul 28, 2023 at 9:24 AM Christian Göttsche > > <cgzones@googlemail.com> wrote: > > > > > > On Fri, 28 Jul 2023 at 15:14, Ondrej Mosnacek <omosnace@redhat.com> wrote: > > > > > > > > On Fri, Jul 28, 2023 at 1:52 PM Stephen Smalley > > > > <stephen.smalley.work@gmail.com> wrote: > > > > > > > > > > On Fri, Jul 28, 2023 at 7:36 AM Ondrej Mosnacek <omosnace@redhat.com> wrote: > > > > > > > > > > > > On Fri, Jul 28, 2023 at 4:12 AM Michael Ellerman <mpe@ellerman.id.au> wrote: > > > > > > > > > > > > > > Ondrej Mosnacek <omosnace@redhat.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; > > > > I think I'd rather see something that does the following: > > > > 1. Convert all direct access of @initial_sid_to_string to calls to > > security_get_initial_sid_context(). I think we can leave all the > > stuff under scripts/ as-is, but I didn't think about it that hard, so > > some additional thought would be required here. > > What should we then do with the reverse translation in > security_context_to_sid_core()? It seems it is currently possible for > a process to e.g. change its SID to another initial SID before the > policy is loaded - would we let it to set itself to INIT and yet still > return back KERNEL afterwards? Yeah, I wasn't thinking of the reverse translation. While the sid to context string mapping is definitely flexible, I really don't like the idea of changing the sid assigned to an entity, even if there isn't a policy loaded (yet). > > 2. Modify security_get_initial_sid_context() to so something similar > > to what Christian proposed, e.g. translate INIT to KERNEL, but do so > > only when POLICYDB_CAP_USERSPACE_INITIAL_CONTEXT is not enabled. I > > believe this should cover both the uninitialized and old policy case. > > You don't know whether the policycap is enabled or not until the > policy is loaded and at that point it doesn't matter because then you > already have a full context assigned to the SID. My perspective is that there are really only two states we care about: policy loaded with the INITIAL_CONTEXT policycap, and everything else (including no policy loaded). > OTOH, I don't know if we have another choice given the "no regressions" rule... Here's the thing, we're at -rc4 right now and I'm a little concerned that I haven't seen a fix on-list for this. If we don't at least have some sort of design by the end of this week, with a patch early next week, I'm going to have to revert the patch in selinux/next. You don't have to like the silly little design above, but we need *something* to fix this.
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 99ded60a6b911..83d71433e23e9 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -2264,6 +2264,19 @@ static int selinux_bprm_creds_for_exec(struct linux_binprm *bprm) new_tsec->keycreate_sid = 0; new_tsec->sockcreate_sid = 0; + /* + * Before policy is loaded, label any task outside kernel space + * as SECINITSID_INIT, so that any userspace tasks surviving from + * early boot end up with a label different from SECINITSID_KERNEL + * (if the policy chooses to set SECINITSID_INIT != SECINITSID_KERNEL). + */ + if (!selinux_initialized()) { + new_tsec->sid = SECINITSID_INIT; + /* also clear the exec_sid just in case */ + new_tsec->exec_sid = 0; + return 0; + } + if (old_tsec->exec_sid) { new_tsec->sid = old_tsec->exec_sid; /* Reset exec SID on execve. */ @@ -4480,6 +4493,21 @@ static int sock_has_perm(struct sock *sk, u32 perms) if (sksec->sid == SECINITSID_KERNEL) return 0; + /* + * Before POLICYDB_CAP_USERSPACE_INITIAL_CONTEXT, sockets that + * inherited the kernel context from early boot used to be skipped + * here, so preserve that behavior unless the capability is set. + * + * By setting the capability the policy signals that it is ready + * for this quirk to be fixed. Note that sockets created by a kernel + * thread or a usermode helper executed without a transition will + * still be skipped in this check regardless of the policycap + * setting. + */ + if (!selinux_policycap_userspace_initial_context() && + sksec->sid == SECINITSID_INIT) + return 0; + ad.type = LSM_AUDIT_DATA_NET; ad.u.net = &net; ad.u.net->sk = sk; diff --git a/security/selinux/include/initial_sid_to_string.h b/security/selinux/include/initial_sid_to_string.h index 60820517aa438..6d450669e9c68 100644 --- a/security/selinux/include/initial_sid_to_string.h +++ b/security/selinux/include/initial_sid_to_string.h @@ -7,7 +7,7 @@ static const char *const initial_sid_to_string[] = { NULL, "file", NULL, - NULL, + "init", "any_socket", "port", "netif", diff --git a/security/selinux/include/policycap.h b/security/selinux/include/policycap.h index f35d3458e71de..c7373e6effe5d 100644 --- a/security/selinux/include/policycap.h +++ b/security/selinux/include/policycap.h @@ -12,6 +12,7 @@ enum { POLICYDB_CAP_NNP_NOSUID_TRANSITION, POLICYDB_CAP_GENFS_SECLABEL_SYMLINKS, POLICYDB_CAP_IOCTL_SKIP_CLOEXEC, + POLICYDB_CAP_USERSPACE_INITIAL_CONTEXT, __POLICYDB_CAP_MAX }; #define POLICYDB_CAP_MAX (__POLICYDB_CAP_MAX - 1) diff --git a/security/selinux/include/policycap_names.h b/security/selinux/include/policycap_names.h index 2a87fc3702b81..28e4c9ee23997 100644 --- a/security/selinux/include/policycap_names.h +++ b/security/selinux/include/policycap_names.h @@ -13,7 +13,8 @@ const char *const selinux_policycap_names[__POLICYDB_CAP_MAX] = { "cgroup_seclabel", "nnp_nosuid_transition", "genfs_seclabel_symlinks", - "ioctl_skip_cloexec" + "ioctl_skip_cloexec", + "userspace_initial_context", }; #endif /* _SELINUX_POLICYCAP_NAMES_H_ */ diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h index 8746fafeb7789..c08b8b58439c9 100644 --- a/security/selinux/include/security.h +++ b/security/selinux/include/security.h @@ -201,6 +201,12 @@ static inline bool selinux_policycap_ioctl_skip_cloexec(void) return READ_ONCE(state->policycap[POLICYDB_CAP_IOCTL_SKIP_CLOEXEC]); } +static inline bool selinux_policycap_userspace_initial_context(void) +{ + return READ_ONCE( + selinux_state.policycap[POLICYDB_CAP_USERSPACE_INITIAL_CONTEXT]); +} + struct selinux_policy_convert_data; struct selinux_load_state { diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c index 97c0074f9312a..c5465a0b8055a 100644 --- a/security/selinux/ss/policydb.c +++ b/security/selinux/ss/policydb.c @@ -863,6 +863,8 @@ void policydb_destroy(struct policydb *p) int policydb_load_isids(struct policydb *p, struct sidtab *s) { struct ocontext *head, *c; + bool isid_init_supported = ebitmap_get_bit(&p->policycaps, + POLICYDB_CAP_USERSPACE_INITIAL_CONTEXT); int rc; rc = sidtab_init(s); @@ -886,6 +888,13 @@ int policydb_load_isids(struct policydb *p, struct sidtab *s) if (!name) continue; + /* + * Also ignore SECINITSID_INIT if the policy doesn't declare + * support for it + */ + if (sid == SECINITSID_INIT && !isid_init_supported) + continue; + rc = sidtab_set_initial(s, sid, &c->context[0]); if (rc) { pr_err("SELinux: unable to load initial SID %s.\n", @@ -893,6 +902,24 @@ int policydb_load_isids(struct policydb *p, struct sidtab *s) sidtab_destroy(s); return rc; } + + /* + * If the policy doesn't support the "userspace_initial_context" + * capability, set SECINITSID_INIT to the same context as + * SECINITSID_KERNEL. This ensures the same behavior as before + * the reintroduction of SECINITSID_INIT, where all tasks + * started before policy load would initially get the context + * corresponding to SECINITSID_KERNEL. + */ + if (sid == SECINITSID_KERNEL && !isid_init_supported) { + rc = sidtab_set_initial(s, SECINITSID_INIT, &c->context[0]); + if (rc) { + pr_err("SELinux: unable to load initial SID %s.\n", + name); + sidtab_destroy(s); + return rc; + } + } } return 0; }
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. Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com> --- v2: apply Paul's style suggestions security/selinux/hooks.c | 28 +++++++++++++++++++ .../selinux/include/initial_sid_to_string.h | 2 +- security/selinux/include/policycap.h | 1 + security/selinux/include/policycap_names.h | 3 +- security/selinux/include/security.h | 6 ++++ security/selinux/ss/policydb.c | 27 ++++++++++++++++++ 6 files changed, 65 insertions(+), 2 deletions(-)