LSM: SafeSetID: Add GID security policy handling

The SafeSetID LSM has functionality for restricting setuid() calls based
on its configured security policies. This patch adds the analogous
functionality for setgid() calls. This is mostly a copy-and-paste change
with some code deduplication, plus slight modifications/name changes to
the policy-rule-related structs (now contain GID rules in addition to
the UID ones) and some type generalization since SafeSetID now needs to
deal with kgid_t and kuid_t types.

Signed-off-by: Thomas Cedeno <thomascedeno@google.com>
Signed-off-by: Micah Morton <mortonm@chromium.org>

1 files changed, 137 insertions, 53 deletions

diff --git a/security/safesetid/securityfs.c b/security/safesetid/securityfs.c
index f8bc574cea9c..642139008d42 100644
--- a/security/safesetid/securityfs.c
+++ b/security/safesetid/securityfs.c
@@ -19,22 +19,23 @@
 
 #include "lsm.h"
 
-static DEFINE_MUTEX(policy_update_lock);
+static DEFINE_MUTEX(uid_policy_update_lock);
+static DEFINE_MUTEX(gid_policy_update_lock);
 
 /*
- * In the case the input buffer contains one or more invalid UIDs, the kuid_t
+ * In the case the input buffer contains one or more invalid IDs, the kid_t
  * variables pointed to by @parent and @child will get updated but this
  * function will return an error.
  * Contents of @buf may be modified.
  */
 static int parse_policy_line(struct file *file, char *buf,
-	struct setuid_rule *rule)
+	struct setid_rule *rule)
 {
 	char *child_str;
 	int ret;
 	u32 parsed_parent, parsed_child;
 
-	/* Format of |buf| string should be <UID>:<UID>. */
+	/* Format of |buf| string should be <UID>:<UID> or <GID>:<GID> */
 	child_str = strchr(buf, ':');
 	if (child_str == NULL)
 		return -EINVAL;
@@ -49,20 +50,29 @@ static int parse_policy_line(struct file *file, char *buf,
 	if (ret)
 		return ret;
 
-	rule->src_uid = make_kuid(file->f_cred->user_ns, parsed_parent);
-	rule->dst_uid = make_kuid(file->f_cred->user_ns, parsed_child);
-	if (!uid_valid(rule->src_uid) || !uid_valid(rule->dst_uid))
+	if (rule->type == UID){
+		rule->src_id.uid = make_kuid(file->f_cred->user_ns, parsed_parent);
+		rule->dst_id.uid = make_kuid(file->f_cred->user_ns, parsed_child);
+		if (!uid_valid(rule->src_id.uid) || !uid_valid(rule->dst_id.uid))
+			return -EINVAL;
+	} else if (rule->type == GID){
+		rule->src_id.gid = make_kgid(file->f_cred->user_ns, parsed_parent);
+		rule->dst_id.gid = make_kgid(file->f_cred->user_ns, parsed_child);
+		if (!gid_valid(rule->src_id.gid) || !gid_valid(rule->dst_id.gid))
+			return -EINVAL;
+	} else {
+		/* Error, rule->type is an invalid type */
 		return -EINVAL;
-
+	}
 	return 0;
 }
 
 static void __release_ruleset(struct rcu_head *rcu)
 {
-	struct setuid_ruleset *pol =
-		container_of(rcu, struct setuid_ruleset, rcu);
+	struct setid_ruleset *pol =
+		container_of(rcu, struct setid_ruleset, rcu);
 	int bucket;
-	struct setuid_rule *rule;
+	struct setid_rule *rule;
 	struct hlist_node *tmp;
 
 	hash_for_each_safe(pol->rules, bucket, tmp, rule, next)
@@ -71,36 +81,55 @@ static void __release_ruleset(struct rcu_head *rcu)
 	kfree(pol);
 }
 
-static void release_ruleset(struct setuid_ruleset *pol)
-{
+static void release_ruleset(struct setid_ruleset *pol){
 	call_rcu(&pol->rcu, __release_ruleset);
 }
 
-static void insert_rule(struct setuid_ruleset *pol, struct setuid_rule *rule)
+static void insert_rule(struct setid_ruleset *pol, struct setid_rule *rule)
 {
-	hash_add(pol->rules, &rule->next, __kuid_val(rule->src_uid));
+	if (pol->type == UID)
+		hash_add(pol->rules, &rule->next, __kuid_val(rule->src_id.uid));
+	else if (pol->type == GID)
+		hash_add(pol->rules, &rule->next, __kgid_val(rule->src_id.gid));
+	else /* Error, pol->type is neither UID or GID */
+		return;
 }
 
-static int verify_ruleset(struct setuid_ruleset *pol)
+static int verify_ruleset(struct setid_ruleset *pol)
 {
 	int bucket;
-	struct setuid_rule *rule, *nrule;
+	struct setid_rule *rule, *nrule;
 	int res = 0;
 
 	hash_for_each(pol->rules, bucket, rule, next) {
-		if (_setuid_policy_lookup(pol, rule->dst_uid, INVALID_UID) ==
-		    SIDPOL_DEFAULT) {
-			pr_warn("insecure policy detected: uid %d is constrained but transitively unconstrained through uid %d\n",
-				__kuid_val(rule->src_uid),
-				__kuid_val(rule->dst_uid));
+		if (_setid_policy_lookup(pol, rule->dst_id, INVALID_ID) == SIDPOL_DEFAULT) {
+			if (pol->type == UID) {
+				pr_warn("insecure policy detected: uid %d is constrained but transitively unconstrained through uid %d\n",
+					__kuid_val(rule->src_id.uid),
+					__kuid_val(rule->dst_id.uid));
+			} else if (pol->type == GID) {
+				pr_warn("insecure policy detected: gid %d is constrained but transitively unconstrained through gid %d\n",
+					__kgid_val(rule->src_id.gid),
+					__kgid_val(rule->dst_id.gid));
+			} else { /* pol->type is an invalid type */
+				res = -EINVAL;
+				return res;
+			}
 			res = -EINVAL;
 
 			/* fix it up */
-			nrule = kmalloc(sizeof(struct setuid_rule), GFP_KERNEL);
+			nrule = kmalloc(sizeof(struct setid_rule), GFP_KERNEL);
 			if (!nrule)
 				return -ENOMEM;
-			nrule->src_uid = rule->dst_uid;
-			nrule->dst_uid = rule->dst_uid;
+			if (pol->type == UID){
+				nrule->src_id.uid = rule->dst_id.uid;
+				nrule->dst_id.uid = rule->dst_id.uid;
+				nrule->type = UID;
+			} else { /* pol->type must be GID if we've made it to here */
+				nrule->src_id.gid = rule->dst_id.gid;
+				nrule->dst_id.gid = rule->dst_id.gid;
+				nrule->type = GID;
+			}
 			insert_rule(pol, nrule);
 		}
 	}
@@ -108,16 +137,17 @@ static int verify_ruleset(struct setuid_ruleset *pol)
 }
 
 static ssize_t handle_policy_update(struct file *file,
-				    const char __user *ubuf, size_t len)
+				    const char __user *ubuf, size_t len, enum setid_type policy_type)
 {
-	struct setuid_ruleset *pol;
+	struct setid_ruleset *pol;
 	char *buf, *p, *end;
 	int err;
 
-	pol = kmalloc(sizeof(struct setuid_ruleset), GFP_KERNEL);
+	pol = kmalloc(sizeof(struct setid_ruleset), GFP_KERNEL);
 	if (!pol)
 		return -ENOMEM;
 	pol->policy_str = NULL;
+	pol->type = policy_type;
 	hash_init(pol->rules);
 
 	p = buf = memdup_user_nul(ubuf, len);
@@ -133,7 +163,7 @@ static ssize_t handle_policy_update(struct file *file,
 
 	/* policy lines, including the last one, end with \n */
 	while (*p != '\0') {
-		struct setuid_rule *rule;
+		struct setid_rule *rule;
 
 		end = strchr(p, '\n');
 		if (end == NULL) {
@@ -142,18 +172,18 @@ static ssize_t handle_policy_update(struct file *file,
 		}
 		*end = '\0';
 
-		rule = kmalloc(sizeof(struct setuid_rule), GFP_KERNEL);
+		rule = kmalloc(sizeof(struct setid_rule), GFP_KERNEL);
 		if (!rule) {
 			err = -ENOMEM;
 			goto out_free_buf;
 		}
 
+		rule->type = policy_type;
 		err = parse_policy_line(file, p, rule);
 		if (err)
 			goto out_free_rule;
 
-		if (_setuid_policy_lookup(pol, rule->src_uid, rule->dst_uid) ==
-		    SIDPOL_ALLOWED) {
+		if (_setid_policy_lookup(pol, rule->src_id, rule->dst_id) == SIDPOL_ALLOWED) {
 			pr_warn("bad policy: duplicate entry\n");
 			err = -EEXIST;
 			goto out_free_rule;
@@ -178,21 +208,31 @@ out_free_rule:
 	 * What we really want here is an xchg() wrapper for RCU, but since that
 	 * doesn't currently exist, just use a spinlock for now.
 	 */
-	mutex_lock(&policy_update_lock);
-	pol = rcu_replace_pointer(safesetid_setuid_rules, pol,
-				  lockdep_is_held(&policy_update_lock));
-	mutex_unlock(&policy_update_lock);
+	if (policy_type == UID) {
+		mutex_lock(&uid_policy_update_lock);
+		pol = rcu_replace_pointer(safesetid_setuid_rules, pol,
+					  lockdep_is_held(&uid_policy_update_lock));
+		mutex_unlock(&uid_policy_update_lock);
+	} else if (policy_type == GID) {
+		mutex_lock(&gid_policy_update_lock);
+		pol = rcu_replace_pointer(safesetid_setgid_rules, pol,
+					  lockdep_is_held(&gid_policy_update_lock));
+		mutex_unlock(&gid_policy_update_lock);
+	} else {
+		/* Error, policy type is neither UID or GID */
+		pr_warn("error: bad policy type");
+	}
 	err = len;
 
 out_free_buf:
 	kfree(buf);
 out_free_pol:
 	if (pol)
-                release_ruleset(pol);
+		release_ruleset(pol);
 	return err;
 }
 
-static ssize_t safesetid_file_write(struct file *file,
+static ssize_t safesetid_uid_file_write(struct file *file,
 				    const char __user *buf,
 				    size_t len,
 				    loff_t *ppos)
@@ -203,38 +243,74 @@ static ssize_t safesetid_file_write(struct file *file,
 	if (*ppos != 0)
 		return -EINVAL;
 
-	return handle_policy_update(file, buf, len);
+	return handle_policy_update(file, buf, len, UID);
+}
+
+static ssize_t safesetid_gid_file_write(struct file *file,
+				    const char __user *buf,
+				    size_t len,
+				    loff_t *ppos)
+{
+	if (!file_ns_capable(file, &init_user_ns, CAP_MAC_ADMIN))
+		return -EPERM;
+
+	if (*ppos != 0)
+		return -EINVAL;
+
+	return handle_policy_update(file, buf, len, GID);
 }
 
 static ssize_t safesetid_file_read(struct file *file, char __user *buf,
-				   size_t len, loff_t *ppos)
+				   size_t len, loff_t *ppos, struct mutex *policy_update_lock, struct setid_ruleset* ruleset)
 {
 	ssize_t res = 0;
-	struct setuid_ruleset *pol;
+	struct setid_ruleset *pol;
 	const char *kbuf;
 
-	mutex_lock(&policy_update_lock);
-	pol = rcu_dereference_protected(safesetid_setuid_rules,
-					lockdep_is_held(&policy_update_lock));
+	mutex_lock(policy_update_lock);
+	pol = rcu_dereference_protected(ruleset, lockdep_is_held(policy_update_lock));
 	if (pol) {
 		kbuf = pol->policy_str;
 		res = simple_read_from_buffer(buf, len, ppos,
 					      kbuf, strlen(kbuf));
 	}
-	mutex_unlock(&policy_update_lock);
+	mutex_unlock(policy_update_lock);
+
 	return res;
 }
 
-static const struct file_operations safesetid_file_fops = {
-	.read = safesetid_file_read,
-	.write = safesetid_file_write,
+static ssize_t safesetid_uid_file_read(struct file *file, char __user *buf,
+				   size_t len, loff_t *ppos)
+{
+	return safesetid_file_read(file, buf, len, ppos,
+				   &uid_policy_update_lock, safesetid_setuid_rules);
+}
+
+static ssize_t safesetid_gid_file_read(struct file *file, char __user *buf,
+				   size_t len, loff_t *ppos)
+{
+	return safesetid_file_read(file, buf, len, ppos,
+				   &gid_policy_update_lock, safesetid_setgid_rules);
+}
+
+
+
+static const struct file_operations safesetid_uid_file_fops = {
+	.read = safesetid_uid_file_read,
+	.write = safesetid_uid_file_write,
+};
+
+static const struct file_operations safesetid_gid_file_fops = {
+	.read = safesetid_gid_file_read,
+	.write = safesetid_gid_file_write,
 };
 
 static int __init safesetid_init_securityfs(void)
 {
 	int ret;
 	struct dentry *policy_dir;
-	struct dentry *policy_file;
+	struct dentry *uid_policy_file;
+	struct dentry *gid_policy_file;
 
 	if (!safesetid_initialized)
 		return 0;
@@ -245,13 +321,21 @@ static int __init safesetid_init_securityfs(void)
 		goto error;
 	}
 
-	policy_file = securityfs_create_file("whitelist_policy", 0600,
-			policy_dir, NULL, &safesetid_file_fops);
-	if (IS_ERR(policy_file)) {
-		ret = PTR_ERR(policy_file);
+	uid_policy_file = securityfs_create_file("uid_allowlist_policy", 0600,
+			policy_dir, NULL, &safesetid_uid_file_fops);
+	if (IS_ERR(uid_policy_file)) {
+		ret = PTR_ERR(uid_policy_file);
 		goto error;
 	}
 
+	gid_policy_file = securityfs_create_file("gid_allowlist_policy", 0600,
+			policy_dir, NULL, &safesetid_gid_file_fops);
+	if (IS_ERR(gid_policy_file)) {
+		ret = PTR_ERR(gid_policy_file);
+		goto error;
+	}
+
+
 	return 0;
 
 error: