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 <ivanov.mikhail1@xxxxxxxxxxxxxxxxxxx> > --- > 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(-) > 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; > +}
