Skip to contentSkip to conversation

selinux: fix the default socket labeling in sock_graft()

PATCH In review Paul Moore 2 participants 3 messages Jul 14, 2014 – Jul 16, 2014

Subsystems (2)
SELinux security module Security (LSM)
lore
Share & tools
lore

Paul Moore

author
#0 lore raw

PATCH v2 1/1 selinux: fix the default socket labeling in sock_graft()

The sock_graft() hook has special handling for AF_INET, AF_INET, and AF_UNIX sockets as those address families have special hooks which label the sock before it is attached its associated socket. Unfortunately, the sock_graft() hook was missing a default approach to labeling sockets which meant that any other address family which made use of connections or the accept() syscall would find the returned socket to be in an "unlabeled" state. This was recently demonstrated by the kcrypto/AF_ALG subsystem and the newly released cryptsetup package (cryptsetup v1.6.5 and later).

This patch preserves the special handling in selinux_sock_graft(), but adds a default behavior - setting the sock's label equal to the associated socket - which resolves the problem with AF_ALG and presumably any other address family which makes use of accept().

Cc [email protected]
Signed-off-by Paul Moore <[email protected]>
---
 include/linux/security.h |    5 ++++-
 security/selinux/hooks.c |   13 +++++++++++--
 2 files changed, 15 insertions(+), 3 deletions(-)
include/linux/security.h +4 -1
index 6478ce3..794be73 100644
@@ -987,7 +987,10 @@ static inline void security_free_mnt_opts(struct security_mnt_opts *opts)
  *	Retrieve the LSM-specific secid for the sock to enable caching of network
  *	authorizations.
  * @sock_graft:
- *	Sets the socket's isec sid to the sock's sid.
+ *	This hook is called in response to a newly created sock struct being
+ *	grafted onto an existing socket and allows the security module to
+ *	perform whatever security attribute management is necessary for both
+ *	the sock and socket.
  * @inet_conn_request:
  *	Sets the openreq's sid to socket's sid with MLS portion taken from peer sid.
  * @inet_csk_clone:
security/selinux/hooks.c +11 -2
index 336f0a0..b3a6754 100644
@@ -4499,9 +4499,18 @@ static void selinux_sock_graft(struct sock *sk, struct socket *parent)
 	struct inode_security_struct *isec = SOCK_INODE(parent)->i_security;
 	struct sk_security_struct *sksec = sk->sk_security;
 
-	if (sk->sk_family == PF_INET || sk->sk_family == PF_INET6 ||
-	    sk->sk_family == PF_UNIX)
+	switch (sk->sk_family) {
+	case PF_INET:
+	case PF_INET6:
+	case PF_UNIX:
 		isec->sid = sksec->sid;
+		break;
+	default:
+		/* by default there is no special labeling mechanism for the
+		 * sksec label so inherit the label from the parent socket */
+		BUG_ON(sksec->sid != SECINITSID_UNLABELED);
+		sksec->sid = isec->sid;
+	}
 	sksec->sclass = isec->sclass;
 }
Tested-by on v2 1/1
#1 lore raw
Quoted 10 lines · Paul Moore

The sock_graft() hook has special handling for AF_INET, AF_INET, and AF_UNIX sockets as those address families have special hooks which label the sock before it is attached its associated socket. Unfortunately, the sock_graft() hook was missing a default approach to labeling sockets which meant that any other address family which made use of connections or the accept() syscall would find the returned socket to be in an "unlabeled" state. This was recently demonstrated by the kcrypto/AF_ALG subsystem and the newly released cryptsetup package (cryptsetup v1.6.5 and later).

Quoted 7 lines

This patch preserves the special handling in selinux_sock_graft(), but adds a default behavior - setting the sock's label equal to the associated socket - which resolves the problem with AF_ALG and presumably any other address family which makes use of accept().

Cc [email protected]
Signed-off-by Paul Moore <[email protected]>

I tested v2 patch for the cryptsetup use case (ALG_IF crypto subsystem) and it fixes the problem in enforcing mode.

So, if you wish, add

Tested-by Milan Broz <[email protected]>

Thanks!
Milan

author
#2 lore raw
Quoted 16 lines · Milan Broz
Quoted 15 lines · Paul Moore

The sock_graft() hook has special handling for AF_INET, AF_INET, and AF_UNIX sockets as those address families have special hooks which label the sock before it is attached its associated socket. Unfortunately, the sock_graft() hook was missing a default approach to labeling sockets which meant that any other address family which made use of connections or the accept() syscall would find the returned socket to be in an "unlabeled" state. This was recently demonstrated by the kcrypto/AF_ALG subsystem and the newly released cryptsetup package (cryptsetup v1.6.5 and later).

This patch preserves the special handling in selinux_sock_graft(), but adds a default behavior - setting the sock's label equal to the associated socket - which resolves the problem with AF_ALG and presumably any other address family which makes use of accept().

Quoted 8 lines
Quoted 2 lines
Cc [email protected]
Signed-off-by Paul Moore <[email protected]>

I tested v2 patch for the cryptsetup use case (ALG_IF crypto subsystem) and it fixes the problem in enforcing mode.

So, if you wish, add

Tested-by Milan Broz <[email protected]>

Thanks for your help, since there have been no further comments I'll add your "Tested-by" and push this upstream.

Signature · 2 lines
paul moore
security and virtualization @ redhat