[PATCH v7 0/4] Landlock: Abstract Unix Socket Scoping Support

[PATCH v7 0/4] Landlock: Abstract Unix Socket Scoping Support

[Tahera Fahimi]

This patch series adds scoping mechanism for abstract unix sockets.
Closes: https://github.com/landlock-lsm/linux/issues/7

Problem
=======

Abstract unix sockets are used for local inter-process communications
independent of the filesystem. Currently, a sandboxed process can
connect to a socket outside of the sandboxed environment, since Landlock
has no restriction for connecting to an abstract socket address(see more
details in [1,2]). Access to such sockets for a sandboxed process should
be scoped the same way ptrace is limited.

[1] https://lore.kernel.org/all/20231023.ahphah4Wii4v@digikod.net/
[2] https://lore.kernel.org/all/20231102.MaeWaepav8nu@digikod.net/

Solution
========

To solve this issue, we extend the user space interface by adding a new
"scoped" field to Landlock ruleset attribute structure. This field can
contains different rights to restrict different functionalities. For
abstract unix sockets, we introduce
"LANDLOCK_SCOPED_ABSTRACT_UNIX_SOCKET" field to specify that a ruleset
will deny any connection from within the sandbox domain to its parent
(i.e. any parent sandbox or non-sandbox processes).

Example
=======

Starting a listening socket with socat(1):
	socat abstract-listen:mysocket -

Starting a sandboxed shell from $HOME with samples/landlock/sandboxer:
	LL_FS_RO=/ LL_FS_RW=. LL_SCOPED="a" ./sandboxer /bin/bash

If we try to connect to the listening socket, the connection would be
refused.
	socat - abstract-connect:mysocket --> fails


Notes of Implementation
=======================

* Using the "scoped" field provides enough compatibility and flexibility
  to extend the scoping mechanism for other IPCs(e.g. signals).

* To access the domain of a socket, we use its credentials of the file's FD
  which point to the credentials of the process that created the socket.
  (see more details in [3]). Cases where the process using the socket has
  a different domain than the process created it are covered in the 
  unix_sock_special_cases test.

[3] https://lore.kernel.org/outreachy/Zmi8Ydz4Z6tYtpY1@tahera-OptiPlex-5000/T/#m8cdf33180d86c7ec22932e2eb4ef7dd4fc94c792

Thanks to Mickaël Salaün and Paul Moore for guiding me through this
implementation.

Previous Versions
=================
v6: https://lore.kernel.org/all/Zn32CYZiu7pY+rdI@tahera-OptiPlex-5000/
and https://lore.kernel.org/all/Zn32KKIJrY7Zi51K@tahera-OptiPlex-5000/
v5: https://lore.kernel.org/all/ZnSZnhGBiprI6FRk@tahera-OptiPlex-5000/
v4: https://lore.kernel.org/all/ZnNcE3ph2SWi1qmd@tahera-OptiPlex-5000/
v3: https://lore.kernel.org/all/ZmJJ7lZdQuQop7e5@tahera-OptiPlex-5000/
v2: https://lore.kernel.org/all/ZgX5TRTrSDPrJFfF@tahera-OptiPlex-5000/
v1: https://lore.kernel.org/all/ZgXN5fi6A1YQKiAQ@tahera-OptiPlex-5000/


Tahera Fahimi (4):
  Landlock: Add abstract unix socket connect restriction
  selftests/landlock: Abstract unix socket restriction tests
  samples/landlock: Support abstract unix socket restriction
  documentation/landlock: Adding scoping mechanism documentation

 Documentation/userspace-api/landlock.rst      |  23 +-
 include/uapi/linux/landlock.h                 |  29 +
 samples/landlock/sandboxer.c                  |  25 +-
 security/landlock/limits.h                    |   3 +
 security/landlock/ruleset.c                   |   7 +-
 security/landlock/ruleset.h                   |  23 +-
 security/landlock/syscalls.c                  |  14 +-
 security/landlock/task.c                      | 112 +++
 tools/testing/selftests/landlock/base_test.c  |   2 +-
 .../testing/selftests/landlock/ptrace_test.c  | 867 ++++++++++++++++++
 10 files changed, 1088 insertions(+), 17 deletions(-)

-- 
2.34.1

[PATCH v7 1/4] Landlock: Add abstract unix socket connect restriction

[Tahera Fahimi]

The patch introduces a new "scoped" attribute to the
landlock_ruleset_attr that can specify "LANDLOCK_SCOPED_ABSTRACT_UNIX_SOCKET"
to scope abstract unix sockets from connecting to a process outside of
the same landlock domain.

This patch implement two hooks, "unix_stream_connect" and "unix_may_send" to
enforce this restriction.

Signed-off-by: Tahera Fahimi <fahimitahera@gmail.com>

-------
v7:
 - Using socket's file credentials for both connected(STREAM) and
   non-connected(DGRAM) sockets.
 - Adding "domain_sock_scope" instead of the domain scoping mechanism used in
   ptrace ensures that if a server's domain is accessible from the client's
   domain (where the client is more privileged than the server), the client
   can connect to the server in all edge cases.
 - Removing debug codes.
v6:
 - Removing curr_ruleset from landlock_hierarchy, and switching back to use
   the same domain scoping as ptrace.
 - code clean up.
v5:
 - Renaming "LANDLOCK_*_ACCESS_SCOPE" to "LANDLOCK_*_SCOPE"
 - Adding curr_ruleset to hierarachy_ruleset structure to have access from
   landlock_hierarchy to its respective landlock_ruleset.
 - Using curr_ruleset to check if a domain is scoped while walking in the
   hierarchy of domains.
 - Modifying inline comments.
V4:
 - Rebased on Günther's Patch:
   https://lore.kernel.org/all/20240610082115.1693267-1-gnoack@google.com/
   so there is no need for "LANDLOCK_SHIFT_ACCESS_SCOPE", then it is removed.
 - Adding get_scope_accesses function to check all scoped access masks in a ruleset.
 - Using file's FD credentials instead of credentials stored in peer_cred
   for datagram sockets. (see discussion in [1])
 - Modifying inline comments.
V3:
 - Improving commit description.
 - Introducing "scoped" attribute to landlock_ruleset_attr for IPC scoping
   purpose, and adding related functions.
 - Changing structure of ruleset based on "scoped".
 - Removing rcu lock and using unix_sk lock instead.
 - Introducing scoping for datagram sockets in unix_may_send.
V2:
 - Removing wrapper functions

[1]https://lore.kernel.org/outreachy/Zmi8Ydz4Z6tYtpY1@tahera-OptiPlex-5000/T/#m8cdf33180d86c7ec22932e2eb4ef7dd4fc94c792
-------

Signed-off-by: Tahera Fahimi <fahimitahera@gmail.com>
---
 include/uapi/linux/landlock.h |  29 +++++++++
 security/landlock/limits.h    |   3 +
 security/landlock/ruleset.c   |   7 ++-
 security/landlock/ruleset.h   |  23 ++++++-
 security/landlock/syscalls.c  |  14 +++--
 security/landlock/task.c      | 112 ++++++++++++++++++++++++++++++++++
 6 files changed, 181 insertions(+), 7 deletions(-)

diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
index 68625e728f43..9cd881673434 100644
--- a/include/uapi/linux/landlock.h
+++ b/include/uapi/linux/landlock.h
@@ -37,6 +37,12 @@ struct landlock_ruleset_attr {
 	 * rule explicitly allow them.
 	 */
 	__u64 handled_access_net;
+	/**
+	 * @scoped: Bitmask of scopes (cf. `Scope flags`_)
+	 * restricting a Landlock domain from accessing outside
+	 * resources(e.g. IPCs).
+	 */
+	__u64 scoped;
 };
 
 /*
@@ -266,4 +272,27 @@ 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: scope
+ *
+ * .scoped attribute handles a set of restrictions on kernel IPCs through
+ * the following flags.
+ *
+ * Scope flags
+ * ~~~~~~~~~~~
+ *
+ * These flags enable to restrict a sandboxed process from a set of IPC
+ * actions. Setting a flag for a ruleset will isolate the Landlock domain
+ * to forbid connections to resources outside the domain.
+ *
+ * IPCs with scoped actions:
+ * - %LANDLOCK_SCOPED_ABSTRACT_UNIX_SOCKET: Restrict a sandboxed process
+ *   from connecting to an abstract unix socket created by a process
+ *   outside the related Landlock domain (e.g. a parent domain or a
+ *   non-sandboxed process).
+ */
+/* clang-format off */
+#define LANDLOCK_SCOPED_ABSTRACT_UNIX_SOCKET		(1ULL << 0)
+/* clang-format on*/
 #endif /* _UAPI_LINUX_LANDLOCK_H */
diff --git a/security/landlock/limits.h b/security/landlock/limits.h
index 4eb643077a2a..eb01d0fb2165 100644
--- a/security/landlock/limits.h
+++ b/security/landlock/limits.h
@@ -26,6 +26,9 @@
 #define LANDLOCK_MASK_ACCESS_NET	((LANDLOCK_LAST_ACCESS_NET << 1) - 1)
 #define LANDLOCK_NUM_ACCESS_NET		__const_hweight64(LANDLOCK_MASK_ACCESS_NET)
 
+#define LANDLOCK_LAST_SCOPE		LANDLOCK_SCOPED_ABSTRACT_UNIX_SOCKET
+#define LANDLOCK_MASK_SCOPE		((LANDLOCK_LAST_SCOPE << 1) - 1)
+#define LANDLOCK_NUM_SCOPE		__const_hweight64(LANDLOCK_MASK_SCOPE)
 /* clang-format on */
 
 #endif /* _SECURITY_LANDLOCK_LIMITS_H */
diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c
index 6ff232f58618..a93bdbf52fff 100644
--- a/security/landlock/ruleset.c
+++ b/security/landlock/ruleset.c
@@ -52,12 +52,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 scope_mask)
 {
 	struct landlock_ruleset *new_ruleset;
 
 	/* Informs about useless ruleset. */
-	if (!fs_access_mask && !net_access_mask)
+	if (!fs_access_mask && !net_access_mask && !scope_mask)
 		return ERR_PTR(-ENOMSG);
 	new_ruleset = create_ruleset(1);
 	if (IS_ERR(new_ruleset))
@@ -66,6 +67,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 (scope_mask)
+		landlock_add_scope_mask(new_ruleset, scope_mask, 0);
 	return new_ruleset;
 }
 
diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h
index 0f1b5b4c8f6b..c749fa0b3ecd 100644
--- a/security/landlock/ruleset.h
+++ b/security/landlock/ruleset.h
@@ -35,6 +35,8 @@ typedef u16 access_mask_t;
 static_assert(BITS_PER_TYPE(access_mask_t) >= LANDLOCK_NUM_ACCESS_FS);
 /* Makes sure all network access rights can be stored. */
 static_assert(BITS_PER_TYPE(access_mask_t) >= LANDLOCK_NUM_ACCESS_NET);
+/* Makes sure all scoped rights can be stored*/
+static_assert(BITS_PER_TYPE(access_mask_t) >= LANDLOCK_NUM_SCOPE);
 /* Makes sure for_each_set_bit() and for_each_clear_bit() calls are OK. */
 static_assert(sizeof(unsigned long) >= sizeof(access_mask_t));
 
@@ -42,6 +44,7 @@ static_assert(sizeof(unsigned long) >= sizeof(access_mask_t));
 struct access_masks {
 	access_mask_t fs : LANDLOCK_NUM_ACCESS_FS;
 	access_mask_t net : LANDLOCK_NUM_ACCESS_NET;
+	access_mask_t scoped : LANDLOCK_NUM_SCOPE;
 };
 
 typedef u16 layer_mask_t;
@@ -233,7 +236,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 scope_mask);
 
 void landlock_put_ruleset(struct landlock_ruleset *const ruleset);
 void landlock_put_ruleset_deferred(struct landlock_ruleset *const ruleset);
@@ -280,6 +284,16 @@ landlock_add_net_access_mask(struct landlock_ruleset *const ruleset,
 	ruleset->access_masks[layer_level].net |= net_mask;
 }
 
+static inline void
+landlock_add_scope_mask(struct landlock_ruleset *const ruleset,
+			const access_mask_t scope_mask, const u16 layer_level)
+{
+	access_mask_t scoped_mask = scope_mask & LANDLOCK_MASK_SCOPE;
+
+	WARN_ON_ONCE(scope_mask != scoped_mask);
+	ruleset->access_masks[layer_level].scoped |= scoped_mask;
+}
+
 static inline access_mask_t
 landlock_get_raw_fs_access_mask(const struct landlock_ruleset *const ruleset,
 				const u16 layer_level)
@@ -303,6 +317,13 @@ landlock_get_net_access_mask(const struct landlock_ruleset *const ruleset,
 	return ruleset->access_masks[layer_level].net;
 }
 
+static inline access_mask_t
+landlock_get_scope_mask(const struct landlock_ruleset *const ruleset,
+			const u16 layer_level)
+{
+	return ruleset->access_masks[layer_level].scoped;
+}
+
 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/syscalls.c b/security/landlock/syscalls.c
index 03b470f5a85a..799a50f11d79 100644
--- a/security/landlock/syscalls.c
+++ b/security/landlock/syscalls.c
@@ -97,8 +97,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.scoped);
 	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);
@@ -149,7 +150,7 @@ static const struct file_operations ruleset_fops = {
 	.write = fop_dummy_write,
 };
 
-#define LANDLOCK_ABI_VERSION 5
+#define LANDLOCK_ABI_VERSION 6
 
 /**
  * sys_landlock_create_ruleset - Create a new ruleset
@@ -170,7 +171,7 @@ static const struct file_operations ruleset_fops = {
  * Possible returned errors are:
  *
  * - %EOPNOTSUPP: Landlock is supported by the kernel but disabled at boot time;
- * - %EINVAL: unknown @flags, or unknown access, or too small @size;
+ * - %EINVAL: unknown @flags, or unknown access, or uknown scope, or too small @size;
  * - %E2BIG or %EFAULT: @attr or @size inconsistencies;
  * - %ENOMSG: empty &landlock_ruleset_attr.handled_access_fs.
  */
@@ -213,9 +214,14 @@ SYSCALL_DEFINE3(landlock_create_ruleset,
 	    LANDLOCK_MASK_ACCESS_NET)
 		return -EINVAL;
 
+	/* Checks IPC scoping content (and 32-bits cast). */
+	if ((ruleset_attr.scoped | LANDLOCK_MASK_SCOPE) != LANDLOCK_MASK_SCOPE)
+		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.scoped);
 	if (IS_ERR(ruleset))
 		return PTR_ERR(ruleset);
 
diff --git a/security/landlock/task.c b/security/landlock/task.c
index 849f5123610b..597d89e54aae 100644
--- a/security/landlock/task.c
+++ b/security/landlock/task.c
@@ -13,6 +13,8 @@
 #include <linux/lsm_hooks.h>
 #include <linux/rcupdate.h>
 #include <linux/sched.h>
+#include <net/sock.h>
+#include <net/af_unix.h>
 
 #include "common.h"
 #include "cred.h"
@@ -108,9 +110,119 @@ static int hook_ptrace_traceme(struct task_struct *const parent)
 	return task_ptrace(parent, current);
 }
 
+static int walk_and_check(const struct landlock_ruleset *const child,
+			  struct landlock_hierarchy **walker, int i, int j,
+			  bool check)
+{
+	if (!child || i < 0)
+		return -1;
+
+	while (i < j && *walker) {
+		if (check && landlock_get_scope_mask(child, j))
+			return -1;
+		*walker = (*walker)->parent;
+		j--;
+	}
+	if (!*walker)
+		pr_warn_once("inconsistency in landlock hierarchy and layers");
+	return j;
+}
+
+/**
+ * domain_sock_scope - Checks if client domain is scoped in the same
+ *			domain as server.
+ *
+ * @client: Connecting socket domain.
+ * @server: Listening socket domain.
+ *
+ * Checks if the @client domain is scoped, then the server should be
+ * in the same domain to connect. If not, @client can connect to @server.
+ */
+static bool domain_sock_scope(const struct landlock_ruleset *const client,
+			      const struct landlock_ruleset *const server)
+{
+	size_t l1, l2;
+	int scope_layer;
+	struct landlock_hierarchy *cli_walker, *srv_walker;
+
+	if (!client)
+		return true;
+
+	l1 = client->num_layers - 1;
+	cli_walker = client->hierarchy;
+	if (server) {
+		l2 = server->num_layers - 1;
+		srv_walker = server->hierarchy;
+	} else
+		l2 = 0;
+
+	if (l1 > l2)
+		scope_layer = walk_and_check(client, &cli_walker, l2, l1, true);
+	else if (l2 > l1)
+		scope_layer =
+			walk_and_check(server, &srv_walker, l1, l2, false);
+	else
+		scope_layer = l1;
+
+	if (scope_layer == -1)
+		return false;
+
+	while (scope_layer >= 0 && cli_walker) {
+		if (landlock_get_scope_mask(client, scope_layer) &
+		    LANDLOCK_SCOPED_ABSTRACT_UNIX_SOCKET) {
+			if (!server)
+				return false;
+
+			if (srv_walker == cli_walker)
+				return true;
+
+			return false;
+		}
+		cli_walker = cli_walker->parent;
+		srv_walker = srv_walker->parent;
+		scope_layer--;
+	}
+	return true;
+}
+
+static bool sock_is_scoped(struct sock *const other)
+{
+	const struct landlock_ruleset *dom_other;
+	const struct landlock_ruleset *const dom =
+		landlock_get_current_domain();
+
+	/* the credentials will not change */
+	lockdep_assert_held(&unix_sk(other)->lock);
+	dom_other = landlock_cred(other->sk_socket->file->f_cred)->domain;
+
+	/* other is scoped, they connect if they are in the same domain */
+	return domain_sock_scope(dom, dom_other);
+}
+
+static int hook_unix_stream_connect(struct sock *const sock,
+				    struct sock *const other,
+				    struct sock *const newsk)
+{
+	if (sock_is_scoped(other))
+		return 0;
+
+	return -EPERM;
+}
+
+static int hook_unix_may_send(struct socket *const sock,
+			      struct socket *const other)
+{
+	if (sock_is_scoped(other->sk))
+		return 0;
+
+	return -EPERM;
+}
+
 static struct security_hook_list landlock_hooks[] __ro_after_init = {
 	LSM_HOOK_INIT(ptrace_access_check, hook_ptrace_access_check),
 	LSM_HOOK_INIT(ptrace_traceme, hook_ptrace_traceme),
+	LSM_HOOK_INIT(unix_stream_connect, hook_unix_stream_connect),
+	LSM_HOOK_INIT(unix_may_send, hook_unix_may_send),
 };
 
 __init void landlock_add_task_hooks(void)
-- 
2.34.1

[PATCH v7 2/4] selftests/landlock: Abstract unix socket restriction tests

[Tahera Fahimi]

The patch has three types of tests:
  1) unix_socket: base tests the scoping mechanism for a landlocked process,
     same as the ptrace test.
  2) optional_scoping: generates three processes with different domains and
     tests if a process with a non-scoped domain can connect to other processes.
  3) unix_sock_special_cases: since the socket's creator credentials are used
     for scoping datagram sockets, this test examines the cases where the
     socket's credentials are different from the process using it.

Signed-off-by: Tahera Fahimi <fahimitahera@gmail.com>

Changes in versions:
V7:
 - Introducing landlock ABI version 6.
 - Adding some edge test cases to optional_scoping test.
 - Using `enum` for different domains in optional_scoping tests.
 - Extend unix_sock_special_cases test cases for connected(SOCK_STREAM) sockets.
 - Modifying inline comments.
V6:
 - Introducing optional_scoping test which ensures a sandboxed process with a
   non-scoped domain can still connect to another abstract unix socket(either
   sandboxed or non-sandboxed).
 - Introducing unix_sock_special_cases test which tests examines scenarios where
   the connecting sockets have different domain than the process using them.
V4:
 - Introducing unix_socket to evaluate the basic scoping mechanism for abstract
   unix sockets.

Signed-off-by: Tahera Fahimi <fahimitahera@gmail.com>
---
 tools/testing/selftests/landlock/base_test.c  |   2 +-
 .../testing/selftests/landlock/ptrace_test.c  | 867 ++++++++++++++++++
 2 files changed, 868 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/landlock/base_test.c b/tools/testing/selftests/landlock/base_test.c
index 3c1e9f35b531..52b00472a487 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(5, landlock_create_ruleset(NULL, 0,
+	ASSERT_EQ(6, landlock_create_ruleset(NULL, 0,
 					     LANDLOCK_CREATE_RULESET_VERSION));
 
 	ASSERT_EQ(-1, landlock_create_ruleset(&ruleset_attr, 0,
diff --git a/tools/testing/selftests/landlock/ptrace_test.c b/tools/testing/selftests/landlock/ptrace_test.c
index a19db4d0b3bd..e7dcefda8ce0 100644
--- a/tools/testing/selftests/landlock/ptrace_test.c
+++ b/tools/testing/selftests/landlock/ptrace_test.c
@@ -17,6 +17,10 @@
 #include <sys/wait.h>
 #include <unistd.h>
 
+#include <sys/socket.h>
+#include <sys/un.h>
+#include <stddef.h>
+
 #include "common.h"
 
 /* Copied from security/yama/yama_lsm.c */
@@ -436,4 +440,867 @@ TEST_F(hierarchy, trace)
 		_metadata->exit_code = KSFT_FAIL;
 }
 
+static void create_unix_domain(struct __test_metadata *const _metadata)
+{
+	int ruleset_fd;
+	const struct landlock_ruleset_attr ruleset_attr = {
+		.scoped = LANDLOCK_SCOPED_ABSTRACT_UNIX_SOCKET,
+	};
+
+	ruleset_fd =
+		landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
+	EXPECT_LE(0, ruleset_fd)
+	{
+		TH_LOG("Failed to create a ruleset: %s", strerror(errno));
+	}
+	enforce_ruleset(_metadata, ruleset_fd);
+	EXPECT_EQ(0, close(ruleset_fd));
+}
+
+/* clang-format off */
+FIXTURE(unix_socket)
+{
+	int server, client, dgram_server, dgram_client;
+};
+
+/* clang-format on */
+FIXTURE_VARIANT(unix_socket)
+{
+	bool domain_both;
+	bool domain_parent;
+	bool domain_child;
+	bool connect_to_parent;
+};
+
+/*
+ *        No domain
+ *
+ *   P1-.               P1 -> P2 : allow
+ *       \              P2 -> P1 : allow
+ *        'P2
+ */
+/* clang-format off */
+FIXTURE_VARIANT_ADD(unix_socket, allow_without_domain_connect_to_parent) {
+	/* clang-format on */
+	.domain_both = false,
+	.domain_parent = false,
+	.domain_child = false,
+	.connect_to_parent = true,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(unix_socket, allow_without_domain_connect_to_child) {
+	/* clang-format on */
+	.domain_both = false,
+	.domain_parent = false,
+	.domain_child = false,
+	.connect_to_parent = false,
+};
+
+/*
+ *        Child domain
+ *
+ *   P1--.              P1 -> P2 : allow
+ *        \             P2 -> P1 : deny
+ *        .'-----.
+ *        |  P2  |
+ *        '------'
+ */
+/* clang-format off */
+FIXTURE_VARIANT_ADD(unix_socket, deny_with_one_domain_connect_to_parent) {
+	/* clang-format on */
+	.domain_both = false,
+	.domain_parent = false,
+	.domain_child = true,
+	.connect_to_parent = true,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(unix_socket, allow_with_one_domain_connect_to_child) {
+	/* clang-format on */
+	.domain_both = false,
+	.domain_parent = false,
+	.domain_child = true,
+	.connect_to_parent = false,
+};
+
+/*
+ *        Parent domain
+ * .------.
+ * |  P1  --.           P1 -> P2 : deny
+ * '------'  \          P2 -> P1 : allow
+ *            '
+ *            P2
+ */
+/* clang-format off */
+FIXTURE_VARIANT_ADD(unix_socket, allow_with_parent_domain_connect_to_parent) {
+	/* clang-format on */
+	.domain_both = false,
+	.domain_parent = true,
+	.domain_child = false,
+	.connect_to_parent = true,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(unix_socket, deny_with_parent_domain_connect_to_child) {
+	/* clang-format on */
+	.domain_both = false,
+	.domain_parent = true,
+	.domain_child = false,
+	.connect_to_parent = false,
+};
+
+/*
+ *        Parent + child domain (siblings)
+ * .------.
+ * |  P1  ---.          P1 -> P2 : deny
+ * '------'   \         P2 -> P1 : deny
+ *         .---'--.
+ *         |  P2  |
+ *         '------'
+ */
+/* clang-format off */
+FIXTURE_VARIANT_ADD(unix_socket, deny_with_sibling_domain_connect_to_parent) {
+	/* clang-format on */
+	.domain_both = false,
+	.domain_parent = true,
+	.domain_child = true,
+	.connect_to_parent = true,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(unix_socket, deny_with_sibling_domain_connect_to_child) {
+	/* clang-format on */
+	.domain_both = false,
+	.domain_parent = true,
+	.domain_child = true,
+	.connect_to_parent = false,
+};
+
+/*
+ *         Same domain (inherited)
+ * .-------------.
+ * | P1----.     |      P1 -> P2 : allow
+ * |        \    |      P2 -> P1 : allow
+ * |         '   |
+ * |         P2  |
+ * '-------------'
+ */
+/* clang-format off */
+FIXTURE_VARIANT_ADD(unix_socket, allow_inherited_domain_connect_to_parent) {
+	/* clang-format on */
+	.domain_both = true,
+	.domain_parent = false,
+	.domain_child = false,
+	.connect_to_parent = true,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(unix_socket, allow_inherited_domain_connect_to_child) {
+	/* clang-format on */
+	.domain_both = true,
+	.domain_parent = false,
+	.domain_child = false,
+	.connect_to_parent = false,
+};
+
+/*
+ *         Inherited + child domain
+ * .-----------------.
+ * |  P1----.        |  P1 -> P2 : allow
+ * |         \       |  P2 -> P1 : deny
+ * |        .-'----. |
+ * |        |  P2  | |
+ * |        '------' |
+ * '-----------------'
+ */
+/* clang-format off */
+FIXTURE_VARIANT_ADD(unix_socket, deny_nested_domain_connect_to_parent) {
+	/* clang-format on */
+	.domain_both = true,
+	.domain_parent = false,
+	.domain_child = true,
+	.connect_to_parent = true,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(unix_socket, allow_nested_domain_connect_to_child) {
+	/* clang-format on */
+	.domain_both = true,
+	.domain_parent = false,
+	.domain_child = true,
+	.connect_to_parent = false,
+};
+
+/*
+ *         Inherited + parent domain
+ * .-----------------.
+ * |.------.         |  P1 -> P2 : deny
+ * ||  P1  ----.     |  P2 -> P1 : allow
+ * |'------'    \    |
+ * |             '   |
+ * |             P2  |
+ * '-----------------'
+ */
+/* clang-format off */
+FIXTURE_VARIANT_ADD(unix_socket, allow_with_nested_and_parent_domain_connect_to_parent) {
+	/* clang-format on */
+	.domain_both = true,
+	.domain_parent = true,
+	.domain_child = false,
+	.connect_to_parent = true,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(unix_socket, deny_with_nested_and_parent_domain_connect_to_child) {
+	/* clang-format on */
+	.domain_both = true,
+	.domain_parent = true,
+	.domain_child = false,
+	.connect_to_parent = false,
+};
+
+/*
+ *         Inherited + parent and child domain (siblings)
+ * .-----------------.
+ * | .------.        |  P1 -> P2 : deny
+ * | |  P1  .        |  P2 -> P1 : deny
+ * | '------'\       |
+ * |          \      |
+ * |        .--'---. |
+ * |        |  P2  | |
+ * |        '------' |
+ * '-----------------'
+ */
+/* clang-format off */
+FIXTURE_VARIANT_ADD(unix_socket, deny_with_forked_domain_connect_to_parent) {
+	/* clang-format on */
+	.domain_both = true,
+	.domain_parent = true,
+	.domain_child = true,
+	.connect_to_parent = true,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(unix_socket, deny_with_forked_domain_connect_to_child) {
+	/* clang-format on */
+	.domain_both = true,
+	.domain_parent = true,
+	.domain_child = true,
+	.connect_to_parent = false,
+};
+
+FIXTURE_SETUP(unix_socket)
+{
+}
+
+FIXTURE_TEARDOWN(unix_socket)
+{
+	close(self->server);
+	close(self->client);
+	close(self->dgram_server);
+	close(self->dgram_client);
+}
+
+/* Test UNIX_STREAM_CONNECT and UNIX_MAY_SEND for parent and child,
+ * when they have scoped domain or no domain.
+ */
+TEST_F(unix_socket, abstract_unix_socket)
+{
+	int status;
+	pid_t child;
+	socklen_t addrlen;
+	int sock_len = 5;
+	struct sockaddr_un addr, dgram_addr = {
+		.sun_family = AF_UNIX,
+	};
+	const char sun_path[8] = "\0test";
+	const char sun_path_dgram[8] = "\0dgrm";
+	bool can_connect_to_parent, can_connect_to_child;
+	int err, err_dgram;
+	int pipe_child[2], pipe_parent[2];
+	char buf_parent;
+
+	/*
+	 * can_connect_to_child is true if a parent process can connect to its
+	 * child process. The parent process is not isolated from the child
+	 * with a dedicated Landlock domain.
+	 */
+	can_connect_to_child = !variant->domain_parent;
+	/*
+	 * can_connect_to_parent is true if a child process can connect to its
+	 * parent process. This depends on the child process is not isolated from
+	 * the parent with a dedicated Landlock domain.
+	 */
+	can_connect_to_parent = !variant->domain_child;
+
+	ASSERT_EQ(0, pipe2(pipe_child, O_CLOEXEC));
+	ASSERT_EQ(0, pipe2(pipe_parent, O_CLOEXEC));
+	if (variant->domain_both) {
+		create_unix_domain(_metadata);
+		if (!__test_passed(_metadata))
+			return;
+	}
+
+	addrlen = offsetof(struct sockaddr_un, sun_path) + sock_len;
+	memcpy(&addr.sun_path, sun_path, sock_len);
+	memcpy(&dgram_addr.sun_path, sun_path_dgram, sock_len);
+
+	child = fork();
+	ASSERT_LE(0, child);
+	if (child == 0) {
+		char buf_child;
+
+		ASSERT_EQ(0, close(pipe_parent[1]));
+		ASSERT_EQ(0, close(pipe_child[0]));
+		if (variant->domain_child)
+			create_unix_domain(_metadata);
+
+		/* Waits for the parent to be in a domain, if any. */
+		ASSERT_EQ(1, read(pipe_parent[0], &buf_child, 1));
+
+		if (variant->connect_to_parent) {
+			self->client = socket(AF_UNIX, SOCK_STREAM, 0);
+			self->dgram_client = socket(AF_UNIX, SOCK_DGRAM, 0);
+
+			ASSERT_NE(-1, self->client);
+			ASSERT_NE(-1, self->dgram_client);
+			ASSERT_EQ(1, read(pipe_parent[0], &buf_child, 1));
+
+			err = connect(self->client, (struct sockaddr *)&addr,
+				      addrlen);
+			err_dgram = connect(self->dgram_client,
+					    (struct sockaddr *)&dgram_addr,
+					    addrlen);
+			if (can_connect_to_parent) {
+				EXPECT_EQ(0, err);
+				EXPECT_EQ(0, err_dgram);
+			} else {
+				EXPECT_EQ(-1, err);
+				EXPECT_EQ(-1, err_dgram);
+				EXPECT_EQ(EPERM, errno);
+			}
+		} else {
+			self->server = socket(AF_UNIX, SOCK_STREAM, 0);
+			self->dgram_server = socket(AF_UNIX, SOCK_DGRAM, 0);
+			ASSERT_NE(-1, self->server);
+			ASSERT_NE(-1, self->dgram_server);
+
+			ASSERT_EQ(0, bind(self->server,
+					  (struct sockaddr *)&addr, addrlen));
+			ASSERT_EQ(0, bind(self->dgram_server,
+					  (struct sockaddr *)&dgram_addr,
+					  addrlen));
+			ASSERT_EQ(0, listen(self->server, 32));
+
+			/* signal to parent that child is listening */
+			ASSERT_EQ(1, write(pipe_child[1], ".", 1));
+			/* wait to connect */
+			ASSERT_EQ(1, read(pipe_parent[0], &buf_child, 1));
+		}
+		_exit(_metadata->exit_code);
+		return;
+	}
+
+	ASSERT_EQ(0, close(pipe_child[1]));
+	ASSERT_EQ(0, close(pipe_parent[0]));
+
+	if (variant->domain_parent)
+		create_unix_domain(_metadata);
+
+	/* Signals that the parent is in a domain, if any. */
+	ASSERT_EQ(1, write(pipe_parent[1], ".", 1));
+
+	if (!variant->connect_to_parent) {
+		self->client = socket(AF_UNIX, SOCK_STREAM, 0);
+		self->dgram_client = socket(AF_UNIX, SOCK_DGRAM, 0);
+
+		ASSERT_NE(-1, self->client);
+		ASSERT_NE(-1, self->dgram_client);
+
+		/* Waits for the child to listen */
+		ASSERT_EQ(1, read(pipe_child[0], &buf_parent, 1));
+		err = connect(self->client, (struct sockaddr *)&addr, addrlen);
+		err_dgram = connect(self->dgram_client,
+				    (struct sockaddr *)&dgram_addr, addrlen);
+
+		if (can_connect_to_child) {
+			EXPECT_EQ(0, err);
+			EXPECT_EQ(0, err_dgram);
+		} else {
+			EXPECT_EQ(-1, err);
+			EXPECT_EQ(-1, err_dgram);
+			EXPECT_EQ(EPERM, errno);
+		}
+		ASSERT_EQ(1, write(pipe_parent[1], ".", 1));
+	} else {
+		self->server = socket(AF_UNIX, SOCK_STREAM, 0);
+		self->dgram_server = socket(AF_UNIX, SOCK_DGRAM, 0);
+		ASSERT_NE(-1, self->server);
+		ASSERT_NE(-1, self->dgram_server);
+		ASSERT_EQ(0, bind(self->server, (struct sockaddr *)&addr,
+				  addrlen));
+		ASSERT_EQ(0, bind(self->dgram_server,
+				  (struct sockaddr *)&dgram_addr, addrlen));
+		ASSERT_EQ(0, listen(self->server, 32));
+
+		/* signal to child that parent is listening */
+		ASSERT_EQ(1, write(pipe_parent[1], ".", 1));
+	}
+
+	ASSERT_EQ(child, waitpid(child, &status, 0));
+
+	if (WIFSIGNALED(status) || !WIFEXITED(status) ||
+	    WEXITSTATUS(status) != EXIT_SUCCESS)
+		_metadata->exit_code = KSFT_FAIL;
+}
+
+enum sandbox_type {
+	NO_SANDBOX,
+	SCOPE_SANDBOX,
+	/* Any other type of sandboxing domain */
+	OTHER_SANDBOX,
+};
+
+/* clang-format off */
+FIXTURE(optional_scoping)
+{
+	int parent_server, child_server, client;
+};
+
+/* clang-format on */
+FIXTURE_VARIANT(optional_scoping)
+{
+	const int domain_all;
+	const int domain_parent;
+	const int domain_children;
+	const int domain_child;
+	const int domain_grand_child;
+	const int type;
+};
+
+/*
+ * .-----------------.
+ * |         ####### |  P3 -> P2 : allow
+ * |   P1----# P2  # |  P3 -> P1 : deny
+ * |         #  |  # |
+ * |         # P3  # |
+ * |         ####### |
+ * '-----------------'
+ */
+/* clang-format off */
+FIXTURE_VARIANT_ADD(optional_scoping, deny_scoped) {
+	.domain_all = OTHER_SANDBOX,
+	.domain_parent = NO_SANDBOX,
+	.domain_children = SCOPE_SANDBOX,
+	.domain_child = NO_SANDBOX,
+	.domain_grand_child = NO_SANDBOX,
+	.type = SOCK_DGRAM,
+	/* clang-format on */
+};
+
+/*
+ * .-----------------.
+ * |         .-----. |  P3 -> P2 : allow
+ * |   P1----| P2  | |  P3 -> P1 : allow
+ * |         |     | |
+ * |         | P3  | |
+ * |         '-----' |
+ * '-----------------'
+ */
+/* clang-format off */
+FIXTURE_VARIANT_ADD(optional_scoping, allow_with_other_domain) {
+	.domain_all = OTHER_SANDBOX,
+	.domain_parent = NO_SANDBOX,
+	.domain_children = OTHER_SANDBOX,
+	.domain_child = NO_SANDBOX,
+	.domain_grand_child = NO_SANDBOX,
+	.type = SOCK_DGRAM,
+	/* clang-format on */
+};
+
+/*
+ *  .----.    ######   P3 -> P2 : allow
+ *  | P1 |----# P2 #   P3 -> P1 : allow
+ *  '----'    ######
+ *              |
+ *              P3
+ */
+/* clang-format off */
+FIXTURE_VARIANT_ADD(optional_scoping, allow_with_one_domain) {
+        .domain_all = NO_SANDBOX,
+        .domain_parent = OTHER_SANDBOX,
+        .domain_children = NO_SANDBOX,
+        .domain_child = SCOPE_SANDBOX,
+        .domain_grand_child = NO_SANDBOX,
+        .type = SOCK_DGRAM,
+	/* clang-format on */
+};
+
+/*
+ *  ######    .-----.   P3 -> P2 : allow
+ *  # P1 #----| P2  |   P3 -> P1 : allow
+ *  ######    '-----'
+ *              |
+ *              P3    
+ */
+/* clang-format off */
+FIXTURE_VARIANT_ADD(optional_scoping, allow_with_grand_parent_scoped) {
+        .domain_all = NO_SANDBOX,
+        .domain_parent = SCOPE_SANDBOX,
+        .domain_children = NO_SANDBOX,
+        .domain_child = OTHER_SANDBOX,
+        .domain_grand_child = NO_SANDBOX,
+        .type = SOCK_STREAM,
+	/* clang-format on */
+};
+
+/*
+ *  ######    ######   P3 -> P2 : allow
+ *  # P1 #----# P2 #   P3 -> P1 : allow
+ *  ######    ######
+ *               |
+ *             .----.
+ *             | P3 |
+ *             '----'  
+ */
+/* clang-format off */
+FIXTURE_VARIANT_ADD(optional_scoping, allow_with_parents_domain) {
+        .domain_all = NO_SANDBOX,
+        .domain_parent = SCOPE_SANDBOX,
+        .domain_children = NO_SANDBOX,
+        .domain_child = SCOPE_SANDBOX,
+        .domain_grand_child = NO_SANDBOX,
+        .type = SOCK_STREAM,
+	/* clang-format on */
+};
+
+FIXTURE_SETUP(optional_scoping)
+{
+}
+
+FIXTURE_TEARDOWN(optional_scoping)
+{
+	close(self->parent_server);
+	close(self->child_server);
+	close(self->client);
+}
+
+/* Test UNIX_STREAM_CONNECT and UNIX_MAY_SEND for parent, child
+ * and grand child processes when they can have scoped or non-scoped
+ * domains.
+ **/
+TEST_F(optional_scoping, unix_scoping)
+{
+	pid_t child;
+	socklen_t addrlen;
+	int sock_len = 5;
+	int status;
+	struct sockaddr_un addr = {
+		.sun_family = AF_UNIX,
+	};
+	const char sun_path[8] = "\0test";
+	bool can_connect_to_parent, can_connect_to_child;
+	int pipe_parent[2];
+
+	if (variant->domain_grand_child == SCOPE_SANDBOX)
+		can_connect_to_child = false;
+	else
+		can_connect_to_child = true;
+
+	if (!can_connect_to_child || variant->domain_children == SCOPE_SANDBOX)
+		can_connect_to_parent = false;
+	else
+		can_connect_to_parent = true;
+
+	addrlen = offsetof(struct sockaddr_un, sun_path) + sock_len;
+	memcpy(&addr.sun_path, sun_path, sock_len);
+
+	ASSERT_EQ(0, pipe2(pipe_parent, O_CLOEXEC));
+
+	if (variant->domain_all == OTHER_SANDBOX)
+		create_domain(_metadata);
+	else if (variant->domain_all == SCOPE_SANDBOX)
+		create_unix_domain(_metadata);
+
+	child = fork();
+	ASSERT_LE(0, child);
+	if (child == 0) {
+		int pipe_child[2];
+		ASSERT_EQ(0, pipe2(pipe_child, O_CLOEXEC));
+		pid_t grand_child;
+		struct sockaddr_un child_addr = {
+			.sun_family = AF_UNIX,
+		};
+		const char child_sun_path[8] = "\0tsst";
+
+		memcpy(&child_addr.sun_path, child_sun_path, sock_len);
+
+		if (variant->domain_children == OTHER_SANDBOX)
+			create_domain(_metadata);
+		else if (variant->domain_children == SCOPE_SANDBOX)
+			create_unix_domain(_metadata);
+
+		grand_child = fork();
+		ASSERT_LE(0, grand_child);
+		if (grand_child == 0) {
+			ASSERT_EQ(0, close(pipe_parent[1]));
+			ASSERT_EQ(0, close(pipe_child[1]));
+
+			char buf1, buf2;
+			int err;
+
+			if (variant->domain_grand_child == OTHER_SANDBOX)
+				create_domain(_metadata);
+			else if (variant->domain_grand_child == SCOPE_SANDBOX)
+				create_unix_domain(_metadata);
+
+			self->client = socket(AF_UNIX, variant->type, 0);
+			ASSERT_NE(-1, self->client);
+
+			ASSERT_EQ(1, read(pipe_child[0], &buf2, 1));
+			err = connect(self->client,
+				      (struct sockaddr *)&child_addr, addrlen);
+			if (can_connect_to_child) {
+				EXPECT_EQ(0, err);
+			} else {
+				EXPECT_EQ(-1, err);
+				EXPECT_EQ(EPERM, errno);
+			}
+
+			if (variant->type == SOCK_STREAM) {
+				EXPECT_EQ(0, close(self->client));
+				self->client =
+					socket(AF_UNIX, variant->type, 0);
+				ASSERT_NE(-1, self->client);
+			}
+
+			ASSERT_EQ(1, read(pipe_parent[0], &buf1, 1));
+			err = connect(self->client, (struct sockaddr *)&addr,
+				      addrlen);
+			if (can_connect_to_parent) {
+				EXPECT_EQ(0, err);
+			} else {
+				EXPECT_EQ(-1, err);
+				EXPECT_EQ(EPERM, errno);
+			}
+			EXPECT_EQ(0, close(self->client));
+
+			_exit(_metadata->exit_code);
+			return;
+		}
+
+		ASSERT_EQ(0, close(pipe_child[0]));
+		if (variant->domain_child == OTHER_SANDBOX)
+			create_domain(_metadata);
+		else if (variant->domain_child == SCOPE_SANDBOX)
+			create_unix_domain(_metadata);
+
+		self->child_server = socket(AF_UNIX, variant->type, 0);
+		ASSERT_NE(-1, self->child_server);
+		ASSERT_EQ(0, bind(self->child_server,
+				  (struct sockaddr *)&child_addr, addrlen));
+		if (variant->type == SOCK_STREAM)
+			ASSERT_EQ(0, listen(self->child_server, 32));
+
+		ASSERT_EQ(1, write(pipe_child[1], ".", 1));
+		ASSERT_EQ(grand_child, waitpid(grand_child, &status, 0));
+		return;
+	}
+	ASSERT_EQ(0, close(pipe_parent[0]));
+
+	if (variant->domain_parent == OTHER_SANDBOX)
+		create_domain(_metadata);
+	else if (variant->domain_parent == SCOPE_SANDBOX)
+		create_unix_domain(_metadata);
+
+	self->parent_server = socket(AF_UNIX, variant->type, 0);
+	ASSERT_NE(-1, self->parent_server);
+	ASSERT_EQ(0,
+		  bind(self->parent_server, (struct sockaddr *)&addr, addrlen));
+
+	if (variant->type == SOCK_STREAM)
+		ASSERT_EQ(0, listen(self->parent_server, 32));
+
+	ASSERT_EQ(1, write(pipe_parent[1], ".", 1));
+	ASSERT_EQ(child, waitpid(child, &status, 0));
+	if (WIFSIGNALED(status) || !WIFEXITED(status) ||
+	    WEXITSTATUS(status) != EXIT_SUCCESS)
+		_metadata->exit_code = KSFT_FAIL;
+}
+
+/*
+ * Since the special case of scoping only happens when the connecting socket
+ * is scoped, the client's domain is true for all the following test cases.
+ */
+/* clang-format off */
+FIXTURE(unix_sock_special_cases) {
+	int server_socket, client;
+	int stream_server, stream_client;
+};
+
+/* clang-format on */
+FIXTURE_VARIANT(unix_sock_special_cases)
+{
+	const bool domain_server;
+	const bool domain_server_socket;
+	const int type;
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(unix_sock_special_cases, allow_dgram_server_sock_domain) {
+	/* clang-format on */
+	.domain_server = false,
+	.domain_server_socket = true,
+	.type = SOCK_DGRAM,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(unix_sock_special_cases, deny_dgram_server_domain) {
+	/* clang-format off */
+	.domain_server = true,
+	.domain_server_socket = false,
+	.type = SOCK_DGRAM,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(unix_sock_special_cases, allow_stream_server_sock_domain) {
+	/* clang-format on */
+	.domain_server = false,
+	.domain_server_socket = true,
+	.type = SOCK_STREAM,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(unix_sock_special_cases, deny_stream_server_domain) {
+        /* clang-format off */
+        .domain_server = true,
+        .domain_server_socket = false,
+        .type = SOCK_STREAM,
+};
+
+FIXTURE_SETUP(unix_sock_special_cases)
+{
+}
+
+FIXTURE_TEARDOWN(unix_sock_special_cases)
+{
+	close(self->client);
+	close(self->server_socket);
+	close(self->stream_server);
+	close(self->stream_client);
+}
+
+/* Test UNIX_STREAM_CONNECT and UNIX_MAY_SEND for parent and
+ * child processes when connecting socket has different domain
+ * than the process using it.
+ **/
+TEST_F(unix_sock_special_cases, dgram_cases)
+{
+	pid_t child;
+	socklen_t addrlen;
+	int sock_len = 5;
+	struct sockaddr_un addr, addr_stream = {
+		.sun_family = AF_UNIX,
+	};
+	const char sun_path[8] = "\0test";
+	const char sun_path_stream[8] = "\0strm";
+	int err, status;
+	int pipe_child[2], pipe_parent[2];
+	char buf_parent;
+
+	ASSERT_EQ(0, pipe2(pipe_child, O_CLOEXEC));
+	ASSERT_EQ(0, pipe2(pipe_parent, O_CLOEXEC));
+
+	addrlen = offsetof(struct sockaddr_un, sun_path) + sock_len;
+	memcpy(&addr.sun_path, sun_path, sock_len);
+
+	child = fork();
+	ASSERT_LE(0, child);
+	if (child == 0) {
+		char buf_child;
+
+		ASSERT_EQ(0, close(pipe_parent[1]));
+		ASSERT_EQ(0, close(pipe_child[0]));
+
+		/* client always has domain */
+		create_unix_domain(_metadata);
+
+		if (variant->domain_server_socket) {
+			int data_socket;
+			int fd_sock = socket(AF_UNIX, variant->type, 0);
+
+			ASSERT_NE(-1, fd_sock);
+
+			self->stream_server = socket(AF_UNIX, SOCK_STREAM, 0);
+
+			ASSERT_NE(-1, self->stream_server);
+			memcpy(&addr_stream.sun_path, sun_path_stream,
+			       sock_len);
+			ASSERT_EQ(0, bind(self->stream_server,
+					  (struct sockaddr *)&addr_stream,
+					  addrlen));
+			ASSERT_EQ(0, listen(self->stream_server, 32));
+
+			ASSERT_EQ(1, write(pipe_child[1], ".", 1));
+
+			data_socket = accept(self->stream_server, NULL, NULL);
+
+			ASSERT_EQ(0, send_fd(data_socket, fd_sock));
+			ASSERT_EQ(0, close(fd_sock));
+			TH_LOG("sending completed\n");
+		}
+
+		self->client = socket(AF_UNIX, variant->type, 0);
+		ASSERT_NE(-1, self->client);
+		/* wait for parent signal for connection */
+		ASSERT_EQ(1, read(pipe_parent[0], &buf_child, 1));
+
+		err = connect(self->client, (struct sockaddr *)&addr, addrlen);
+		if (!variant->domain_server_socket) {
+			EXPECT_EQ(-1, err);
+			EXPECT_EQ(EPERM, errno);
+		} else {
+			EXPECT_EQ(0, err);
+		}
+		_exit(_metadata->exit_code);
+		return;
+	}
+
+	ASSERT_EQ(0, close(pipe_child[1]));
+	ASSERT_EQ(0, close(pipe_parent[0]));
+
+	if (!variant->domain_server_socket) {
+		self->server_socket = socket(AF_UNIX, variant->type, 0);
+	} else {
+		int cli = socket(AF_UNIX, SOCK_STREAM, 0);
+
+		ASSERT_NE(-1, cli);
+		memcpy(&addr_stream.sun_path, sun_path_stream, sock_len);
+		ASSERT_EQ(1, read(pipe_child[0], &buf_parent, 1));
+		ASSERT_EQ(0, connect(cli, (struct sockaddr *)&addr_stream,
+				     addrlen));
+
+		self->server_socket = recv_fd(cli);
+		ASSERT_LE(0, self->server_socket);
+	}
+
+	ASSERT_NE(-1, self->server_socket);
+
+	if (variant->domain_server)
+		create_unix_domain(_metadata);
+
+	ASSERT_EQ(0,
+		  bind(self->server_socket, (struct sockaddr *)&addr, addrlen));
+	if (variant->type == SOCK_STREAM)
+		ASSERT_EQ(0, listen(self->server_socket, 32));
+	/* signal to child that parent is listening */
+	ASSERT_EQ(1, write(pipe_parent[1], ".", 1));
+
+	ASSERT_EQ(child, waitpid(child, &status, 0));
+
+	if (WIFSIGNALED(status) || !WIFEXITED(status) ||
+	    WEXITSTATUS(status) != EXIT_SUCCESS)
+		_metadata->exit_code = KSFT_FAIL;
+}
 TEST_HARNESS_MAIN
-- 
2.34.1

[PATCH v7 3/4] samples/landlock: Support abstract unix socket restriction

[Tahera Fahimi]

- Adding IPC scoping to the sandbox demo by defining a new "LL_SCOPED"
  environment variable. "LL_SCOPED" gets value "a" to restrict abstract
  unix sockets.
- Change to LANDLOCK_ABI_LAST to 6.

Signed-off-by: Tahera Fahimi <fahimitahera@gmail.com>
---
 samples/landlock/sandboxer.c | 25 +++++++++++++++++++------
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/samples/landlock/sandboxer.c b/samples/landlock/sandboxer.c
index e8223c3e781a..d280616585d4 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,6 +56,7 @@ 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_SCOPED_NAME "LL_SCOPED"
 #define ENV_DELIMITER ":"
 
 static int parse_path(char *env_path, const char ***const path_list)
@@ -208,7 +210,7 @@ static int populate_ruleset_net(const char *const env_var, const int ruleset_fd,
 
 /* clang-format on */
 
-#define LANDLOCK_ABI_LAST 5
+#define LANDLOCK_ABI_LAST 6
 
 int main(const int argc, char *const argv[], char *const *const envp)
 {
@@ -216,6 +218,7 @@ int main(const int argc, char *const argv[], char *const *const envp)
 	char *const *cmd_argv;
 	int ruleset_fd, abi;
 	char *env_port_name;
+	char *env_scoped_name;
 	__u64 access_fs_ro = ACCESS_FS_ROUGHLY_READ,
 	      access_fs_rw = ACCESS_FS_ROUGHLY_READ | ACCESS_FS_ROUGHLY_WRITE;
 
@@ -223,14 +226,15 @@ 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,
+		.scoped = LANDLOCK_SCOPED_ABSTRACT_UNIX_SOCKET,
 	};
 
 	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_SCOPED_NAME, argv[0]);
 		fprintf(stderr,
 			"Execute a command in a restricted environment.\n\n");
 		fprintf(stderr,
@@ -251,15 +255,18 @@ 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 allowed restriction on IPCs.\n",
+			ENV_SCOPED_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=\"a\" "
 			"%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_SCOPED_NAME, argv[0]);
 		fprintf(stderr,
 			"This sandboxer can use Landlock features "
 			"up to ABI version %d.\n",
@@ -326,7 +333,10 @@ int main(const int argc, char *const argv[], char *const *const envp)
 	case 4:
 		/* Removes LANDLOCK_ACCESS_FS_IOCTL_DEV for ABI < 5 */
 		ruleset_attr.handled_access_fs &= ~LANDLOCK_ACCESS_FS_IOCTL_DEV;
-
+		__attribute__((fallthrough));
+	case 5:
+		/* Removes IPC scoping mechanism for ABI < 6 */
+		ruleset_attr.scoped &= ~LANDLOCK_SCOPED_ABSTRACT_UNIX_SOCKET;
 		fprintf(stderr,
 			"Hint: You should update the running kernel "
 			"to leverage Landlock features "
@@ -357,7 +367,10 @@ int main(const int argc, char *const argv[], char *const *const envp)
 		ruleset_attr.handled_access_net &=
 			~LANDLOCK_ACCESS_NET_CONNECT_TCP;
 	}
-
+	/* Removes IPC scoping attribute if not supported by a user. */
+	env_scoped_name = getenv(ENV_SCOPED_NAME);
+	if (!env_scoped_name)
+		ruleset_attr.scoped &= ~LANDLOCK_SCOPED_ABSTRACT_UNIX_SOCKET;
 	ruleset_fd =
 		landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
 	if (ruleset_fd < 0) {
-- 
2.34.1

[PATCH v7 4/4] documentation/landlock: Adding scoping mechanism documentation

[Tahera Fahimi]

- Defining ABI version 6 that supports IPC restriction.
- Adding "scoped" to the "Access rights".
- In current limitation, unnamed sockets are specified as
  sockets that are not restricted.

Signed-off-by: Tahera Fahimi <fahimitahera@gmail.com>
---
 Documentation/userspace-api/landlock.rst | 23 ++++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

diff --git a/Documentation/userspace-api/landlock.rst b/Documentation/userspace-api/landlock.rst
index 07b63aec56fa..61b91cc03560 100644
--- a/Documentation/userspace-api/landlock.rst
+++ b/Documentation/userspace-api/landlock.rst
@@ -8,7 +8,7 @@ Landlock: unprivileged access control
 =====================================
 
 :Author: Mickaël Salaün
-:Date: April 2024
+:Date: July 2024
 
 The goal of Landlock is to enable to restrict ambient rights (e.g. global
 filesystem or network access) for a set of processes.  Because Landlock
@@ -306,6 +306,16 @@ To be allowed to use :manpage:`ptrace(2)` and related syscalls on a target
 process, a sandboxed process should have a subset of the target process rules,
 which means the tracee must be in a sub-domain of the tracer.
 
+IPC Scoping
+-----------
+
+Similar to Ptrace, a sandboxed process should not be able to access the resources
+(like abstract unix sockets, or signals) outside of the sandbox domain. For example,
+a sandboxed process should not be able to :manpage:`connect(2)` to a non-sandboxed
+process through abstract unix sockets (:manpage:`unix(7)`). This restriction is
+applicable by optionally specifying ``LANDLOCK_SCOPED_ABSTRACT_UNIX_SOCKET`` in
+the ruleset.
+
 Truncating files
 ----------------
 
@@ -404,7 +414,7 @@ Access rights
 -------------
 
 .. kernel-doc:: include/uapi/linux/landlock.h
-    :identifiers: fs_access net_access
+    :identifiers: fs_access net_access scoped
 
 Creating a new ruleset
 ----------------------
@@ -446,7 +456,7 @@ Special filesystems
 
 Access to regular files and directories can be restricted by Landlock,
 according to the handled accesses of a ruleset.  However, files that do not
-come from a user-visible filesystem (e.g. pipe, socket), but can still be
+come from a user-visible filesystem (e.g. pipe, unnamed socket), but can still be
 accessed through ``/proc/<pid>/fd/*``, cannot currently be explicitly
 restricted.  Likewise, some special kernel filesystems such as nsfs, which can
 be accessed through ``/proc/<pid>/ns/*``, cannot currently be explicitly
@@ -541,6 +551,13 @@ earlier ABI.
 Starting with the Landlock ABI version 5, it is possible to restrict the use of
 :manpage:`ioctl(2)` using the new ``LANDLOCK_ACCESS_FS_IOCTL_DEV`` right.
 
+Special filesystems (ABI < 6)
+-----------------------------
+
+With ABI version 6, it is possible to restrict IPC actions such as connecting to
+an abstract Unix socket through ``LANDLOCK_SCOPED_ABSTRACT_UNIX_SOCKET``, thanks
+to the ``.scoped`` ruleset attribute.
+
 .. _kernel_support:
 
 Kernel support
-- 
2.34.1

Re: [PATCH v7 1/4] Landlock: Add abstract unix socket connect restriction

[Mickaël Salaün]

On Wed, Jul 17, 2024 at 10:15:19PM -0600, Tahera Fahimi wrote:
> The patch introduces a new "scoped" attribute to the
> landlock_ruleset_attr that can specify "LANDLOCK_SCOPED_ABSTRACT_UNIX_SOCKET"
> to scope abstract unix sockets from connecting to a process outside of
> the same landlock domain.
> 
> This patch implement two hooks, "unix_stream_connect" and "unix_may_send" to
> enforce this restriction.
> 
> Signed-off-by: Tahera Fahimi <fahimitahera@gmail.com>
> 
> -------

Only "---"

> v7:

Thanks for the detailed changelog, it helps!

>  - Using socket's file credentials for both connected(STREAM) and
>    non-connected(DGRAM) sockets.
>  - Adding "domain_sock_scope" instead of the domain scoping mechanism used in
>    ptrace ensures that if a server's domain is accessible from the client's
>    domain (where the client is more privileged than the server), the client
>    can connect to the server in all edge cases.
>  - Removing debug codes.
> v6:
>  - Removing curr_ruleset from landlock_hierarchy, and switching back to use
>    the same domain scoping as ptrace.
>  - code clean up.
> v5:
>  - Renaming "LANDLOCK_*_ACCESS_SCOPE" to "LANDLOCK_*_SCOPE"
>  - Adding curr_ruleset to hierarachy_ruleset structure to have access from
>    landlock_hierarchy to its respective landlock_ruleset.
>  - Using curr_ruleset to check if a domain is scoped while walking in the
>    hierarchy of domains.
>  - Modifying inline comments.
> V4:
>  - Rebased on Günther's Patch:
>    https://lore.kernel.org/all/20240610082115.1693267-1-gnoack@google.com/
>    so there is no need for "LANDLOCK_SHIFT_ACCESS_SCOPE", then it is removed.
>  - Adding get_scope_accesses function to check all scoped access masks in a ruleset.
>  - Using file's FD credentials instead of credentials stored in peer_cred
>    for datagram sockets. (see discussion in [1])
>  - Modifying inline comments.
> V3:
>  - Improving commit description.
>  - Introducing "scoped" attribute to landlock_ruleset_attr for IPC scoping
>    purpose, and adding related functions.
>  - Changing structure of ruleset based on "scoped".
>  - Removing rcu lock and using unix_sk lock instead.
>  - Introducing scoping for datagram sockets in unix_may_send.
> V2:
>  - Removing wrapper functions
> 
> [1]https://lore.kernel.org/outreachy/Zmi8Ydz4Z6tYtpY1@tahera-OptiPlex-5000/T/#m8cdf33180d86c7ec22932e2eb4ef7dd4fc94c792


> -------
> 
> Signed-off-by: Tahera Fahimi <fahimitahera@gmail.com>

No need for this hunk.


> ---
>  include/uapi/linux/landlock.h |  29 +++++++++
>  security/landlock/limits.h    |   3 +
>  security/landlock/ruleset.c   |   7 ++-
>  security/landlock/ruleset.h   |  23 ++++++-
>  security/landlock/syscalls.c  |  14 +++--
>  security/landlock/task.c      | 112 ++++++++++++++++++++++++++++++++++
>  6 files changed, 181 insertions(+), 7 deletions(-)

> diff --git a/security/landlock/task.c b/security/landlock/task.c
> index 849f5123610b..597d89e54aae 100644
> --- a/security/landlock/task.c
> +++ b/security/landlock/task.c
> @@ -13,6 +13,8 @@
>  #include <linux/lsm_hooks.h>
>  #include <linux/rcupdate.h>
>  #include <linux/sched.h>
> +#include <net/sock.h>
> +#include <net/af_unix.h>
>  
>  #include "common.h"
>  #include "cred.h"
> @@ -108,9 +110,119 @@ static int hook_ptrace_traceme(struct task_struct *const parent)
>  	return task_ptrace(parent, current);
>  }
>  
> +static int walk_and_check(const struct landlock_ruleset *const child,
> +			  struct landlock_hierarchy **walker, int i, int j,

We don't know what are "i" and "j" are while reading this function's
signature.  They need a better name.

Also, they are ingegers (signed), whereas l1 and l2 are size_t (unsigned).

> +			  bool check)
> +{
> +	if (!child || i < 0)
> +		return -1;
> +
> +	while (i < j && *walker) {

This would be more readable with a for() loop.

> +		if (check && landlock_get_scope_mask(child, j))

This is correct now but it will be a bug when we'll have other scope.
Instead, you can replace the "check" boolean with a variable containing
LANDLOCK_SCOPED_ABSTRACT_UNIX_SOCKET.

> +			return -1;
> +		*walker = (*walker)->parent;
> +		j--;
> +	}
> +	if (!*walker)
> +		pr_warn_once("inconsistency in landlock hierarchy and layers");

This must indeed never happen, but WARN_ON_ONCE(!*walker) would be
better than this check+pr_warn.

Anyway, if this happen this pointer will still be dereferenced in
domain_sock_scope() right?  This must not be possible.


> +	return j;

Because j is now equal to i, no need to return it.  This function can
return a boolean instead, or a struct landlock_ruleset pointer/NULL to
avoid the pointer of pointer?

> +}
> +
> +/**
> + * domain_sock_scope - Checks if client domain is scoped in the same
> + *			domain as server.
> + *
> + * @client: Connecting socket domain.
> + * @server: Listening socket domain.
> + *
> + * Checks if the @client domain is scoped, then the server should be
> + * in the same domain to connect. If not, @client can connect to @server.
> + */
> +static bool domain_sock_scope(const struct landlock_ruleset *const client,

This function can have a more generic name if
LANDLOCK_SCOPED_ABSTRACT_UNIX_SOCKET is passed as argument.  This could
be reused as-is for other kind of scope.

> +			      const struct landlock_ruleset *const server)
> +{
> +	size_t l1, l2;
> +	int scope_layer;
> +	struct landlock_hierarchy *cli_walker, *srv_walker;

We have some room for a bit more characters ;)
client_walker, server_walker;

> +
> +	if (!client)
> +		return true;
> +
> +	l1 = client->num_layers - 1;

Please rename variables in a consistent way, in this case something like
client_layer?

> +	cli_walker = client->hierarchy;
> +	if (server) {
> +		l2 = server->num_layers - 1;
> +		srv_walker = server->hierarchy;
> +	} else
> +		l2 = 0;
> +
> +	if (l1 > l2)
> +		scope_layer = walk_and_check(client, &cli_walker, l2, l1, true);

Instead of mixing the layer number with an error code, walk_and_check()
can return a boolean, take as argument &scope_layer, and update it.

> +	else if (l2 > l1)
> +		scope_layer =
> +			walk_and_check(server, &srv_walker, l1, l2, false);
> +	else
> +		scope_layer = l1;
> +
> +	if (scope_layer == -1)
> +		return false;

All these domains and layers checks are difficult to review. It needs at
least some comments, and preferably also some code refactoring to avoid
potential inconsistencies (checks).

> +
> +	while (scope_layer >= 0 && cli_walker) {

Why srv_walker is not checked?  Could this happen?  What would be the
result?

Please also use a for() loop here.

> +		if (landlock_get_scope_mask(client, scope_layer) &
> +		    LANDLOCK_SCOPED_ABSTRACT_UNIX_SOCKET) {

The logic needs to be explained.

> +			if (!server)
> +				return false;
> +
> +			if (srv_walker == cli_walker)
> +				return true;
> +
> +			return false;
> +		}
> +		cli_walker = cli_walker->parent;
> +		srv_walker = srv_walker->parent;
> +		scope_layer--;
> +	}
> +	return true;
> +}
> +
> +static bool sock_is_scoped(struct sock *const other)
> +{
> +	const struct landlock_ruleset *dom_other;
> +	const struct landlock_ruleset *const dom =
> +		landlock_get_current_domain();
> +
> +	/* the credentials will not change */
> +	lockdep_assert_held(&unix_sk(other)->lock);
> +	dom_other = landlock_cred(other->sk_socket->file->f_cred)->domain;
> +
> +	/* other is scoped, they connect if they are in the same domain */
> +	return domain_sock_scope(dom, dom_other);
> +}
> +
> +static int hook_unix_stream_connect(struct sock *const sock,
> +				    struct sock *const other,
> +				    struct sock *const newsk)
> +{
> +	if (sock_is_scoped(other))
> +		return 0;
> +
> +	return -EPERM;
> +}
> +
> +static int hook_unix_may_send(struct socket *const sock,
> +			      struct socket *const other)
> +{
> +	if (sock_is_scoped(other->sk))
> +		return 0;
> +
> +	return -EPERM;
> +}
> +
>  static struct security_hook_list landlock_hooks[] __ro_after_init = {
>  	LSM_HOOK_INIT(ptrace_access_check, hook_ptrace_access_check),
>  	LSM_HOOK_INIT(ptrace_traceme, hook_ptrace_traceme),
> +	LSM_HOOK_INIT(unix_stream_connect, hook_unix_stream_connect),
> +	LSM_HOOK_INIT(unix_may_send, hook_unix_may_send),
>  };
>  
>  __init void landlock_add_task_hooks(void)
> -- 
> 2.34.1
> 
> 

Re: [PATCH v7 1/4] Landlock: Add abstract unix socket connect restriction

[Tahera Fahimi]

On Fri, Jul 19, 2024 at 08:14:02PM +0200, Mickaël Salaün wrote:
> On Wed, Jul 17, 2024 at 10:15:19PM -0600, Tahera Fahimi wrote:
> > The patch introduces a new "scoped" attribute to the
> > landlock_ruleset_attr that can specify "LANDLOCK_SCOPED_ABSTRACT_UNIX_SOCKET"
> > to scope abstract unix sockets from connecting to a process outside of
> > the same landlock domain.
> > 
> > This patch implement two hooks, "unix_stream_connect" and "unix_may_send" to
> > enforce this restriction.
> > 
> > Signed-off-by: Tahera Fahimi <fahimitahera@gmail.com>
> > 
> > -------
Hello Mickaël, 
Thanks for the feedback.
> Only "---"
> 
> > v7:
> 
> Thanks for the detailed changelog, it helps!
> 
> >  - Using socket's file credentials for both connected(STREAM) and
> >    non-connected(DGRAM) sockets.
> >  - Adding "domain_sock_scope" instead of the domain scoping mechanism used in
> >    ptrace ensures that if a server's domain is accessible from the client's
> >    domain (where the client is more privileged than the server), the client
> >    can connect to the server in all edge cases.
> >  - Removing debug codes.
> > v6:
> >  - Removing curr_ruleset from landlock_hierarchy, and switching back to use
> >    the same domain scoping as ptrace.
> >  - code clean up.
> > v5:
> >  - Renaming "LANDLOCK_*_ACCESS_SCOPE" to "LANDLOCK_*_SCOPE"
> >  - Adding curr_ruleset to hierarachy_ruleset structure to have access from
> >    landlock_hierarchy to its respective landlock_ruleset.
> >  - Using curr_ruleset to check if a domain is scoped while walking in the
> >    hierarchy of domains.
> >  - Modifying inline comments.
> > V4:
> >  - Rebased on Günther's Patch:
> >    https://lore.kernel.org/all/20240610082115.1693267-1-gnoack@google.com/
> >    so there is no need for "LANDLOCK_SHIFT_ACCESS_SCOPE", then it is removed.
> >  - Adding get_scope_accesses function to check all scoped access masks in a ruleset.
> >  - Using file's FD credentials instead of credentials stored in peer_cred
> >    for datagram sockets. (see discussion in [1])
> >  - Modifying inline comments.
> > V3:
> >  - Improving commit description.
> >  - Introducing "scoped" attribute to landlock_ruleset_attr for IPC scoping
> >    purpose, and adding related functions.
> >  - Changing structure of ruleset based on "scoped".
> >  - Removing rcu lock and using unix_sk lock instead.
> >  - Introducing scoping for datagram sockets in unix_may_send.
> > V2:
> >  - Removing wrapper functions
> > 
> > [1]https://lore.kernel.org/outreachy/Zmi8Ydz4Z6tYtpY1@tahera-OptiPlex-5000/T/#m8cdf33180d86c7ec22932e2eb4ef7dd4fc94c792
> 
> 
> > -------
> > 
> > Signed-off-by: Tahera Fahimi <fahimitahera@gmail.com>
> 
> No need for this hunk.
> 
> 
> > ---
> >  include/uapi/linux/landlock.h |  29 +++++++++
> >  security/landlock/limits.h    |   3 +
> >  security/landlock/ruleset.c   |   7 ++-
> >  security/landlock/ruleset.h   |  23 ++++++-
> >  security/landlock/syscalls.c  |  14 +++--
> >  security/landlock/task.c      | 112 ++++++++++++++++++++++++++++++++++
> >  6 files changed, 181 insertions(+), 7 deletions(-)
> 
> > diff --git a/security/landlock/task.c b/security/landlock/task.c
> > index 849f5123610b..597d89e54aae 100644
> > --- a/security/landlock/task.c
> > +++ b/security/landlock/task.c
> > @@ -13,6 +13,8 @@
> >  #include <linux/lsm_hooks.h>
> >  #include <linux/rcupdate.h>
> >  #include <linux/sched.h>
> > +#include <net/sock.h>
> > +#include <net/af_unix.h>
> >  
> >  #include "common.h"
> >  #include "cred.h"
> > @@ -108,9 +110,119 @@ static int hook_ptrace_traceme(struct task_struct *const parent)
> >  	return task_ptrace(parent, current);
> >  }
> >  
> > +static int walk_and_check(const struct landlock_ruleset *const child,
> > +			  struct landlock_hierarchy **walker, int i, int j,
> 
> We don't know what are "i" and "j" are while reading this function's
> signature.  They need a better name.
> 
> Also, they are ingegers (signed), whereas l1 and l2 are size_t (unsigned).
> 
> > +			  bool check)
> > +{
> > +	if (!child || i < 0)
> > +		return -1;
> > +
> > +	while (i < j && *walker) {
> 
> This would be more readable with a for() loop.
> 
> > +		if (check && landlock_get_scope_mask(child, j))
> 
> This is correct now but it will be a bug when we'll have other scope.
> Instead, you can replace the "check" boolean with a variable containing
> LANDLOCK_SCOPED_ABSTRACT_UNIX_SOCKET.
> 
> > +			return -1;
> > +		*walker = (*walker)->parent;
> > +		j--;
> > +	}
> > +	if (!*walker)
> > +		pr_warn_once("inconsistency in landlock hierarchy and layers");
> 
> This must indeed never happen, but WARN_ON_ONCE(!*walker) would be
> better than this check+pr_warn.
> 
> Anyway, if this happen this pointer will still be dereferenced in
> domain_sock_scope() right?  This must not be possible.
> 
> 
> > +	return j;
> 
> Because j is now equal to i, no need to return it.  This function can
> return a boolean instead, or a struct landlock_ruleset pointer/NULL to
> avoid the pointer of pointer?
corret, in the next version, this function will return a boolean that
shows chcking the hierarchy of domain is successful or not. 

> > +}
> > +
> > +/**
> > + * domain_sock_scope - Checks if client domain is scoped in the same
> > + *			domain as server.
> > + *
> > + * @client: Connecting socket domain.
> > + * @server: Listening socket domain.
> > + *
> > + * Checks if the @client domain is scoped, then the server should be
> > + * in the same domain to connect. If not, @client can connect to @server.
> > + */
> > +static bool domain_sock_scope(const struct landlock_ruleset *const client,
> 
> This function can have a more generic name if
> LANDLOCK_SCOPED_ABSTRACT_UNIX_SOCKET is passed as argument.  This could
> be reused as-is for other kind of scope.
> 
> > +			      const struct landlock_ruleset *const server)
> > +{
> > +	size_t l1, l2;
> > +	int scope_layer;
> > +	struct landlock_hierarchy *cli_walker, *srv_walker;
> 
> We have some room for a bit more characters ;)
> client_walker, server_walker;
> 
> > +
> > +	if (!client)
> > +		return true;
> > +
> > +	l1 = client->num_layers - 1;
> 
> Please rename variables in a consistent way, in this case something like
> client_layer?
done 
> > +	cli_walker = client->hierarchy;
> > +	if (server) {
> > +		l2 = server->num_layers - 1;
> > +		srv_walker = server->hierarchy;
> > +	} else
> > +		l2 = 0;
> > +
> > +	if (l1 > l2)
> > +		scope_layer = walk_and_check(client, &cli_walker, l2, l1, true);
> 
> Instead of mixing the layer number with an error code, walk_and_check()
> can return a boolean, take as argument &scope_layer, and update it.
> 
> > +	else if (l2 > l1)
> > +		scope_layer =
> > +			walk_and_check(server, &srv_walker, l1, l2, false);
> > +	else
> > +		scope_layer = l1;
> > +
> > +	if (scope_layer == -1)
> > +		return false;
> 
> All these domains and layers checks are difficult to review. It needs at
> least some comments, and preferably also some code refactoring to avoid
> potential inconsistencies (checks).
> 
> > +
> > +	while (scope_layer >= 0 && cli_walker) {
> 
> Why srv_walker is not checked?  Could this happen?  What would be the
> result?
This is the same scenario as "walk_and_check". If the loop breaks
because of cli_walker is null, then there is an inconsistency between
num_layers and landlock_hierarchy. In normal scenario, we expect the
loop breaks with condition(scope_layer>=0).

> Please also use a for() loop here.
> 
> > +		if (landlock_get_scope_mask(client, scope_layer) &
> > +		    LANDLOCK_SCOPED_ABSTRACT_UNIX_SOCKET) {
> 
> The logic needs to be explained.
> 
> > +			if (!server)
> > +				return false;
> > +
> > +			if (srv_walker == cli_walker)
> > +				return true;
> > +
> > +			return false;
> > +		}
> > +		cli_walker = cli_walker->parent;
> > +		srv_walker = srv_walker->parent;
> > +		scope_layer--;
> > +	}
> > +	return true;
> > +}
> > +
> > +static bool sock_is_scoped(struct sock *const other)
> > +{
> > +	const struct landlock_ruleset *dom_other;
> > +	const struct landlock_ruleset *const dom =
> > +		landlock_get_current_domain();
> > +
> > +	/* the credentials will not change */
> > +	lockdep_assert_held(&unix_sk(other)->lock);
> > +	dom_other = landlock_cred(other->sk_socket->file->f_cred)->domain;
> > +
> > +	/* other is scoped, they connect if they are in the same domain */
> > +	return domain_sock_scope(dom, dom_other);
> > +}
> > +
> > +static int hook_unix_stream_connect(struct sock *const sock,
> > +				    struct sock *const other,
> > +				    struct sock *const newsk)
> > +{
> > +	if (sock_is_scoped(other))
> > +		return 0;
> > +
> > +	return -EPERM;
> > +}
> > +
> > +static int hook_unix_may_send(struct socket *const sock,
> > +			      struct socket *const other)
> > +{
> > +	if (sock_is_scoped(other->sk))
> > +		return 0;
> > +
> > +	return -EPERM;
> > +}
> > +
> >  static struct security_hook_list landlock_hooks[] __ro_after_init = {
> >  	LSM_HOOK_INIT(ptrace_access_check, hook_ptrace_access_check),
> >  	LSM_HOOK_INIT(ptrace_traceme, hook_ptrace_traceme),
> > +	LSM_HOOK_INIT(unix_stream_connect, hook_unix_stream_connect),
> > +	LSM_HOOK_INIT(unix_may_send, hook_unix_may_send),
> >  };
> >  
> >  __init void landlock_add_task_hooks(void)
> > -- 
> > 2.34.1
> > 
> > 

Re: [PATCH v7 1/4] Landlock: Add abstract unix socket connect restriction

[Mickaël Salaün]

On Wed, Jul 17, 2024 at 10:15:19PM -0600, Tahera Fahimi wrote:
> The patch introduces a new "scoped" attribute to the
> landlock_ruleset_attr that can specify "LANDLOCK_SCOPED_ABSTRACT_UNIX_SOCKET"
> to scope abstract unix sockets from connecting to a process outside of
> the same landlock domain.
> 
> This patch implement two hooks, "unix_stream_connect" and "unix_may_send" to
> enforce this restriction.
> 
> Signed-off-by: Tahera Fahimi <fahimitahera@gmail.com>
> 
> -------

> diff --git a/security/landlock/syscalls.c b/security/landlock/syscalls.c
> index 03b470f5a85a..799a50f11d79 100644
> --- a/security/landlock/syscalls.c
> +++ b/security/landlock/syscalls.c
> @@ -97,8 +97,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.scoped);
>  	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);
> @@ -149,7 +150,7 @@ static const struct file_operations ruleset_fops = {
>  	.write = fop_dummy_write,
>  };
>  
> -#define LANDLOCK_ABI_VERSION 5
> +#define LANDLOCK_ABI_VERSION 6
>  
>  /**
>   * sys_landlock_create_ruleset - Create a new ruleset
> @@ -170,7 +171,7 @@ static const struct file_operations ruleset_fops = {
>   * Possible returned errors are:
>   *
>   * - %EOPNOTSUPP: Landlock is supported by the kernel but disabled at boot time;
> - * - %EINVAL: unknown @flags, or unknown access, or too small @size;
> + * - %EINVAL: unknown @flags, or unknown access, or uknown scope, or too small @size;

You'll need to rebase on top of my next branch to take into account
recent Günther's changes.

>   * - %E2BIG or %EFAULT: @attr or @size inconsistencies;
>   * - %ENOMSG: empty &landlock_ruleset_attr.handled_access_fs.
>   */

Re: [PATCH v7 3/4] samples/landlock: Support abstract unix socket restriction

[Mickaël Salaün]

On Wed, Jul 17, 2024 at 10:15:21PM -0600, Tahera Fahimi wrote:
> - Adding IPC scoping to the sandbox demo by defining a new "LL_SCOPED"
>   environment variable. "LL_SCOPED" gets value "a" to restrict abstract
>   unix sockets.
> - Change to LANDLOCK_ABI_LAST to 6.
> 
> Signed-off-by: Tahera Fahimi <fahimitahera@gmail.com>
> ---
>  samples/landlock/sandboxer.c | 25 +++++++++++++++++++------
>  1 file changed, 19 insertions(+), 6 deletions(-)
> 
> diff --git a/samples/landlock/sandboxer.c b/samples/landlock/sandboxer.c
> index e8223c3e781a..d280616585d4 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,6 +56,7 @@ 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_SCOPED_NAME "LL_SCOPED"
>  #define ENV_DELIMITER ":"
>  
>  static int parse_path(char *env_path, const char ***const path_list)
> @@ -208,7 +210,7 @@ static int populate_ruleset_net(const char *const env_var, const int ruleset_fd,
>  
>  /* clang-format on */
>  
> -#define LANDLOCK_ABI_LAST 5
> +#define LANDLOCK_ABI_LAST 6
>  
>  int main(const int argc, char *const argv[], char *const *const envp)
>  {
> @@ -216,6 +218,7 @@ int main(const int argc, char *const argv[], char *const *const envp)
>  	char *const *cmd_argv;
>  	int ruleset_fd, abi;
>  	char *env_port_name;
> +	char *env_scoped_name;
>  	__u64 access_fs_ro = ACCESS_FS_ROUGHLY_READ,
>  	      access_fs_rw = ACCESS_FS_ROUGHLY_READ | ACCESS_FS_ROUGHLY_WRITE;
>  
> @@ -223,14 +226,15 @@ 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,
> +		.scoped = LANDLOCK_SCOPED_ABSTRACT_UNIX_SOCKET,
>  	};
>  
>  	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_SCOPED_NAME, argv[0]);
>  		fprintf(stderr,
>  			"Execute a command in a restricted environment.\n\n");
>  		fprintf(stderr,
> @@ -251,15 +255,18 @@ 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 allowed restriction on IPCs.\n",

"allowed restrictions" or "restrictions"?

> +			ENV_SCOPED_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=\"a\" "
>  			"%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_SCOPED_NAME, argv[0]);
>  		fprintf(stderr,
>  			"This sandboxer can use Landlock features "
>  			"up to ABI version %d.\n",
> @@ -326,7 +333,10 @@ int main(const int argc, char *const argv[], char *const *const envp)
>  	case 4:
>  		/* Removes LANDLOCK_ACCESS_FS_IOCTL_DEV for ABI < 5 */
>  		ruleset_attr.handled_access_fs &= ~LANDLOCK_ACCESS_FS_IOCTL_DEV;
> -

No need to remove this line.

> +		__attribute__((fallthrough));
> +	case 5:
> +		/* Removes IPC scoping mechanism for ABI < 6 */
> +		ruleset_attr.scoped &= ~LANDLOCK_SCOPED_ABSTRACT_UNIX_SOCKET;

There is an inconsistency here, please take a look at previous similar
changes.

>  		fprintf(stderr,
>  			"Hint: You should update the running kernel "
>  			"to leverage Landlock features "
> @@ -357,7 +367,10 @@ int main(const int argc, char *const argv[], char *const *const envp)
>  		ruleset_attr.handled_access_net &=
>  			~LANDLOCK_ACCESS_NET_CONNECT_TCP;
>  	}
> -
> +	/* Removes IPC scoping attribute if not supported by a user. */
> +	env_scoped_name = getenv(ENV_SCOPED_NAME);
> +	if (!env_scoped_name)
> +		ruleset_attr.scoped &= ~LANDLOCK_SCOPED_ABSTRACT_UNIX_SOCKET;
>  	ruleset_fd =
>  		landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
>  	if (ruleset_fd < 0) {
> -- 
> 2.34.1
> 
> 

Re: [PATCH v7 4/4] documentation/landlock: Adding scoping mechanism documentation

[Mickaël Salaün]

The subject should start with "landlock:" not "documentation/landlock:"
See similar commits.

On Wed, Jul 17, 2024 at 10:15:22PM -0600, Tahera Fahimi wrote:
> - Defining ABI version 6 that supports IPC restriction.
> - Adding "scoped" to the "Access rights".
> - In current limitation, unnamed sockets are specified as
>   sockets that are not restricted.

It would help to write (small) paragraphs instead of bullet points (here
and for other patches).

> 
> Signed-off-by: Tahera Fahimi <fahimitahera@gmail.com>
> ---
>  Documentation/userspace-api/landlock.rst | 23 ++++++++++++++++++++---
>  1 file changed, 20 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/userspace-api/landlock.rst b/Documentation/userspace-api/landlock.rst
> index 07b63aec56fa..61b91cc03560 100644
> --- a/Documentation/userspace-api/landlock.rst
> +++ b/Documentation/userspace-api/landlock.rst
> @@ -8,7 +8,7 @@ Landlock: unprivileged access control
>  =====================================
>  
>  :Author: Mickaël Salaün
> -:Date: April 2024
> +:Date: July 2024
>  
>  The goal of Landlock is to enable to restrict ambient rights (e.g. global
>  filesystem or network access) for a set of processes.  Because Landlock
> @@ -306,6 +306,16 @@ To be allowed to use :manpage:`ptrace(2)` and related syscalls on a target
>  process, a sandboxed process should have a subset of the target process rules,
>  which means the tracee must be in a sub-domain of the tracer.
>  
> +IPC Scoping
> +-----------
> +
> +Similar to Ptrace, a sandboxed process should not be able to access the resources
> +(like abstract unix sockets, or signals) outside of the sandbox domain. For example,
> +a sandboxed process should not be able to :manpage:`connect(2)` to a non-sandboxed
> +process through abstract unix sockets (:manpage:`unix(7)`). This restriction is
> +applicable by optionally specifying ``LANDLOCK_SCOPED_ABSTRACT_UNIX_SOCKET`` in
> +the ruleset.

Here is a proposal based on your text:

Complementary to the implicit `ptrace restrictions`_, we may want to
further restrict interactions between sandboxes.  Each Landlock domain
can be explicitly scoped for a set of actions by specifying it on a
ruleset.

For example, if a sandboxed process should not be able to
:manpage:`connect(2)` to a non-sandboxed process through abstract
:manpage:`unix(7)` sockets, we can specify such restriction with
``LANDLOCK_SCOPED_ABSTRACT_UNIX_SOCKET``.


(We also need to explain how scoping works, especially between scoped
and non-scoped domains)

> +
>  Truncating files
>  ----------------
>  
> @@ -404,7 +414,7 @@ Access rights
>  -------------
>  
>  .. kernel-doc:: include/uapi/linux/landlock.h
> -    :identifiers: fs_access net_access
> +    :identifiers: fs_access net_access scoped
>  
>  Creating a new ruleset
>  ----------------------
> @@ -446,7 +456,7 @@ Special filesystems
>  
>  Access to regular files and directories can be restricted by Landlock,
>  according to the handled accesses of a ruleset.  However, files that do not
> -come from a user-visible filesystem (e.g. pipe, socket), but can still be
> +come from a user-visible filesystem (e.g. pipe, unnamed socket), but can still be

Why this change? Opened named sockets are still visible in /proc/self/fd/

>  accessed through ``/proc/<pid>/fd/*``, cannot currently be explicitly
>  restricted.  Likewise, some special kernel filesystems such as nsfs, which can
>  be accessed through ``/proc/<pid>/ns/*``, cannot currently be explicitly
> @@ -541,6 +551,13 @@ earlier ABI.
>  Starting with the Landlock ABI version 5, it is possible to restrict the use of
>  :manpage:`ioctl(2)` using the new ``LANDLOCK_ACCESS_FS_IOCTL_DEV`` right.
>  
> +Special filesystems (ABI < 6)

"Special filesystems"? This patch series is about abstract unix socket
scoping.  The signal scoping one can inlcude a patch rewriting this title.

> +-----------------------------
> +
> +With ABI version 6, it is possible to restrict IPC actions such as connecting to

The signal patch series may be merged with this one for the same kernel
release but we should be explicit about the *current" changes.  You can
write this section talking only about
LANDLOCK_SCOPED_ABSTRACT_UNIX_SOCKET, and in the signal scoping patch
series you can extend this section.

> +an abstract Unix socket through ``LANDLOCK_SCOPED_ABSTRACT_UNIX_SOCKET``, thanks
> +to the ``.scoped`` ruleset attribute.

The dot is superfluous (here and in comments):

"thanks to the ruleset's ``scoped`` attribute."

> +
>  .. _kernel_support:
>  
>  Kernel support
> -- 
> 2.34.1
> 
> 

Re: [PATCH v7 2/4] selftests/landlock: Abstract unix socket restriction tests

[Mickaël Salaün]

On Wed, Jul 17, 2024 at 10:15:20PM -0600, Tahera Fahimi wrote:
> The patch has three types of tests:
>   1) unix_socket: base tests the scoping mechanism for a landlocked process,
>      same as the ptrace test.
>   2) optional_scoping: generates three processes with different domains and
>      tests if a process with a non-scoped domain can connect to other processes.
>   3) unix_sock_special_cases: since the socket's creator credentials are used
>      for scoping datagram sockets, this test examines the cases where the
>      socket's credentials are different from the process using it.
> 
> Signed-off-by: Tahera Fahimi <fahimitahera@gmail.com>

You need to add "---" here.  You can test the result by applying this
patch with `git am`: the comments after these three dashes will not be
inlcuded in the commit message.

> 
> Changes in versions:
> V7:
>  - Introducing landlock ABI version 6.
>  - Adding some edge test cases to optional_scoping test.
>  - Using `enum` for different domains in optional_scoping tests.
>  - Extend unix_sock_special_cases test cases for connected(SOCK_STREAM) sockets.
>  - Modifying inline comments.
> V6:
>  - Introducing optional_scoping test which ensures a sandboxed process with a
>    non-scoped domain can still connect to another abstract unix socket(either
>    sandboxed or non-sandboxed).
>  - Introducing unix_sock_special_cases test which tests examines scenarios where
>    the connecting sockets have different domain than the process using them.
> V4:
>  - Introducing unix_socket to evaluate the basic scoping mechanism for abstract
>    unix sockets.
> 
> Signed-off-by: Tahera Fahimi <fahimitahera@gmail.com>
> ---
>  tools/testing/selftests/landlock/base_test.c  |   2 +-
>  .../testing/selftests/landlock/ptrace_test.c  | 867 ++++++++++++++++++
>  2 files changed, 868 insertions(+), 1 deletion(-)

Even if these tests are similar to the ptrace's ones, they don't share
code.  For maintainance reasons and test flexibility, we should have
these new tests in a "scoped_abstract_unix_test.c" file.

> 
> diff --git a/tools/testing/selftests/landlock/base_test.c b/tools/testing/selftests/landlock/base_test.c
> index 3c1e9f35b531..52b00472a487 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(5, landlock_create_ruleset(NULL, 0,
> +	ASSERT_EQ(6, landlock_create_ruleset(NULL, 0,
>  					     LANDLOCK_CREATE_RULESET_VERSION));
>  
>  	ASSERT_EQ(-1, landlock_create_ruleset(&ruleset_attr, 0,
> diff --git a/tools/testing/selftests/landlock/ptrace_test.c b/tools/testing/selftests/landlock/ptrace_test.c
> index a19db4d0b3bd..e7dcefda8ce0 100644
> --- a/tools/testing/selftests/landlock/ptrace_test.c
> +++ b/tools/testing/selftests/landlock/ptrace_test.c


> +/* Test UNIX_STREAM_CONNECT and UNIX_MAY_SEND for parent, child
> + * and grand child processes when they can have scoped or non-scoped
> + * domains.
> + **/
> +TEST_F(optional_scoping, unix_scoping)
> +{
> +	pid_t child;
> +	socklen_t addrlen;
> +	int sock_len = 5;
> +	int status;
> +	struct sockaddr_un addr = {
> +		.sun_family = AF_UNIX,
> +	};
> +	const char sun_path[8] = "\0test";
> +	bool can_connect_to_parent, can_connect_to_child;
> +	int pipe_parent[2];
> +
> +	if (variant->domain_grand_child == SCOPE_SANDBOX)
> +		can_connect_to_child = false;
> +	else
> +		can_connect_to_child = true;
> +
> +	if (!can_connect_to_child || variant->domain_children == SCOPE_SANDBOX)
> +		can_connect_to_parent = false;
> +	else
> +		can_connect_to_parent = true;
> +
> +	addrlen = offsetof(struct sockaddr_un, sun_path) + sock_len;
> +	memcpy(&addr.sun_path, sun_path, sock_len);
> +
> +	ASSERT_EQ(0, pipe2(pipe_parent, O_CLOEXEC));
> +
> +	if (variant->domain_all == OTHER_SANDBOX)
> +		create_domain(_metadata);
> +	else if (variant->domain_all == SCOPE_SANDBOX)
> +		create_unix_domain(_metadata);
> +
> +	child = fork();
> +	ASSERT_LE(0, child);
> +	if (child == 0) {
> +		int pipe_child[2];
> +		ASSERT_EQ(0, pipe2(pipe_child, O_CLOEXEC));
> +		pid_t grand_child;
> +		struct sockaddr_un child_addr = {
> +			.sun_family = AF_UNIX,
> +		};
> +		const char child_sun_path[8] = "\0tsst";

To avoid potential issues with conflicting names, we should use the same
sun_path as in net_test.c:set_service().  Same for all use of sun_path.

> +
> +		memcpy(&child_addr.sun_path, child_sun_path, sock_len);
> +
> +		if (variant->domain_children == OTHER_SANDBOX)
> +			create_domain(_metadata);
> +		else if (variant->domain_children == SCOPE_SANDBOX)
> +			create_unix_domain(_metadata);
> +
> +		grand_child = fork();
> +		ASSERT_LE(0, grand_child);
> +		if (grand_child == 0) {
> +			ASSERT_EQ(0, close(pipe_parent[1]));
> +			ASSERT_EQ(0, close(pipe_child[1]));
> +
> +			char buf1, buf2;
> +			int err;
> +
> +			if (variant->domain_grand_child == OTHER_SANDBOX)
> +				create_domain(_metadata);
> +			else if (variant->domain_grand_child == SCOPE_SANDBOX)
> +				create_unix_domain(_metadata);
> +
> +			self->client = socket(AF_UNIX, variant->type, 0);
> +			ASSERT_NE(-1, self->client);
> +
> +			ASSERT_EQ(1, read(pipe_child[0], &buf2, 1));
> +			err = connect(self->client,
> +				      (struct sockaddr *)&child_addr, addrlen);
> +			if (can_connect_to_child) {
> +				EXPECT_EQ(0, err);
> +			} else {
> +				EXPECT_EQ(-1, err);
> +				EXPECT_EQ(EPERM, errno);
> +			}
> +
> +			if (variant->type == SOCK_STREAM) {
> +				EXPECT_EQ(0, close(self->client));
> +				self->client =
> +					socket(AF_UNIX, variant->type, 0);
> +				ASSERT_NE(-1, self->client);
> +			}
> +
> +			ASSERT_EQ(1, read(pipe_parent[0], &buf1, 1));
> +			err = connect(self->client, (struct sockaddr *)&addr,
> +				      addrlen);
> +			if (can_connect_to_parent) {
> +				EXPECT_EQ(0, err);
> +			} else {
> +				EXPECT_EQ(-1, err);
> +				EXPECT_EQ(EPERM, errno);
> +			}
> +			EXPECT_EQ(0, close(self->client));
> +
> +			_exit(_metadata->exit_code);
> +			return;
> +		}
> +
> +		ASSERT_EQ(0, close(pipe_child[0]));
> +		if (variant->domain_child == OTHER_SANDBOX)
> +			create_domain(_metadata);
> +		else if (variant->domain_child == SCOPE_SANDBOX)
> +			create_unix_domain(_metadata);
> +
> +		self->child_server = socket(AF_UNIX, variant->type, 0);
> +		ASSERT_NE(-1, self->child_server);
> +		ASSERT_EQ(0, bind(self->child_server,
> +				  (struct sockaddr *)&child_addr, addrlen));
> +		if (variant->type == SOCK_STREAM)
> +			ASSERT_EQ(0, listen(self->child_server, 32));
> +
> +		ASSERT_EQ(1, write(pipe_child[1], ".", 1));
> +		ASSERT_EQ(grand_child, waitpid(grand_child, &status, 0));
> +		return;
> +	}
> +	ASSERT_EQ(0, close(pipe_parent[0]));
> +
> +	if (variant->domain_parent == OTHER_SANDBOX)
> +		create_domain(_metadata);
> +	else if (variant->domain_parent == SCOPE_SANDBOX)
> +		create_unix_domain(_metadata);
> +
> +	self->parent_server = socket(AF_UNIX, variant->type, 0);
> +	ASSERT_NE(-1, self->parent_server);
> +	ASSERT_EQ(0,
> +		  bind(self->parent_server, (struct sockaddr *)&addr, addrlen));
> +
> +	if (variant->type == SOCK_STREAM)
> +		ASSERT_EQ(0, listen(self->parent_server, 32));
> +
> +	ASSERT_EQ(1, write(pipe_parent[1], ".", 1));
> +	ASSERT_EQ(child, waitpid(child, &status, 0));
> +	if (WIFSIGNALED(status) || !WIFEXITED(status) ||
> +	    WEXITSTATUS(status) != EXIT_SUCCESS)
> +		_metadata->exit_code = KSFT_FAIL;
> +}
> +
> +/*
> + * Since the special case of scoping only happens when the connecting socket
> + * is scoped, the client's domain is true for all the following test cases.
> + */
> +/* clang-format off */
> +FIXTURE(unix_sock_special_cases) {
> +	int server_socket, client;
> +	int stream_server, stream_client;
> +};
> +
> +/* clang-format on */
> +FIXTURE_VARIANT(unix_sock_special_cases)
> +{
> +	const bool domain_server;
> +	const bool domain_server_socket;
> +	const int type;
> +};
> +
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(unix_sock_special_cases, allow_dgram_server_sock_domain) {
> +	/* clang-format on */
> +	.domain_server = false,
> +	.domain_server_socket = true,
> +	.type = SOCK_DGRAM,
> +};
> +
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(unix_sock_special_cases, deny_dgram_server_domain) {
> +	/* clang-format off */
> +	.domain_server = true,
> +	.domain_server_socket = false,
> +	.type = SOCK_DGRAM,
> +};
> +
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(unix_sock_special_cases, allow_stream_server_sock_domain) {
> +	/* clang-format on */
> +	.domain_server = false,
> +	.domain_server_socket = true,
> +	.type = SOCK_STREAM,
> +};
> +
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(unix_sock_special_cases, deny_stream_server_domain) {
> +        /* clang-format off */
> +        .domain_server = true,
> +        .domain_server_socket = false,
> +        .type = SOCK_STREAM,
> +};
> +
> +FIXTURE_SETUP(unix_sock_special_cases)
> +{
> +}
> +
> +FIXTURE_TEARDOWN(unix_sock_special_cases)
> +{
> +	close(self->client);
> +	close(self->server_socket);
> +	close(self->stream_server);
> +	close(self->stream_client);
> +}
> +
> +/* Test UNIX_STREAM_CONNECT and UNIX_MAY_SEND for parent and
> + * child processes when connecting socket has different domain
> + * than the process using it.
> + **/
> +TEST_F(unix_sock_special_cases, dgram_cases)
> +{
> +	pid_t child;
> +	socklen_t addrlen;
> +	int sock_len = 5;
> +	struct sockaddr_un addr, addr_stream = {
> +		.sun_family = AF_UNIX,
> +	};
> +	const char sun_path[8] = "\0test";
> +	const char sun_path_stream[8] = "\0strm";
> +	int err, status;
> +	int pipe_child[2], pipe_parent[2];
> +	char buf_parent;
> +
> +	ASSERT_EQ(0, pipe2(pipe_child, O_CLOEXEC));
> +	ASSERT_EQ(0, pipe2(pipe_parent, O_CLOEXEC));
> +
> +	addrlen = offsetof(struct sockaddr_un, sun_path) + sock_len;
> +	memcpy(&addr.sun_path, sun_path, sock_len);
> +
> +	child = fork();
> +	ASSERT_LE(0, child);
> +	if (child == 0) {
> +		char buf_child;
> +
> +		ASSERT_EQ(0, close(pipe_parent[1]));
> +		ASSERT_EQ(0, close(pipe_child[0]));
> +
> +		/* client always has domain */
> +		create_unix_domain(_metadata);
> +
> +		if (variant->domain_server_socket) {
> +			int data_socket;
> +			int fd_sock = socket(AF_UNIX, variant->type, 0);
> +
> +			ASSERT_NE(-1, fd_sock);
> +
> +			self->stream_server = socket(AF_UNIX, SOCK_STREAM, 0);
> +
> +			ASSERT_NE(-1, self->stream_server);
> +			memcpy(&addr_stream.sun_path, sun_path_stream,
> +			       sock_len);
> +			ASSERT_EQ(0, bind(self->stream_server,
> +					  (struct sockaddr *)&addr_stream,
> +					  addrlen));
> +			ASSERT_EQ(0, listen(self->stream_server, 32));
> +
> +			ASSERT_EQ(1, write(pipe_child[1], ".", 1));
> +
> +			data_socket = accept(self->stream_server, NULL, NULL);
> +
> +			ASSERT_EQ(0, send_fd(data_socket, fd_sock));
> +			ASSERT_EQ(0, close(fd_sock));
> +			TH_LOG("sending completed\n");
> +		}
> +
> +		self->client = socket(AF_UNIX, variant->type, 0);
> +		ASSERT_NE(-1, self->client);
> +		/* wait for parent signal for connection */
> +		ASSERT_EQ(1, read(pipe_parent[0], &buf_child, 1));
> +
> +		err = connect(self->client, (struct sockaddr *)&addr, addrlen);
> +		if (!variant->domain_server_socket) {
> +			EXPECT_EQ(-1, err);
> +			EXPECT_EQ(EPERM, errno);
> +		} else {
> +			EXPECT_EQ(0, err);
> +		}
> +		_exit(_metadata->exit_code);
> +		return;
> +	}
> +
> +	ASSERT_EQ(0, close(pipe_child[1]));
> +	ASSERT_EQ(0, close(pipe_parent[0]));
> +
> +	if (!variant->domain_server_socket) {
> +		self->server_socket = socket(AF_UNIX, variant->type, 0);
> +	} else {
> +		int cli = socket(AF_UNIX, SOCK_STREAM, 0);
> +
> +		ASSERT_NE(-1, cli);
> +		memcpy(&addr_stream.sun_path, sun_path_stream, sock_len);
> +		ASSERT_EQ(1, read(pipe_child[0], &buf_parent, 1));
> +		ASSERT_EQ(0, connect(cli, (struct sockaddr *)&addr_stream,
> +				     addrlen));
> +
> +		self->server_socket = recv_fd(cli);
> +		ASSERT_LE(0, self->server_socket);
> +	}
> +
> +	ASSERT_NE(-1, self->server_socket);
> +
> +	if (variant->domain_server)
> +		create_unix_domain(_metadata);
> +
> +	ASSERT_EQ(0,
> +		  bind(self->server_socket, (struct sockaddr *)&addr, addrlen));
> +	if (variant->type == SOCK_STREAM)
> +		ASSERT_EQ(0, listen(self->server_socket, 32));
> +	/* signal to child that parent is listening */
> +	ASSERT_EQ(1, write(pipe_parent[1], ".", 1));
> +
> +	ASSERT_EQ(child, waitpid(child, &status, 0));
> +
> +	if (WIFSIGNALED(status) || !WIFEXITED(status) ||
> +	    WEXITSTATUS(status) != EXIT_SUCCESS)
> +		_metadata->exit_code = KSFT_FAIL;
> +}
>  TEST_HARNESS_MAIN
> -- 
> 2.34.1
> 
> 

Re: [PATCH v7 1/4] Landlock: Add abstract unix socket connect restriction

[Günther Noack]

On Thu, Jul 25, 2024 at 04:18:29PM +0200, Mickaël Salaün wrote:
> On Wed, Jul 17, 2024 at 10:15:19PM -0600, Tahera Fahimi wrote:
> > diff --git a/security/landlock/syscalls.c b/security/landlock/syscalls.c
> > index 03b470f5a85a..799a50f11d79 100644
> > --- a/security/landlock/syscalls.c
> > +++ b/security/landlock/syscalls.c
> >  /**
> >   * sys_landlock_create_ruleset - Create a new ruleset
> > @@ -170,7 +171,7 @@ static const struct file_operations ruleset_fops = {
> >   * Possible returned errors are:
> >   *
> >   * - %EOPNOTSUPP: Landlock is supported by the kernel but disabled at boot time;
> > - * - %EINVAL: unknown @flags, or unknown access, or too small @size;
> > + * - %EINVAL: unknown @flags, or unknown access, or uknown scope, or too small @size;
> 
> You'll need to rebase on top of my next branch to take into account
> recent Günther's changes.

Actually, I have missed this particular line in my recent documentation changes,
but I agree, we should follow the advice from man-pages(7) consistently -- the
preferred style is to list the same error code multiple times, if there are
multiple possible conditions under which it can be returned.

(Please also fix the typo in "uknown".)

Thanks,
—Günther

Re: [PATCH v7 4/4] documentation/landlock: Adding scoping mechanism documentation

[Mickaël Salaün]

On Wed, Jul 17, 2024 at 10:15:22PM -0600, Tahera Fahimi wrote:
> - Defining ABI version 6 that supports IPC restriction.
> - Adding "scoped" to the "Access rights".
> - In current limitation, unnamed sockets are specified as
>   sockets that are not restricted.
> 
> Signed-off-by: Tahera Fahimi <fahimitahera@gmail.com>
> ---
>  Documentation/userspace-api/landlock.rst | 23 ++++++++++++++++++++---
>  1 file changed, 20 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/userspace-api/landlock.rst b/Documentation/userspace-api/landlock.rst
> index 07b63aec56fa..61b91cc03560 100644
> --- a/Documentation/userspace-api/landlock.rst
> +++ b/Documentation/userspace-api/landlock.rst
> @@ -8,7 +8,7 @@ Landlock: unprivileged access control
>  =====================================
>  
>  :Author: Mickaël Salaün
> -:Date: April 2024
> +:Date: July 2024
>  
>  The goal of Landlock is to enable to restrict ambient rights (e.g. global
>  filesystem or network access) for a set of processes.  Because Landlock
> @@ -306,6 +306,16 @@ To be allowed to use :manpage:`ptrace(2)` and related syscalls on a target
>  process, a sandboxed process should have a subset of the target process rules,
>  which means the tracee must be in a sub-domain of the tracer.
>  
> +IPC Scoping
> +-----------
> +
> +Similar to Ptrace, a sandboxed process should not be able to access the resources
> +(like abstract unix sockets, or signals) outside of the sandbox domain. For example,
> +a sandboxed process should not be able to :manpage:`connect(2)` to a non-sandboxed
> +process through abstract unix sockets (:manpage:`unix(7)`). This restriction is
> +applicable by optionally specifying ``LANDLOCK_SCOPED_ABSTRACT_UNIX_SOCKET`` in
> +the ruleset.
> +
>  Truncating files
>  ----------------
>  
> @@ -404,7 +414,7 @@ Access rights
>  -------------
>  
>  .. kernel-doc:: include/uapi/linux/landlock.h
> -    :identifiers: fs_access net_access
> +    :identifiers: fs_access net_access scoped

If you look at the generated documentation, you'll see that the `Scope
flags` links are broken, and the related section is missing.  This is
because it should not be "scoped" but "scope" here.

With `make htmldocs` you'll also see that there are formating issues in
this (missing) section.

Re: [PATCH v7 1/4] Landlock: Add abstract unix socket connect restriction

[Mickaël Salaün]

On Wed, Jul 17, 2024 at 10:15:19PM -0600, Tahera Fahimi wrote:
> The patch introduces a new "scoped" attribute to the
> landlock_ruleset_attr that can specify "LANDLOCK_SCOPED_ABSTRACT_UNIX_SOCKET"
> to scope abstract unix sockets from connecting to a process outside of
> the same landlock domain.
> 
> This patch implement two hooks, "unix_stream_connect" and "unix_may_send" to
> enforce this restriction.
> 
> Signed-off-by: Tahera Fahimi <fahimitahera@gmail.com>
> 
> -------
> v7:
>  - Using socket's file credentials for both connected(STREAM) and
>    non-connected(DGRAM) sockets.
>  - Adding "domain_sock_scope" instead of the domain scoping mechanism used in
>    ptrace ensures that if a server's domain is accessible from the client's
>    domain (where the client is more privileged than the server), the client
>    can connect to the server in all edge cases.
>  - Removing debug codes.
> v6:
>  - Removing curr_ruleset from landlock_hierarchy, and switching back to use
>    the same domain scoping as ptrace.
>  - code clean up.
> v5:
>  - Renaming "LANDLOCK_*_ACCESS_SCOPE" to "LANDLOCK_*_SCOPE"
>  - Adding curr_ruleset to hierarachy_ruleset structure to have access from
>    landlock_hierarchy to its respective landlock_ruleset.
>  - Using curr_ruleset to check if a domain is scoped while walking in the
>    hierarchy of domains.
>  - Modifying inline comments.
> V4:
>  - Rebased on Günther's Patch:
>    https://lore.kernel.org/all/20240610082115.1693267-1-gnoack@google.com/
>    so there is no need for "LANDLOCK_SHIFT_ACCESS_SCOPE", then it is removed.
>  - Adding get_scope_accesses function to check all scoped access masks in a ruleset.
>  - Using file's FD credentials instead of credentials stored in peer_cred
>    for datagram sockets. (see discussion in [1])
>  - Modifying inline comments.
> V3:
>  - Improving commit description.
>  - Introducing "scoped" attribute to landlock_ruleset_attr for IPC scoping
>    purpose, and adding related functions.
>  - Changing structure of ruleset based on "scoped".
>  - Removing rcu lock and using unix_sk lock instead.
>  - Introducing scoping for datagram sockets in unix_may_send.
> V2:
>  - Removing wrapper functions
> 
> [1]https://lore.kernel.org/outreachy/Zmi8Ydz4Z6tYtpY1@tahera-OptiPlex-5000/T/#m8cdf33180d86c7ec22932e2eb4ef7dd4fc94c792
> -------
> 
> Signed-off-by: Tahera Fahimi <fahimitahera@gmail.com>
> ---
>  include/uapi/linux/landlock.h |  29 +++++++++
>  security/landlock/limits.h    |   3 +
>  security/landlock/ruleset.c   |   7 ++-
>  security/landlock/ruleset.h   |  23 ++++++-
>  security/landlock/syscalls.c  |  14 +++--
>  security/landlock/task.c      | 112 ++++++++++++++++++++++++++++++++++
>  6 files changed, 181 insertions(+), 7 deletions(-)
> 
> diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
> index 68625e728f43..9cd881673434 100644
> --- a/include/uapi/linux/landlock.h
> +++ b/include/uapi/linux/landlock.h
> @@ -37,6 +37,12 @@ struct landlock_ruleset_attr {
>  	 * rule explicitly allow them.
>  	 */
>  	__u64 handled_access_net;
> +	/**
> +	 * @scoped: Bitmask of scopes (cf. `Scope flags`_)
> +	 * restricting a Landlock domain from accessing outside
> +	 * resources(e.g. IPCs).
> +	 */
> +	__u64 scoped;
>  };
>  
>  /*
> @@ -266,4 +272,27 @@ 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: scope
> + *
> + * .scoped attribute handles a set of restrictions on kernel IPCs through
> + * the following flags.

If you look at the generated documentation (once this doc is properly
included), you'll see that this line ends in the Network flags section.

> + *
> + * Scope flags
> + * ~~~~~~~~~~~
> + *
> + * These flags enable to restrict a sandboxed process from a set of IPC
> + * actions. Setting a flag for a ruleset will isolate the Landlock domain
> + * to forbid connections to resources outside the domain.
> + *
> + * IPCs with scoped actions:

There is a formating issue here.

> + * - %LANDLOCK_SCOPED_ABSTRACT_UNIX_SOCKET: Restrict a sandboxed process
> + *   from connecting to an abstract unix socket created by a process
> + *   outside the related Landlock domain (e.g. a parent domain or a
> + *   non-sandboxed process).
> + */
> +/* clang-format off */
> +#define LANDLOCK_SCOPED_ABSTRACT_UNIX_SOCKET		(1ULL << 0)
> +/* clang-format on*/
>  #endif /* _UAPI_LINUX_LANDLOCK_H */

Re: [PATCH v7 1/4] Landlock: Add abstract unix socket connect restriction

[Mickaël Salaün]

On Wed, Jul 17, 2024 at 10:15:19PM -0600, Tahera Fahimi wrote:
> The patch introduces a new "scoped" attribute to the
> landlock_ruleset_attr that can specify "LANDLOCK_SCOPED_ABSTRACT_UNIX_SOCKET"
> to scope abstract unix sockets from connecting to a process outside of
> the same landlock domain.
> 
> This patch implement two hooks, "unix_stream_connect" and "unix_may_send" to
> enforce this restriction.
> 
> Signed-off-by: Tahera Fahimi <fahimitahera@gmail.com>
> 
> -------
> v7:
>  - Using socket's file credentials for both connected(STREAM) and
>    non-connected(DGRAM) sockets.
>  - Adding "domain_sock_scope" instead of the domain scoping mechanism used in
>    ptrace ensures that if a server's domain is accessible from the client's
>    domain (where the client is more privileged than the server), the client
>    can connect to the server in all edge cases.
>  - Removing debug codes.
> v6:
>  - Removing curr_ruleset from landlock_hierarchy, and switching back to use
>    the same domain scoping as ptrace.
>  - code clean up.
> v5:
>  - Renaming "LANDLOCK_*_ACCESS_SCOPE" to "LANDLOCK_*_SCOPE"
>  - Adding curr_ruleset to hierarachy_ruleset structure to have access from
>    landlock_hierarchy to its respective landlock_ruleset.
>  - Using curr_ruleset to check if a domain is scoped while walking in the
>    hierarchy of domains.
>  - Modifying inline comments.
> V4:
>  - Rebased on Günther's Patch:
>    https://lore.kernel.org/all/20240610082115.1693267-1-gnoack@google.com/
>    so there is no need for "LANDLOCK_SHIFT_ACCESS_SCOPE", then it is removed.
>  - Adding get_scope_accesses function to check all scoped access masks in a ruleset.
>  - Using file's FD credentials instead of credentials stored in peer_cred
>    for datagram sockets. (see discussion in [1])
>  - Modifying inline comments.
> V3:
>  - Improving commit description.
>  - Introducing "scoped" attribute to landlock_ruleset_attr for IPC scoping
>    purpose, and adding related functions.
>  - Changing structure of ruleset based on "scoped".
>  - Removing rcu lock and using unix_sk lock instead.
>  - Introducing scoping for datagram sockets in unix_may_send.
> V2:
>  - Removing wrapper functions
> 
> [1]https://lore.kernel.org/outreachy/Zmi8Ydz4Z6tYtpY1@tahera-OptiPlex-5000/T/#m8cdf33180d86c7ec22932e2eb4ef7dd4fc94c792
> -------
> 
> Signed-off-by: Tahera Fahimi <fahimitahera@gmail.com>
> ---
>  include/uapi/linux/landlock.h |  29 +++++++++
>  security/landlock/limits.h    |   3 +
>  security/landlock/ruleset.c   |   7 ++-
>  security/landlock/ruleset.h   |  23 ++++++-
>  security/landlock/syscalls.c  |  14 +++--
>  security/landlock/task.c      | 112 ++++++++++++++++++++++++++++++++++
>  6 files changed, 181 insertions(+), 7 deletions(-)

> diff --git a/security/landlock/task.c b/security/landlock/task.c
> index 849f5123610b..597d89e54aae 100644
> --- a/security/landlock/task.c
> +++ b/security/landlock/task.c

> +static int hook_unix_stream_connect(struct sock *const sock,
> +				    struct sock *const other,
> +				    struct sock *const newsk)
> +{

We must check if the unix sockets use an abstract address.  We need a
new test to check against a the three types of addresse: pathname,
unnamed (socketpair), and abstract.  This should result into 6 variants
because of the stream-oriented or dgram-oriented sockets to check both
hooks.  The test can do 3 checks: without any Landlock domain, with the
client being sandboxed with a Landlock domain for filesystem
restrictions only, and with a domain scoped for abstract unix sockets.

For pathname you'll need to move the TMP_DIR define from fs_test.c into
common.h and create a static path for the named socket.  A fixture
teardown should remove the socket and the directory.

> +	if (sock_is_scoped(other))
> +		return 0;
> +
> +	return -EPERM;
> +}
> +
> +static int hook_unix_may_send(struct socket *const sock,
> +			      struct socket *const other)
> +{

Same here

> +	if (sock_is_scoped(other->sk))
> +		return 0;
> +
> +	return -EPERM;
> +}
> +
>  static struct security_hook_list landlock_hooks[] __ro_after_init = {
>  	LSM_HOOK_INIT(ptrace_access_check, hook_ptrace_access_check),
>  	LSM_HOOK_INIT(ptrace_traceme, hook_ptrace_traceme),
> +	LSM_HOOK_INIT(unix_stream_connect, hook_unix_stream_connect),
> +	LSM_HOOK_INIT(unix_may_send, hook_unix_may_send),

Future access controls dedicated to pathname unix sockets may need these
hooks too, but I guess we'll see where tehy fit the best when we'll be
there.

>  };
>  
>  __init void landlock_add_task_hooks(void)
> -- 
> 2.34.1
> 
> 

Re: [PATCH v7 1/4] Landlock: Add abstract unix socket connect restriction

[Tahera Fahimi]

On Tue, Jul 30, 2024 at 06:05:15PM +0200, Mickaël Salaün wrote:
> On Wed, Jul 17, 2024 at 10:15:19PM -0600, Tahera Fahimi wrote:
> > The patch introduces a new "scoped" attribute to the
> > landlock_ruleset_attr that can specify "LANDLOCK_SCOPED_ABSTRACT_UNIX_SOCKET"
> > to scope abstract unix sockets from connecting to a process outside of
> > the same landlock domain.
> > 
> > This patch implement two hooks, "unix_stream_connect" and "unix_may_send" to
> > enforce this restriction.
> > 
> > Signed-off-by: Tahera Fahimi <fahimitahera@gmail.com>
> > 
> > -------
> > v7:
> >  - Using socket's file credentials for both connected(STREAM) and
> >    non-connected(DGRAM) sockets.
> >  - Adding "domain_sock_scope" instead of the domain scoping mechanism used in
> >    ptrace ensures that if a server's domain is accessible from the client's
> >    domain (where the client is more privileged than the server), the client
> >    can connect to the server in all edge cases.
> >  - Removing debug codes.
> > v6:
> >  - Removing curr_ruleset from landlock_hierarchy, and switching back to use
> >    the same domain scoping as ptrace.
> >  - code clean up.
> > v5:
> >  - Renaming "LANDLOCK_*_ACCESS_SCOPE" to "LANDLOCK_*_SCOPE"
> >  - Adding curr_ruleset to hierarachy_ruleset structure to have access from
> >    landlock_hierarchy to its respective landlock_ruleset.
> >  - Using curr_ruleset to check if a domain is scoped while walking in the
> >    hierarchy of domains.
> >  - Modifying inline comments.
> > V4:
> >  - Rebased on Günther's Patch:
> >    https://lore.kernel.org/all/20240610082115.1693267-1-gnoack@google.com/
> >    so there is no need for "LANDLOCK_SHIFT_ACCESS_SCOPE", then it is removed.
> >  - Adding get_scope_accesses function to check all scoped access masks in a ruleset.
> >  - Using file's FD credentials instead of credentials stored in peer_cred
> >    for datagram sockets. (see discussion in [1])
> >  - Modifying inline comments.
> > V3:
> >  - Improving commit description.
> >  - Introducing "scoped" attribute to landlock_ruleset_attr for IPC scoping
> >    purpose, and adding related functions.
> >  - Changing structure of ruleset based on "scoped".
> >  - Removing rcu lock and using unix_sk lock instead.
> >  - Introducing scoping for datagram sockets in unix_may_send.
> > V2:
> >  - Removing wrapper functions
> > 
> > [1]https://lore.kernel.org/outreachy/Zmi8Ydz4Z6tYtpY1@tahera-OptiPlex-5000/T/#m8cdf33180d86c7ec22932e2eb4ef7dd4fc94c792
> > -------
> > 
> > Signed-off-by: Tahera Fahimi <fahimitahera@gmail.com>
> > ---
> >  include/uapi/linux/landlock.h |  29 +++++++++
> >  security/landlock/limits.h    |   3 +
> >  security/landlock/ruleset.c   |   7 ++-
> >  security/landlock/ruleset.h   |  23 ++++++-
> >  security/landlock/syscalls.c  |  14 +++--
> >  security/landlock/task.c      | 112 ++++++++++++++++++++++++++++++++++
> >  6 files changed, 181 insertions(+), 7 deletions(-)
> 
> > diff --git a/security/landlock/task.c b/security/landlock/task.c
> > index 849f5123610b..597d89e54aae 100644
> > --- a/security/landlock/task.c
> > +++ b/security/landlock/task.c
> 
> > +static int hook_unix_stream_connect(struct sock *const sock,
> > +				    struct sock *const other,
> > +				    struct sock *const newsk)
> > +{
> 
> We must check if the unix sockets use an abstract address.  We need a
> new test to check against a the three types of addresse: pathname,
> unnamed (socketpair), and abstract.  This should result into 6 variants
> because of the stream-oriented or dgram-oriented sockets to check both
> hooks.  The test can do 3 checks: without any Landlock domain, with the
> client being sandboxed with a Landlock domain for filesystem
> restrictions only, and with a domain scoped for abstract unix sockets.
correct. Since the current tests check all the possible scenarios for
abstract unix sockets, we only need to add the scenarios where a socket
with pathname or unnamed address and scoped domain wants to connect to a
listening socket. In this case, the access should guaranteed since there
is no scoping mechanism for these sockets yet.

> For pathname you'll need to move the TMP_DIR define from fs_test.c into
> common.h and create a static path for the named socket.  A fixture
> teardown should remove the socket and the directory.
Thanks for the help :) 
> > +	if (sock_is_scoped(other))
> > +		return 0;
> > +
> > +	return -EPERM;
> > +}
> > +
> > +static int hook_unix_may_send(struct socket *const sock,
> > +			      struct socket *const other)
> > +{
> 
> Same here
> 
> > +	if (sock_is_scoped(other->sk))
> > +		return 0;
> > +
> > +	return -EPERM;
> > +}
> > +
> >  static struct security_hook_list landlock_hooks[] __ro_after_init = {
> >  	LSM_HOOK_INIT(ptrace_access_check, hook_ptrace_access_check),
> >  	LSM_HOOK_INIT(ptrace_traceme, hook_ptrace_traceme),
> > +	LSM_HOOK_INIT(unix_stream_connect, hook_unix_stream_connect),
> > +	LSM_HOOK_INIT(unix_may_send, hook_unix_may_send),
> 
> Future access controls dedicated to pathname unix sockets may need these
> hooks too, but I guess we'll see where tehy fit the best when we'll be
> there.
> 
> >  };
> >  
> >  __init void landlock_add_task_hooks(void)
> > -- 
> > 2.34.1
> > 
> >