[PATCH v4] selinux: Generalize support for NNP/nosuid SELinux domain transitions

[PATCH v4] selinux: Generalize support for NNP/nosuid SELinux domain transitions

[Stephen Smalley]

As systemd ramps up enabling NNP (NoNewPrivileges) for system services,
it is increasingly breaking SELinux domain transitions for those services
and their descendants.  systemd enables NNP not only for services whose
unit files explicitly specify NoNewPrivileges=yes but also for services
whose unit files specify any of the following options in combination with
running without CAP_SYS_ADMIN (e.g. specifying User= or a
CapabilityBoundingSet= without CAP_SYS_ADMIN): SystemCallFilter=,
SystemCallArchitectures=, RestrictAddressFamilies=, RestrictNamespaces=,
PrivateDevices=, ProtectKernelTunables=, ProtectKernelModules=,
MemoryDenyWriteExecute=, or RestrictRealtime= as per the systemd.exec(5)
man page.

The end result is bad for the security of both SELinux-disabled and
SELinux-enabled systems.  Packagers have to turn off these
options in the unit files to preserve SELinux domain transitions.  For
users who choose to disable SELinux, this means that they miss out on
at least having the systemd-supported protections.  For users who keep
SELinux enabled, they may still be missing out on some protections
because it isn't necessarily guaranteed that the SELinux policy for
that service provides the same protections in all cases.

commit 7b0d0b40cd78 ("selinux: Permit bounded transitions under
NO_NEW_PRIVS or NOSUID.") allowed bounded transitions under NNP in
order to support limited usage for sandboxing programs.  However,
defining typebounds for all of the affected service domains
is impractical to implement in policy, since typebounds requires us
to ensure that each domain is allowed everything all of its descendant
domains are allowed, and this has to be repeated for the entire chain
of domain transitions.  There is no way to clone all allow rules from
descendants to their ancestors in policy currently, and doing so would
be undesirable even if it were practical, as it requires leaking
permissions to objects and operations into ancestor domains that could
weaken their own security in order to allow them to the descendants
(e.g. if a descendant requires execmem permission, then so do all of
its ancestors; if a descendant requires execute permission to a file,
then so do all of its ancestors; if a descendant requires read to a
symbolic link or temporary file, then so do all of its ancestors...).
SELinux domains are intentionally not hierarchical / bounded in this
manner normally, and making them so would undermine their protections
and least privilege.

We have long had a similar tension with SELinux transitions and nosuid
mounts, albeit not as severe.  Users often have had to choose between
retaining nosuid on a mount and allowing SELinux domain transitions on
files within those mounts.  This likewise leads to unfortunate tradeoffs
in security.

Decouple NNP/nosuid from SELinux transitions, so that we don't have to
make a choice between them. Introduce a nnp_nosuid_transition policy
capability that enables transitions under NNP/nosuid to be based on
a permission (nnp_transition for NNP; nosuid_transition for nosuid)
between the old and new contexts in addition to the current support
for bounded transitions.  Domain transitions can then be allowed in
policy without requiring the parent to be a strict superset of all of
its children.

With this change, systemd unit files can be left unmodified from upstream.
SELinux-disabled and SELinux-enabled users will benefit from retaining any
of the systemd-provided protections.  SELinux policy will only need to
be adapted to enable the new policy capability and to allow the
new permissions between domain pairs as appropriate.

NB: Allowing nnp_transition between two contexts opens up the potential
for the old context to subvert the new context by installing seccomp
filters before the execve.  Allowing nosuid_transition between two contexts
opens up the potential for a context transition to occur on a file from
an untrusted filesystem (e.g. removable media or remote filesystem).  Use
with care.

Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
---
v4 moves both of the new permissions to the new process2 class and
checks both if both NNP is enabled and the mount is nosuid.

 security/selinux/hooks.c            | 47 +++++++++++++++++++++++++------------
 security/selinux/include/classmap.h |  2 ++
 security/selinux/include/security.h |  2 ++
 security/selinux/ss/services.c      |  7 +++++-
 4 files changed, 42 insertions(+), 16 deletions(-)

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 00ad46e..04b8e10 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -2318,6 +2318,7 @@ static int check_nnp_nosuid(const struct linux_binprm *bprm,
 	int nnp = (bprm->unsafe & LSM_UNSAFE_NO_NEW_PRIVS);
 	int nosuid = !mnt_may_suid(bprm->file->f_path.mnt);
 	int rc;
+	u32 av;
 
 	if (!nnp && !nosuid)
 		return 0; /* neither NNP nor nosuid */
@@ -2326,24 +2327,40 @@ static int check_nnp_nosuid(const struct linux_binprm *bprm,
 		return 0; /* No change in credentials */
 
 	/*
-	 * The only transitions we permit under NNP or nosuid
-	 * are transitions to bounded SIDs, i.e. SIDs that are
-	 * guaranteed to only be allowed a subset of the permissions
-	 * of the current SID.
+	 * If the policy enables the nnp_nosuid_transition policy capability,
+	 * then we permit transitions under NNP or nosuid if the
+	 * policy allows the corresponding permission between
+	 * the old and new contexts.
 	 */
-	rc = security_bounded_transition(old_tsec->sid, new_tsec->sid);
-	if (rc) {
-		/*
-		 * On failure, preserve the errno values for NNP vs nosuid.
-		 * NNP:  Operation not permitted for caller.
-		 * nosuid:  Permission denied to file.
-		 */
+	if (selinux_policycap_nnp_nosuid_transition) {
+		av = 0;
 		if (nnp)
-			return -EPERM;
-		else
-			return -EACCES;
+			av |= PROCESS2__NNP_TRANSITION;
+		if (nosuid)
+			av |= PROCESS2__NOSUID_TRANSITION;
+		rc = avc_has_perm(old_tsec->sid, new_tsec->sid,
+				  SECCLASS_PROCESS2, av, NULL);
+		if (!rc)
+			return 0;
 	}
-	return 0;
+
+	/*
+	 * We also permit NNP or nosuid transitions to bounded SIDs,
+	 * i.e. SIDs that are guaranteed to only be allowed a subset
+	 * of the permissions of the current SID.
+	 */
+	rc = security_bounded_transition(old_tsec->sid, new_tsec->sid);
+	if (!rc)
+		return 0;
+
+	/*
+	 * On failure, preserve the errno values for NNP vs nosuid.
+	 * NNP:  Operation not permitted for caller.
+	 * nosuid:  Permission denied to file.
+	 */
+	if (nnp)
+		return -EPERM;
+	return -EACCES;
 }
 
 static int selinux_bprm_set_creds(struct linux_binprm *bprm)
diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h
index b9fe343..35ffb29 100644
--- a/security/selinux/include/classmap.h
+++ b/security/selinux/include/classmap.h
@@ -48,6 +48,8 @@ struct security_class_mapping secclass_map[] = {
 	    "setrlimit", "rlimitinh", "dyntransition", "setcurrent",
 	    "execmem", "execstack", "execheap", "setkeycreate",
 	    "setsockcreate", "getrlimit", NULL } },
+	{ "process2",
+	  { "nnp_transition", "nosuid_transition", NULL } },
 	{ "system",
 	  { "ipc_info", "syslog_read", "syslog_mod",
 	    "syslog_console", "module_request", "module_load", NULL } },
diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h
index e91f08c..3e32317 100644
--- a/security/selinux/include/security.h
+++ b/security/selinux/include/security.h
@@ -73,6 +73,7 @@ enum {
 	POLICYDB_CAPABILITY_EXTSOCKCLASS,
 	POLICYDB_CAPABILITY_ALWAYSNETWORK,
 	POLICYDB_CAPABILITY_CGROUPSECLABEL,
+	POLICYDB_CAPABILITY_NNP_NOSUID_TRANSITION,
 	__POLICYDB_CAPABILITY_MAX
 };
 #define POLICYDB_CAPABILITY_MAX (__POLICYDB_CAPABILITY_MAX - 1)
@@ -84,6 +85,7 @@ extern int selinux_policycap_openperm;
 extern int selinux_policycap_extsockclass;
 extern int selinux_policycap_alwaysnetwork;
 extern int selinux_policycap_cgroupseclabel;
+extern int selinux_policycap_nnp_nosuid_transition;
 
 /*
  * type_datum properties
diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index 2f02fa6..16c55de 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -76,7 +76,8 @@ char *selinux_policycap_names[__POLICYDB_CAPABILITY_MAX] = {
 	"open_perms",
 	"extended_socket_class",
 	"always_check_network",
-	"cgroup_seclabel"
+	"cgroup_seclabel",
+	"nnp_nosuid_transition"
 };
 
 int selinux_policycap_netpeer;
@@ -84,6 +85,7 @@ int selinux_policycap_openperm;
 int selinux_policycap_extsockclass;
 int selinux_policycap_alwaysnetwork;
 int selinux_policycap_cgroupseclabel;
+int selinux_policycap_nnp_nosuid_transition;
 
 static DEFINE_RWLOCK(policy_rwlock);
 
@@ -2009,6 +2011,9 @@ static void security_load_policycaps(void)
 	selinux_policycap_cgroupseclabel =
 		ebitmap_get_bit(&policydb.policycaps,
 				POLICYDB_CAPABILITY_CGROUPSECLABEL);
+	selinux_policycap_nnp_nosuid_transition =
+		ebitmap_get_bit(&policydb.policycaps,
+				POLICYDB_CAPABILITY_NNP_NOSUID_TRANSITION);
 
 	for (i = 0; i < ARRAY_SIZE(selinux_policycap_names); i++)
 		pr_info("SELinux:  policy capability %s=%d\n",
-- 
2.9.4

Re: [PATCH v4] selinux: Generalize support for NNP/nosuid SELinux domain transitions

[Paul Moore]

On Mon, Jul 31, 2017 at 10:12 AM, Stephen Smalley <sds@tycho.nsa.gov> wrote:
> As systemd ramps up enabling NNP (NoNewPrivileges) for system services,
> it is increasingly breaking SELinux domain transitions for those services
> and their descendants ...

...

> v4 moves both of the new permissions to the new process2 class and
> checks both if both NNP is enabled and the mount is nosuid.

This looks good to me, but I'm going to give it another day or two in
case the the policy folks want to comment.

-- 
paul moore
www.paul-moore.com

Re: [PATCH v4] selinux: Generalize support for NNP/nosuid SELinux domain transitions

[Chris PeBenito]

On 07/31/2017 06:43 PM, Paul Moore wrote:
> On Mon, Jul 31, 2017 at 10:12 AM, Stephen Smalley <sds@tycho.nsa.gov> wrote:
>> As systemd ramps up enabling NNP (NoNewPrivileges) for system services,
>> it is increasingly breaking SELinux domain transitions for those services
>> and their descendants ...
>
> ...
>
>> v4 moves both of the new permissions to the new process2 class and
>> checks both if both NNP is enabled and the mount is nosuid.
>
> This looks good to me, but I'm going to give it another day or two in
> case the the policy folks want to comment.

Looks good to me too.

-- 
Chris PeBenito

Re: [PATCH v4] selinux: Generalize support for NNP/nosuid SELinux domain transitions

[Paul Moore]

On Mon, Jul 31, 2017 at 10:12 AM, Stephen Smalley <sds@tycho.nsa.gov> wrote:
> As systemd ramps up enabling NNP (NoNewPrivileges) for system services,
> it is increasingly breaking SELinux domain transitions for those services
> and their descendants.  systemd enables NNP not only for services whose
> unit files explicitly specify NoNewPrivileges=yes but also for services
> whose unit files specify any of the following options in combination with
> running without CAP_SYS_ADMIN (e.g. specifying User= or a
> CapabilityBoundingSet= without CAP_SYS_ADMIN): SystemCallFilter=,
> SystemCallArchitectures=, RestrictAddressFamilies=, RestrictNamespaces=,
> PrivateDevices=, ProtectKernelTunables=, ProtectKernelModules=,
> MemoryDenyWriteExecute=, or RestrictRealtime= as per the systemd.exec(5)
> man page.
>
> The end result is bad for the security of both SELinux-disabled and
> SELinux-enabled systems.  Packagers have to turn off these
> options in the unit files to preserve SELinux domain transitions.  For
> users who choose to disable SELinux, this means that they miss out on
> at least having the systemd-supported protections.  For users who keep
> SELinux enabled, they may still be missing out on some protections
> because it isn't necessarily guaranteed that the SELinux policy for
> that service provides the same protections in all cases.
>
> commit 7b0d0b40cd78 ("selinux: Permit bounded transitions under
> NO_NEW_PRIVS or NOSUID.") allowed bounded transitions under NNP in
> order to support limited usage for sandboxing programs.  However,
> defining typebounds for all of the affected service domains
> is impractical to implement in policy, since typebounds requires us
> to ensure that each domain is allowed everything all of its descendant
> domains are allowed, and this has to be repeated for the entire chain
> of domain transitions.  There is no way to clone all allow rules from
> descendants to their ancestors in policy currently, and doing so would
> be undesirable even if it were practical, as it requires leaking
> permissions to objects and operations into ancestor domains that could
> weaken their own security in order to allow them to the descendants
> (e.g. if a descendant requires execmem permission, then so do all of
> its ancestors; if a descendant requires execute permission to a file,
> then so do all of its ancestors; if a descendant requires read to a
> symbolic link or temporary file, then so do all of its ancestors...).
> SELinux domains are intentionally not hierarchical / bounded in this
> manner normally, and making them so would undermine their protections
> and least privilege.
>
> We have long had a similar tension with SELinux transitions and nosuid
> mounts, albeit not as severe.  Users often have had to choose between
> retaining nosuid on a mount and allowing SELinux domain transitions on
> files within those mounts.  This likewise leads to unfortunate tradeoffs
> in security.
>
> Decouple NNP/nosuid from SELinux transitions, so that we don't have to
> make a choice between them. Introduce a nnp_nosuid_transition policy
> capability that enables transitions under NNP/nosuid to be based on
> a permission (nnp_transition for NNP; nosuid_transition for nosuid)
> between the old and new contexts in addition to the current support
> for bounded transitions.  Domain transitions can then be allowed in
> policy without requiring the parent to be a strict superset of all of
> its children.
>
> With this change, systemd unit files can be left unmodified from upstream.
> SELinux-disabled and SELinux-enabled users will benefit from retaining any
> of the systemd-provided protections.  SELinux policy will only need to
> be adapted to enable the new policy capability and to allow the
> new permissions between domain pairs as appropriate.
>
> NB: Allowing nnp_transition between two contexts opens up the potential
> for the old context to subvert the new context by installing seccomp
> filters before the execve.  Allowing nosuid_transition between two contexts
> opens up the potential for a context transition to occur on a file from
> an untrusted filesystem (e.g. removable media or remote filesystem).  Use
> with care.
>
> Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
> ---
> v4 moves both of the new permissions to the new process2 class and
> checks both if both NNP is enabled and the mount is nosuid.
>
>  security/selinux/hooks.c            | 47 +++++++++++++++++++++++++------------
>  security/selinux/include/classmap.h |  2 ++
>  security/selinux/include/security.h |  2 ++
>  security/selinux/ss/services.c      |  7 +++++-
>  4 files changed, 42 insertions(+), 16 deletions(-)

Merged.  Thanks Stephen and everyone else who reviewed and commented
on this patch.

> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 00ad46e..04b8e10 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -2318,6 +2318,7 @@ static int check_nnp_nosuid(const struct linux_binprm *bprm,
>         int nnp = (bprm->unsafe & LSM_UNSAFE_NO_NEW_PRIVS);
>         int nosuid = !mnt_may_suid(bprm->file->f_path.mnt);
>         int rc;
> +       u32 av;
>
>         if (!nnp && !nosuid)
>                 return 0; /* neither NNP nor nosuid */
> @@ -2326,24 +2327,40 @@ static int check_nnp_nosuid(const struct linux_binprm *bprm,
>                 return 0; /* No change in credentials */
>
>         /*
> -        * The only transitions we permit under NNP or nosuid
> -        * are transitions to bounded SIDs, i.e. SIDs that are
> -        * guaranteed to only be allowed a subset of the permissions
> -        * of the current SID.
> +        * If the policy enables the nnp_nosuid_transition policy capability,
> +        * then we permit transitions under NNP or nosuid if the
> +        * policy allows the corresponding permission between
> +        * the old and new contexts.
>          */
> -       rc = security_bounded_transition(old_tsec->sid, new_tsec->sid);
> -       if (rc) {
> -               /*
> -                * On failure, preserve the errno values for NNP vs nosuid.
> -                * NNP:  Operation not permitted for caller.
> -                * nosuid:  Permission denied to file.
> -                */
> +       if (selinux_policycap_nnp_nosuid_transition) {
> +               av = 0;
>                 if (nnp)
> -                       return -EPERM;
> -               else
> -                       return -EACCES;
> +                       av |= PROCESS2__NNP_TRANSITION;
> +               if (nosuid)
> +                       av |= PROCESS2__NOSUID_TRANSITION;
> +               rc = avc_has_perm(old_tsec->sid, new_tsec->sid,
> +                                 SECCLASS_PROCESS2, av, NULL);
> +               if (!rc)
> +                       return 0;
>         }
> -       return 0;
> +
> +       /*
> +        * We also permit NNP or nosuid transitions to bounded SIDs,
> +        * i.e. SIDs that are guaranteed to only be allowed a subset
> +        * of the permissions of the current SID.
> +        */
> +       rc = security_bounded_transition(old_tsec->sid, new_tsec->sid);
> +       if (!rc)
> +               return 0;
> +
> +       /*
> +        * On failure, preserve the errno values for NNP vs nosuid.
> +        * NNP:  Operation not permitted for caller.
> +        * nosuid:  Permission denied to file.
> +        */
> +       if (nnp)
> +               return -EPERM;
> +       return -EACCES;
>  }
>
>  static int selinux_bprm_set_creds(struct linux_binprm *bprm)
> diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h
> index b9fe343..35ffb29 100644
> --- a/security/selinux/include/classmap.h
> +++ b/security/selinux/include/classmap.h
> @@ -48,6 +48,8 @@ struct security_class_mapping secclass_map[] = {
>             "setrlimit", "rlimitinh", "dyntransition", "setcurrent",
>             "execmem", "execstack", "execheap", "setkeycreate",
>             "setsockcreate", "getrlimit", NULL } },
> +       { "process2",
> +         { "nnp_transition", "nosuid_transition", NULL } },
>         { "system",
>           { "ipc_info", "syslog_read", "syslog_mod",
>             "syslog_console", "module_request", "module_load", NULL } },
> diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h
> index e91f08c..3e32317 100644
> --- a/security/selinux/include/security.h
> +++ b/security/selinux/include/security.h
> @@ -73,6 +73,7 @@ enum {
>         POLICYDB_CAPABILITY_EXTSOCKCLASS,
>         POLICYDB_CAPABILITY_ALWAYSNETWORK,
>         POLICYDB_CAPABILITY_CGROUPSECLABEL,
> +       POLICYDB_CAPABILITY_NNP_NOSUID_TRANSITION,
>         __POLICYDB_CAPABILITY_MAX
>  };
>  #define POLICYDB_CAPABILITY_MAX (__POLICYDB_CAPABILITY_MAX - 1)
> @@ -84,6 +85,7 @@ extern int selinux_policycap_openperm;
>  extern int selinux_policycap_extsockclass;
>  extern int selinux_policycap_alwaysnetwork;
>  extern int selinux_policycap_cgroupseclabel;
> +extern int selinux_policycap_nnp_nosuid_transition;
>
>  /*
>   * type_datum properties
> diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
> index 2f02fa6..16c55de 100644
> --- a/security/selinux/ss/services.c
> +++ b/security/selinux/ss/services.c
> @@ -76,7 +76,8 @@ char *selinux_policycap_names[__POLICYDB_CAPABILITY_MAX] = {
>         "open_perms",
>         "extended_socket_class",
>         "always_check_network",
> -       "cgroup_seclabel"
> +       "cgroup_seclabel",
> +       "nnp_nosuid_transition"
>  };
>
>  int selinux_policycap_netpeer;
> @@ -84,6 +85,7 @@ int selinux_policycap_openperm;
>  int selinux_policycap_extsockclass;
>  int selinux_policycap_alwaysnetwork;
>  int selinux_policycap_cgroupseclabel;
> +int selinux_policycap_nnp_nosuid_transition;
>
>  static DEFINE_RWLOCK(policy_rwlock);
>
> @@ -2009,6 +2011,9 @@ static void security_load_policycaps(void)
>         selinux_policycap_cgroupseclabel =
>                 ebitmap_get_bit(&policydb.policycaps,
>                                 POLICYDB_CAPABILITY_CGROUPSECLABEL);
> +       selinux_policycap_nnp_nosuid_transition =
> +               ebitmap_get_bit(&policydb.policycaps,
> +                               POLICYDB_CAPABILITY_NNP_NOSUID_TRANSITION);
>
>         for (i = 0; i < ARRAY_SIZE(selinux_policycap_names); i++)
>                 pr_info("SELinux:  policy capability %s=%d\n",
> --
> 2.9.4
>



-- 
paul moore
www.paul-moore.com