8/1/2024 5:45 PM, Mickaël Salaün wrote:
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);
Ok, let's add it. I just haven't seen this being a common practice in
the network stack.
+ /* 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;
+}
