lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20241214184540.3835222-4-matthieu@buffet.re>
Date: Sat, 14 Dec 2024 19:45:37 +0100
From: Matthieu Buffet <matthieu@...fet.re>
To: Mickael Salaun <mic@...ikod.net>
Cc: Gunther Noack <gnoack@...gle.com>,
	Mikhail Ivanov <ivanov.mikhail1@...wei-partners.com>,
	konstantin.meskhidze@...wei.com,
	Paul Moore <paul@...l-moore.com>,
	James Morris <jmorris@...ei.org>,
	"Serge E . Hallyn" <serge@...lyn.com>,
	linux-security-module@...r.kernel.org,
	netdev@...r.kernel.org,
	Matthieu Buffet <matthieu@...fet.re>
Subject: [PATCH v2 3/6] landlock: Add UDP sendmsg access control

Add support for a LANDLOCK_ACCESS_NET_SENDTO_UDP access right,
complementing the two previous LANDLOCK_ACCESS_NET_CONNECT_UDP and
LANDLOCK_ACCESS_NET_BIND_UDP.
It allows denying and delegating the right to sendto() datagrams with an
explicit destination address and port, without requiring to connect() the
socket first.

Performance is of course worse if you send many datagrams this way,
compared to just connect() then sending without an address (except if you
use sendmmsg() which caches LSM results). This may still be desired by
applications which send few enough datagrams to different clients that
opening and connecting a socket for each one of them is not worth it.

Signed-off-by: Matthieu Buffet <matthieu@...fet.re>
---
 include/uapi/linux/landlock.h | 14 ++++++
 security/landlock/limits.h    |  2 +-
 security/landlock/net.c       | 88 +++++++++++++++++++++++++++++++++++
 3 files changed, 103 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
index 3f7b8e85822d..8b355891e986 100644
--- a/include/uapi/linux/landlock.h
+++ b/include/uapi/linux/landlock.h
@@ -295,6 +295,19 @@ struct landlock_net_port_attr {
  *   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)
+ * - %LANDLOCK_ACCESS_NET_SENDTO_UDP: send datagrams with an explicit
+ *   destination address set to the given remote port. This access right
+ *   is checked in sendto(), sendmsg() and sendmmsg() when the destination
+ *   address passed is not NULL. This access right is not required when
+ *   sending datagrams without an explicit destination (via a connected
+ *   socket, e.g. with send()). Sending datagrams with explicit addresses
+ *   induces a non-negligible overhead, so calling connect() once and for
+ *   all should be preferred. When not possible and sending many datagrams,
+ *   using sendmmsg() may reduce the access control overhead.
+ *
+ * Blocking an application from sending UDP traffic requires adding both
+ * %LANDLOCK_ACCESS_NET_SENDTO_UDP and %LANDLOCK_ACCESS_NET_CONNECT_UDP
+ * to the handled access rights list.
  *
  * Note that binding on port 0 means binding to an ephemeral
  * kernel-assigned port, in the range configured in
@@ -306,6 +319,7 @@ struct landlock_net_port_attr {
 #define LANDLOCK_ACCESS_NET_CONNECT_TCP			(1ULL << 1)
 #define LANDLOCK_ACCESS_NET_BIND_UDP			(1ULL << 2)
 #define LANDLOCK_ACCESS_NET_CONNECT_UDP			(1ULL << 3)
+#define LANDLOCK_ACCESS_NET_SENDTO_UDP			(1ULL << 4)
 /* clang-format on */
 
 /**
diff --git a/security/landlock/limits.h b/security/landlock/limits.h
index ca90c1c56458..8d12ca39cf2e 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_UDP
+#define LANDLOCK_LAST_ACCESS_NET	LANDLOCK_ACCESS_NET_SENDTO_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 1c5cf2ddb7c1..0556d8a21d0b 100644
--- a/security/landlock/net.c
+++ b/security/landlock/net.c
@@ -10,6 +10,8 @@
 #include <linux/net.h>
 #include <linux/socket.h>
 #include <net/ipv6.h>
+#include <net/transp_v6.h>
+#include <net/ip.h>
 
 #include "common.h"
 #include "cred.h"
@@ -155,6 +157,27 @@ static int current_check_access_socket(struct socket *const sock,
 	return -EACCES;
 }
 
+static int check_access_port(const struct landlock_ruleset *const dom,
+			     access_mask_t access_request, __be16 port)
+{
+	layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_NET] = {};
+	const struct landlock_rule *rule;
+	const struct landlock_id id = {
+		.key.data = (__force uintptr_t)port,
+		.type = LANDLOCK_KEY_NET_PORT,
+	};
+	BUILD_BUG_ON(sizeof(port) > sizeof(id.key.data));
+
+	rule = landlock_find_rule(dom, id);
+	access_request = landlock_init_layer_masks(
+		dom, access_request, &layer_masks, LANDLOCK_KEY_NET_PORT);
+	if (landlock_unmask_layers(rule, access_request, &layer_masks,
+				   ARRAY_SIZE(layer_masks)))
+		return 0;
+
+	return -EACCES;
+}
+
 static int hook_socket_bind(struct socket *const sock,
 			    struct sockaddr *const address, const int addrlen)
 {
@@ -190,9 +213,74 @@ static int hook_socket_connect(struct socket *const sock,
 					   access_request);
 }
 
+static int hook_socket_sendmsg(struct socket *const sock,
+			       struct msghdr *const msg, const int size)
+{
+	const struct landlock_ruleset *const dom =
+		landlock_get_applicable_domain(landlock_get_current_domain(),
+					       any_net);
+	const struct sockaddr *address = (const struct sockaddr *)msg->msg_name;
+	const int addrlen = msg->msg_namelen;
+	__be16 port;
+
+	if (!dom)
+		return 0;
+	if (WARN_ON_ONCE(dom->num_layers < 1))
+		return -EACCES;
+	/*
+	 * If there is no explicit address in the message, we have no
+	 * policy to enforce here because either:
+	 * - the socket was previously connect()ed, so the appropriate
+	 *   access check has already been done back then;
+	 * - the socket is unconnected, so we can let the networking stack
+	 *   reply -EDESTADDRREQ
+	 */
+	if (!address)
+		return 0;
+
+	if (!sk_is_udp(sock->sk))
+		return 0;
+
+	/* Checks for minimal header length to safely read sa_family. */
+	if (addrlen < offsetofend(typeof(*address), sa_family))
+		return -EINVAL;
+
+	switch (address->sa_family) {
+	case AF_UNSPEC:
+		/*
+		 * Parsed as "no address" in udpv6_sendmsg(), which means
+		 * we fall back into the case checked earlier: policy was
+		 * enforced at connect() time, nothing to enforce here.
+		 */
+		if (sock->sk->sk_prot == &udpv6_prot)
+			return 0;
+		/* Parsed as "AF_INET" in udp_sendmsg() */
+		fallthrough;
+	case AF_INET:
+		if (addrlen < sizeof(struct sockaddr_in))
+			return -EINVAL;
+		port = ((struct sockaddr_in *)address)->sin_port;
+		break;
+
+#if IS_ENABLED(CONFIG_IPV6)
+	case AF_INET6:
+		if (addrlen < SIN6_LEN_RFC2133)
+			return -EINVAL;
+		port = ((struct sockaddr_in6 *)address)->sin6_port;
+		break;
+#endif /* IS_ENABLED(CONFIG_IPV6) */
+
+	default:
+		return -EAFNOSUPPORT;
+	}
+
+	return check_access_port(dom, LANDLOCK_ACCESS_NET_SENDTO_UDP, port);
+}
+
 static struct security_hook_list landlock_hooks[] __ro_after_init = {
 	LSM_HOOK_INIT(socket_bind, hook_socket_bind),
 	LSM_HOOK_INIT(socket_connect, hook_socket_connect),
+	LSM_HOOK_INIT(socket_sendmsg, hook_socket_sendmsg),
 };
 
 __init void landlock_add_net_hooks(void)
-- 
2.39.5


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ