[PATCH v2 1/6] landlock: Add UDP bind+connect access control
Mikhail Ivanov
ivanov.mikhail1 at huawei-partners.com
Fri Jan 24 11:01:08 UTC 2025
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 at 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
More information about the Linux-security-module-archive
mailing list