[RFC PATCH v1 3/7] landlock: Add UDP bind+connect access control

Matthieu Buffet posted 7 patches 2 months, 2 weeks ago
[RFC PATCH v1 3/7] landlock: Add UDP bind+connect access control
Posted by Matthieu Buffet 2 months, 2 weeks ago
Add support for two more access rights:

- LANDLOCK_ACCESS_NET_CONNECT_UDP, to gate the possibility to connect()
  an inet SOCK_DGRAM socket. This will be used by some client applications
  (those who want to avoid specifying a destination for each datagram in
  sendmsg), and for a few servers (those creating a socket per-client, who
  want to only receive traffic from each client on these sockets)

- LANDLOCK_ACCESS_NET_BIND_UDP, to gate the possibility to bind() an
  inet SOCK_DGRAM socket. This will be required for most server
  applications (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)

Also bump the ABI version from 5 to 6 so that userland can detect
whether these rights are supported and actually use them.

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

diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
index 2c8dbc74b955..7f9aa1cd2912 100644
--- a/include/uapi/linux/landlock.h
+++ b/include/uapi/linux/landlock.h
@@ -113,12 +113,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 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.
+	 * 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 related port range.
 	 */
 	__u64 port;
 };
@@ -261,17 +264,38 @@ 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 6.
+ *
+ * TCP access rights:
+ * - %LANDLOCK_ACCESS_NET_BIND_TCP: bind sockets to the given local port,
+ *   for servers that will listen on that port
+ * - %LANDLOCK_ACCESS_NET_CONNECT_TCP: connect sockets to the given remote port,
+ *   to establish client connections to servers listening on that port
+ *
+ * UDP access rights:
+ * - %LANDLOCK_ACCESS_NET_BIND_UDP: bind sockets to the given local port,
+ *   either for servers that will listen on that port, or for clients wishing
+ *   to set the source port of datagrams they will send instead of using a
+ *   kernel-assigned random ephemeral port (some protocols require a specific
+ *   source port, e.g. mDNS with UDP/5353)
+ * - %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 (if you create a client-specific socket for a
+ *   client-specific process, e.g. using the established-over-unconnected
+ *   method)
+ *
+ * Note that ``bind(0)`` means binding to an ephemeral kernel-assigned port,
+ * in the range configured in ``/proc/sys/net/ipv4/ip_local_port_range``
+ * globally (or 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 */
 #endif /* _UAPI_LINUX_LANDLOCK_H */
diff --git a/security/landlock/limits.h b/security/landlock/limits.h
index 4eb643077a2a..182b6a8d2976 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 c8bcd29bde09..becc62c02cc9 100644
--- a/security/landlock/net.c
+++ b/security/landlock/net.c
@@ -79,8 +79,8 @@ 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)
+	/* Checks if it's a (potential) UDP or TCP socket. */
+	if (sock->type != SOCK_STREAM && sock->type != SOCK_DGRAM)
 		return 0;
 
 	/* Checks for minimal header length to safely read sa_family. */
@@ -110,17 +110,18 @@ 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 socket
+		 * association. For SOCK_STREAM, it has the same effect as closing
+		 * the connection while retaining the socket object (i.e., the
+		 * file descriptor). For SOCK_DGRAM, it removes any configured
+		 * destination address. Both cases remove accessible resources, so
+		 * they are always legitimate and allowed, like dropping any
+		 * privilege.
 		 * 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;
 
 		/*
@@ -134,7 +135,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;
@@ -175,16 +177,42 @@ 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;
+
+	/*
+	 * Check if it's a (potential) TCP or UDP socket. These checks could
+	 * match e.g. Unix, netlink, or udplite sockets.
+	 */
+	if (sock->type == SOCK_STREAM)
+		access_request = LANDLOCK_ACCESS_NET_BIND_TCP;
+	else if (sock->type == SOCK_DGRAM)
+		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;
+
+	/*
+	 * Check if it's a (potential) TCP or UDP socket. These checks could
+	 * match e.g. Unix, netlink, or udplite sockets.
+	 */
+	if (sock->type == SOCK_STREAM)
+		access_request = LANDLOCK_ACCESS_NET_CONNECT_TCP;
+	else if (sock->type == SOCK_DGRAM)
+		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 ccc8bc6c1584..328198e8a9f5 100644
--- a/security/landlock/syscalls.c
+++ b/security/landlock/syscalls.c
@@ -149,7 +149,7 @@ static const struct file_operations ruleset_fops = {
 	.write = fop_dummy_write,
 };
 
-#define LANDLOCK_ABI_VERSION 5
+#define LANDLOCK_ABI_VERSION 6
 
 /**
  * sys_landlock_create_ruleset - Create a new ruleset
-- 
2.39.5
Re: [RFC PATCH v1 3/7] landlock: Add UDP bind+connect access control
Posted by Mickaël Salaün 2 months, 1 week ago
On Mon, Sep 16, 2024 at 02:22:26PM +0200, Matthieu Buffet wrote:
> Add support for two more access rights:
> 
> - LANDLOCK_ACCESS_NET_CONNECT_UDP, to gate the possibility to connect()
>   an inet SOCK_DGRAM socket. This will be used by some client applications
>   (those who want to avoid specifying a destination for each datagram in
>   sendmsg), and for a few servers (those creating a socket per-client, who
>   want to only receive traffic from each client on these sockets)
> 
> - LANDLOCK_ACCESS_NET_BIND_UDP, to gate the possibility to bind() an
>   inet SOCK_DGRAM socket. This will be required for most server
>   applications (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)
> 
> Also bump the ABI version from 5 to 6 so that userland can detect
> whether these rights are supported and actually use them.
> 

Closes: https://github.com/landlock-lsm/linux/issues/10

> Signed-off-by: Matthieu Buffet <matthieu@buffet.re>
> ---
>  include/uapi/linux/landlock.h | 48 +++++++++++++++++++++++--------
>  security/landlock/limits.h    |  2 +-
>  security/landlock/net.c       | 54 ++++++++++++++++++++++++++---------
>  security/landlock/syscalls.c  |  2 +-
>  4 files changed, 79 insertions(+), 27 deletions(-)
> 
> diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
> index 2c8dbc74b955..7f9aa1cd2912 100644
> --- a/include/uapi/linux/landlock.h
> +++ b/include/uapi/linux/landlock.h
> @@ -113,12 +113,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 on a per-socket basis using
> +	 * ``setsockopt(IP_LOCAL_PORT_RANGE)``.

Interesting... setsockopt(IP_LOCAL_PORT_RANGE) can always be set
independant of the sysctl, but fortunately it is only taken into account
if it fits into the sysctl range (see inet_sk_get_local_port_range),
which makes sense.

>  	 *
>  	 * 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.
> +	 * 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 related port range.
>  	 */
>  	__u64 port;
>  };