[RFC PATCH v1 00/10] Socket type control for Landlock

[RFC PATCH v1 00/10] Socket type control for Landlock

[Ivanov Mikhail]

Patchset implements new type of Landlock rule, that restricts actions for
sockets of any protocol. Such restriction would be useful to ensure
that a sandboxed process uses only necessary protocols.
See [2] for more cases.

The rules store information about the socket family(aka domain) and type.

struct landlock_socket_attr {
	__u64 allowed_access;
	int domain; // see socket(2)
	int type; // see socket(2)
}

Patchset currently implements rule only for socket_create() method, but
other necessary rules will also be impemented. [1]

Code coverage(gcov) report with the launch of all the landlock selftests:
* security/landlock:
lines......: 94.7% (784 of 828 lines)
functions..: 97.2% (105 of 108 functions)

* security/landlock/socket.c:
lines......: 100.0% (33 of 33 lines)
functions..: 100.0% (5 of 5 functions)

[1] https://lore.kernel.org/all/b8a2045a-e7e8-d141-7c01-bf47874c7930@digikod.net/
[2] https://lore.kernel.org/all/ZJvy2SViorgc+cZI@google.com/

Ivanov Mikhail (10):
  landlock: Support socket access-control
  landlock: Add hook on socket_create()
  selftests/landlock: Create 'create' test
  selftests/landlock: Create 'socket_access_rights' test
  selftests/landlock: Create 'rule_with_unknown_access' test
  selftests/landlock: Create 'rule_with_unhandled_access' test
  selftests/landlock: Create 'inval' test
  selftests/landlock: Create 'ruleset_overlap' test
  selftests/landlock: Create 'ruleset_with_unknown_access' test
  samples/landlock: Support socket protocol restrictions

 include/uapi/linux/landlock.h                 |  49 ++
 samples/landlock/sandboxer.c                  | 149 +++++-
 security/landlock/Makefile                    |   2 +-
 security/landlock/limits.h                    |   5 +
 security/landlock/net.c                       |   2 +-
 security/landlock/ruleset.c                   |  35 +-
 security/landlock/ruleset.h                   |  44 +-
 security/landlock/setup.c                     |   2 +
 security/landlock/socket.c                    | 115 +++++
 security/landlock/socket.h                    |  19 +
 security/landlock/syscalls.c                  |  55 ++-
 tools/testing/selftests/landlock/base_test.c  |   2 +-
 .../testing/selftests/landlock/socket_test.c  | 457 ++++++++++++++++++
 13 files changed, 910 insertions(+), 26 deletions(-)
 create mode 100644 security/landlock/socket.c
 create mode 100644 security/landlock/socket.h
 create mode 100644 tools/testing/selftests/landlock/socket_test.c

-- 
2.34.1

[RFC PATCH v1 01/10] landlock: Support socket access-control

[Ivanov Mikhail]

Add new socket-related rule type, presented via landlock_socket_attr
struct. Add all neccessary entities for socket ruleset support.
Add flag LANDLOCK_ACCESS_SOCKET_CREATE that will provide the
ability to control socket creation.

Change landlock_key.data type from uinptr_t to u64. Socket rule has to
contain 32-bit socket family and type values, so landlock_key can be
represented as 64-bit number the first 32 bits of which correspond to
the socket family and last - to the type.

Change ABI version to 5.

Signed-off-by: Ivanov Mikhail <ivanov.mikhail1@huawei-partners.com>
Reviewed-by: Konstantin Meskhidze <konstantin.meskhidze@huawei.com>
---
 include/uapi/linux/landlock.h                | 49 +++++++++++++++++
 security/landlock/Makefile                   |  2 +-
 security/landlock/limits.h                   |  5 ++
 security/landlock/net.c                      |  2 +-
 security/landlock/ruleset.c                  | 35 +++++++++++--
 security/landlock/ruleset.h                  | 44 ++++++++++++++--
 security/landlock/socket.c                   | 43 +++++++++++++++
 security/landlock/socket.h                   | 17 ++++++
 security/landlock/syscalls.c                 | 55 ++++++++++++++++++--
 tools/testing/selftests/landlock/base_test.c |  2 +-
 10 files changed, 241 insertions(+), 13 deletions(-)
 create mode 100644 security/landlock/socket.c
 create mode 100644 security/landlock/socket.h

diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
index 25c8d7677..8551ade38 100644
--- a/include/uapi/linux/landlock.h
+++ b/include/uapi/linux/landlock.h
@@ -37,6 +37,13 @@ struct landlock_ruleset_attr {
 	 * rule explicitly allow them.
 	 */
 	__u64 handled_access_net;
+
+	/**
+	 * @handled_access_net: Bitmask of actions (cf. `Socket flags`_)
+	 * that is handled by this ruleset and should then be forbidden if no
+	 * rule explicitly allow them.
+	 */
+	__u64 handled_access_socket;
 };
 
 /*
@@ -65,6 +72,11 @@ enum landlock_rule_type {
 	 * landlock_net_port_attr .
 	 */
 	LANDLOCK_RULE_NET_PORT,
+	/**
+	 * @LANDLOCK_RULE_SOCKET: Type of a &struct
+	 * landlock_socket_attr .
+	 */
+	LANDLOCK_RULE_SOCKET,
 };
 
 /**
@@ -115,6 +127,27 @@ struct landlock_net_port_attr {
 	__u64 port;
 };
 
+/**
+ * struct landlock_socket_attr - Socket definition
+ *
+ * Argument of sys_landlock_add_rule().
+ */
+struct landlock_socket_attr {
+	/**
+	 * @allowed_access: Bitmask of allowed access for a socket
+	 * (cf. `Socket flags`_).
+	 */
+	__u64 allowed_access;
+	/**
+	 * @domain: Protocol family used for communication (see socket(2)).
+	 */
+	int domain;
+	/**
+	 * @type: Socket type (see socket(2)).
+	 */
+	int type;
+};
+
 /**
  * DOC: fs_access
  *
@@ -244,4 +277,20 @@ struct landlock_net_port_attr {
 #define LANDLOCK_ACCESS_NET_BIND_TCP			(1ULL << 0)
 #define LANDLOCK_ACCESS_NET_CONNECT_TCP			(1ULL << 1)
 /* clang-format on */
+
+/**
+ * DOC: socket_acess
+ *
+ * Socket flags
+ * ~~~~~~~~~~~~~~~~
+ *
+ * These flags enable to restrict a sandboxed process to a set of
+ * socket-related actions for specific protocols. This is supported
+ * since the Landlock ABI version 5.
+ *
+ * - %LANDLOCK_ACCESS_SOCKET_CREATE: Create a socket
+ */
+/* clang-format off */
+#define LANDLOCK_ACCESS_SOCKET_CREATE			(1ULL << 0)
+/* clang-format on */
 #endif /* _UAPI_LINUX_LANDLOCK_H */
diff --git a/security/landlock/Makefile b/security/landlock/Makefile
index b4538b7cf..ff1dd98f6 100644
--- a/security/landlock/Makefile
+++ b/security/landlock/Makefile
@@ -1,6 +1,6 @@
 obj-$(CONFIG_SECURITY_LANDLOCK) := landlock.o
 
 landlock-y := setup.o syscalls.o object.o ruleset.o \
-	cred.o task.o fs.o
+	cred.o task.o fs.o socket.o
 
 landlock-$(CONFIG_INET) += net.o
diff --git a/security/landlock/limits.h b/security/landlock/limits.h
index 93c9c6f91..ebdab587c 100644
--- a/security/landlock/limits.h
+++ b/security/landlock/limits.h
@@ -28,6 +28,11 @@
 #define LANDLOCK_NUM_ACCESS_NET		__const_hweight64(LANDLOCK_MASK_ACCESS_NET)
 #define LANDLOCK_SHIFT_ACCESS_NET	LANDLOCK_NUM_ACCESS_FS
 
+#define LANDLOCK_LAST_ACCESS_SOCKET	    LANDLOCK_ACCESS_SOCKET_CREATE
+#define LANDLOCK_MASK_ACCESS_SOCKET	    ((LANDLOCK_LAST_ACCESS_SOCKET << 1) - 1)
+#define LANDLOCK_NUM_ACCESS_SOCKET		__const_hweight64(LANDLOCK_MASK_ACCESS_SOCKET)
+#define LANDLOCK_SHIFT_ACCESS_SOCKET	LANDLOCK_NUM_ACCESS_SOCKET
+
 /* clang-format on */
 
 #endif /* _SECURITY_LANDLOCK_LIMITS_H */
diff --git a/security/landlock/net.c b/security/landlock/net.c
index c8bcd29bd..0e3770d14 100644
--- a/security/landlock/net.c
+++ b/security/landlock/net.c
@@ -159,7 +159,7 @@ static int current_check_access_socket(struct socket *const sock,
 			return -EINVAL;
 	}
 
-	id.key.data = (__force uintptr_t)port;
+	id.key.data = (__force u64)port;
 	BUILD_BUG_ON(sizeof(port) > sizeof(id.key.data));
 
 	rule = landlock_find_rule(dom, id);
diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c
index e0a5fbf92..1f1ed8181 100644
--- a/security/landlock/ruleset.c
+++ b/security/landlock/ruleset.c
@@ -40,6 +40,7 @@ static struct landlock_ruleset *create_ruleset(const u32 num_layers)
 #if IS_ENABLED(CONFIG_INET)
 	new_ruleset->root_net_port = RB_ROOT;
 #endif /* IS_ENABLED(CONFIG_INET) */
+	new_ruleset->root_socket = RB_ROOT;
 
 	new_ruleset->num_layers = num_layers;
 	/*
@@ -52,12 +53,13 @@ static struct landlock_ruleset *create_ruleset(const u32 num_layers)
 
 struct landlock_ruleset *
 landlock_create_ruleset(const access_mask_t fs_access_mask,
-			const access_mask_t net_access_mask)
+			const access_mask_t net_access_mask,
+			const access_mask_t socket_access_mask)
 {
 	struct landlock_ruleset *new_ruleset;
 
 	/* Informs about useless ruleset. */
-	if (!fs_access_mask && !net_access_mask)
+	if (!fs_access_mask && !net_access_mask && !socket_access_mask)
 		return ERR_PTR(-ENOMSG);
 	new_ruleset = create_ruleset(1);
 	if (IS_ERR(new_ruleset))
@@ -66,6 +68,8 @@ landlock_create_ruleset(const access_mask_t fs_access_mask,
 		landlock_add_fs_access_mask(new_ruleset, fs_access_mask, 0);
 	if (net_access_mask)
 		landlock_add_net_access_mask(new_ruleset, net_access_mask, 0);
+	if (socket_access_mask)
+		landlock_add_socket_access_mask(new_ruleset, socket_access_mask, 0);
 	return new_ruleset;
 }
 
@@ -89,6 +93,9 @@ static bool is_object_pointer(const enum landlock_key_type key_type)
 		return false;
 #endif /* IS_ENABLED(CONFIG_INET) */
 
+	case LANDLOCK_KEY_SOCKET:
+		return false;
+
 	default:
 		WARN_ON_ONCE(1);
 		return false;
@@ -146,6 +153,9 @@ static struct rb_root *get_root(struct landlock_ruleset *const ruleset,
 		return &ruleset->root_net_port;
 #endif /* IS_ENABLED(CONFIG_INET) */
 
+	case LANDLOCK_KEY_SOCKET:
+		return &ruleset->root_socket;
+
 	default:
 		WARN_ON_ONCE(1);
 		return ERR_PTR(-EINVAL);
@@ -175,7 +185,8 @@ static void build_check_ruleset(void)
 	BUILD_BUG_ON(ruleset.num_layers < LANDLOCK_MAX_NUM_LAYERS);
 	BUILD_BUG_ON(access_masks <
 		     ((LANDLOCK_MASK_ACCESS_FS << LANDLOCK_SHIFT_ACCESS_FS) |
-		      (LANDLOCK_MASK_ACCESS_NET << LANDLOCK_SHIFT_ACCESS_NET)));
+		      (LANDLOCK_MASK_ACCESS_NET << LANDLOCK_SHIFT_ACCESS_NET) |
+			  (LANDLOCK_MASK_ACCESS_SOCKET << LANDLOCK_SHIFT_ACCESS_SOCKET)));
 }
 
 /**
@@ -399,6 +410,11 @@ static int merge_ruleset(struct landlock_ruleset *const dst,
 		goto out_unlock;
 #endif /* IS_ENABLED(CONFIG_INET) */
 
+	/* Merges the @src socket tree. */
+	err = merge_tree(dst, src, LANDLOCK_KEY_SOCKET);
+	if (err)
+		goto out_unlock;
+
 out_unlock:
 	mutex_unlock(&src->lock);
 	mutex_unlock(&dst->lock);
@@ -462,6 +478,11 @@ static int inherit_ruleset(struct landlock_ruleset *const parent,
 		goto out_unlock;
 #endif /* IS_ENABLED(CONFIG_INET) */
 
+	/* Copies the @parent socket tree. */
+	err = inherit_tree(parent, child, LANDLOCK_KEY_SOCKET);
+	if (err)
+		goto out_unlock;
+
 	if (WARN_ON_ONCE(child->num_layers <= parent->num_layers)) {
 		err = -EINVAL;
 		goto out_unlock;
@@ -498,6 +519,10 @@ static void free_ruleset(struct landlock_ruleset *const ruleset)
 		free_rule(freeme, LANDLOCK_KEY_NET_PORT);
 #endif /* IS_ENABLED(CONFIG_INET) */
 
+	rbtree_postorder_for_each_entry_safe(freeme, next,
+					     &ruleset->root_socket, node)
+		free_rule(freeme, LANDLOCK_KEY_SOCKET);
+
 	put_hierarchy(ruleset->hierarchy);
 	kfree(ruleset);
 }
@@ -708,6 +733,10 @@ landlock_init_layer_masks(const struct landlock_ruleset *const domain,
 		break;
 #endif /* IS_ENABLED(CONFIG_INET) */
 
+	case LANDLOCK_KEY_SOCKET:
+		get_access_mask = landlock_get_socket_access_mask;
+		num_access = LANDLOCK_NUM_ACCESS_SOCKET;
+		break;
 	default:
 		WARN_ON_ONCE(1);
 		return 0;
diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h
index c7f152678..f4213db09 100644
--- a/security/landlock/ruleset.h
+++ b/security/landlock/ruleset.h
@@ -72,10 +72,10 @@ union landlock_key {
 	 */
 	struct landlock_object *object;
 	/**
-	 * @data: Raw data to identify an arbitrary 32-bit value
+	 * @data: Raw data to identify an arbitrary 64-bit value
 	 * (e.g. a TCP port).
 	 */
-	uintptr_t data;
+	u64 data;
 };
 
 /**
@@ -92,6 +92,12 @@ enum landlock_key_type {
 	 * node keys.
 	 */
 	LANDLOCK_KEY_NET_PORT,
+
+	/**
+	 * @LANDLOCK_KEY_SOCKET: Type of &landlock_ruleset.root_socket's
+	 * node keys.
+	 */
+	LANDLOCK_KEY_SOCKET,
 };
 
 /**
@@ -177,6 +183,15 @@ struct landlock_ruleset {
 	struct rb_root root_net_port;
 #endif /* IS_ENABLED(CONFIG_INET) */
 
+	/**
+	 * @root_socket: Root of a red-black tree containing &struct
+	 * landlock_rule nodes with socket type, described by (domain, type)
+	 * pair (see socket(2)). Once a ruleset is tied to a
+	 * process (i.e. as a domain), this tree is immutable until @usage
+	 * reaches zero.
+	 */
+	struct rb_root root_socket;
+
 	/**
 	 * @hierarchy: Enables hierarchy identification even when a parent
 	 * domain vanishes.  This is needed for the ptrace protection.
@@ -233,7 +248,8 @@ struct landlock_ruleset {
 
 struct landlock_ruleset *
 landlock_create_ruleset(const access_mask_t access_mask_fs,
-			const access_mask_t access_mask_net);
+			const access_mask_t access_mask_net,
+			const access_mask_t socket_access_mask);
 
 void landlock_put_ruleset(struct landlock_ruleset *const ruleset);
 void landlock_put_ruleset_deferred(struct landlock_ruleset *const ruleset);
@@ -282,6 +298,19 @@ landlock_add_net_access_mask(struct landlock_ruleset *const ruleset,
 		(net_mask << LANDLOCK_SHIFT_ACCESS_NET);
 }
 
+static inline void
+landlock_add_socket_access_mask(struct landlock_ruleset *const ruleset,
+			     const access_mask_t socket_access_mask,
+			     const u16 layer_level)
+{
+	access_mask_t socket_mask = socket_access_mask & LANDLOCK_MASK_ACCESS_SOCKET;
+
+	/* Should already be checked in sys_landlock_create_ruleset(). */
+	WARN_ON_ONCE(socket_access_mask != socket_mask);
+	ruleset->access_masks[layer_level] |=
+		(socket_mask << LANDLOCK_SHIFT_ACCESS_SOCKET);
+}
+
 static inline access_mask_t
 landlock_get_raw_fs_access_mask(const struct landlock_ruleset *const ruleset,
 				const u16 layer_level)
@@ -309,6 +338,15 @@ landlock_get_net_access_mask(const struct landlock_ruleset *const ruleset,
 	       LANDLOCK_MASK_ACCESS_NET;
 }
 
+static inline access_mask_t
+landlock_get_socket_access_mask(const struct landlock_ruleset *const ruleset,
+			     const u16 layer_level)
+{
+	return (ruleset->access_masks[layer_level] >>
+		LANDLOCK_SHIFT_ACCESS_SOCKET) &
+	       LANDLOCK_MASK_ACCESS_SOCKET;
+}
+
 bool landlock_unmask_layers(const struct landlock_rule *const rule,
 			    const access_mask_t access_request,
 			    layer_mask_t (*const layer_masks)[],
diff --git a/security/landlock/socket.c b/security/landlock/socket.c
new file mode 100644
index 000000000..88b4ef3a1
--- /dev/null
+++ b/security/landlock/socket.c
@@ -0,0 +1,43 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Landlock LSM - Socket management and hooks
+ *
+ * Copyright © 2024 Huawei Tech. Co., Ltd.
+ */
+
+#include "limits.h"
+#include "ruleset.h"
+#include "socket.h"
+
+union socket_key {
+	struct {
+		int domain;
+		int type;
+	} __packed content;
+	u64 val;
+};
+
+int landlock_append_socket_rule(struct landlock_ruleset *const ruleset,
+			     const int domain, const int type, access_mask_t access_rights)
+{
+	int err;
+	const union socket_key socket_key = {
+		.content.domain = domain,
+		.content.type = type
+	};
+
+	const struct landlock_id id = {
+		.key.data = socket_key.val,
+		.type = LANDLOCK_KEY_SOCKET,
+	};
+
+	/* Transforms relative access rights to absolute ones. */
+	access_rights |= LANDLOCK_MASK_ACCESS_SOCKET &
+			 ~landlock_get_socket_access_mask(ruleset, 0);
+
+	mutex_lock(&ruleset->lock);
+	err = landlock_insert_rule(ruleset, id, access_rights);
+	mutex_unlock(&ruleset->lock);
+
+	return err;
+}
diff --git a/security/landlock/socket.h b/security/landlock/socket.h
new file mode 100644
index 000000000..2b8f9ae7d
--- /dev/null
+++ b/security/landlock/socket.h
@@ -0,0 +1,17 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Landlock LSM - Socket management and hooks
+ *
+ * Copyright © 2024 Huawei Tech. Co., Ltd.
+ */
+
+#ifndef _SECURITY_LANDLOCK_SOCKET_H
+#define _SECURITY_LANDLOCK_SOCKET_H
+
+#include "ruleset.h"
+
+int landlock_append_socket_rule(struct landlock_ruleset *const ruleset,
+				 const int domain, const int type,
+				 access_mask_t access_rights);
+
+#endif /* _SECURITY_LANDLOCK_SOCKET_H */
diff --git a/security/landlock/syscalls.c b/security/landlock/syscalls.c
index 6788e73b6..66c9800c2 100644
--- a/security/landlock/syscalls.c
+++ b/security/landlock/syscalls.c
@@ -30,6 +30,7 @@
 #include "fs.h"
 #include "limits.h"
 #include "net.h"
+#include "socket.h"
 #include "ruleset.h"
 #include "setup.h"
 
@@ -88,7 +89,8 @@ static void build_check_abi(void)
 	struct landlock_ruleset_attr ruleset_attr;
 	struct landlock_path_beneath_attr path_beneath_attr;
 	struct landlock_net_port_attr net_port_attr;
-	size_t ruleset_size, path_beneath_size, net_port_size;
+	struct landlock_socket_attr socket_attr;
+	size_t ruleset_size, path_beneath_size, net_port_size, socket_size;
 
 	/*
 	 * For each user space ABI structures, first checks that there is no
@@ -97,8 +99,9 @@ static void build_check_abi(void)
 	 */
 	ruleset_size = sizeof(ruleset_attr.handled_access_fs);
 	ruleset_size += sizeof(ruleset_attr.handled_access_net);
+	ruleset_size += sizeof(ruleset_attr.handled_access_socket);
 	BUILD_BUG_ON(sizeof(ruleset_attr) != ruleset_size);
-	BUILD_BUG_ON(sizeof(ruleset_attr) != 16);
+	BUILD_BUG_ON(sizeof(ruleset_attr) != 24);
 
 	path_beneath_size = sizeof(path_beneath_attr.allowed_access);
 	path_beneath_size += sizeof(path_beneath_attr.parent_fd);
@@ -109,6 +112,12 @@ static void build_check_abi(void)
 	net_port_size += sizeof(net_port_attr.port);
 	BUILD_BUG_ON(sizeof(net_port_attr) != net_port_size);
 	BUILD_BUG_ON(sizeof(net_port_attr) != 16);
+
+	socket_size = sizeof(socket_attr.allowed_access);
+	socket_size += sizeof(socket_attr.domain);
+	socket_size += sizeof(socket_attr.type);
+	BUILD_BUG_ON(sizeof(socket_attr) != socket_size);
+	BUILD_BUG_ON(sizeof(socket_attr) != 16);
 }
 
 /* Ruleset handling */
@@ -149,7 +158,7 @@ static const struct file_operations ruleset_fops = {
 	.write = fop_dummy_write,
 };
 
-#define LANDLOCK_ABI_VERSION 4
+#define LANDLOCK_ABI_VERSION 5
 
 /**
  * sys_landlock_create_ruleset - Create a new ruleset
@@ -213,9 +222,15 @@ SYSCALL_DEFINE3(landlock_create_ruleset,
 	    LANDLOCK_MASK_ACCESS_NET)
 		return -EINVAL;
 
+	/* Checks socket content (and 32-bits cast). */
+	if ((ruleset_attr.handled_access_socket | LANDLOCK_MASK_ACCESS_SOCKET) !=
+	    LANDLOCK_MASK_ACCESS_SOCKET)
+		return -EINVAL;
+
 	/* Checks arguments and transforms to kernel struct. */
 	ruleset = landlock_create_ruleset(ruleset_attr.handled_access_fs,
-					  ruleset_attr.handled_access_net);
+					  ruleset_attr.handled_access_net,
+					  ruleset_attr.handled_access_socket);
 	if (IS_ERR(ruleset))
 		return PTR_ERR(ruleset);
 
@@ -371,6 +386,35 @@ static int add_rule_net_port(struct landlock_ruleset *ruleset,
 					net_port_attr.allowed_access);
 }
 
+static int add_rule_socket(struct landlock_ruleset *ruleset,
+			     const void __user *const rule_attr)
+{
+	struct landlock_socket_attr socket_attr;
+	int res;
+	access_mask_t mask;
+
+	/* Copies raw user space buffer. */
+	res = copy_from_user(&socket_attr, rule_attr, sizeof(socket_attr));
+	if (res)
+		return -EFAULT;
+
+	/*
+	 * Informs about useless rule: empty allowed_access (i.e. deny rules)
+	 * are ignored by network actions.
+	 */
+	if (!socket_attr.allowed_access)
+		return -ENOMSG;
+
+	/* Checks that allowed_access matches the @ruleset constraints. */
+	mask = landlock_get_socket_access_mask(ruleset, 0);
+	if ((socket_attr.allowed_access | mask) != mask)
+		return -EINVAL;
+
+	/* Imports the new rule. */
+	return landlock_append_socket_rule(ruleset, socket_attr.domain,
+					socket_attr.type, socket_attr.allowed_access);
+}
+
 /**
  * sys_landlock_add_rule - Add a new rule to a ruleset
  *
@@ -429,6 +473,9 @@ SYSCALL_DEFINE4(landlock_add_rule, const int, ruleset_fd,
 	case LANDLOCK_RULE_NET_PORT:
 		err = add_rule_net_port(ruleset, rule_attr);
 		break;
+	case LANDLOCK_RULE_SOCKET:
+		err = add_rule_socket(ruleset, rule_attr);
+		break;
 	default:
 		err = -EINVAL;
 		break;
diff --git a/tools/testing/selftests/landlock/base_test.c b/tools/testing/selftests/landlock/base_test.c
index 646f778df..d292b419c 100644
--- a/tools/testing/selftests/landlock/base_test.c
+++ b/tools/testing/selftests/landlock/base_test.c
@@ -75,7 +75,7 @@ TEST(abi_version)
 	const struct landlock_ruleset_attr ruleset_attr = {
 		.handled_access_fs = LANDLOCK_ACCESS_FS_READ_FILE,
 	};
-	ASSERT_EQ(4, landlock_create_ruleset(NULL, 0,
+	ASSERT_EQ(5, landlock_create_ruleset(NULL, 0,
 					     LANDLOCK_CREATE_RULESET_VERSION));
 
 	ASSERT_EQ(-1, landlock_create_ruleset(&ruleset_attr, 0,
-- 
2.34.1

Re: [RFC PATCH v1 01/10] landlock: Support socket access-control

[Günther Noack]

Hello!

Just zooming in on what I think are the most high level questions here,
so that we get the more dramatic changes out of the way early, if needed.

On Mon, Apr 08, 2024 at 05:39:18PM +0800, Ivanov Mikhail wrote:
> diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
> index 25c8d7677..8551ade38 100644
> --- a/include/uapi/linux/landlock.h
> +++ b/include/uapi/linux/landlock.h
> @@ -37,6 +37,13 @@ struct landlock_ruleset_attr {
>  	 * rule explicitly allow them.
>  	 */
>  	__u64 handled_access_net;
> +
> +	/**
> +	 * @handled_access_net: Bitmask of actions (cf. `Socket flags`_)
                           ^^^
			   Typo

> +	 * that is handled by this ruleset and should then be forbidden if no
> +	 * rule explicitly allow them.
> +	 */
> +	__u64 handled_access_socket;

What is your rationale for introducing and naming this additional field?

I am not convinced that "socket" is the right name to use in this field,
but it is well possible that I'm missing some context.

* If we introduce this additional field in the landlock_ruleset_attr, which
  other socket-related operations will go in the remaining 63 bits?  (I'm having
  a hard time coming up with so many of them.)

* Should this have a more general name than "socket", so that other planned
  features from the bug tracker [1] fit in?

The other alternative is of course to piggy back on the existing
handled_access_net field, whose name already is pretty generic.

For that, I believe we would need to clarify in struct landlock_net_port_attr
which exact values are permitted there.

I imagine you have considered this approach?  Are there more reasons why this
was ruled out, which I am overlooking?

[1] https://github.com/orgs/landlock-lsm/projects/1/views/1


> @@ -244,4 +277,20 @@ struct landlock_net_port_attr {
>  #define LANDLOCK_ACCESS_NET_BIND_TCP			(1ULL << 0)
>  #define LANDLOCK_ACCESS_NET_CONNECT_TCP			(1ULL << 1)
>  /* clang-format on */
> +
> +/**
> + * DOC: socket_acess
> + *
> + * Socket flags
> + * ~~~~~~~~~~~~~~~~

Mega-Nit: This ~~~ underline should only be as long as the text above it ;-)
You might want to fix it for the "Network Flags" headline as well.

> + *
> + * These flags enable to restrict a sandboxed process to a set of
> + * socket-related actions for specific protocols. This is supported
> + * since the Landlock ABI version 5.
> + *
> + * - %LANDLOCK_ACCESS_SOCKET_CREATE: Create a socket
> + */


> diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h
> index c7f152678..f4213db09 100644
> --- a/security/landlock/ruleset.h
> +++ b/security/landlock/ruleset.h
> @@ -92,6 +92,12 @@ enum landlock_key_type {
>  	 * node keys.
>  	 */
>  	LANDLOCK_KEY_NET_PORT,
> +
> +	/**
> +	 * @LANDLOCK_KEY_SOCKET: Type of &landlock_ruleset.root_socket's
> +	 * node keys.
> +	 */
> +	LANDLOCK_KEY_SOCKET,
>  };
>  
>  /**
> @@ -177,6 +183,15 @@ struct landlock_ruleset {
>  	struct rb_root root_net_port;
>  #endif /* IS_ENABLED(CONFIG_INET) */
>  
> +	/**
> +	 * @root_socket: Root of a red-black tree containing &struct
> +	 * landlock_rule nodes with socket type, described by (domain, type)
> +	 * pair (see socket(2)). Once a ruleset is tied to a
> +	 * process (i.e. as a domain), this tree is immutable until @usage
> +	 * reaches zero.
> +	 */
> +	struct rb_root root_socket;

The domain is a value between 0 and 45,
and the socket type is one of 1, 2, 3, 4, 5, 6, 10.

The bounds of these are defined with AF_MAX (include/linux/socket.h) and
SOCK_MAX (include/linux/net.h).

Why don't we just combine these two numbers into an index and create a big bit
vector here, like this:

    socket_type_mask_t socket_domains[AF_MAX];

socket_type_mask_t would need to be typedef'd to u16 and ideally have a static
check to test that it has more bits than SOCK_MAX.

Then you can look up whether a socket creation is permitted by checking:

    /* assuming appropriate bounds checks */
    if (dom->socket_domains[domain] & (1 << type)) { /* permitted */ }

and merging the socket_domains of two domains would be a bitwise-AND.

(We can also cram socket_type_mask_t in a u8 but it would require mapping the
existing socket types onto a different number space.)


As I said before, I am very excited to see this patch.

I think this will unlock a tremendous amount of use cases for many programs,
especially for programs that do not use networking at all, which can now lock
themselves down to guarantee that with a sandbox.

Thank you very much for looking into it!
—Günther

Re: [RFC PATCH v1 01/10] landlock: Support socket access-control

[Ivanov Mikhail]

Hello! Big thanks for your review and ideas :)

P.S.: Sorry, previous mail was rejected by linux mailboxes
due to HTML formatting.

4/8/2024 10:49 PM, Günther Noack wrote:
> Hello!
> 
> Just zooming in on what I think are the most high level questions here,
> so that we get the more dramatic changes out of the way early, if needed.
> 
> On Mon, Apr 08, 2024 at 05:39:18PM +0800, Ivanov Mikhail wrote:
>> diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
>> index 25c8d7677..8551ade38 100644
>> --- a/include/uapi/linux/landlock.h
>> +++ b/include/uapi/linux/landlock.h
>> @@ -37,6 +37,13 @@ struct landlock_ruleset_attr {
>>   	 * rule explicitly allow them.
>>   	 */
>>   	__u64 handled_access_net;
>> +
>> +	/**
>> +	 * @handled_access_net: Bitmask of actions (cf. `Socket flags`_)
>                             ^^^
> 			   Typo
>

Thanks, will be fixed.

>> +	 * that is handled by this ruleset and should then be forbidden if no
>> +	 * rule explicitly allow them.
>> +	 */
>> +	__u64 handled_access_socket;
> 
> What is your rationale for introducing and naming this additional field?
> 
> I am not convinced that "socket" is the right name to use in this field,
> but it is well possible that I'm missing some context.
> 
> * If we introduce this additional field in the landlock_ruleset_attr, which
>    other socket-related operations will go in the remaining 63 bits?  (I'm having
>    a hard time coming up with so many of them.)

If i understood correctly Mickaël suggested saving some space for
actions related not only to creating sockets, but also to sending
and receiving socket FDs from another processes, marking pre-sandboxed
sockets as allowed or denied after sandboxing [2]. This may be necessary
in order to achieve complete isolation of the sandbox, which will be
able to create, receive and send sockets of specific protocols.

In future this field may become more generic by including rules for
other entities with similar actions (e.g. files, pipes).

I think it is good approach, but we should discuss this design before
generalizing the name. For now `handled_access_socket` can be a good
name for actions related to accessing specific sockets (protocols).
What do you think?

[2] 
https://lore.kernel.org/all/b8a2045a-e7e8-d141-7c01-bf47874c7930@digikod.net/

> 
> * Should this have a more general name than "socket", so that other planned
>    features from the bug tracker [1] fit in?

I have not found any similar features for our case. Do you have any in
mind?

> 
> The other alternative is of course to piggy back on the existing
> handled_access_net field, whose name already is pretty generic.
> 
> For that, I believe we would need to clarify in struct landlock_net_port_attr
> which exact values are permitted there.
> 
> I imagine you have considered this approach?  Are there more reasons why this
> was ruled out, which I am overlooking?
> 
> [1] https://github.com/orgs/landlock-lsm/projects/1/views/1
> 
>

Currently `handled_access_net` stands for restricting actions for
specific network protocols by port values: LANDLOCK_ACCESS_NET_BIND_TCP,
LANDLOCK_ACCESS_NET_SEND_UDP (possibly will be added with UDP feature
[3]).

I dont think that complicating semantics with adding fields for
socket_create()-like actions would fit well here. Purpose of current
patch is to restrict usage of unwanted protocols, not to add logic
to restrict their actions. In addition, it is worth considering that we
want to restrict not only network protocols (e.g. Bluetooth).

[3] https://github.com/landlock-lsm/linux/issues/1

>> @@ -244,4 +277,20 @@ struct landlock_net_port_attr {
>>   #define LANDLOCK_ACCESS_NET_BIND_TCP			(1ULL << 0)
>>   #define LANDLOCK_ACCESS_NET_CONNECT_TCP			(1ULL << 1)
>>   /* clang-format on */
>> +
>> +/**
>> + * DOC: socket_acess
>> + *
>> + * Socket flags
>> + * ~~~~~~~~~~~~~~~~
> 
> Mega-Nit: This ~~~ underline should only be as long as the text above it ;-)
> You might want to fix it for the "Network Flags" headline as well.
> 

Ofc, thanks!

>> + *
>> + * These flags enable to restrict a sandboxed process to a set of
>> + * socket-related actions for specific protocols. This is supported
>> + * since the Landlock ABI version 5.
>> + *
>> + * - %LANDLOCK_ACCESS_SOCKET_CREATE: Create a socket
>> + */
> 
> 
>> diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h
>> index c7f152678..f4213db09 100644
>> --- a/security/landlock/ruleset.h
>> +++ b/security/landlock/ruleset.h
>> @@ -92,6 +92,12 @@ enum landlock_key_type {
>>   	 * node keys.
>>   	 */
>>   	LANDLOCK_KEY_NET_PORT,
>> +
>> +	/**
>> +	 * @LANDLOCK_KEY_SOCKET: Type of &landlock_ruleset.root_socket's
>> +	 * node keys.
>> +	 */
>> +	LANDLOCK_KEY_SOCKET,
>>   };
>>   
>>   /**
>> @@ -177,6 +183,15 @@ struct landlock_ruleset {
>>   	struct rb_root root_net_port;
>>   #endif /* IS_ENABLED(CONFIG_INET) */
>>   
>> +	/**
>> +	 * @root_socket: Root of a red-black tree containing &struct
>> +	 * landlock_rule nodes with socket type, described by (domain, type)
>> +	 * pair (see socket(2)). Once a ruleset is tied to a
>> +	 * process (i.e. as a domain), this tree is immutable until @usage
>> +	 * reaches zero.
>> +	 */
>> +	struct rb_root root_socket;
> 
> The domain is a value between 0 and 45,
> and the socket type is one of 1, 2, 3, 4, 5, 6, 10.
> 
> The bounds of these are defined with AF_MAX (include/linux/socket.h) and
> SOCK_MAX (include/linux/net.h).
> 
> Why don't we just combine these two numbers into an index and create a big bit
> vector here, like this:
> 
>      socket_type_mask_t socket_domains[AF_MAX];
> 
> socket_type_mask_t would need to be typedef'd to u16 and ideally have a static
> check to test that it has more bits than SOCK_MAX.
> 
> Then you can look up whether a socket creation is permitted by checking:
> 
>      /* assuming appropriate bounds checks */
>      if (dom->socket_domains[domain] & (1 << type)) { /* permitted */ }
> 
> and merging the socket_domains of two domains would be a bitwise-AND.
> 
> (We can also cram socket_type_mask_t in a u8 but it would require mapping the
> existing socket types onto a different number space.)
> 

I chose rbtree based on the current storage implementation in fs,net and
decided to leave the implementation of better variants in a separate
patch, which should redesign the entire storage system in Landlock
(e.g. implementation of a hashtable for storing rules by FDs,
port values) [4].

Do you think that it is bad idea and more appropriate storage for socket
rules(e.g. what you suggested) should be implemented by current patch?

[4] https://github.com/landlock-lsm/linux/issues/1

> 
> As I said before, I am very excited to see this patch.
> 
> I think this will unlock a tremendous amount of use cases for many programs,
> especially for programs that do not use networking at all, which can now lock
> themselves down to guarantee that with a sandbox.
> 
> Thank you very much for looking into it!
> —Günther

Re: [RFC PATCH v1 01/10] landlock: Support socket access-control

[Günther Noack]

Hello!

On Thu, Apr 11, 2024 at 06:16:31PM +0300, Ivanov Mikhail wrote:
> 4/8/2024 10:49 PM, Günther Noack wrote:
> > On Mon, Apr 08, 2024 at 05:39:18PM +0800, Ivanov Mikhail wrote:
> > > diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h
> > > index c7f152678..f4213db09 100644
> > > --- a/security/landlock/ruleset.h
> > > +++ b/security/landlock/ruleset.h
> > > @@ -92,6 +92,12 @@ enum landlock_key_type {
> > >   	 * node keys.
> > >   	 */
> > >   	LANDLOCK_KEY_NET_PORT,
> > > +
> > > +	/**
> > > +	 * @LANDLOCK_KEY_SOCKET: Type of &landlock_ruleset.root_socket's
> > > +	 * node keys.
> > > +	 */
> > > +	LANDLOCK_KEY_SOCKET,
> > >   };
> > >   /**
> > > @@ -177,6 +183,15 @@ struct landlock_ruleset {
> > >   	struct rb_root root_net_port;
> > >   #endif /* IS_ENABLED(CONFIG_INET) */
> > > +	/**
> > > +	 * @root_socket: Root of a red-black tree containing &struct
> > > +	 * landlock_rule nodes with socket type, described by (domain, type)
> > > +	 * pair (see socket(2)). Once a ruleset is tied to a
> > > +	 * process (i.e. as a domain), this tree is immutable until @usage
> > > +	 * reaches zero.
> > > +	 */
> > > +	struct rb_root root_socket;
> > 
> > The domain is a value between 0 and 45,
> > and the socket type is one of 1, 2, 3, 4, 5, 6, 10.
> > 
> > The bounds of these are defined with AF_MAX (include/linux/socket.h) and
> > SOCK_MAX (include/linux/net.h).
> > 
> > Why don't we just combine these two numbers into an index and create a big bit
> > vector here, like this:
> > 
> >      socket_type_mask_t socket_domains[AF_MAX];
> > 
> > socket_type_mask_t would need to be typedef'd to u16 and ideally have a static
> > check to test that it has more bits than SOCK_MAX.
> > 
> > Then you can look up whether a socket creation is permitted by checking:
> > 
> >      /* assuming appropriate bounds checks */
> >      if (dom->socket_domains[domain] & (1 << type)) { /* permitted */ }
> > 
> > and merging the socket_domains of two domains would be a bitwise-AND.
> > 
> > (We can also cram socket_type_mask_t in a u8 but it would require mapping the
> > existing socket types onto a different number space.)
> > 
> 
> I chose rbtree based on the current storage implementation in fs,net and
> decided to leave the implementation of better variants in a separate
> patch, which should redesign the entire storage system in Landlock
> (e.g. implementation of a hashtable for storing rules by FDs,
> port values) [4].
> 
> Do you think that it is bad idea and more appropriate storage for socket
> rules(e.g. what you suggested) should be implemented by current patch?
> 
> [4] https://github.com/landlock-lsm/linux/issues/1

I realized that my suggestion might be at odds with Mickaël's Landlock audit
patch set [1].  IIRC, the goal there is to log the reasons for a denial,
together with the Landlock ruleset on which this decision was based.

[1] https://lore.kernel.org/all/20230921061641.273654-1-mic@digikod.net/

I'd recommend to wait for Mickaël to chime in on this one before spending the
time to reimplement that.


—Günther

Re: [RFC PATCH v1 01/10] landlock: Support socket access-control

[Mickaël Salaün]

Thanks Ivanov, this looks really good!  Let me some time to review the
rest.

You can add this tag to the commit message (as reference and
documentation):
Closes: https://github.com/landlock-lsm/linux/issues/6

On Thu, Apr 11, 2024 at 06:16:31PM +0300, Ivanov Mikhail wrote:
> Hello! Big thanks for your review and ideas :)
> 
> P.S.: Sorry, previous mail was rejected by linux mailboxes
> due to HTML formatting.
> 
> 4/8/2024 10:49 PM, Günther Noack wrote:
> > Hello!
> > 
> > Just zooming in on what I think are the most high level questions here,
> > so that we get the more dramatic changes out of the way early, if needed.
> > 
> > On Mon, Apr 08, 2024 at 05:39:18PM +0800, Ivanov Mikhail wrote:
> > > diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
> > > index 25c8d7677..8551ade38 100644
> > > --- a/include/uapi/linux/landlock.h
> > > +++ b/include/uapi/linux/landlock.h
> > > @@ -37,6 +37,13 @@ struct landlock_ruleset_attr {
> > >   	 * rule explicitly allow them.
> > >   	 */
> > >   	__u64 handled_access_net;
> > > +
> > > +	/**
> > > +	 * @handled_access_net: Bitmask of actions (cf. `Socket flags`_)
> >                             ^^^
> > 			   Typo
> > 
> 
> Thanks, will be fixed.
> 
> > > +	 * that is handled by this ruleset and should then be forbidden if no
> > > +	 * rule explicitly allow them.
> > > +	 */
> > > +	__u64 handled_access_socket;
> > 
> > What is your rationale for introducing and naming this additional field?
> > 
> > I am not convinced that "socket" is the right name to use in this field,
> > but it is well possible that I'm missing some context.
> > 
> > * If we introduce this additional field in the landlock_ruleset_attr, which
> >    other socket-related operations will go in the remaining 63 bits?  (I'm having
> >    a hard time coming up with so many of them.)
> 
> If i understood correctly Mickaël suggested saving some space for
> actions related not only to creating sockets, but also to sending
> and receiving socket FDs from another processes, marking pre-sandboxed
> sockets as allowed or denied after sandboxing [2]. This may be necessary
> in order to achieve complete isolation of the sandbox, which will be
> able to create, receive and send sockets of specific protocols.
> 
> In future this field may become more generic by including rules for
> other entities with similar actions (e.g. files, pipes).

I think it would make sense to have one field per file kind (not
necessarily type) because not all actions would make sense.

> 
> I think it is good approach, but we should discuss this design before
> generalizing the name. For now `handled_access_socket` can be a good
> name for actions related to accessing specific sockets (protocols).
> What do you think?

I'm OK with this name for now unless someone has a better proposition.

> 
> [2]
> https://lore.kernel.org/all/b8a2045a-e7e8-d141-7c01-bf47874c7930@digikod.net/
> 
> > 
> > * Should this have a more general name than "socket", so that other planned
> >    features from the bug tracker [1] fit in?
> 
> I have not found any similar features for our case. Do you have any in
> mind?
> 
> > 
> > The other alternative is of course to piggy back on the existing
> > handled_access_net field, whose name already is pretty generic.

handled_access_net is indeed quite generic, but the question is: would
this new access right make sense for the net_port rule?  In the case of
socket creation, this is not the case because we don't know at this time
which port will be used.

> > 
> > For that, I believe we would need to clarify in struct landlock_net_port_attr
> > which exact values are permitted there.

Potentially anything that would be possible to check against a port.

> > 
> > I imagine you have considered this approach?  Are there more reasons why this
> > was ruled out, which I am overlooking?
> > 
> > [1] https://github.com/orgs/landlock-lsm/projects/1/views/1
> > 
> > 
> 
> Currently `handled_access_net` stands for restricting actions for
> specific network protocols by port values: LANDLOCK_ACCESS_NET_BIND_TCP,
> LANDLOCK_ACCESS_NET_SEND_UDP (possibly will be added with UDP feature
> [3]).
> 
> I dont think that complicating semantics with adding fields for
> socket_create()-like actions would fit well here. Purpose of current
> patch is to restrict usage of unwanted protocols, not to add logic
> to restrict their actions. In addition, it is worth considering that we
> want to restrict not only network protocols (e.g. Bluetooth).

Correct.  It's worth it mentionning this rationale in the patch
description.

> 
> [3] https://github.com/landlock-lsm/linux/issues/1
> 
> > > @@ -244,4 +277,20 @@ struct landlock_net_port_attr {
> > >   #define LANDLOCK_ACCESS_NET_BIND_TCP			(1ULL << 0)
> > >   #define LANDLOCK_ACCESS_NET_CONNECT_TCP			(1ULL << 1)
> > >   /* clang-format on */
> > > +
> > > +/**
> > > + * DOC: socket_acess
> > > + *
> > > + * Socket flags
> > > + * ~~~~~~~~~~~~~~~~
> > 
> > Mega-Nit: This ~~~ underline should only be as long as the text above it ;-)
> > You might want to fix it for the "Network Flags" headline as well.
> > 
> 
> Ofc, thanks!
> 
> > > + *
> > > + * These flags enable to restrict a sandboxed process to a set of
> > > + * socket-related actions for specific protocols. This is supported
> > > + * since the Landlock ABI version 5.
> > > + *
> > > + * - %LANDLOCK_ACCESS_SOCKET_CREATE: Create a socket
> > > + */
> > 
> > 
> > > diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h
> > > index c7f152678..f4213db09 100644
> > > --- a/security/landlock/ruleset.h
> > > +++ b/security/landlock/ruleset.h
> > > @@ -92,6 +92,12 @@ enum landlock_key_type {
> > >   	 * node keys.
> > >   	 */
> > >   	LANDLOCK_KEY_NET_PORT,
> > > +
> > > +	/**
> > > +	 * @LANDLOCK_KEY_SOCKET: Type of &landlock_ruleset.root_socket's
> > > +	 * node keys.
> > > +	 */
> > > +	LANDLOCK_KEY_SOCKET,
> > >   };
> > >   /**
> > > @@ -177,6 +183,15 @@ struct landlock_ruleset {
> > >   	struct rb_root root_net_port;
> > >   #endif /* IS_ENABLED(CONFIG_INET) */
> > > +	/**
> > > +	 * @root_socket: Root of a red-black tree containing &struct
> > > +	 * landlock_rule nodes with socket type, described by (domain, type)
> > > +	 * pair (see socket(2)). Once a ruleset is tied to a
> > > +	 * process (i.e. as a domain), this tree is immutable until @usage
> > > +	 * reaches zero.
> > > +	 */
> > > +	struct rb_root root_socket;
> > 
> > The domain is a value between 0 and 45,
> > and the socket type is one of 1, 2, 3, 4, 5, 6, 10.
> > 
> > The bounds of these are defined with AF_MAX (include/linux/socket.h) and
> > SOCK_MAX (include/linux/net.h).
> > 
> > Why don't we just combine these two numbers into an index and create a big bit
> > vector here, like this:
> > 
> >      socket_type_mask_t socket_domains[AF_MAX];
> > 
> > socket_type_mask_t would need to be typedef'd to u16 and ideally have a static
> > check to test that it has more bits than SOCK_MAX.
> > 
> > Then you can look up whether a socket creation is permitted by checking:
> > 
> >      /* assuming appropriate bounds checks */
> >      if (dom->socket_domains[domain] & (1 << type)) { /* permitted */ }
> > 
> > and merging the socket_domains of two domains would be a bitwise-AND.
> > 
> > (We can also cram socket_type_mask_t in a u8 but it would require mapping the
> > existing socket types onto a different number space.)
> > 
> 
> I chose rbtree based on the current storage implementation in fs,net and
> decided to leave the implementation of better variants in a separate
> patch, which should redesign the entire storage system in Landlock
> (e.g. implementation of a hashtable for storing rules by FDs,
> port values) [4].
> 
> Do you think that it is bad idea and more appropriate storage for socket
> rules(e.g. what you suggested) should be implemented by current patch?

Günther's suggestion would be a good optimization, but I agree that it
should be part of another series.  We also need to keep in mind that the
layer level should be known for audit and debugging reasons.

> 
> [4] https://github.com/landlock-lsm/linux/issues/1
> 
> > 
> > As I said before, I am very excited to see this patch.
> > 
> > I think this will unlock a tremendous amount of use cases for many programs,
> > especially for programs that do not use networking at all, which can now lock
> > themselves down to guarantee that with a sandbox.
> > 
> > Thank you very much for looking into it!

Same :)

Re: [RFC PATCH v1 01/10] landlock: Support socket access-control

[Mickaël Salaün]

On Mon, Apr 08, 2024 at 05:39:18PM +0800, Ivanov Mikhail wrote:
> Add new socket-related rule type, presented via landlock_socket_attr
> struct. Add all neccessary entities for socket ruleset support.
> Add flag LANDLOCK_ACCESS_SOCKET_CREATE that will provide the
> ability to control socket creation.
> 
> Change landlock_key.data type from uinptr_t to u64. Socket rule has to
> contain 32-bit socket family and type values, so landlock_key can be
> represented as 64-bit number the first 32 bits of which correspond to
> the socket family and last - to the type.
> 
> Change ABI version to 5.
> 
> Signed-off-by: Ivanov Mikhail <ivanov.mikhail1@huawei-partners.com>
> Reviewed-by: Konstantin Meskhidze <konstantin.meskhidze@huawei.com>
> ---
>  include/uapi/linux/landlock.h                | 49 +++++++++++++++++
>  security/landlock/Makefile                   |  2 +-
>  security/landlock/limits.h                   |  5 ++
>  security/landlock/net.c                      |  2 +-
>  security/landlock/ruleset.c                  | 35 +++++++++++--
>  security/landlock/ruleset.h                  | 44 ++++++++++++++--
>  security/landlock/socket.c                   | 43 +++++++++++++++
>  security/landlock/socket.h                   | 17 ++++++
>  security/landlock/syscalls.c                 | 55 ++++++++++++++++++--
>  tools/testing/selftests/landlock/base_test.c |  2 +-
>  10 files changed, 241 insertions(+), 13 deletions(-)
>  create mode 100644 security/landlock/socket.c
>  create mode 100644 security/landlock/socket.h
> 
> diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
> index 25c8d7677..8551ade38 100644
> --- a/include/uapi/linux/landlock.h
> +++ b/include/uapi/linux/landlock.h
> @@ -37,6 +37,13 @@ struct landlock_ruleset_attr {
>  	 * rule explicitly allow them.
>  	 */
>  	__u64 handled_access_net;
> +
> +	/**
> +	 * @handled_access_net: Bitmask of actions (cf. `Socket flags`_)
> +	 * that is handled by this ruleset and should then be forbidden if no
> +	 * rule explicitly allow them.
> +	 */
> +	__u64 handled_access_socket;
>  };
>  
>  /*
> @@ -65,6 +72,11 @@ enum landlock_rule_type {
>  	 * landlock_net_port_attr .
>  	 */
>  	LANDLOCK_RULE_NET_PORT,
> +	/**
> +	 * @LANDLOCK_RULE_SOCKET: Type of a &struct
> +	 * landlock_socket_attr .
> +	 */
> +	LANDLOCK_RULE_SOCKET,
>  };
>  
>  /**
> @@ -115,6 +127,27 @@ struct landlock_net_port_attr {
>  	__u64 port;
>  };
>  
> +/**
> + * struct landlock_socket_attr - Socket definition
> + *
> + * Argument of sys_landlock_add_rule().
> + */
> +struct landlock_socket_attr {
> +	/**
> +	 * @allowed_access: Bitmask of allowed access for a socket
> +	 * (cf. `Socket flags`_).
> +	 */
> +	__u64 allowed_access;
> +	/**
> +	 * @domain: Protocol family used for communication (see socket(2)).
> +	 */
> +	int domain;
> +	/**
> +	 * @type: Socket type (see socket(2)).
> +	 */
> +	int type;
> +};
> +
>  /**
>   * DOC: fs_access
>   *
> @@ -244,4 +277,20 @@ struct landlock_net_port_attr {
>  #define LANDLOCK_ACCESS_NET_BIND_TCP			(1ULL << 0)
>  #define LANDLOCK_ACCESS_NET_CONNECT_TCP			(1ULL << 1)
>  /* clang-format on */
> +
> +/**
> + * DOC: socket_acess
> + *
> + * Socket flags
> + * ~~~~~~~~~~~~~~~~
> + *
> + * These flags enable to restrict a sandboxed process to a set of
> + * socket-related actions for specific protocols. This is supported
> + * since the Landlock ABI version 5.
> + *
> + * - %LANDLOCK_ACCESS_SOCKET_CREATE: Create a socket
> + */
> +/* clang-format off */
> +#define LANDLOCK_ACCESS_SOCKET_CREATE			(1ULL << 0)
> +/* clang-format on */
>  #endif /* _UAPI_LINUX_LANDLOCK_H */
> diff --git a/security/landlock/Makefile b/security/landlock/Makefile
> index b4538b7cf..ff1dd98f6 100644
> --- a/security/landlock/Makefile
> +++ b/security/landlock/Makefile
> @@ -1,6 +1,6 @@
>  obj-$(CONFIG_SECURITY_LANDLOCK) := landlock.o
>  
>  landlock-y := setup.o syscalls.o object.o ruleset.o \
> -	cred.o task.o fs.o
> +	cred.o task.o fs.o socket.o
>  
>  landlock-$(CONFIG_INET) += net.o
> diff --git a/security/landlock/limits.h b/security/landlock/limits.h
> index 93c9c6f91..ebdab587c 100644
> --- a/security/landlock/limits.h
> +++ b/security/landlock/limits.h
> @@ -28,6 +28,11 @@
>  #define LANDLOCK_NUM_ACCESS_NET		__const_hweight64(LANDLOCK_MASK_ACCESS_NET)
>  #define LANDLOCK_SHIFT_ACCESS_NET	LANDLOCK_NUM_ACCESS_FS
>  
> +#define LANDLOCK_LAST_ACCESS_SOCKET	    LANDLOCK_ACCESS_SOCKET_CREATE
> +#define LANDLOCK_MASK_ACCESS_SOCKET	    ((LANDLOCK_LAST_ACCESS_SOCKET << 1) - 1)
> +#define LANDLOCK_NUM_ACCESS_SOCKET		__const_hweight64(LANDLOCK_MASK_ACCESS_SOCKET)
> +#define LANDLOCK_SHIFT_ACCESS_SOCKET	LANDLOCK_NUM_ACCESS_SOCKET
> +
>  /* clang-format on */
>  
>  #endif /* _SECURITY_LANDLOCK_LIMITS_H */
> diff --git a/security/landlock/net.c b/security/landlock/net.c
> index c8bcd29bd..0e3770d14 100644
> --- a/security/landlock/net.c
> +++ b/security/landlock/net.c
> @@ -159,7 +159,7 @@ static int current_check_access_socket(struct socket *const sock,
>  			return -EINVAL;
>  	}
>  
> -	id.key.data = (__force uintptr_t)port;
> +	id.key.data = (__force u64)port;
>  	BUILD_BUG_ON(sizeof(port) > sizeof(id.key.data));
>  
>  	rule = landlock_find_rule(dom, id);
> diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c
> index e0a5fbf92..1f1ed8181 100644
> --- a/security/landlock/ruleset.c
> +++ b/security/landlock/ruleset.c
> @@ -40,6 +40,7 @@ static struct landlock_ruleset *create_ruleset(const u32 num_layers)
>  #if IS_ENABLED(CONFIG_INET)
>  	new_ruleset->root_net_port = RB_ROOT;
>  #endif /* IS_ENABLED(CONFIG_INET) */
> +	new_ruleset->root_socket = RB_ROOT;
>  
>  	new_ruleset->num_layers = num_layers;
>  	/*
> @@ -52,12 +53,13 @@ static struct landlock_ruleset *create_ruleset(const u32 num_layers)
>  
>  struct landlock_ruleset *
>  landlock_create_ruleset(const access_mask_t fs_access_mask,
> -			const access_mask_t net_access_mask)
> +			const access_mask_t net_access_mask,
> +			const access_mask_t socket_access_mask)
>  {
>  	struct landlock_ruleset *new_ruleset;
>  
>  	/* Informs about useless ruleset. */
> -	if (!fs_access_mask && !net_access_mask)
> +	if (!fs_access_mask && !net_access_mask && !socket_access_mask)
>  		return ERR_PTR(-ENOMSG);
>  	new_ruleset = create_ruleset(1);
>  	if (IS_ERR(new_ruleset))
> @@ -66,6 +68,8 @@ landlock_create_ruleset(const access_mask_t fs_access_mask,
>  		landlock_add_fs_access_mask(new_ruleset, fs_access_mask, 0);
>  	if (net_access_mask)
>  		landlock_add_net_access_mask(new_ruleset, net_access_mask, 0);
> +	if (socket_access_mask)
> +		landlock_add_socket_access_mask(new_ruleset, socket_access_mask, 0);
>  	return new_ruleset;
>  }
>  
> @@ -89,6 +93,9 @@ static bool is_object_pointer(const enum landlock_key_type key_type)
>  		return false;
>  #endif /* IS_ENABLED(CONFIG_INET) */
>  
> +	case LANDLOCK_KEY_SOCKET:
> +		return false;
> +
>  	default:
>  		WARN_ON_ONCE(1);
>  		return false;
> @@ -146,6 +153,9 @@ static struct rb_root *get_root(struct landlock_ruleset *const ruleset,
>  		return &ruleset->root_net_port;
>  #endif /* IS_ENABLED(CONFIG_INET) */
>  
> +	case LANDLOCK_KEY_SOCKET:
> +		return &ruleset->root_socket;
> +
>  	default:
>  		WARN_ON_ONCE(1);
>  		return ERR_PTR(-EINVAL);
> @@ -175,7 +185,8 @@ static void build_check_ruleset(void)
>  	BUILD_BUG_ON(ruleset.num_layers < LANDLOCK_MAX_NUM_LAYERS);
>  	BUILD_BUG_ON(access_masks <
>  		     ((LANDLOCK_MASK_ACCESS_FS << LANDLOCK_SHIFT_ACCESS_FS) |
> -		      (LANDLOCK_MASK_ACCESS_NET << LANDLOCK_SHIFT_ACCESS_NET)));
> +		      (LANDLOCK_MASK_ACCESS_NET << LANDLOCK_SHIFT_ACCESS_NET) |
> +			  (LANDLOCK_MASK_ACCESS_SOCKET << LANDLOCK_SHIFT_ACCESS_SOCKET)));
>  }
>  
>  /**
> @@ -399,6 +410,11 @@ static int merge_ruleset(struct landlock_ruleset *const dst,
>  		goto out_unlock;
>  #endif /* IS_ENABLED(CONFIG_INET) */
>  
> +	/* Merges the @src socket tree. */
> +	err = merge_tree(dst, src, LANDLOCK_KEY_SOCKET);
> +	if (err)
> +		goto out_unlock;
> +
>  out_unlock:
>  	mutex_unlock(&src->lock);
>  	mutex_unlock(&dst->lock);
> @@ -462,6 +478,11 @@ static int inherit_ruleset(struct landlock_ruleset *const parent,
>  		goto out_unlock;
>  #endif /* IS_ENABLED(CONFIG_INET) */
>  
> +	/* Copies the @parent socket tree. */
> +	err = inherit_tree(parent, child, LANDLOCK_KEY_SOCKET);
> +	if (err)
> +		goto out_unlock;
> +
>  	if (WARN_ON_ONCE(child->num_layers <= parent->num_layers)) {
>  		err = -EINVAL;
>  		goto out_unlock;
> @@ -498,6 +519,10 @@ static void free_ruleset(struct landlock_ruleset *const ruleset)
>  		free_rule(freeme, LANDLOCK_KEY_NET_PORT);
>  #endif /* IS_ENABLED(CONFIG_INET) */
>  
> +	rbtree_postorder_for_each_entry_safe(freeme, next,
> +					     &ruleset->root_socket, node)
> +		free_rule(freeme, LANDLOCK_KEY_SOCKET);
> +
>  	put_hierarchy(ruleset->hierarchy);
>  	kfree(ruleset);
>  }
> @@ -708,6 +733,10 @@ landlock_init_layer_masks(const struct landlock_ruleset *const domain,
>  		break;
>  #endif /* IS_ENABLED(CONFIG_INET) */
>  
> +	case LANDLOCK_KEY_SOCKET:
> +		get_access_mask = landlock_get_socket_access_mask;
> +		num_access = LANDLOCK_NUM_ACCESS_SOCKET;
> +		break;
>  	default:
>  		WARN_ON_ONCE(1);
>  		return 0;
> diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h
> index c7f152678..f4213db09 100644
> --- a/security/landlock/ruleset.h
> +++ b/security/landlock/ruleset.h
> @@ -72,10 +72,10 @@ union landlock_key {
>  	 */
>  	struct landlock_object *object;
>  	/**
> -	 * @data: Raw data to identify an arbitrary 32-bit value
> +	 * @data: Raw data to identify an arbitrary 64-bit value
>  	 * (e.g. a TCP port).
>  	 */
> -	uintptr_t data;
> +	u64 data;
>  };
>  
>  /**
> @@ -92,6 +92,12 @@ enum landlock_key_type {
>  	 * node keys.
>  	 */
>  	LANDLOCK_KEY_NET_PORT,
> +
> +	/**
> +	 * @LANDLOCK_KEY_SOCKET: Type of &landlock_ruleset.root_socket's
> +	 * node keys.
> +	 */
> +	LANDLOCK_KEY_SOCKET,
>  };
>  
>  /**
> @@ -177,6 +183,15 @@ struct landlock_ruleset {
>  	struct rb_root root_net_port;
>  #endif /* IS_ENABLED(CONFIG_INET) */
>  
> +	/**
> +	 * @root_socket: Root of a red-black tree containing &struct
> +	 * landlock_rule nodes with socket type, described by (domain, type)
> +	 * pair (see socket(2)). Once a ruleset is tied to a
> +	 * process (i.e. as a domain), this tree is immutable until @usage
> +	 * reaches zero.
> +	 */
> +	struct rb_root root_socket;
> +
>  	/**
>  	 * @hierarchy: Enables hierarchy identification even when a parent
>  	 * domain vanishes.  This is needed for the ptrace protection.
> @@ -233,7 +248,8 @@ struct landlock_ruleset {
>  
>  struct landlock_ruleset *
>  landlock_create_ruleset(const access_mask_t access_mask_fs,
> -			const access_mask_t access_mask_net);
> +			const access_mask_t access_mask_net,
> +			const access_mask_t socket_access_mask);
>  
>  void landlock_put_ruleset(struct landlock_ruleset *const ruleset);
>  void landlock_put_ruleset_deferred(struct landlock_ruleset *const ruleset);
> @@ -282,6 +298,19 @@ landlock_add_net_access_mask(struct landlock_ruleset *const ruleset,
>  		(net_mask << LANDLOCK_SHIFT_ACCESS_NET);
>  }
>  
> +static inline void
> +landlock_add_socket_access_mask(struct landlock_ruleset *const ruleset,
> +			     const access_mask_t socket_access_mask,
> +			     const u16 layer_level)
> +{
> +	access_mask_t socket_mask = socket_access_mask & LANDLOCK_MASK_ACCESS_SOCKET;
> +
> +	/* Should already be checked in sys_landlock_create_ruleset(). */
> +	WARN_ON_ONCE(socket_access_mask != socket_mask);
> +	ruleset->access_masks[layer_level] |=
> +		(socket_mask << LANDLOCK_SHIFT_ACCESS_SOCKET);
> +}
> +
>  static inline access_mask_t
>  landlock_get_raw_fs_access_mask(const struct landlock_ruleset *const ruleset,
>  				const u16 layer_level)
> @@ -309,6 +338,15 @@ landlock_get_net_access_mask(const struct landlock_ruleset *const ruleset,
>  	       LANDLOCK_MASK_ACCESS_NET;
>  }
>  
> +static inline access_mask_t
> +landlock_get_socket_access_mask(const struct landlock_ruleset *const ruleset,
> +			     const u16 layer_level)
> +{
> +	return (ruleset->access_masks[layer_level] >>
> +		LANDLOCK_SHIFT_ACCESS_SOCKET) &
> +	       LANDLOCK_MASK_ACCESS_SOCKET;
> +}
> +
>  bool landlock_unmask_layers(const struct landlock_rule *const rule,
>  			    const access_mask_t access_request,
>  			    layer_mask_t (*const layer_masks)[],
> diff --git a/security/landlock/socket.c b/security/landlock/socket.c
> new file mode 100644
> index 000000000..88b4ef3a1
> --- /dev/null
> +++ b/security/landlock/socket.c
> @@ -0,0 +1,43 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Landlock LSM - Socket management and hooks
> + *
> + * Copyright © 2024 Huawei Tech. Co., Ltd.
> + */
> +
> +#include "limits.h"
> +#include "ruleset.h"
> +#include "socket.h"
> +
> +union socket_key {
> +	struct {
> +		int domain;
> +		int type;
> +	} __packed content;
> +	u64 val;
> +};
> +
> +int landlock_append_socket_rule(struct landlock_ruleset *const ruleset,
> +			     const int domain, const int type, access_mask_t access_rights)
> +{
> +	int err;
> +	const union socket_key socket_key = {
> +		.content.domain = domain,
> +		.content.type = type
> +	};

I'm not convinced this landlock_key.data needs to be changed to u64. We
could have an helper to fit the SOCK_MAX and AF_MAX values into 32-bits,
and a related built-time check to make sure this works.

> +
> +	const struct landlock_id id = {
> +		.key.data = socket_key.val,
> +		.type = LANDLOCK_KEY_SOCKET,
> +	};

Re: [RFC PATCH v1 01/10] landlock: Support socket access-control

[Ivanov Mikhail]

4/12/2024 6:46 PM, Mickaël Salaün wrote:
> On Mon, Apr 08, 2024 at 05:39:18PM +0800, Ivanov Mikhail wrote:
>> Add new socket-related rule type, presented via landlock_socket_attr
>> struct. Add all neccessary entities for socket ruleset support.
>> Add flag LANDLOCK_ACCESS_SOCKET_CREATE that will provide the
>> ability to control socket creation.
>>
>> Change landlock_key.data type from uinptr_t to u64. Socket rule has to
>> contain 32-bit socket family and type values, so landlock_key can be
>> represented as 64-bit number the first 32 bits of which correspond to
>> the socket family and last - to the type.
>>
>> Change ABI version to 5.
>>
>> Signed-off-by: Ivanov Mikhail <ivanov.mikhail1@huawei-partners.com>
>> Reviewed-by: Konstantin Meskhidze <konstantin.meskhidze@huawei.com>
>> ---
>>   include/uapi/linux/landlock.h                | 49 +++++++++++++++++
>>   security/landlock/Makefile                   |  2 +-
>>   security/landlock/limits.h                   |  5 ++
>>   security/landlock/net.c                      |  2 +-
>>   security/landlock/ruleset.c                  | 35 +++++++++++--
>>   security/landlock/ruleset.h                  | 44 ++++++++++++++--
>>   security/landlock/socket.c                   | 43 +++++++++++++++
>>   security/landlock/socket.h                   | 17 ++++++
>>   security/landlock/syscalls.c                 | 55 ++++++++++++++++++--
>>   tools/testing/selftests/landlock/base_test.c |  2 +-
>>   10 files changed, 241 insertions(+), 13 deletions(-)
>>   create mode 100644 security/landlock/socket.c
>>   create mode 100644 security/landlock/socket.h
>>
>> diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
>> index 25c8d7677..8551ade38 100644
>> --- a/include/uapi/linux/landlock.h
>> +++ b/include/uapi/linux/landlock.h
>> @@ -37,6 +37,13 @@ struct landlock_ruleset_attr {
>>   	 * rule explicitly allow them.
>>   	 */
>>   	__u64 handled_access_net;
>> +
>> +	/**
>> +	 * @handled_access_net: Bitmask of actions (cf. `Socket flags`_)
>> +	 * that is handled by this ruleset and should then be forbidden if no
>> +	 * rule explicitly allow them.
>> +	 */
>> +	__u64 handled_access_socket;
>>   };
>>   
>>   /*
>> @@ -65,6 +72,11 @@ enum landlock_rule_type {
>>   	 * landlock_net_port_attr .
>>   	 */
>>   	LANDLOCK_RULE_NET_PORT,
>> +	/**
>> +	 * @LANDLOCK_RULE_SOCKET: Type of a &struct
>> +	 * landlock_socket_attr .
>> +	 */
>> +	LANDLOCK_RULE_SOCKET,
>>   };
>>   
>>   /**
>> @@ -115,6 +127,27 @@ struct landlock_net_port_attr {
>>   	__u64 port;
>>   };
>>   
>> +/**
>> + * struct landlock_socket_attr - Socket definition
>> + *
>> + * Argument of sys_landlock_add_rule().
>> + */
>> +struct landlock_socket_attr {
>> +	/**
>> +	 * @allowed_access: Bitmask of allowed access for a socket
>> +	 * (cf. `Socket flags`_).
>> +	 */
>> +	__u64 allowed_access;
>> +	/**
>> +	 * @domain: Protocol family used for communication (see socket(2)).
>> +	 */
>> +	int domain;
>> +	/**
>> +	 * @type: Socket type (see socket(2)).
>> +	 */
>> +	int type;
>> +};
>> +
>>   /**
>>    * DOC: fs_access
>>    *
>> @@ -244,4 +277,20 @@ struct landlock_net_port_attr {
>>   #define LANDLOCK_ACCESS_NET_BIND_TCP			(1ULL << 0)
>>   #define LANDLOCK_ACCESS_NET_CONNECT_TCP			(1ULL << 1)
>>   /* clang-format on */
>> +
>> +/**
>> + * DOC: socket_acess
>> + *
>> + * Socket flags
>> + * ~~~~~~~~~~~~~~~~
>> + *
>> + * These flags enable to restrict a sandboxed process to a set of
>> + * socket-related actions for specific protocols. This is supported
>> + * since the Landlock ABI version 5.
>> + *
>> + * - %LANDLOCK_ACCESS_SOCKET_CREATE: Create a socket
>> + */
>> +/* clang-format off */
>> +#define LANDLOCK_ACCESS_SOCKET_CREATE			(1ULL << 0)
>> +/* clang-format on */
>>   #endif /* _UAPI_LINUX_LANDLOCK_H */
>> diff --git a/security/landlock/Makefile b/security/landlock/Makefile
>> index b4538b7cf..ff1dd98f6 100644
>> --- a/security/landlock/Makefile
>> +++ b/security/landlock/Makefile
>> @@ -1,6 +1,6 @@
>>   obj-$(CONFIG_SECURITY_LANDLOCK) := landlock.o
>>   
>>   landlock-y := setup.o syscalls.o object.o ruleset.o \
>> -	cred.o task.o fs.o
>> +	cred.o task.o fs.o socket.o
>>   
>>   landlock-$(CONFIG_INET) += net.o
>> diff --git a/security/landlock/limits.h b/security/landlock/limits.h
>> index 93c9c6f91..ebdab587c 100644
>> --- a/security/landlock/limits.h
>> +++ b/security/landlock/limits.h
>> @@ -28,6 +28,11 @@
>>   #define LANDLOCK_NUM_ACCESS_NET		__const_hweight64(LANDLOCK_MASK_ACCESS_NET)
>>   #define LANDLOCK_SHIFT_ACCESS_NET	LANDLOCK_NUM_ACCESS_FS
>>   
>> +#define LANDLOCK_LAST_ACCESS_SOCKET	    LANDLOCK_ACCESS_SOCKET_CREATE
>> +#define LANDLOCK_MASK_ACCESS_SOCKET	    ((LANDLOCK_LAST_ACCESS_SOCKET << 1) - 1)
>> +#define LANDLOCK_NUM_ACCESS_SOCKET		__const_hweight64(LANDLOCK_MASK_ACCESS_SOCKET)
>> +#define LANDLOCK_SHIFT_ACCESS_SOCKET	LANDLOCK_NUM_ACCESS_SOCKET
>> +
>>   /* clang-format on */
>>   
>>   #endif /* _SECURITY_LANDLOCK_LIMITS_H */
>> diff --git a/security/landlock/net.c b/security/landlock/net.c
>> index c8bcd29bd..0e3770d14 100644
>> --- a/security/landlock/net.c
>> +++ b/security/landlock/net.c
>> @@ -159,7 +159,7 @@ static int current_check_access_socket(struct socket *const sock,
>>   			return -EINVAL;
>>   	}
>>   
>> -	id.key.data = (__force uintptr_t)port;
>> +	id.key.data = (__force u64)port;
>>   	BUILD_BUG_ON(sizeof(port) > sizeof(id.key.data));
>>   
>>   	rule = landlock_find_rule(dom, id);
>> diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c
>> index e0a5fbf92..1f1ed8181 100644
>> --- a/security/landlock/ruleset.c
>> +++ b/security/landlock/ruleset.c
>> @@ -40,6 +40,7 @@ static struct landlock_ruleset *create_ruleset(const u32 num_layers)
>>   #if IS_ENABLED(CONFIG_INET)
>>   	new_ruleset->root_net_port = RB_ROOT;
>>   #endif /* IS_ENABLED(CONFIG_INET) */
>> +	new_ruleset->root_socket = RB_ROOT;
>>   
>>   	new_ruleset->num_layers = num_layers;
>>   	/*
>> @@ -52,12 +53,13 @@ static struct landlock_ruleset *create_ruleset(const u32 num_layers)
>>   
>>   struct landlock_ruleset *
>>   landlock_create_ruleset(const access_mask_t fs_access_mask,
>> -			const access_mask_t net_access_mask)
>> +			const access_mask_t net_access_mask,
>> +			const access_mask_t socket_access_mask)
>>   {
>>   	struct landlock_ruleset *new_ruleset;
>>   
>>   	/* Informs about useless ruleset. */
>> -	if (!fs_access_mask && !net_access_mask)
>> +	if (!fs_access_mask && !net_access_mask && !socket_access_mask)
>>   		return ERR_PTR(-ENOMSG);
>>   	new_ruleset = create_ruleset(1);
>>   	if (IS_ERR(new_ruleset))
>> @@ -66,6 +68,8 @@ landlock_create_ruleset(const access_mask_t fs_access_mask,
>>   		landlock_add_fs_access_mask(new_ruleset, fs_access_mask, 0);
>>   	if (net_access_mask)
>>   		landlock_add_net_access_mask(new_ruleset, net_access_mask, 0);
>> +	if (socket_access_mask)
>> +		landlock_add_socket_access_mask(new_ruleset, socket_access_mask, 0);
>>   	return new_ruleset;
>>   }
>>   
>> @@ -89,6 +93,9 @@ static bool is_object_pointer(const enum landlock_key_type key_type)
>>   		return false;
>>   #endif /* IS_ENABLED(CONFIG_INET) */
>>   
>> +	case LANDLOCK_KEY_SOCKET:
>> +		return false;
>> +
>>   	default:
>>   		WARN_ON_ONCE(1);
>>   		return false;
>> @@ -146,6 +153,9 @@ static struct rb_root *get_root(struct landlock_ruleset *const ruleset,
>>   		return &ruleset->root_net_port;
>>   #endif /* IS_ENABLED(CONFIG_INET) */
>>   
>> +	case LANDLOCK_KEY_SOCKET:
>> +		return &ruleset->root_socket;
>> +
>>   	default:
>>   		WARN_ON_ONCE(1);
>>   		return ERR_PTR(-EINVAL);
>> @@ -175,7 +185,8 @@ static void build_check_ruleset(void)
>>   	BUILD_BUG_ON(ruleset.num_layers < LANDLOCK_MAX_NUM_LAYERS);
>>   	BUILD_BUG_ON(access_masks <
>>   		     ((LANDLOCK_MASK_ACCESS_FS << LANDLOCK_SHIFT_ACCESS_FS) |
>> -		      (LANDLOCK_MASK_ACCESS_NET << LANDLOCK_SHIFT_ACCESS_NET)));
>> +		      (LANDLOCK_MASK_ACCESS_NET << LANDLOCK_SHIFT_ACCESS_NET) |
>> +			  (LANDLOCK_MASK_ACCESS_SOCKET << LANDLOCK_SHIFT_ACCESS_SOCKET)));
>>   }
>>   
>>   /**
>> @@ -399,6 +410,11 @@ static int merge_ruleset(struct landlock_ruleset *const dst,
>>   		goto out_unlock;
>>   #endif /* IS_ENABLED(CONFIG_INET) */
>>   
>> +	/* Merges the @src socket tree. */
>> +	err = merge_tree(dst, src, LANDLOCK_KEY_SOCKET);
>> +	if (err)
>> +		goto out_unlock;
>> +
>>   out_unlock:
>>   	mutex_unlock(&src->lock);
>>   	mutex_unlock(&dst->lock);
>> @@ -462,6 +478,11 @@ static int inherit_ruleset(struct landlock_ruleset *const parent,
>>   		goto out_unlock;
>>   #endif /* IS_ENABLED(CONFIG_INET) */
>>   
>> +	/* Copies the @parent socket tree. */
>> +	err = inherit_tree(parent, child, LANDLOCK_KEY_SOCKET);
>> +	if (err)
>> +		goto out_unlock;
>> +
>>   	if (WARN_ON_ONCE(child->num_layers <= parent->num_layers)) {
>>   		err = -EINVAL;
>>   		goto out_unlock;
>> @@ -498,6 +519,10 @@ static void free_ruleset(struct landlock_ruleset *const ruleset)
>>   		free_rule(freeme, LANDLOCK_KEY_NET_PORT);
>>   #endif /* IS_ENABLED(CONFIG_INET) */
>>   
>> +	rbtree_postorder_for_each_entry_safe(freeme, next,
>> +					     &ruleset->root_socket, node)
>> +		free_rule(freeme, LANDLOCK_KEY_SOCKET);
>> +
>>   	put_hierarchy(ruleset->hierarchy);
>>   	kfree(ruleset);
>>   }
>> @@ -708,6 +733,10 @@ landlock_init_layer_masks(const struct landlock_ruleset *const domain,
>>   		break;
>>   #endif /* IS_ENABLED(CONFIG_INET) */
>>   
>> +	case LANDLOCK_KEY_SOCKET:
>> +		get_access_mask = landlock_get_socket_access_mask;
>> +		num_access = LANDLOCK_NUM_ACCESS_SOCKET;
>> +		break;
>>   	default:
>>   		WARN_ON_ONCE(1);
>>   		return 0;
>> diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h
>> index c7f152678..f4213db09 100644
>> --- a/security/landlock/ruleset.h
>> +++ b/security/landlock/ruleset.h
>> @@ -72,10 +72,10 @@ union landlock_key {
>>   	 */
>>   	struct landlock_object *object;
>>   	/**
>> -	 * @data: Raw data to identify an arbitrary 32-bit value
>> +	 * @data: Raw data to identify an arbitrary 64-bit value
>>   	 * (e.g. a TCP port).
>>   	 */
>> -	uintptr_t data;
>> +	u64 data;
>>   };
>>   
>>   /**
>> @@ -92,6 +92,12 @@ enum landlock_key_type {
>>   	 * node keys.
>>   	 */
>>   	LANDLOCK_KEY_NET_PORT,
>> +
>> +	/**
>> +	 * @LANDLOCK_KEY_SOCKET: Type of &landlock_ruleset.root_socket's
>> +	 * node keys.
>> +	 */
>> +	LANDLOCK_KEY_SOCKET,
>>   };
>>   
>>   /**
>> @@ -177,6 +183,15 @@ struct landlock_ruleset {
>>   	struct rb_root root_net_port;
>>   #endif /* IS_ENABLED(CONFIG_INET) */
>>   
>> +	/**
>> +	 * @root_socket: Root of a red-black tree containing &struct
>> +	 * landlock_rule nodes with socket type, described by (domain, type)
>> +	 * pair (see socket(2)). Once a ruleset is tied to a
>> +	 * process (i.e. as a domain), this tree is immutable until @usage
>> +	 * reaches zero.
>> +	 */
>> +	struct rb_root root_socket;
>> +
>>   	/**
>>   	 * @hierarchy: Enables hierarchy identification even when a parent
>>   	 * domain vanishes.  This is needed for the ptrace protection.
>> @@ -233,7 +248,8 @@ struct landlock_ruleset {
>>   
>>   struct landlock_ruleset *
>>   landlock_create_ruleset(const access_mask_t access_mask_fs,
>> -			const access_mask_t access_mask_net);
>> +			const access_mask_t access_mask_net,
>> +			const access_mask_t socket_access_mask);
>>   
>>   void landlock_put_ruleset(struct landlock_ruleset *const ruleset);
>>   void landlock_put_ruleset_deferred(struct landlock_ruleset *const ruleset);
>> @@ -282,6 +298,19 @@ landlock_add_net_access_mask(struct landlock_ruleset *const ruleset,
>>   		(net_mask << LANDLOCK_SHIFT_ACCESS_NET);
>>   }
>>   
>> +static inline void
>> +landlock_add_socket_access_mask(struct landlock_ruleset *const ruleset,
>> +			     const access_mask_t socket_access_mask,
>> +			     const u16 layer_level)
>> +{
>> +	access_mask_t socket_mask = socket_access_mask & LANDLOCK_MASK_ACCESS_SOCKET;
>> +
>> +	/* Should already be checked in sys_landlock_create_ruleset(). */
>> +	WARN_ON_ONCE(socket_access_mask != socket_mask);
>> +	ruleset->access_masks[layer_level] |=
>> +		(socket_mask << LANDLOCK_SHIFT_ACCESS_SOCKET);
>> +}
>> +
>>   static inline access_mask_t
>>   landlock_get_raw_fs_access_mask(const struct landlock_ruleset *const ruleset,
>>   				const u16 layer_level)
>> @@ -309,6 +338,15 @@ landlock_get_net_access_mask(const struct landlock_ruleset *const ruleset,
>>   	       LANDLOCK_MASK_ACCESS_NET;
>>   }
>>   
>> +static inline access_mask_t
>> +landlock_get_socket_access_mask(const struct landlock_ruleset *const ruleset,
>> +			     const u16 layer_level)
>> +{
>> +	return (ruleset->access_masks[layer_level] >>
>> +		LANDLOCK_SHIFT_ACCESS_SOCKET) &
>> +	       LANDLOCK_MASK_ACCESS_SOCKET;
>> +}
>> +
>>   bool landlock_unmask_layers(const struct landlock_rule *const rule,
>>   			    const access_mask_t access_request,
>>   			    layer_mask_t (*const layer_masks)[],
>> diff --git a/security/landlock/socket.c b/security/landlock/socket.c
>> new file mode 100644
>> index 000000000..88b4ef3a1
>> --- /dev/null
>> +++ b/security/landlock/socket.c
>> @@ -0,0 +1,43 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Landlock LSM - Socket management and hooks
>> + *
>> + * Copyright © 2024 Huawei Tech. Co., Ltd.
>> + */
>> +
>> +#include "limits.h"
>> +#include "ruleset.h"
>> +#include "socket.h"
>> +
>> +union socket_key {
>> +	struct {
>> +		int domain;
>> +		int type;
>> +	} __packed content;
>> +	u64 val;
>> +};
>> +
>> +int landlock_append_socket_rule(struct landlock_ruleset *const ruleset,
>> +			     const int domain, const int type, access_mask_t access_rights)
>> +{
>> +	int err;
>> +	const union socket_key socket_key = {
>> +		.content.domain = domain,
>> +		.content.type = type
>> +	};
> 
> I'm not convinced this landlock_key.data needs to be changed to u64. We
> could have an helper to fit the SOCK_MAX and AF_MAX values into 32-bits,
> and a related built-time check to make sure this works.

agreed

> 
>> +
>> +	const struct landlock_id id = {
>> +		.key.data = socket_key.val,
>> +		.type = LANDLOCK_KEY_SOCKET,
>> +	};

[RFC PATCH v1 02/10] landlock: Add hook on socket_create()

[Ivanov Mikhail]

Add hook on socket_create() method. Since it'll be better to have control
over possible socket changes after family-related create() call, hook is
called on socket_post_create(). Handler only checks if the socket type
and family are allowed by domain.

Signed-off-by: Ivanov Mikhail <ivanov.mikhail1@huawei-partners.com>
Reviewed-by: Konstantin Meskhidze <konstantin.meskhidze@huawei.com>
---
 security/landlock/setup.c  |  2 ++
 security/landlock/socket.c | 72 ++++++++++++++++++++++++++++++++++++++
 security/landlock/socket.h |  2 ++
 3 files changed, 76 insertions(+)

diff --git a/security/landlock/setup.c b/security/landlock/setup.c
index 28519a45b..fd4e7e8f3 100644
--- a/security/landlock/setup.c
+++ b/security/landlock/setup.c
@@ -14,6 +14,7 @@
 #include "cred.h"
 #include "fs.h"
 #include "net.h"
+#include "socket.h"
 #include "setup.h"
 #include "task.h"
 
@@ -37,6 +38,7 @@ static int __init landlock_init(void)
 	landlock_add_task_hooks();
 	landlock_add_fs_hooks();
 	landlock_add_net_hooks();
+	landlock_add_socket_hooks();
 	landlock_initialized = true;
 	pr_info("Up and running.\n");
 	return 0;
diff --git a/security/landlock/socket.c b/security/landlock/socket.c
index 88b4ef3a1..cba584543 100644
--- a/security/landlock/socket.c
+++ b/security/landlock/socket.c
@@ -5,6 +5,10 @@
  * Copyright © 2024 Huawei Tech. Co., Ltd.
  */
 
+#include <linux/net.h>
+#include <net/sock.h>
+
+#include "cred.h"
 #include "limits.h"
 #include "ruleset.h"
 #include "socket.h"
@@ -41,3 +45,71 @@ int landlock_append_socket_rule(struct landlock_ruleset *const ruleset,
 
 	return err;
 }
+
+static access_mask_t
+get_raw_handled_socket_accesses(const struct landlock_ruleset *const domain)
+{
+	access_mask_t access_dom = 0;
+	size_t layer_level;
+
+	for (layer_level = 0; layer_level < domain->num_layers; layer_level++)
+		access_dom |= landlock_get_socket_access_mask(domain, layer_level);
+	return access_dom;
+}
+
+static const struct landlock_ruleset *get_current_socket_domain(void)
+{
+	const struct landlock_ruleset *const dom =
+		landlock_get_current_domain();
+
+	if (!dom || !get_raw_handled_socket_accesses(dom))
+		return NULL;
+
+	return dom;
+}
+
+static int current_check_access_socket(struct socket *const sock,
+				       const access_mask_t access_request)
+{
+	union socket_key socket_key;
+	layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_SOCKET] = {};
+	const struct landlock_rule *rule;
+	access_mask_t handled_access;
+	struct landlock_id id = {
+		.type = LANDLOCK_KEY_SOCKET,
+	};
+	const struct landlock_ruleset *const dom = get_current_socket_domain();
+
+	if (!dom)
+		return 0;
+	if (WARN_ON_ONCE(dom->num_layers < 1))
+		return -EACCES;
+
+	socket_key.content.type = sock->type;
+	socket_key.content.domain = sock->sk->__sk_common.skc_family;
+	id.key.data = socket_key.val;
+
+	rule = landlock_find_rule(dom, id);
+	handled_access = landlock_init_layer_masks(
+		dom, access_request, &layer_masks, LANDLOCK_KEY_SOCKET);
+	if (landlock_unmask_layers(rule, handled_access, &layer_masks,
+				   ARRAY_SIZE(layer_masks)))
+		return 0;
+	return -EACCES;
+}
+
+static int hook_socket_create(struct socket *const sock,
+			    int family, int type, int protocol, int kern)
+{
+	return current_check_access_socket(sock, LANDLOCK_ACCESS_SOCKET_CREATE);
+}
+
+static struct security_hook_list landlock_hooks[] __ro_after_init = {
+	LSM_HOOK_INIT(socket_post_create, hook_socket_create),
+};
+
+__init void landlock_add_socket_hooks(void)
+{
+	security_add_hooks(landlock_hooks, ARRAY_SIZE(landlock_hooks),
+			   &landlock_lsmid);
+}
diff --git a/security/landlock/socket.h b/security/landlock/socket.h
index 2b8f9ae7d..152f4d427 100644
--- a/security/landlock/socket.h
+++ b/security/landlock/socket.h
@@ -10,6 +10,8 @@
 
 #include "ruleset.h"
 
+__init void landlock_add_socket_hooks(void);
+
 int landlock_append_socket_rule(struct landlock_ruleset *const ruleset,
 				 const int domain, const int type,
 				 access_mask_t access_rights);
-- 
2.34.1

[RFC PATCH v1 03/10] selftests/landlock: Create 'create' test

[Ivanov Mikhail]

Initiate socket_test.c selftests. Add protocol fixture for tests
with changeable domain/type values. Only most common variants of
protocols (like ipv4-tcp,ipv6-udp, unix) were added.
Add simple socket access right checking test.

Signed-off-by: Ivanov Mikhail <ivanov.mikhail1@huawei-partners.com>
Reviewed-by: Konstantin Meskhidze <konstantin.meskhidze@huawei.com>
---
 .../testing/selftests/landlock/socket_test.c  | 197 ++++++++++++++++++
 1 file changed, 197 insertions(+)
 create mode 100644 tools/testing/selftests/landlock/socket_test.c

diff --git a/tools/testing/selftests/landlock/socket_test.c b/tools/testing/selftests/landlock/socket_test.c
new file mode 100644
index 000000000..525f4f7df
--- /dev/null
+++ b/tools/testing/selftests/landlock/socket_test.c
@@ -0,0 +1,197 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Landlock tests - Socket
+ *
+ * Copyright © 2024 Huawei Tech. Co., Ltd.
+ */
+
+#define _GNU_SOURCE
+
+#include <errno.h>
+#include <linux/landlock.h>
+#include <sched.h>
+#include <string.h>
+#include <sys/prctl.h>
+#include <sys/socket.h>
+
+#include "common.h"
+
+/* clang-format off */
+
+#define ACCESS_LAST LANDLOCK_ACCESS_SOCKET_CREATE
+
+#define ACCESS_ALL ( \
+	LANDLOCK_ACCESS_SOCKET_CREATE)
+
+/* clang-format on */
+
+struct protocol_variant {
+	int domain;
+	int type;
+};
+
+struct service_fixture {
+	struct protocol_variant protocol;
+};
+
+static void setup_namespace(struct __test_metadata *const _metadata)
+{
+	set_cap(_metadata, CAP_SYS_ADMIN);
+	ASSERT_EQ(0, unshare(CLONE_NEWNET));
+	clear_cap(_metadata, CAP_SYS_ADMIN);
+}
+
+static int socket_variant(const struct service_fixture *const srv)
+{
+	int ret;
+
+	ret = socket(srv->protocol.domain, srv->protocol.type | SOCK_CLOEXEC,
+			 0);
+	if (ret < 0)
+		return -errno;
+	return ret;
+}
+
+FIXTURE(protocol)
+{
+	struct service_fixture srv0;
+};
+
+FIXTURE_VARIANT(protocol)
+{
+	const struct protocol_variant protocol;
+};
+
+FIXTURE_SETUP(protocol)
+{
+	disable_caps(_metadata);
+	self->srv0.protocol = variant->protocol;
+	setup_namespace(_metadata);
+};
+
+FIXTURE_TEARDOWN(protocol)
+{
+}
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(protocol, unspec) {
+	/* clang-format on */
+	.protocol = {
+		.domain = AF_UNSPEC,
+		.type = SOCK_STREAM,
+	},
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(protocol, unix_stream) {
+	/* clang-format on */
+	.protocol = {
+		.domain = AF_UNIX,
+		.type = SOCK_STREAM,
+	},
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(protocol, unix_dgram) {
+	/* clang-format on */
+	.protocol = {
+		.domain = AF_UNIX,
+		.type = SOCK_DGRAM,
+	},
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(protocol, ipv4_tcp) {
+	/* clang-format on */
+	.protocol = {
+		.domain = AF_INET,
+		.type = SOCK_STREAM,
+	},
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(protocol, ipv4_udp) {
+	/* clang-format on */
+	.protocol = {
+		.domain = AF_INET,
+		.type = SOCK_DGRAM,
+	},
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(protocol, ipv6_tcp) {
+	/* clang-format on */
+	.protocol = {
+		.domain = AF_INET6,
+		.type = SOCK_STREAM,
+	},
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(protocol, ipv6_udp) {
+	/* clang-format on */
+	.protocol = {
+		.domain = AF_INET6,
+		.type = SOCK_DGRAM,
+	},
+};
+
+static void test_socket_create(struct __test_metadata *const _metadata,
+				  const struct service_fixture *const srv,
+				  const bool deny_create)
+{
+	int fd;
+
+	fd = socket_variant(srv);
+	if (srv->protocol.domain == AF_UNSPEC) {
+		EXPECT_EQ(fd, -EAFNOSUPPORT);
+	} else if (deny_create) {
+		EXPECT_EQ(fd, -EACCES);
+	} else {
+		EXPECT_LE(0, fd)
+		{
+			TH_LOG("Failed to create socket: %s", strerror(errno));
+		}
+		EXPECT_EQ(0, close(fd));
+	}
+}
+
+TEST_F(protocol, create)
+{
+	const struct landlock_ruleset_attr ruleset_attr = {
+		.handled_access_socket = LANDLOCK_ACCESS_SOCKET_CREATE,
+	};
+	const struct landlock_socket_attr create_socket_attr = {
+		.allowed_access = LANDLOCK_ACCESS_SOCKET_CREATE,
+		.domain = self->srv0.protocol.domain,
+		.type = self->srv0.protocol.type,
+	};
+
+	int ruleset_fd;
+
+	/* Allowed create */
+	ruleset_fd = landlock_create_ruleset(&ruleset_attr,
+							sizeof(ruleset_attr), 0);
+	ASSERT_LE(0, ruleset_fd);
+
+	ASSERT_EQ(0,
+			landlock_add_rule(ruleset_fd, LANDLOCK_RULE_SOCKET,
+					&create_socket_attr, 0));
+
+	enforce_ruleset(_metadata, ruleset_fd);
+	EXPECT_EQ(0, close(ruleset_fd));
+
+	test_socket_create(_metadata, &self->srv0, false);
+
+	/* Denied create */
+	ruleset_fd = landlock_create_ruleset(&ruleset_attr,
+							sizeof(ruleset_attr), 0);
+	ASSERT_LE(0, ruleset_fd);
+
+	enforce_ruleset(_metadata, ruleset_fd);
+	EXPECT_EQ(0, close(ruleset_fd));
+
+	test_socket_create(_metadata, &self->srv0, true);
+}
+
+TEST_HARNESS_MAIN
-- 
2.34.1

Re: [RFC PATCH v1 03/10] selftests/landlock: Create 'create' test

[Günther Noack]

Hello!

I am very happy to see this patch set, this is a very valuable feature, IMHO! :)

Regarding the subject of this patch:

  [RFC PATCH v1 03/10] selftests/landlock: Create 'create' test
                                                   ^^^^^^

This was probably meant to say "socket"?

(In my mind, it is a good call to put the test in a separate file -
the existing test files have grown too large already.)


On Mon, Apr 08, 2024 at 05:39:20PM +0800, Ivanov Mikhail wrote:
> Initiate socket_test.c selftests. Add protocol fixture for tests
> with changeable domain/type values. Only most common variants of
> protocols (like ipv4-tcp,ipv6-udp, unix) were added.
> Add simple socket access right checking test.
> 
> Signed-off-by: Ivanov Mikhail <ivanov.mikhail1@huawei-partners.com>
> Reviewed-by: Konstantin Meskhidze <konstantin.meskhidze@huawei.com>
> ---
>  .../testing/selftests/landlock/socket_test.c  | 197 ++++++++++++++++++
>  1 file changed, 197 insertions(+)
>  create mode 100644 tools/testing/selftests/landlock/socket_test.c
> 
> diff --git a/tools/testing/selftests/landlock/socket_test.c b/tools/testing/selftests/landlock/socket_test.c
> new file mode 100644
> index 000000000..525f4f7df
> --- /dev/null
> +++ b/tools/testing/selftests/landlock/socket_test.c
> @@ -0,0 +1,197 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Landlock tests - Socket
> + *
> + * Copyright © 2024 Huawei Tech. Co., Ltd.
> + */
> +
> +#define _GNU_SOURCE
> +
> +#include <errno.h>
> +#include <linux/landlock.h>
> +#include <sched.h>
> +#include <string.h>
> +#include <sys/prctl.h>
> +#include <sys/socket.h>
> +
> +#include "common.h"
> +
> +/* clang-format off */
> +
> +#define ACCESS_LAST LANDLOCK_ACCESS_SOCKET_CREATE
> +
> +#define ACCESS_ALL ( \
> +	LANDLOCK_ACCESS_SOCKET_CREATE)
> +
> +/* clang-format on */
> +
> +struct protocol_variant {
> +	int domain;
> +	int type;
> +};
> +
> +struct service_fixture {
> +	struct protocol_variant protocol;
> +};
> +
> +static void setup_namespace(struct __test_metadata *const _metadata)
> +{
> +	set_cap(_metadata, CAP_SYS_ADMIN);
> +	ASSERT_EQ(0, unshare(CLONE_NEWNET));
> +	clear_cap(_metadata, CAP_SYS_ADMIN);
> +}
> +
> +static int socket_variant(const struct service_fixture *const srv)
> +{
> +	int ret;
> +
> +	ret = socket(srv->protocol.domain, srv->protocol.type | SOCK_CLOEXEC,
> +			 0);
> +	if (ret < 0)
> +		return -errno;
> +	return ret;
> +}

This helper is mostly concerned with mapping the error code.

In the fs_test.c, we have dealt with such use cases with helpers like
test_open_rel() and test_open().  These helpers attempt to open the file, take
the same arguments as open(2), but instead of returning the opened fd, they only
return 0 or errno.  Do you think this would be an option here?

Then you could write your tests as

  ASSERT_EQ(EACCES, test_socket(p->domain, p->type, 0));

and the test would (a) more obviously map to socket(2), and (b) keep relevant
information like the expected error code at the top level of the test.

> +
> +FIXTURE(protocol)
> +{
> +	struct service_fixture srv0;
> +};
> +
> +FIXTURE_VARIANT(protocol)
> +{
> +	const struct protocol_variant protocol;
> +};
> +
> +FIXTURE_SETUP(protocol)
> +{
> +	disable_caps(_metadata);
> +	self->srv0.protocol = variant->protocol;
> +	setup_namespace(_metadata);
> +};
> +
> +FIXTURE_TEARDOWN(protocol)
> +{
> +}
> +
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(protocol, unspec) {
> +	/* clang-format on */
> +	.protocol = {
> +		.domain = AF_UNSPEC,
> +		.type = SOCK_STREAM,
> +	},
> +};
> +
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(protocol, unix_stream) {
> +	/* clang-format on */
> +	.protocol = {
> +		.domain = AF_UNIX,
> +		.type = SOCK_STREAM,
> +	},
> +};
> +
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(protocol, unix_dgram) {
> +	/* clang-format on */
> +	.protocol = {
> +		.domain = AF_UNIX,
> +		.type = SOCK_DGRAM,
> +	},
> +};
> +
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(protocol, ipv4_tcp) {
> +	/* clang-format on */
> +	.protocol = {
> +		.domain = AF_INET,
> +		.type = SOCK_STREAM,
> +	},
> +};
> +
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(protocol, ipv4_udp) {
> +	/* clang-format on */
> +	.protocol = {
> +		.domain = AF_INET,
> +		.type = SOCK_DGRAM,
> +	},
> +};
> +
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(protocol, ipv6_tcp) {
> +	/* clang-format on */
> +	.protocol = {
> +		.domain = AF_INET6,
> +		.type = SOCK_STREAM,
> +	},
> +};
> +
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(protocol, ipv6_udp) {
> +	/* clang-format on */
> +	.protocol = {
> +		.domain = AF_INET6,
> +		.type = SOCK_DGRAM,
> +	},
> +};
> +
> +static void test_socket_create(struct __test_metadata *const _metadata,
> +				  const struct service_fixture *const srv,
> +				  const bool deny_create)
> +{
> +	int fd;
> +
> +	fd = socket_variant(srv);
> +	if (srv->protocol.domain == AF_UNSPEC) {
> +		EXPECT_EQ(fd, -EAFNOSUPPORT);
> +	} else if (deny_create) {
> +		EXPECT_EQ(fd, -EACCES);
> +	} else {
> +		EXPECT_LE(0, fd)
> +		{
> +			TH_LOG("Failed to create socket: %s", strerror(errno));
> +		}
> +		EXPECT_EQ(0, close(fd));
> +	}
> +}

This is slightly too much logic in a test helper, for my taste,
and the meaning of the true/false argument in the main test below
is not very obvious.

Extending the idea from above, if test_socket() was simpler, would it
be possible to turn these fixtures into something shorter like this:

  ASSERT_EQ(EAFNOSUPPORT, test_socket(AF_UNSPEC, SOCK_STREAM, 0));
  ASSERT_EQ(EACCES, test_socket(AF_UNIX, SOCK_STREAM, 0));
  ASSERT_EQ(EACCES, test_socket(AF_UNIX, SOCK_DGRAM, 0));
  ASSERT_EQ(EACCES, test_socket(AF_INET, SOCK_STREAM, 0));
  // etc.

Would that make the tests easier to write, to list out the table of
expected values aspect like that, rather than in a fixture?


> +
> +TEST_F(protocol, create)
> +{
> +	const struct landlock_ruleset_attr ruleset_attr = {
> +		.handled_access_socket = LANDLOCK_ACCESS_SOCKET_CREATE,
> +	};
> +	const struct landlock_socket_attr create_socket_attr = {
> +		.allowed_access = LANDLOCK_ACCESS_SOCKET_CREATE,
> +		.domain = self->srv0.protocol.domain,
> +		.type = self->srv0.protocol.type,
> +	};
> +
> +	int ruleset_fd;
> +
> +	/* Allowed create */
> +	ruleset_fd = landlock_create_ruleset(&ruleset_attr,
> +							sizeof(ruleset_attr), 0);
> +	ASSERT_LE(0, ruleset_fd);
> +
> +	ASSERT_EQ(0,
> +			landlock_add_rule(ruleset_fd, LANDLOCK_RULE_SOCKET,
> +					&create_socket_attr, 0));

The indentation looks wrong?  We run clang-format on Landlock files.

  clang-format -i security/landlock/*.[ch] \
  	include/uapi/linux/landlock.h \
  	tools/testing/selftests/landlock/*.[ch]

—Günther

Re: [RFC PATCH v1 03/10] selftests/landlock: Create 'create' test

[Ivanov Mikhail]

4/8/2024 4:08 PM, Günther Noack wrote:
> Hello!
> 
> I am very happy to see this patch set, this is a very valuable feature, IMHO! :)
> 
> Regarding the subject of this patch:
> 
>    [RFC PATCH v1 03/10] selftests/landlock: Create 'create' test
>                                                     ^^^^^^
> 
> This was probably meant to say "socket"?

I wanted each such patch to have the name of the test that this patch
adds (without specifying the fixture, since this is not necessary
information, which only complicates the name). I think

     [RFC PATCH v1 03/10] selftests/landlock: Add 'create' test
                                              ~~~
renaming should be fine.

> 
> (In my mind, it is a good call to put the test in a separate file -
> the existing test files have grown too large already.)
> 
> 
> On Mon, Apr 08, 2024 at 05:39:20PM +0800, Ivanov Mikhail wrote:
>> Initiate socket_test.c selftests. Add protocol fixture for tests
>> with changeable domain/type values. Only most common variants of
>> protocols (like ipv4-tcp,ipv6-udp, unix) were added.
>> Add simple socket access right checking test.
>>
>> Signed-off-by: Ivanov Mikhail <ivanov.mikhail1@huawei-partners.com>
>> Reviewed-by: Konstantin Meskhidze <konstantin.meskhidze@huawei.com>
>> ---
>>   .../testing/selftests/landlock/socket_test.c  | 197 ++++++++++++++++++
>>   1 file changed, 197 insertions(+)
>>   create mode 100644 tools/testing/selftests/landlock/socket_test.c
>>
>> diff --git a/tools/testing/selftests/landlock/socket_test.c b/tools/testing/selftests/landlock/socket_test.c
>> new file mode 100644
>> index 000000000..525f4f7df
>> --- /dev/null
>> +++ b/tools/testing/selftests/landlock/socket_test.c
>> @@ -0,0 +1,197 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Landlock tests - Socket
>> + *
>> + * Copyright © 2024 Huawei Tech. Co., Ltd.
>> + */
>> +
>> +#define _GNU_SOURCE
>> +
>> +#include <errno.h>
>> +#include <linux/landlock.h>
>> +#include <sched.h>
>> +#include <string.h>
>> +#include <sys/prctl.h>
>> +#include <sys/socket.h>
>> +
>> +#include "common.h"
>> +
>> +/* clang-format off */
>> +
>> +#define ACCESS_LAST LANDLOCK_ACCESS_SOCKET_CREATE
>> +
>> +#define ACCESS_ALL ( \
>> +	LANDLOCK_ACCESS_SOCKET_CREATE)
>> +
>> +/* clang-format on */
>> +
>> +struct protocol_variant {
>> +	int domain;
>> +	int type;
>> +};
>> +
>> +struct service_fixture {
>> +	struct protocol_variant protocol;
>> +};
>> +
>> +static void setup_namespace(struct __test_metadata *const _metadata)
>> +{
>> +	set_cap(_metadata, CAP_SYS_ADMIN);
>> +	ASSERT_EQ(0, unshare(CLONE_NEWNET));
>> +	clear_cap(_metadata, CAP_SYS_ADMIN);
>> +}
>> +
>> +static int socket_variant(const struct service_fixture *const srv)
>> +{
>> +	int ret;
>> +
>> +	ret = socket(srv->protocol.domain, srv->protocol.type | SOCK_CLOEXEC,
>> +			 0);
>> +	if (ret < 0)
>> +		return -errno;
>> +	return ret;
>> +}
> 
> This helper is mostly concerned with mapping the error code.
> 
> In the fs_test.c, we have dealt with such use cases with helpers like
> test_open_rel() and test_open().  These helpers attempt to open the file, take
> the same arguments as open(2), but instead of returning the opened fd, they only
> return 0 or errno.  Do you think this would be an option here?
> 
> Then you could write your tests as
> 
>    ASSERT_EQ(EACCES, test_socket(p->domain, p->type, 0));
> 
> and the test would (a) more obviously map to socket(2), and (b) keep relevant
> information like the expected error code at the top level of the test.
> 

I thought that `socket_variant()` would be suitable for future tests
where sockets can be used after creation (e.g. for sending FDs). But
until then, it's really better to replace it with what you suggested.

>> +
>> +FIXTURE(protocol)
>> +{
>> +	struct service_fixture srv0;
>> +};
>> +
>> +FIXTURE_VARIANT(protocol)
>> +{
>> +	const struct protocol_variant protocol;
>> +};
>> +
>> +FIXTURE_SETUP(protocol)
>> +{
>> +	disable_caps(_metadata);
>> +	self->srv0.protocol = variant->protocol;
>> +	setup_namespace(_metadata);
>> +};
>> +
>> +FIXTURE_TEARDOWN(protocol)
>> +{
>> +}
>> +
>> +/* clang-format off */
>> +FIXTURE_VARIANT_ADD(protocol, unspec) {
>> +	/* clang-format on */
>> +	.protocol = {
>> +		.domain = AF_UNSPEC,
>> +		.type = SOCK_STREAM,
>> +	},
>> +};
>> +
>> +/* clang-format off */
>> +FIXTURE_VARIANT_ADD(protocol, unix_stream) {
>> +	/* clang-format on */
>> +	.protocol = {
>> +		.domain = AF_UNIX,
>> +		.type = SOCK_STREAM,
>> +	},
>> +};
>> +
>> +/* clang-format off */
>> +FIXTURE_VARIANT_ADD(protocol, unix_dgram) {
>> +	/* clang-format on */
>> +	.protocol = {
>> +		.domain = AF_UNIX,
>> +		.type = SOCK_DGRAM,
>> +	},
>> +};
>> +
>> +/* clang-format off */
>> +FIXTURE_VARIANT_ADD(protocol, ipv4_tcp) {
>> +	/* clang-format on */
>> +	.protocol = {
>> +		.domain = AF_INET,
>> +		.type = SOCK_STREAM,
>> +	},
>> +};
>> +
>> +/* clang-format off */
>> +FIXTURE_VARIANT_ADD(protocol, ipv4_udp) {
>> +	/* clang-format on */
>> +	.protocol = {
>> +		.domain = AF_INET,
>> +		.type = SOCK_DGRAM,
>> +	},
>> +};
>> +
>> +/* clang-format off */
>> +FIXTURE_VARIANT_ADD(protocol, ipv6_tcp) {
>> +	/* clang-format on */
>> +	.protocol = {
>> +		.domain = AF_INET6,
>> +		.type = SOCK_STREAM,
>> +	},
>> +};
>> +
>> +/* clang-format off */
>> +FIXTURE_VARIANT_ADD(protocol, ipv6_udp) {
>> +	/* clang-format on */
>> +	.protocol = {
>> +		.domain = AF_INET6,
>> +		.type = SOCK_DGRAM,
>> +	},
>> +};
>> +
>> +static void test_socket_create(struct __test_metadata *const _metadata,
>> +				  const struct service_fixture *const srv,
>> +				  const bool deny_create)
>> +{
>> +	int fd;
>> +
>> +	fd = socket_variant(srv);
>> +	if (srv->protocol.domain == AF_UNSPEC) {
>> +		EXPECT_EQ(fd, -EAFNOSUPPORT);
>> +	} else if (deny_create) {
>> +		EXPECT_EQ(fd, -EACCES);
>> +	} else {
>> +		EXPECT_LE(0, fd)
>> +		{
>> +			TH_LOG("Failed to create socket: %s", strerror(errno));
>> +		}
>> +		EXPECT_EQ(0, close(fd));
>> +	}
>> +}
> 
> This is slightly too much logic in a test helper, for my taste,
> and the meaning of the true/false argument in the main test below
> is not very obvious.
> 
> Extending the idea from above, if test_socket() was simpler, would it
> be possible to turn these fixtures into something shorter like this:
> 
>    ASSERT_EQ(EAFNOSUPPORT, test_socket(AF_UNSPEC, SOCK_STREAM, 0));
>    ASSERT_EQ(EACCES, test_socket(AF_UNIX, SOCK_STREAM, 0));
>    ASSERT_EQ(EACCES, test_socket(AF_UNIX, SOCK_DGRAM, 0));
>    ASSERT_EQ(EACCES, test_socket(AF_INET, SOCK_STREAM, 0));
>    // etc.
> 
> Would that make the tests easier to write, to list out the table of
> expected values aspect like that, rather than in a fixture?
> 
> 

Initially, I conceived this function as an entity that allows to
separate the logic associated with the tested methods or usecases from
the logic of configuring the state of the Landlock ruleset in the
sandbox.

But at the moment, `test_socket_create()` is obviously a wrapper over
socket(2). So for now it's worth removing unnecessary logic.

But i don't think it's worth removing the fixtures here:

   * in my opinion, the design of the fixtures is quite convenient.
     It allows you to separate the definition of the object under test
     from the test case. E.g. test protocol.create checks the ability of
     Landlock to restrict the creation of a socket of a certain type,
     rather than the ability to restrict creation of UNIX, TCP, UDP...
     sockets

   * with adding more tests, it may be necessary to check all protocols
     in each of them

AF_UNSPEC should be removed from fixture variant to separate test,
though.

>> +
>> +TEST_F(protocol, create)
>> +{
>> +	const struct landlock_ruleset_attr ruleset_attr = {
>> +		.handled_access_socket = LANDLOCK_ACCESS_SOCKET_CREATE,
>> +	};
>> +	const struct landlock_socket_attr create_socket_attr = {
>> +		.allowed_access = LANDLOCK_ACCESS_SOCKET_CREATE,
>> +		.domain = self->srv0.protocol.domain,
>> +		.type = self->srv0.protocol.type,
>> +	};
>> +
>> +	int ruleset_fd;
>> +
>> +	/* Allowed create */
>> +	ruleset_fd = landlock_create_ruleset(&ruleset_attr,
>> +							sizeof(ruleset_attr), 0);
>> +	ASSERT_LE(0, ruleset_fd);
>> +
>> +	ASSERT_EQ(0,
>> +			landlock_add_rule(ruleset_fd, LANDLOCK_RULE_SOCKET,
>> +					&create_socket_attr, 0));
> 
> The indentation looks wrong?  We run clang-format on Landlock files.
> 
>    clang-format -i security/landlock/*.[ch] \
>    	include/uapi/linux/landlock.h \
>    	tools/testing/selftests/landlock/*.[ch]
> 

Thanks! I'll fix indentation in the patch.

> —Günther

Re: [RFC PATCH v1 03/10] selftests/landlock: Create 'create' test

[Mickaël Salaün]

On Thu, Apr 11, 2024 at 06:58:34PM +0300, Ivanov Mikhail wrote:
> 
> 4/8/2024 4:08 PM, Günther Noack wrote:
> > Hello!
> > 
> > I am very happy to see this patch set, this is a very valuable feature, IMHO! :)
> > 
> > Regarding the subject of this patch:
> > 
> >    [RFC PATCH v1 03/10] selftests/landlock: Create 'create' test
> >                                                     ^^^^^^
> > 
> > This was probably meant to say "socket"?
> 
> I wanted each such patch to have the name of the test that this patch
> adds (without specifying the fixture, since this is not necessary
> information, which only complicates the name). I think
> 
>     [RFC PATCH v1 03/10] selftests/landlock: Add 'create' test
>                                              ~~~
> renaming should be fine.


Maybe something like this?
"selftests/landlock: Add protocol.create to socket tests"


> 
> > 
> > (In my mind, it is a good call to put the test in a separate file -
> > the existing test files have grown too large already.)
> > 
> > 
> > On Mon, Apr 08, 2024 at 05:39:20PM +0800, Ivanov Mikhail wrote:
> > > Initiate socket_test.c selftests. Add protocol fixture for tests
> > > with changeable domain/type values. Only most common variants of
> > > protocols (like ipv4-tcp,ipv6-udp, unix) were added.
> > > Add simple socket access right checking test.
> > > 
> > > Signed-off-by: Ivanov Mikhail <ivanov.mikhail1@huawei-partners.com>
> > > Reviewed-by: Konstantin Meskhidze <konstantin.meskhidze@huawei.com>
> > > ---
> > >   .../testing/selftests/landlock/socket_test.c  | 197 ++++++++++++++++++
> > >   1 file changed, 197 insertions(+)
> > >   create mode 100644 tools/testing/selftests/landlock/socket_test.c
> > > 
> > > diff --git a/tools/testing/selftests/landlock/socket_test.c b/tools/testing/selftests/landlock/socket_test.c
> > > new file mode 100644
> > > index 000000000..525f4f7df
> > > --- /dev/null
> > > +++ b/tools/testing/selftests/landlock/socket_test.c
> > > @@ -0,0 +1,197 @@
> > > +// SPDX-License-Identifier: GPL-2.0-only
> > > +/*
> > > + * Landlock tests - Socket
> > > + *
> > > + * Copyright © 2024 Huawei Tech. Co., Ltd.
> > > + */
> > > +
> > > +#define _GNU_SOURCE
> > > +
> > > +#include <errno.h>
> > > +#include <linux/landlock.h>
> > > +#include <sched.h>
> > > +#include <string.h>
> > > +#include <sys/prctl.h>
> > > +#include <sys/socket.h>
> > > +
> > > +#include "common.h"
> > > +
> > > +/* clang-format off */
> > > +
> > > +#define ACCESS_LAST LANDLOCK_ACCESS_SOCKET_CREATE
> > > +
> > > +#define ACCESS_ALL ( \
> > > +	LANDLOCK_ACCESS_SOCKET_CREATE)
> > > +
> > > +/* clang-format on */
> > > +
> > > +struct protocol_variant {
> > > +	int domain;
> > > +	int type;
> > > +};
> > > +
> > > +struct service_fixture {
> > > +	struct protocol_variant protocol;
> > > +};
> > > +
> > > +static void setup_namespace(struct __test_metadata *const _metadata)
> > > +{
> > > +	set_cap(_metadata, CAP_SYS_ADMIN);
> > > +	ASSERT_EQ(0, unshare(CLONE_NEWNET));
> > > +	clear_cap(_metadata, CAP_SYS_ADMIN);
> > > +}
> > > +
> > > +static int socket_variant(const struct service_fixture *const srv)
> > > +{
> > > +	int ret;
> > > +
> > > +	ret = socket(srv->protocol.domain, srv->protocol.type | SOCK_CLOEXEC,
> > > +			 0);
> > > +	if (ret < 0)
> > > +		return -errno;
> > > +	return ret;
> > > +}
> > 
> > This helper is mostly concerned with mapping the error code.
> > 
> > In the fs_test.c, we have dealt with such use cases with helpers like
> > test_open_rel() and test_open().  These helpers attempt to open the file, take
> > the same arguments as open(2), but instead of returning the opened fd, they only
> > return 0 or errno.  Do you think this would be an option here?
> > 
> > Then you could write your tests as
> > 
> >    ASSERT_EQ(EACCES, test_socket(p->domain, p->type, 0));
> > 
> > and the test would (a) more obviously map to socket(2), and (b) keep relevant
> > information like the expected error code at the top level of the test.
> > 
> 
> I thought that `socket_variant()` would be suitable for future tests
> where sockets can be used after creation (e.g. for sending FDs). But
> until then, it's really better to replace it with what you suggested.

You can move common code/helpers from net_test.c to common.h (with a
dedicated patch) to avoid duplicating code.

> 
> > > +
> > > +FIXTURE(protocol)
> > > +{
> > > +	struct service_fixture srv0;
> > > +};
> > > +
> > > +FIXTURE_VARIANT(protocol)
> > > +{
> > > +	const struct protocol_variant protocol;
> > > +};
> > > +
> > > +FIXTURE_SETUP(protocol)
> > > +{
> > > +	disable_caps(_metadata);
> > > +	self->srv0.protocol = variant->protocol;
> > > +	setup_namespace(_metadata);
> > > +};
> > > +
> > > +FIXTURE_TEARDOWN(protocol)
> > > +{
> > > +}
> > > +
> > > +/* clang-format off */
> > > +FIXTURE_VARIANT_ADD(protocol, unspec) {
> > > +	/* clang-format on */
> > > +	.protocol = {
> > > +		.domain = AF_UNSPEC,
> > > +		.type = SOCK_STREAM,
> > > +	},
> > > +};
> > > +
> > > +/* clang-format off */
> > > +FIXTURE_VARIANT_ADD(protocol, unix_stream) {
> > > +	/* clang-format on */
> > > +	.protocol = {
> > > +		.domain = AF_UNIX,
> > > +		.type = SOCK_STREAM,
> > > +	},
> > > +};
> > > +
> > > +/* clang-format off */
> > > +FIXTURE_VARIANT_ADD(protocol, unix_dgram) {
> > > +	/* clang-format on */
> > > +	.protocol = {
> > > +		.domain = AF_UNIX,
> > > +		.type = SOCK_DGRAM,
> > > +	},
> > > +};
> > > +
> > > +/* clang-format off */
> > > +FIXTURE_VARIANT_ADD(protocol, ipv4_tcp) {
> > > +	/* clang-format on */
> > > +	.protocol = {
> > > +		.domain = AF_INET,
> > > +		.type = SOCK_STREAM,
> > > +	},
> > > +};
> > > +
> > > +/* clang-format off */
> > > +FIXTURE_VARIANT_ADD(protocol, ipv4_udp) {
> > > +	/* clang-format on */
> > > +	.protocol = {
> > > +		.domain = AF_INET,
> > > +		.type = SOCK_DGRAM,
> > > +	},
> > > +};
> > > +
> > > +/* clang-format off */
> > > +FIXTURE_VARIANT_ADD(protocol, ipv6_tcp) {
> > > +	/* clang-format on */
> > > +	.protocol = {
> > > +		.domain = AF_INET6,
> > > +		.type = SOCK_STREAM,
> > > +	},
> > > +};
> > > +
> > > +/* clang-format off */
> > > +FIXTURE_VARIANT_ADD(protocol, ipv6_udp) {
> > > +	/* clang-format on */
> > > +	.protocol = {
> > > +		.domain = AF_INET6,
> > > +		.type = SOCK_DGRAM,
> > > +	},
> > > +};
> > > +
> > > +static void test_socket_create(struct __test_metadata *const _metadata,
> > > +				  const struct service_fixture *const srv,
> > > +				  const bool deny_create)
> > > +{
> > > +	int fd;
> > > +
> > > +	fd = socket_variant(srv);
> > > +	if (srv->protocol.domain == AF_UNSPEC) {
> > > +		EXPECT_EQ(fd, -EAFNOSUPPORT);
> > > +	} else if (deny_create) {
> > > +		EXPECT_EQ(fd, -EACCES);

The deny_create argument/check should not be in this helper but in the
caller.

> > > +	} else {
> > > +		EXPECT_LE(0, fd)

This should be an ASSERT because the following code using fd would make
no sense if executed.

> > > +		{
> > > +			TH_LOG("Failed to create socket: %s", strerror(errno));
> > > +		}
> > > +		EXPECT_EQ(0, close(fd));
> > > +	}
> > > +}
> > 
> > This is slightly too much logic in a test helper, for my taste,
> > and the meaning of the true/false argument in the main test below
> > is not very obvious.
> > 
> > Extending the idea from above, if test_socket() was simpler, would it
> > be possible to turn these fixtures into something shorter like this:
> > 
> >    ASSERT_EQ(EAFNOSUPPORT, test_socket(AF_UNSPEC, SOCK_STREAM, 0));
> >    ASSERT_EQ(EACCES, test_socket(AF_UNIX, SOCK_STREAM, 0));
> >    ASSERT_EQ(EACCES, test_socket(AF_UNIX, SOCK_DGRAM, 0));
> >    ASSERT_EQ(EACCES, test_socket(AF_INET, SOCK_STREAM, 0));
> >    // etc.

I'd prefer that too.

> > 
> > Would that make the tests easier to write, to list out the table of
> > expected values aspect like that, rather than in a fixture?
> > 
> > 
> 
> Initially, I conceived this function as an entity that allows to
> separate the logic associated with the tested methods or usecases from
> the logic of configuring the state of the Landlock ruleset in the
> sandbox.
> 
> But at the moment, `test_socket_create()` is obviously a wrapper over
> socket(2). So for now it's worth removing unnecessary logic.
> 
> But i don't think it's worth removing the fixtures here:
> 
>   * in my opinion, the design of the fixtures is quite convenient.
>     It allows you to separate the definition of the object under test
>     from the test case. E.g. test protocol.create checks the ability of
>     Landlock to restrict the creation of a socket of a certain type,
>     rather than the ability to restrict creation of UNIX, TCP, UDP...
>     sockets

I'm not sure to understand, but we need to have positive and negative
tests, potentially in separate TEST_F().

> 
>   * with adding more tests, it may be necessary to check all protocols
>     in each of them
> 
> AF_UNSPEC should be removed from fixture variant to separate test,
> though.

Or you can add a new field to mark it as such.

A test should check that AF_UNSPEC cannot be restricted though.

> 
> > > +
> > > +TEST_F(protocol, create)
> > > +{
> > > +	const struct landlock_ruleset_attr ruleset_attr = {
> > > +		.handled_access_socket = LANDLOCK_ACCESS_SOCKET_CREATE,
> > > +	};
> > > +	const struct landlock_socket_attr create_socket_attr = {
> > > +		.allowed_access = LANDLOCK_ACCESS_SOCKET_CREATE,
> > > +		.domain = self->srv0.protocol.domain,
> > > +		.type = self->srv0.protocol.type,
> > > +	};
> > > +
> > > +	int ruleset_fd;
> > > +
> > > +	/* Allowed create */
> > > +	ruleset_fd = landlock_create_ruleset(&ruleset_attr,
> > > +							sizeof(ruleset_attr), 0);
> > > +	ASSERT_LE(0, ruleset_fd);
> > > +
> > > +	ASSERT_EQ(0,
> > > +			landlock_add_rule(ruleset_fd, LANDLOCK_RULE_SOCKET,
> > > +					&create_socket_attr, 0));
> > 
> > The indentation looks wrong?  We run clang-format on Landlock files.
> > 
> >    clang-format -i security/landlock/*.[ch] \
> >    	include/uapi/linux/landlock.h \
> >    	tools/testing/selftests/landlock/*.[ch]
> > 
> 
> Thanks! I'll fix indentation in the patch.

Please fix formatting in all patches.

You should have enough for a second patch series. :)

Re: [RFC PATCH v1 03/10] selftests/landlock: Create 'create' test

[Ivanov Mikhail]

5/8/2024 1:38 PM, Mickaël Salaün wrote:
> On Thu, Apr 11, 2024 at 06:58:34PM +0300, Ivanov Mikhail wrote:
>>
>> 4/8/2024 4:08 PM, Günther Noack wrote:
>>> Hello!
>>>
>>> I am very happy to see this patch set, this is a very valuable feature, IMHO! :)
>>>
>>> Regarding the subject of this patch:
>>>
>>>     [RFC PATCH v1 03/10] selftests/landlock: Create 'create' test
>>>                                                      ^^^^^^
>>>
>>> This was probably meant to say "socket"?
>>
>> I wanted each such patch to have the name of the test that this patch
>> adds (without specifying the fixture, since this is not necessary
>> information, which only complicates the name). I think
>>
>>      [RFC PATCH v1 03/10] selftests/landlock: Add 'create' test
>>                                               ~~~
>> renaming should be fine.
> 
> 
> Maybe something like this?
> "selftests/landlock: Add protocol.create to socket tests"

Ok, let's try this one.

> 
> 
>>
>>>
>>> (In my mind, it is a good call to put the test in a separate file -
>>> the existing test files have grown too large already.)
>>>
>>>
>>> On Mon, Apr 08, 2024 at 05:39:20PM +0800, Ivanov Mikhail wrote:
>>>> Initiate socket_test.c selftests. Add protocol fixture for tests
>>>> with changeable domain/type values. Only most common variants of
>>>> protocols (like ipv4-tcp,ipv6-udp, unix) were added.
>>>> Add simple socket access right checking test.
>>>>
>>>> Signed-off-by: Ivanov Mikhail <ivanov.mikhail1@huawei-partners.com>
>>>> Reviewed-by: Konstantin Meskhidze <konstantin.meskhidze@huawei.com>
>>>> ---
>>>>    .../testing/selftests/landlock/socket_test.c  | 197 ++++++++++++++++++
>>>>    1 file changed, 197 insertions(+)
>>>>    create mode 100644 tools/testing/selftests/landlock/socket_test.c
>>>>
>>>> diff --git a/tools/testing/selftests/landlock/socket_test.c b/tools/testing/selftests/landlock/socket_test.c
>>>> new file mode 100644
>>>> index 000000000..525f4f7df
>>>> --- /dev/null
>>>> +++ b/tools/testing/selftests/landlock/socket_test.c
>>>> @@ -0,0 +1,197 @@
>>>> +// SPDX-License-Identifier: GPL-2.0-only
>>>> +/*
>>>> + * Landlock tests - Socket
>>>> + *
>>>> + * Copyright © 2024 Huawei Tech. Co., Ltd.
>>>> + */
>>>> +
>>>> +#define _GNU_SOURCE
>>>> +
>>>> +#include <errno.h>
>>>> +#include <linux/landlock.h>
>>>> +#include <sched.h>
>>>> +#include <string.h>
>>>> +#include <sys/prctl.h>
>>>> +#include <sys/socket.h>
>>>> +
>>>> +#include "common.h"
>>>> +
>>>> +/* clang-format off */
>>>> +
>>>> +#define ACCESS_LAST LANDLOCK_ACCESS_SOCKET_CREATE
>>>> +
>>>> +#define ACCESS_ALL ( \
>>>> +	LANDLOCK_ACCESS_SOCKET_CREATE)
>>>> +
>>>> +/* clang-format on */
>>>> +
>>>> +struct protocol_variant {
>>>> +	int domain;
>>>> +	int type;
>>>> +};
>>>> +
>>>> +struct service_fixture {
>>>> +	struct protocol_variant protocol;
>>>> +};
>>>> +
>>>> +static void setup_namespace(struct __test_metadata *const _metadata)
>>>> +{
>>>> +	set_cap(_metadata, CAP_SYS_ADMIN);
>>>> +	ASSERT_EQ(0, unshare(CLONE_NEWNET));
>>>> +	clear_cap(_metadata, CAP_SYS_ADMIN);
>>>> +}
>>>> +
>>>> +static int socket_variant(const struct service_fixture *const srv)
>>>> +{
>>>> +	int ret;
>>>> +
>>>> +	ret = socket(srv->protocol.domain, srv->protocol.type | SOCK_CLOEXEC,
>>>> +			 0);
>>>> +	if (ret < 0)
>>>> +		return -errno;
>>>> +	return ret;
>>>> +}
>>>
>>> This helper is mostly concerned with mapping the error code.
>>>
>>> In the fs_test.c, we have dealt with such use cases with helpers like
>>> test_open_rel() and test_open().  These helpers attempt to open the file, take
>>> the same arguments as open(2), but instead of returning the opened fd, they only
>>> return 0 or errno.  Do you think this would be an option here?
>>>
>>> Then you could write your tests as
>>>
>>>     ASSERT_EQ(EACCES, test_socket(p->domain, p->type, 0));
>>>
>>> and the test would (a) more obviously map to socket(2), and (b) keep relevant
>>> information like the expected error code at the top level of the test.
>>>
>>
>> I thought that `socket_variant()` would be suitable for future tests
>> where sockets can be used after creation (e.g. for sending FDs). But
>> until then, it's really better to replace it with what you suggested.
> 
> You can move common code/helpers from net_test.c to common.h (with a
> dedicated patch) to avoid duplicating code.

Good idea, thanks!

> 
>>
>>>> +
>>>> +FIXTURE(protocol)
>>>> +{
>>>> +	struct service_fixture srv0;
>>>> +};
>>>> +
>>>> +FIXTURE_VARIANT(protocol)
>>>> +{
>>>> +	const struct protocol_variant protocol;
>>>> +};
>>>> +
>>>> +FIXTURE_SETUP(protocol)
>>>> +{
>>>> +	disable_caps(_metadata);
>>>> +	self->srv0.protocol = variant->protocol;
>>>> +	setup_namespace(_metadata);
>>>> +};
>>>> +
>>>> +FIXTURE_TEARDOWN(protocol)
>>>> +{
>>>> +}
>>>> +
>>>> +/* clang-format off */
>>>> +FIXTURE_VARIANT_ADD(protocol, unspec) {
>>>> +	/* clang-format on */
>>>> +	.protocol = {
>>>> +		.domain = AF_UNSPEC,
>>>> +		.type = SOCK_STREAM,
>>>> +	},
>>>> +};
>>>> +
>>>> +/* clang-format off */
>>>> +FIXTURE_VARIANT_ADD(protocol, unix_stream) {
>>>> +	/* clang-format on */
>>>> +	.protocol = {
>>>> +		.domain = AF_UNIX,
>>>> +		.type = SOCK_STREAM,
>>>> +	},
>>>> +};
>>>> +
>>>> +/* clang-format off */
>>>> +FIXTURE_VARIANT_ADD(protocol, unix_dgram) {
>>>> +	/* clang-format on */
>>>> +	.protocol = {
>>>> +		.domain = AF_UNIX,
>>>> +		.type = SOCK_DGRAM,
>>>> +	},
>>>> +};
>>>> +
>>>> +/* clang-format off */
>>>> +FIXTURE_VARIANT_ADD(protocol, ipv4_tcp) {
>>>> +	/* clang-format on */
>>>> +	.protocol = {
>>>> +		.domain = AF_INET,
>>>> +		.type = SOCK_STREAM,
>>>> +	},
>>>> +};
>>>> +
>>>> +/* clang-format off */
>>>> +FIXTURE_VARIANT_ADD(protocol, ipv4_udp) {
>>>> +	/* clang-format on */
>>>> +	.protocol = {
>>>> +		.domain = AF_INET,
>>>> +		.type = SOCK_DGRAM,
>>>> +	},
>>>> +};
>>>> +
>>>> +/* clang-format off */
>>>> +FIXTURE_VARIANT_ADD(protocol, ipv6_tcp) {
>>>> +	/* clang-format on */
>>>> +	.protocol = {
>>>> +		.domain = AF_INET6,
>>>> +		.type = SOCK_STREAM,
>>>> +	},
>>>> +};
>>>> +
>>>> +/* clang-format off */
>>>> +FIXTURE_VARIANT_ADD(protocol, ipv6_udp) {
>>>> +	/* clang-format on */
>>>> +	.protocol = {
>>>> +		.domain = AF_INET6,
>>>> +		.type = SOCK_DGRAM,
>>>> +	},
>>>> +};
>>>> +
>>>> +static void test_socket_create(struct __test_metadata *const _metadata,
>>>> +				  const struct service_fixture *const srv,
>>>> +				  const bool deny_create)
>>>> +{
>>>> +	int fd;
>>>> +
>>>> +	fd = socket_variant(srv);
>>>> +	if (srv->protocol.domain == AF_UNSPEC) {
>>>> +		EXPECT_EQ(fd, -EAFNOSUPPORT);
>>>> +	} else if (deny_create) {
>>>> +		EXPECT_EQ(fd, -EACCES);
> 
> The deny_create argument/check should not be in this helper but in the
> caller.

got it

> 
>>>> +	} else {
>>>> +		EXPECT_LE(0, fd)
> 
> This should be an ASSERT because the following code using fd would make
> no sense if executed.

got it

> 
>>>> +		{
>>>> +			TH_LOG("Failed to create socket: %s", strerror(errno));
>>>> +		}
>>>> +		EXPECT_EQ(0, close(fd));
>>>> +	}
>>>> +}
>>>
>>> This is slightly too much logic in a test helper, for my taste,
>>> and the meaning of the true/false argument in the main test below
>>> is not very obvious.
>>>
>>> Extending the idea from above, if test_socket() was simpler, would it
>>> be possible to turn these fixtures into something shorter like this:
>>>
>>>     ASSERT_EQ(EAFNOSUPPORT, test_socket(AF_UNSPEC, SOCK_STREAM, 0));
>>>     ASSERT_EQ(EACCES, test_socket(AF_UNIX, SOCK_STREAM, 0));
>>>     ASSERT_EQ(EACCES, test_socket(AF_UNIX, SOCK_DGRAM, 0));
>>>     ASSERT_EQ(EACCES, test_socket(AF_INET, SOCK_STREAM, 0));
>>>     // etc.
> 
> I'd prefer that too.
> 
>>>
>>> Would that make the tests easier to write, to list out the table of
>>> expected values aspect like that, rather than in a fixture?
>>>
>>>
>>
>> Initially, I conceived this function as an entity that allows to
>> separate the logic associated with the tested methods or usecases from
>> the logic of configuring the state of the Landlock ruleset in the
>> sandbox.
>>
>> But at the moment, `test_socket_create()` is obviously a wrapper over
>> socket(2). So for now it's worth removing unnecessary logic.
>>
>> But i don't think it's worth removing the fixtures here:
>>
>>    * in my opinion, the design of the fixtures is quite convenient.
>>      It allows you to separate the definition of the object under test
>>      from the test case. E.g. test protocol.create checks the ability of
>>      Landlock to restrict the creation of a socket of a certain type,
>>      rather than the ability to restrict creation of UNIX, TCP, UDP...
>>      sockets
> 
> I'm not sure to understand, but we need to have positive and negative
> tests, potentially in separate TEST_F().

I mean we can use fixtures in order to not add ASSERT_EQ for
each protocol, as suggested by Günther. It's gonna look like this:

      ASSERT_EQ(EAFNOSUPPORT, test_socket(&self->unspec_srv0));
      ASSERT_EQ(EACCES, test_socket(&self->srv0));

I think it would make the test easier to read, don't you think so?

> 
>>
>>    * with adding more tests, it may be necessary to check all protocols
>>      in each of them
>>
>> AF_UNSPEC should be removed from fixture variant to separate test,
>> though.
> 
> Or you can add a new field to mark it as such.
> 
> A test should check that AF_UNSPEC cannot be restricted though.

yeah, thanks

> 
>>
>>>> +
>>>> +TEST_F(protocol, create)
>>>> +{
>>>> +	const struct landlock_ruleset_attr ruleset_attr = {
>>>> +		.handled_access_socket = LANDLOCK_ACCESS_SOCKET_CREATE,
>>>> +	};
>>>> +	const struct landlock_socket_attr create_socket_attr = {
>>>> +		.allowed_access = LANDLOCK_ACCESS_SOCKET_CREATE,
>>>> +		.domain = self->srv0.protocol.domain,
>>>> +		.type = self->srv0.protocol.type,
>>>> +	};
>>>> +
>>>> +	int ruleset_fd;
>>>> +
>>>> +	/* Allowed create */
>>>> +	ruleset_fd = landlock_create_ruleset(&ruleset_attr,
>>>> +							sizeof(ruleset_attr), 0);
>>>> +	ASSERT_LE(0, ruleset_fd);
>>>> +
>>>> +	ASSERT_EQ(0,
>>>> +			landlock_add_rule(ruleset_fd, LANDLOCK_RULE_SOCKET,
>>>> +					&create_socket_attr, 0));
>>>
>>> The indentation looks wrong?  We run clang-format on Landlock files.
>>>
>>>     clang-format -i security/landlock/*.[ch] \
>>>     	include/uapi/linux/landlock.h \
>>>     	tools/testing/selftests/landlock/*.[ch]
>>>
>>
>> Thanks! I'll fix indentation in the patch.
> 
> Please fix formatting in all patches.
> 
> You should have enough for a second patch series. :)

Yeah, thank you!)

Re: [RFC PATCH v1 03/10] selftests/landlock: Create 'create' test

[Mickaël Salaün]

On Thu, May 16, 2024 at 04:54:58PM +0300, Ivanov Mikhail wrote:
> 
> 
> 5/8/2024 1:38 PM, Mickaël Salaün wrote:
> > On Thu, Apr 11, 2024 at 06:58:34PM +0300, Ivanov Mikhail wrote:
> > > 
> > > 4/8/2024 4:08 PM, Günther Noack wrote:


> > > > > +		{
> > > > > +			TH_LOG("Failed to create socket: %s", strerror(errno));
> > > > > +		}
> > > > > +		EXPECT_EQ(0, close(fd));
> > > > > +	}
> > > > > +}
> > > > 
> > > > This is slightly too much logic in a test helper, for my taste,
> > > > and the meaning of the true/false argument in the main test below
> > > > is not very obvious.
> > > > 
> > > > Extending the idea from above, if test_socket() was simpler, would it
> > > > be possible to turn these fixtures into something shorter like this:
> > > > 
> > > >     ASSERT_EQ(EAFNOSUPPORT, test_socket(AF_UNSPEC, SOCK_STREAM, 0));
> > > >     ASSERT_EQ(EACCES, test_socket(AF_UNIX, SOCK_STREAM, 0));
> > > >     ASSERT_EQ(EACCES, test_socket(AF_UNIX, SOCK_DGRAM, 0));
> > > >     ASSERT_EQ(EACCES, test_socket(AF_INET, SOCK_STREAM, 0));
> > > >     // etc.
> > 
> > I'd prefer that too.
> > 
> > > > 
> > > > Would that make the tests easier to write, to list out the table of
> > > > expected values aspect like that, rather than in a fixture?
> > > > 
> > > > 
> > > 
> > > Initially, I conceived this function as an entity that allows to
> > > separate the logic associated with the tested methods or usecases from
> > > the logic of configuring the state of the Landlock ruleset in the
> > > sandbox.
> > > 
> > > But at the moment, `test_socket_create()` is obviously a wrapper over
> > > socket(2). So for now it's worth removing unnecessary logic.
> > > 
> > > But i don't think it's worth removing the fixtures here:
> > > 
> > >    * in my opinion, the design of the fixtures is quite convenient.
> > >      It allows you to separate the definition of the object under test
> > >      from the test case. E.g. test protocol.create checks the ability of
> > >      Landlock to restrict the creation of a socket of a certain type,
> > >      rather than the ability to restrict creation of UNIX, TCP, UDP...
> > >      sockets
> > 
> > I'm not sure to understand, but we need to have positive and negative
> > tests, potentially in separate TEST_F().
> 
> I mean we can use fixtures in order to not add ASSERT_EQ for
> each protocol, as suggested by Günther. It's gonna look like this:
> 
>      ASSERT_EQ(EAFNOSUPPORT, test_socket(&self->unspec_srv0));
>      ASSERT_EQ(EACCES, test_socket(&self->srv0));
> 
> I think it would make the test easier to read, don't you think so?

Yes, this looks good.

[RFC PATCH v1 04/10] selftests/landlock: Create 'socket_access_rights' test

[Ivanov Mikhail]

Add test that checks possibility of adding rule with every possible
access right.

Signed-off-by: Ivanov Mikhail <ivanov.mikhail1@huawei-partners.com>
Reviewed-by: Konstantin Meskhidze <konstantin.meskhidze@huawei.com>
---
 .../testing/selftests/landlock/socket_test.c  | 29 +++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/tools/testing/selftests/landlock/socket_test.c b/tools/testing/selftests/landlock/socket_test.c
index 525f4f7df..7f31594bf 100644
--- a/tools/testing/selftests/landlock/socket_test.c
+++ b/tools/testing/selftests/landlock/socket_test.c
@@ -194,4 +194,33 @@ TEST_F(protocol, create)
 	test_socket_create(_metadata, &self->srv0, true);
 }
 
+TEST_F(protocol, socket_access_rights)
+{
+	const struct landlock_ruleset_attr ruleset_attr = {
+		.handled_access_socket = ACCESS_ALL,
+	};
+	struct landlock_socket_attr protocol = {
+		.domain = self->srv0.protocol.domain,
+		.type = self->srv0.protocol.type,
+	};
+	int ruleset_fd;
+	__u64 access;
+
+	ruleset_fd =
+		landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
+	ASSERT_LE(0, ruleset_fd);
+
+	for (access = 1; access <= ACCESS_LAST; access <<= 1) {
+		protocol.allowed_access = access;
+		EXPECT_EQ(0,
+			  landlock_add_rule(ruleset_fd, LANDLOCK_RULE_SOCKET,
+					    &protocol, 0))
+		{
+			TH_LOG("Failed to add rule with access 0x%llx: %s",
+			       access, strerror(errno));
+		}
+	}
+	EXPECT_EQ(0, close(ruleset_fd));
+}
+
 TEST_HARNESS_MAIN
-- 
2.34.1

[RFC PATCH v1 05/10] selftests/landlock: Create 'rule_with_unknown_access' test

[Ivanov Mikhail]

Add test that validates behavior of landlock after rule with
unknown access is added.

Signed-off-by: Ivanov Mikhail <ivanov.mikhail1@huawei-partners.com>
Reviewed-by: Konstantin Meskhidze <konstantin.meskhidze@huawei.com>
---
 .../testing/selftests/landlock/socket_test.c  | 26 +++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/tools/testing/selftests/landlock/socket_test.c b/tools/testing/selftests/landlock/socket_test.c
index 7f31594bf..5577b08d5 100644
--- a/tools/testing/selftests/landlock/socket_test.c
+++ b/tools/testing/selftests/landlock/socket_test.c
@@ -223,4 +223,30 @@ TEST_F(protocol, socket_access_rights)
 	EXPECT_EQ(0, close(ruleset_fd));
 }
 
+TEST_F(protocol, rule_with_unknown_access)
+{
+	const struct landlock_ruleset_attr ruleset_attr = {
+		.handled_access_net = ACCESS_ALL,
+	};
+	struct landlock_socket_attr protocol = {
+		.domain = self->srv0.protocol.domain,
+		.type = self->srv0.protocol.type,
+	};
+	int ruleset_fd;
+	__u64 access;
+
+	ruleset_fd =
+		landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
+	ASSERT_LE(0, ruleset_fd);
+
+	for (access = 1ULL << 63; access != ACCESS_LAST; access >>= 1) {
+		protocol.allowed_access = access;
+		EXPECT_EQ(-1,
+			  landlock_add_rule(ruleset_fd, LANDLOCK_RULE_SOCKET,
+					    &protocol, 0));
+		EXPECT_EQ(EINVAL, errno);
+	}
+	EXPECT_EQ(0, close(ruleset_fd));
+}
+
 TEST_HARNESS_MAIN
-- 
2.34.1

[RFC PATCH v1 06/10] selftests/landlock: Create 'rule_with_unhandled_access' test

[Ivanov Mikhail]

Add test that validates behavior of landlock after rule with
unhandled access is added.

Signed-off-by: Ivanov Mikhail <ivanov.mikhail1@huawei-partners.com>
Reviewed-by: Konstantin Meskhidze <konstantin.meskhidze@huawei.com>
---
 .../testing/selftests/landlock/socket_test.c  | 33 +++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/tools/testing/selftests/landlock/socket_test.c b/tools/testing/selftests/landlock/socket_test.c
index 5577b08d5..c1c2b5d30 100644
--- a/tools/testing/selftests/landlock/socket_test.c
+++ b/tools/testing/selftests/landlock/socket_test.c
@@ -249,4 +249,37 @@ TEST_F(protocol, rule_with_unknown_access)
 	EXPECT_EQ(0, close(ruleset_fd));
 }
 
+TEST_F(protocol, rule_with_unhandled_access)
+{
+	struct landlock_ruleset_attr ruleset_attr = {
+		.handled_access_socket = LANDLOCK_ACCESS_SOCKET_CREATE,
+	};
+	struct landlock_socket_attr protocol = {
+		.domain = self->srv0.protocol.domain,
+		.type = self->srv0.protocol.type,
+	};
+	int ruleset_fd;
+	__u64 access;
+
+	ruleset_fd =
+		landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
+	ASSERT_LE(0, ruleset_fd);
+
+	for (access = 1; access > 0; access <<= 1) {
+		int err;
+
+		protocol.allowed_access = access;
+		err = landlock_add_rule(ruleset_fd, LANDLOCK_RULE_SOCKET,
+					&protocol, 0);
+		if (access == ruleset_attr.handled_access_socket) {
+			EXPECT_EQ(0, err);
+		} else {
+			EXPECT_EQ(-1, err);
+			EXPECT_EQ(EINVAL, errno);
+		}
+	}
+
+	EXPECT_EQ(0, close(ruleset_fd));
+}
+
 TEST_HARNESS_MAIN
-- 
2.34.1

[RFC PATCH v1 07/10] selftests/landlock: Create 'inval' test

[Ivanov Mikhail]

Add test that validates behavior of landlock with fully
access restriction.

Signed-off-by: Ivanov Mikhail <ivanov.mikhail1@huawei-partners.com>
Reviewed-by: Konstantin Meskhidze <konstantin.meskhidze@huawei.com>
---
 .../testing/selftests/landlock/socket_test.c  | 34 +++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/tools/testing/selftests/landlock/socket_test.c b/tools/testing/selftests/landlock/socket_test.c
index c1c2b5d30..dd105489d 100644
--- a/tools/testing/selftests/landlock/socket_test.c
+++ b/tools/testing/selftests/landlock/socket_test.c
@@ -282,4 +282,38 @@ TEST_F(protocol, rule_with_unhandled_access)
 	EXPECT_EQ(0, close(ruleset_fd));
 }
 
+TEST_F(protocol, inval)
+{
+	const struct landlock_ruleset_attr ruleset_attr = {
+		.handled_access_socket = LANDLOCK_ACCESS_SOCKET_CREATE
+	};
+
+	struct landlock_socket_attr protocol = {
+		.allowed_access = LANDLOCK_ACCESS_SOCKET_CREATE,
+		.domain = self->srv0.protocol.domain,
+		.type = self->srv0.protocol.type,
+	};
+
+	struct landlock_socket_attr protocol_denied = {
+		.allowed_access = 0,
+		.domain = self->srv0.protocol.domain,
+		.type = self->srv0.protocol.type,
+	};
+
+	int ruleset_fd;
+
+	ruleset_fd =
+		landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
+	ASSERT_LE(0, ruleset_fd);
+
+	/* Checks zero access value. */
+	EXPECT_EQ(-1, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_SOCKET,
+					&protocol_denied, 0));
+	EXPECT_EQ(ENOMSG, errno);
+
+	/* Adds with legitimate values. */
+	ASSERT_EQ(0, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_SOCKET,
+				       &protocol, 0));
+}
+
 TEST_HARNESS_MAIN
-- 
2.34.1

[RFC PATCH v1 08/10] selftests/landlock: Create 'ruleset_overlap' test

[Ivanov Mikhail]

Add tcp_layers fixture for tests that check multiple layer configuration
scenarios.
Add test that validates multiple layer behavior with overlapped
restrictions.

Signed-off-by: Ivanov Mikhail <ivanov.mikhail1@huawei-partners.com>
Reviewed-by: Konstantin Meskhidze <konstantin.meskhidze@huawei.com>
---
 .../testing/selftests/landlock/socket_test.c  | 107 ++++++++++++++++++
 1 file changed, 107 insertions(+)

diff --git a/tools/testing/selftests/landlock/socket_test.c b/tools/testing/selftests/landlock/socket_test.c
index dd105489d..07f73618d 100644
--- a/tools/testing/selftests/landlock/socket_test.c
+++ b/tools/testing/selftests/landlock/socket_test.c
@@ -316,4 +316,111 @@ TEST_F(protocol, inval)
 				       &protocol, 0));
 }
 
+FIXTURE(tcp_layers)
+{
+	struct service_fixture srv0;
+};
+
+FIXTURE_VARIANT(tcp_layers)
+{
+	const size_t num_layers;
+};
+
+FIXTURE_SETUP(tcp_layers)
+{
+	const struct protocol_variant prot = {
+		.domain = AF_INET,
+		.type = SOCK_STREAM,
+	};
+
+	disable_caps(_metadata);
+	self->srv0.protocol = prot;
+	setup_namespace(_metadata);
+};
+
+FIXTURE_TEARDOWN(tcp_layers)
+{
+}
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(tcp_layers, no_sandbox_with_ipv4) {
+	/* clang-format on */
+	.num_layers = 0,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(tcp_layers, one_sandbox_with_ipv4) {
+	/* clang-format on */
+	.num_layers = 1,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(tcp_layers, two_sandboxes_with_ipv4) {
+	/* clang-format on */
+	.num_layers = 2,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(tcp_layers, three_sandboxes_with_ipv4) {
+	/* clang-format on */
+	.num_layers = 3,
+};
+
+TEST_F(tcp_layers, ruleset_overlap)
+{
+	const struct landlock_ruleset_attr ruleset_attr = {
+		.handled_access_socket = LANDLOCK_ACCESS_SOCKET_CREATE,
+	};
+	const struct landlock_socket_attr tcp_create = {
+		.allowed_access = LANDLOCK_ACCESS_SOCKET_CREATE,
+		.domain = self->srv0.protocol.domain,
+		.type = self->srv0.protocol.type,
+	};
+
+	if (variant->num_layers >= 1) {
+		int ruleset_fd;
+
+		ruleset_fd = landlock_create_ruleset(&ruleset_attr,
+						     sizeof(ruleset_attr), 0);
+		ASSERT_LE(0, ruleset_fd);
+
+		/* Allows create. */
+		ASSERT_EQ(0,
+			  landlock_add_rule(ruleset_fd, LANDLOCK_RULE_SOCKET,
+					    &tcp_create, 0));
+		enforce_ruleset(_metadata, ruleset_fd);
+		EXPECT_EQ(0, close(ruleset_fd));
+	}
+
+	if (variant->num_layers >= 2) {
+		int ruleset_fd;
+
+		/* Creates another ruleset layer with denied create. */
+		ruleset_fd = landlock_create_ruleset(&ruleset_attr,
+						     sizeof(ruleset_attr), 0);
+		ASSERT_LE(0, ruleset_fd);
+
+		enforce_ruleset(_metadata, ruleset_fd);
+		EXPECT_EQ(0, close(ruleset_fd));
+	}
+
+	if (variant->num_layers >= 3) {
+		int ruleset_fd;
+
+		/* Creates another ruleset layer. */
+		ruleset_fd = landlock_create_ruleset(&ruleset_attr,
+						     sizeof(ruleset_attr), 0);
+		ASSERT_LE(0, ruleset_fd);
+
+		/* Try to allow create second time. */
+		ASSERT_EQ(0,
+			  landlock_add_rule(ruleset_fd, LANDLOCK_RULE_SOCKET,
+					    &tcp_create, 0));
+		enforce_ruleset(_metadata, ruleset_fd);
+		EXPECT_EQ(0, close(ruleset_fd));
+	}
+
+	test_socket_create(_metadata, &self->srv0, variant->num_layers >= 2);
+}
+
 TEST_HARNESS_MAIN
-- 
2.34.1

[RFC PATCH v1 09/10] selftests/landlock: Create 'ruleset_with_unknown_access' test

[Ivanov Mikhail]

Add fixture mini for tests that check specific cases.
Add test that validates behavior of landlock after ruleset with
unknown access is created.

Signed-off-by: Ivanov Mikhail <ivanov.mikhail1@huawei-partners.com>
Reviewed-by: Konstantin Meskhidze <konstantin.meskhidze@huawei.com>
---
 .../testing/selftests/landlock/socket_test.c  | 31 +++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/tools/testing/selftests/landlock/socket_test.c b/tools/testing/selftests/landlock/socket_test.c
index 07f73618d..afc57556d 100644
--- a/tools/testing/selftests/landlock/socket_test.c
+++ b/tools/testing/selftests/landlock/socket_test.c
@@ -423,4 +423,35 @@ TEST_F(tcp_layers, ruleset_overlap)
 	test_socket_create(_metadata, &self->srv0, variant->num_layers >= 2);
 }
 
+/* clang-format off */
+FIXTURE(mini) {};
+/* clang-format on */
+
+FIXTURE_SETUP(mini)
+{
+	disable_caps(_metadata);
+
+	setup_namespace(_metadata);
+};
+
+FIXTURE_TEARDOWN(mini)
+{
+}
+
+TEST_F(mini, ruleset_with_unknown_access)
+{
+	__u64 access_mask;
+
+	for (access_mask = 1ULL << 63; access_mask != ACCESS_LAST;
+	     access_mask >>= 1) {
+		const struct landlock_ruleset_attr ruleset_attr = {
+			.handled_access_socket = access_mask,
+		};
+
+		EXPECT_EQ(-1, landlock_create_ruleset(&ruleset_attr,
+						      sizeof(ruleset_attr), 0));
+		EXPECT_EQ(EINVAL, errno);
+	}
+}
+
 TEST_HARNESS_MAIN
-- 
2.34.1

[RFC PATCH v1 10/10] samples/landlock: Support socket protocol restrictions

[Ivanov Mikhail]

Add socket protocol control support in sandboxer demo. It's possible
to allow a sandboxer to create sockets with specified family(domain)
and type values. This is controlled with the new LL_SOCKET_CREATE
environment variable. Single token in this variable looks like this:
'FAMILY.TYPE', where FAMILY corresponds to one of the possible socket
family name and TYPE to the possible socket type name (see socket(2)).
Add ENV_TOKEN_INTERNAL_DELIMITER.

Add get_socket_protocol() method to parse socket family and type strings
to the appropriate constants. Add CHECK_DOMAIN() and CHECK_TYPE()
macroses to prevent copypaste.

Signed-off-by: Ivanov Mikhail <ivanov.mikhail1@huawei-partners.com>
Reviewed-by: Konstantin Meskhidze <konstantin.meskhidze@huawei.com>
---
 samples/landlock/sandboxer.c | 149 ++++++++++++++++++++++++++++++++---
 1 file changed, 136 insertions(+), 13 deletions(-)

diff --git a/samples/landlock/sandboxer.c b/samples/landlock/sandboxer.c
index 32e930c85..4642a7437 100644
--- a/samples/landlock/sandboxer.c
+++ b/samples/landlock/sandboxer.c
@@ -14,6 +14,7 @@
 #include <fcntl.h>
 #include <linux/landlock.h>
 #include <linux/prctl.h>
+#include <linux/socket.h>
 #include <stddef.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -55,8 +56,11 @@ static inline int landlock_restrict_self(const int ruleset_fd,
 #define ENV_FS_RW_NAME "LL_FS_RW"
 #define ENV_TCP_BIND_NAME "LL_TCP_BIND"
 #define ENV_TCP_CONNECT_NAME "LL_TCP_CONNECT"
+#define ENV_SOCKET_CREATE_NAME "LL_SOCKET_CREATE"
 #define ENV_DELIMITER ":"
 
+#define ENV_TOKEN_INTERNAL_DELIMITER "."
+
 static int parse_path(char *env_path, const char ***const path_list)
 {
 	int i, num_paths = 0;
@@ -85,6 +89,49 @@ static int parse_path(char *env_path, const char ***const path_list)
 
 /* clang-format on */
 
+#define CHECK_DOMAIN(domain_variant) \
+	do { \
+		if (strcmp(strdomain, #domain_variant) == 0) { \
+			protocol->domain = domain_variant; \
+			domain_parsed = 1; \
+			goto domain_check; \
+		} \
+	} while (0)
+
+#define CHECK_TYPE(type_variant) \
+	do { \
+		if (strcmp(strtype, #type_variant) == 0) { \
+			protocol->type = type_variant; \
+			type_parsed = 1; \
+			goto type_check; \
+		} \
+	} while (0)
+
+static int get_socket_protocol(char *strdomain, char *strtype,
+				struct landlock_socket_attr *protocol)
+{
+	int domain_parsed = 0, type_parsed = 0;
+
+	CHECK_DOMAIN(AF_UNIX);
+	CHECK_DOMAIN(AF_INET);
+	CHECK_DOMAIN(AF_INET6);
+
+domain_check:
+	if (!domain_parsed)
+		return 1;
+
+	CHECK_TYPE(SOCK_STREAM);
+	CHECK_TYPE(SOCK_DGRAM);
+
+type_check:
+	if (!type_parsed)
+		return 1;
+	return 0;
+}
+
+#undef CHECK_DOMAIN
+#undef CHECK_TYPE
+
 static int populate_ruleset_fs(const char *const env_var, const int ruleset_fd,
 			       const __u64 allowed_access)
 {
@@ -182,6 +229,58 @@ static int populate_ruleset_net(const char *const env_var, const int ruleset_fd,
 	return ret;
 }
 
+static int populate_ruleset_socket(const char *const env_var,
+				const int ruleset_fd, const __u64 allowed_access)
+{
+	int ret = 1;
+	char *env_protocol_name, *env_protocol_name_next;
+	char *strprotocol, *strdomain, *strtype;
+	struct landlock_socket_attr protocol = {
+		.allowed_access = allowed_access,
+		.domain = 0,
+		.type = 0,
+	};
+
+	env_protocol_name = getenv(env_var);
+	if (!env_protocol_name)
+		return 0;
+	env_protocol_name = strdup(env_protocol_name);
+	unsetenv(env_var);
+
+	env_protocol_name_next = env_protocol_name;
+	while ((strprotocol = strsep(&env_protocol_name_next, ENV_DELIMITER))) {
+		strdomain = strsep(&strprotocol, ENV_TOKEN_INTERNAL_DELIMITER);
+		strtype   = strsep(&strprotocol, ENV_TOKEN_INTERNAL_DELIMITER);
+
+		if (!strtype) {
+			fprintf(stderr, "Failed to extract socket protocol with "
+							"unspecified type value\n");
+			goto out_free_name;
+		}
+
+		if (get_socket_protocol(strdomain, strtype, &protocol)) {
+			fprintf(stderr, "Failed to extract socket protocol with "
+							"domain: \"%s\", type: \"%s\"\n",
+				strdomain, strtype);
+			goto out_free_name;
+		}
+
+		if (landlock_add_rule(ruleset_fd, LANDLOCK_RULE_SOCKET,
+				      &protocol, 0)) {
+			fprintf(stderr,
+				"Failed to update the ruleset with "
+				"domain \"%s\" and type \"%s\": %s\n",
+				strdomain, strtype, strerror(errno));
+			goto out_free_name;
+		}
+	}
+	ret = 0;
+
+out_free_name:
+	free(env_protocol_name);
+	return ret;
+}
+
 /* clang-format off */
 
 #define ACCESS_FS_ROUGHLY_READ ( \
@@ -205,14 +304,14 @@ static int populate_ruleset_net(const char *const env_var, const int ruleset_fd,
 
 /* clang-format on */
 
-#define LANDLOCK_ABI_LAST 4
+#define LANDLOCK_ABI_LAST 5
 
 int main(const int argc, char *const argv[], char *const *const envp)
 {
 	const char *cmd_path;
 	char *const *cmd_argv;
 	int ruleset_fd, abi;
-	char *env_port_name;
+	char *env_optional_name;
 	__u64 access_fs_ro = ACCESS_FS_ROUGHLY_READ,
 	      access_fs_rw = ACCESS_FS_ROUGHLY_READ | ACCESS_FS_ROUGHLY_WRITE;
 
@@ -220,18 +319,19 @@ int main(const int argc, char *const argv[], char *const *const envp)
 		.handled_access_fs = access_fs_rw,
 		.handled_access_net = LANDLOCK_ACCESS_NET_BIND_TCP |
 				      LANDLOCK_ACCESS_NET_CONNECT_TCP,
+		.handled_access_socket = LANDLOCK_ACCESS_SOCKET_CREATE,
 	};
 
 	if (argc < 2) {
 		fprintf(stderr,
-			"usage: %s=\"...\" %s=\"...\" %s=\"...\" %s=\"...\"%s "
+			"usage: %s=\"...\" %s=\"...\" %s=\"...\" %s=\"...\" %s=\"...\"%s "
 			"<cmd> [args]...\n\n",
 			ENV_FS_RO_NAME, ENV_FS_RW_NAME, ENV_TCP_BIND_NAME,
-			ENV_TCP_CONNECT_NAME, argv[0]);
+			ENV_TCP_CONNECT_NAME, ENV_SOCKET_CREATE_NAME, argv[0]);
 		fprintf(stderr,
 			"Execute a command in a restricted environment.\n\n");
 		fprintf(stderr,
-			"Environment variables containing paths and ports "
+			"Environment variables containing paths, ports and protocols "
 			"each separated by a colon:\n");
 		fprintf(stderr,
 			"* %s: list of paths allowed to be used in a read-only way.\n",
@@ -240,7 +340,7 @@ int main(const int argc, char *const argv[], char *const *const envp)
 			"* %s: list of paths allowed to be used in a read-write way.\n\n",
 			ENV_FS_RW_NAME);
 		fprintf(stderr,
-			"Environment variables containing ports are optional "
+			"Environment variables containing ports or protocols are optional "
 			"and could be skipped.\n");
 		fprintf(stderr,
 			"* %s: list of ports allowed to bind (server).\n",
@@ -248,22 +348,25 @@ int main(const int argc, char *const argv[], char *const *const envp)
 		fprintf(stderr,
 			"* %s: list of ports allowed to connect (client).\n",
 			ENV_TCP_CONNECT_NAME);
+		fprintf(stderr,
+			"* %s: list of socket protocols allowed to be created.\n",
+			ENV_SOCKET_CREATE_NAME);
 		fprintf(stderr,
 			"\nexample:\n"
 			"%s=\"${PATH}:/lib:/usr:/proc:/etc:/dev/urandom\" "
 			"%s=\"/dev/null:/dev/full:/dev/zero:/dev/pts:/tmp\" "
 			"%s=\"9418\" "
 			"%s=\"80:443\" "
+			"%s=\"AF_INET6.SOCK_STREAM:AF_UNIX.SOCK_STREAM\" "
 			"%s bash -i\n\n",
 			ENV_FS_RO_NAME, ENV_FS_RW_NAME, ENV_TCP_BIND_NAME,
-			ENV_TCP_CONNECT_NAME, argv[0]);
+			ENV_TCP_CONNECT_NAME, ENV_SOCKET_CREATE_NAME, argv[0]);
 		fprintf(stderr,
 			"This sandboxer can use Landlock features "
 			"up to ABI version %d.\n",
 			LANDLOCK_ABI_LAST);
 		return 1;
 	}
-
 	abi = landlock_create_ruleset(NULL, 0, LANDLOCK_CREATE_RULESET_VERSION);
 	if (abi < 0) {
 		const int err = errno;
@@ -325,6 +428,16 @@ int main(const int argc, char *const argv[], char *const *const envp)
 			"provided by ABI version %d (instead of %d).\n",
 			LANDLOCK_ABI_LAST, abi);
 		__attribute__((fallthrough));
+	case 4:
+		/* Removes socket support for ABI < 5 */
+		ruleset_attr.handled_access_socket &=
+			~LANDLOCK_ACCESS_SOCKET_CREATE;
+		fprintf(stderr,
+			"Hint: You should update the running kernel "
+			"to leverage Landlock features "
+			"provided by ABI version %d (instead of %d).\n",
+			LANDLOCK_ABI_LAST, abi);
+		__attribute__((fallthrough));
 	case LANDLOCK_ABI_LAST:
 		break;
 	default:
@@ -338,18 +451,23 @@ int main(const int argc, char *const argv[], char *const *const envp)
 	access_fs_rw &= ruleset_attr.handled_access_fs;
 
 	/* Removes bind access attribute if not supported by a user. */
-	env_port_name = getenv(ENV_TCP_BIND_NAME);
-	if (!env_port_name) {
+	env_optional_name = getenv(ENV_TCP_BIND_NAME);
+	if (!env_optional_name) {
 		ruleset_attr.handled_access_net &=
 			~LANDLOCK_ACCESS_NET_BIND_TCP;
 	}
 	/* Removes connect access attribute if not supported by a user. */
-	env_port_name = getenv(ENV_TCP_CONNECT_NAME);
-	if (!env_port_name) {
+	env_optional_name = getenv(ENV_TCP_CONNECT_NAME);
+	if (!env_optional_name) {
 		ruleset_attr.handled_access_net &=
 			~LANDLOCK_ACCESS_NET_CONNECT_TCP;
 	}
-
+	/* Removes socket create access attribute if not supported by a user. */
+	env_optional_name = getenv(ENV_SOCKET_CREATE_NAME);
+	if (!env_optional_name) {
+		ruleset_attr.handled_access_socket &=
+			~LANDLOCK_ACCESS_SOCKET_CREATE;
+	}
 	ruleset_fd =
 		landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
 	if (ruleset_fd < 0) {
@@ -373,6 +491,11 @@ int main(const int argc, char *const argv[], char *const *const envp)
 		goto err_close_ruleset;
 	}
 
+	if (populate_ruleset_socket(ENV_SOCKET_CREATE_NAME, ruleset_fd,
+				 LANDLOCK_ACCESS_SOCKET_CREATE)) {
+		goto err_close_ruleset;
+	}
+
 	if (prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0)) {
 		perror("Failed to restrict privileges");
 		goto err_close_ruleset;
-- 
2.34.1

Re: [RFC PATCH v1 00/10] Socket type control for Landlock

[Günther Noack]

On Mon, Apr 08, 2024 at 05:39:17PM +0800, Ivanov Mikhail wrote:
> Patchset implements new type of Landlock rule, that restricts actions for
> sockets of any protocol. Such restriction would be useful to ensure
> that a sandboxed process uses only necessary protocols.
> See [2] for more cases.
> 
> The rules store information about the socket family(aka domain) and type.
> 
> struct landlock_socket_attr {
> 	__u64 allowed_access;
> 	int domain; // see socket(2)
> 	int type; // see socket(2)
> }
> 
> Patchset currently implements rule only for socket_create() method, but
> other necessary rules will also be impemented. [1]
> 
> Code coverage(gcov) report with the launch of all the landlock selftests:
> * security/landlock:
> lines......: 94.7% (784 of 828 lines)
> functions..: 97.2% (105 of 108 functions)
> 
> * security/landlock/socket.c:
> lines......: 100.0% (33 of 33 lines)
> functions..: 100.0% (5 of 5 functions)
> 
> [1] https://lore.kernel.org/all/b8a2045a-e7e8-d141-7c01-bf47874c7930@digikod.net/
> [2] https://lore.kernel.org/all/ZJvy2SViorgc+cZI@google.com/

Thank you, I am very excited to see this patch set! :)

You might want to also link to https://github.com/landlock-lsm/linux/issues/6
where the feature idea is tracked.

—Günther