selinux.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v7 00/16] LSM: Full module stacking
@ 2019-08-07 22:42 Casey Schaufler
  2019-08-07 22:42 ` Casey Schaufler
                   ` (16 more replies)
  0 siblings, 17 replies; 18+ messages in thread
From: Casey Schaufler @ 2019-08-07 22:42 UTC (permalink / raw)
  To: casey.schaufler, jmorris, linux-security-module, selinux
  Cc: casey, keescook, john.johansen, penguin-kernel, paul, sds

Provide mechanisms for security modules that use network interfaces
to operate on the same system safely. Provide mechanisms for kerfs
to maintain information about all the security modules active on
a system. With these mechanisms in place, any combination of
existing security modules can be used. 

It is not clear that all issues with the integrity calls have
been addressed. Any review there would be much appreciated.
There should be a mechanism in netfilter to identify which
security module a given rule is associated with, but none is
proposed here. Instead, the module first registered is given
the secmark. Labeled NFS is an incomplete implementation,
missing the "LFS" data needed to identify the format of the
labels passed. The first registered assumption is made here
as well. The Netlabel restriction that all sent attributes must
be agreeable to all modules is reasonable. The implementation
is awkward, and insights into improvement would be most welcome.

It is important to be aware that while the mechanisms have
been made cohabitational, there is no guarantee that the policies
imposed by the security modules will be compatible. The handling
of unlabeled IP packets using Netlabel is very different in
SELinux and Smack. The use of capabilities is also very different
in those modules.

This has been tested on Fedora29 and Ubuntu19.04 using
unmodified policies. The SELinux test suite demonstrates
policy conflict issues when used with Smack.

Patches 0001-0002 change the secmark_refcount LSM interfaces
to call only the first registered security module's hooks.
Smack is modified to use these interfaces to determine if the
Internet Protocol secmark is meaningful to Smack.

Patches 0003 and 0009 refactor security_inode_init_security() to
accomodate integrity checking on multiple active security
attributes.

Patch 0004 updates security_inode_listsecurity() to provide
multiple security attributes instead of just one.

Patches 0005-0007 address NFS and kernfs. Labeled nfs has no
mechanism to identify what kind of label (e.g. CIPSO or Flask)
that is being sent across the wire, so an arbitrary choice must
be made on how NFS labels are handled. The module in lsmslot 0
is always used. Kernfs uses context strings to store security
attributes, so those are now saved in the "compound" format.

Patch 0008 provides infrastructure management of mount
option data.

Patch 0010 addresses the case where one security module may fail
to provide a valid secid on datagrams where another succeeds.

Patches 0011-0015 allow the LSM infrastructure to check whether
the security modules agree on network security attributes when
netlabel is being used.

Patch 0016 removes the exclusive flag from Smack.

A note on the "v7" designation: This depends on the stack-5.2-v7-apparmor
patches provided earlier. Since that is "v7" I've used the same number
here in the hopes that it will reduce confusion.

https://github.com/cschaufler/lsm-stacking.git#stack-5.2-v7-full

Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
---
 fs/kernfs/inode.c                   |   3 +-
 fs/nfs/inode.c                      |   9 +-
 fs/nfsd/nfs4proc.c                  |   6 +-
 fs/nfsd/vfs.c                       |   5 +-
 include/linux/lsm_hooks.h           |  23 +-
 include/linux/security.h            |  30 ++-
 include/net/netlabel.h              |   8 +
 net/ipv4/ip_sockglue.c              |   4 +-
 net/netlabel/netlabel_kapi.c        | 120 ++++++++---
 security/security.c                 | 419 +++++++++++++++++++++++++++---------
 security/selinux/hooks.c            |  54 +++--
 security/selinux/include/netlabel.h |   7 +
 security/selinux/include/objsec.h   |   1 +
 security/selinux/netlabel.c         |  46 ++--
 security/smack/smack.h              |  17 ++
 security/smack/smack_lsm.c          | 128 +++++++----
 security/smack/smack_netfilter.c    |  45 +++-
 17 files changed, 689 insertions(+), 236 deletions(-)

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

* [PATCH v7 00/16] LSM: Full module stacking
  2019-08-07 22:42 [PATCH v7 00/16] LSM: Full module stacking Casey Schaufler
@ 2019-08-07 22:42 ` Casey Schaufler
  2019-08-07 22:42 ` [PATCH v7 01/16] LSM: Single hook called in secmark refcounting Casey Schaufler
                   ` (15 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: Casey Schaufler @ 2019-08-07 22:42 UTC (permalink / raw)
  To: casey.schaufler, jmorris, linux-security-module, selinux
  Cc: casey, keescook, john.johansen, penguin-kernel, paul, sds

Provide mechanisms for security modules that use network interfaces
to operate on the same system safely. Provide mechanisms for kerfs
to maintain information about all the security modules active on
a system. With these mechanisms in place, any combination of
existing security modules can be used. 

It is not clear that all issues with the integrity calls have
been addressed. Any review there would be much appreciated.
There should be a mechanism in netfilter to identify which
security module a given rule is associated with, but none is
proposed here. Instead, the module first registered is given
the secmark. Labeled NFS is an incomplete implementation,
missing the "LFS" data needed to identify the format of the
labels passed. The first registered assumption is made here
as well. The Netlabel restriction that all sent attributes must
be agreeable to all modules is reasonable. The implementation
is awkward, and insights into improvement would be most welcome.

It is important to be aware that while the mechanisms have
been made cohabitational, there is no guarantee that the policies
imposed by the security modules will be compatible. The handling
of unlabeled IP packets using Netlabel is very different in
SELinux and Smack. The use of capabilities is also very different
in those modules.

This has been tested on Fedora29 and Ubuntu19.04 using
unmodified policies. The SELinux test suite demonstrates
policy conflict issues when used with Smack.

Patches 0001-0002 change the secmark_refcount LSM interfaces
to call only the first registered security module's hooks.
Smack is modified to use these interfaces to determine if the
Internet Protocol secmark is meaningful to Smack.

Patches 0003 and 0009 refactor security_inode_init_security() to
accommodate integrity checking on multiple active security
attributes.

Patch 0004 updates security_inode_listsecurity() to provide
multiple security attributes instead of just one.

Patches 0005-0007 address NFS and kernfs. Labeled nfs has no
mechanism to identify what kind of label (e.g. CIPSO or Flask)
that is being sent across the wire, so an arbitrary choice must
be made on how NFS labels are handled. The module in lsmslot 0
is always used. Kernfs uses context strings to store security
attributes, so those are now saved in the "compound" format.

Patch 0008 provides infrastructure management of mount
option data.

Patch 0010 addresses the case where one security module may fail
to provide a valid secid on datagrams where another succeeds.

Patches 0011-0015 allow the LSM infrastructure to check whether
the security modules agree on network security attributes when
netlabel is being used.

Patch 0016 removes the exclusive flag from Smack.

A note on the "v7" designation: This depends on the stack-5.2-v7-apparmor
patches provided earlier. Since that is "v7" I've used the same number
here in the hopes that it will reduce confusion.

https://github.com/cschaufler/lsm-stacking.git#stack-5.2-v7-full

Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
---
 fs/kernfs/inode.c                   |   3 +-
 fs/nfs/inode.c                      |   9 +-
 fs/nfsd/nfs4proc.c                  |   6 +-
 fs/nfsd/vfs.c                       |   5 +-
 include/linux/lsm_hooks.h           |  23 +-
 include/linux/security.h            |  30 ++-
 include/net/netlabel.h              |   8 +
 net/ipv4/ip_sockglue.c              |   4 +-
 net/netlabel/netlabel_kapi.c        | 120 ++++++++---
 security/security.c                 | 419 +++++++++++++++++++++++++++---------
 security/selinux/hooks.c            |  54 +++--
 security/selinux/include/netlabel.h |   7 +
 security/selinux/include/objsec.h   |   1 +
 security/selinux/netlabel.c         |  46 ++--
 security/smack/smack.h              |  17 ++
 security/smack/smack_lsm.c          | 128 +++++++----
 security/smack/smack_netfilter.c    |  45 +++-
 17 files changed, 689 insertions(+), 236 deletions(-)

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

* [PATCH v7 01/16] LSM: Single hook called in secmark refcounting
  2019-08-07 22:42 [PATCH v7 00/16] LSM: Full module stacking Casey Schaufler
  2019-08-07 22:42 ` Casey Schaufler
@ 2019-08-07 22:42 ` Casey Schaufler
  2019-08-07 22:42 ` [PATCH v7 02/16] Smack: Detect if secmarks can be safely used Casey Schaufler
                   ` (14 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: Casey Schaufler @ 2019-08-07 22:42 UTC (permalink / raw)
  To: casey.schaufler, jmorris, linux-security-module, selinux
  Cc: casey, keescook, john.johansen, penguin-kernel, paul, sds

Restrict the secmark_refcount_dec and secmark_refcount_inc
interfaces to a single module. The secmark is too small to
share, and this allows the modules a way to detect if the
secmark is theirs to use.

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

diff --git a/security/security.c b/security/security.c
index e9f579483d12..0467f194d87d 100644
--- a/security/security.c
+++ b/security/security.c
@@ -2564,13 +2564,25 @@ EXPORT_SYMBOL(security_secmark_relabel_packet);
 
 void security_secmark_refcount_inc(void)
 {
-	call_void_hook(secmark_refcount_inc);
+	struct security_hook_list *hp;
+
+	hlist_for_each_entry(hp, &security_hook_heads.secmark_refcount_inc,
+			     list) {
+		hp->hook.secmark_refcount_inc();
+		break;
+	}
 }
 EXPORT_SYMBOL(security_secmark_refcount_inc);
 
 void security_secmark_refcount_dec(void)
 {
-	call_void_hook(secmark_refcount_dec);
+	struct security_hook_list *hp;
+
+	hlist_for_each_entry(hp, &security_hook_heads.secmark_refcount_dec,
+			     list) {
+		hp->hook.secmark_refcount_dec();
+		break;
+	}
 }
 EXPORT_SYMBOL(security_secmark_refcount_dec);
 
-- 
2.20.1


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

* [PATCH v7 02/16] Smack: Detect if secmarks can be safely used
  2019-08-07 22:42 [PATCH v7 00/16] LSM: Full module stacking Casey Schaufler
  2019-08-07 22:42 ` Casey Schaufler
  2019-08-07 22:42 ` [PATCH v7 01/16] LSM: Single hook called in secmark refcounting Casey Schaufler
@ 2019-08-07 22:42 ` Casey Schaufler
  2019-08-07 22:42 ` [PATCH v7 03/16] LSM: Support multiple LSMs using inode_init_security Casey Schaufler
                   ` (13 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: Casey Schaufler @ 2019-08-07 22:42 UTC (permalink / raw)
  To: casey.schaufler, jmorris, linux-security-module, selinux
  Cc: casey, keescook, john.johansen, penguin-kernel, paul, sds

Utilize the security_secmark_refcount_in() hooks to determine
if Smack can safely assume that IP secmarks are not being used
by another LSM. Only use secmarks if they can be determined to
belong to Smack.

Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
---
 security/smack/smack.h           | 16 ++++++++++
 security/smack/smack_lsm.c       | 54 +++++++++++++++++++-------------
 security/smack/smack_netfilter.c | 39 +++++++++++++++++++++--
 3 files changed, 85 insertions(+), 24 deletions(-)

diff --git a/security/smack/smack.h b/security/smack/smack.h
index 039bf5de56b4..f28db5a42b7b 100644
--- a/security/smack/smack.h
+++ b/security/smack/smack.h
@@ -545,4 +545,20 @@ static inline void smk_ad_setfield_u_net_sk(struct smk_audit_info *a,
 }
 #endif
 
+#ifdef CONFIG_SECURITY_SMACK_NETFILTER
+extern bool smack_use_secmark;
+void smack_secmark_refcount_inc(void);
+void smack_secmark_refcount_dec(void);
+
+static inline bool smk_use_secmark(void)
+{
+	return smack_use_secmark;
+}
+#else
+static inline bool smk_use_secmark(void)
+{
+	return false;
+}
+#endif
+
 #endif  /* _SECURITY_SMACK_H */
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 40c75205a914..341a9927ed5c 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -3801,6 +3801,20 @@ static int smk_skb_to_addr_ipv6(struct sk_buff *skb, struct sockaddr_in6 *sip)
 }
 #endif /* CONFIG_IPV6 */
 
+/**
+ * smack_from_skb - Smack data from the secmark in an skb
+ * @skb: packet
+ *
+ * Returns smack_known of the secmark or NULL if that won't work.
+ */
+static struct smack_known *smack_from_skb(struct sk_buff *skb)
+{
+	if (skb == NULL || skb->secmark == 0 || !smk_use_secmark())
+		return NULL;
+
+	return smack_from_secid(skb->secmark);
+}
+
 /**
  * smack_socket_sock_rcv_skb - Smack packet delivery access check
  * @sk: socket
@@ -3829,17 +3843,14 @@ static int smack_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb)
 
 	switch (family) {
 	case PF_INET:
-#ifdef CONFIG_SECURITY_SMACK_NETFILTER
 		/*
 		 * If there is a secmark use it rather than the CIPSO label.
 		 * If there is no secmark fall back to CIPSO.
 		 * The secmark is assumed to reflect policy better.
 		 */
-		if (skb && skb->secmark != 0) {
-			skp = smack_from_secid(skb->secmark);
+		skp = smack_from_skb(skb);
+		if (skp)
 			goto access_check;
-		}
-#endif /* CONFIG_SECURITY_SMACK_NETFILTER */
 		/*
 		 * Translate what netlabel gave us.
 		 */
@@ -3853,9 +3864,8 @@ static int smack_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb)
 
 		netlbl_secattr_destroy(&secattr);
 
-#ifdef CONFIG_SECURITY_SMACK_NETFILTER
 access_check:
-#endif
+
 #ifdef CONFIG_AUDIT
 		smk_ad_init_net(&ad, __func__, LSM_AUDIT_DATA_NET, &net);
 		ad.a.u.net->family = family;
@@ -3881,9 +3891,8 @@ static int smack_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb)
 		    proto != IPPROTO_TCP && proto != IPPROTO_DCCP)
 			break;
 #ifdef SMACK_IPV6_SECMARK_LABELING
-		if (skb && skb->secmark != 0)
-			skp = smack_from_secid(skb->secmark);
-		else
+		skp = smack_from_skb(skb);
+		if (skp == NULL)
 			skp = smack_ipv6host_label(&sadd);
 		if (skp == NULL)
 			skp = smack_net_ambient;
@@ -3983,11 +3992,11 @@ static int smack_socket_getpeersec_dgram(struct socket *sock,
 		s = ssp->smk_out->smk_secid;
 		break;
 	case PF_INET:
-#ifdef CONFIG_SECURITY_SMACK_NETFILTER
-		s = skb->secmark;
-		if (s != 0)
+		skp = smack_from_skb(skb);
+		if (skp) {
+			s = skp->smk_secid;
 			break;
-#endif
+		}
 		/*
 		 * Translate what netlabel gave us.
 		 */
@@ -4003,7 +4012,9 @@ static int smack_socket_getpeersec_dgram(struct socket *sock,
 		break;
 	case PF_INET6:
 #ifdef SMACK_IPV6_SECMARK_LABELING
-		s = skb->secmark;
+		skp = smack_from_skb(skb);
+		if (skp)
+			s = skp->smk_secid;
 #endif
 		break;
 	}
@@ -4075,17 +4086,14 @@ static int smack_inet_conn_request(struct sock *sk, struct sk_buff *skb,
 	}
 #endif /* CONFIG_IPV6 */
 
-#ifdef CONFIG_SECURITY_SMACK_NETFILTER
 	/*
 	 * If there is a secmark use it rather than the CIPSO label.
 	 * If there is no secmark fall back to CIPSO.
 	 * The secmark is assumed to reflect policy better.
 	 */
-	if (skb && skb->secmark != 0) {
-		skp = smack_from_secid(skb->secmark);
+	skp = smack_from_skb(skb);
+	if (skp)
 		goto access_check;
-	}
-#endif /* CONFIG_SECURITY_SMACK_NETFILTER */
 
 	netlbl_secattr_init(&secattr);
 	rc = netlbl_skbuff_getattr(skb, family, &secattr);
@@ -4095,9 +4103,7 @@ static int smack_inet_conn_request(struct sock *sk, struct sk_buff *skb,
 		skp = &smack_known_huh;
 	netlbl_secattr_destroy(&secattr);
 
-#ifdef CONFIG_SECURITY_SMACK_NETFILTER
 access_check:
-#endif
 
 #ifdef CONFIG_AUDIT
 	smk_ad_init_net(&ad, __func__, LSM_AUDIT_DATA_NET, &net);
@@ -4673,6 +4679,10 @@ static struct security_hook_list smack_hooks[] __lsm_ro_after_init = {
 	LSM_HOOK_INIT(sk_alloc_security, smack_sk_alloc_security),
 #ifdef SMACK_IPV6_PORT_LABELING
 	LSM_HOOK_INIT(sk_free_security, smack_sk_free_security),
+#endif
+#ifdef CONFIG_SECURITY_SMACK_NETFILTER
+	LSM_HOOK_INIT(secmark_refcount_inc, smack_secmark_refcount_inc),
+	LSM_HOOK_INIT(secmark_refcount_dec, smack_secmark_refcount_dec),
 #endif
 	LSM_HOOK_INIT(sock_graft, smack_sock_graft),
 	LSM_HOOK_INIT(inet_conn_request, smack_inet_conn_request),
diff --git a/security/smack/smack_netfilter.c b/security/smack/smack_netfilter.c
index 701a1cc1bdcc..7b9c8d5d8408 100644
--- a/security/smack/smack_netfilter.c
+++ b/security/smack/smack_netfilter.c
@@ -21,6 +21,29 @@
 #include <net/net_namespace.h>
 #include "smack.h"
 
+bool smack_use_secmark;
+static bool smack_checked_secmark;
+
+/**
+ * smack_secmark_refcount_inc - Seize the secmark
+ *
+ * Note to the rest of the Smack code that secmarks may be used.
+ */
+void smack_secmark_refcount_inc(void)
+{
+	smack_use_secmark = true;
+	pr_info("Smack: Using network secmarks.\n");
+}
+
+/**
+ * smack_secmark_refcount_dec - Do nothing about the secmark
+ *
+ * Matches the incrementing function, but does nothing.
+ */
+void smack_secmark_refcount_dec(void)
+{
+}
+
 #if IS_ENABLED(CONFIG_IPV6)
 
 static unsigned int smack_ipv6_output(void *priv,
@@ -31,7 +54,13 @@ static unsigned int smack_ipv6_output(void *priv,
 	struct socket_smack *ssp;
 	struct smack_known *skp;
 
-	if (sk && smack_sock(sk)) {
+	if (!smack_checked_secmark) {
+		security_secmark_refcount_inc();
+		security_secmark_refcount_dec();
+		smack_checked_secmark = true;
+	}
+
+	if (smack_use_secmark && sk && smack_sock(sk)) {
 		ssp = smack_sock(sk);
 		skp = ssp->smk_out;
 		skb->secmark = skp->smk_secid;
@@ -49,7 +78,13 @@ static unsigned int smack_ipv4_output(void *priv,
 	struct socket_smack *ssp;
 	struct smack_known *skp;
 
-	if (sk && smack_sock(sk)) {
+	if (!smack_checked_secmark) {
+		security_secmark_refcount_inc();
+		security_secmark_refcount_dec();
+		smack_checked_secmark = true;
+	}
+
+	if (smack_use_secmark && sk && smack_sock(sk)) {
 		ssp = smack_sock(sk);
 		skp = ssp->smk_out;
 		skb->secmark = skp->smk_secid;
-- 
2.20.1


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

* [PATCH v7 03/16] LSM: Support multiple LSMs using inode_init_security
  2019-08-07 22:42 [PATCH v7 00/16] LSM: Full module stacking Casey Schaufler
                   ` (2 preceding siblings ...)
  2019-08-07 22:42 ` [PATCH v7 02/16] Smack: Detect if secmarks can be safely used Casey Schaufler
@ 2019-08-07 22:42 ` Casey Schaufler
  2019-08-07 22:42 ` [PATCH v7 04/16] LSM: List multiple security attributes in security_inode_listsecurity Casey Schaufler
                   ` (12 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: Casey Schaufler @ 2019-08-07 22:42 UTC (permalink / raw)
  To: casey.schaufler, jmorris, linux-security-module, selinux
  Cc: casey, keescook, john.johansen, penguin-kernel, paul, sds

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 0467f194d87d..a58e60970035 100644
--- a/security/security.c
+++ b/security/security.c
@@ -1096,9 +1096,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;
@@ -1106,24 +1107,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.20.1


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

* [PATCH v7 04/16] LSM: List multiple security attributes in security_inode_listsecurity
  2019-08-07 22:42 [PATCH v7 00/16] LSM: Full module stacking Casey Schaufler
                   ` (3 preceding siblings ...)
  2019-08-07 22:42 ` [PATCH v7 03/16] LSM: Support multiple LSMs using inode_init_security Casey Schaufler
@ 2019-08-07 22:42 ` Casey Schaufler
  2019-08-07 22:42 ` [PATCH v7 05/16] LSM: Multiple modules using security_ismaclabel Casey Schaufler
                   ` (11 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: Casey Schaufler @ 2019-08-07 22:42 UTC (permalink / raw)
  To: casey.schaufler, jmorris, linux-security-module, selinux
  Cc: casey, keescook, john.johansen, penguin-kernel, paul, sds

Listing security extended attributes is extended to the case
where there is more than one security module that provides them.
The same format used in other xattr list providers:
	name1\0name2\0name3
is used.

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

diff --git a/security/security.c b/security/security.c
index a58e60970035..87cb3562646b 100644
--- a/security/security.c
+++ b/security/security.c
@@ -1475,9 +1475,34 @@ int security_inode_setsecurity(struct inode *inode, const char *name, const void
 
 int security_inode_listsecurity(struct inode *inode, char *buffer, size_t buffer_size)
 {
+	struct security_hook_list *hp;
+	bool first = true;
+	int finallen = 0;
+	int len;
+
 	if (unlikely(IS_PRIVATE(inode)))
 		return 0;
-	return call_int_hook(inode_listsecurity, 0, inode, buffer, buffer_size);
+
+	hlist_for_each_entry(hp, &security_hook_heads.inode_listsecurity,
+			     list) {
+		len = hp->hook.inode_listsecurity(inode, buffer, buffer_size);
+		if (len < buffer_size) {
+			if (buffer)
+				buffer[len] = '\0';
+			buffer_size -= len + 1;
+		} else {
+			buffer = NULL;
+			buffer_size = 0;
+		}
+		if (first) {
+			finallen = len;
+			first = false;
+		} else
+			finallen += len + 1;
+		if (buffer)
+			buffer += len + 1;
+	}
+	return finallen;
 }
 EXPORT_SYMBOL(security_inode_listsecurity);
 
-- 
2.20.1


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

* [PATCH v7 05/16] LSM: Multiple modules using security_ismaclabel
  2019-08-07 22:42 [PATCH v7 00/16] LSM: Full module stacking Casey Schaufler
                   ` (4 preceding siblings ...)
  2019-08-07 22:42 ` [PATCH v7 04/16] LSM: List multiple security attributes in security_inode_listsecurity Casey Schaufler
@ 2019-08-07 22:42 ` Casey Schaufler
  2019-08-07 22:42 ` [PATCH v7 06/16] LSM: Make multiple MAC modules safe in nfs and kernfs Casey Schaufler
                   ` (10 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: Casey Schaufler @ 2019-08-07 22:42 UTC (permalink / raw)
  To: casey.schaufler, jmorris, linux-security-module, selinux
  Cc: casey, keescook, john.johansen, penguin-kernel, paul, sds

Correct the infrastructure logic calling ismaclabel hooks
to reflect the yes/no result of the call. Instead of the
usual "any failure is an error" this hook uses "any success
is success".

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

diff --git a/security/security.c b/security/security.c
index 87cb3562646b..13102d16bf2c 100644
--- a/security/security.c
+++ b/security/security.c
@@ -2212,7 +2212,12 @@ int security_netlink_send(struct sock *sk, struct sk_buff *skb)
 
 int security_ismaclabel(const char *name)
 {
-	return call_int_hook(ismaclabel, 0, name);
+	struct security_hook_list *hp;
+
+	hlist_for_each_entry(hp, &security_hook_heads.ismaclabel, list)
+		if (hp->hook.ismaclabel(name) != 0)
+			return 1;
+	return 0;
 }
 EXPORT_SYMBOL(security_ismaclabel);
 
-- 
2.20.1


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

* [PATCH v7 06/16] LSM: Make multiple MAC modules safe in nfs and kernfs
  2019-08-07 22:42 [PATCH v7 00/16] LSM: Full module stacking Casey Schaufler
                   ` (5 preceding siblings ...)
  2019-08-07 22:42 ` [PATCH v7 05/16] LSM: Multiple modules using security_ismaclabel Casey Schaufler
@ 2019-08-07 22:42 ` Casey Schaufler
  2019-08-07 22:42 ` [PATCH v7 07/16] LSM: Correct handling of ENOSYS in inode_setxattr Casey Schaufler
                   ` (9 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: Casey Schaufler @ 2019-08-07 22:42 UTC (permalink / raw)
  To: casey.schaufler, jmorris, linux-security-module, selinux
  Cc: casey, keescook, john.johansen, penguin-kernel, paul, sds

Add use of "compound" security contexts to kernfs so that
multiple security modules using contexts can be represented
in the internal kernfs data. Disambiguate which security
module will be represented in NFS4.2 transactions by using only
the first encountered ismaclabel hook.

Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
---
 fs/kernfs/inode.c        |   3 +-
 fs/nfs/inode.c           |   9 +-
 fs/nfsd/nfs4proc.c       |   6 +-
 fs/nfsd/vfs.c            |   5 +-
 include/linux/security.h |  10 ++-
 security/security.c      | 179 ++++++++++++++++++++++++++-------------
 6 files changed, 143 insertions(+), 69 deletions(-)

diff --git a/fs/kernfs/inode.c b/fs/kernfs/inode.c
index ffbf7863306d..cd225121aff7 100644
--- a/fs/kernfs/inode.c
+++ b/fs/kernfs/inode.c
@@ -185,8 +185,7 @@ static void kernfs_refresh_inode(struct kernfs_node *kn, struct inode *inode)
 		 * persistent copy in kernfs_node.
 		 */
 		set_inode_attr(inode, &attrs->ia_iattr);
-		security_inode_notifysecctx(inode, attrs->ia_context.context,
-					    attrs->ia_context.len);
+		security_inode_notifysecctx(inode, &attrs->ia_context);
 	}
 
 	if (kernfs_type(kn) == KERNFS_DIR)
diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
index 414a90d48493..8acc5eef4d08 100644
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -341,13 +341,16 @@ void nfs_setsecurity(struct inode *inode, struct nfs_fattr *fattr,
 					struct nfs4_label *label)
 {
 	int error;
+	struct lsmcontext context = { .slot = LSMBLOB_FIRST };
 
 	if (label == NULL)
 		return;
 
-	if ((fattr->valid & NFS_ATTR_FATTR_V4_SECURITY_LABEL) && inode->i_security) {
-		error = security_inode_notifysecctx(inode, label->label,
-				label->len);
+	if ((fattr->valid & NFS_ATTR_FATTR_V4_SECURITY_LABEL) &&
+	    inode->i_security) {
+		context.context = label->label;
+		context.len = label->len;
+		error = security_inode_notifysecctx(inode, &context);
 		if (error)
 			printk(KERN_ERR "%s() %s %d "
 					"security_inode_notifysecctx() %d\n",
diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 0cfd257ffdaf..0f166c81f596 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -54,12 +54,14 @@
 static inline void
 nfsd4_security_inode_setsecctx(struct svc_fh *resfh, struct xdr_netobj *label, u32 *bmval)
 {
+	struct lsmcontext context = { .slot = LSMBLOB_FIRST };
 	struct inode *inode = d_inode(resfh->fh_dentry);
 	int status;
 
 	inode_lock(inode);
-	status = security_inode_setsecctx(resfh->fh_dentry,
-		label->data, label->len);
+	context.context = label->data;
+	context.len = label->len;
+	status = security_inode_setsecctx(resfh->fh_dentry, &context);
 	inode_unlock(inode);
 
 	if (status)
diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index 7dc98e14655d..274a998cc123 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -531,6 +531,7 @@ __be32 nfsd4_set_nfs4_label(struct svc_rqst *rqstp, struct svc_fh *fhp,
 	__be32 error;
 	int host_error;
 	struct dentry *dentry;
+	struct lsmcontext context = { .slot = LSMBLOB_FIRST };
 
 	error = fh_verify(rqstp, fhp, 0 /* S_IFREG */, NFSD_MAY_SATTR);
 	if (error)
@@ -539,7 +540,9 @@ __be32 nfsd4_set_nfs4_label(struct svc_rqst *rqstp, struct svc_fh *fhp,
 	dentry = fhp->fh_dentry;
 
 	inode_lock(d_inode(dentry));
-	host_error = security_inode_setsecctx(dentry, label->data, label->len);
+	context.context = label->data;
+	context.len = label->len;
+	host_error = security_inode_setsecctx(dentry, &context);
 	inode_unlock(d_inode(dentry));
 	return nfserrno(host_error);
 }
diff --git a/include/linux/security.h b/include/linux/security.h
index 0665a27a2891..2f442746dede 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -493,8 +493,8 @@ int security_secctx_to_secid(const char *secdata, u32 seclen,
 void security_release_secctx(struct lsmcontext *cp);
 
 void security_inode_invalidate_secctx(struct inode *inode);
-int security_inode_notifysecctx(struct inode *inode, void *ctx, u32 ctxlen);
-int security_inode_setsecctx(struct dentry *dentry, void *ctx, u32 ctxlen);
+int security_inode_notifysecctx(struct inode *inode, struct lsmcontext *cp);
+int security_inode_setsecctx(struct dentry *dentry, struct lsmcontext *cp);
 int security_inode_getsecctx(struct inode *inode, struct lsmcontext *cp);
 #else /* CONFIG_SECURITY */
 
@@ -1288,11 +1288,13 @@ static inline void security_inode_invalidate_secctx(struct inode *inode)
 {
 }
 
-static inline int security_inode_notifysecctx(struct inode *inode, void *ctx, u32 ctxlen)
+static inline int security_inode_notifysecctx(struct inode *inode,
+					      struct lsmcontext *cp);
 {
 	return -EOPNOTSUPP;
 }
-static inline int security_inode_setsecctx(struct dentry *dentry, void *ctx, u32 ctxlen)
+static inline int security_inode_setsecctx(struct dentry *dentry,
+					   struct lsmcontext *cp)
 {
 	return -EOPNOTSUPP;
 }
diff --git a/security/security.c b/security/security.c
index 13102d16bf2c..c71ddae6760e 100644
--- a/security/security.c
+++ b/security/security.c
@@ -743,6 +743,42 @@ static int lsm_superblock_alloc(struct super_block *sb)
 	return 0;
 }
 
+/**
+ * append_ctx - append a lsm/context pair to a compound context
+ * @ctx: the existing compound context
+ * @ctxlen: size of the old context, including terminating nul byte
+ * @lsm: new lsm name, nul terminated
+ * @new: new context, possibly nul terminated
+ * @newlen: maximum size of @new
+ *
+ * replace @ctx with a new compound context, appending @newlsm and @new
+ * to @ctx. On exit the new data replaces the old, which is freed.
+ * @ctxlen is set to the new size, which includes a trailing nul byte.
+ *
+ * Returns 0 on success, -ENOMEM if no memory is available.
+ */
+static int append_ctx(char **ctx, int *ctxlen, const char *lsm, char *new,
+		      int newlen)
+{
+	char *final;
+	int llen;
+
+	llen = strlen(lsm) + 1;
+	newlen = strnlen(new, newlen) + 1;
+
+	final = kzalloc(*ctxlen + llen + newlen, GFP_KERNEL);
+	if (final == NULL)
+		return -ENOMEM;
+	if (*ctxlen)
+		memcpy(final, *ctx, *ctxlen);
+	memcpy(final + *ctxlen, lsm, llen);
+	memcpy(final + *ctxlen + llen, new, newlen);
+	kfree(*ctx);
+	*ctx = final;
+	*ctxlen = *ctxlen + llen + newlen;
+	return 0;
+}
+
 /*
  * Hook list operation macros.
  *
@@ -2083,12 +2119,8 @@ int security_getprocattr(struct task_struct *p, const char *lsm, char *name,
 	struct security_hook_list *hp;
 	char *final = NULL;
 	char *cp;
-	char *tp;
 	int rc = 0;
 	int finallen = 0;
-	int llen;
-	int clen;
-	int tlen;
 	int display = lsm_task_display(current);
 	int slot = 0;
 
@@ -2116,26 +2148,12 @@ int security_getprocattr(struct task_struct *p, const char *lsm, char *name,
 				kfree(final);
 				return rc;
 			}
-			llen = strlen(hp->lsmid->lsm) + 1;
-			clen = strlen(cp) + 1;
-			tlen = llen + clen;
-			if (final)
-				tlen += finallen;
-			tp = kzalloc(tlen, GFP_KERNEL);
-			if (tp == NULL) {
-				kfree(cp);
+			rc = append_ctx(&final, &finallen, hp->lsmid->lsm,
+					cp, rc);
+			if (rc < 0) {
 				kfree(final);
-				return -ENOMEM;
+				return rc;
 			}
-			if (final)
-				memcpy(tp, final, finallen);
-			memcpy(tp + finallen, hp->lsmid->lsm, llen);
-			memcpy(tp + finallen + llen, cp, clen);
-			kfree(cp);
-			if (final)
-				kfree(final);
-			final = tp;
-			finallen = tlen;
 		}
 		if (final == NULL)
 			return -EINVAL;
@@ -2210,13 +2228,22 @@ int security_netlink_send(struct sock *sk, struct sk_buff *skb)
 	return call_int_hook(netlink_send, 0, sk, skb);
 }
 
+/**
+ * security_ismaclabel - Does @name identify a MAC attribute
+ * @name: attribute name in question
+ *
+ * If @name is the name of a Mandatory Access Control (MAC) attribute
+ * that the first module on the list recognizes return 1. Don't look
+ * beyond the first module, as this is only used by NFS and NFS can't
+ * differentiate which module to use.
+ */
 int security_ismaclabel(const char *name)
 {
 	struct security_hook_list *hp;
 
 	hlist_for_each_entry(hp, &security_hook_heads.ismaclabel, list)
-		if (hp->hook.ismaclabel(name) != 0)
-			return 1;
+		return hp->hook.ismaclabel(name);
+
 	return 0;
 }
 EXPORT_SYMBOL(security_ismaclabel);
@@ -2284,6 +2311,15 @@ void security_release_secctx(struct lsmcontext *cp)
 	struct security_hook_list *hp;
 	bool found = false;
 
+	if (cp->slot == LSMBLOB_INVALID)
+		return;
+
+	if (cp->slot == LSMBLOB_COMPOUND) {
+		kfree(cp->context);
+		found = true;
+		goto clear_out;
+	}
+
 	hlist_for_each_entry(hp, &security_hook_heads.release_secctx, list)
 		if (cp->slot == hp->lsmid->slot) {
 			hp->hook.release_secctx(cp->context, cp->len);
@@ -2291,6 +2327,7 @@ void security_release_secctx(struct lsmcontext *cp)
 			break;
 		}
 
+clear_out:
 	memset(cp, 0, sizeof(*cp));
 
 	if (!found)
@@ -2305,30 +2342,82 @@ void security_inode_invalidate_secctx(struct inode *inode)
 }
 EXPORT_SYMBOL(security_inode_invalidate_secctx);
 
-int security_inode_notifysecctx(struct inode *inode, void *ctx, u32 ctxlen)
+int security_inode_notifysecctx(struct inode *inode, struct lsmcontext *cp)
 {
-	return call_int_hook(inode_notifysecctx, 0, inode, ctx, ctxlen);
+	struct security_hook_list *hp;
+	char *raw = cp->context;
+	char *ctx;
+	int llen;
+	int clen;
+	int rc;
+
+	if (cp->slot == LSMBLOB_COMPOUND) {
+		hlist_for_each_entry(hp,
+		    &security_hook_heads.inode_notifysecctx, list) {
+			llen = strlen(raw) + 1;
+			ctx = raw + llen;
+			clen = strlen(ctx) + 1;
+			if (!strcmp(hp->lsmid->lsm, raw)) {
+				rc = hp->hook.inode_notifysecctx(inode, ctx,
+								 clen);
+				if (WARN_ON(rc != 0))
+					return rc;
+			}
+			raw = ctx + clen;
+		}
+		return 0;
+	}
+
+	hlist_for_each_entry(hp, &security_hook_heads.inode_notifysecctx, list)
+		if (cp->slot == LSMBLOB_FIRST || cp->slot == hp->lsmid->slot)
+			return hp->hook.inode_notifysecctx(inode, cp->context,
+							   cp->len);
+	return 0;
 }
 EXPORT_SYMBOL(security_inode_notifysecctx);
 
-int security_inode_setsecctx(struct dentry *dentry, void *ctx, u32 ctxlen)
+int security_inode_setsecctx(struct dentry *dentry, struct lsmcontext *cp)
 {
-	return call_int_hook(inode_setsecctx, 0, dentry, ctx, ctxlen);
+	struct security_hook_list *hp;
+
+	if (WARN_ON(cp->slot != LSMBLOB_FIRST))
+		return -EINVAL;
+
+	hlist_for_each_entry(hp, &security_hook_heads.inode_setsecctx, list)
+		return hp->hook.inode_setsecctx(dentry, cp->context, cp->len);
+	return 0;
 }
 EXPORT_SYMBOL(security_inode_setsecctx);
 
 int security_inode_getsecctx(struct inode *inode, struct lsmcontext *cp)
 {
 	struct security_hook_list *hp;
+	char *finalctx = NULL;
+	int rc = -EOPNOTSUPP;
+	int finallen = 0;
 
 	memset(cp, 0, sizeof(*cp));
 
 	hlist_for_each_entry(hp, &security_hook_heads.inode_getsecctx, list) {
+		rc = hp->hook.inode_getsecctx(inode, (void **)&cp->context,
+					      &cp->len);
+		if (rc) {
+			kfree(finalctx);
+			return rc;
+		}
 		cp->slot = hp->lsmid->slot;
-		return hp->hook.inode_getsecctx(inode, (void **)&cp->context,
-						&cp->len);
+		rc = append_ctx(&finalctx, &finallen, hp->lsmid->lsm,
+				cp->context, cp->len);
+		security_release_secctx(cp);
+		if (rc) {
+			kfree(finalctx);
+			return rc;
+		}
 	}
-	return -EOPNOTSUPP;
+	cp->slot = LSMBLOB_COMPOUND;
+	cp->context = finalctx;
+	cp->len = finallen;
+	return 0;
 }
 EXPORT_SYMBOL(security_inode_getsecctx);
 
@@ -2433,12 +2522,9 @@ int security_socket_getpeersec_stream(struct socket *sock, char __user *optval,
 	struct security_hook_list *hp;
 	char *final = NULL;
 	char *cp;
-	char *tp;
 	int rc = 0;
 	unsigned finallen = 0;
-	unsigned llen;
 	unsigned clen = 0;
-	unsigned tlen;
 
 	switch (display) {
 	case LSMBLOB_DISPLAY:
@@ -2471,29 +2557,8 @@ int security_socket_getpeersec_stream(struct socket *sock, char __user *optval,
 				kfree(final);
 				return rc;
 			}
-			/*
-			 * Don't propogate trailing nul bytes.
-			 */
-			clen = strnlen(cp, clen) + 1;
-			llen = strlen(hp->lsmid->lsm) + 1;
-			tlen = llen + clen;
-			if (final)
-				tlen += finallen;
-			tp = kzalloc(tlen, GFP_KERNEL);
-			if (tp == NULL) {
-				kfree(cp);
-				kfree(final);
-				return -ENOMEM;
-			}
-			if (final)
-				memcpy(tp, final, finallen);
-			memcpy(tp + finallen, hp->lsmid->lsm, llen);
-			memcpy(tp + finallen + llen, cp, clen);
-			kfree(cp);
-			if (final)
-				kfree(final);
-			final = tp;
-			finallen = tlen;
+			rc = append_ctx(&final, &finallen, hp->lsmid->lsm,
+					cp, clen);
 		}
 		if (final == NULL)
 			return -EINVAL;
-- 
2.20.1


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

* [PATCH v7 07/16] LSM: Correct handling of ENOSYS in inode_setxattr
  2019-08-07 22:42 [PATCH v7 00/16] LSM: Full module stacking Casey Schaufler
                   ` (6 preceding siblings ...)
  2019-08-07 22:42 ` [PATCH v7 06/16] LSM: Make multiple MAC modules safe in nfs and kernfs Casey Schaufler
@ 2019-08-07 22:42 ` Casey Schaufler
  2019-08-07 22:42 ` [PATCH v7 08/16] LSM: Infrastructure security blobs for mount options Casey Schaufler
                   ` (8 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: Casey Schaufler @ 2019-08-07 22:42 UTC (permalink / raw)
  To: casey.schaufler, jmorris, linux-security-module, selinux
  Cc: casey, keescook, john.johansen, penguin-kernel, paul, sds

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 c71ddae6760e..e3ea48c87dba 100644
--- a/security/security.c
+++ b/security/security.c
@@ -1397,24 +1397,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 5e7d61754798..021694b4aca7 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -3125,13 +3125,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 341a9927ed5c..f253d569dee6 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -1264,7 +1264,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;
@@ -1278,11 +1278,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.20.1


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

* [PATCH v7 08/16] LSM: Infrastructure security blobs for mount options
  2019-08-07 22:42 [PATCH v7 00/16] LSM: Full module stacking Casey Schaufler
                   ` (7 preceding siblings ...)
  2019-08-07 22:42 ` [PATCH v7 07/16] LSM: Correct handling of ENOSYS in inode_setxattr Casey Schaufler
@ 2019-08-07 22:42 ` Casey Schaufler
  2019-08-07 22:42 ` [PATCH v7 09/16] LSM: Fix for security_init_inode_security Casey Schaufler
                   ` (7 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: Casey Schaufler @ 2019-08-07 22:42 UTC (permalink / raw)
  To: casey.schaufler, jmorris, linux-security-module, selinux
  Cc: casey, keescook, john.johansen, penguin-kernel, paul, sds

Manage LSM data for mount options in the infrastructure
rather than in the individual modules. This allows multiple
security modules to provide mount options.

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 b0f788bf82b6..a54a2f4788af 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -2060,6 +2060,7 @@ struct lsm_blob_sizes {
 	int	lbs_key;
 	int	lbs_msg_msg;
 	int	lbs_task;
+	int	lbs_mnt_opts;
 };
 
 /*
@@ -2148,4 +2149,8 @@ static inline int lsm_task_display(struct task_struct *task)
 	return LSMBLOB_INVALID;
 }
 
+#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 e3ea48c87dba..6dbc7ed2a00d 100644
--- a/security/security.c
+++ b/security/security.c
@@ -183,6 +183,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);
@@ -321,6 +322,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);
@@ -779,6 +781,21 @@ static int append_ctx(char **ctx, int *ctxlen, const char *lsm, char *new,
 	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.
  *
@@ -974,6 +991,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 021694b4aca7..65bd62dca9e9 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;
 }
@@ -2645,10 +2651,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 {
@@ -2671,7 +2675,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;
@@ -6640,6 +6644,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 f253d569dee6..a9fb5f53a248 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -557,26 +557,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;
@@ -724,7 +731,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 {
@@ -767,7 +773,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)
@@ -4561,6 +4567,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.20.1


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

* [PATCH v7 09/16] LSM: Fix for security_init_inode_security
  2019-08-07 22:42 [PATCH v7 00/16] LSM: Full module stacking Casey Schaufler
                   ` (8 preceding siblings ...)
  2019-08-07 22:42 ` [PATCH v7 08/16] LSM: Infrastructure security blobs for mount options Casey Schaufler
@ 2019-08-07 22:42 ` Casey Schaufler
  2019-08-07 22:42 ` [PATCH v7 10/16] LSM: Change error detection for UDP peer security Casey Schaufler
                   ` (6 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: Casey Schaufler @ 2019-08-07 22:42 UTC (permalink / raw)
  To: casey.schaufler, jmorris, linux-security-module, selinux
  Cc: casey, keescook, john.johansen, penguin-kernel, paul, sds

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 6dbc7ed2a00d..325e745ac8f5 100644
--- a/security/security.c
+++ b/security/security.c
@@ -1158,11 +1158,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;
 
@@ -1173,18 +1186,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.20.1


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

* [PATCH v7 10/16] LSM: Change error detection for UDP peer security
  2019-08-07 22:42 [PATCH v7 00/16] LSM: Full module stacking Casey Schaufler
                   ` (9 preceding siblings ...)
  2019-08-07 22:42 ` [PATCH v7 09/16] LSM: Fix for security_init_inode_security Casey Schaufler
@ 2019-08-07 22:42 ` Casey Schaufler
  2019-08-07 22:42 ` [PATCH v7 11/16] Netlabel: Add a secattr comparison API function Casey Schaufler
                   ` (5 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: Casey Schaufler @ 2019-08-07 22:42 UTC (permalink / raw)
  To: casey.schaufler, jmorris, linux-security-module, selinux
  Cc: casey, keescook, john.johansen, penguin-kernel, paul, sds

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        | 12 ++++--------
 security/selinux/hooks.c   | 10 ++++++----
 security/smack/smack_lsm.c | 15 +++++++++------
 6 files changed, 26 insertions(+), 29 deletions(-)

diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index a54a2f4788af..67797c67093b 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -881,7 +881,6 @@
  *	@sock contains the peer socket. May be NULL.
  *	@skb is the sk_buff for the packet being queried. May be NULL.
  *	@secid pointer to store the secid of the packet.
- *	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_sock_rcv_skb)(struct sock *sk, struct sk_buff *skb);
 	int (*socket_getpeersec_stream)(struct socket *sock, char **optval,
 					int *optlen, unsigned len);
-	int (*socket_getpeersec_dgram)(struct socket *sock,
+	void (*socket_getpeersec_dgram)(struct socket *sock,
 					struct sk_buff *skb, u32 *secid);
 	int (*sk_alloc_security)(struct sock *sk, int family, gfp_t priority);
 	void (*sk_free_security)(struct sock *sk);
diff --git a/include/linux/security.h b/include/linux/security.h
index 2f442746dede..0e699d4ed13a 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -1329,8 +1329,8 @@ 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 display);
-int security_socket_getpeersec_dgram(struct socket *sock, struct sk_buff *skb,
-				     struct lsmblob *blob);
+void security_socket_getpeersec_dgram(struct socket *sock, struct sk_buff *skb,
+				      struct lsmblob *blob);
 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);
@@ -1470,11 +1470,10 @@ static inline int security_socket_getpeersec_stream(struct socket *sock,
 	return -ENOPROTOOPT;
 }
 
-static inline int security_socket_getpeersec_dgram(struct socket *sock,
-						   struct sk_buff *skb,
-						   struct lsmblob *blob)
+static inline void security_socket_getpeersec_dgram(struct socket *sock,
+						    struct sk_buff *skb,
+						    struct lsmblob *blob)
 {
-	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 447fe60af0cd..c28cbb15cee2 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 lsmblob lb;
 	int err;
 
-	err = security_socket_getpeersec_dgram(NULL, skb, &lb);
-	if (err)
-		return;
+	security_socket_getpeersec_dgram(NULL, skb, &lb);
 
 	err = security_secid_to_secctx(&lb, &context, LSMBLOB_DISPLAY);
 	if (err)
diff --git a/security/security.c b/security/security.c
index 325e745ac8f5..e726fc7c6712 100644
--- a/security/security.c
+++ b/security/security.c
@@ -2612,22 +2612,18 @@ int security_socket_getpeersec_stream(struct socket *sock, char __user *optval,
 	return rc;
 }
 
-int security_socket_getpeersec_dgram(struct socket *sock, struct sk_buff *skb,
-				     struct lsmblob *blob)
+void security_socket_getpeersec_dgram(struct socket *sock, struct sk_buff *skb,
+				      struct lsmblob *blob)
 {
 	struct security_hook_list *hp;
-	int rc = -ENOPROTOOPT;
 
 	hlist_for_each_entry(hp, &security_hook_heads.socket_getpeersec_dgram,
 			     list) {
 		if (WARN_ON(hp->lsmid->slot < 0 || hp->lsmid->slot >= lsm_slot))
 			continue;
-		rc = hp->hook.socket_getpeersec_dgram(sock, skb,
-						&blob->secid[hp->lsmid->slot]);
-		if (rc != 0)
-			break;
+		hp->hook.socket_getpeersec_dgram(sock, skb,
+						 &blob->secid[hp->lsmid->slot]);
 	}
-	return rc;
 }
 EXPORT_SYMBOL(security_socket_getpeersec_dgram);
 
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 65bd62dca9e9..91ef2ae77abb 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -4954,7 +4954,8 @@ static int selinux_socket_getpeersec_stream(struct socket *sock, char **optval,
 	return err;
 }
 
-static int selinux_socket_getpeersec_dgram(struct socket *sock, struct sk_buff *skb, u32 *secid)
+static void selinux_socket_getpeersec_dgram(struct socket *sock,
+					    struct sk_buff *skb, u32 *secid)
 {
 	u32 peer_secid = SECSID_NULL;
 	u16 family;
@@ -4977,9 +4978,7 @@ static int selinux_socket_getpeersec_dgram(struct socket *sock, struct sk_buff *
 
 out:
 	*secid = 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)
@@ -6321,6 +6320,9 @@ static int selinux_ismaclabel(const char *name)
 
 static int selinux_secid_to_secctx(u32 secid, char **secdata, u32 *seclen)
 {
+	if (secid == SECSID_NULL)
+		return -EINVAL;
+
 	return security_sid_to_context(&selinux_state, secid,
 				       secdata, seclen);
 }
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index a9fb5f53a248..2d88983868e8 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -3970,8 +3970,8 @@ static int smack_socket_getpeersec_stream(struct socket *sock, char **optval,
  *
  * Sets the netlabel socket state on sk from parent
  */
-static int smack_socket_getpeersec_dgram(struct socket *sock,
-					 struct sk_buff *skb, u32 *secid)
+static void smack_socket_getpeersec_dgram(struct socket *sock,
+					  struct sk_buff *skb, u32 *secid)
 
 {
 	struct netlbl_lsm_secattr secattr;
@@ -4025,9 +4025,7 @@ static int smack_socket_getpeersec_dgram(struct socket *sock,
 		break;
 	}
 	*secid = s;
-	if (s == 0)
-		return -EINVAL;
-	return 0;
+	return;
 }
 
 /**
@@ -4426,7 +4424,12 @@ static int smack_ismaclabel(const char *name)
  */
 static int smack_secid_to_secctx(u32 secid, char **secdata, u32 *seclen)
 {
-	struct smack_known *skp = smack_from_secid(secid);
+	struct smack_known *skp;
+
+	if (secid == 0)
+		return -EINVAL;
+
+	skp = smack_from_secid(secid);
 
 	if (secdata)
 		*secdata = skp->smk_known;
-- 
2.20.1


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

* [PATCH v7 11/16] Netlabel: Add a secattr comparison API function
  2019-08-07 22:42 [PATCH v7 00/16] LSM: Full module stacking Casey Schaufler
                   ` (10 preceding siblings ...)
  2019-08-07 22:42 ` [PATCH v7 10/16] LSM: Change error detection for UDP peer security Casey Schaufler
@ 2019-08-07 22:42 ` Casey Schaufler
  2019-08-07 22:42 ` [PATCH v7 12/16] Netlabel: Provide labeling type to security modules Casey Schaufler
                   ` (4 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: Casey Schaufler @ 2019-08-07 22:42 UTC (permalink / raw)
  To: casey.schaufler, jmorris, linux-security-module, selinux
  Cc: casey, keescook, john.johansen, penguin-kernel, paul, sds

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 6c550455e69f..fc4fca7d65d3 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 724d44943543..a0996bdc8595 100644
--- a/net/netlabel/netlabel_kapi.c
+++ b/net/netlabel/netlabel_kapi.c
@@ -1462,6 +1462,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 security blobs, 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.20.1


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

* [PATCH v7 12/16] Netlabel: Provide labeling type to security modules
  2019-08-07 22:42 [PATCH v7 00/16] LSM: Full module stacking Casey Schaufler
                   ` (11 preceding siblings ...)
  2019-08-07 22:42 ` [PATCH v7 11/16] Netlabel: Add a secattr comparison API function Casey Schaufler
@ 2019-08-07 22:42 ` Casey Schaufler
  2019-08-07 22:42 ` [PATCH v7 13/16] LSM: Remember the NLTYPE of netlabel sockets Casey Schaufler
                   ` (3 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: Casey Schaufler @ 2019-08-07 22:42 UTC (permalink / raw)
  To: casey.schaufler, jmorris, linux-security-module, selinux
  Cc: casey, keescook, john.johansen, penguin-kernel, paul, sds

Return the labeling type when setting network security attributes.
This allows for later comparison of the complete label information
to determine if the security modules agree on how a packet should
be labeled.

Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
---
 net/netlabel/netlabel_kapi.c | 70 +++++++++++++++++++++---------------
 security/selinux/netlabel.c  | 23 +++++++-----
 security/smack/smack_lsm.c   |  8 +++--
 3 files changed, 61 insertions(+), 40 deletions(-)

diff --git a/net/netlabel/netlabel_kapi.c b/net/netlabel/netlabel_kapi.c
index a0996bdc8595..496d6a38b2aa 100644
--- a/net/netlabel/netlabel_kapi.c
+++ b/net/netlabel/netlabel_kapi.c
@@ -975,15 +975,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;
 
@@ -995,17 +994,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;
@@ -1013,17 +1012,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;
@@ -1104,14 +1103,16 @@ int netlbl_sock_getattr(struct sock *sk,
  * Description:
  * Attach the correct label to the given connected socket using the security
  * attributes specified in @secattr.  The caller is responsible for ensuring
- * that @sk is locked.  Returns zero on success, negative values on failure.
+ * that @sk is locked.  Returns the NLTYPE on success, negative values on
+ * failure.
  *
  */
 int netlbl_conn_setattr(struct sock *sk,
 			struct sockaddr *addr,
 			const struct netlbl_lsm_secattr *secattr)
 {
-	int ret_val;
+	int rc;
+	int ret_val = 0;
 	struct sockaddr_in *addr4;
 #if IS_ENABLED(CONFIG_IPV6)
 	struct sockaddr_in6 *addr6;
@@ -1128,16 +1129,17 @@ int netlbl_conn_setattr(struct sock *sk,
 			ret_val = -ENOENT;
 			goto conn_setattr_return;
 		}
+		ret_val = entry->type;
 		switch (entry->type) {
 		case NETLBL_NLTYPE_CIPSOV4:
-			ret_val = cipso_v4_sock_setattr(sk,
-							entry->cipso, secattr);
+			rc = cipso_v4_sock_setattr(sk, entry->cipso, secattr);
+			if (rc < 0)
+				ret_val = rc;
 			break;
 		case NETLBL_NLTYPE_UNLABELED:
 			/* just delete the protocols we support for right now
 			 * but we could remove other protocols if needed */
 			netlbl_sock_delattr(sk);
-			ret_val = 0;
 			break;
 		default:
 			ret_val = -ENOENT;
@@ -1152,16 +1154,17 @@ int netlbl_conn_setattr(struct sock *sk,
 			ret_val = -ENOENT;
 			goto conn_setattr_return;
 		}
+		ret_val = entry->type;
 		switch (entry->type) {
 		case NETLBL_NLTYPE_CALIPSO:
-			ret_val = calipso_sock_setattr(sk,
-						       entry->calipso, secattr);
+			rc = calipso_sock_setattr(sk, entry->calipso, secattr);
+			if (rc < 0)
+				ret_val = rc;
 			break;
 		case NETLBL_NLTYPE_UNLABELED:
 			/* just delete the protocols we support for right now
 			 * but we could remove other protocols if needed */
 			netlbl_sock_delattr(sk);
-			ret_val = 0;
 			break;
 		default:
 			ret_val = -ENOENT;
@@ -1184,12 +1187,14 @@ int netlbl_conn_setattr(struct sock *sk,
  *
  * Description:
  * Attach the correct label to the given socket using the security attributes
- * specified in @secattr.  Returns zero on success, negative values on failure.
+ * specified in @secattr.  Returns the NLTYPE on success, negative values on
+ * failure.
  *
  */
 int netlbl_req_setattr(struct request_sock *req,
 		       const struct netlbl_lsm_secattr *secattr)
 {
+	int rc;
 	int ret_val;
 	struct netlbl_dommap_def *entry;
 	struct inet_request_sock *ireq = inet_rsk(req);
@@ -1203,14 +1208,15 @@ int netlbl_req_setattr(struct request_sock *req,
 			ret_val = -ENOENT;
 			goto req_setattr_return;
 		}
+		ret_val = entry->type;
 		switch (entry->type) {
 		case NETLBL_NLTYPE_CIPSOV4:
-			ret_val = cipso_v4_req_setattr(req,
-						       entry->cipso, secattr);
+			rc = cipso_v4_req_setattr(req, entry->cipso, secattr);
+			if (rc < 0)
+				ret_val = rc;
 			break;
 		case NETLBL_NLTYPE_UNLABELED:
 			netlbl_req_delattr(req);
-			ret_val = 0;
 			break;
 		default:
 			ret_val = -ENOENT;
@@ -1224,14 +1230,15 @@ int netlbl_req_setattr(struct request_sock *req,
 			ret_val = -ENOENT;
 			goto req_setattr_return;
 		}
+		ret_val = entry->type;
 		switch (entry->type) {
 		case NETLBL_NLTYPE_CALIPSO:
-			ret_val = calipso_req_setattr(req,
-						      entry->calipso, secattr);
+			rc = calipso_req_setattr(req, entry->calipso, secattr);
+			if (rc < 0)
+				ret_val = rc;
 			break;
 		case NETLBL_NLTYPE_UNLABELED:
 			netlbl_req_delattr(req);
-			ret_val = 0;
 			break;
 		default:
 			ret_val = -ENOENT;
@@ -1277,7 +1284,8 @@ void netlbl_req_delattr(struct request_sock *req)
  *
  * Description:
  * Attach the correct label to the given packet using the security attributes
- * specified in @secattr.  Returns zero on success, negative values on failure.
+ * specified in @secattr.  Returns the NLTYPE on success, negative values on
+ * failure.
  *
  */
 int netlbl_skbuff_setattr(struct sk_buff *skb,
@@ -1314,6 +1322,8 @@ int netlbl_skbuff_setattr(struct sk_buff *skb,
 		default:
 			ret_val = -ENOENT;
 		}
+		if (ret_val == 0)
+			ret_val = entry->type;
 		break;
 #if IS_ENABLED(CONFIG_IPV6)
 	case AF_INET6:
@@ -1337,6 +1347,8 @@ int netlbl_skbuff_setattr(struct sk_buff *skb,
 		default:
 			ret_val = -ENOENT;
 		}
+		if (ret_val == 0)
+			ret_val = entry->type;
 		break;
 #endif /* IPv6 */
 	default:
diff --git a/security/selinux/netlabel.c b/security/selinux/netlabel.c
index 120d50c1bcac..8088a787777a 100644
--- a/security/selinux/netlabel.c
+++ b/security/selinux/netlabel.c
@@ -266,6 +266,8 @@ int selinux_netlbl_skbuff_setsid(struct sk_buff *skb,
 	}
 
 	rc = netlbl_skbuff_setattr(skb, family, secattr);
+	if (rc > 0)
+		rc = 0;
 
 skbuff_setsid_return:
 	if (secattr == &secattr_storage)
@@ -321,8 +323,10 @@ int selinux_netlbl_sctp_assoc_request(struct sctp_endpoint *ep,
 	}
 
 	rc = netlbl_conn_setattr(ep->base.sk, addr, &secattr);
-	if (rc == 0)
+	if (rc >= 0) {
 		sksec->nlbl_state = NLBL_LABELED;
+		rc = 0;
+	}
 
 assoc_request_return:
 	netlbl_secattr_destroy(&secattr);
@@ -354,6 +358,8 @@ int selinux_netlbl_inet_conn_request(struct request_sock *req, u16 family)
 	if (rc != 0)
 		goto inet_conn_request_return;
 	rc = netlbl_req_setattr(req, &secattr);
+	if (rc > 0)
+		rc = 0;
 inet_conn_request_return:
 	netlbl_secattr_destroy(&secattr);
 	return rc;
@@ -418,15 +424,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;
 }
@@ -579,8 +582,10 @@ static int selinux_netlbl_socket_connect_helper(struct sock *sk,
 		return rc;
 	}
 	rc = netlbl_conn_setattr(sk, addr, secattr);
-	if (rc == 0)
+	if (rc >= 0) {
 		sksec->nlbl_state = NLBL_CONNLABELED;
+		rc = 0;
+	}
 
 	return rc;
 }
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 2d88983868e8..62189558bb6a 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -2414,6 +2414,8 @@ static int smack_netlabel(struct sock *sk, int labeled)
 	else {
 		skp = ssp->smk_out;
 		rc = netlbl_sock_setattr(sk, sk->sk_family, &skp->smk_netlabel);
+		if (rc > 0)
+			rc = 0;
 	}
 
 	bh_unlock_sock(sk);
@@ -4141,9 +4143,11 @@ static int smack_inet_conn_request(struct sock *sk, struct sk_buff *skb,
 	hskp = smack_ipv4host_label(&addr);
 	rcu_read_unlock();
 
-	if (hskp == NULL)
+	if (hskp == NULL) {
 		rc = netlbl_req_setattr(req, &skp->smk_netlabel);
-	else
+		if (rc > 0)
+			rc = 0;
+	} else
 		netlbl_req_delattr(req);
 
 	return rc;
-- 
2.20.1


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

* [PATCH v7 13/16] LSM: Remember the NLTYPE of netlabel sockets
  2019-08-07 22:42 [PATCH v7 00/16] LSM: Full module stacking Casey Schaufler
                   ` (12 preceding siblings ...)
  2019-08-07 22:42 ` [PATCH v7 12/16] Netlabel: Provide labeling type to security modules Casey Schaufler
@ 2019-08-07 22:42 ` Casey Schaufler
  2019-08-07 22:42 ` [PATCH v7 14/16] LSM: Hook for netlabel reconciliation Casey Schaufler
                   ` (2 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: Casey Schaufler @ 2019-08-07 22:42 UTC (permalink / raw)
  To: casey.schaufler, jmorris, linux-security-module, selinux
  Cc: casey, keescook, john.johansen, penguin-kernel, paul, sds

Add the NLTYPE returned when setting labels on sockets
to the information retained by SELinux and Smack.

Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
---
 security/selinux/include/objsec.h |  1 +
 security/selinux/netlabel.c       | 20 ++++++++++++++------
 security/smack/smack.h            |  1 +
 security/smack/smack_lsm.c        | 17 ++++++++++++-----
 4 files changed, 28 insertions(+), 11 deletions(-)

diff --git a/security/selinux/include/objsec.h b/security/selinux/include/objsec.h
index 3b78aa4ee98f..5ab0d0d212bd 100644
--- a/security/selinux/include/objsec.h
+++ b/security/selinux/include/objsec.h
@@ -124,6 +124,7 @@ struct sk_security_struct {
 		NLBL_REQSKB,
 		NLBL_CONNLABELED,
 	} nlbl_state;
+	int nlbl_set;			/* Raw NLTYPE	*/
 	struct netlbl_lsm_secattr *nlbl_secattr; /* NetLabel sec attributes */
 #endif
 	u32 sid;			/* SID of this object */
diff --git a/security/selinux/netlabel.c b/security/selinux/netlabel.c
index 8088a787777a..56e780340114 100644
--- a/security/selinux/netlabel.c
+++ b/security/selinux/netlabel.c
@@ -185,6 +185,7 @@ void selinux_netlbl_sk_security_free(struct sk_security_struct *sksec)
 void selinux_netlbl_sk_security_reset(struct sk_security_struct *sksec)
 {
 	sksec->nlbl_state = NLBL_UNSET;
+	sksec->nlbl_set = NETLBL_NLTYPE_NONE;
 }
 
 /**
@@ -244,14 +245,14 @@ int selinux_netlbl_skbuff_setsid(struct sk_buff *skb,
 	int rc;
 	struct netlbl_lsm_secattr secattr_storage;
 	struct netlbl_lsm_secattr *secattr = NULL;
+	struct sk_security_struct *sksec;
 	struct sock *sk;
 
 	/* if this is a locally generated packet check to see if it is already
 	 * being labeled by it's parent socket, if it is just exit */
 	sk = skb_to_full_sk(skb);
 	if (sk != NULL) {
-		struct sk_security_struct *sksec = selinux_sock(sk);
-
+		sksec = selinux_sock(sk);
 		if (sksec->nlbl_state != NLBL_REQSKB)
 			return 0;
 		secattr = selinux_netlbl_sock_getattr(sk, sid);
@@ -266,8 +267,11 @@ int selinux_netlbl_skbuff_setsid(struct sk_buff *skb,
 	}
 
 	rc = netlbl_skbuff_setattr(skb, family, secattr);
-	if (rc > 0)
+	if (rc >= 0) {
+		if (sk != NULL)
+			sksec->nlbl_set = rc;
 		rc = 0;
+	}
 
 skbuff_setsid_return:
 	if (secattr == &secattr_storage)
@@ -325,6 +329,7 @@ int selinux_netlbl_sctp_assoc_request(struct sctp_endpoint *ep,
 	rc = netlbl_conn_setattr(ep->base.sk, addr, &secattr);
 	if (rc >= 0) {
 		sksec->nlbl_state = NLBL_LABELED;
+		sksec->nlbl_set = rc;
 		rc = 0;
 	}
 
@@ -428,8 +433,10 @@ int selinux_netlbl_socket_post_create(struct sock *sk, u16 family)
 		sksec->nlbl_state = NLBL_REQSKB;
 	else if (rc >= 0)
 		sksec->nlbl_state = NLBL_LABELED;
-	if (rc > 0)
+	if (rc >= 0) {
+		sksec->nlbl_set = rc;
 		rc = 0;
+	}
 
 	return rc;
 }
@@ -573,8 +580,8 @@ static int selinux_netlbl_socket_connect_helper(struct sock *sk,
 	if (addr->sa_family == AF_UNSPEC) {
 		netlbl_sock_delattr(sk);
 		sksec->nlbl_state = NLBL_REQSKB;
-		rc = 0;
-		return rc;
+		sksec->nlbl_set = NETLBL_NLTYPE_ADDRSELECT;
+		return 0;
 	}
 	secattr = selinux_netlbl_sock_genattr(sk);
 	if (secattr == NULL) {
@@ -584,6 +591,7 @@ static int selinux_netlbl_socket_connect_helper(struct sock *sk,
 	rc = netlbl_conn_setattr(sk, addr, secattr);
 	if (rc >= 0) {
 		sksec->nlbl_state = NLBL_CONNLABELED;
+		sksec->nlbl_set = rc;
 		rc = 0;
 	}
 
diff --git a/security/smack/smack.h b/security/smack/smack.h
index f28db5a42b7b..b531f7ea21a7 100644
--- a/security/smack/smack.h
+++ b/security/smack/smack.h
@@ -104,6 +104,7 @@ struct socket_smack {
 	struct smack_known	*smk_out;	/* outbound label */
 	struct smack_known	*smk_in;	/* inbound label */
 	struct smack_known	*smk_packet;	/* TCP peer label */
+	int			smk_set;	/* Netlabel NLTYPE */
 };
 
 /*
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 62189558bb6a..87c81cbc8c67 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -2409,13 +2409,16 @@ static int smack_netlabel(struct sock *sk, int labeled)
 	bh_lock_sock_nested(sk);
 
 	if (ssp->smk_out == smack_net_ambient ||
-	    labeled == SMACK_UNLABELED_SOCKET)
+	    labeled == SMACK_UNLABELED_SOCKET) {
 		netlbl_sock_delattr(sk);
-	else {
+		ssp->smk_set = NETLBL_NLTYPE_UNLABELED;
+	} else {
 		skp = ssp->smk_out;
 		rc = netlbl_sock_setattr(sk, sk->sk_family, &skp->smk_netlabel);
-		if (rc > 0)
+		if (rc >= 0) {
 			rc = 0;
+			ssp->smk_set = rc;
+		}
 	}
 
 	bh_unlock_sock(sk);
@@ -4145,10 +4148,14 @@ static int smack_inet_conn_request(struct sock *sk, struct sk_buff *skb,
 
 	if (hskp == NULL) {
 		rc = netlbl_req_setattr(req, &skp->smk_netlabel);
-		if (rc > 0)
+		if (rc >= 0) {
+			ssp->smk_set = rc;
 			rc = 0;
-	} else
+		}
+	} else {
 		netlbl_req_delattr(req);
+		rc = NETLBL_NLTYPE_UNLABELED;
+	}
 
 	return rc;
 }
-- 
2.20.1


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

* [PATCH v7 14/16] LSM: Hook for netlabel reconciliation
  2019-08-07 22:42 [PATCH v7 00/16] LSM: Full module stacking Casey Schaufler
                   ` (13 preceding siblings ...)
  2019-08-07 22:42 ` [PATCH v7 13/16] LSM: Remember the NLTYPE of netlabel sockets Casey Schaufler
@ 2019-08-07 22:42 ` Casey Schaufler
  2019-08-07 22:42 ` [PATCH v7 15/16] LSM: Avoid network conflicts in SELinux and Smack Casey Schaufler
  2019-08-07 22:42 ` [PATCH v7 16/16] Smack: Remove the exclusive flag Casey Schaufler
  16 siblings, 0 replies; 18+ messages in thread
From: Casey Schaufler @ 2019-08-07 22:42 UTC (permalink / raw)
  To: casey.schaufler, jmorris, linux-security-module, selinux
  Cc: casey, keescook, john.johansen, penguin-kernel, paul, sds

Add an LSM function security_reconcile_netlbl() which
uses the new LSM hook socket_netlbl_secattr() to decide
if the active security modules are in agreement regarding
the labeling of a network packet.

Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
---
 include/linux/lsm_hooks.h           | 15 +++++++++
 include/linux/security.h            |  9 ++++++
 security/security.c                 | 50 +++++++++++++++++++++++++++++
 security/selinux/hooks.c            |  3 ++
 security/selinux/include/netlabel.h |  7 ++++
 security/selinux/netlabel.c         |  9 ++++++
 security/smack/smack_lsm.c          |  9 ++++++
 7 files changed, 102 insertions(+)

diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 67797c67093b..4bf88fa5b55d 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -29,6 +29,9 @@
 #include <linux/init.h>
 #include <linux/rculist.h>
 
+#ifdef CONFIG_NETLABEL
+struct netlbl_lsm_secattr;
+#endif
 /**
  * union security_list_options - Linux Security Module hook function list
  *
@@ -1432,6 +1435,10 @@
  * @bpf_prog_free_security:
  *	Clean up the security information stored inside bpf prog.
  *
+ * Security hooks for network labeling (Netlabel) operations.
+ *
+ * @socket_netlbl_secattr:
+ *	Report the netlabel attributes this module wants for this socket.
  */
 union security_list_options {
 	int (*binder_set_context_mgr)(struct task_struct *mgr);
@@ -1788,6 +1795,11 @@ union security_list_options {
 	int (*bpf_prog_alloc_security)(struct bpf_prog_aux *aux);
 	void (*bpf_prog_free_security)(struct bpf_prog_aux *aux);
 #endif /* CONFIG_BPF_SYSCALL */
+#ifdef CONFIG_NETLABEL
+	void (*socket_netlbl_secattr)(struct sock *sk,
+				      struct netlbl_lsm_secattr **secattr,
+				      int *set);
+#endif
 };
 
 struct security_hook_heads {
@@ -2025,6 +2037,9 @@ struct security_hook_heads {
 	struct hlist_head bpf_prog_alloc_security;
 	struct hlist_head bpf_prog_free_security;
 #endif /* CONFIG_BPF_SYSCALL */
+#ifdef CONFIG_NETLABEL
+	struct hlist_head socket_netlbl_secattr;
+#endif
 } __randomize_layout;
 
 /*
diff --git a/include/linux/security.h b/include/linux/security.h
index 0e699d4ed13a..c234d881c206 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -1934,5 +1934,14 @@ static inline void security_bpf_prog_free(struct bpf_prog_aux *aux)
 #endif /* CONFIG_SECURITY */
 #endif /* CONFIG_BPF_SYSCALL */
 
+#ifdef CONFIG_NETLABEL
+extern int security_reconcile_netlbl(struct sock *sk);
+#else
+static inline int security_reconcile_netlbl(struct sock *sk)
+{
+	return 0;
+}
+#endif
+
 #endif /* ! __LINUX_SECURITY_H */
 
diff --git a/security/security.c b/security/security.c
index e726fc7c6712..bfe40c11f5bf 100644
--- a/security/security.c
+++ b/security/security.c
@@ -34,6 +34,9 @@
 #include <linux/binfmts.h>
 #include <net/flow.h>
 #include <net/sock.h>
+#ifdef CONFIG_NETLABEL
+#include <net/netlabel.h>
+#endif
 
 #define MAX_LSM_EVM_XATTR	2
 
@@ -3003,3 +3006,50 @@ void security_bpf_prog_free(struct bpf_prog_aux *aux)
 	call_void_hook(bpf_prog_free_security, aux);
 }
 #endif /* CONFIG_BPF_SYSCALL */
+
+#ifdef CONFIG_NETLABEL
+int security_reconcile_netlbl(struct sock *sk)
+{
+	struct netlbl_lsm_secattr *prev = NULL;
+	struct netlbl_lsm_secattr *this = NULL;
+	int prev_set = 0;
+	int this_set = 0;
+	struct security_hook_list *hp;
+
+	hlist_for_each_entry(hp, &security_hook_heads.socket_netlbl_secattr,
+				list) {
+		hp->hook.socket_netlbl_secattr(sk, &this, &this_set);
+		if (this_set == 0 || this == NULL)
+			continue;
+		if (prev != NULL) {
+			/*
+			 * Both unlabeled is easily acceptable.
+			 */
+			if (prev_set == NETLBL_NLTYPE_UNLABELED &&
+			    this_set == NETLBL_NLTYPE_UNLABELED)
+				continue;
+			/*
+			 * The nltype being different means that
+			 * the secattrs aren't comparible. Except
+			 * that ADDRSELECT means that couldn't know
+			 * when the socket was created.
+			 */
+			if (prev_set != this_set &&
+			    prev_set != NETLBL_NLTYPE_ADDRSELECT &&
+			    this_set != NETLBL_NLTYPE_ADDRSELECT)
+				return -EACCES;
+			/*
+			 * Count on the Netlabel system's judgement.
+			 */
+			if (!netlbl_secattr_equal(prev, this))
+				return -EACCES;
+		}
+		prev = this;
+		prev_set = this_set;
+	}
+	/*
+	 * No conflicts have been found.
+	 */
+	return 0;
+}
+#endif /* CONFIG_NETLABEL */
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 91ef2ae77abb..48468a4b478c 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -6887,6 +6887,9 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = {
 	LSM_HOOK_INIT(bpf_map_free_security, selinux_bpf_map_free),
 	LSM_HOOK_INIT(bpf_prog_free_security, selinux_bpf_prog_free),
 #endif
+#ifdef CONFIG_NETLABEL
+	LSM_HOOK_INIT(socket_netlbl_secattr, selinux_socket_netlbl_secattr),
+#endif
 };
 
 static __init int selinux_init(void)
diff --git a/security/selinux/include/netlabel.h b/security/selinux/include/netlabel.h
index 8671de09c363..b316c62e7bcc 100644
--- a/security/selinux/include/netlabel.h
+++ b/security/selinux/include/netlabel.h
@@ -69,6 +69,9 @@ int selinux_netlbl_socket_setsockopt(struct socket *sock,
 int selinux_netlbl_socket_connect(struct sock *sk, struct sockaddr *addr);
 int selinux_netlbl_socket_connect_locked(struct sock *sk,
 					 struct sockaddr *addr);
+void selinux_socket_netlbl_secattr(struct sock *sk,
+				   struct netlbl_lsm_secattr **secattr,
+				   int *set);
 
 #else
 static inline void selinux_netlbl_cache_invalidate(void)
@@ -165,6 +168,10 @@ static inline int selinux_netlbl_socket_connect_locked(struct sock *sk,
 {
 	return 0;
 }
+static inline void selinux_socket_netlbl_secattr(struct sock *sk,
+					struct netlbl_lsm_secattr **secattr)
+{
+}
 #endif /* CONFIG_NETLABEL */
 
 #endif
diff --git a/security/selinux/netlabel.c b/security/selinux/netlabel.c
index 56e780340114..0f50a646c8cd 100644
--- a/security/selinux/netlabel.c
+++ b/security/selinux/netlabel.c
@@ -642,3 +642,12 @@ int selinux_netlbl_socket_connect(struct sock *sk, struct sockaddr *addr)
 
 	return rc;
 }
+
+void selinux_socket_netlbl_secattr(struct sock *sk,
+				   struct netlbl_lsm_secattr **secattr,
+				   int *set)
+{
+	struct sk_security_struct *sksec = selinux_sock(sk);
+	*secattr = sksec->nlbl_secattr;
+	*set = sksec->nlbl_set;
+}
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 87c81cbc8c67..122c13604d28 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -4572,6 +4572,14 @@ static int smack_dentry_create_files_as(struct dentry *dentry, int mode,
 	}
 	return 0;
 }
+void smack_socket_netlbl_secattr(struct sock *sk,
+				 struct netlbl_lsm_secattr **secattr,
+				 int *set)
+{
+	struct socket_smack *ssp = smack_sock(sk);
+	*secattr = &ssp->smk_out->smk_netlabel;
+	*set = ssp->smk_set;
+}
 
 struct lsm_blob_sizes smack_blob_sizes __lsm_ro_after_init = {
 	.lbs_cred = sizeof(struct task_smack),
@@ -4733,6 +4741,7 @@ static struct security_hook_list smack_hooks[] __lsm_ro_after_init = {
 	LSM_HOOK_INIT(inode_copy_up, smack_inode_copy_up),
 	LSM_HOOK_INIT(inode_copy_up_xattr, smack_inode_copy_up_xattr),
 	LSM_HOOK_INIT(dentry_create_files_as, smack_dentry_create_files_as),
+	LSM_HOOK_INIT(socket_netlbl_secattr, smack_socket_netlbl_secattr),
 };
 
 
-- 
2.20.1


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

* [PATCH v7 15/16] LSM: Avoid network conflicts in SELinux and Smack
  2019-08-07 22:42 [PATCH v7 00/16] LSM: Full module stacking Casey Schaufler
                   ` (14 preceding siblings ...)
  2019-08-07 22:42 ` [PATCH v7 14/16] LSM: Hook for netlabel reconciliation Casey Schaufler
@ 2019-08-07 22:42 ` Casey Schaufler
  2019-08-07 22:42 ` [PATCH v7 16/16] Smack: Remove the exclusive flag Casey Schaufler
  16 siblings, 0 replies; 18+ messages in thread
From: Casey Schaufler @ 2019-08-07 22:42 UTC (permalink / raw)
  To: casey.schaufler, jmorris, linux-security-module, selinux
  Cc: casey, keescook, john.johansen, penguin-kernel, paul, sds

Add calls to security_reconcile_netlbl() in SELinux and
Smack to ensure that only packets that are acceptable to
all active security modules get sent. Verify that all
security modules agree on the network labeling for sendmsg
and connect.

Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
---
 security/security.c              | 43 ++++++++++++++++++++++----------
 security/selinux/hooks.c         |  3 +++
 security/smack/smack_netfilter.c |  8 ++++--
 3 files changed, 39 insertions(+), 15 deletions(-)

diff --git a/security/security.c b/security/security.c
index bfe40c11f5bf..4897c68cdb71 100644
--- a/security/security.c
+++ b/security/security.c
@@ -2496,7 +2496,13 @@ int security_socket_bind(struct socket *sock, struct sockaddr *address, int addr
 
 int security_socket_connect(struct socket *sock, struct sockaddr *address, int addrlen)
 {
-	return call_int_hook(socket_connect, 0, sock, address, addrlen);
+	int rc;
+
+	rc = call_int_hook(socket_connect, 0, sock, address, addrlen);
+	if (rc)
+		return rc;
+
+	return security_reconcile_netlbl(sock->sk);
 }
 
 int security_socket_listen(struct socket *sock, int backlog)
@@ -2511,6 +2517,12 @@ int security_socket_accept(struct socket *sock, struct socket *newsock)
 
 int security_socket_sendmsg(struct socket *sock, struct msghdr *msg, int size)
 {
+	int rc;
+
+	rc = security_reconcile_netlbl(sock->sk);
+	if (rc)
+		return rc;
+
 	return call_int_hook(socket_sendmsg, 0, sock, msg, size);
 }
 
@@ -3016,28 +3028,33 @@ int security_reconcile_netlbl(struct sock *sk)
 	int this_set = 0;
 	struct security_hook_list *hp;
 
+	if (sk->sk_family != PF_INET && sk->sk_family != PF_INET6)
+		return 0;
+
 	hlist_for_each_entry(hp, &security_hook_heads.socket_netlbl_secattr,
 				list) {
 		hp->hook.socket_netlbl_secattr(sk, &this, &this_set);
+		/*
+		 * If the NLTYPE has been deferred it's not
+		 * possible to decide now. A decision will be made
+		 * later.
+		 */
+		if (this_set == NETLBL_NLTYPE_ADDRSELECT)
+			return 0;
 		if (this_set == 0 || this == NULL)
 			continue;
 		if (prev != NULL) {
-			/*
-			 * Both unlabeled is easily acceptable.
-			 */
-			if (prev_set == NETLBL_NLTYPE_UNLABELED &&
-			    this_set == NETLBL_NLTYPE_UNLABELED)
-				continue;
 			/*
 			 * The nltype being different means that
-			 * the secattrs aren't comparible. Except
-			 * that ADDRSELECT means that couldn't know
-			 * when the socket was created.
+			 * the secattrs aren't comparible.
 			 */
-			if (prev_set != this_set &&
-			    prev_set != NETLBL_NLTYPE_ADDRSELECT &&
-			    this_set != NETLBL_NLTYPE_ADDRSELECT)
+			if (prev_set != this_set)
 				return -EACCES;
+			/*
+			 * Both unlabeled is easily acceptable.
+			 */
+			if (this_set == NETLBL_NLTYPE_UNLABELED)
+				continue;
 			/*
 			 * Count on the Netlabel system's judgement.
 			 */
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 48468a4b478c..293350b672a8 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -5522,6 +5522,9 @@ static unsigned int selinux_ip_output(struct sk_buff *skb,
 	if (selinux_netlbl_skbuff_setsid(skb, family, sid) != 0)
 		return NF_DROP;
 
+	if (sk && security_reconcile_netlbl(sk))
+		return NF_DROP;
+
 	return NF_ACCEPT;
 }
 
diff --git a/security/smack/smack_netfilter.c b/security/smack/smack_netfilter.c
index 7b9c8d5d8408..92aeffbbb27c 100644
--- a/security/smack/smack_netfilter.c
+++ b/security/smack/smack_netfilter.c
@@ -75,7 +75,7 @@ static unsigned int smack_ipv4_output(void *priv,
 					const struct nf_hook_state *state)
 {
 	struct sock *sk = skb_to_full_sk(skb);
-	struct socket_smack *ssp;
+	struct socket_smack *ssp = NULL;
 	struct smack_known *skp;
 
 	if (!smack_checked_secmark) {
@@ -84,11 +84,15 @@ static unsigned int smack_ipv4_output(void *priv,
 		smack_checked_secmark = true;
 	}
 
-	if (smack_use_secmark && sk && smack_sock(sk)) {
+	if (sk && smack_sock(sk))
 		ssp = smack_sock(sk);
+
+	if (smack_use_secmark && ssp) {
 		skp = ssp->smk_out;
 		skb->secmark = skp->smk_secid;
 	}
+	if (sk && security_reconcile_netlbl(sk))
+		return NF_DROP;
 
 	return NF_ACCEPT;
 }
-- 
2.20.1


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

* [PATCH v7 16/16] Smack: Remove the exclusive flag
  2019-08-07 22:42 [PATCH v7 00/16] LSM: Full module stacking Casey Schaufler
                   ` (15 preceding siblings ...)
  2019-08-07 22:42 ` [PATCH v7 15/16] LSM: Avoid network conflicts in SELinux and Smack Casey Schaufler
@ 2019-08-07 22:42 ` Casey Schaufler
  16 siblings, 0 replies; 18+ messages in thread
From: Casey Schaufler @ 2019-08-07 22:42 UTC (permalink / raw)
  To: casey.schaufler, jmorris, linux-security-module, selinux
  Cc: casey, keescook, john.johansen, penguin-kernel, paul, sds

Smack no longer needs to be treated as an "exclusive" security module.
Remove the flag that indicates it is exclusive.

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 122c13604d28..3b76ec6cf960 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -4822,7 +4822,7 @@ static __init int smack_init(void)
  */
 DEFINE_LSM(smack) = {
 	.name = "smack",
-	.flags = LSM_FLAG_LEGACY_MAJOR | LSM_FLAG_EXCLUSIVE,
+	.flags = LSM_FLAG_LEGACY_MAJOR,
 	.blobs = &smack_blob_sizes,
 	.init = smack_init,
 };
-- 
2.20.1


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

end of thread, other threads:[~2019-08-07 22:43 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-07 22:42 [PATCH v7 00/16] LSM: Full module stacking Casey Schaufler
2019-08-07 22:42 ` Casey Schaufler
2019-08-07 22:42 ` [PATCH v7 01/16] LSM: Single hook called in secmark refcounting Casey Schaufler
2019-08-07 22:42 ` [PATCH v7 02/16] Smack: Detect if secmarks can be safely used Casey Schaufler
2019-08-07 22:42 ` [PATCH v7 03/16] LSM: Support multiple LSMs using inode_init_security Casey Schaufler
2019-08-07 22:42 ` [PATCH v7 04/16] LSM: List multiple security attributes in security_inode_listsecurity Casey Schaufler
2019-08-07 22:42 ` [PATCH v7 05/16] LSM: Multiple modules using security_ismaclabel Casey Schaufler
2019-08-07 22:42 ` [PATCH v7 06/16] LSM: Make multiple MAC modules safe in nfs and kernfs Casey Schaufler
2019-08-07 22:42 ` [PATCH v7 07/16] LSM: Correct handling of ENOSYS in inode_setxattr Casey Schaufler
2019-08-07 22:42 ` [PATCH v7 08/16] LSM: Infrastructure security blobs for mount options Casey Schaufler
2019-08-07 22:42 ` [PATCH v7 09/16] LSM: Fix for security_init_inode_security Casey Schaufler
2019-08-07 22:42 ` [PATCH v7 10/16] LSM: Change error detection for UDP peer security Casey Schaufler
2019-08-07 22:42 ` [PATCH v7 11/16] Netlabel: Add a secattr comparison API function Casey Schaufler
2019-08-07 22:42 ` [PATCH v7 12/16] Netlabel: Provide labeling type to security modules Casey Schaufler
2019-08-07 22:42 ` [PATCH v7 13/16] LSM: Remember the NLTYPE of netlabel sockets Casey Schaufler
2019-08-07 22:42 ` [PATCH v7 14/16] LSM: Hook for netlabel reconciliation Casey Schaufler
2019-08-07 22:42 ` [PATCH v7 15/16] LSM: Avoid network conflicts in SELinux and Smack Casey Schaufler
2019-08-07 22:42 ` [PATCH v7 16/16] Smack: Remove the exclusive flag Casey Schaufler

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).