[PATCH v2 0/6] landlock: Add UDP access control support

[PATCH v2 0/6] landlock: Add UDP access control support

[Matthieu Buffet]

Hi Mickael,

Thanks for your comments on the v1 of this patch, I should have everything
fixed so (hopefully) this v2 boils down to something simpler.

This patchset is based on
https://git.kernel.org/pub/scm/linux/kernel/git/mic/linux.git
Linux 6.12 (adc218676eef).

This patchset should add basic support to completely block a process
from sending and receiving UDP datagrams, and delegate the right to
send/receive based on remote/local port. It should fit nicely with
the socket creation restrictions WIP (either don't have UDP at all, or
have it with just the rights needed).

@Mikhail: I saw the discussions around TCP error code inconsistencies +
over-restriction, and your patch v1. I took extra care to minimize this
diff size: no unnecessary comment/refactor, especially in
current_check_access_socket(). It should be just what is required for a
basic UDP support without changing error handling in that main function.

The only question that remained open from v1 was about UDP rights naming.
Since there were no strong preferences and the hooks now only handle
sendmsg() if an explicit address is specified, that's now
LANDLOCK_ACCESS_NET_UDP_SENDTO since the name (and prototype with a
destination address parameter) of sendto(3) is closer to these semantics.

Changes since v1 (link below):
- recvmsg hook is gone and sendmsg hook doesn't apply to connected
  sockets anymore, to improve performance
- don't add a get_addr_port() helper function, which required a weird "am
  I in IPv4 or IPv6 context" to avoid a addrlen>sizeof(struct sockaddr_in)
  check in connect(AF_UNSPEC) IPv6 context. A helper was useful when ports
  also needed to be read in a recvmsg() hook, now it's just a simple
  switch case in the sendmsg() hook, more readable
- rename sendmsg access right to LANDLOCK_ACCESS_NET_UDP_SENDTO
- reorder hook prologue for consistency: check domain, then type and
  family
- add additional selftests cases around minimal address length
- update documentation

lcov gives me net.c going from 94% lines/80% functions to 96.6% lines/
85.7% functions

Any feedback welcome!

Link: https://lore.kernel.org/all/20240916122230.114800-1-matthieu@buffet.re/
Closes: https://github.com/landlock-lsm/linux/issues/10

Link: https://lore.kernel.org/all/20241017110454.265818-1-ivanov.mikhail1@huawei-partners.com/
Cc: Mikhail Ivanov <ivanov.mikhail1@huawei-partners.com>

Matthieu Buffet (6):
  landlock: Add UDP bind+connect access control
  selftests/landlock: Adapt existing bind/connect for UDP
  landlock: Add UDP sendmsg access control
  selftests/landlock: Add ACCESS_NET_SENDTO_UDP
  samples/landlock: Add sandboxer UDP access control
  doc: Add landlock UDP support

 Documentation/userspace-api/landlock.rst     |  84 +++-
 include/uapi/linux/landlock.h                |  67 ++-
 samples/landlock/sandboxer.c                 |  58 ++-
 security/landlock/limits.h                   |   2 +-
 security/landlock/net.c                      | 137 +++++-
 security/landlock/syscalls.c                 |   2 +-
 tools/testing/selftests/landlock/base_test.c |   2 +-
 tools/testing/selftests/landlock/net_test.c  | 455 +++++++++++++++++--
 8 files changed, 715 insertions(+), 92 deletions(-)


base-commit: adc218676eef25575469234709c2d87185ca223a
-- 
2.39.5

[PATCH v2 1/6] landlock: Add UDP bind+connect access control

[Matthieu Buffet]

If an app doesn't need to be able to open UDP sockets, it should be
denied the right to create UDP sockets altogether (via seccomp and/or
https://github.com/landlock-lsm/linux/issues/6 when it lands).
For apps using UDP, add support for two more fine-grained access rights:

- LANDLOCK_ACCESS_NET_CONNECT_UDP, to gate the possibility to connect()
  a UDP socket. For client apps (those which want to avoid specifying a
  destination for each datagram in sendmsg()), and for a few servers
  (those creating per-client sockets, which want to receive traffic only
  from a specific address)

- LANDLOCK_ACCESS_NET_BIND_UDP, to gate the possibility to bind() a UDP
  socket. For most servers (to start listening for datagrams on a
  non-ephemeral port) and can be useful for some client applications (to
  set the source port of future datagrams, e.g. mDNS requires to use
  source port 5353)

No restriction is enforced on send()/recv() to preserve performance.
The security boundary is to prevent acquiring a bound/connected socket.

Bump ABI to v7 to allow userland to detect and use these new restrictions.

Signed-off-by: Matthieu Buffet <matthieu@buffet.re>
---
 include/uapi/linux/landlock.h | 53 ++++++++++++++++++++++++++---------
 security/landlock/limits.h    |  2 +-
 security/landlock/net.c       | 49 ++++++++++++++++++++++----------
 security/landlock/syscalls.c  |  2 +-
 4 files changed, 76 insertions(+), 30 deletions(-)

diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
index 33745642f787..3f7b8e85822d 100644
--- a/include/uapi/linux/landlock.h
+++ b/include/uapi/linux/landlock.h
@@ -119,12 +119,15 @@ struct landlock_net_port_attr {
 	 *
 	 * It should be noted that port 0 passed to :manpage:`bind(2)` will bind
 	 * to an available port from the ephemeral port range.  This can be
-	 * configured with the ``/proc/sys/net/ipv4/ip_local_port_range`` sysctl
-	 * (also used for IPv6).
+	 * configured globally with the
+	 * ``/proc/sys/net/ipv4/ip_local_port_range`` sysctl (also used for
+	 * IPv6), and, within that first range, on a per-socket basis using
+	 * ``setsockopt(IP_LOCAL_PORT_RANGE)``.
 	 *
-	 * A Landlock rule with port 0 and the ``LANDLOCK_ACCESS_NET_BIND_TCP``
-	 * right means that requesting to bind on port 0 is allowed and it will
-	 * automatically translate to binding on the related port range.
+	 * A Landlock rule with port 0 and the %LANDLOCK_ACCESS_NET_BIND_TCP
+	 * or %LANDLOCK_ACCESS_NET_BIND_UDP right means that requesting to
+	 * bind on port 0 is allowed and it will automatically translate to
+	 * binding on the ephemeral port range.
 	 */
 	__u64 port;
 };
@@ -267,18 +270,42 @@ struct landlock_net_port_attr {
  * Network flags
  * ~~~~~~~~~~~~~~~~
  *
- * These flags enable to restrict a sandboxed process to a set of network
- * actions. This is supported since the Landlock ABI version 4.
- *
- * The following access rights apply to TCP port numbers:
- *
- * - %LANDLOCK_ACCESS_NET_BIND_TCP: Bind a TCP socket to a local port.
- * - %LANDLOCK_ACCESS_NET_CONNECT_TCP: Connect an active TCP socket to
- *   a remote port.
+ * These flags enable to restrict which network-related actions a sandboxed
+ * process can take. TCP support was added in Landlock ABI version 4, and UDP
+ * support in version 7.
+ *
+ * TCP access rights:
+ *
+ * - %LANDLOCK_ACCESS_NET_BIND_TCP: bind sockets to the given local port,
+ *   for servers that will listen() on that port, or for clients that want
+ *   to open connections with that specific source port instead of using a
+ *   kernel-assigned random ephemeral one
+ * - %LANDLOCK_ACCESS_NET_CONNECT_TCP: connect client sockets to servers
+ *   listening on that remote port
+ *
+ * UDP access rights:
+ *
+ * - %LANDLOCK_ACCESS_NET_BIND_UDP: bind sockets to the given local port,
+ *   for servers that will listen() on that port, or for clients that want
+ *   to send datagrams with that specific source port instead of using a
+ *   kernel-assigned random ephemeral one
+ * - %LANDLOCK_ACCESS_NET_CONNECT_UDP: connect sockets to the given remote
+ *   port, either for clients that will send datagrams to that destination
+ *   (and want to send them faster without specifying an explicit address
+ *   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)
+ *
+ * Note that binding on port 0 means binding to an ephemeral
+ * kernel-assigned port, in the range configured in
+ * ``/proc/sys/net/ipv4/ip_local_port_range`` globally (and, within that
+ * range, on a per-socket basis with ``setsockopt(IP_LOCAL_PORT_RANGE)``).
  */
 /* clang-format off */
 #define LANDLOCK_ACCESS_NET_BIND_TCP			(1ULL << 0)
 #define LANDLOCK_ACCESS_NET_CONNECT_TCP			(1ULL << 1)
+#define LANDLOCK_ACCESS_NET_BIND_UDP			(1ULL << 2)
+#define LANDLOCK_ACCESS_NET_CONNECT_UDP			(1ULL << 3)
 /* clang-format on */
 
 /**
diff --git a/security/landlock/limits.h b/security/landlock/limits.h
index 15f7606066c8..ca90c1c56458 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_TCP
+#define LANDLOCK_LAST_ACCESS_NET	LANDLOCK_ACCESS_NET_CONNECT_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 d5dcc4407a19..1c5cf2ddb7c1 100644
--- a/security/landlock/net.c
+++ b/security/landlock/net.c
@@ -63,10 +63,6 @@ static int current_check_access_socket(struct socket *const sock,
 	if (WARN_ON_ONCE(dom->num_layers < 1))
 		return -EACCES;
 
-	/* Checks if it's a (potential) TCP socket. */
-	if (sock->type != SOCK_STREAM)
-		return 0;
-
 	/* Checks for minimal header length to safely read sa_family. */
 	if (addrlen < offsetofend(typeof(*address), sa_family))
 		return -EINVAL;
@@ -94,17 +90,19 @@ static int current_check_access_socket(struct socket *const sock,
 	/* Specific AF_UNSPEC handling. */
 	if (address->sa_family == AF_UNSPEC) {
 		/*
-		 * Connecting to an address with AF_UNSPEC dissolves the TCP
-		 * association, which have the same effect as closing the
-		 * connection while retaining the socket object (i.e., the file
-		 * descriptor).  As for dropping privileges, closing
-		 * connections is always allowed.
-		 *
-		 * For a TCP access control system, this request is legitimate.
+		 * Connecting to an address with AF_UNSPEC dissolves the
+		 * remote association while retaining the socket object
+		 * (i.e., the file descriptor). For TCP, it has the same
+		 * effect as closing the connection. For UDP, it removes
+		 * any preset destination for future datagrams.
+		 * Like dropping privileges, these actions are always
+		 * allowed: access control is performed when bind()ing or
+		 * connect()ing.
 		 * Let the network stack handle potential inconsistencies and
 		 * return -EINVAL if needed.
 		 */
-		if (access_request == LANDLOCK_ACCESS_NET_CONNECT_TCP)
+		if (access_request == LANDLOCK_ACCESS_NET_CONNECT_TCP ||
+		    access_request == LANDLOCK_ACCESS_NET_CONNECT_UDP)
 			return 0;
 
 		/*
@@ -118,7 +116,8 @@ static int current_check_access_socket(struct socket *const sock,
 		 * checks, but it is safer to return a proper error and test
 		 * consistency thanks to kselftest.
 		 */
-		if (access_request == LANDLOCK_ACCESS_NET_BIND_TCP) {
+		if (access_request == LANDLOCK_ACCESS_NET_BIND_TCP ||
+		    access_request == LANDLOCK_ACCESS_NET_BIND_UDP) {
 			/* addrlen has already been checked for AF_UNSPEC. */
 			const struct sockaddr_in *const sockaddr =
 				(struct sockaddr_in *)address;
@@ -159,16 +158,36 @@ static int current_check_access_socket(struct socket *const sock,
 static int hook_socket_bind(struct socket *const sock,
 			    struct sockaddr *const address, const int addrlen)
 {
+	access_mask_t access_request;
+
+	/* Checks if it's a (potential) TCP socket. */
+	if (sock->type == SOCK_STREAM)
+		access_request = LANDLOCK_ACCESS_NET_BIND_TCP;
+	else if (sk_is_udp(sock->sk))
+		access_request = LANDLOCK_ACCESS_NET_BIND_UDP;
+	else
+		return 0;
+
 	return current_check_access_socket(sock, address, addrlen,
-					   LANDLOCK_ACCESS_NET_BIND_TCP);
+					   access_request);
 }
 
 static int hook_socket_connect(struct socket *const sock,
 			       struct sockaddr *const address,
 			       const int addrlen)
 {
+	access_mask_t access_request;
+
+	/* Checks if it's a (potential) TCP socket. */
+	if (sock->type == SOCK_STREAM)
+		access_request = LANDLOCK_ACCESS_NET_CONNECT_TCP;
+	else if (sk_is_udp(sock->sk))
+		access_request = LANDLOCK_ACCESS_NET_CONNECT_UDP;
+	else
+		return 0;
+
 	return current_check_access_socket(sock, address, addrlen,
-					   LANDLOCK_ACCESS_NET_CONNECT_TCP);
+					   access_request);
 }
 
 static struct security_hook_list landlock_hooks[] __ro_after_init = {
diff --git a/security/landlock/syscalls.c b/security/landlock/syscalls.c
index c097d356fa45..200f771fa3a4 100644
--- a/security/landlock/syscalls.c
+++ b/security/landlock/syscalls.c
@@ -150,7 +150,7 @@ static const struct file_operations ruleset_fops = {
 	.write = fop_dummy_write,
 };
 
-#define LANDLOCK_ABI_VERSION 6
+#define LANDLOCK_ABI_VERSION 7
 
 /**
  * sys_landlock_create_ruleset - Create a new ruleset
-- 
2.39.5

[PATCH v2 2/6] selftests/landlock: Adapt existing bind/connect for UDP

[Matthieu Buffet]

Make basic changes to the existing bind() and connect() test suite
to also encompass testing UDP access control.

Signed-off-by: Matthieu Buffet <matthieu@buffet.re>
---
 tools/testing/selftests/landlock/base_test.c |   2 +-
 tools/testing/selftests/landlock/net_test.c  | 174 ++++++++++++++-----
 2 files changed, 135 insertions(+), 41 deletions(-)

diff --git a/tools/testing/selftests/landlock/base_test.c b/tools/testing/selftests/landlock/base_test.c
index 1bc16fde2e8a..fbd687691b3c 100644
--- a/tools/testing/selftests/landlock/base_test.c
+++ b/tools/testing/selftests/landlock/base_test.c
@@ -76,7 +76,7 @@ TEST(abi_version)
 	const struct landlock_ruleset_attr ruleset_attr = {
 		.handled_access_fs = LANDLOCK_ACCESS_FS_READ_FILE,
 	};
-	ASSERT_EQ(6, landlock_create_ruleset(NULL, 0,
+	ASSERT_EQ(7, landlock_create_ruleset(NULL, 0,
 					     LANDLOCK_CREATE_RULESET_VERSION));
 
 	ASSERT_EQ(-1, landlock_create_ruleset(&ruleset_attr, 0,
diff --git a/tools/testing/selftests/landlock/net_test.c b/tools/testing/selftests/landlock/net_test.c
index 4e0aeb53b225..40f66cccce69 100644
--- a/tools/testing/selftests/landlock/net_test.c
+++ b/tools/testing/selftests/landlock/net_test.c
@@ -34,6 +34,7 @@ enum sandbox_type {
 	NO_SANDBOX,
 	/* This may be used to test rules that allow *and* deny accesses. */
 	TCP_SANDBOX,
+	UDP_SANDBOX,
 };
 
 static int set_service(struct service_fixture *const srv,
@@ -94,6 +95,8 @@ static bool is_restricted(const struct protocol_variant *const prot,
 		switch (prot->type) {
 		case SOCK_STREAM:
 			return sandbox == TCP_SANDBOX;
+		case SOCK_DGRAM:
+			return sandbox == UDP_SANDBOX;
 		}
 		break;
 	}
@@ -409,6 +412,66 @@ FIXTURE_VARIANT_ADD(protocol, tcp_sandbox_with_unix_datagram) {
 	},
 };
 
+/* clang-format off */
+FIXTURE_VARIANT_ADD(protocol, udp_sandbox_with_ipv4_udp) {
+	/* clang-format on */
+	.sandbox = UDP_SANDBOX,
+	.prot = {
+		.domain = AF_INET,
+		.type = SOCK_DGRAM,
+	},
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(protocol, udp_sandbox_with_ipv4_tcp) {
+	/* clang-format on */
+	.sandbox = UDP_SANDBOX,
+	.prot = {
+		.domain = AF_INET,
+		.type = SOCK_STREAM,
+	},
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(protocol, udp_sandbox_with_ipv6_udp) {
+	/* clang-format on */
+	.sandbox = UDP_SANDBOX,
+	.prot = {
+		.domain = AF_INET6,
+		.type = SOCK_DGRAM,
+	},
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(protocol, udp_sandbox_with_ipv6_tcp) {
+	/* clang-format on */
+	.sandbox = UDP_SANDBOX,
+	.prot = {
+		.domain = AF_INET6,
+		.type = SOCK_STREAM,
+	},
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(protocol, udp_sandbox_with_unix_stream) {
+	/* clang-format on */
+	.sandbox = UDP_SANDBOX,
+	.prot = {
+		.domain = AF_UNIX,
+		.type = SOCK_STREAM,
+	},
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(protocol, udp_sandbox_with_unix_datagram) {
+	/* clang-format on */
+	.sandbox = UDP_SANDBOX,
+	.prot = {
+		.domain = AF_UNIX,
+		.type = SOCK_DGRAM,
+	},
+};
+
 static void test_bind_and_connect(struct __test_metadata *const _metadata,
 				  const struct service_fixture *const srv,
 				  const bool deny_bind, const bool deny_connect)
@@ -480,7 +543,11 @@ static void test_bind_and_connect(struct __test_metadata *const _metadata,
 	if (deny_bind) {
 		EXPECT_EQ(-EACCES, ret);
 	} else {
-		EXPECT_EQ(0, ret);
+		EXPECT_EQ(0, ret)
+		{
+			TH_LOG("Failed to bind socket: %s",
+			       strerror(errno));
+		}
 
 		/* Creates a listening socket. */
 		if (srv->protocol.type == SOCK_STREAM)
@@ -501,7 +568,7 @@ static void test_bind_and_connect(struct __test_metadata *const _metadata,
 		ret = connect_variant(connect_fd, srv);
 		if (deny_connect) {
 			EXPECT_EQ(-EACCES, ret);
-		} else if (deny_bind) {
+		} else if (deny_bind && srv->protocol.type == SOCK_STREAM) {
 			/* No listening server. */
 			EXPECT_EQ(-ECONNREFUSED, ret);
 		} else {
@@ -540,18 +607,23 @@ static void test_bind_and_connect(struct __test_metadata *const _metadata,
 
 TEST_F(protocol, bind)
 {
-	if (variant->sandbox == TCP_SANDBOX) {
+	if (variant->sandbox != NO_SANDBOX) {
+		__u64 bind_access = (variant->sandbox == TCP_SANDBOX ?
+					     LANDLOCK_ACCESS_NET_BIND_TCP :
+					     LANDLOCK_ACCESS_NET_BIND_UDP);
+		__u64 connect_access =
+			(variant->sandbox == TCP_SANDBOX ?
+				 LANDLOCK_ACCESS_NET_CONNECT_TCP :
+				 LANDLOCK_ACCESS_NET_CONNECT_UDP);
 		const struct landlock_ruleset_attr ruleset_attr = {
-			.handled_access_net = LANDLOCK_ACCESS_NET_BIND_TCP |
-					      LANDLOCK_ACCESS_NET_CONNECT_TCP,
+			.handled_access_net = bind_access | connect_access,
 		};
-		const struct landlock_net_port_attr tcp_bind_connect_p0 = {
-			.allowed_access = LANDLOCK_ACCESS_NET_BIND_TCP |
-					  LANDLOCK_ACCESS_NET_CONNECT_TCP,
+		const struct landlock_net_port_attr bind_connect_p0 = {
+			.allowed_access = bind_access | connect_access,
 			.port = self->srv0.port,
 		};
-		const struct landlock_net_port_attr tcp_connect_p1 = {
-			.allowed_access = LANDLOCK_ACCESS_NET_CONNECT_TCP,
+		const struct landlock_net_port_attr connect_p1 = {
+			.allowed_access = connect_access,
 			.port = self->srv1.port,
 		};
 		int ruleset_fd;
@@ -563,12 +635,12 @@ TEST_F(protocol, bind)
 		/* Allows connect and bind for the first port.  */
 		ASSERT_EQ(0,
 			  landlock_add_rule(ruleset_fd, LANDLOCK_RULE_NET_PORT,
-					    &tcp_bind_connect_p0, 0));
+					    &bind_connect_p0, 0));
 
 		/* Allows connect and denies bind for the second port. */
 		ASSERT_EQ(0,
 			  landlock_add_rule(ruleset_fd, LANDLOCK_RULE_NET_PORT,
-					    &tcp_connect_p1, 0));
+					    &connect_p1, 0));
 
 		enforce_ruleset(_metadata, ruleset_fd);
 		EXPECT_EQ(0, close(ruleset_fd));
@@ -590,18 +662,23 @@ TEST_F(protocol, bind)
 
 TEST_F(protocol, connect)
 {
-	if (variant->sandbox == TCP_SANDBOX) {
+	if (variant->sandbox != NO_SANDBOX) {
+		__u64 bind_access = (variant->sandbox == TCP_SANDBOX ?
+					     LANDLOCK_ACCESS_NET_BIND_TCP :
+					     LANDLOCK_ACCESS_NET_BIND_UDP);
+		__u64 connect_access =
+			(variant->sandbox == TCP_SANDBOX ?
+				 LANDLOCK_ACCESS_NET_CONNECT_TCP :
+				 LANDLOCK_ACCESS_NET_CONNECT_UDP);
 		const struct landlock_ruleset_attr ruleset_attr = {
-			.handled_access_net = LANDLOCK_ACCESS_NET_BIND_TCP |
-					      LANDLOCK_ACCESS_NET_CONNECT_TCP,
+			.handled_access_net = bind_access | connect_access,
 		};
-		const struct landlock_net_port_attr tcp_bind_connect_p0 = {
-			.allowed_access = LANDLOCK_ACCESS_NET_BIND_TCP |
-					  LANDLOCK_ACCESS_NET_CONNECT_TCP,
+		const struct landlock_net_port_attr bind_connect_p0 = {
+			.allowed_access = bind_access | connect_access,
 			.port = self->srv0.port,
 		};
-		const struct landlock_net_port_attr tcp_bind_p1 = {
-			.allowed_access = LANDLOCK_ACCESS_NET_BIND_TCP,
+		const struct landlock_net_port_attr bind_p1 = {
+			.allowed_access = bind_access,
 			.port = self->srv1.port,
 		};
 		int ruleset_fd;
@@ -613,12 +690,12 @@ TEST_F(protocol, connect)
 		/* Allows connect and bind for the first port. */
 		ASSERT_EQ(0,
 			  landlock_add_rule(ruleset_fd, LANDLOCK_RULE_NET_PORT,
-					    &tcp_bind_connect_p0, 0));
+					    &bind_connect_p0, 0));
 
 		/* Allows bind and denies connect for the second port. */
 		ASSERT_EQ(0,
 			  landlock_add_rule(ruleset_fd, LANDLOCK_RULE_NET_PORT,
-					    &tcp_bind_p1, 0));
+					    &bind_p1, 0));
 
 		enforce_ruleset(_metadata, ruleset_fd);
 		EXPECT_EQ(0, close(ruleset_fd));
@@ -636,16 +713,23 @@ TEST_F(protocol, connect)
 
 TEST_F(protocol, bind_unspec)
 {
-	const struct landlock_ruleset_attr ruleset_attr = {
-		.handled_access_net = LANDLOCK_ACCESS_NET_BIND_TCP,
-	};
-	const struct landlock_net_port_attr tcp_bind = {
-		.allowed_access = LANDLOCK_ACCESS_NET_BIND_TCP,
-		.port = self->srv0.port,
-	};
 	int bind_fd, ret;
 
-	if (variant->sandbox == TCP_SANDBOX) {
+	if (variant->sandbox != NO_SANDBOX) {
+		const struct landlock_ruleset_attr ruleset_attr = {
+			.handled_access_net =
+				(variant->sandbox == TCP_SANDBOX ?
+					 LANDLOCK_ACCESS_NET_BIND_TCP :
+					 LANDLOCK_ACCESS_NET_BIND_UDP),
+		};
+		const struct landlock_net_port_attr bind = {
+			.allowed_access =
+				(variant->sandbox == TCP_SANDBOX ?
+					 LANDLOCK_ACCESS_NET_BIND_TCP :
+					 LANDLOCK_ACCESS_NET_BIND_UDP),
+			.port = self->srv0.port,
+		};
+
 		const int ruleset_fd = landlock_create_ruleset(
 			&ruleset_attr, sizeof(ruleset_attr), 0);
 		ASSERT_LE(0, ruleset_fd);
@@ -653,7 +737,7 @@ TEST_F(protocol, bind_unspec)
 		/* Allows bind. */
 		ASSERT_EQ(0,
 			  landlock_add_rule(ruleset_fd, LANDLOCK_RULE_NET_PORT,
-					    &tcp_bind, 0));
+					    &bind, 0));
 		enforce_ruleset(_metadata, ruleset_fd);
 		EXPECT_EQ(0, close(ruleset_fd));
 	}
@@ -674,7 +758,13 @@ TEST_F(protocol, bind_unspec)
 	}
 	EXPECT_EQ(0, close(bind_fd));
 
-	if (variant->sandbox == TCP_SANDBOX) {
+	if (variant->sandbox != NO_SANDBOX) {
+		const struct landlock_ruleset_attr ruleset_attr = {
+			.handled_access_net =
+				(variant->sandbox == TCP_SANDBOX ?
+					 LANDLOCK_ACCESS_NET_BIND_TCP :
+					 LANDLOCK_ACCESS_NET_BIND_UDP),
+		};
 		const int ruleset_fd = landlock_create_ruleset(
 			&ruleset_attr, sizeof(ruleset_attr), 0);
 		ASSERT_LE(0, ruleset_fd);
@@ -718,10 +808,12 @@ TEST_F(protocol, bind_unspec)
 TEST_F(protocol, connect_unspec)
 {
 	const struct landlock_ruleset_attr ruleset_attr = {
-		.handled_access_net = LANDLOCK_ACCESS_NET_CONNECT_TCP,
+		.handled_access_net = LANDLOCK_ACCESS_NET_CONNECT_TCP |
+				      LANDLOCK_ACCESS_NET_CONNECT_UDP,
 	};
-	const struct landlock_net_port_attr tcp_connect = {
-		.allowed_access = LANDLOCK_ACCESS_NET_CONNECT_TCP,
+	const struct landlock_net_port_attr connect = {
+		.allowed_access = LANDLOCK_ACCESS_NET_CONNECT_TCP |
+				  LANDLOCK_ACCESS_NET_CONNECT_UDP,
 		.port = self->srv0.port,
 	};
 	int bind_fd, client_fd, status;
@@ -754,7 +846,7 @@ TEST_F(protocol, connect_unspec)
 			EXPECT_EQ(0, ret);
 		}
 
-		if (variant->sandbox == TCP_SANDBOX) {
+		if (variant->sandbox != NO_SANDBOX) {
 			const int ruleset_fd = landlock_create_ruleset(
 				&ruleset_attr, sizeof(ruleset_attr), 0);
 			ASSERT_LE(0, ruleset_fd);
@@ -762,7 +854,7 @@ TEST_F(protocol, connect_unspec)
 			/* Allows connect. */
 			ASSERT_EQ(0, landlock_add_rule(ruleset_fd,
 						       LANDLOCK_RULE_NET_PORT,
-						       &tcp_connect, 0));
+						       &connect, 0));
 			enforce_ruleset(_metadata, ruleset_fd);
 			EXPECT_EQ(0, close(ruleset_fd));
 		}
@@ -785,7 +877,7 @@ TEST_F(protocol, connect_unspec)
 			EXPECT_EQ(0, ret);
 		}
 
-		if (variant->sandbox == TCP_SANDBOX) {
+		if (variant->sandbox != NO_SANDBOX) {
 			const int ruleset_fd = landlock_create_ruleset(
 				&ruleset_attr, sizeof(ruleset_attr), 0);
 			ASSERT_LE(0, ruleset_fd);
@@ -1203,11 +1295,13 @@ FIXTURE_TEARDOWN(mini)
 
 /* clang-format off */
 
-#define ACCESS_LAST LANDLOCK_ACCESS_NET_CONNECT_TCP
+#define ACCESS_LAST LANDLOCK_ACCESS_NET_CONNECT_UDP
 
 #define ACCESS_ALL ( \
 	LANDLOCK_ACCESS_NET_BIND_TCP | \
-	LANDLOCK_ACCESS_NET_CONNECT_TCP)
+	LANDLOCK_ACCESS_NET_CONNECT_TCP | \
+	LANDLOCK_ACCESS_NET_BIND_UDP | \
+	LANDLOCK_ACCESS_NET_CONNECT_UDP)
 
 /* clang-format on */
 
-- 
2.39.5

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

[Matthieu Buffet]

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(-)

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)
-- 
2.39.5

[PATCH v2 4/6] selftests/landlock: Add ACCESS_NET_SENDTO_UDP

[Matthieu Buffet]

Add tests specific to UDP sendmsg(), orthogonal to whether the process
is allowed to bind()/connect(). Make these tests part of the protocol_*
variants, to ensure behaviour is consistent across AF_INET, AF_INET6
and AF_UNIX.

Signed-off-by: Matthieu Buffet <matthieu@buffet.re>
---
 tools/testing/selftests/landlock/net_test.c | 285 +++++++++++++++++++-
 1 file changed, 282 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/landlock/net_test.c b/tools/testing/selftests/landlock/net_test.c
index 40f66cccce69..05304e6b4d8f 100644
--- a/tools/testing/selftests/landlock/net_test.c
+++ b/tools/testing/selftests/landlock/net_test.c
@@ -256,9 +256,34 @@ static int connect_variant(const int sock_fd,
 	return connect_variant_addrlen(sock_fd, srv, get_addrlen(srv, false));
 }
 
+static int set_msg_dest(struct msghdr *msg,
+			const struct service_fixture *const srv,
+			const bool minimal_addrlen)
+{
+	switch (srv->protocol.domain) {
+	case AF_UNSPEC:
+	case AF_INET:
+		msg->msg_name = (void *)&srv->ipv4_addr;
+		break;
+	case AF_INET6:
+		msg->msg_name = (void *)&srv->ipv6_addr;
+		break;
+	case AF_UNIX:
+		msg->msg_name = (void *)&srv->unix_addr;
+		break;
+	default:
+		errno = -EAFNOSUPPORT;
+		return -errno;
+	}
+
+	msg->msg_namelen = get_addrlen(srv, minimal_addrlen);
+	return 0;
+}
+
 FIXTURE(protocol)
 {
-	struct service_fixture srv0, srv1, srv2, unspec_any0, unspec_srv0;
+	struct service_fixture srv0, srv1, srv2;
+	struct service_fixture unspec_any0, unspec_srv0, unspec_srv1;
 };
 
 FIXTURE_VARIANT(protocol)
@@ -281,6 +306,7 @@ FIXTURE_SETUP(protocol)
 	ASSERT_EQ(0, set_service(&self->srv2, variant->prot, 2));
 
 	ASSERT_EQ(0, set_service(&self->unspec_srv0, prot_unspec, 0));
+	ASSERT_EQ(0, set_service(&self->unspec_srv1, prot_unspec, 1));
 
 	ASSERT_EQ(0, set_service(&self->unspec_any0, prot_unspec, 0));
 	self->unspec_any0.ipv4_addr.sin_addr.s_addr = htonl(INADDR_ANY);
@@ -919,6 +945,258 @@ TEST_F(protocol, connect_unspec)
 	EXPECT_EQ(0, close(bind_fd));
 }
 
+TEST_F(protocol, sendmsg)
+{
+	const struct timeval read_timeout = {
+		.tv_sec = 0,
+		.tv_usec = 100000,
+	};
+	const bool sandboxed = variant->sandbox == UDP_SANDBOX &&
+			       variant->prot.type == SOCK_DGRAM &&
+			       (variant->prot.domain == AF_INET ||
+				variant->prot.domain == AF_INET6);
+	int srv1_fd, srv0_fd, client_fd;
+	char read_buf[1] = { 0 };
+	struct iovec testmsg_contents = { 0 };
+	struct msghdr testmsg = { 0 };
+
+	if (variant->prot.type != SOCK_DGRAM)
+		return;
+
+	disable_caps(_metadata);
+
+	/* Prepare one server socket to be denied */
+	ASSERT_EQ(0, set_service(&self->srv0, variant->prot, 0));
+	srv0_fd = socket_variant(&self->srv0);
+	ASSERT_LE(0, srv0_fd);
+	ASSERT_EQ(0, bind_variant(srv0_fd, &self->srv0));
+
+	/* Prepare one server socket on specific port to be allowed */
+	ASSERT_EQ(0, set_service(&self->srv1, variant->prot, 1));
+	srv1_fd = socket_variant(&self->srv1);
+	ASSERT_LE(0, srv1_fd);
+	ASSERT_EQ(0, bind_variant(srv1_fd, &self->srv1));
+
+	/* Arbitrary value just to not block other tests indefinitely */
+	EXPECT_EQ(0, setsockopt(srv1_fd, SOL_SOCKET, SO_RCVTIMEO, &read_timeout,
+				sizeof(read_timeout)));
+	EXPECT_EQ(0, setsockopt(srv0_fd, SOL_SOCKET, SO_RCVTIMEO, &read_timeout,
+				sizeof(read_timeout)));
+
+	client_fd = socket_variant(&self->srv0);
+	ASSERT_LE(0, client_fd);
+
+	if (sandboxed) {
+		const struct landlock_ruleset_attr ruleset_attr = {
+			.handled_access_net = LANDLOCK_ACCESS_NET_SENDTO_UDP |
+					      LANDLOCK_ACCESS_NET_BIND_UDP,
+		};
+		const struct landlock_net_port_attr allow_one_server = {
+			.allowed_access = LANDLOCK_ACCESS_NET_SENDTO_UDP,
+			.port = self->srv1.port,
+		};
+		const int ruleset_fd = landlock_create_ruleset(
+			&ruleset_attr, sizeof(ruleset_attr), 0);
+		ASSERT_LE(0, ruleset_fd);
+		ASSERT_EQ(0,
+			  landlock_add_rule(ruleset_fd, LANDLOCK_RULE_NET_PORT,
+					    &allow_one_server, 0));
+		enforce_ruleset(_metadata, ruleset_fd);
+		EXPECT_EQ(0, close(ruleset_fd));
+	}
+
+	testmsg_contents.iov_len = 1;
+	testmsg.msg_iov = &testmsg_contents;
+	testmsg.msg_iovlen = 1;
+
+	/* Without bind nor explicit address+port */
+	EXPECT_EQ(-1, write(client_fd, "A", 1));
+	if (variant->prot.domain == AF_UNIX) {
+		EXPECT_EQ(ENOTCONN, errno);
+	} else {
+		EXPECT_EQ(EDESTADDRREQ, errno);
+	}
+
+	/* With non-NULL but too small explicit address */
+	testmsg_contents.iov_base = "B";
+	EXPECT_EQ(0, set_msg_dest(&testmsg, &self->srv0, false));
+	testmsg.msg_namelen = get_addrlen(&self->srv0, true) - 1;
+	EXPECT_EQ(-1, sendmsg(client_fd, &testmsg, 0));
+	EXPECT_EQ(EINVAL, errno);
+
+	/* With minimal explicit address length and denied port */
+	testmsg_contents.iov_base = "C";
+	EXPECT_EQ(0, set_msg_dest(&testmsg, &self->srv0, true));
+	if (self->srv0.protocol.domain == AF_UNIX) {
+		EXPECT_EQ(-1, sendmsg(client_fd, &testmsg, 0));
+		EXPECT_EQ(EINVAL, errno);
+	} else if (sandboxed) {
+		EXPECT_EQ(-1, sendmsg(client_fd, &testmsg, 0));
+		EXPECT_EQ(EACCES, errno);
+	} else {
+		EXPECT_EQ(1, sendmsg(client_fd, &testmsg, 0));
+		EXPECT_EQ(1, read(srv0_fd, read_buf, 1));
+		EXPECT_EQ(read_buf[0], 'C');
+	}
+
+	/* With minimal explicit address length and allowed port */
+	testmsg_contents.iov_base = "D";
+	EXPECT_EQ(0, set_msg_dest(&testmsg, &self->srv1, true));
+	if (self->srv0.protocol.domain == AF_UNIX) {
+		EXPECT_EQ(-1, sendmsg(client_fd, &testmsg, 0));
+		EXPECT_EQ(EINVAL, errno);
+	} else {
+		EXPECT_EQ(1, sendmsg(client_fd, &testmsg, 0));
+		EXPECT_EQ(1, read(srv1_fd, read_buf, 1));
+		EXPECT_EQ(read_buf[0], 'D');
+	}
+
+	/* With explicit denied port */
+	testmsg_contents.iov_base = "E";
+	EXPECT_EQ(0, set_msg_dest(&testmsg, &self->srv0, false));
+	if (sandboxed) {
+		EXPECT_EQ(-1, sendmsg(client_fd, &testmsg, 0));
+		EXPECT_EQ(EACCES, errno);
+		EXPECT_EQ(-1, read(srv0_fd, read_buf, 1));
+		EXPECT_EQ(EAGAIN, errno);
+	} else {
+		EXPECT_EQ(1, sendmsg(client_fd, &testmsg, 0))
+		{
+			TH_LOG("sendmsg failed: %s", strerror(errno));
+		}
+		EXPECT_EQ(1, read(srv0_fd, read_buf, 1));
+		EXPECT_EQ(read_buf[0], 'E');
+	}
+
+	/* With explicit allowed port */
+	testmsg_contents.iov_base = "F";
+	EXPECT_EQ(0, set_msg_dest(&testmsg, &self->srv1, false));
+	EXPECT_EQ(1, sendmsg(client_fd, &testmsg, 0))
+	{
+		TH_LOG("sendmsg failed: %s", strerror(errno));
+	}
+	EXPECT_EQ(1, read(srv1_fd, read_buf, 1));
+	EXPECT_EQ(read_buf[0], 'F');
+
+	/*
+	 * With an explicit (AF_UNSPEC, port), should be equivalent to
+	 * (AF_INET, port) in IPv4, and to not specifying a destination
+	 * in IPv6 (which fails since the socket is not connected).
+	 */
+	testmsg_contents.iov_base = "G";
+	EXPECT_EQ(0, set_msg_dest(&testmsg, &self->unspec_srv0, false));
+	if (sandboxed || variant->prot.domain != AF_INET) {
+		EXPECT_EQ(-1, sendmsg(client_fd, &testmsg, 0));
+		if (variant->prot.domain == AF_INET) {
+			EXPECT_EQ(EACCES, errno);
+		} else if (variant->prot.domain == AF_UNIX) {
+			EXPECT_EQ(EINVAL, errno);
+		} else {
+			EXPECT_EQ(EDESTADDRREQ, errno);
+		}
+	} else {
+		EXPECT_EQ(1, sendmsg(client_fd, &testmsg, 0))
+		{
+			TH_LOG("sendmsg failed: %s", strerror(errno));
+		}
+		EXPECT_EQ(1, read(srv0_fd, read_buf, 1));
+		EXPECT_EQ(read_buf[0], 'G');
+	}
+	testmsg_contents.iov_base = "H";
+	EXPECT_EQ(0, set_msg_dest(&testmsg, &self->unspec_srv1, false));
+	if (variant->prot.domain == AF_INET) {
+		EXPECT_EQ(1, sendmsg(client_fd, &testmsg, 0))
+		{
+			TH_LOG("sendmsg failed: %s", strerror(errno));
+		}
+		EXPECT_EQ(1, read(srv1_fd, read_buf, 1));
+		EXPECT_EQ(read_buf[0], 'H');
+	} else {
+		EXPECT_EQ(-1, sendmsg(client_fd, &testmsg, 0));
+		if (variant->prot.domain == AF_INET6) {
+			EXPECT_EQ(EDESTADDRREQ, errno);
+		} else {
+			EXPECT_EQ(EINVAL, errno);
+		}
+	}
+
+	/* Without address, connect()ed to an allowed address+port */
+	testmsg.msg_name = NULL;
+	testmsg.msg_namelen = 0;
+	testmsg_contents.iov_base = "I";
+	ASSERT_EQ(0, connect_variant(client_fd, &self->srv1));
+	EXPECT_EQ(1, sendmsg(client_fd, &testmsg, 0))
+	{
+		TH_LOG("sendmsg failed: %s", strerror(errno));
+	}
+	EXPECT_EQ(1, read(srv1_fd, read_buf, 1));
+	EXPECT_EQ(read_buf[0], 'I');
+
+	/* Without address, connect()ed to a denied address+port */
+	testmsg.msg_name = NULL;
+	testmsg.msg_namelen = 0;
+	testmsg_contents.iov_base = "J";
+	ASSERT_EQ(0, connect_variant(client_fd, &self->srv0));
+	EXPECT_EQ(1, sendmsg(client_fd, &testmsg, 0))
+	{
+		TH_LOG("sendmsg failed: %s", strerror(errno));
+	}
+	EXPECT_EQ(1, read(srv0_fd, read_buf, 1));
+	EXPECT_EQ(read_buf[0], 'J');
+
+	/*
+	 * Sending to AF_UNSPEC should be equivalent to not specifying an
+	 * address (in IPv6 only) and falling back to the connected address
+	 */
+	testmsg_contents.iov_base = "K";
+	EXPECT_EQ(0, set_msg_dest(&testmsg, &self->unspec_srv0, false));
+	if (sandboxed && variant->prot.domain == AF_INET) {
+		/* IPv4 -> parsed as srv0 which is denied */
+		EXPECT_EQ(-1, sendmsg(client_fd, &testmsg, 0));
+		EXPECT_EQ(EACCES, errno);
+	} else if (variant->prot.domain == AF_UNIX) {
+		/* Unix socket don't accept AF_UNSPEC */
+		EXPECT_EQ(-1, sendmsg(client_fd, &testmsg, 0));
+		EXPECT_EQ(EINVAL, errno);
+	} else {
+		/*
+		 * IPv6 -> parsed as "no address" so it uses the connected
+		 * one srv1 which is allowed
+		 */
+		EXPECT_EQ(1, sendmsg(client_fd, &testmsg, 0))
+		{
+			TH_LOG("sendmsg failed: %s", strerror(errno));
+		}
+		EXPECT_EQ(1, read(srv0_fd, read_buf, 1));
+		EXPECT_EQ(read_buf[0], 'K');
+	}
+
+	testmsg_contents.iov_base = "L";
+	EXPECT_EQ(0, set_msg_dest(&testmsg, &self->unspec_srv1, false));
+	if (variant->prot.domain == AF_UNIX) {
+		EXPECT_EQ(-1, sendmsg(client_fd, &testmsg, 0));
+		EXPECT_EQ(EINVAL, errno);
+	} else {
+		EXPECT_EQ(1, sendmsg(client_fd, &testmsg, 0))
+		{
+			TH_LOG("sendmsg failed: %s", strerror(errno));
+		}
+
+		if (variant->prot.domain == AF_INET) {
+			/* IPv4 -> parsed as srv1 */
+			EXPECT_EQ(1, read(srv1_fd, read_buf, 1));
+		} else {
+			/*
+			 * IPv6 -> parsed as "no address" so it uses the
+			 * connected one, srv0
+			 */
+			EXPECT_EQ(1, read(srv0_fd, read_buf, 1));
+		}
+
+		EXPECT_EQ(read_buf[0], 'L');
+	}
+}
+
 FIXTURE(ipv4)
 {
 	struct service_fixture srv0, srv1;
@@ -1295,13 +1573,14 @@ FIXTURE_TEARDOWN(mini)
 
 /* clang-format off */
 
-#define ACCESS_LAST LANDLOCK_ACCESS_NET_CONNECT_UDP
+#define ACCESS_LAST LANDLOCK_ACCESS_NET_SENDTO_UDP
 
 #define ACCESS_ALL ( \
 	LANDLOCK_ACCESS_NET_BIND_TCP | \
 	LANDLOCK_ACCESS_NET_CONNECT_TCP | \
 	LANDLOCK_ACCESS_NET_BIND_UDP | \
-	LANDLOCK_ACCESS_NET_CONNECT_UDP)
+	LANDLOCK_ACCESS_NET_CONNECT_UDP | \
+	LANDLOCK_ACCESS_NET_SENDTO_UDP)
 
 /* clang-format on */
 
-- 
2.39.5

[PATCH v2 5/6] samples/landlock: Add sandboxer UDP access control

[Matthieu Buffet]

Add environment variables to control associated access rights:
(each one takes a list of ports separated by colons, like other
list options)

- LL_UDP_BIND
- LL_UDP_CONNECT
- LL_UDP_SENDTO

Signed-off-by: Matthieu Buffet <matthieu@buffet.re>
---
 samples/landlock/sandboxer.c | 58 ++++++++++++++++++++++++++++++++++--
 1 file changed, 56 insertions(+), 2 deletions(-)

diff --git a/samples/landlock/sandboxer.c b/samples/landlock/sandboxer.c
index 57565dfd74a2..61dc2645371e 100644
--- a/samples/landlock/sandboxer.c
+++ b/samples/landlock/sandboxer.c
@@ -58,6 +58,9 @@ static inline int landlock_restrict_self(const int ruleset_fd,
 #define ENV_TCP_BIND_NAME "LL_TCP_BIND"
 #define ENV_TCP_CONNECT_NAME "LL_TCP_CONNECT"
 #define ENV_SCOPED_NAME "LL_SCOPED"
+#define ENV_UDP_BIND_NAME "LL_UDP_BIND"
+#define ENV_UDP_CONNECT_NAME "LL_UDP_CONNECT"
+#define ENV_UDP_SENDTO_NAME "LL_UDP_SENDTO"
 #define ENV_DELIMITER ":"
 
 static int str2num(const char *numstr, __u64 *num_dst)
@@ -288,7 +291,7 @@ static bool check_ruleset_scope(const char *const env_var,
 
 /* clang-format on */
 
-#define LANDLOCK_ABI_LAST 6
+#define LANDLOCK_ABI_LAST 7
 
 #define XSTR(s) #s
 #define STR(s) XSTR(s)
@@ -311,6 +314,11 @@ static const char help[] =
 	"means an empty list):\n"
 	"* " ENV_TCP_BIND_NAME ": ports allowed to bind (server)\n"
 	"* " ENV_TCP_CONNECT_NAME ": ports allowed to connect (client)\n"
+	"* " ENV_UDP_BIND_NAME ": UDP ports allowed to bind (client: set as "
+	"source port / server: prepare to listen on port)\n"
+	"* " ENV_UDP_CONNECT_NAME ": UDP ports allowed to connect (client: "
+	"set as destination port / server: only receive from one client)\n"
+	"* " ENV_UDP_SENDTO_NAME ": UDP ports allowed to send to (client/server)\n"
 	"* " ENV_SCOPED_NAME ": actions denied on the outside of the landlock domain\n"
 	"  - \"a\" to restrict opening abstract unix sockets\n"
 	"  - \"s\" to restrict sending signals\n"
@@ -320,6 +328,8 @@ static const char help[] =
 	ENV_FS_RW_NAME "=\"/dev/null:/dev/full:/dev/zero:/dev/pts:/tmp\" "
 	ENV_TCP_BIND_NAME "=\"9418\" "
 	ENV_TCP_CONNECT_NAME "=\"80:443\" "
+	ENV_UDP_CONNECT_NAME "=\"53\" "
+	ENV_UDP_SENDTO_NAME "=\"53\" "
 	ENV_SCOPED_NAME "=\"a:s\" "
 	"%1$s bash -i\n"
 	"\n"
@@ -340,7 +350,10 @@ int main(const int argc, char *const argv[], char *const *const envp)
 	struct landlock_ruleset_attr ruleset_attr = {
 		.handled_access_fs = access_fs_rw,
 		.handled_access_net = LANDLOCK_ACCESS_NET_BIND_TCP |
-				      LANDLOCK_ACCESS_NET_CONNECT_TCP,
+				      LANDLOCK_ACCESS_NET_CONNECT_TCP |
+				      LANDLOCK_ACCESS_NET_BIND_UDP |
+				      LANDLOCK_ACCESS_NET_CONNECT_UDP |
+				      LANDLOCK_ACCESS_NET_SENDTO_UDP,
 		.scoped = LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET |
 			  LANDLOCK_SCOPE_SIGNAL,
 	};
@@ -415,6 +428,14 @@ int main(const int argc, char *const argv[], char *const *const envp)
 		/* Removes LANDLOCK_SCOPE_* for ABI < 6 */
 		ruleset_attr.scoped &= ~(LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET |
 					 LANDLOCK_SCOPE_SIGNAL);
+
+		__attribute__((fallthrough));
+	case 6:
+		/* Removes UDP support for ABI < 7 */
+		ruleset_attr.handled_access_fs &=
+			~(LANDLOCK_ACCESS_NET_BIND_UDP |
+			  LANDLOCK_ACCESS_NET_CONNECT_UDP |
+			  LANDLOCK_ACCESS_NET_SENDTO_UDP);
 		fprintf(stderr,
 			"Hint: You should update the running kernel "
 			"to leverage Landlock features "
@@ -445,6 +466,27 @@ int main(const int argc, char *const argv[], char *const *const envp)
 		ruleset_attr.handled_access_net &=
 			~LANDLOCK_ACCESS_NET_CONNECT_TCP;
 	}
+	/* Removes UDP bind access control if not supported by a user */
+	env_port_name = getenv(ENV_UDP_BIND_NAME);
+	if (!env_port_name) {
+		ruleset_attr.handled_access_net &=
+			~LANDLOCK_ACCESS_NET_BIND_UDP;
+	}
+	/* Removes UDP connect access control if not supported by a user */
+	env_port_name = getenv(ENV_UDP_CONNECT_NAME);
+	if (!env_port_name) {
+		ruleset_attr.handled_access_net &=
+			~LANDLOCK_ACCESS_NET_CONNECT_UDP;
+	}
+	/*
+	 * Removes UDP sendmsg(addr != NULL) access control if not
+	 * supported by a user
+	 */
+	env_port_name = getenv(ENV_UDP_SENDTO_NAME);
+	if (!env_port_name) {
+		ruleset_attr.handled_access_net &=
+			~LANDLOCK_ACCESS_NET_SENDTO_UDP;
+	}
 
 	if (check_ruleset_scope(ENV_SCOPED_NAME, &ruleset_attr))
 		return 1;
@@ -471,6 +513,18 @@ int main(const int argc, char *const argv[], char *const *const envp)
 				 LANDLOCK_ACCESS_NET_CONNECT_TCP)) {
 		goto err_close_ruleset;
 	}
+	if (populate_ruleset_net(ENV_UDP_BIND_NAME, ruleset_fd,
+				 LANDLOCK_ACCESS_NET_BIND_UDP)) {
+		goto err_close_ruleset;
+	}
+	if (populate_ruleset_net(ENV_UDP_CONNECT_NAME, ruleset_fd,
+				 LANDLOCK_ACCESS_NET_CONNECT_UDP)) {
+		goto err_close_ruleset;
+	}
+	if (populate_ruleset_net(ENV_UDP_SENDTO_NAME, ruleset_fd,
+				 LANDLOCK_ACCESS_NET_SENDTO_UDP)) {
+		goto err_close_ruleset;
+	}
 
 	if (prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0)) {
 		perror("Failed to restrict privileges");
-- 
2.39.5

[PATCH v2 6/6] doc: Add landlock UDP support

[Matthieu Buffet]

Add example of UDP usage, without detailing each access right, but with
an explicit note about the need to handle both SENDTO and CONNECT to
completely block sending (which could otherwise be overlooked).

Slightly change the example used in code blocks: build a ruleset for a
DNS client, so that it uses both TCP and UDP. Also consider an opaque
implementation so that we get to introduce both the right to connect()
and sendmsg(addr != NULL) within the same example.

Signed-off-by: Matthieu Buffet <matthieu@buffet.re>
---
 Documentation/userspace-api/landlock.rst | 84 +++++++++++++++++++-----
 1 file changed, 66 insertions(+), 18 deletions(-)

diff --git a/Documentation/userspace-api/landlock.rst b/Documentation/userspace-api/landlock.rst
index d639c61cb472..7018a03096e2 100644
--- a/Documentation/userspace-api/landlock.rst
+++ b/Documentation/userspace-api/landlock.rst
@@ -40,8 +40,8 @@ Filesystem rules
     and the related filesystem actions are defined with
     `filesystem access rights`.
 
-Network rules (since ABI v4)
-    For these rules, the object is a TCP port,
+Network rules (since ABI v4 for TCP and v7 for UDP)
+    For these rules, the object is a TCP or UDP port,
     and the related actions are defined with `network access rights`.
 
 Defining and enforcing a security policy
@@ -49,11 +49,11 @@ Defining and enforcing a security policy
 
 We first need to define the ruleset that will contain our rules.
 
-For this example, the ruleset will contain rules that only allow filesystem
-read actions and establish a specific TCP connection. Filesystem write
-actions and other TCP actions will be denied.
+For this example, the ruleset will contain rules that only allow some
+filesystem read actions and some specific UDP and TCP accesses. Filesystem
+write actions and other TCP/UDP actions will be denied.
 
-The ruleset then needs to handle both these kinds of actions.  This is
+The ruleset then needs to handle all these kinds of actions.  This is
 required for backward and forward compatibility (i.e. the kernel and user
 space may not know each other's supported restrictions), hence the need
 to be explicit about the denied-by-default access rights.
@@ -80,7 +80,10 @@ to be explicit about the denied-by-default access rights.
             LANDLOCK_ACCESS_FS_IOCTL_DEV,
         .handled_access_net =
             LANDLOCK_ACCESS_NET_BIND_TCP |
-            LANDLOCK_ACCESS_NET_CONNECT_TCP,
+            LANDLOCK_ACCESS_NET_CONNECT_TCP |
+            LANDLOCK_ACCESS_NET_BIND_UDP |
+            LANDLOCK_ACCESS_NET_CONNECT_UDP |
+            LANDLOCK_ACCESS_NET_SENDTO_UDP,
         .scoped =
             LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET |
             LANDLOCK_SCOPE_SIGNAL,
@@ -127,6 +130,12 @@ version, and only use the available subset of access rights:
         /* Removes LANDLOCK_SCOPE_* for ABI < 6 */
         ruleset_attr.scoped &= ~(LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET |
                                  LANDLOCK_SCOPE_SIGNAL);
+        __attribute__((fallthrough));
+    case 6:
+        /* Removes UDP support for ABI < 7 */
+        ruleset_attr.handled_access_net &= ~(LANDLOCK_ACCESS_NET_BIND_UDP |
+                                             LANDLOCK_ACCESS_NET_CONNECT_UDP |
+                                             LANDLOCK_ACCESS_NET_SENDTO_UDP);
     }
 
 This enables the creation of an inclusive ruleset that will contain our rules.
@@ -175,26 +184,53 @@ descriptor.
 
 It may also be required to create rules following the same logic as explained
 for the ruleset creation, by filtering access rights according to the Landlock
-ABI version.  In this example, this is not required because all of the requested
-``allowed_access`` rights are already available in ABI 1.
+ABI version.  So far, this was not required because all of the requested
+``allowed_access`` rights have always been available, from ABI 1.
 
-For network access-control, we can add a set of rules that allow to use a port
-number for a specific action: HTTPS connections.
+For network access-control, we will add a set of rules to allow DNS
+queries, which requires both UDP and TCP. For TCP, we need to allow
+outbound connections to port 53, which can be handled and granted starting
+with ABI 4:
 
 .. code-block:: c
 
-    struct landlock_net_port_attr net_port = {
-        .allowed_access = LANDLOCK_ACCESS_NET_CONNECT_TCP,
-        .port = 443,
-    };
+    if (ruleset_attr.handled_access_net & LANDLOCK_ACCESS_NET_CONNECT_TCP) {
+        struct landlock_net_port_attr net_port = {
+            .allowed_access = LANDLOCK_ACCESS_NET_CONNECT_TCP,
+            .port = 53,
+        };
+
+        err = landlock_add_rule(ruleset_fd, LANDLOCK_RULE_NET_PORT,
+                                &net_port, 0);
+
+We also need to be able to send UDP datagrams to port 53: we don't know if
+the client will call :manpage:`sendto(2)` with an explicit destination
+address, or :manpage:`connect(2)` then :manpage:`send(2)`, so we allow
+both. Note that granting ``LANDLOCK_ACCESS_NET_BIND_UDP`` is not necessary
+here because the client's socket will be automatically bound to an
+ephemeral port by the kernel. Also note that we need to handle both
+``LANDLOCK_ACCESS_NET_CONNECT_UDP`` and ``LANDLOCK_ACCESS_NET_SENDTO_UDP``
+to effectively block sending UDP datagrams to arbitrary ports.
+
+.. code-block:: c
+
+    if ((ruleset_attr.handled_access_net & (LANDLOCK_ACCESS_NET_CONNECT_UDP |
+                                            LANDLOCK_ACCESS_NET_SENDTO_UDP)) ==
+                                           (LANDLOCK_ACCESS_NET_CONNECT_UDP |
+                                            LANDLOCK_ACCESS_NET_SENDTO_UDP)) {
+        struct landlock_net_port_attr net_port = {
+            .allowed_access = LANDLOCK_ACCESS_NET_CONNECT_UDP |
+                              LANDLOCK_ACCESS_NET_SENDTO_UDP,
+            .port = 53,
+        };
 
-    err = landlock_add_rule(ruleset_fd, LANDLOCK_RULE_NET_PORT,
-                            &net_port, 0);
+        err = landlock_add_rule(ruleset_fd, LANDLOCK_RULE_NET_PORT,
+                                &net_port, 0);
 
 The next step is to restrict the current thread from gaining more privileges
 (e.g. through a SUID binary).  We now have a ruleset with the first rule
 allowing read access to ``/usr`` while denying all other handled accesses for
-the filesystem, and a second rule allowing HTTPS connections.
+the filesystem, and two more rules allowing DNS queries.
 
 .. code-block:: c
 
@@ -595,6 +631,18 @@ Starting with the Landlock ABI version 6, it is possible to restrict
 :manpage:`signal(7)` sending by setting ``LANDLOCK_SCOPE_SIGNAL`` to the
 ``scoped`` ruleset attribute.
 
+UDP networking (ABI < 7)
+------------------------
+
+Starting with the Landlock ABI version 7, it is possible to restrict
+sending and receiving UDP datagrams to/from specific ports. Restrictions
+are now enforced at :manpage:`bind(2)` time with the new
+``LANDLOCK_ACCESS_NET_BIND_UDP`` access right, and at :manpage:`connect(2)`
+time with ``LANDLOCK_ACCESS_NET_CONNECT_UDP``. Finally,
+``LANDLOCK_ACCESS_NET_SENDTO_UDP`` also restricts the destination port
+when sending datagrams with an explicit address, orthogonal to whether
+the socket is connected or not.
+
 .. _kernel_support:
 
 Kernel support
-- 
2.39.5

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

[kernel test robot]

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'

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

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

[kernel test robot]

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'

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

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

[Matthieu Buffet]

Hi,

(for netfilter folks added a bit late: this should be self-contained but 
original patch is here[1], it now raises a question about netfilter hook 
execution context at the end of this email - you can just skip to it if 
not interested in the LSM part)

On 12/14/2024 7:45 PM, Matthieu Buffet wrote:
> 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.
> [...]
> +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 (!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),
>   };

Looking back at this part of the patch to fix the stupid #ifdef, I 
noticed sk->sk_prot can change under our feet, just like sk->sk_family 
as highlighted by Mikhail in [2] due to setsockopt(IPV6_ADDRFORM).
Replacing the check with READ_ONCE(sock->sk->sk_family) == AF_INET6 or 
even taking the socket's lock would not change anything:
setsockopt(IPV6_ADDRFORM) runs concurrently and locklessly.

So with this patch, any Landlock domain with rights to connect(port A) 
and no port allowed to be set explicitly in sendto() could actually 
sendto(arbitrary port B) :
1. create an IPv6 UDP socket
2. connect it to (any IPv4-mapped-IPv6 like ::ffff:127.0.0.1, port A)
3a. sendmsg(AF_UNSPEC + actual IPv4 target, port B)
3b. race setsockopt(IPV6_ADDRFORM) on another thread
4. retry from 1. until sendmsg() succeeds

I've put together a quick PoC, the race works. SELinux does not have 
this problem because it uses a netfilter hook, later down the packet 
path. I see three "fixes", I probably missed some others:

A: block IPV6_ADDRFORM support in a setsockopt() hook, if UDP_SENDMSG is 
handled. AFAIU, not an option since this breaks a userland API

B: remove sendmsg(AF_UNSPEC) support on IPv6 sockets. Same problem as A

C: use a netfilter NF_INET_LOCAL_OUT hook like selinux_ip_output() 
instead of an LSM hook

For C, problem is to get the sender process' credentials, and ideally to 
avoid tagging sockets (what SELinux uses to fetch its security context, 
also why it does not have this problem). Otherwise, we would add another 
case of varying semantics (like rights to truncate/ioctl) to keep in 
mind for Landlock users, this time with sockets kept after enforcing a 
new ruleset, or passed to/from another domain - not a fan.

I don't know if it is safe to assume for UDP that NF_INET_LOCAL_OUT 
executes in process context: [3] doesn't specify, and [4] mentions the 
possibility to execute in interrupt context due to e.g. retransmits, but 
that does not apply to UDP. Looking at the code, it looks like it has to 
run in process context to be able to make the syscall return EPERM if 
the verdict is NF_DROP, but I don't know if that's something that can be 
relied upon to be always true, including in future revisions. Could use 
some input from someone knowledgeable in netfilter.

What do you think?

[1] https://lore.kernel.org/all/20241214184540.3835222-1-matthieu@buffet.re/
[2] https://lore.kernel.org/netdev/20241212.zoh7Eezee9ka@digikod.net/T/
[3] 
https://www.netfilter.org/documentation/HOWTO/netfilter-hacking-HOWTO-4.html#ss4.6
[4] 
https://netfilter-devel.vger.kernel.narkive.com/yZHiFEVh/execution-context-in-netfilter-hooks#post5

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

[Mikhail Ivanov]

On 1/21/2025 1:30 AM, Matthieu Buffet wrote:
> Hi,
> 
> (for netfilter folks added a bit late: this should be self-contained but 
> original patch is here[1], it now raises a question about netfilter hook 
> execution context at the end of this email - you can just skip to it if 
> not interested in the LSM part)
> 
> On 12/14/2024 7:45 PM, Matthieu Buffet wrote:
>> 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.
>> [...]
>> +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 (!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),
>>   };
> 
> Looking back at this part of the patch to fix the stupid #ifdef, I 
> noticed sk->sk_prot can change under our feet, just like sk->sk_family 
> as highlighted by Mikhail in [2] due to setsockopt(IPV6_ADDRFORM).
> Replacing the check with READ_ONCE(sock->sk->sk_family) == AF_INET6 or 
> even taking the socket's lock would not change anything:
> setsockopt(IPV6_ADDRFORM) runs concurrently and locklessly.
> 
> So with this patch, any Landlock domain with rights to connect(port A) 
> and no port allowed to be set explicitly in sendto() could actually 
> sendto(arbitrary port B) :
> 1. create an IPv6 UDP socket
> 2. connect it to (any IPv4-mapped-IPv6 like ::ffff:127.0.0.1, port A)
> 3a. sendmsg(AF_UNSPEC + actual IPv4 target, port B)
> 3b. race setsockopt(IPV6_ADDRFORM) on another thread
> 4. retry from 1. until sendmsg() succeeds
> 
> I've put together a quick PoC, the race works. SELinux does not have 
> this problem because it uses a netfilter hook, later down the packet 
> path. I see three "fixes", I probably missed some others:
> 
> A: block IPV6_ADDRFORM support in a setsockopt() hook, if UDP_SENDMSG is 
> handled. AFAIU, not an option since this breaks a userland API
> 
> B: remove sendmsg(AF_UNSPEC) support on IPv6 sockets. Same problem as A
> 
> C: use a netfilter NF_INET_LOCAL_OUT hook like selinux_ip_output() 
> instead of an LSM hook

We can naively follow the semantics of this flag: "This access right is
checked [...] when the destination address passed is not NULL", and
check address even for IPV6+AF_UNSPEC. Calling sendto() on IPV6 socket
with specified AF_UNSPEC address does not look like common or useful
practice and can be restricted.

> 
> For C, problem is to get the sender process' credentials, and ideally to 
> avoid tagging sockets (what SELinux uses to fetch its security context, 
> also why it does not have this problem). Otherwise, we would add another 
> case of varying semantics (like rights to truncate/ioctl) to keep in 
> mind for Landlock users, this time with sockets kept after enforcing a 
> new ruleset, or passed to/from another domain - not a fan.
> 
> I don't know if it is safe to assume for UDP that NF_INET_LOCAL_OUT 
> executes in process context: [3] doesn't specify, and [4] mentions the 
> possibility to execute in interrupt context due to e.g. retransmits, but 
> that does not apply to UDP. Looking at the code, it looks like it has to 
> run in process context to be able to make the syscall return EPERM if 
> the verdict is NF_DROP, but I don't know if that's something that can be 
> relied upon to be always true, including in future revisions. Could use 
> some input from someone knowledgeable in netfilter.
> 
> What do you think?
> 
> [1] 
> https://lore.kernel.org/all/20241214184540.3835222-1-matthieu@buffet.re/
> [2] https://lore.kernel.org/netdev/20241212.zoh7Eezee9ka@digikod.net/T/
> [3] 
> https://www.netfilter.org/documentation/HOWTO/netfilter-hacking-HOWTO-4.html#ss4.6
> [4] 
> https://netfilter-devel.vger.kernel.narkive.com/yZHiFEVh/execution-context-in-netfilter-hooks#post5

Re: [PATCH v2 0/6] landlock: Add UDP access control support

[Mikhail Ivanov]

On 12/14/2024 9:45 PM, Matthieu Buffet wrote:
> Hi Mickael,
> 
> Thanks for your comments on the v1 of this patch, I should have everything
> fixed so (hopefully) this v2 boils down to something simpler.
> 
> This patchset is based on
> https://git.kernel.org/pub/scm/linux/kernel/git/mic/linux.git
> Linux 6.12 (adc218676eef).
> 
> This patchset should add basic support to completely block a process
> from sending and receiving UDP datagrams, and delegate the right to
> send/receive based on remote/local port. It should fit nicely with
> the socket creation restrictions WIP (either don't have UDP at all, or
> have it with just the rights needed).
> 
> @Mikhail: I saw the discussions around TCP error code inconsistencies +
> over-restriction, and your patch v1. I took extra care to minimize this
> diff size: no unnecessary comment/refactor, especially in
> current_check_access_socket(). It should be just what is required for a
> basic UDP support without changing error handling in that main function.

Hello, Matthieu! Thank you for sending the second version and
checking these fix patches.

We decided to implement errors consistency fix for the whole LSM [1],
but your patches will merge well with current_check_access_socket()
refactoring [2].

[1] https://lore.kernel.org/all/20241210.ahg9Zawoobie@digikod.net/
[2] 
https://lore.kernel.org/all/20241017110454.265818-3-ivanov.mikhail1@huawei-partners.com/

> 
> The only question that remained open from v1 was about UDP rights naming.
> Since there were no strong preferences and the hooks now only handle
> sendmsg() if an explicit address is specified, that's now
> LANDLOCK_ACCESS_NET_UDP_SENDTO since the name (and prototype with a
> destination address parameter) of sendto(3) is closer to these semantics.
This can be interpreted as a restriction only for sendto(2), but I don't 
see any better options.

> 
> Changes since v1 (link below):
> - recvmsg hook is gone and sendmsg hook doesn't apply to connected
>    sockets anymore, to improve performance
> - don't add a get_addr_port() helper function, which required a weird "am
>    I in IPv4 or IPv6 context" to avoid a addrlen>sizeof(struct sockaddr_in)
>    check in connect(AF_UNSPEC) IPv6 context. A helper was useful when ports
>    also needed to be read in a recvmsg() hook, now it's just a simple
>    switch case in the sendmsg() hook, more readable
> - rename sendmsg access right to LANDLOCK_ACCESS_NET_UDP_SENDTO
> - reorder hook prologue for consistency: check domain, then type and
>    family
> - add additional selftests cases around minimal address length
> - update documentation
> 
> lcov gives me net.c going from 94% lines/80% functions to 96.6% lines/
> 85.7% functions
> 
> Any feedback welcome!
> 
> Link: https://lore.kernel.org/all/20240916122230.114800-1-matthieu@buffet.re/
> Closes: https://github.com/landlock-lsm/linux/issues/10
> 
> Link: https://lore.kernel.org/all/20241017110454.265818-1-ivanov.mikhail1@huawei-partners.com/
> Cc: Mikhail Ivanov <ivanov.mikhail1@huawei-partners.com>
> 
> Matthieu Buffet (6):
>    landlock: Add UDP bind+connect access control
>    selftests/landlock: Adapt existing bind/connect for UDP
>    landlock: Add UDP sendmsg access control
>    selftests/landlock: Add ACCESS_NET_SENDTO_UDP
>    samples/landlock: Add sandboxer UDP access control
>    doc: Add landlock UDP support
> 
>   Documentation/userspace-api/landlock.rst     |  84 +++-
>   include/uapi/linux/landlock.h                |  67 ++-
>   samples/landlock/sandboxer.c                 |  58 ++-
>   security/landlock/limits.h                   |   2 +-
>   security/landlock/net.c                      | 137 +++++-
>   security/landlock/syscalls.c                 |   2 +-
>   tools/testing/selftests/landlock/base_test.c |   2 +-
>   tools/testing/selftests/landlock/net_test.c  | 455 +++++++++++++++++--
>   8 files changed, 715 insertions(+), 92 deletions(-)
> 
> 
> base-commit: adc218676eef25575469234709c2d87185ca223a

Re: [PATCH v2 1/6] landlock: Add UDP bind+connect access control

[Mikhail Ivanov]

On 12/14/2024 9:45 PM, Matthieu Buffet wrote:
> If an app doesn't need to be able to open UDP sockets, it should be
> denied the right to create UDP sockets altogether (via seccomp and/or
> https://github.com/landlock-lsm/linux/issues/6 when it lands).
> For apps using UDP, add support for two more fine-grained access rights:
> 
> - LANDLOCK_ACCESS_NET_CONNECT_UDP, to gate the possibility to connect()
>    a UDP socket. For client apps (those which want to avoid specifying a
>    destination for each datagram in sendmsg()), and for a few servers
>    (those creating per-client sockets, which want to receive traffic only
>    from a specific address)
> 
> - LANDLOCK_ACCESS_NET_BIND_UDP, to gate the possibility to bind() a UDP
>    socket. For most servers (to start listening for datagrams on a
>    non-ephemeral port) and can be useful for some client applications (to
>    set the source port of future datagrams, e.g. mDNS requires to use
>    source port 5353)
> 
> No restriction is enforced on send()/recv() to preserve performance.
> The security boundary is to prevent acquiring a bound/connected socket.
> 
> Bump ABI to v7 to allow userland to detect and use these new restrictions.
> 
> Signed-off-by: Matthieu Buffet <matthieu@buffet.re>
> ---
>   include/uapi/linux/landlock.h | 53 ++++++++++++++++++++++++++---------
>   security/landlock/limits.h    |  2 +-
>   security/landlock/net.c       | 49 ++++++++++++++++++++++----------
>   security/landlock/syscalls.c  |  2 +-
>   4 files changed, 76 insertions(+), 30 deletions(-)
> 
> diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
> index 33745642f787..3f7b8e85822d 100644
> --- a/include/uapi/linux/landlock.h
> +++ b/include/uapi/linux/landlock.h
> @@ -119,12 +119,15 @@ struct landlock_net_port_attr {
>   	 *
>   	 * It should be noted that port 0 passed to :manpage:`bind(2)` will bind
>   	 * to an available port from the ephemeral port range.  This can be
> -	 * configured with the ``/proc/sys/net/ipv4/ip_local_port_range`` sysctl
> -	 * (also used for IPv6).
> +	 * configured globally with the
> +	 * ``/proc/sys/net/ipv4/ip_local_port_range`` sysctl (also used for
> +	 * IPv6), and, within that first range, on a per-socket basis using
> +	 * ``setsockopt(IP_LOCAL_PORT_RANGE)``.
>   	 *
> -	 * A Landlock rule with port 0 and the ``LANDLOCK_ACCESS_NET_BIND_TCP``
> -	 * right means that requesting to bind on port 0 is allowed and it will
> -	 * automatically translate to binding on the related port range.
> +	 * A Landlock rule with port 0 and the %LANDLOCK_ACCESS_NET_BIND_TCP
> +	 * or %LANDLOCK_ACCESS_NET_BIND_UDP right means that requesting to
> +	 * bind on port 0 is allowed and it will automatically translate to
> +	 * binding on the ephemeral port range.
>   	 */
>   	__u64 port;
>   };
> @@ -267,18 +270,42 @@ struct landlock_net_port_attr {
>    * Network flags
>    * ~~~~~~~~~~~~~~~~
>    *
> - * These flags enable to restrict a sandboxed process to a set of network
> - * actions. This is supported since the Landlock ABI version 4.
> - *
> - * The following access rights apply to TCP port numbers:
> - *
> - * - %LANDLOCK_ACCESS_NET_BIND_TCP: Bind a TCP socket to a local port.
> - * - %LANDLOCK_ACCESS_NET_CONNECT_TCP: Connect an active TCP socket to
> - *   a remote port.
> + * These flags enable to restrict which network-related actions a sandboxed
> + * process can take. TCP support was added in Landlock ABI version 4, and UDP
> + * support in version 7.

Better to place ABI version of TCP and UDP support in the related
headers (similar to fs flags).

> + *
> + * TCP access rights:
> + *
> + * - %LANDLOCK_ACCESS_NET_BIND_TCP: bind sockets to the given local port,
> + *   for servers that will listen() on that port, or for clients that want
> + *   to open connections with that specific source port instead of using a
> + *   kernel-assigned random ephemeral one
> + * - %LANDLOCK_ACCESS_NET_CONNECT_TCP: connect client sockets to servers
> + *   listening on that remote port

Changes related to refactoring are better to be placed in a separate
commit.

(Following notes related to UDP flags as well):

Please follow format of the comments: sentences should start with an
upper letter and end with a dot (Cf. fs flags).

I'm not sure if we need to explain the semantics of bind(2), connect(2)
here and use "server - client" wording.

I suggest specifying that socket is a "TCP socket" for the sake of
clarity.

> + *
> + * UDP access rights:
> + *
> + * - %LANDLOCK_ACCESS_NET_BIND_UDP: bind sockets to the given local port,
> + *   for servers that will listen() on that port, or for clients that want
> + *   to send datagrams with that specific source port instead of using a
> + *   kernel-assigned random ephemeral one

listen(2) is not supported for UDP sockets.

> + * - %LANDLOCK_ACCESS_NET_CONNECT_UDP: connect sockets to the given remote
> + *   port, either for clients that will send datagrams to that destination
> + *   (and want to send them faster without specifying an explicit address
> + *   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)

It's not very correct to say that a UDP socket *connects* to a remote
port with connect(2), since UDP is connectionless and in this case
connect(2) only assigns UDP socket with a destination port.

> + *
> + * Note that binding on port 0 means binding to an ephemeral
> + * kernel-assigned port, in the range configured in
> + * ``/proc/sys/net/ipv4/ip_local_port_range`` globally (and, within that
> + * range, on a per-socket basis with ``setsockopt(IP_LOCAL_PORT_RANGE)``).

I think it'll be better to have note about handling 0 port in a single
place only.

>    */
>   /* clang-format off */
>   #define LANDLOCK_ACCESS_NET_BIND_TCP			(1ULL << 0)
>   #define LANDLOCK_ACCESS_NET_CONNECT_TCP			(1ULL << 1)
> +#define LANDLOCK_ACCESS_NET_BIND_UDP			(1ULL << 2)
> +#define LANDLOCK_ACCESS_NET_CONNECT_UDP			(1ULL << 3)
>   /* clang-format on */
>   
>   /**
> diff --git a/security/landlock/limits.h b/security/landlock/limits.h
> index 15f7606066c8..ca90c1c56458 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_TCP
> +#define LANDLOCK_LAST_ACCESS_NET	LANDLOCK_ACCESS_NET_CONNECT_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 d5dcc4407a19..1c5cf2ddb7c1 100644
> --- a/security/landlock/net.c
> +++ b/security/landlock/net.c
> @@ -63,10 +63,6 @@ static int current_check_access_socket(struct socket *const sock,
>   	if (WARN_ON_ONCE(dom->num_layers < 1))
>   		return -EACCES;
>   
> -	/* Checks if it's a (potential) TCP socket. */
> -	if (sock->type != SOCK_STREAM)
> -		return 0;
> -
>   	/* Checks for minimal header length to safely read sa_family. */
>   	if (addrlen < offsetofend(typeof(*address), sa_family))
>   		return -EINVAL;
> @@ -94,17 +90,19 @@ static int current_check_access_socket(struct socket *const sock,
>   	/* Specific AF_UNSPEC handling. */
>   	if (address->sa_family == AF_UNSPEC) {
>   		/*
> -		 * Connecting to an address with AF_UNSPEC dissolves the TCP
> -		 * association, which have the same effect as closing the
> -		 * connection while retaining the socket object (i.e., the file
> -		 * descriptor).  As for dropping privileges, closing
> -		 * connections is always allowed.
> -		 *
> -		 * For a TCP access control system, this request is legitimate.
> +		 * Connecting to an address with AF_UNSPEC dissolves the
> +		 * remote association while retaining the socket object
> +		 * (i.e., the file descriptor). For TCP, it has the same
> +		 * effect as closing the connection. For UDP, it removes
> +		 * any preset destination for future datagrams.
> +		 * Like dropping privileges, these actions are always
> +		 * allowed: access control is performed when bind()ing or
> +		 * connect()ing.

May be better to remove the last line - "access control is performed..".

>   		 * Let the network stack handle potential inconsistencies and
>   		 * return -EINVAL if needed.
>   		 */
> -		if (access_request == LANDLOCK_ACCESS_NET_CONNECT_TCP)
> +		if (access_request == LANDLOCK_ACCESS_NET_CONNECT_TCP ||
> +		    access_request == LANDLOCK_ACCESS_NET_CONNECT_UDP)
>   			return 0;
>   
>   		/*
> @@ -118,7 +116,8 @@ static int current_check_access_socket(struct socket *const sock,
>   		 * checks, but it is safer to return a proper error and test
>   		 * consistency thanks to kselftest.
>   		 */
> -		if (access_request == LANDLOCK_ACCESS_NET_BIND_TCP) {
> +		if (access_request == LANDLOCK_ACCESS_NET_BIND_TCP ||
> +		    access_request == LANDLOCK_ACCESS_NET_BIND_UDP) {
>   			/* addrlen has already been checked for AF_UNSPEC. */
>   			const struct sockaddr_in *const sockaddr =
>   				(struct sockaddr_in *)address;
> @@ -159,16 +158,36 @@ static int current_check_access_socket(struct socket *const sock,
>   static int hook_socket_bind(struct socket *const sock,
>   			    struct sockaddr *const address, const int addrlen)
>   {
> +	access_mask_t access_request;
> +
> +	/* Checks if it's a (potential) TCP socket. */
> +	if (sock->type == SOCK_STREAM)
> +		access_request = LANDLOCK_ACCESS_NET_BIND_TCP;
> +	else if (sk_is_udp(sock->sk))
> +		access_request = LANDLOCK_ACCESS_NET_BIND_UDP;
> +	else
> +		return 0;
> +
>   	return current_check_access_socket(sock, address, addrlen,
> -					   LANDLOCK_ACCESS_NET_BIND_TCP);
> +					   access_request);
>   }
>   
>   static int hook_socket_connect(struct socket *const sock,
>   			       struct sockaddr *const address,
>   			       const int addrlen)
>   {
> +	access_mask_t access_request;
> +
> +	/* Checks if it's a (potential) TCP socket. */
> +	if (sock->type == SOCK_STREAM)
> +		access_request = LANDLOCK_ACCESS_NET_CONNECT_TCP;
> +	else if (sk_is_udp(sock->sk))
> +		access_request = LANDLOCK_ACCESS_NET_CONNECT_UDP;
> +	else
> +		return 0;
> +
>   	return current_check_access_socket(sock, address, addrlen,
> -					   LANDLOCK_ACCESS_NET_CONNECT_TCP);
> +					   access_request);
>   }
>   
>   static struct security_hook_list landlock_hooks[] __ro_after_init = {
> diff --git a/security/landlock/syscalls.c b/security/landlock/syscalls.c
> index c097d356fa45..200f771fa3a4 100644
> --- a/security/landlock/syscalls.c
> +++ b/security/landlock/syscalls.c
> @@ -150,7 +150,7 @@ static const struct file_operations ruleset_fops = {
>   	.write = fop_dummy_write,
>   };
>   
> -#define LANDLOCK_ABI_VERSION 6
> +#define LANDLOCK_ABI_VERSION 7
>   
>   /**
>    * sys_landlock_create_ruleset - Create a new ruleset

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

[Mikhail Ivanov]

On 12/14/2024 9:45 PM, Matthieu Buffet wrote:
> 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.

What do you mean by "delegating" here? I suggest changing last sentence
to something like

"This provides control over setting of the UDP socket destination
address in sendto(), sendmsg(), send(), sendmmsg(), complementing
the control of connect(2)".

> 
> 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.

I'm not sure if overhead is gonna be sensible for the average case, we
need to get some testing results first.

> 
> 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(-)
> 
> 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.

I suggest changing:
* "send datagrams" to "Send datagrams",
* "send datagrams" to "send UDP datagrams" for clarity,
* "This access right is not required when sending [...]" to "This access
   right do not control sending [...]".

Again, I don't think that overhead should be noted here: we do not have
any data yet.

> + *
> + * 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;

I suggest changing "connect()ed" to "assigned with destination address":
connected socket usually implies connection-oriented protocol and
"connect()ed" implies connect(2) operation, but socket may be connected
by previous sendto() call.

> +	 * - the socket is unconnected, so we can let the networking stack
> +	 *   reply -EDESTADDRREQ

nit: missing dot

> +	 */
> +	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;

IPv6 socket should return -EINVAL here (Cf. udpv6_sendmsg()).

> +	}
> +
> +	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)

Re: [PATCH v2 5/6] samples/landlock: Add sandboxer UDP access control

[Mikhail Ivanov]

On 12/14/2024 9:45 PM, Matthieu Buffet wrote:
> Add environment variables to control associated access rights:
> (each one takes a list of ports separated by colons, like other
> list options)
> 
> - LL_UDP_BIND
> - LL_UDP_CONNECT
> - LL_UDP_SENDTO
> 
> Signed-off-by: Matthieu Buffet <matthieu@buffet.re>
> ---
>   samples/landlock/sandboxer.c | 58 ++++++++++++++++++++++++++++++++++--
>   1 file changed, 56 insertions(+), 2 deletions(-)
> 
> diff --git a/samples/landlock/sandboxer.c b/samples/landlock/sandboxer.c
> index 57565dfd74a2..61dc2645371e 100644
> --- a/samples/landlock/sandboxer.c
> +++ b/samples/landlock/sandboxer.c
> @@ -58,6 +58,9 @@ static inline int landlock_restrict_self(const int ruleset_fd,
>   #define ENV_TCP_BIND_NAME "LL_TCP_BIND"
>   #define ENV_TCP_CONNECT_NAME "LL_TCP_CONNECT"
>   #define ENV_SCOPED_NAME "LL_SCOPED"
> +#define ENV_UDP_BIND_NAME "LL_UDP_BIND"
> +#define ENV_UDP_CONNECT_NAME "LL_UDP_CONNECT"
> +#define ENV_UDP_SENDTO_NAME "LL_UDP_SENDTO"
>   #define ENV_DELIMITER ":"
>   
>   static int str2num(const char *numstr, __u64 *num_dst)
> @@ -288,7 +291,7 @@ static bool check_ruleset_scope(const char *const env_var,
>   
>   /* clang-format on */
>   
> -#define LANDLOCK_ABI_LAST 6
> +#define LANDLOCK_ABI_LAST 7
>   
>   #define XSTR(s) #s
>   #define STR(s) XSTR(s)
> @@ -311,6 +314,11 @@ static const char help[] =
>   	"means an empty list):\n"
>   	"* " ENV_TCP_BIND_NAME ": ports allowed to bind (server)\n"
>   	"* " ENV_TCP_CONNECT_NAME ": ports allowed to connect (client)\n"
> +	"* " ENV_UDP_BIND_NAME ": UDP ports allowed to bind (client: set as "
> +	"source port / server: prepare to listen on port)\n"

listen(2) is not supported for UDP sockets.

> +	"* " ENV_UDP_CONNECT_NAME ": UDP ports allowed to connect (client: "
> +	"set as destination port / server: only receive from one client)\n"
> +	"* " ENV_UDP_SENDTO_NAME ": UDP ports allowed to send to (client/server)\n"
>   	"* " ENV_SCOPED_NAME ": actions denied on the outside of the landlock domain\n"
>   	"  - \"a\" to restrict opening abstract unix sockets\n"
>   	"  - \"s\" to restrict sending signals\n"
> @@ -320,6 +328,8 @@ static const char help[] =
>   	ENV_FS_RW_NAME "=\"/dev/null:/dev/full:/dev/zero:/dev/pts:/tmp\" "
>   	ENV_TCP_BIND_NAME "=\"9418\" "
>   	ENV_TCP_CONNECT_NAME "=\"80:443\" "
> +	ENV_UDP_CONNECT_NAME "=\"53\" "
> +	ENV_UDP_SENDTO_NAME "=\"53\" "
>   	ENV_SCOPED_NAME "=\"a:s\" "
>   	"%1$s bash -i\n"
>   	"\n"
> @@ -340,7 +350,10 @@ int main(const int argc, char *const argv[], char *const *const envp)
>   	struct landlock_ruleset_attr ruleset_attr = {
>   		.handled_access_fs = access_fs_rw,
>   		.handled_access_net = LANDLOCK_ACCESS_NET_BIND_TCP |
> -				      LANDLOCK_ACCESS_NET_CONNECT_TCP,
> +				      LANDLOCK_ACCESS_NET_CONNECT_TCP |
> +				      LANDLOCK_ACCESS_NET_BIND_UDP |
> +				      LANDLOCK_ACCESS_NET_CONNECT_UDP |
> +				      LANDLOCK_ACCESS_NET_SENDTO_UDP,
>   		.scoped = LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET |
>   			  LANDLOCK_SCOPE_SIGNAL,
>   	};
> @@ -415,6 +428,14 @@ int main(const int argc, char *const argv[], char *const *const envp)
>   		/* Removes LANDLOCK_SCOPE_* for ABI < 6 */
>   		ruleset_attr.scoped &= ~(LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET |
>   					 LANDLOCK_SCOPE_SIGNAL);
> +
> +		__attribute__((fallthrough));
> +	case 6:
> +		/* Removes UDP support for ABI < 7 */
> +		ruleset_attr.handled_access_fs &=

typo: handled_access_fs -> handled_access_net

> +			~(LANDLOCK_ACCESS_NET_BIND_UDP |
> +			  LANDLOCK_ACCESS_NET_CONNECT_UDP |
> +			  LANDLOCK_ACCESS_NET_SENDTO_UDP);
>   		fprintf(stderr,
>   			"Hint: You should update the running kernel "
>   			"to leverage Landlock features "
> @@ -445,6 +466,27 @@ int main(const int argc, char *const argv[], char *const *const envp)
>   		ruleset_attr.handled_access_net &=
>   			~LANDLOCK_ACCESS_NET_CONNECT_TCP;
>   	}
> +	/* Removes UDP bind access control if not supported by a user */

nit: missing dot here and in some other places.

> +	env_port_name = getenv(ENV_UDP_BIND_NAME);
> +	if (!env_port_name) {
> +		ruleset_attr.handled_access_net &=
> +			~LANDLOCK_ACCESS_NET_BIND_UDP;
> +	}
> +	/* Removes UDP connect access control if not supported by a user */
> +	env_port_name = getenv(ENV_UDP_CONNECT_NAME);
> +	if (!env_port_name) {
> +		ruleset_attr.handled_access_net &=
> +			~LANDLOCK_ACCESS_NET_CONNECT_UDP;
> +	}
> +	/*
> +	 * Removes UDP sendmsg(addr != NULL) access control if not
> +	 * supported by a user
> +	 */
> +	env_port_name = getenv(ENV_UDP_SENDTO_NAME);
> +	if (!env_port_name) {
> +		ruleset_attr.handled_access_net &=
> +			~LANDLOCK_ACCESS_NET_SENDTO_UDP;
> +	}
>   
>   	if (check_ruleset_scope(ENV_SCOPED_NAME, &ruleset_attr))
>   		return 1;
> @@ -471,6 +513,18 @@ int main(const int argc, char *const argv[], char *const *const envp)
>   				 LANDLOCK_ACCESS_NET_CONNECT_TCP)) {
>   		goto err_close_ruleset;
>   	}
> +	if (populate_ruleset_net(ENV_UDP_BIND_NAME, ruleset_fd,
> +				 LANDLOCK_ACCESS_NET_BIND_UDP)) {
> +		goto err_close_ruleset;
> +	}
> +	if (populate_ruleset_net(ENV_UDP_CONNECT_NAME, ruleset_fd,
> +				 LANDLOCK_ACCESS_NET_CONNECT_UDP)) {
> +		goto err_close_ruleset;
> +	}
> +	if (populate_ruleset_net(ENV_UDP_SENDTO_NAME, ruleset_fd,
> +				 LANDLOCK_ACCESS_NET_SENDTO_UDP)) {
> +		goto err_close_ruleset;
> +	}
>   
>   	if (prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0)) {
>   		perror("Failed to restrict privileges");