[PATCH mptcp-net] mptcp: fix LSM labeling for passive msk

* [PATCH mptcp-net] mptcp: fix LSM labeling for passive msk
@ 2022-12-07 17:51 Paolo Abeni
  2022-12-07 19:05 ` mptcp: fix LSM labeling for passive msk: Tests Results MPTCP CI
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Paolo Abeni @ 2022-12-07 17:51 UTC (permalink / raw)
  To: mptcp
  Cc: Paul Moore, Ondrej Mosnacek, SElinux list, Linux Security Module list

MPTCP sockets created via accept() inherit their LSM label
from the initial request socket, which in turn get it from the
listener socket's first subflow. The latter is a kernel socket,
and get the relevant labeling at creation time.

Due to all the above even the accepted MPTCP socket get a kernel
label, causing unexpected behaviour and failure on later LSM tests.

Address the issue factoring out a socket creation helper that does
not include the post-creation LSM checks. Use such helper to create
mptcp subflow as in-kernel sockets and doing explicitly LSM validation:
vs the current user for the first subflow, as a kernel socket otherwise.

Fixes: 0c14846032f2 ("mptcp: fix security context on server socket")
Reported-by: Ondrej Mosnacek <omosnace@redhat.com>
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
 include/linux/net.h |  2 ++
 net/mptcp/subflow.c | 19 ++++++++++++--
 net/socket.c        | 60 ++++++++++++++++++++++++++++++---------------
 3 files changed, 59 insertions(+), 22 deletions(-)

diff --git a/include/linux/net.h b/include/linux/net.h
index b73ad8e3c212..91713012504d 100644
--- a/include/linux/net.h
+++ b/include/linux/net.h
@@ -251,6 +251,8 @@ int sock_wake_async(struct socket_wq *sk_wq, int how, int band);
 int sock_register(const struct net_proto_family *fam);
 void sock_unregister(int family);
 bool sock_is_registered(int family);
+int __sock_create_nosec(struct net *net, int family, int type, int proto,
+			struct socket **res, int kern);
 int __sock_create(struct net *net, int family, int type, int proto,
 		  struct socket **res, int kern);
 int sock_create(int family, int type, int proto, struct socket **res);
diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index d5ff502c88d7..e7e6f17df7ef 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -1646,11 +1646,26 @@ int mptcp_subflow_create_socket(struct sock *sk, struct socket **new_sock)
 	if (unlikely(!sk->sk_socket))
 		return -EINVAL;
 
-	err = sock_create_kern(net, sk->sk_family, SOCK_STREAM, IPPROTO_TCP,
-			       &sf);
+	/* the subflow is created by the kernel, and we need kernel annotation
+	 * for lockdep's sake...
+	 */
+	err = __sock_create_nosec(net, sk->sk_family, SOCK_STREAM, IPPROTO_TCP,
+				  &sf, 1);
 	if (err)
 		return err;
 
+	/* ... but the MPC subflow will be indirectly exposed to the
+	 * user-space via accept(). Let's attach the current user security
+	 * label to the first subflow, that is when msk->first is not yet
+	 * initialized.
+	 */
+	err = security_socket_post_create(sf, sk->sk_family, SOCK_STREAM,
+					  IPPROTO_TCP, !!mptcp_sk(sk)->first);
+	if (err) {
+		sock_release(sf);
+		return err;
+	}
+
 	lock_sock(sf->sk);
 
 	/* the newly created socket has to be in the same cgroup as its parent */
diff --git a/net/socket.c b/net/socket.c
index 55c5d536e5f6..d5d51e4e26ae 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -1426,23 +1426,11 @@ int sock_wake_async(struct socket_wq *wq, int how, int band)
 }
 EXPORT_SYMBOL(sock_wake_async);
 
-/**
- *	__sock_create - creates a socket
- *	@net: net namespace
- *	@family: protocol family (AF_INET, ...)
- *	@type: communication type (SOCK_STREAM, ...)
- *	@protocol: protocol (0, ...)
- *	@res: new socket
- *	@kern: boolean for kernel space sockets
- *
- *	Creates a new socket and assigns it to @res, passing through LSM.
- *	Returns 0 or an error. On failure @res is set to %NULL. @kern must
- *	be set to true if the socket resides in kernel space.
- *	This function internally uses GFP_KERNEL.
- */
 
-int __sock_create(struct net *net, int family, int type, int protocol,
-			 struct socket **res, int kern)
+
+/*creates a socket leaving LSM post-creation checks to the caller */
+int __sock_create_nosec(struct net *net, int family, int type, int protocol,
+			struct socket **res, int kern)
 {
 	int err;
 	struct socket *sock;
@@ -1528,11 +1516,8 @@ int __sock_create(struct net *net, int family, int type, int protocol,
 	 * module can have its refcnt decremented
 	 */
 	module_put(pf->owner);
-	err = security_socket_post_create(sock, family, type, protocol, kern);
-	if (err)
-		goto out_sock_release;
-	*res = sock;
 
+	*res = sock;
 	return 0;
 
 out_module_busy:
@@ -1548,6 +1533,41 @@ int __sock_create(struct net *net, int family, int type, int protocol,
 	rcu_read_unlock();
 	goto out_sock_release;
 }
+
+/**
+ *	__sock_create - creates a socket
+ *	@net: net namespace
+ *	@family: protocol family (AF_INET, ...)
+ *	@type: communication type (SOCK_STREAM, ...)
+ *	@protocol: protocol (0, ...)
+ *	@res: new socket
+ *	@kern: boolean for kernel space sockets
+ *
+ *	Creates a new socket and assigns it to @res, passing through LSM.
+ *	Returns 0 or an error. On failure @res is set to %NULL. @kern must
+ *	be set to true if the socket resides in kernel space.
+ *	This function internally uses GFP_KERNEL.
+ */
+
+int __sock_create(struct net *net, int family, int type, int protocol,
+		  struct socket **res, int kern)
+{
+	struct socket *sock;
+	int err;
+
+	err = __sock_create_nosec(net, family, type, protocol, &sock, kern);
+	if (err)
+		return err;
+
+	err = security_socket_post_create(sock, family, type, protocol, kern);
+	if (err) {
+		sock_release(sock);
+		return err;
+	}
+
+	*res = sock;
+	return 0;
+}
 EXPORT_SYMBOL(__sock_create);
 
 /**
-- 
2.38.1


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