diff mbox series

[v2,3/6] landlock: Add UDP sendmsg access control

Message ID 20241214184540.3835222-4-matthieu@buffet.re (mailing list archive)
State Handled Elsewhere
Headers show
Series landlock: Add UDP access control support | expand

Commit Message

Matthieu Buffet Dec. 14, 2024, 6:45 p.m. UTC
Add support for a LANDLOCK_ACCESS_NET_SENDTO_UDP access right,
complementing the two previous LANDLOCK_ACCESS_NET_CONNECT_UDP and
LANDLOCK_ACCESS_NET_BIND_UDP.
It allows denying and delegating the right to sendto() datagrams with an
explicit destination address and port, without requiring to connect() the
socket first.

Performance is of course worse if you send many datagrams this way,
compared to just connect() then sending without an address (except if you
use sendmmsg() which caches LSM results). This may still be desired by
applications which send few enough datagrams to different clients that
opening and connecting a socket for each one of them is not worth it.

Signed-off-by: Matthieu Buffet <matthieu@buffet.re>
---
 include/uapi/linux/landlock.h | 14 ++++++
 security/landlock/limits.h    |  2 +-
 security/landlock/net.c       | 88 +++++++++++++++++++++++++++++++++++
 3 files changed, 103 insertions(+), 1 deletion(-)

Comments

kernel test robot Dec. 15, 2024, 3:33 p.m. UTC | #1
Hi Matthieu,

kernel test robot noticed the following build errors:

[auto build test ERROR on adc218676eef25575469234709c2d87185ca223a]

url:    https://github.com/intel-lab-lkp/linux/commits/Matthieu-Buffet/landlock-Add-UDP-bind-connect-access-control/20241215-025450
base:   adc218676eef25575469234709c2d87185ca223a
patch link:    https://lore.kernel.org/r/20241214184540.3835222-4-matthieu%40buffet.re
patch subject: [PATCH v2 3/6] landlock: Add UDP sendmsg access control
config: nios2-randconfig-001-20241215 (https://download.01.org/0day-ci/archive/20241215/202412152346.CRX05ecl-lkp@intel.com/config)
compiler: nios2-linux-gcc (GCC) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241215/202412152346.CRX05ecl-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202412152346.CRX05ecl-lkp@intel.com/

All errors (new ones prefixed by >>):

   nios2-linux-ld: security/landlock/net.o: in function `hook_socket_sendmsg':
>> net.c:(.text+0x448): undefined reference to `udpv6_prot'
>> nios2-linux-ld: net.c:(.text+0x44c): undefined reference to `udpv6_prot'
kernel test robot Dec. 15, 2024, 10:38 p.m. UTC | #2
Hi Matthieu,

kernel test robot noticed the following build errors:

[auto build test ERROR on adc218676eef25575469234709c2d87185ca223a]

url:    https://github.com/intel-lab-lkp/linux/commits/Matthieu-Buffet/landlock-Add-UDP-bind-connect-access-control/20241215-025450
base:   adc218676eef25575469234709c2d87185ca223a
patch link:    https://lore.kernel.org/r/20241214184540.3835222-4-matthieu%40buffet.re
patch subject: [PATCH v2 3/6] landlock: Add UDP sendmsg access control
config: arm-allmodconfig (https://download.01.org/0day-ci/archive/20241216/202412160648.ACrVdKoZ-lkp@intel.com/config)
compiler: arm-linux-gnueabi-gcc (GCC) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241216/202412160648.ACrVdKoZ-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202412160648.ACrVdKoZ-lkp@intel.com/

All errors (new ones prefixed by >>):

   arm-linux-gnueabi-ld: security/landlock/net.o: in function `hook_socket_sendmsg':
>> net.c:(.text.hook_socket_sendmsg+0x288): undefined reference to `udpv6_prot'
diff mbox series

Patch

diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
index 3f7b8e85822d..8b355891e986 100644
--- a/include/uapi/linux/landlock.h
+++ b/include/uapi/linux/landlock.h
@@ -295,6 +295,19 @@  struct landlock_net_port_attr {
  *   every time), or for servers that want to filter which client address
  *   they want to receive datagrams from (e.g. creating a client-specific
  *   socket)
+ * - %LANDLOCK_ACCESS_NET_SENDTO_UDP: send datagrams with an explicit
+ *   destination address set to the given remote port. This access right
+ *   is checked in sendto(), sendmsg() and sendmmsg() when the destination
+ *   address passed is not NULL. This access right is not required when
+ *   sending datagrams without an explicit destination (via a connected
+ *   socket, e.g. with send()). Sending datagrams with explicit addresses
+ *   induces a non-negligible overhead, so calling connect() once and for
+ *   all should be preferred. When not possible and sending many datagrams,
+ *   using sendmmsg() may reduce the access control overhead.
+ *
+ * Blocking an application from sending UDP traffic requires adding both
+ * %LANDLOCK_ACCESS_NET_SENDTO_UDP and %LANDLOCK_ACCESS_NET_CONNECT_UDP
+ * to the handled access rights list.
  *
  * Note that binding on port 0 means binding to an ephemeral
  * kernel-assigned port, in the range configured in
@@ -306,6 +319,7 @@  struct landlock_net_port_attr {
 #define LANDLOCK_ACCESS_NET_CONNECT_TCP			(1ULL << 1)
 #define LANDLOCK_ACCESS_NET_BIND_UDP			(1ULL << 2)
 #define LANDLOCK_ACCESS_NET_CONNECT_UDP			(1ULL << 3)
+#define LANDLOCK_ACCESS_NET_SENDTO_UDP			(1ULL << 4)
 /* clang-format on */
 
 /**
diff --git a/security/landlock/limits.h b/security/landlock/limits.h
index ca90c1c56458..8d12ca39cf2e 100644
--- a/security/landlock/limits.h
+++ b/security/landlock/limits.h
@@ -22,7 +22,7 @@ 
 #define LANDLOCK_MASK_ACCESS_FS		((LANDLOCK_LAST_ACCESS_FS << 1) - 1)
 #define LANDLOCK_NUM_ACCESS_FS		__const_hweight64(LANDLOCK_MASK_ACCESS_FS)
 
-#define LANDLOCK_LAST_ACCESS_NET	LANDLOCK_ACCESS_NET_CONNECT_UDP
+#define LANDLOCK_LAST_ACCESS_NET	LANDLOCK_ACCESS_NET_SENDTO_UDP
 #define LANDLOCK_MASK_ACCESS_NET	((LANDLOCK_LAST_ACCESS_NET << 1) - 1)
 #define LANDLOCK_NUM_ACCESS_NET		__const_hweight64(LANDLOCK_MASK_ACCESS_NET)
 
diff --git a/security/landlock/net.c b/security/landlock/net.c
index 1c5cf2ddb7c1..0556d8a21d0b 100644
--- a/security/landlock/net.c
+++ b/security/landlock/net.c
@@ -10,6 +10,8 @@ 
 #include <linux/net.h>
 #include <linux/socket.h>
 #include <net/ipv6.h>
+#include <net/transp_v6.h>
+#include <net/ip.h>
 
 #include "common.h"
 #include "cred.h"
@@ -155,6 +157,27 @@  static int current_check_access_socket(struct socket *const sock,
 	return -EACCES;
 }
 
+static int check_access_port(const struct landlock_ruleset *const dom,
+			     access_mask_t access_request, __be16 port)
+{
+	layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_NET] = {};
+	const struct landlock_rule *rule;
+	const struct landlock_id id = {
+		.key.data = (__force uintptr_t)port,
+		.type = LANDLOCK_KEY_NET_PORT,
+	};
+	BUILD_BUG_ON(sizeof(port) > sizeof(id.key.data));
+
+	rule = landlock_find_rule(dom, id);
+	access_request = landlock_init_layer_masks(
+		dom, access_request, &layer_masks, LANDLOCK_KEY_NET_PORT);
+	if (landlock_unmask_layers(rule, access_request, &layer_masks,
+				   ARRAY_SIZE(layer_masks)))
+		return 0;
+
+	return -EACCES;
+}
+
 static int hook_socket_bind(struct socket *const sock,
 			    struct sockaddr *const address, const int addrlen)
 {
@@ -190,9 +213,74 @@  static int hook_socket_connect(struct socket *const sock,
 					   access_request);
 }
 
+static int hook_socket_sendmsg(struct socket *const sock,
+			       struct msghdr *const msg, const int size)
+{
+	const struct landlock_ruleset *const dom =
+		landlock_get_applicable_domain(landlock_get_current_domain(),
+					       any_net);
+	const struct sockaddr *address = (const struct sockaddr *)msg->msg_name;
+	const int addrlen = msg->msg_namelen;
+	__be16 port;
+
+	if (!dom)
+		return 0;
+	if (WARN_ON_ONCE(dom->num_layers < 1))
+		return -EACCES;
+	/*
+	 * If there is no explicit address in the message, we have no
+	 * policy to enforce here because either:
+	 * - the socket was previously connect()ed, so the appropriate
+	 *   access check has already been done back then;
+	 * - the socket is unconnected, so we can let the networking stack
+	 *   reply -EDESTADDRREQ
+	 */
+	if (!address)
+		return 0;
+
+	if (!sk_is_udp(sock->sk))
+		return 0;
+
+	/* Checks for minimal header length to safely read sa_family. */
+	if (addrlen < offsetofend(typeof(*address), sa_family))
+		return -EINVAL;
+
+	switch (address->sa_family) {
+	case AF_UNSPEC:
+		/*
+		 * Parsed as "no address" in udpv6_sendmsg(), which means
+		 * we fall back into the case checked earlier: policy was
+		 * enforced at connect() time, nothing to enforce here.
+		 */
+		if (sock->sk->sk_prot == &udpv6_prot)
+			return 0;
+		/* Parsed as "AF_INET" in udp_sendmsg() */
+		fallthrough;
+	case AF_INET:
+		if (addrlen < sizeof(struct sockaddr_in))
+			return -EINVAL;
+		port = ((struct sockaddr_in *)address)->sin_port;
+		break;
+
+#if IS_ENABLED(CONFIG_IPV6)
+	case AF_INET6:
+		if (addrlen < SIN6_LEN_RFC2133)
+			return -EINVAL;
+		port = ((struct sockaddr_in6 *)address)->sin6_port;
+		break;
+#endif /* IS_ENABLED(CONFIG_IPV6) */
+
+	default:
+		return -EAFNOSUPPORT;
+	}
+
+	return check_access_port(dom, LANDLOCK_ACCESS_NET_SENDTO_UDP, port);
+}
+
 static struct security_hook_list landlock_hooks[] __ro_after_init = {
 	LSM_HOOK_INIT(socket_bind, hook_socket_bind),
 	LSM_HOOK_INIT(socket_connect, hook_socket_connect),
+	LSM_HOOK_INIT(socket_sendmsg, hook_socket_sendmsg),
 };
 
 __init void landlock_add_net_hooks(void)