Linux Security Modules development
 help / color / mirror / Atom feed
* [PATCH 68/90] LSM: Support multiple LSMs using inode_init_security
From: Casey Schaufler @ 2019-04-19  0:45 UTC (permalink / raw)
  To: casey.schaufler, jmorris, linux-security-module, selinux; +Cc: casey
In-Reply-To: <20190419004617.64627-1-casey@schaufler-ca.com>

Refactor security_inode_init_security() so that it can
do the integrity processing for more than one LSM.

Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
---
 security/security.c | 48 +++++++++++++++++++++++++++------------------
 1 file changed, 29 insertions(+), 19 deletions(-)

diff --git a/security/security.c b/security/security.c
index 0c749816fb7b..b8c90e7c4554 100644
--- a/security/security.c
+++ b/security/security.c
@@ -1064,9 +1064,10 @@ int security_inode_init_security(struct inode *inode, struct inode *dir,
 				 const struct qstr *qstr,
 				 const initxattrs initxattrs, void *fs_data)
 {
-	struct xattr new_xattrs[MAX_LSM_EVM_XATTR + 1];
-	struct xattr *lsm_xattr, *evm_xattr, *xattr;
-	int ret;
+	struct security_hook_list *p;
+	struct xattr *repo;
+	int rc;
+	int i;
 
 	if (unlikely(IS_PRIVATE(inode)))
 		return 0;
@@ -1074,24 +1075,33 @@ int security_inode_init_security(struct inode *inode, struct inode *dir,
 	if (!initxattrs)
 		return call_int_hook(inode_init_security, -EOPNOTSUPP, inode,
 				     dir, qstr, NULL, NULL, NULL);
-	memset(new_xattrs, 0, sizeof(new_xattrs));
-	lsm_xattr = new_xattrs;
-	ret = call_int_hook(inode_init_security, -EOPNOTSUPP, inode, dir, qstr,
-						&lsm_xattr->name,
-						&lsm_xattr->value,
-						&lsm_xattr->value_len);
-	if (ret)
-		goto out;
 
-	evm_xattr = lsm_xattr + 1;
-	ret = evm_inode_init_security(inode, lsm_xattr, evm_xattr);
-	if (ret)
-		goto out;
-	ret = initxattrs(inode, new_xattrs, fs_data);
+	repo = kzalloc((LSM_COUNT * 2) * sizeof(*repo), GFP_NOFS);
+	if (repo == NULL)
+		return -ENOMEM;
+
+	i = 0;
+	rc = -EOPNOTSUPP;
+	hlist_for_each_entry(p, &security_hook_heads.inode_init_security,
+			     list) {
+		rc = p->hook.inode_init_security(inode, dir, qstr,
+						 &repo[i].name, &repo[i].value,
+						 &repo[i].value_len);
+		if (rc)
+			goto out;
+
+		rc = evm_inode_init_security(inode, &repo[i], &repo[i + 1]);
+		if (rc)
+			goto out;
+
+		i += 2;
+	}
+	rc = initxattrs(inode, repo, fs_data);
 out:
-	for (xattr = new_xattrs; xattr->value != NULL; xattr++)
-		kfree(xattr->value);
-	return (ret == -EOPNOTSUPP) ? 0 : ret;
+	for (i-- ; i >= 0; i--)
+		kfree(repo[i].value);
+	kfree(repo);
+	return (rc == -EOPNOTSUPP) ? 0 : rc;
 }
 EXPORT_SYMBOL(security_inode_init_security);
 
-- 
2.19.1


^ permalink raw reply related

* [PATCH 69/90] LSM: Use full security context in security_inode_setsecctx
From: Casey Schaufler @ 2019-04-19  0:45 UTC (permalink / raw)
  To: casey.schaufler, jmorris, linux-security-module, selinux; +Cc: casey
In-Reply-To: <20190419004617.64627-1-casey@schaufler-ca.com>

The security hooks security_inode_setsecctx and security_inode_getsecctx
need to maintain the context strings for any and all LSMs that
provide contexts. This information is internal to the kernel
and volitile. If only one LSM uses this information the raw form is
used.

Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
---
 security/security.c | 110 +++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 108 insertions(+), 2 deletions(-)

diff --git a/security/security.c b/security/security.c
index b8c90e7c4554..05a19b28e105 100644
--- a/security/security.c
+++ b/security/security.c
@@ -425,6 +425,9 @@ static int lsm_append(char *new, char **result)
 /* Base list of once-only hooks */
 struct lsm_one_hooks lsm_base_one;
 
+/* Count of inode_[gs]etsecctx hooks */
+static int lsm_inode_secctx_count;
+
 /**
  * security_add_hooks - Add a modules hooks to the hook lists.
  * @hooks: the hooks to add
@@ -442,6 +445,15 @@ void __init security_add_hooks(struct security_hook_list *hooks, int count,
 		hooks[i].lsm = lsm;
 		hlist_add_tail_rcu(&hooks[i].list, hooks[i].head);
 
+		/*
+		 * Keep count of the internal security context using hooks.
+		 * Assume that there is a 1:1 mapping from inode_getsecctx
+		 * to inode_setsecctx in the security modules.
+		 */
+		if (hooks[i].head == &security_hook_heads.inode_getsecctx) {
+			lsm_inode_secctx_count++;
+			continue;
+		}
 		/*
 		 * Check for the special hooks that are restricted to
 		 * a single module to create the base set. Use the hooks
@@ -2150,15 +2162,109 @@ int security_inode_notifysecctx(struct inode *inode, struct lsm_context *cp)
 }
 EXPORT_SYMBOL(security_inode_notifysecctx);
 
+/*
+ * The inode_[gs]etsecctx functions need to proved a context
+ * for multiple security modules. If there is more than one
+ * LSM supplying hooks the format will be
+ *	lsm1='value',lsm2='value'[,lsmN='value']...
+ */
+static void lsm_release_secctx(struct lsm_context *cp)
+{
+	kfree(cp->context);
+}
+
 int security_inode_setsecctx(struct dentry *dentry, struct lsm_context *cp)
 {
-	return call_int_hook(inode_setsecctx, 0, dentry, cp);
+	struct security_hook_list *hp;
+	struct lsm_context lc;
+	char *full;
+	char *ctx;
+	char *quote;
+	int rc = 0;
+
+	if (lsm_inode_secctx_count <= 1)
+		return call_int_hook(inode_setsecctx, 0, dentry, cp);
+
+	full = kstrndup(cp->context, cp->len, GFP_KERNEL);
+	if (full == NULL)
+		return -ENOMEM;
+
+	ctx = full;
+	hlist_for_each_entry(hp, &security_hook_heads.inode_setsecctx, list) {
+		if (strncmp(ctx, hp->lsm, strlen(hp->lsm))) {
+			WARN_ONCE(1, "security_inode_setsecctx form1 error\n");
+			rc = -EINVAL;
+			break;
+		}
+		ctx += strlen(hp->lsm);
+		if (ctx[0] != '=' || ctx[1] != '\'') {
+			WARN_ONCE(1, "security_inode_setsecctx form2 error\n");
+			rc = -EINVAL;
+			break;
+		}
+		ctx += 2;
+		quote = strnchr(ctx, cp->len, '\'');
+		if (quote == NULL) {
+			WARN_ONCE(1, "security_inode_setsecctx form3 error\n");
+			rc = -EINVAL;
+			break;
+		}
+		quote[0] = '\0';
+		if (quote[1] != ',' && quote[1] != '\0') {
+			WARN_ONCE(1, "security_inode_setsecctx form4 error\n");
+			rc = -EINVAL;
+			break;
+		}
+		lc.context = ctx;
+		lc.len = strlen(ctx);
+
+		ctx = quote + 2;
+
+		rc = hp->hook.inode_setsecctx(dentry, &lc);
+		if (rc)
+			break;
+	}
+
+	kfree(full);
+	return rc;
 }
 EXPORT_SYMBOL(security_inode_setsecctx);
 
 int security_inode_getsecctx(struct inode *inode, struct lsm_context *cp)
 {
-	return call_int_hook(inode_getsecctx, -EOPNOTSUPP, inode, cp);
+	struct security_hook_list *hp;
+	struct lsm_context lc;
+	char *final = NULL;
+	char *tp;
+	int rc;
+
+	if (lsm_inode_secctx_count <= 1)
+		return call_int_hook(inode_getsecctx, -EOPNOTSUPP, inode, cp);
+
+	hlist_for_each_entry(hp, &security_hook_heads.inode_getsecctx, list) {
+		rc = hp->hook.inode_getsecctx(inode, &lc);
+		if (rc) {
+			kfree(final);
+			return rc;
+		}
+		if (final) {
+			tp = kasprintf(GFP_KERNEL, "%s,%s='%s'", final,
+				       hp->lsm, lc.context);
+			kfree(final);
+		} else
+			tp = kasprintf(GFP_KERNEL, "%s='%s'", hp->lsm,
+				       lc.context);
+		security_release_secctx(&lc);
+		if (tp == NULL) {
+			kfree(final);
+			return -ENOMEM;
+		}
+		final = tp;
+	}
+	cp->context = final;
+	cp->len = strlen(final);
+	cp->release = lsm_release_secctx;
+	return 0;
 }
 EXPORT_SYMBOL(security_inode_getsecctx);
 
-- 
2.19.1


^ permalink raw reply related

* [PATCH 70/90] LSM: Correct handling of ENOSYS in inode_setxattr
From: Casey Schaufler @ 2019-04-19  0:45 UTC (permalink / raw)
  To: casey.schaufler, jmorris, linux-security-module, selinux; +Cc: casey
In-Reply-To: <20190419004617.64627-1-casey@schaufler-ca.com>

The usual "bail on fail" behavior of LSM hooks doesn't
work for security_inode_setxattr(). Modules are allowed
to return -ENOSYS if the attribute specifed isn't one
they manage. Fix the code to accomodate this unusal case.
This requires changes to the hooks in SELinux and Smack.

Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
---
 security/security.c        | 28 ++++++++++++++--------------
 security/selinux/hooks.c   |  7 ++-----
 security/smack/smack_lsm.c | 10 +++++-----
 3 files changed, 21 insertions(+), 24 deletions(-)

diff --git a/security/security.c b/security/security.c
index 05a19b28e105..f1e2ffe81829 100644
--- a/security/security.c
+++ b/security/security.c
@@ -1341,24 +1341,24 @@ int security_inode_getattr(const struct path *path)
 int security_inode_setxattr(struct dentry *dentry, const char *name,
 			    const void *value, size_t size, int flags)
 {
-	int ret;
+	struct security_hook_list *hp;
+	int rc = -ENOSYS;
 
 	if (unlikely(IS_PRIVATE(d_backing_inode(dentry))))
 		return 0;
-	/*
-	 * SELinux and Smack integrate the cap call,
-	 * so assume that all LSMs supplying this call do so.
-	 */
-	ret = call_int_hook(inode_setxattr, 1, dentry, name, value, size,
-				flags);
 
-	if (ret == 1)
-		ret = cap_inode_setxattr(dentry, name, value, size, flags);
-	if (ret)
-		return ret;
-	ret = ima_inode_setxattr(dentry, name, value, size);
-	if (ret)
-		return ret;
+	hlist_for_each_entry(hp, &security_hook_heads.inode_setxattr, list) {
+		rc = hp->hook.inode_setxattr(dentry, name, value, size, flags);
+		if (rc != -ENOSYS)
+			break;
+	}
+	if (rc == -ENOSYS)
+		rc = cap_inode_setxattr(dentry, name, value, size, flags);
+	if (rc)
+		return rc;
+	rc = ima_inode_setxattr(dentry, name, value, size);
+	if (rc)
+		return rc;
 	return evm_inode_setxattr(dentry, name, value, size);
 }
 
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index af0d98f4dd37..17ba47f9f4e2 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -3097,13 +3097,10 @@ static int selinux_inode_setxattr(struct dentry *dentry, const char *name,
 	int rc = 0;
 
 	if (strcmp(name, XATTR_NAME_SELINUX)) {
-		rc = cap_inode_setxattr(dentry, name, value, size, flags);
-		if (rc)
-			return rc;
-
 		/* Not an attribute we recognize, so just check the
 		   ordinary setattr permission. */
-		return dentry_has_perm(current_cred(), dentry, FILE__SETATTR);
+		rc = dentry_has_perm(current_cred(), dentry, FILE__SETATTR);
+		return rc ? rc : -ENOSYS;
 	}
 
 	sbsec = selinux_superblock(inode->i_sb);
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index d76aa0fc37a4..0e2f68e5b895 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -1281,7 +1281,7 @@ static int smack_inode_setxattr(struct dentry *dentry, const char *name,
 		    strncmp(value, TRANS_TRUE, TRANS_TRUE_SIZE) != 0)
 			rc = -EINVAL;
 	} else
-		rc = cap_inode_setxattr(dentry, name, value, size, flags);
+		rc = -ENOSYS;
 
 	if (check_priv && !smack_privileged(CAP_MAC_ADMIN))
 		rc = -EPERM;
@@ -1295,11 +1295,11 @@ static int smack_inode_setxattr(struct dentry *dentry, const char *name,
 			rc = -EINVAL;
 	}
 
-	smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_DENTRY);
-	smk_ad_setfield_u_fs_path_dentry(&ad, dentry);
-
 	if (rc == 0) {
-		rc = smk_curacc(smk_of_inode(d_backing_inode(dentry)), MAY_WRITE, &ad);
+		smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_DENTRY);
+		smk_ad_setfield_u_fs_path_dentry(&ad, dentry);
+		rc = smk_curacc(smk_of_inode(d_backing_inode(dentry)),
+				MAY_WRITE, &ad);
 		rc = smk_bu_inode(d_backing_inode(dentry), MAY_WRITE, rc);
 	}
 
-- 
2.19.1


^ permalink raw reply related

* [PATCH 73/90] Smack: Advertise the secid to netlabel
From: Casey Schaufler @ 2019-04-19  0:46 UTC (permalink / raw)
  To: casey.schaufler, jmorris, linux-security-module, selinux; +Cc: casey
In-Reply-To: <20190419004617.64627-1-casey@schaufler-ca.com>

Add the secid to the attributes shared with netlabel.

Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
---
 security/smack/smack_access.c | 8 ++++++--
 security/smack/smackfs.c      | 8 ++++++--
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/security/smack/smack_access.c b/security/smack/smack_access.c
index fe2ce3a65822..0764bb85daee 100644
--- a/security/smack/smack_access.c
+++ b/security/smack/smack_access.c
@@ -549,8 +549,12 @@ struct smack_known *smk_import_entry(const char *string, int len)
 	skp->smk_known = smack;
 	skp->smk_secid = smack_next_secid++;
 	skp->smk_netlabel.domain = skp->smk_known;
-	skp->smk_netlabel.flags =
-		NETLBL_SECATTR_DOMAIN | NETLBL_SECATTR_MLS_LVL;
+	lsm_export_init(&skp->smk_netlabel.attr.le);
+	skp->smk_netlabel.attr.le.flags = LSM_EXPORT_SMACK;
+	skp->smk_netlabel.attr.le.smack = skp->smk_secid;
+	skp->smk_netlabel.flags = NETLBL_SECATTR_DOMAIN |
+				  NETLBL_SECATTR_MLS_LVL |
+				  NETLBL_SECATTR_SECID;
 	/*
 	 * If direct labeling works use it.
 	 * Otherwise use mapped labeling.
diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c
index 28c567465f6c..abaa5325c32f 100644
--- a/security/smack/smackfs.c
+++ b/security/smack/smackfs.c
@@ -2953,8 +2953,12 @@ static struct vfsmount *smackfs_mount;
 static int __init smk_preset_netlabel(struct smack_known *skp)
 {
 	skp->smk_netlabel.domain = skp->smk_known;
-	skp->smk_netlabel.flags =
-		NETLBL_SECATTR_DOMAIN | NETLBL_SECATTR_MLS_LVL;
+	lsm_export_init(&skp->smk_netlabel.attr.le);
+	skp->smk_netlabel.attr.le.flags = LSM_EXPORT_SMACK;
+	skp->smk_netlabel.attr.le.smack = skp->smk_secid;
+	skp->smk_netlabel.flags = NETLBL_SECATTR_DOMAIN |
+				  NETLBL_SECATTR_MLS_LVL |
+				  NETLBL_SECATTR_SECID;
 	return smk_netlbl_mls(smack_cipso_direct, skp->smk_known,
 				&skp->smk_netlabel, strlen(skp->smk_known));
 }
-- 
2.19.1


^ permalink raw reply related

* [PATCH 72/90] LSM: Fix for security_init_inode_security
From: Casey Schaufler @ 2019-04-19  0:45 UTC (permalink / raw)
  To: casey.schaufler, jmorris, linux-security-module, selinux; +Cc: casey
In-Reply-To: <20190419004617.64627-1-casey@schaufler-ca.com>

The code assumes you can call evm_init_inode_security more
than once for an inode, but that won't work because security.evm
is a single value attribute. This does not make EVM work properly,
but does allow the security modules to initialize their attributes.

Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
---
 security/security.c | 35 +++++++++++++++++++++++++----------
 1 file changed, 25 insertions(+), 10 deletions(-)

diff --git a/security/security.c b/security/security.c
index 63b001e60b59..1a54e7b1196e 100644
--- a/security/security.c
+++ b/security/security.c
@@ -1102,11 +1102,24 @@ int security_inode_init_security(struct inode *inode, struct inode *dir,
 	if (unlikely(IS_PRIVATE(inode)))
 		return 0;
 
-	if (!initxattrs)
-		return call_int_hook(inode_init_security, -EOPNOTSUPP, inode,
-				     dir, qstr, NULL, NULL, NULL);
+	if (!initxattrs) {
+		rc = -EOPNOTSUPP;
+		hlist_for_each_entry(p,
+				     &security_hook_heads.inode_init_security,
+				     list) {
+			rc = p->hook.inode_init_security(inode, dir, qstr,
+							 NULL, NULL, NULL);
+			if (rc == -EOPNOTSUPP) {
+				rc = 0;
+				continue;
+			}
+			if (rc)
+				break;
+		}
+		return rc;
+	}
 
-	repo = kzalloc((LSM_COUNT * 2) * sizeof(*repo), GFP_NOFS);
+	repo = kzalloc((LSM_COUNT + 1) * sizeof(*repo), GFP_NOFS);
 	if (repo == NULL)
 		return -ENOMEM;
 
@@ -1117,18 +1130,20 @@ int security_inode_init_security(struct inode *inode, struct inode *dir,
 		rc = p->hook.inode_init_security(inode, dir, qstr,
 						 &repo[i].name, &repo[i].value,
 						 &repo[i].value_len);
+		if (rc == -EOPNOTSUPP)
+			continue;
 		if (rc)
 			goto out;
 
-		rc = evm_inode_init_security(inode, &repo[i], &repo[i + 1]);
-		if (rc)
-			goto out;
-
-		i += 2;
+		i++;
 	}
+	rc = evm_inode_init_security(inode, &repo[i], &repo[i + 1]);
+	if (rc)
+		goto out;
+
 	rc = initxattrs(inode, repo, fs_data);
 out:
-	for (i-- ; i >= 0; i--)
+	for (i++ ; i >= 0; i--)
 		kfree(repo[i].value);
 	kfree(repo);
 	return (rc == -EOPNOTSUPP) ? 0 : rc;
-- 
2.19.1


^ permalink raw reply related

* [PATCH 71/90] LSM: Infrastructure security blobs for mount options
From: Casey Schaufler @ 2019-04-19  0:45 UTC (permalink / raw)
  To: casey.schaufler, jmorris, linux-security-module, selinux; +Cc: casey
In-Reply-To: <20190419004617.64627-1-casey@schaufler-ca.com>

Manage LSM data for mount options in the infrastructure
rather than in the individual modules.

Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
---
 include/linux/lsm_hooks.h  |  5 +++++
 security/security.c        | 18 ++++++++++++++++++
 security/selinux/hooks.c   | 31 ++++++++++++++++++-------------
 security/smack/smack_lsm.c | 19 +++++++++++++------
 4 files changed, 54 insertions(+), 19 deletions(-)

diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 5135b8d1d759..34f98cfe2ffd 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -2066,6 +2066,7 @@ struct lsm_blob_sizes {
 	int	lbs_key;
 	int	lbs_msg_msg;
 	int	lbs_task;
+	int	lbs_mnt_opts;
 };
 
 /*
@@ -2139,4 +2140,8 @@ static inline void security_delete_hooks(struct security_hook_list *hooks,
 
 extern int lsm_inode_alloc(struct inode *inode);
 
+#ifdef CONFIG_SECURITY
+void *lsm_mnt_opts_alloc(void);
+#endif
+
 #endif /* ! __LINUX_LSM_HOOKS_H */
diff --git a/security/security.c b/security/security.c
index f1e2ffe81829..63b001e60b59 100644
--- a/security/security.c
+++ b/security/security.c
@@ -177,6 +177,7 @@ static void __init lsm_set_blob_sizes(struct lsm_blob_sizes *needed)
 #ifdef CONFIG_KEYS
 	lsm_set_blob_size(&needed->lbs_key, &blob_sizes.lbs_key);
 #endif
+	lsm_set_blob_size(&needed->lbs_mnt_opts, &blob_sizes.lbs_mnt_opts);
 	lsm_set_blob_size(&needed->lbs_msg_msg, &blob_sizes.lbs_msg_msg);
 	lsm_set_blob_size(&needed->lbs_sock, &blob_sizes.lbs_sock);
 	lsm_set_blob_size(&needed->lbs_superblock, &blob_sizes.lbs_superblock);
@@ -315,6 +316,7 @@ static void __init ordered_lsm_init(void)
 #ifdef CONFIG_KEYS
 	init_debug("key blob size        = %d\n", blob_sizes.lbs_key);
 #endif /* CONFIG_KEYS */
+	init_debug("mnt_opts blob size   = %d\n", blob_sizes.lbs_mnt_opts);
 	init_debug("msg_msg blob size    = %d\n", blob_sizes.lbs_msg_msg);
 	init_debug("sock blob size       = %d\n", blob_sizes.lbs_sock);
 	init_debug("superblock blob size = %d\n", blob_sizes.lbs_superblock);
@@ -726,6 +728,21 @@ int lsm_superblock_alloc(struct super_block *sb)
 	return 0;
 }
 
+/**
+ * lsm_mnt_opts_alloc - allocate a composite mnt_opts blob
+ *
+ * Allocate the mount options blob
+ *
+ * Returns the blob, or NULL if memory can't be allocated.
+ */
+void *lsm_mnt_opts_alloc(void)
+{
+	if (blob_sizes.lbs_mnt_opts == 0)
+		return NULL;
+
+	return kzalloc(blob_sizes.lbs_mnt_opts, GFP_KERNEL);
+}
+
 /*
  * Hook list operation macros.
  *
@@ -939,6 +956,7 @@ void security_free_mnt_opts(void **mnt_opts)
 	if (!*mnt_opts)
 		return;
 	call_void_hook(sb_free_mnt_opts, *mnt_opts);
+	kfree(*mnt_opts);
 	*mnt_opts = NULL;
 }
 EXPORT_SYMBOL(security_free_mnt_opts);
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 17ba47f9f4e2..86578f7de131 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -383,14 +383,20 @@ struct selinux_mnt_opts {
 	const char *fscontext, *context, *rootcontext, *defcontext;
 };
 
+static void *selinux_mnt_opts(void *mnt_opts)
+{
+	if (mnt_opts)
+		return mnt_opts + selinux_blob_sizes.lbs_mnt_opts;
+	return NULL;
+}
+
 static void selinux_free_mnt_opts(void *mnt_opts)
 {
-	struct selinux_mnt_opts *opts = mnt_opts;
+	struct selinux_mnt_opts *opts = selinux_mnt_opts(mnt_opts);
 	kfree(opts->fscontext);
 	kfree(opts->context);
 	kfree(opts->rootcontext);
 	kfree(opts->defcontext);
-	kfree(opts);
 }
 
 static inline int inode_doinit(struct inode *inode)
@@ -638,7 +644,7 @@ static int selinux_set_mnt_opts(struct super_block *sb,
 	const struct cred *cred = current_cred();
 	struct superblock_security_struct *sbsec = selinux_superblock(sb);
 	struct dentry *root = sbsec->sb->s_root;
-	struct selinux_mnt_opts *opts = mnt_opts;
+	struct selinux_mnt_opts *opts = selinux_mnt_opts(mnt_opts);
 	struct inode_security_struct *root_isec;
 	u32 fscontext_sid = 0, context_sid = 0, rootcontext_sid = 0;
 	u32 defcontext_sid = 0;
@@ -653,7 +659,8 @@ static int selinux_set_mnt_opts(struct super_block *sb,
 			   server is ready to handle calls. */
 			goto out;
 		}
-		rc = -EINVAL;
+		/* Don't set any SELinux options. Allow any other LSM
+		   that's on the stack to do so. */
 		pr_warn("SELinux: Unable to set superblock options "
 			"before the security server is initialized\n");
 		goto out;
@@ -980,16 +987,17 @@ static int selinux_sb_clone_mnt_opts(const struct super_block *oldsb,
 
 static int selinux_add_opt(int token, const char *s, void **mnt_opts)
 {
-	struct selinux_mnt_opts *opts = *mnt_opts;
+	struct selinux_mnt_opts *opts = selinux_mnt_opts(*mnt_opts);
 
 	if (token == Opt_seclabel)	/* eaten and completely ignored */
 		return 0;
 
 	if (!opts) {
-		opts = kzalloc(sizeof(struct selinux_mnt_opts), GFP_KERNEL);
+		opts = lsm_mnt_opts_alloc();
 		if (!opts)
 			return -ENOMEM;
 		*mnt_opts = opts;
+		opts = selinux_mnt_opts(opts);
 	}
 	if (!s)
 		return -ENOMEM;
@@ -1042,10 +1050,8 @@ static int selinux_add_mnt_opt(const char *option, const char *val, int len,
 	rc = selinux_add_opt(token, val, mnt_opts);
 	if (unlikely(rc)) {
 		kfree(val);
-		if (*mnt_opts) {
+		if (*mnt_opts)
 			selinux_free_mnt_opts(*mnt_opts);
-			*mnt_opts = NULL;
-		}
 	}
 	return rc;
 }
@@ -2611,10 +2617,8 @@ static int selinux_sb_eat_lsm_opts(char *options, void **mnt_opts)
 			rc = selinux_add_opt(token, arg, mnt_opts);
 			if (unlikely(rc)) {
 				kfree(arg);
-				if (*mnt_opts) {
+				if (*mnt_opts)
 					selinux_free_mnt_opts(*mnt_opts);
-					*mnt_opts = NULL;
-				}
 				return rc;
 			}
 		} else {
@@ -2637,7 +2641,7 @@ static int selinux_sb_eat_lsm_opts(char *options, void **mnt_opts)
 
 static int selinux_sb_remount(struct super_block *sb, void *mnt_opts)
 {
-	struct selinux_mnt_opts *opts = mnt_opts;
+	struct selinux_mnt_opts *opts = selinux_mnt_opts(mnt_opts);
 	struct superblock_security_struct *sbsec = selinux_superblock(sb);
 	u32 sid;
 	int rc;
@@ -6641,6 +6645,7 @@ struct lsm_blob_sizes selinux_blob_sizes __lsm_ro_after_init = {
 #ifdef CONFIG_KEYS
 	.lbs_key = sizeof(struct key_security_struct),
 #endif /* CONFIG_KEYS */
+	.lbs_mnt_opts = sizeof(struct selinux_mnt_opts),
 	.lbs_msg_msg = sizeof(struct msg_security_struct),
 	.lbs_sock = sizeof(struct sk_security_struct),
 	.lbs_superblock = sizeof(struct superblock_security_struct),
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 0e2f68e5b895..3fd46cd2c4b1 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -574,26 +574,33 @@ struct smack_mnt_opts {
 	const char *fsdefault, *fsfloor, *fshat, *fsroot, *fstransmute;
 };
 
+static void *smack_mnt_opts(void *opts)
+{
+	if (opts)
+		return opts + smack_blob_sizes.lbs_mnt_opts;
+	return NULL;
+}
+
 static void smack_free_mnt_opts(void *mnt_opts)
 {
-	struct smack_mnt_opts *opts = mnt_opts;
+	struct smack_mnt_opts *opts = smack_mnt_opts(mnt_opts);
 	kfree(opts->fsdefault);
 	kfree(opts->fsfloor);
 	kfree(opts->fshat);
 	kfree(opts->fsroot);
 	kfree(opts->fstransmute);
-	kfree(opts);
 }
 
 static int smack_add_opt(int token, const char *s, void **mnt_opts)
 {
-	struct smack_mnt_opts *opts = *mnt_opts;
+	struct smack_mnt_opts *opts = smack_mnt_opts(*mnt_opts);
 
 	if (!opts) {
-		opts = kzalloc(sizeof(struct smack_mnt_opts), GFP_KERNEL);
+		opts = lsm_mnt_opts_alloc();
 		if (!opts)
 			return -ENOMEM;
 		*mnt_opts = opts;
+		opts = smack_mnt_opts(opts);
 	}
 	if (!s)
 		return -ENOMEM;
@@ -741,7 +748,6 @@ static int smack_sb_eat_lsm_opts(char *options, void **mnt_opts)
 				kfree(arg);
 				if (*mnt_opts)
 					smack_free_mnt_opts(*mnt_opts);
-				*mnt_opts = NULL;
 				return rc;
 			}
 		} else {
@@ -784,7 +790,7 @@ static int smack_set_mnt_opts(struct super_block *sb,
 	struct superblock_smack *sp = smack_superblock(sb);
 	struct inode_smack *isp;
 	struct smack_known *skp;
-	struct smack_mnt_opts *opts = mnt_opts;
+	struct smack_mnt_opts *opts = smack_mnt_opts(mnt_opts);
 	bool transmute = false;
 
 	if (sp->smk_flags & SMK_SB_INITIALIZED)
@@ -4586,6 +4592,7 @@ struct lsm_blob_sizes smack_blob_sizes __lsm_ro_after_init = {
 #ifdef CONFIG_KEYS
 	.lbs_key = sizeof(struct smack_known *),
 #endif /* CONFIG_KEYS */
+	.lbs_mnt_opts = sizeof(struct smack_mnt_opts),
 	.lbs_msg_msg = sizeof(struct smack_known *),
 	.lbs_sock = sizeof(struct socket_smack),
 	.lbs_superblock = sizeof(struct superblock_smack),
-- 
2.19.1


^ permalink raw reply related

* [PATCH 74/90] LSM: Change error detection for UDP peer security
From: Casey Schaufler @ 2019-04-19  0:46 UTC (permalink / raw)
  To: casey.schaufler, jmorris, linux-security-module, selinux; +Cc: casey
In-Reply-To: <20190419004617.64627-1-casey@schaufler-ca.com>

security_socket_getpeercred_dgram() supplies secids for use
by security_secid_to_secctx(). Sometimes a secid will be invalid.
Move the check for an invalid secid from the LSM specific
socket_getpeercred_dgram hooks into the secid_to_secctx hooks.
This allows for the case where one LSM (Smack) will provide a
secid and another (SELinux) to have an error for the same call.
Regardless of which LSM the caller wants to see the peer security
attributes for the correct result will be provided.

As there is no longer any reason for security_secid_to_secctx()
to return a value make all the secid_to_secctx functions void
instead of int. Add checking for a invalid secid to the Smack
and SELinux secid_to_secctx hooks.

Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
---
 include/linux/lsm_hooks.h  |  3 +--
 include/linux/security.h   | 11 +++++------
 net/ipv4/ip_sockglue.c     |  4 +---
 security/security.c        |  7 +++----
 security/selinux/hooks.c   | 13 +++++++------
 security/smack/smack_lsm.c | 17 ++++++++---------
 6 files changed, 25 insertions(+), 30 deletions(-)

diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 34f98cfe2ffd..0bb064c8b2dd 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -883,7 +883,6 @@
  *	@sock is the socket
  *	@skb is the skbuff for the packet being queried
  *	@l is a pointer to a buffer in which to copy the security data
- *	Return 0 on success, error on failure.
  * @sk_alloc_security:
  *	Allocate and attach a security structure to the sk->sk_security field,
  *	which is used to copy security attributes between local stream sockets.
@@ -1699,7 +1698,7 @@ union security_list_options {
 	int (*socket_getpeersec_stream)(struct socket *sock,
 					char __user *optval,
 					int __user *optlen, unsigned len);
-	int (*socket_getpeersec_dgram)(struct socket *sock,
+	void (*socket_getpeersec_dgram)(struct socket *sock,
 					struct sk_buff *skb,
 					struct lsm_export *l);
 	int (*sk_alloc_security)(struct sock *sk, int family, gfp_t priority);
diff --git a/include/linux/security.h b/include/linux/security.h
index 8eb849d71e9d..99f9824ec230 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -1288,8 +1288,8 @@ int security_socket_shutdown(struct socket *sock, int how);
 int security_sock_rcv_skb(struct sock *sk, struct sk_buff *skb);
 int security_socket_getpeersec_stream(struct socket *sock, char __user *optval,
 				      int __user *optlen, unsigned len);
-int security_socket_getpeersec_dgram(struct socket *sock, struct sk_buff *skb,
-				     struct lsm_export *l);
+void security_socket_getpeersec_dgram(struct socket *sock, struct sk_buff *skb,
+				      struct lsm_export *l);
 int security_sk_alloc(struct sock *sk, int family, gfp_t priority);
 void security_sk_free(struct sock *sk);
 void security_sk_clone(const struct sock *sk, struct sock *newsk);
@@ -1427,11 +1427,10 @@ static inline int security_socket_getpeersec_stream(struct socket *sock, char __
 	return -ENOPROTOOPT;
 }
 
-static inline int security_socket_getpeersec_dgram(struct socket *sock,
-						   struct sk_buff *skb,
-						   struct lsm_export *l)
+static inline void security_socket_getpeersec_dgram(struct socket *sock,
+						    struct sk_buff *skb,
+						    struct lsm_export *l)
 {
-	return -ENOPROTOOPT;
 }
 
 static inline int security_sk_alloc(struct sock *sk, int family, gfp_t priority)
diff --git a/net/ipv4/ip_sockglue.c b/net/ipv4/ip_sockglue.c
index 56035b53952d..ae69718d87ae 100644
--- a/net/ipv4/ip_sockglue.c
+++ b/net/ipv4/ip_sockglue.c
@@ -134,9 +134,7 @@ static void ip_cmsg_recv_security(struct msghdr *msg, struct sk_buff *skb)
 	struct lsm_context lc;
 	int err;
 
-	err = security_socket_getpeersec_dgram(NULL, skb, &le);
-	if (err)
-		return;
+	security_socket_getpeersec_dgram(NULL, skb, &le);
 
 	err = security_secid_to_secctx(&le, &lc);
 	if (err)
diff --git a/security/security.c b/security/security.c
index 1a54e7b1196e..0bbe0dfd3cfc 100644
--- a/security/security.c
+++ b/security/security.c
@@ -2402,12 +2402,11 @@ int security_socket_getpeersec_stream(struct socket *sock, char __user *optval,
 				optval, optlen, len);
 }
 
-int security_socket_getpeersec_dgram(struct socket *sock, struct sk_buff *skb,
-				     struct lsm_export *l)
+void security_socket_getpeersec_dgram(struct socket *sock, struct sk_buff *skb,
+				      struct lsm_export *l)
 {
 	lsm_export_init(l);
-	return call_int_hook(socket_getpeersec_dgram, -ENOPROTOOPT, sock, skb,
-			     l);
+	call_void_hook(socket_getpeersec_dgram, sock, skb, l);
 }
 EXPORT_SYMBOL(security_socket_getpeersec_dgram);
 
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 86578f7de131..93c3982d940c 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -4939,9 +4939,9 @@ static int selinux_socket_getpeersec_stream(struct socket *sock,
 	return err;
 }
 
-static int selinux_socket_getpeersec_dgram(struct socket *sock,
-					   struct sk_buff *skb,
-					   struct lsm_export *l)
+static void selinux_socket_getpeersec_dgram(struct socket *sock,
+					    struct sk_buff *skb,
+					    struct lsm_export *l)
 {
 	u32 peer_secid = SECSID_NULL;
 	u16 family;
@@ -4964,9 +4964,7 @@ static int selinux_socket_getpeersec_dgram(struct socket *sock,
 
 out:
 	selinux_export_secid(l, peer_secid);
-	if (peer_secid == SECSID_NULL)
-		return -EINVAL;
-	return 0;
+	return;
 }
 
 static int selinux_sk_alloc_security(struct sock *sk, int family, gfp_t priority)
@@ -6313,6 +6311,9 @@ static int selinux_secid_to_secctx(struct lsm_export *l, struct lsm_context *cp)
 	u32 secid;
 
 	selinux_import_secid(l, &secid);
+	if (secid == SECSID_NULL)
+		return -EINVAL;
+
 	cp->release = selinux_release_secctx;
 	if (l->flags & LSM_EXPORT_LENGTH)
 		return security_sid_to_context(&selinux_state, secid,
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 3fd46cd2c4b1..e18245a52e80 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -3988,9 +3988,9 @@ static int smack_socket_getpeersec_stream(struct socket *sock,
  *
  * Sets the netlabel socket state on sk from parent
  */
-static int smack_socket_getpeersec_dgram(struct socket *sock,
-					 struct sk_buff *skb,
-					 struct lsm_export *l)
+static void smack_socket_getpeersec_dgram(struct socket *sock,
+					  struct sk_buff *skb,
+					  struct lsm_export *l)
 
 {
 	struct netlbl_lsm_secattr secattr;
@@ -3998,7 +3998,6 @@ static int smack_socket_getpeersec_dgram(struct socket *sock,
 	struct smack_known *skp;
 	int family = PF_UNSPEC;
 	u32 s = 0;	/* 0 is the invalid secid */
-	int rc;
 
 	if (skb != NULL) {
 		if (skb->protocol == htons(ETH_P_IP))
@@ -4028,8 +4027,7 @@ static int smack_socket_getpeersec_dgram(struct socket *sock,
 		if (sock != NULL && sock->sk != NULL)
 			ssp = smack_sock(sock->sk);
 		netlbl_secattr_init(&secattr);
-		rc = netlbl_skbuff_getattr(skb, family, &secattr);
-		if (rc == 0) {
+		if (netlbl_skbuff_getattr(skb, family, &secattr) == 0) {
 			skp = smack_from_secattr(&secattr, ssp);
 			s = skp->smk_secid;
 		}
@@ -4044,9 +4042,7 @@ static int smack_socket_getpeersec_dgram(struct socket *sock,
 		break;
 	}
 	smack_export_secid(l, s);
-	if (s == 0)
-		return -EINVAL;
-	return 0;
+	return;
 }
 
 /**
@@ -4458,6 +4454,9 @@ static int smack_secid_to_secctx(struct lsm_export *l, struct lsm_context *cp)
 	u32 secid;
 
 	smack_import_secid(l, &secid);
+	if (secid == 0)
+		return -EINVAL;
+
 	skp = smack_from_secid(secid);
 
 	cp->context = (l->flags & LSM_EXPORT_LENGTH) ? NULL : skp->smk_known;
-- 
2.19.1


^ permalink raw reply related

* [PATCH 77/90] Netlabel: Add a secattr comparison API function
From: Casey Schaufler @ 2019-04-19  0:46 UTC (permalink / raw)
  To: casey.schaufler, jmorris, linux-security-module, selinux; +Cc: casey
In-Reply-To: <20190419004617.64627-1-casey@schaufler-ca.com>

Add a new API function netlbl_secattr_equal() that
determines if two secattr structures would result in the
same on-wire representation.

Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
---
 include/net/netlabel.h       |  8 ++++++
 net/netlabel/netlabel_kapi.c | 50 ++++++++++++++++++++++++++++++++++++
 2 files changed, 58 insertions(+)

diff --git a/include/net/netlabel.h b/include/net/netlabel.h
index 546c75f27d05..00000d53cdcd 100644
--- a/include/net/netlabel.h
+++ b/include/net/netlabel.h
@@ -472,6 +472,8 @@ int netlbl_catmap_setlong(struct netlbl_lsm_catmap **catmap,
 			  u32 offset,
 			  unsigned long bitmap,
 			  gfp_t flags);
+bool netlbl_secattr_equal(const struct netlbl_lsm_secattr *secattr_a,
+			  const struct netlbl_lsm_secattr *secattr_b);
 
 /* Bitmap functions
  */
@@ -623,6 +625,12 @@ static inline int netlbl_catmap_setlong(struct netlbl_lsm_catmap **catmap,
 {
 	return 0;
 }
+static inline bool netlbl_secattr_equal(
+				const struct netlbl_lsm_secattr *secattr_a,
+				const struct netlbl_lsm_secattr *secattr_b)
+{
+	return true;
+}
 static inline int netlbl_enabled(void)
 {
 	return 0;
diff --git a/net/netlabel/netlabel_kapi.c b/net/netlabel/netlabel_kapi.c
index 849064422e0b..648103ecc48b 100644
--- a/net/netlabel/netlabel_kapi.c
+++ b/net/netlabel/netlabel_kapi.c
@@ -1461,6 +1461,56 @@ int netlbl_cache_add(const struct sk_buff *skb, u16 family,
 	return -ENOMSG;
 }
 
+/**
+ * netlbl_secattr_equal - Compare two lsm secattrs
+ * @secattr_a: one security attribute
+ * @secattr_b: the other security attribute
+ *
+ * Description:
+ * Compare two lsm security attribute structures.
+ * Don't compare secid fields, as those are distinct.
+ * Returns true if they are the same, false otherwise.
+ *
+ */
+bool netlbl_secattr_equal(const struct netlbl_lsm_secattr *secattr_a,
+			  const struct netlbl_lsm_secattr *secattr_b)
+{
+	struct netlbl_lsm_catmap *iter_a;
+	struct netlbl_lsm_catmap *iter_b;
+
+	if (secattr_a == secattr_b)
+		return true;
+	if (!secattr_a || !secattr_b)
+		return false;
+
+	if ((secattr_a->flags & NETLBL_SECATTR_MLS_LVL) !=
+	    (secattr_b->flags & NETLBL_SECATTR_MLS_LVL))
+		return false;
+
+	if ((secattr_a->flags & NETLBL_SECATTR_MLS_LVL) &&
+	    secattr_a->attr.mls.lvl != secattr_b->attr.mls.lvl)
+		return false;
+
+	if ((secattr_a->flags & NETLBL_SECATTR_MLS_CAT) !=
+	    (secattr_b->flags & NETLBL_SECATTR_MLS_CAT))
+		return false;
+
+	iter_a = secattr_a->attr.mls.cat;
+	iter_b = secattr_b->attr.mls.cat;
+
+	while (iter_a && iter_b) {
+		if (iter_a->startbit != iter_b->startbit)
+			return false;
+		if (memcmp(iter_a->bitmap, iter_b->bitmap,
+			   sizeof(iter_a->bitmap)))
+			return false;
+		iter_a = iter_a->next;
+		iter_b = iter_b->next;
+	}
+
+	return !iter_a && !iter_b;
+}
+
 /*
  * Protocol Engine Functions
  */
-- 
2.19.1


^ permalink raw reply related

* [PATCH 75/90] Smack: Fix setting of the CIPSO MLS_CAT flags
From: Casey Schaufler @ 2019-04-19  0:46 UTC (permalink / raw)
  To: casey.schaufler, jmorris, linux-security-module, selinux; +Cc: casey
In-Reply-To: <20190419004617.64627-1-casey@schaufler-ca.com>

Don't tell CIPSO that a netlabel created by Smack has
categories set when it does not.

Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
---
 security/smack/smack_access.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/security/smack/smack_access.c b/security/smack/smack_access.c
index 0764bb85daee..5fe5c6799b27 100644
--- a/security/smack/smack_access.c
+++ b/security/smack/smack_access.c
@@ -494,8 +494,8 @@ int smk_netlbl_mls(int level, char *catset, struct netlbl_lsm_secattr *sap,
 	int cat;
 	int rc;
 	int byte;
+	bool has = false;
 
-	sap->flags |= NETLBL_SECATTR_MLS_CAT;
 	sap->attr.mls.lvl = level;
 	sap->attr.mls.cat = NULL;
 
@@ -503,6 +503,7 @@ int smk_netlbl_mls(int level, char *catset, struct netlbl_lsm_secattr *sap,
 		for (m = 0x80; m != 0; m >>= 1, cat++) {
 			if ((m & *cp) == 0)
 				continue;
+			has = true;
 			rc = netlbl_catmap_setbit(&sap->attr.mls.cat,
 						  cat, GFP_KERNEL);
 			if (rc < 0) {
@@ -511,6 +512,9 @@ int smk_netlbl_mls(int level, char *catset, struct netlbl_lsm_secattr *sap,
 			}
 		}
 
+	if (has)
+		sap->flags |= NETLBL_SECATTR_MLS_CAT;
+
 	return 0;
 }
 
-- 
2.19.1


^ permalink raw reply related

* [PATCH 76/90] Smack: Set netlabel flags properly on new label import
From: Casey Schaufler @ 2019-04-19  0:46 UTC (permalink / raw)
  To: casey.schaufler, jmorris, linux-security-module, selinux; +Cc: casey
In-Reply-To: <20190419004617.64627-1-casey@schaufler-ca.com>

Ensure that all netlabel flags are correctly set on the
netlabel attribute of a newly imported Smack label.

Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
---
 security/smack/smackfs.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c
index abaa5325c32f..0abfa4315fb1 100644
--- a/security/smack/smackfs.c
+++ b/security/smack/smackfs.c
@@ -931,6 +931,9 @@ static ssize_t smk_set_cipso(struct file *file, const char __user *buf,
 		smack_catset_bit(cat, mapcatset);
 	}
 
+	skp->smk_netlabel.flags = NETLBL_SECATTR_DOMAIN |
+				  NETLBL_SECATTR_MLS_LVL |
+				  NETLBL_SECATTR_SECID;
 	rc = smk_netlbl_mls(maplevel, mapcatset, &ncats, SMK_CIPSOLEN);
 	if (rc >= 0) {
 		netlbl_catmap_free(skp->smk_netlabel.attr.mls.cat);
-- 
2.19.1


^ permalink raw reply related

* [PATCH 78/90] Smack: Let netlabel do the work on the ambient domain
From: Casey Schaufler @ 2019-04-19  0:46 UTC (permalink / raw)
  To: casey.schaufler, jmorris, linux-security-module, selinux; +Cc: casey
In-Reply-To: <20190419004617.64627-1-casey@schaufler-ca.com>

Don't delete the netlabel data from sockets on the
ambient domain as netlabel will do it correctly without
any help.

Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
---
 security/smack/smack_lsm.c | 31 ++++++++-----------------------
 1 file changed, 8 insertions(+), 23 deletions(-)

diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index e18245a52e80..ace5b48f90dc 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -2403,37 +2403,27 @@ static struct smack_known *smack_ipv6host_label(struct sockaddr_in6 *sip)
 /**
  * smack_netlabel - Set the secattr on a socket
  * @sk: the socket
- * @labeled: socket label scheme
  *
  * Convert the outbound smack value (smk_out) to a
  * secattr and attach it to the socket.
  *
  * Returns 0 on success or an error code
  */
-static int smack_netlabel(struct sock *sk, int labeled)
+static int smack_netlabel(struct sock *sk)
 {
 	struct smack_known *skp;
 	struct socket_smack *ssp = smack_sock(sk);
 	int rc = 0;
 
 	/*
-	 * Usually the netlabel code will handle changing the
+	 * The netlabel code will handle changing the
 	 * packet labeling based on the label.
-	 * The case of a single label host is different, because
-	 * a single label host should never get a labeled packet
-	 * even though the label is usually associated with a packet
-	 * label.
 	 */
 	local_bh_disable();
 	bh_lock_sock_nested(sk);
 
-	if (ssp->smk_out == smack_net_ambient ||
-	    labeled == SMACK_UNLABELED_SOCKET)
-		netlbl_sock_delattr(sk);
-	else {
-		skp = ssp->smk_out;
-		rc = netlbl_sock_setattr(sk, sk->sk_family, &skp->smk_netlabel);
-	}
+	skp = ssp->smk_out;
+	rc = netlbl_sock_setattr(sk, sk->sk_family, &skp->smk_netlabel);
 
 	bh_unlock_sock(sk);
 	local_bh_enable();
@@ -2455,8 +2445,7 @@ static int smack_netlabel(struct sock *sk, int labeled)
 static int smack_netlabel_send(struct sock *sk, struct sockaddr_in *sap)
 {
 	struct smack_known *skp;
-	int rc;
-	int sk_lbl;
+	int rc = 0;
 	struct smack_known *hkp;
 	struct socket_smack *ssp = smack_sock(sk);
 	struct smk_audit_info ad;
@@ -2472,19 +2461,15 @@ static int smack_netlabel_send(struct sock *sk, struct sockaddr_in *sap)
 		ad.a.u.net->dport = sap->sin_port;
 		ad.a.u.net->v4info.daddr = sap->sin_addr.s_addr;
 #endif
-		sk_lbl = SMACK_UNLABELED_SOCKET;
 		skp = ssp->smk_out;
 		rc = smk_access(skp, hkp, MAY_WRITE, &ad);
 		rc = smk_bu_note("IPv4 host check", skp, hkp, MAY_WRITE, rc);
-	} else {
-		sk_lbl = SMACK_CIPSO_SOCKET;
-		rc = 0;
 	}
 	rcu_read_unlock();
 	if (rc != 0)
 		return rc;
 
-	return smack_netlabel(sk, sk_lbl);
+	return smack_netlabel(sk);
 }
 
 #if IS_ENABLED(CONFIG_IPV6)
@@ -2722,7 +2707,7 @@ static int smack_inode_setsecurity(struct inode *inode, const char *name,
 	else if (strcmp(name, XATTR_SMACK_IPOUT) == 0) {
 		ssp->smk_out = skp;
 		if (sock->sk->sk_family == PF_INET) {
-			rc = smack_netlabel(sock->sk, SMACK_CIPSO_SOCKET);
+			rc = smack_netlabel(sock->sk);
 			if (rc != 0)
 				printk(KERN_WARNING
 					"Smack: \"%s\" netlbl error %d.\n",
@@ -2773,7 +2758,7 @@ static int smack_socket_post_create(struct socket *sock, int family,
 	/*
 	 * Set the outbound netlbl.
 	 */
-	return smack_netlabel(sock->sk, SMACK_CIPSO_SOCKET);
+	return smack_netlabel(sock->sk);
 }
 
 /**
-- 
2.19.1


^ permalink raw reply related

* [PATCH 79/90] Smack: Don't set the socket label on each send
From: Casey Schaufler @ 2019-04-19  0:46 UTC (permalink / raw)
  To: casey.schaufler, jmorris, linux-security-module, selinux; +Cc: casey
In-Reply-To: <20190419004617.64627-1-casey@schaufler-ca.com>

The socket does not need to be relabeled on each send.
Remove the code that does that.

Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
---
 security/smack/smack_lsm.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index ace5b48f90dc..25b5160e343b 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -2466,10 +2466,7 @@ static int smack_netlabel_send(struct sock *sk, struct sockaddr_in *sap)
 		rc = smk_bu_note("IPv4 host check", skp, hkp, MAY_WRITE, rc);
 	}
 	rcu_read_unlock();
-	if (rc != 0)
-		return rc;
-
-	return smack_netlabel(sk);
+	return rc;
 }
 
 #if IS_ENABLED(CONFIG_IPV6)
-- 
2.19.1


^ permalink raw reply related

* [PATCH 80/90] Smack: Let netlabel do the work on connections
From: Casey Schaufler @ 2019-04-19  0:46 UTC (permalink / raw)
  To: casey.schaufler, jmorris, linux-security-module, selinux; +Cc: casey
In-Reply-To: <20190419004617.64627-1-casey@schaufler-ca.com>

Rather than removing the netlabel socket attribute
on connections set the ambient domain. This is more
in line with the way netlabel "should" be used.

Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
---
 security/smack/smack_lsm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 25b5160e343b..337a05c34931 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -4143,7 +4143,7 @@ static int smack_inet_conn_request(struct sock *sk, struct sk_buff *skb,
 	if (hskp == NULL)
 		rc = netlbl_req_setattr(req, &skp->smk_netlabel);
 	else
-		netlbl_req_delattr(req);
+		rc = netlbl_req_setattr(req, &smack_net_ambient->smk_netlabel);
 
 	return rc;
 }
-- 
2.19.1


^ permalink raw reply related

* [PATCH 81/90] Netlabel: Return the labeling type on socket
From: Casey Schaufler @ 2019-04-19  0:46 UTC (permalink / raw)
  To: casey.schaufler, jmorris, linux-security-module, selinux; +Cc: casey
In-Reply-To: <20190419004617.64627-1-casey@schaufler-ca.com>

Change netlbl_sock_setattr() to return the labeling
type of the domain. This allows the labeling types to
be compared when two LSMs want to determine how a socket
should be used.

Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
---
 net/netlabel/netlabel_kapi.c | 25 ++++++++++++-------------
 security/selinux/netlabel.c  | 11 ++++-------
 security/smack/smack_lsm.c   |  2 ++
 3 files changed, 18 insertions(+), 20 deletions(-)

diff --git a/net/netlabel/netlabel_kapi.c b/net/netlabel/netlabel_kapi.c
index 648103ecc48b..2f7ba0e2e436 100644
--- a/net/netlabel/netlabel_kapi.c
+++ b/net/netlabel/netlabel_kapi.c
@@ -974,15 +974,14 @@ int netlbl_enabled(void)
  * Attach the correct label to the given socket using the security attributes
  * specified in @secattr.  This function requires exclusive access to @sk,
  * which means it either needs to be in the process of being created or locked.
- * Returns zero on success, -EDESTADDRREQ if the domain is configured to use
- * network address selectors (can't blindly label the socket), and negative
- * values on all other failures.
+ * Returns the labeling type of the domain, or negative values on failures.
  *
  */
 int netlbl_sock_setattr(struct sock *sk,
 			u16 family,
 			const struct netlbl_lsm_secattr *secattr)
 {
+	int rc;
 	int ret_val;
 	struct netlbl_dom_map *dom_entry;
 
@@ -994,17 +993,17 @@ int netlbl_sock_setattr(struct sock *sk,
 	}
 	switch (family) {
 	case AF_INET:
+		ret_val = dom_entry->def.type;
 		switch (dom_entry->def.type) {
 		case NETLBL_NLTYPE_ADDRSELECT:
-			ret_val = -EDESTADDRREQ;
 			break;
 		case NETLBL_NLTYPE_CIPSOV4:
-			ret_val = cipso_v4_sock_setattr(sk,
-							dom_entry->def.cipso,
-							secattr);
+			rc = cipso_v4_sock_setattr(sk, dom_entry->def.cipso,
+						   secattr);
+			if (rc < 0)
+				ret_val = rc;
 			break;
 		case NETLBL_NLTYPE_UNLABELED:
-			ret_val = 0;
 			break;
 		default:
 			ret_val = -ENOENT;
@@ -1012,17 +1011,17 @@ int netlbl_sock_setattr(struct sock *sk,
 		break;
 #if IS_ENABLED(CONFIG_IPV6)
 	case AF_INET6:
+		ret_val = dom_entry->def.type;
 		switch (dom_entry->def.type) {
 		case NETLBL_NLTYPE_ADDRSELECT:
-			ret_val = -EDESTADDRREQ;
 			break;
 		case NETLBL_NLTYPE_CALIPSO:
-			ret_val = calipso_sock_setattr(sk,
-						       dom_entry->def.calipso,
-						       secattr);
+			rc = calipso_sock_setattr(sk, dom_entry->def.calipso,
+						  secattr);
+			if (rc < 0)
+				ret_val = rc;
 			break;
 		case NETLBL_NLTYPE_UNLABELED:
-			ret_val = 0;
 			break;
 		default:
 			ret_val = -ENOENT;
diff --git a/security/selinux/netlabel.c b/security/selinux/netlabel.c
index 4bbd50237a8a..85156a0cdfc3 100644
--- a/security/selinux/netlabel.c
+++ b/security/selinux/netlabel.c
@@ -418,15 +418,12 @@ int selinux_netlbl_socket_post_create(struct sock *sk, u16 family)
 	if (secattr == NULL)
 		return -ENOMEM;
 	rc = netlbl_sock_setattr(sk, family, secattr);
-	switch (rc) {
-	case 0:
-		sksec->nlbl_state = NLBL_LABELED;
-		break;
-	case -EDESTADDRREQ:
+	if (rc == NETLBL_NLTYPE_ADDRSELECT)
 		sksec->nlbl_state = NLBL_REQSKB;
+	else if (rc >= 0)
+		sksec->nlbl_state = NLBL_LABELED;
+	if (rc > 0)
 		rc = 0;
-		break;
-	}
 
 	return rc;
 }
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 337a05c34931..a787f8010067 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -2424,6 +2424,8 @@ static int smack_netlabel(struct sock *sk)
 
 	skp = ssp->smk_out;
 	rc = netlbl_sock_setattr(sk, sk->sk_family, &skp->smk_netlabel);
+	if (rc > 0)
+		rc = 0;
 
 	bh_unlock_sock(sk);
 	local_bh_enable();
-- 
2.19.1


^ permalink raw reply related

* [PATCH 49/90] LSM: Use lsm_context in security_secid_to_secctx
From: Casey Schaufler @ 2019-04-19  0:45 UTC (permalink / raw)
  To: casey.schaufler, jmorris, linux-security-module, selinux; +Cc: casey
In-Reply-To: <20190419004617.64627-1-casey@schaufler-ca.com>

Convert security_secid_to_secctx to use the lsm_context structure
instead of a context/secid pair. There is some scaffolding involved
that will be removed when the related data is updated.

Add a flag for lsm_export to indicate that the caller of
security_secid_to_secctx() is only interested in the length
of the context.

Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
---
 drivers/android/binder.c                |  2 +-
 include/linux/security.h                | 13 +++++++------
 include/net/scm.h                       |  2 +-
 kernel/audit.c                          |  5 ++---
 kernel/auditsc.c                        | 10 +++++-----
 net/ipv4/ip_sockglue.c                  |  2 +-
 net/netfilter/nf_conntrack_netlink.c    | 11 ++++++-----
 net/netfilter/nf_conntrack_standalone.c |  2 +-
 net/netfilter/nfnetlink_queue.c         |  2 +-
 net/netlabel/netlabel_unlabeled.c       | 12 ++++--------
 net/netlabel/netlabel_user.c            |  3 +--
 security/apparmor/secid.c               |  3 +--
 security/security.c                     | 13 ++-----------
 security/selinux/hooks.c                |  3 +++
 security/smack/smack_lsm.c              |  2 +-
 15 files changed, 37 insertions(+), 48 deletions(-)

diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index c2cfef13257c..58033c003cc2 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -3121,7 +3121,7 @@ static void binder_transaction(struct binder_proc *proc,
 		struct lsm_export le;
 
 		security_task_getsecid(proc->tsk, &le);
-		ret = security_secid_to_secctx(&le, &lc.context, &lc.len);
+		ret = security_secid_to_secctx(&le, &lc);
 		if (ret) {
 			return_error = BR_FAILED_REPLY;
 			return_error_param = ret;
diff --git a/include/linux/security.h b/include/linux/security.h
index 57ce9b824eef..9a9de2bafa55 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -83,10 +83,11 @@ struct lsm_export {
 	u32	apparmor;
 	u32	flags;
 };
-#define LSM_EXPORT_NONE		0x00
-#define LSM_EXPORT_SELINUX	0x01
-#define LSM_EXPORT_SMACK	0x02
-#define LSM_EXPORT_APPARMOR	0x04
+#define LSM_EXPORT_NONE		0x00000000
+#define LSM_EXPORT_SELINUX	0x00000001
+#define LSM_EXPORT_SMACK	0x00000002
+#define LSM_EXPORT_APPARMOR	0x00000004
+#define LSM_EXPORT_LENGTH	0x80000000	/* Only the length required */
 
 static inline void lsm_export_init(struct lsm_export *l)
 {
@@ -431,7 +432,7 @@ int security_setprocattr(const char *lsm, const char *name, void *value,
 			 size_t size);
 int security_netlink_send(struct sock *sk, struct sk_buff *skb);
 int security_ismaclabel(const char *name);
-int security_secid_to_secctx(struct lsm_export *l, char **secdata, u32 *seclen);
+int security_secid_to_secctx(struct lsm_export *l, struct lsm_context *cp);
 int security_secctx_to_secid(struct lsm_context *cp, struct lsm_export *l);
 void security_release_secctx(struct lsm_context *cp);
 
@@ -1211,7 +1212,7 @@ static inline int security_ismaclabel(const char *name)
 }
 
 static inline int security_secid_to_secctx(struct lsm_export *l,
-					   char **secdata, u32 *seclen)
+					   struct lsm_seccontext *cp)
 {
 	return -EOPNOTSUPP;
 }
diff --git a/include/net/scm.h b/include/net/scm.h
index 7e242ebdd258..b25ca3b6a514 100644
--- a/include/net/scm.h
+++ b/include/net/scm.h
@@ -96,7 +96,7 @@ static inline void scm_passec(struct socket *sock, struct msghdr *msg, struct sc
 	int err;
 
 	if (test_bit(SOCK_PASSSEC, &sock->flags)) {
-		err = security_secid_to_secctx(&scm->le, &lc.context, &lc.len);
+		err = security_secid_to_secctx(&scm->le, &lc);
 
 		if (!err) {
 			put_cmsg(msg, SOL_SOCKET, SCM_SECURITY,
diff --git a/kernel/audit.c b/kernel/audit.c
index 269c76fefe40..203e5b14bea4 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -1428,8 +1428,7 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
 	}
 	case AUDIT_SIGNAL_INFO:
 		if (lsm_export_any(&audit_sig_lsm)) {
-			err = security_secid_to_secctx(&audit_sig_lsm,
-						       &lc.context, &lc.len);
+			err = security_secid_to_secctx(&audit_sig_lsm, &lc);
 			if (err)
 				return err;
 		}
@@ -2076,7 +2075,7 @@ int audit_log_task_context(struct audit_buffer *ab)
 	if (!lsm_export_any(&le))
 		return 0;
 
-	error = security_secid_to_secctx(&le, &lc.context, &lc.len);
+	error = security_secid_to_secctx(&le, &lc);
 	if (error) {
 		if (error != -EINVAL)
 			goto error_path;
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 4dab81c7aca0..ceefd17467f9 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -938,7 +938,7 @@ static int audit_log_pid_context(struct audit_context *context, pid_t pid,
 				 unsigned int sessionid,
 				 struct lsm_export *l, char *comm)
 {
-	struct lsm_context lc = { .context = NULL, };
+	struct lsm_context lc;
 	struct audit_buffer *ab;
 	int rc = 0;
 
@@ -950,7 +950,7 @@ static int audit_log_pid_context(struct audit_context *context, pid_t pid,
 			 from_kuid(&init_user_ns, auid),
 			 from_kuid(&init_user_ns, uid), sessionid);
 	if (lsm_export_any(l)) {
-		if (security_secid_to_secctx(l, &lc.context, &lc.len)) {
+		if (security_secid_to_secctx(l, &lc)) {
 			audit_log_format(ab, " obj=(none)");
 			rc = 1;
 		} else {
@@ -1190,8 +1190,8 @@ static void show_special(struct audit_context *context, int *call_panic)
 				 from_kgid(&init_user_ns, context->ipc.gid),
 				 context->ipc.mode);
 		if (lsm_export_any(l)) {
-			struct lsm_context lc = { .context = NULL, };
-			if (security_secid_to_secctx(l, &lc.context, &lc.len)) {
+			struct lsm_context lc;
+			if (security_secid_to_secctx(l, &lc)) {
 				audit_log_format(ab, " osid=(unknown)");
 				*call_panic = 1;
 			} else {
@@ -1342,7 +1342,7 @@ static void audit_log_name(struct audit_context *context, struct audit_names *n,
 	if (lsm_export_any(&n->olsm)) {
 		struct lsm_context lc;
 
-		if (security_secid_to_secctx(&n->olsm, &lc.context, &lc.len)) {
+		if (security_secid_to_secctx(&n->olsm, &lc)) {
 			audit_log_format(ab, " osid=(unknown)");
 			if (call_panic)
 				*call_panic = 2;
diff --git a/net/ipv4/ip_sockglue.c b/net/ipv4/ip_sockglue.c
index 18a7fab8b2d3..56035b53952d 100644
--- a/net/ipv4/ip_sockglue.c
+++ b/net/ipv4/ip_sockglue.c
@@ -138,7 +138,7 @@ static void ip_cmsg_recv_security(struct msghdr *msg, struct sk_buff *skb)
 	if (err)
 		return;
 
-	err = security_secid_to_secctx(&le, &lc.context, &lc.len);
+	err = security_secid_to_secctx(&le, &lc);
 	if (err)
 		return;
 
diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
index 49bce1b085ce..ea83909af6db 100644
--- a/net/netfilter/nf_conntrack_netlink.c
+++ b/net/netfilter/nf_conntrack_netlink.c
@@ -337,7 +337,7 @@ static int ctnetlink_dump_secctx(struct sk_buff *skb, const struct nf_conn *ct)
 	le.selinux = ct->secmark;
 	le.smack = ct->secmark;
 
-	ret = security_secid_to_secctx(&le, &lc.context, &lc.len);
+	ret = security_secid_to_secctx(&le, &lc);
 	if (ret)
 		return 0;
 
@@ -620,20 +620,21 @@ static inline size_t ctnetlink_acct_size(const struct nf_conn *ct)
 static inline int ctnetlink_secctx_size(const struct nf_conn *ct)
 {
 #ifdef CONFIG_NF_CONNTRACK_SECMARK
-	int len, ret;
+	int ret;
 	struct lsm_export le;
+	struct lsm_context lc;
 
 	lsm_export_init(&le);
-	le.flags = LSM_EXPORT_SELINUX | LSM_EXPORT_SMACK;
+	le.flags = LSM_EXPORT_SELINUX | LSM_EXPORT_SMACK | LSM_EXPORT_LENGTH;
 	le.selinux = ct->secmark;
 	le.smack = ct->secmark;
 
-	ret = security_secid_to_secctx(&le, NULL, &len);
+	ret = security_secid_to_secctx(&le, &lc);
 	if (ret)
 		return 0;
 
 	return nla_total_size(0) /* CTA_SECCTX */
-	       + nla_total_size(sizeof(char) * len); /* CTA_SECCTX_NAME */
+	       + nla_total_size(sizeof(char) * lc.len); /* CTA_SECCTX_NAME */
 #else
 	return 0;
 #endif
diff --git a/net/netfilter/nf_conntrack_standalone.c b/net/netfilter/nf_conntrack_standalone.c
index 97d16a51504b..797abf443a34 100644
--- a/net/netfilter/nf_conntrack_standalone.c
+++ b/net/netfilter/nf_conntrack_standalone.c
@@ -182,7 +182,7 @@ static void ct_show_secctx(struct seq_file *s, const struct nf_conn *ct)
 	le.selinux = ct->secmark;
 	le.smack = ct->secmark;
 
-	ret = security_secid_to_secctx(&le, &lc.context, &lc.len);
+	ret = security_secid_to_secctx(&le, &lc);
 	if (ret)
 		return;
 
diff --git a/net/netfilter/nfnetlink_queue.c b/net/netfilter/nfnetlink_queue.c
index b70871693368..4a3d4b52caef 100644
--- a/net/netfilter/nfnetlink_queue.c
+++ b/net/netfilter/nfnetlink_queue.c
@@ -322,7 +322,7 @@ static u32 nfqnl_get_sk_secctx(struct sk_buff *skb, char **secdata)
 		le.flags = LSM_EXPORT_SELINUX | LSM_EXPORT_SMACK;
 		le.selinux = skb->secmark;
 		le.smack = skb->secmark;
-		security_secid_to_secctx(&le, &lc.context, &lc.len);
+		security_secid_to_secctx(&le, &lc);
 		*secdata = lc.context;
 	}
 
diff --git a/net/netlabel/netlabel_unlabeled.c b/net/netlabel/netlabel_unlabeled.c
index 4c4a8f6df261..336d315ee8eb 100644
--- a/net/netlabel/netlabel_unlabeled.c
+++ b/net/netlabel/netlabel_unlabeled.c
@@ -450,7 +450,7 @@ int netlbl_unlhsh_add(struct net *net,
 	rcu_read_unlock();
 	if (audit_buf != NULL) {
 		struct lsm_context lc;
-		if (security_secid_to_secctx(l, &lc.context, &lc.len) == 0) {
+		if (security_secid_to_secctx(l, &lc) == 0) {
 			audit_log_format(audit_buf, " sec_obj=%s", lc.context);
 			security_release_secctx(&lc);
 		}
@@ -504,8 +504,7 @@ static int netlbl_unlhsh_remove_addr4(struct net *net,
 		if (dev != NULL)
 			dev_put(dev);
 		if (entry != NULL &&
-		    security_secid_to_secctx(&entry->le,
-					     &lc.context, &lc.len) == 0) {
+		    security_secid_to_secctx(&entry->le, &lc) == 0) {
 			audit_log_format(audit_buf, " sec_obj=%s", lc.context);
 			security_release_secctx(&lc);
 		}
@@ -544,8 +543,6 @@ static int netlbl_unlhsh_remove_addr6(struct net *net,
 	struct netlbl_unlhsh_addr6 *entry;
 	struct audit_buffer *audit_buf;
 	struct net_device *dev;
-	char *secctx;
-	u32 secctx_len;
 
 	spin_lock(&netlbl_unlhsh_lock);
 	list_entry = netlbl_af6list_remove(addr, mask, &iface->addr6_list);
@@ -566,8 +563,7 @@ static int netlbl_unlhsh_remove_addr6(struct net *net,
 		if (dev != NULL)
 			dev_put(dev);
 		if (entry != NULL &&
-		    security_secid_to_secctx(&entry->le,
-					     &lc.context, &lc.len) == 0) {
+		    security_secid_to_secctx(&entry->le, &lc) == 0) {
 			audit_log_format(audit_buf, " sec_obj=%s", lc.context);
 			security_release_secctx(&lc);
 		}
@@ -1137,7 +1133,7 @@ static int netlbl_unlabel_staticlist_gen(u32 cmd,
 		lep = (struct lsm_export *)&addr6->le;
 	}
 
-	ret_val = security_secid_to_secctx(lep, &lc.context, &lc.len);
+	ret_val = security_secid_to_secctx(lep, &lc);
 	if (ret_val != 0)
 		goto list_cb_failure;
 	ret_val = nla_put(cb_arg->skb,
diff --git a/net/netlabel/netlabel_user.c b/net/netlabel/netlabel_user.c
index 0418f0935199..11ea98525c4e 100644
--- a/net/netlabel/netlabel_user.c
+++ b/net/netlabel/netlabel_user.c
@@ -112,8 +112,7 @@ struct audit_buffer *netlbl_audit_start_common(int type,
 			 audit_info->sessionid);
 
 	if (lsm_export_any(&audit_info->le) &&
-	    security_secid_to_secctx(&audit_info->le, &lc.context,
-				     &lc.len) == 0) {
+	    security_secid_to_secctx(&audit_info->le, &lc) == 0) {
 		audit_log_format(audit_buf, " subj=%s", lc.context);
 		security_release_secctx(&lc);
 	}
diff --git a/security/apparmor/secid.c b/security/apparmor/secid.c
index 46c8b9a67ac7..9dc17903a936 100644
--- a/security/apparmor/secid.c
+++ b/security/apparmor/secid.c
@@ -92,8 +92,7 @@ int apparmor_secid_to_secctx(struct lsm_export *l, struct lsm_context *cp)
 	if (!label)
 		return -EINVAL;
 
-	/* scaffolding check - Casey */
-	if (cp)
+	if (!(l->flags & LSM_EXPORT_LENGTH))
 		len = aa_label_asxprint(&cp->context, root_ns, label,
 					FLAG_SHOW_MODE | FLAG_VIEW_SUBNS |
 					FLAG_HIDDEN_UNCONFINED | FLAG_ABS_ROOT,
diff --git a/security/security.c b/security/security.c
index 3da7302d20ec..6588172b3ec8 100644
--- a/security/security.c
+++ b/security/security.c
@@ -1972,18 +1972,9 @@ int security_ismaclabel(const char *name)
 }
 EXPORT_SYMBOL(security_ismaclabel);
 
-int security_secid_to_secctx(struct lsm_export *l, char **secdata, u32 *seclen)
+int security_secid_to_secctx(struct lsm_export *l, struct lsm_context *cp)
 {
-	struct lsm_context lc = { .context = NULL, .len = 0, };
-	int rc;
-
-	rc = call_one_int_hook(secid_to_secctx, -EOPNOTSUPP, l, &lc);
-	if (secdata)
-		*secdata = lc.context;
-	else
-		security_release_secctx(&lc);
-	*seclen = lc.len;
-	return rc;
+	return call_one_int_hook(secid_to_secctx, -EOPNOTSUPP, l, cp);
 }
 EXPORT_SYMBOL(security_secid_to_secctx);
 
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 332296f69f76..7bf73493d10d 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -6306,6 +6306,9 @@ static int selinux_secid_to_secctx(struct lsm_export *l, struct lsm_context *cp)
 	u32 secid;
 
 	selinux_import_secid(l, &secid);
+	if (l->flags & LSM_EXPORT_LENGTH)
+		return security_sid_to_context(&selinux_state, secid,
+					       NULL, &cp->len);
 	return security_sid_to_context(&selinux_state, secid,
 				       &cp->context, &cp->len);
 }
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index cf27905ccaa5..1b5b3e421bff 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -4442,7 +4442,7 @@ static int smack_secid_to_secctx(struct lsm_export *l, struct lsm_context *cp)
 	smack_import_secid(l, &secid);
 	skp = smack_from_secid(secid);
 
-	cp->context = skp->smk_known;
+	cp->context = (l->flags & LSM_EXPORT_LENGTH) ? NULL : skp->smk_known;
 	cp->len = strlen(skp->smk_known);
 	return 0;
 }
-- 
2.19.1


^ permalink raw reply related

* Re: kernel BUG at kernel/cred.c:434!
From: Paul Moore @ 2019-04-19  2:04 UTC (permalink / raw)
  To: Yang Yingliang
  Cc: Casey Schaufler, Oleg Nesterov, john.johansen, chengjian (D),
	Kees Cook, NeilBrown, Anna Schumaker,
	linux-kernel@vger.kernel.org, Al Viro, Xiexiuqi (Xie XiuQi),
	Li Bin, Jason Yan, Peter Zijlstra, Ingo Molnar,
	Linux Security Module list, SELinux
In-Reply-To: <5CB7E5D4.2060703@huawei.com>

On Wed, Apr 17, 2019 at 10:50 PM Yang Yingliang
<yangyingliang@huawei.com> wrote:
> On 2019/4/18 8:24, Casey Schaufler wrote:
> > On 4/17/2019 4:39 PM, Paul Moore wrote:
> >>
> >> Since it looks like all three LSMs which implement the setprocattr
> >> hook are vulnerable I'm open to the idea that proc_pid_attr_write() is
> >> a better choice for the cred != read_cred check, but I would want to
> >> make sure John and Casey are okay with that.
> >>
> >> John?
> >>
> >> Casey?
> >
> > I'm fine with the change going into proc_pid_attr_write().
>
> The cred != real_cred checking is not enough.
>
> Consider this situation, when doing override, cred, real_cred and
> new_cred are all same:
>
> after override_creds()    cred == real_cred == new1_cred

I'm sorry, you've lost me.  After override_creds() returns
current->cred and current->real_cred are not going to be the same,
yes?

> after prepare_creds()     new2_cred
> after commit_creds()     becasue the check is false, so cred ==
> real_cred == new2_cred
> after revert_creds()        cred == new1_cred, real_cred == new2_cred
>
> It will cause cred != real_cred finally.

-- 
paul moore
www.paul-moore.com

^ permalink raw reply

* Re: kernel BUG at kernel/cred.c:434!
From: Yang Yingliang @ 2019-04-19  2:34 UTC (permalink / raw)
  To: Paul Moore
  Cc: Casey Schaufler, Oleg Nesterov, john.johansen, chengjian (D),
	Kees Cook, NeilBrown, Anna Schumaker,
	linux-kernel@vger.kernel.org, Al Viro, Xiexiuqi (Xie XiuQi),
	Li Bin, Jason Yan, Peter Zijlstra, Ingo Molnar,
	Linux Security Module list, SELinux
In-Reply-To: <CAHC9VhQkrR+m9_dYLg4VG-26TdUnf8XFjbmuVQ70YCOp6DhN_g@mail.gmail.com>



On 2019/4/19 10:04, Paul Moore wrote:
> On Wed, Apr 17, 2019 at 10:50 PM Yang Yingliang
> <yangyingliang@huawei.com> wrote:
>> On 2019/4/18 8:24, Casey Schaufler wrote:
>>> On 4/17/2019 4:39 PM, Paul Moore wrote:
>>>> Since it looks like all three LSMs which implement the setprocattr
>>>> hook are vulnerable I'm open to the idea that proc_pid_attr_write() is
>>>> a better choice for the cred != read_cred check, but I would want to
>>>> make sure John and Casey are okay with that.
>>>>
>>>> John?
>>>>
>>>> Casey?
>>> I'm fine with the change going into proc_pid_attr_write().
>> The cred != real_cred checking is not enough.
>>
>> Consider this situation, when doing override, cred, real_cred and
>> new_cred are all same:
>>
>> after override_creds()    cred == real_cred == new1_cred
> I'm sorry, you've lost me.  After override_creds() returns
> current->cred and current->real_cred are not going to be the same,
> yes?
It's possible the new  cred is equal to current->real_cred and 
current->cred,
so after overrides_creds(), they have the same value.
>
>> after prepare_creds()     new2_cred
>> after commit_creds()     becasue the check is false, so cred ==
>> real_cred == new2_cred
>> after revert_creds()        cred == new1_cred, real_cred == new2_cred
>>
>> It will cause cred != real_cred finally.



^ permalink raw reply

* Re: [PATCH v20 16/28] x86/sgx: Add provisioning
From: Huang, Kai @ 2019-04-19  3:06 UTC (permalink / raw)
  To: jarkko.sakkinen@linux.intel.com, linux-kernel@vger.kernel.org,
	linux-sgx@vger.kernel.org, x86@kernel.org
  Cc: Svahn, Kai, nhorman@redhat.com, jmorris@namei.org,
	Christopherson, Sean J, josh@joshtriplett.org, tglx@linutronix.de,
	Ayoun, Serge, Huang, Haitao,
	linux-security-module@vger.kernel.org, akpm@linux-foundation.org,
	npmccallum@redhat.com, rientjes@google.com, luto@kernel.org,
	Katz-zamir, Shay, Hansen, Dave, bp@alien8.de, serge@hallyn.com,
	andriy.shevchenko@linux.intel.com
In-Reply-To: <20190417103938.7762-17-jarkko.sakkinen@linux.intel.com>

On Wed, 2019-04-17 at 13:39 +0300, Jarkko Sakkinen wrote:
> In order to provide a mechanism for devilering provisoning rights:
> 
> 1. Add a new device file /dev/sgx/provision that works as a token for
>    allowing an enclave to have the provisioning privileges.
> 2. Add a new ioctl called SGX_IOC_ENCLAVE_SET_ATTRIBUTE that accepts the
>    following data structure:
> 
>    struct sgx_enclave_set_attribute {
>            __u64 addr;
>            __u64 attribute_fd;
>    };
> 
> A daemon could sit on top of /dev/sgx/provision and send a file
> descriptor of this file to a process that needs to be able to provision
> enclaves.
> 
> The way this API is used is straight-forward. Lets assume that dev_fd is
> a handle to /dev/sgx/enclave and prov_fd is a handle to
> /dev/sgx/provision.  You would allow SGX_IOC_ENCLAVE_CREATE to
> initialize an enclave with the PROVISIONKEY attribute by
> 
> params.addr = <enclave address>;
> params.token_fd = prov_fd;
> 
Should be params.attribute_fd = prov_fd;

Thanks,
-Kai

^ permalink raw reply

* Re: kernel BUG at kernel/cred.c:434!
From: Yang Yingliang @ 2019-04-19 14:34 UTC (permalink / raw)
  To: Paul Moore
  Cc: Casey Schaufler, Oleg Nesterov, john.johansen, chengjian (D),
	Kees Cook, NeilBrown, Anna Schumaker,
	linux-kernel@vger.kernel.org, Al Viro, Xiexiuqi (Xie XiuQi),
	Li Bin, Jason Yan, Peter Zijlstra, Ingo Molnar,
	Linux Security Module list, SELinux, Yangyingliang, Cheng Jian
In-Reply-To: <CAHC9VhTdJ6uUOjyxrk=+iOkKe=VN+N9DMPtV=OchAY34x95g=w@mail.gmail.com>



On 2019/4/19 21:24, Paul Moore wrote:
> On Thu, Apr 18, 2019 at 10:42 PM Yang Yingliang
> <yangyingliang@huawei.com> wrote:
>> On 2019/4/19 10:04, Paul Moore wrote:
>>> On Wed, Apr 17, 2019 at 10:50 PM Yang Yingliang
>>> <yangyingliang@huawei.com> wrote:
>>>> On 2019/4/18 8:24, Casey Schaufler wrote:
>>>>> On 4/17/2019 4:39 PM, Paul Moore wrote:
>>>>>> Since it looks like all three LSMs which implement the setprocattr
>>>>>> hook are vulnerable I'm open to the idea that proc_pid_attr_write() is
>>>>>> a better choice for the cred != read_cred check, but I would want to
>>>>>> make sure John and Casey are okay with that.
>>>>>>
>>>>>> John?
>>>>>>
>>>>>> Casey?
>>>>> I'm fine with the change going into proc_pid_attr_write().
>>>> The cred != real_cred checking is not enough.
>>>>
>>>> Consider this situation, when doing override, cred, real_cred and
>>>> new_cred are all same:
>>>>
>>>> after override_creds()    cred == real_cred == new1_cred
>>> I'm sorry, you've lost me.  After override_creds() returns
>>> current->cred and current->real_cred are not going to be the same,
>>> yes?
>> It's possible the new  cred is equal to current->real_cred and
>> current->cred,
>> so after overrides_creds(), they have the same value.
> Both task_struct.cred and task_struct.real_cred are pointer values,
> assuming that one uses prepare_creds() to allocate/initialize a new
> cred struct for use with override_creds() then the newly created cred
> should never be equal to task_struct.real_cred.  Am I missing
> something, or are you thinking of something else?
In do_acct_process(), file->f_cred may equal to current->real_cred, I 
confirm
it by adding some debug message in do_acct_process() like this:

--- a/kernel/acct.c
+++ b/kernel/acct.c
@@ -481,6 +481,7 @@ static void do_acct_process(struct bsd_acct_struct 
*acct)
         flim = current->signal->rlim[RLIMIT_FSIZE].rlim_cur;
         current->signal->rlim[RLIMIT_FSIZE].rlim_cur = RLIM_INFINITY;
         /* Perform file operations on behalf of whoever enabled 
accounting */
+       pr_info("task:%px new cred:%px real cred:%px cred:%px\n", 
current, file->f_cred, current->real_cred, current->cred);
         orig_cred = override_creds(file->f_cred);



Messages:
[   56.643298] task:ffff88841a9595c0 new cred:ffff88841ae450c0 real 
cred:ffff88841ae450c0 cred:ffff88841ae450c0    //They are same.
[   56.646609] Process accounting resumed
[   56.649943] task:ffff88841a9595c0 new cred:ffff88841ae450c0 real 
cred:ffff88841c96c300 cred:ffff88841ae450c0
[   56.653565] ------------[ cut here ]------------
[   56.655119] kernel BUG at kernel/cred.c:434!
[   56.656590] invalid opcode: 0000 [#1] SMP PTI
[   56.658033] CPU: 2 PID: 4169 Comm: syz-executor.15 Not tainted 
5.1.0-rc4-00034-g869e3305f23d-dirty #143
[   56.661077] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), 
BIOS rel-1.12.1-0-ga5cab58e9a3f-prebuilt.qemu.org 04/01/2014
[   56.664895] RIP: 0010:commit_creds+0x1eb/0x230
[   56.666344] Code: 43 1c 0f 85 08 ff ff ff e9 10 ff ff ff 8b 45 10 39 
43 10 0f 85 18 ff ff ff 8b 43 20 39 45 20 0f 85 0c ff ff ff e9 14 ff ff 
ff <0f> 0b 48 c7 c7 d0 d2 49 82 e8 17 3b 3e 00 0f 0b 48 c7 c7 c0 d2 49
[   56.672410] RSP: 0018:ffffc90003a17b20 EFLAGS: 00010287
[   56.674098] RAX: ffff88841a9595c0 RBX: ffff88841ae450c0 RCX: 
0000000000000000
[   56.676410] RDX: 0000000000000001 RSI: 0000000000000020 RDI: 
ffff88841c96ce40
[   56.678691] RBP: 0000000000000001 R08: 0000000000800000 R09: 
0000000000000000
[   56.680997] R10: ffff88841c9265a0 R11: ffffffff810d6940 R12: 
ffff88841a9595c0
[   56.681198] task:ffff88841a9195c0 new cred:ffff88841aeaa0c0 real 
cred:ffff88841aeaa0c0 cred:ffff88841aeaa0c0
[   56.683293] R13: 0000000000000040 R14: ffff88841c96ce40 R15: 
0000000000000040
[   56.683296] FS:  00007f5969a5c700(0000) GS:ffff88842fa80000(0000) 
knlGS:0000000000000000
[   56.683297] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   56.683299] CR2: 00007f82742214f0 CR3: 000000041cbc0005 CR4: 
00000000000206e0
[   56.683305] Call Trace:
[   56.683340]  selinux_setprocattr+0x17b/0x480
[   56.686513] Process accounting resumed
[   56.688849]  proc_pid_attr_write+0xc0/0xf0
[   56.688857]  __kernel_write+0x4f/0xf0
[   56.688866]  do_acct_process+0x538/0x750
[   56.703090]  ? __schedule+0x290/0x960
[   56.704311]  ? __queue_work+0x160/0x5c0
[   56.705571]  acct_pin_kill+0x1e/0x70
[   56.706743]  pin_kill+0x81/0x150
[   56.707813]  ? finish_wait+0x80/0x80
[   56.708985]  mnt_pin_kill+0x1e/0x30
[   56.710127]  cleanup_mnt+0x6e/0x70
[   56.711247]  task_work_run+0x8a/0xb0
[   56.712453]  do_exit+0x2e0/0xc80
[   56.713525]  do_group_exit+0x33/0xb0
[   56.714701]  get_signal+0x143/0x810
[   56.715865]  do_signal+0x36/0x610
[   56.716962]  ? __x64_sys_futex+0x134/0x180
[   56.718307]  ? _copy_to_user+0x22/0x30
[   56.719606]  exit_to_usermode_loop+0x80/0xe0
[   56.721003]  do_syscall_64+0x16c/0x180
[   56.722242]  entry_SYSCALL_64_after_hwframe+0x44/0xa9




^ permalink raw reply

* Re: kernel BUG at kernel/cred.c:434!
From: Paul Moore @ 2019-04-19 13:24 UTC (permalink / raw)
  To: Yang Yingliang
  Cc: Casey Schaufler, Oleg Nesterov, john.johansen, chengjian (D),
	Kees Cook, NeilBrown, Anna Schumaker,
	linux-kernel@vger.kernel.org, Al Viro, Xiexiuqi (Xie XiuQi),
	Li Bin, Jason Yan, Peter Zijlstra, Ingo Molnar,
	Linux Security Module list, SELinux
In-Reply-To: <5CB933C4.7000300@huawei.com>

On Thu, Apr 18, 2019 at 10:42 PM Yang Yingliang
<yangyingliang@huawei.com> wrote:
> On 2019/4/19 10:04, Paul Moore wrote:
> > On Wed, Apr 17, 2019 at 10:50 PM Yang Yingliang
> > <yangyingliang@huawei.com> wrote:
> >> On 2019/4/18 8:24, Casey Schaufler wrote:
> >>> On 4/17/2019 4:39 PM, Paul Moore wrote:
> >>>> Since it looks like all three LSMs which implement the setprocattr
> >>>> hook are vulnerable I'm open to the idea that proc_pid_attr_write() is
> >>>> a better choice for the cred != read_cred check, but I would want to
> >>>> make sure John and Casey are okay with that.
> >>>>
> >>>> John?
> >>>>
> >>>> Casey?
> >>> I'm fine with the change going into proc_pid_attr_write().
> >> The cred != real_cred checking is not enough.
> >>
> >> Consider this situation, when doing override, cred, real_cred and
> >> new_cred are all same:
> >>
> >> after override_creds()    cred == real_cred == new1_cred
> > I'm sorry, you've lost me.  After override_creds() returns
> > current->cred and current->real_cred are not going to be the same,
> > yes?
>
> It's possible the new  cred is equal to current->real_cred and
> current->cred,
> so after overrides_creds(), they have the same value.

Both task_struct.cred and task_struct.real_cred are pointer values,
assuming that one uses prepare_creds() to allocate/initialize a new
cred struct for use with override_creds() then the newly created cred
should never be equal to task_struct.real_cred.  Am I missing
something, or are you thinking of something else?

-- 
paul moore
www.paul-moore.com

^ permalink raw reply

* Re: [PATCH 00/90] LSM: Module stacking for all
From: Stephen Smalley @ 2019-04-19 15:27 UTC (permalink / raw)
  To: Casey Schaufler, casey.schaufler, jmorris, linux-security-module,
	selinux
In-Reply-To: <20190419004617.64627-1-casey@schaufler-ca.com>

On 4/18/19 8:44 PM, Casey Schaufler wrote:
> This patchset provides the changes required for
> the any security module to stack safely with any other.
> 
> A new process attribute identifies which security module
> information should be reported by SO_PEERSEC and the
> /proc/.../attr/current interface. This is provided by
> /proc/.../attr/display. Writing the name of the security
> module desired to this interface will set which LSM hooks
> will be called for this information. The first security
> module providing the hooks will be used by default.
> 
> The use of integer based security tokens (secids) is
> generally (but not completely) replaced by a structure
> lsm_export. The lsm_export structure can contain information
> for each of the security modules that export information
> outside the LSM layer.
> 
> The LSM interfaces that provide "secctx" text strings
> have been changed to use a structure "lsm_context"
> instead of a pointer/length pair. In some cases the
> interfaces used a "char *" pointer and in others a
> "void *". This was necessary to ensure that the correct
> release mechanism for the text is used. It also makes
> many of the interfaces cleaner.
> 
> Security modules that use Netlabel must agree on the
> labels to be used on outgoing packets. If the modules
> do not agree on the label option to be used the operation
> will fail.
> 
> Netfilter secmarks are restricted to a single security
> module. The first module using the facility will "own"
> the secmarks.

Is it expected that enabling all security modules with this change will 
yield permission denials on packet send/receive (e.g. sendmsg() fails 
with permission denied), even without any configuration of NetLabel or 
SECMARK?  That's what I see.

> 
> git://github.com/cschaufler/lsm-stacking.git#stack-5.1-v2-full
> 
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> ---
>   drivers/android/binder.c                |  25 +-
>   fs/kernfs/dir.c                         |   6 +-
>   fs/kernfs/inode.c                       |  31 +-
>   fs/kernfs/kernfs-internal.h             |   3 +-
>   fs/nfs/inode.c                          |  13 +-
>   fs/nfs/internal.h                       |   8 +-
>   fs/nfs/nfs4proc.c                       |  17 +-
>   fs/nfs/nfs4xdr.c                        |  16 +-
>   fs/nfsd/nfs4proc.c                      |   8 +-
>   fs/nfsd/nfs4xdr.c                       |  14 +-
>   fs/nfsd/vfs.c                           |   7 +-
>   fs/proc/base.c                          |   1 +
>   include/linux/cred.h                    |   3 +-
>   include/linux/lsm_hooks.h               | 119 +++---
>   include/linux/nfs4.h                    |   8 +-
>   include/linux/security.h                | 159 ++++++--
>   include/net/af_unix.h                   |   2 +-
>   include/net/netlabel.h                  |  18 +-
>   include/net/scm.h                       |  14 +-
>   kernel/audit.c                          |  43 +--
>   kernel/audit.h                          |   9 +-
>   kernel/auditfilter.c                    |   6 +-
>   kernel/auditsc.c                        |  77 ++--
>   kernel/cred.c                           |  15 +-
>   net/ipv4/cipso_ipv4.c                   |  13 +-
>   net/ipv4/ip_sockglue.c                  |  14 +-
>   net/netfilter/nf_conntrack_netlink.c    |  29 +-
>   net/netfilter/nf_conntrack_standalone.c |  16 +-
>   net/netfilter/nfnetlink_queue.c         |  35 +-
>   net/netfilter/nft_meta.c                |   8 +-
>   net/netfilter/xt_SECMARK.c              |   9 +-
>   net/netlabel/netlabel_kapi.c            | 125 ++++--
>   net/netlabel/netlabel_unlabeled.c       | 101 +++--
>   net/netlabel/netlabel_unlabeled.h       |   2 +-
>   net/netlabel/netlabel_user.c            |  13 +-
>   net/netlabel/netlabel_user.h            |   2 +-
>   net/unix/af_unix.c                      |   6 +-
>   security/apparmor/audit.c               |   4 +-
>   security/apparmor/include/audit.h       |   2 +-
>   security/apparmor/include/net.h         |   6 +-
>   security/apparmor/include/secid.h       |   9 +-
>   security/apparmor/lsm.c                 |  64 ++--
>   security/apparmor/secid.c               |  42 +-
>   security/integrity/ima/ima.h            |  14 +-
>   security/integrity/ima/ima_api.c        |   9 +-
>   security/integrity/ima/ima_appraise.c   |   6 +-
>   security/integrity/ima/ima_main.c       |  34 +-
>   security/integrity/ima/ima_policy.c     |  19 +-
>   security/security.c                     | 653 +++++++++++++++++++++++++++-----
>   security/selinux/hooks.c                | 310 +++++++--------
>   security/selinux/include/audit.h        |   5 +-
>   security/selinux/include/netlabel.h     |   7 +
>   security/selinux/include/objsec.h       |  43 ++-
>   security/selinux/netlabel.c             |  69 ++--
>   security/selinux/ss/services.c          |  18 +-
>   security/smack/smack.h                  |  34 ++
>   security/smack/smack_access.c           |  14 +-
>   security/smack/smack_lsm.c              | 388 ++++++++++---------
>   security/smack/smack_netfilter.c        |  48 ++-
>   security/smack/smackfs.c                |  23 +-
>   60 files changed, 1855 insertions(+), 961 deletions(-)
> 


^ permalink raw reply

* Re: kernel BUG at kernel/cred.c:434!
From: Paul Moore @ 2019-04-19 16:13 UTC (permalink / raw)
  To: Yang Yingliang
  Cc: Casey Schaufler, Oleg Nesterov, john.johansen, chengjian (D),
	Kees Cook, NeilBrown, Anna Schumaker,
	linux-kernel@vger.kernel.org, Al Viro, Xiexiuqi (Xie XiuQi),
	Li Bin, Jason Yan, Peter Zijlstra, Ingo Molnar,
	Linux Security Module list, SELinux
In-Reply-To: <5CB9DC75.7010600@huawei.com>

On Fri, Apr 19, 2019 at 10:34 AM Yang Yingliang
<yangyingliang@huawei.com> wrote:
> On 2019/4/19 21:24, Paul Moore wrote:
> > On Thu, Apr 18, 2019 at 10:42 PM Yang Yingliang
> > <yangyingliang@huawei.com> wrote:
> >> On 2019/4/19 10:04, Paul Moore wrote:
> >>> On Wed, Apr 17, 2019 at 10:50 PM Yang Yingliang
> >>> <yangyingliang@huawei.com> wrote:
> >>>> On 2019/4/18 8:24, Casey Schaufler wrote:
> >>>>> On 4/17/2019 4:39 PM, Paul Moore wrote:
> >>>>>> Since it looks like all three LSMs which implement the setprocattr
> >>>>>> hook are vulnerable I'm open to the idea that proc_pid_attr_write() is
> >>>>>> a better choice for the cred != read_cred check, but I would want to
> >>>>>> make sure John and Casey are okay with that.
> >>>>>>
> >>>>>> John?
> >>>>>>
> >>>>>> Casey?
> >>>>> I'm fine with the change going into proc_pid_attr_write().
> >>>> The cred != real_cred checking is not enough.
> >>>>
> >>>> Consider this situation, when doing override, cred, real_cred and
> >>>> new_cred are all same:
> >>>>
> >>>> after override_creds()    cred == real_cred == new1_cred
> >>> I'm sorry, you've lost me.  After override_creds() returns
> >>> current->cred and current->real_cred are not going to be the same,
> >>> yes?
> >> It's possible the new  cred is equal to current->real_cred and
> >> current->cred,
> >> so after overrides_creds(), they have the same value.
> > Both task_struct.cred and task_struct.real_cred are pointer values,
> > assuming that one uses prepare_creds() to allocate/initialize a new
> > cred struct for use with override_creds() then the newly created cred
> > should never be equal to task_struct.real_cred.  Am I missing
> > something, or are you thinking of something else?
>
> In do_acct_process(), file->f_cred may equal to current->real_cred, I
> confirm
> it by adding some debug message in do_acct_process() like this:

I would expect that; real_cred is the task's objective DAC
credentials, so using it for f_cred makes sense.

What we are now talking about is the task's subjective credentials,
which can be overridden via override_creds(), and are what the LSMs
change via proc_pid_attr_write().

> --- a/kernel/acct.c
> +++ b/kernel/acct.c
> @@ -481,6 +481,7 @@ static void do_acct_process(struct bsd_acct_struct
> *acct)
>          flim = current->signal->rlim[RLIMIT_FSIZE].rlim_cur;
>          current->signal->rlim[RLIMIT_FSIZE].rlim_cur = RLIM_INFINITY;
>          /* Perform file operations on behalf of whoever enabled
> accounting */
> +       pr_info("task:%px new cred:%px real cred:%px cred:%px\n",
> current, file->f_cred, current->real_cred, current->cred);
>          orig_cred = override_creds(file->f_cred);

-- 
paul moore
www.paul-moore.com

^ permalink raw reply

* [PATCH] proc: prevent changes to overridden credentials
From: Paul Moore @ 2019-04-19 18:55 UTC (permalink / raw)
  To: linux-security-module; +Cc: selinux, cj.chengjian, john.johansen, casey

Prevent userspace from changing the the /proc/PID/attr values if the
task's credentials are currently overriden.  This not only makes sense
conceptually, it also prevents some really bizarre error cases caused
when trying to commit credentials to a task with overridden
credentials.

Cc: <stable@vger.kernel.org>
Reported-by: "chengjian (D)" <cj.chengjian@huawei.com>
Signed-off-by: Paul Moore <paul@paul-moore.com>
---
 fs/proc/base.c |    5 +++++
 1 file changed, 5 insertions(+)

diff --git a/fs/proc/base.c b/fs/proc/base.c
index ddef482f1334..87ba007b86db 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -2539,6 +2539,11 @@ static ssize_t proc_pid_attr_write(struct file * file, const char __user * buf,
 		rcu_read_unlock();
 		return -EACCES;
 	}
+	/* Prevent changes to overridden credentials. */
+	if (current_cred() != current_real_cred()) {
+		rcu_read_unlock();
+		return -EBUSY;
+	}
 	rcu_read_unlock();
 
 	if (count > PAGE_SIZE)


^ permalink raw reply related

* Re: [PATCH] proc: prevent changes to overridden credentials
From: Paul Moore @ 2019-04-19 18:59 UTC (permalink / raw)
  To: linux-security-module; +Cc: selinux, cj.chengjian, john.johansen, casey
In-Reply-To: <155570011247.27135.12509150054846153288.stgit@chester>

On Fri, Apr 19, 2019 at 2:55 PM Paul Moore <paul@paul-moore.com> wrote:
> Prevent userspace from changing the the /proc/PID/attr values if the
> task's credentials are currently overriden.  This not only makes sense
> conceptually, it also prevents some really bizarre error cases caused
> when trying to commit credentials to a task with overridden
> credentials.
>
> Cc: <stable@vger.kernel.org>
> Reported-by: "chengjian (D)" <cj.chengjian@huawei.com>
> Signed-off-by: Paul Moore <paul@paul-moore.com>
> ---
>  fs/proc/base.c |    5 +++++
>  1 file changed, 5 insertions(+)

I sent this to the LSM list as I figure it should probably go via
James' linux-security tree since it is cross-LSM and doesn't really
contain any LSM specific code.  That said, if you don't want this
James let me know and I'll send it via the SELinux tree assuming I can
get ACKs from John and Casey (this should only affect SELinux,
AppArmor, and Smack).

> diff --git a/fs/proc/base.c b/fs/proc/base.c
> index ddef482f1334..87ba007b86db 100644
> --- a/fs/proc/base.c
> +++ b/fs/proc/base.c
> @@ -2539,6 +2539,11 @@ static ssize_t proc_pid_attr_write(struct file * file, const char __user * buf,
>                 rcu_read_unlock();
>                 return -EACCES;
>         }
> +       /* Prevent changes to overridden credentials. */
> +       if (current_cred() != current_real_cred()) {
> +               rcu_read_unlock();
> +               return -EBUSY;
> +       }
>         rcu_read_unlock();
>
>         if (count > PAGE_SIZE)
>

-- 
paul moore
www.paul-moore.com

^ permalink raw reply

* Re: [PATCH v2 1/3] security: Create "kernel hardening" config area
From: Alexander Popov @ 2019-04-19 19:15 UTC (permalink / raw)
  To: Kees Cook, Masahiro Yamada
  Cc: Alexander Potapenko, James Morris, Nick Desaulniers,
	Kostya Serebryany, Dmitry Vyukov, Sandeep Patil, Laura Abbott,
	Randy Dunlap, Michal Marek, Emese Revfy, Serge E. Hallyn,
	Kernel Hardening, linux-security-module, linux-kbuild, LKML
In-Reply-To: <CAGXu5j+bVtkXHhs03t_yb1gp51WRhzZGqNpzV5VPb6dfnptw5w@mail.gmail.com>

On 16.04.2019 16:56, Kees Cook wrote:
> On Tue, Apr 16, 2019 at 8:55 AM Alexander Popov <alex.popov@linux.com> wrote:
>>
>> On 16.04.2019 7:02, Kees Cook wrote:
>>> On Mon, Apr 15, 2019 at 11:44 AM Alexander Popov <alex.popov@linux.com> wrote:
>>>>
>>>> What do you think about some separator between memory initialization options and
>>>> CONFIG_CRYPTO?
>>>
>>> This was true before too
>>
>> Hm, yes, it's a generic behavior - there is no any separator at 'endmenu' and
>> config options stick together.
>>
>> I've created a patch to fix that. What do you think about it?
>> I can send it to LKML separately.
>>
>>
>> From 50bf59d30fafcdebb3393fb742e1bd51e7d2f2da Mon Sep 17 00:00:00 2001
>> From: Alexander Popov <alex.popov@linux.com>
>> Date: Tue, 16 Apr 2019 16:09:40 +0300
>> Subject: [PATCH 1/1] kconfig: Terminate menu blocks with a newline in the
>>  generated config
>>
>> Currently menu blocks start with a pretty header but end with nothing in
>> the generated config. So next config options stick together with the
>> options from the menu block.
>>
>> Let's terminate menu blocks with a newline in the generated config.
>>
>> Signed-off-by: Alexander Popov <alex.popov@linux.com>
>> ---
>>  scripts/kconfig/confdata.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
>> index 08ba146..1459153 100644
>> --- a/scripts/kconfig/confdata.c
>> +++ b/scripts/kconfig/confdata.c
>> @@ -888,6 +888,8 @@ int conf_write(const char *name)
>>                 if (menu->next)
>>                         menu = menu->next;
>>                 else while ((menu = menu->parent)) {
>> +                       if (!menu->sym && menu_is_visible(menu))
>> +                               fprintf(out, "\n");
>>                         if (menu->next) {
>>                                 menu = menu->next;
>>                                 break;
> 
> Seems fine to me. I defer to Masahiro, though. :)

Hi! I've sent this patch separately to LKML:

https://lore.kernel.org/lkml/1555669773-9766-1-git-send-email-alex.popov@linux.com/T/#u

Best regards,
Alexander

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox