[PATCH] selinux: remove unused initial SIDs and improve handling

* [PATCH] selinux: remove unused initial SIDs and improve handling
@ 2020-01-27 20:55 Stephen Smalley
  2020-01-28 18:13 ` Stephen Smalley
  0 siblings, 1 reply; 2+ messages in thread
From: Stephen Smalley @ 2020-01-27 20:55 UTC (permalink / raw)
  To: paul; +Cc: selinux, omosnace, Stephen Smalley

Remove initial SIDs that have never been used or are no longer
used by the kernel from its string table, which is also used
to generate the SECINITSID_* symbols referenced in code.
Update the code to gracefully handle the fact that these can
now be NULL. Stop treating it as an error if a policy defines
additional initial SIDs unknown to the kernel, or if the policy
leaves one of the unused initial SIDs without a defined context.
Fix the incorrect usage of the name from the ocontext in error
messages when loading initial SIDs since these are not presently
written to the kernel policy and are therefore always NULL.

This is a first step toward enabling future evolution of
initial SIDs. Further changes are required to both userspace
and the kernel to fully address
https://github.com/SELinuxProject/selinux-kernel/issues/12
but this takes a small step toward that end.  NB Even with
this change, one cannot yet add or remove initial SIDs in
policy without breakage; separate changes to the policy
compiler are still necessary.  Further, fully decoupling
the policy and kernel initial SID values will require a policy
format/version change to include the SID names in the
kernel policy so that they can be dynamically mapped at
policy load.

Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
---
 scripts/selinux/genheaders/genheaders.c       | 11 +++-
 .../selinux/include/initial_sid_to_string.h   | 57 +++++++++----------
 security/selinux/selinuxfs.c                  |  6 +-
 security/selinux/ss/policydb.c                | 28 ++++++---
 security/selinux/ss/services.c                | 26 ++++-----
 5 files changed, 73 insertions(+), 55 deletions(-)

diff --git a/scripts/selinux/genheaders/genheaders.c b/scripts/selinux/genheaders/genheaders.c
index 544ca126a8a8..f355b3e0e968 100644
--- a/scripts/selinux/genheaders/genheaders.c
+++ b/scripts/selinux/genheaders/genheaders.c
@@ -67,8 +67,12 @@ int main(int argc, char *argv[])
 	}
 
 	isids_len = sizeof(initial_sid_to_string) / sizeof (char *);
-	for (i = 1; i < isids_len; i++)
-		initial_sid_to_string[i] = stoupperx(initial_sid_to_string[i]);
+	for (i = 1; i < isids_len; i++) {
+		const char *s = initial_sid_to_string[i];
+
+		if (s)
+			initial_sid_to_string[i] = stoupperx(s);
+	}
 
 	fprintf(fout, "/* This file is automatically generated.  Do not edit. */\n");
 	fprintf(fout, "#ifndef _SELINUX_FLASK_H_\n#define _SELINUX_FLASK_H_\n\n");
@@ -82,7 +86,8 @@ int main(int argc, char *argv[])
 
 	for (i = 1; i < isids_len; i++) {
 		const char *s = initial_sid_to_string[i];
-		fprintf(fout, "#define SECINITSID_%-39s %2d\n", s, i);
+		if (s)
+			fprintf(fout, "#define SECINITSID_%-39s %2d\n", s, i);
 	}
 	fprintf(fout, "\n#define SECINITSID_NUM %d\n", i-1);
 	fprintf(fout, "\nstatic inline bool security_is_socket_class(u16 kern_tclass)\n");
diff --git a/security/selinux/include/initial_sid_to_string.h b/security/selinux/include/initial_sid_to_string.h
index 4f93f697f71c..465ea0923a7c 100644
--- a/security/selinux/include/initial_sid_to_string.h
+++ b/security/selinux/include/initial_sid_to_string.h
@@ -1,34 +1,33 @@
 /* SPDX-License-Identifier: GPL-2.0 */
-/* This file is automatically generated.  Do not edit. */
 static const char *initial_sid_to_string[] =
 {
-    "null",
-    "kernel",
-    "security",
-    "unlabeled",
-    "fs",
-    "file",
-    "file_labels",
-    "init",
-    "any_socket",
-    "port",
-    "netif",
-    "netmsg",
-    "node",
-    "igmp_packet",
-    "icmp_socket",
-    "tcp_socket",
-    "sysctl_modprobe",
-    "sysctl",
-    "sysctl_fs",
-    "sysctl_kernel",
-    "sysctl_net",
-    "sysctl_net_unix",
-    "sysctl_vm",
-    "sysctl_dev",
-    "kmod",
-    "policy",
-    "scmp_packet",
-    "devnull",
+	"null",
+	"kernel",
+	"security",
+	"unlabeled",
+	NULL,
+	"file",
+	NULL,
+	NULL,
+	"any_socket",
+	"port",
+	"netif",
+	"netmsg",
+	"node",
+	NULL,
+	NULL,
+	NULL,
+	NULL,
+	NULL,
+	NULL,
+	NULL,
+	NULL,
+	NULL,
+	NULL,
+	NULL,
+	NULL,
+	NULL,
+	NULL,
+	"devnull",
 };
 
diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c
index 79c710911a3c..daddc880ebfc 100644
--- a/security/selinux/selinuxfs.c
+++ b/security/selinux/selinuxfs.c
@@ -1692,7 +1692,11 @@ static int sel_make_initcon_files(struct dentry *dir)
 	for (i = 1; i <= SECINITSID_NUM; i++) {
 		struct inode *inode;
 		struct dentry *dentry;
-		dentry = d_alloc_name(dir, security_get_initial_sid_context(i));
+		const char *s = security_get_initial_sid_context(i);
+
+		if (!s)
+			continue;
+		dentry = d_alloc_name(dir, s);
 		if (!dentry)
 			return -ENOMEM;
 
diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
index 2aa7f2e1a8e7..6ea2dab339ad 100644
--- a/security/selinux/ss/policydb.c
+++ b/security/selinux/ss/policydb.c
@@ -865,16 +865,27 @@ int policydb_load_isids(struct policydb *p, struct sidtab *s)
 
 	head = p->ocontexts[OCON_ISID];
 	for (c = head; c; c = c->next) {
-		rc = -EINVAL;
-		if (!c->context[0].user) {
-			pr_err("SELinux:  SID %s was never defined.\n",
-				c->u.name);
+		u32 sid = c->sid[0];
+		const char *name;
+
+		if (sid == SECSID_NULL) {
+			pr_err("SELinux:  SID null was assigned a context.\n");
 			sidtab_destroy(s);
 			goto out;
 		}
-		if (c->sid[0] == SECSID_NULL || c->sid[0] > SECINITSID_NUM) {
-			pr_err("SELinux:  Initial SID %s out of range.\n",
-				c->u.name);
+
+		/* Ignore initial SIDs defined for future kernels. */
+		if (sid > SECINITSID_NUM)
+			continue;
+
+		name = security_get_initial_sid_context(sid);
+		rc = -EINVAL;
+		if (!c->context[0].user) {
+			/* Ignore undefined SIDs that are not in use. */
+			if (!name)
+				continue;
+			pr_err("SELinux:  SID %s was never defined.\n",
+				name);
 			sidtab_destroy(s);
 			goto out;
 		}
@@ -883,11 +894,10 @@ int policydb_load_isids(struct policydb *p, struct sidtab *s)
 			sidtab_destroy(s);
 			goto out;
 		}
-
 		rc = sidtab_set_initial(s, c->sid[0], &c->context[0]);
 		if (rc) {
 			pr_err("SELinux:  unable to load initial SID %s.\n",
-				c->u.name);
+				name);
 			sidtab_destroy(s);
 			goto out;
 		}
diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index 216ce602a2b5..bd924a9a6388 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -1323,23 +1323,22 @@ static int security_sid_to_context_core(struct selinux_state *state,
 	if (!selinux_initialized(state)) {
 		if (sid <= SECINITSID_NUM) {
 			char *scontextp;
+			const char *s = initial_sid_to_string[sid];
 
-			*scontext_len = strlen(initial_sid_to_string[sid]) + 1;
+			if (!s)
+				return -EINVAL;
+			*scontext_len = strlen(s) + 1;
 			if (!scontext)
-				goto out;
-			scontextp = kmemdup(initial_sid_to_string[sid],
-					    *scontext_len, GFP_ATOMIC);
-			if (!scontextp) {
-				rc = -ENOMEM;
-				goto out;
-			}
+				return 0;
+			scontextp = kmemdup(s, *scontext_len, GFP_ATOMIC);
+			if (!scontextp)
+				return -ENOMEM;
 			*scontext = scontextp;
-			goto out;
+			return 0;
 		}
 		pr_err("SELinux: %s:  called before initial "
 		       "load_policy on unknown SID %d\n", __func__, sid);
-		rc = -EINVAL;
-		goto out;
+		return -EINVAL;
 	}
 	read_lock(&state->ss->policy_rwlock);
 	policydb = &state->ss->policydb;
@@ -1363,7 +1362,6 @@ static int security_sid_to_context_core(struct selinux_state *state,
 
 out_unlock:
 	read_unlock(&state->ss->policy_rwlock);
-out:
 	return rc;
 
 }
@@ -1553,7 +1551,9 @@ static int security_context_to_sid_core(struct selinux_state *state,
 		int i;
 
 		for (i = 1; i < SECINITSID_NUM; i++) {
-			if (!strcmp(initial_sid_to_string[i], scontext2)) {
+			const char *s = initial_sid_to_string[i];
+
+			if (s && !strcmp(s, scontext2)) {
 				*sid = i;
 				goto out;
 			}
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 2+ messages in thread