| Message ID | 20230808155010.76584-2-paul@paul-moore.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Paul Moore |
| Headers | show |
| Series | selinux: revert SECINITSID_INIT support | expand |
On Tue, Aug 8, 2023 at 6:27 PM Paul Moore <paul@paul-moore.com> wrote: > > This commit reverts 5b0eea835d4e ("selinux: introduce an initial SID > for early boot processes") as it was found to cause problems on > distros with old SELinux userspace tools/libraries, specifically > Ubuntu 16.04. > > Hopefully we will be able to re-add this functionality at a later > date, but let's revert this for now to help ensure a stable and > backwards compatible SELinux tree. > > Link: https://lore.kernel.org/selinux/87edkseqf8.fsf@mail.lhotse > Signed-off-by: Paul Moore <paul@paul-moore.com> > --- > 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 | 1 - > security/selinux/include/security.h | 6 ---- > security/selinux/ss/policydb.c | 27 ------------------ > 6 files changed, 1 insertion(+), 64 deletions(-) I don't think I'm able to post a fix for this quickly enough, so: Acked-by: Ondrej Mosnacek <omosnace@redhat.com>
On Wed, Aug 9, 2023 at 5:30 AM Ondrej Mosnacek <omosnace@redhat.com> wrote: > > On Tue, Aug 8, 2023 at 6:27 PM Paul Moore <paul@paul-moore.com> wrote: > > > > This commit reverts 5b0eea835d4e ("selinux: introduce an initial SID > > for early boot processes") as it was found to cause problems on > > distros with old SELinux userspace tools/libraries, specifically > > Ubuntu 16.04. > > > > Hopefully we will be able to re-add this functionality at a later > > date, but let's revert this for now to help ensure a stable and > > backwards compatible SELinux tree. > > > > Link: https://lore.kernel.org/selinux/87edkseqf8.fsf@mail.lhotse > > Signed-off-by: Paul Moore <paul@paul-moore.com> > > --- > > 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 | 1 - > > security/selinux/include/security.h | 6 ---- > > security/selinux/ss/policydb.c | 27 ------------------ > > 6 files changed, 1 insertion(+), 64 deletions(-) > > I don't think I'm able to post a fix for this quickly enough, so: > > Acked-by: Ondrej Mosnacek <omosnace@redhat.com> > Should we revert the userspace patch as well (just the policy capability one)? Or is a fix expected soon enough to not worry about it? Jim > -- > Ondrej Mosnacek > Senior Software Engineer, Linux Security - SELinux kernel > Red Hat, Inc. >
On Wed, Aug 9, 2023 at 4:57 AM Ondrej Mosnacek <omosnace@redhat.com> wrote: > On Tue, Aug 8, 2023 at 6:27 PM Paul Moore <paul@paul-moore.com> wrote: > > > > This commit reverts 5b0eea835d4e ("selinux: introduce an initial SID > > for early boot processes") as it was found to cause problems on > > distros with old SELinux userspace tools/libraries, specifically > > Ubuntu 16.04. > > > > Hopefully we will be able to re-add this functionality at a later > > date, but let's revert this for now to help ensure a stable and > > backwards compatible SELinux tree. > > > > Link: https://lore.kernel.org/selinux/87edkseqf8.fsf@mail.lhotse > > Signed-off-by: Paul Moore <paul@paul-moore.com> > > --- > > 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 | 1 - > > security/selinux/include/security.h | 6 ---- > > security/selinux/ss/policydb.c | 27 ------------------ > > 6 files changed, 1 insertion(+), 64 deletions(-) > > I don't think I'm able to post a fix for this quickly enough, so: > > Acked-by: Ondrej Mosnacek <omosnace@redhat.com> Merged into selinux/next. FWIW, I really do like the idea behind this, and I'm looking forward to a proper fix so that we can bring it back. Unfortunately the revert is necessary so we have can have a week or two of good code in selinux/next before the merge window. * https://github.com/SELinuxProject/selinux-kernel/blob/main/README.md#kernel-tree-process
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 7cd687284563..9be291315b91 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -2313,19 +2313,6 @@ 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. */ @@ -4542,21 +4529,6 @@ 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_net_init_from_sk(&ad, &net, sk); return avc_has_perm(current_sid(), sksec->sid, sksec->sclass, perms, diff --git a/security/selinux/include/initial_sid_to_string.h b/security/selinux/include/initial_sid_to_string.h index 5e5f0993dac2..ecc6e74fa09b 100644 --- a/security/selinux/include/initial_sid_to_string.h +++ b/security/selinux/include/initial_sid_to_string.h @@ -10,7 +10,7 @@ static const char *const initial_sid_to_string[] = { NULL, "file", NULL, - "init", + NULL, "any_socket", "port", "netif", diff --git a/security/selinux/include/policycap.h b/security/selinux/include/policycap.h index c7373e6effe5..f35d3458e71d 100644 --- a/security/selinux/include/policycap.h +++ b/security/selinux/include/policycap.h @@ -12,7 +12,6 @@ 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 28e4c9ee2399..49bbe120d173 100644 --- a/security/selinux/include/policycap_names.h +++ b/security/selinux/include/policycap_names.h @@ -14,7 +14,6 @@ const char *const selinux_policycap_names[__POLICYDB_CAP_MAX] = { "nnp_nosuid_transition", "genfs_seclabel_symlinks", "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 074d439fe9ad..a9de89af8fdc 100644 --- a/security/selinux/include/security.h +++ b/security/selinux/include/security.h @@ -189,12 +189,6 @@ static inline bool selinux_policycap_ioctl_skip_cloexec(void) selinux_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 a424997c79eb..954842bde3e6 100644 --- a/security/selinux/ss/policydb.c +++ b/security/selinux/ss/policydb.c @@ -864,8 +864,6 @@ 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); @@ -889,13 +887,6 @@ 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", @@ -903,24 +894,6 @@ 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; }
This commit reverts 5b0eea835d4e ("selinux: introduce an initial SID for early boot processes") as it was found to cause problems on distros with old SELinux userspace tools/libraries, specifically Ubuntu 16.04. Hopefully we will be able to re-add this functionality at a later date, but let's revert this for now to help ensure a stable and backwards compatible SELinux tree. Link: https://lore.kernel.org/selinux/87edkseqf8.fsf@mail.lhotse Signed-off-by: Paul Moore <paul@paul-moore.com> --- 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 | 1 - security/selinux/include/security.h | 6 ---- security/selinux/ss/policydb.c | 27 ------------------ 6 files changed, 1 insertion(+), 64 deletions(-)