Re: [RFC PATCH v1 2/9] landlock: Support TCP listen access-control
From: Mickaël Salaün <mic@digikod.net>
Date: 2024-08-01 14:52:11
Also in:
netdev, netfilter-devel
On Sun, Jul 28, 2024 at 08:25:55AM +0800, Mikhail Ivanov wrote:
LANDLOCK_ACCESS_NET_BIND_TCP is useful to limit the scope of "bindable" ports to forbid a malicious sandboxed process to impersonate a legitimate server process. However, bind(2) might be used by (TCP) clients to set the source port to a (legitimate) value. Controlling the ports that can be used for listening would allow (TCP) clients to explicitly bind to ports that are forbidden for listening. Such control is implemented with a new LANDLOCK_ACCESS_NET_LISTEN_TCP access right that restricts listening on undesired ports with listen(2). It's worth noticing that this access right doesn't affect changing backlog value using listen(2) on already listening socket. * Create new LANDLOCK_ACCESS_NET_LISTEN_TCP flag. * Add hook to socket_listen(), which checks whether the socket is allowed to listen on a binded local port. * Add check_tcp_socket_can_listen() helper, which validates socket attributes before the actual access right check. * Update `struct landlock_net_port_attr` documentation with control of binding to ephemeral port with listen(2) description. * Change ABI version to 6. Closes: https://github.com/landlock-lsm/linux/issues/15 Signed-off-by: Mikhail Ivanov <redacted> --- include/uapi/linux/landlock.h | 23 +++-- security/landlock/limits.h | 2 +- security/landlock/net.c | 90 ++++++++++++++++++++ security/landlock/syscalls.c | 2 +- tools/testing/selftests/landlock/base_test.c | 2 +- 5 files changed, 108 insertions(+), 11 deletions(-)
quoted hunk ↗ jump to hunk
diff --git a/security/landlock/net.c b/security/landlock/net.c index 669ba260342f..a29cb27c3f14 100644 --- a/security/landlock/net.c +++ b/security/landlock/net.c@@ -6,10 +6,12 @@ * Copyright © 2022-2023 Microsoft Corporation */ +#include "net/sock.h" #include <linux/in.h> #include <linux/net.h> #include <linux/socket.h> #include <net/ipv6.h> +#include <net/tcp.h> #include "common.h" #include "cred.h"@@ -194,9 +196,97 @@ static int hook_socket_connect(struct socket *const sock, LANDLOCK_ACCESS_NET_CONNECT_TCP); } +/* + * Checks that socket state and attributes are correct for listen. + * It is required to not wrongfully return -EACCES instead of -EINVAL. + * + * This checker requires sock->sk to be locked. + */ +static int check_tcp_socket_can_listen(struct socket *const sock) +{ + struct sock *sk = sock->sk; + unsigned char cur_sk_state = sk->sk_state; + const struct tcp_ulp_ops *icsk_ulp_ops; +
I think we can add this assert: lockdep_assert_held(&sk->sk_lock.slock);
+ /* Allows only unconnected TCP socket to listen (cf. inet_listen). */ + if (sock->state != SS_UNCONNECTED) + return -EINVAL; + + /* + * Checks sock state. This is needed to ensure consistency with inet stack + * error handling (cf. __inet_listen_sk). + */ + if (WARN_ON_ONCE(!((1 << cur_sk_state) & (TCPF_CLOSE | TCPF_LISTEN)))) + return -EINVAL; + + icsk_ulp_ops = inet_csk(sk)->icsk_ulp_ops; + + /* + * ULP (Upper Layer Protocol) stands for protocols which are higher than + * transport protocol in OSI model. Linux has an infrastructure that + * allows TCP sockets to support logic of some ULP (e.g. TLS ULP). + * + * Sockets can listen only if ULP control hook has clone method. + */ + if (icsk_ulp_ops && !icsk_ulp_ops->clone) + return -EINVAL; + return 0; +}