[PATCH v4 0/6] landlock: Signal scoping support

[PATCH v4 0/6] landlock: Signal scoping support

[Tahera Fahimi]

This patch series adds scoping mechanism for signals.
Closes: https://github.com/landlock-lsm/linux/issues/8

Problem
=======

A sandboxed process is currently not restricted from sending signals
(e.g. SIGKILL) to processes outside the sandbox since Landlock has no
restriction on signals(see more details in [1]).

A simple way to apply this restriction would be to scope signals the
same way abstract unix sockets are restricted.

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

Solution
========

To solve this issue, we extend the "scoped" field in the Landlock
ruleset attribute structure by introducing "LANDLOCK_SCOPED_SIGNAL"
field to specify that a ruleset will deny sending any signals from
within the sandbox domain to its parent(i.e. any parent sandbox or
non-sandbox processes).

Example
=======

Create a sansboxed shell and pass the character "s" to LL_SCOPED:
LL_FD_RO=/ LL_FS_RW=. LL_SCOPED="s" ./sandboxer /bin/bash
Try to send a signal(like SIGTRAP) to a process ID <PID> through:
kill -SIGTRAP <PID>
The sandboxed process should not be able to send the signal.

Previous Versions
=================
v3:https://lore.kernel.org/all/cover.1723680305.git.fahimitahera@gmail.com/
v2:https://lore.kernel.org/all/cover.1722966592.git.fahimitahera@gmail.com/
v1:https://lore.kernel.org/all/cover.1720203255.git.fahimitahera@gmail.com/

Tahera Fahimi (6):
  landlock: Add signal scoping control
  selftest/landlock: Signal restriction tests
  selftest/landlock: Add signal_scoping_threads test
  selftest/landlock: Test file_send_sigiotask by sending out-of-bound
    message
  sample/landlock: Support sample for signal scoping restriction
  landlock: Document LANDLOCK_SCOPED_SIGNAL

 Documentation/userspace-api/landlock.rst      |  22 +-
 include/uapi/linux/landlock.h                 |   3 +
 samples/landlock/sandboxer.c                  |  17 +-
 security/landlock/fs.c                        |  17 +
 security/landlock/fs.h                        |   6 +
 security/landlock/limits.h                    |   2 +-
 security/landlock/task.c                      |  59 +++
 .../selftests/landlock/scoped_signal_test.c   | 371 ++++++++++++++++++
 .../testing/selftests/landlock/scoped_test.c  |   2 +-
 9 files changed, 486 insertions(+), 13 deletions(-)
 create mode 100644 tools/testing/selftests/landlock/scoped_signal_test.c

-- 
2.34.1

[PATCH v4 1/6] landlock: Add signal scoping control

[Tahera Fahimi]

Currently, a sandbox process is not restricted to sending a signal (e.g.
SIGKILL) to a process outside the sandbox environment. The ability to
send a signal for a sandboxed process should be scoped the same way
abstract UNIX sockets are scoped. Therefore, we extend the "scoped"
field in a ruleset with "LANDLOCK_SCOPED_SIGNAL" to specify that a
ruleset will deny sending any signal from within a sandbox process to
its parent(i.e. any parent sandbox or non-sandboxed procsses).

This patch adds two new hooks, "hook_file_set_fowner" and
"hook_file_free_security", to set and release a pointer to the file
owner's domain. This pointer, "fown_domain" in "landlock_file_security"
will be used in "file_send_sigiotask" to check if the process can send a
signal. Also, it updates the function "ruleset_with_unknown_scope", to
support the scope mask of signal "LANDLOCK_SCOPED_SIGNAL".

Signed-off-by: Tahera Fahimi <fahimitahera@gmail.com>
---
Changes in versions:
V4:
* Merging file_send_sigiotask and task_kill patches into one.
* Commit improvement.
* Applying feedback of V3 on managing fown_domain pointer.
V3:
* Moving file_send_sigiotask to another patch.
* Minor code refactoring.
V2:
* Remove signal_is_scoped function
* Applying reviews of V1
V1:
* Introducing LANDLOCK_SCOPE_SIGNAL
* Adding two hooks, hook_task_kill and hook_file_send_sigiotask for
  signal scoping.
---
 include/uapi/linux/landlock.h                 |  3 +
 security/landlock/fs.c                        | 17 ++++++
 security/landlock/fs.h                        |  6 ++
 security/landlock/limits.h                    |  2 +-
 security/landlock/task.c                      | 59 +++++++++++++++++++
 .../testing/selftests/landlock/scoped_test.c  |  2 +-
 6 files changed, 87 insertions(+), 2 deletions(-)

diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
index dfd48d722834..197da0c5c264 100644
--- a/include/uapi/linux/landlock.h
+++ b/include/uapi/linux/landlock.h
@@ -297,9 +297,12 @@ struct landlock_net_port_attr {
  *   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).
+ * - %LANDLOCK_SCOPED_SIGNAL: Restrict a sandboxed process from sending
+ *   a signal to another process outside sandbox domain.
  */
 /* clang-format off */
 #define LANDLOCK_SCOPED_ABSTRACT_UNIX_SOCKET		(1ULL << 0)
+#define LANDLOCK_SCOPED_SIGNAL		                (1ULL << 1)
 /* clang-format on*/
 
 #endif /* _UAPI_LINUX_LANDLOCK_H */
diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index 7877a64cc6b8..b1207f0a8cd4 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -1636,6 +1636,20 @@ static int hook_file_ioctl_compat(struct file *file, unsigned int cmd,
 	return -EACCES;
 }
 
+static void hook_file_set_fowner(struct file *file)
+{
+	write_lock_irq(&file->f_owner.lock);
+	landlock_put_ruleset_deferred(landlock_file(file)->fown_domain);
+	landlock_file(file)->fown_domain = landlock_get_current_domain();
+	landlock_get_ruleset(landlock_file(file)->fown_domain);
+	write_unlock_irq(&file->f_owner.lock);
+}
+
+static void hook_file_free_security(struct file *file)
+{
+	landlock_put_ruleset_deferred(landlock_file(file)->fown_domain);
+}
+
 static struct security_hook_list landlock_hooks[] __ro_after_init = {
 	LSM_HOOK_INIT(inode_free_security, hook_inode_free_security),
 
@@ -1660,6 +1674,9 @@ static struct security_hook_list landlock_hooks[] __ro_after_init = {
 	LSM_HOOK_INIT(file_truncate, hook_file_truncate),
 	LSM_HOOK_INIT(file_ioctl, hook_file_ioctl),
 	LSM_HOOK_INIT(file_ioctl_compat, hook_file_ioctl_compat),
+
+	LSM_HOOK_INIT(file_set_fowner, hook_file_set_fowner),
+	LSM_HOOK_INIT(file_free_security, hook_file_free_security),
 };
 
 __init void landlock_add_fs_hooks(void)
diff --git a/security/landlock/fs.h b/security/landlock/fs.h
index 488e4813680a..9a97f9285b90 100644
--- a/security/landlock/fs.h
+++ b/security/landlock/fs.h
@@ -52,6 +52,12 @@ struct landlock_file_security {
 	 * needed to authorize later operations on the open file.
 	 */
 	access_mask_t allowed_access;
+	/**
+	 * @fown_domain: A pointer to a &landlock_ruleset of the process owns
+	 * the file. This ruleset is protected by fowner_struct.lock same as
+	 * pid, uid, euid fields in fown_struct.
+	 */
+	struct landlock_ruleset *fown_domain;
 };
 
 /**
diff --git a/security/landlock/limits.h b/security/landlock/limits.h
index eb01d0fb2165..fa28f9236407 100644
--- a/security/landlock/limits.h
+++ b/security/landlock/limits.h
@@ -26,7 +26,7 @@
 #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_LAST_SCOPE		LANDLOCK_SCOPED_SIGNAL
 #define LANDLOCK_MASK_SCOPE		((LANDLOCK_LAST_SCOPE << 1) - 1)
 #define LANDLOCK_NUM_SCOPE		__const_hweight64(LANDLOCK_MASK_SCOPE)
 /* clang-format on */
diff --git a/security/landlock/task.c b/security/landlock/task.c
index b9390445d242..a72a61e7e6c3 100644
--- a/security/landlock/task.c
+++ b/security/landlock/task.c
@@ -18,6 +18,7 @@
 
 #include "common.h"
 #include "cred.h"
+#include "fs.h"
 #include "ruleset.h"
 #include "setup.h"
 #include "task.h"
@@ -242,11 +243,69 @@ static int hook_unix_may_send(struct socket *const sock,
 	return 0;
 }
 
+static int hook_task_kill(struct task_struct *const p,
+			  struct kernel_siginfo *const info, const int sig,
+			  const struct cred *const cred)
+{
+	bool is_scoped;
+	const struct landlock_ruleset *target_dom, *dom;
+
+	dom = landlock_get_current_domain();
+	rcu_read_lock();
+	target_dom = landlock_get_task_domain(p);
+	if (cred)
+		/* dealing with USB IO */
+		is_scoped = domain_is_scoped(landlock_cred(cred)->domain,
+					     target_dom,
+					     LANDLOCK_SCOPED_SIGNAL);
+	else
+		is_scoped = (!dom) ? false :
+				     domain_is_scoped(dom, target_dom,
+						      LANDLOCK_SCOPED_SIGNAL);
+	rcu_read_unlock();
+	if (is_scoped)
+		return -EPERM;
+
+	return 0;
+}
+
+static int hook_file_send_sigiotask(struct task_struct *tsk,
+				    struct fown_struct *fown, int signum)
+{
+	struct file *file;
+	bool is_scoped;
+	struct landlock_ruleset *dom;
+
+	/* struct fown_struct is never outside the context of a struct file */
+	file = container_of(fown, struct file, f_owner);
+
+	read_lock_irq(&file->f_owner.lock);
+	dom = landlock_file(file)->fown_domain;
+	landlock_get_ruleset(dom);
+	read_unlock_irq(&file->f_owner.lock);
+	if (!dom)
+		goto out_unlock;
+
+	rcu_read_lock();
+	is_scoped = domain_is_scoped(dom, landlock_get_task_domain(tsk),
+				     LANDLOCK_SCOPED_SIGNAL);
+	rcu_read_unlock();
+	if (is_scoped) {
+		landlock_put_ruleset(dom);
+		return -EPERM;
+	}
+out_unlock:
+	landlock_put_ruleset(dom);
+	return 0;
+}
+
 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),
+	LSM_HOOK_INIT(task_kill, hook_task_kill),
+	LSM_HOOK_INIT(file_send_sigiotask, hook_file_send_sigiotask),
 };
 
 __init void landlock_add_task_hooks(void)
diff --git a/tools/testing/selftests/landlock/scoped_test.c b/tools/testing/selftests/landlock/scoped_test.c
index 36d7266de9dc..237f98369b25 100644
--- a/tools/testing/selftests/landlock/scoped_test.c
+++ b/tools/testing/selftests/landlock/scoped_test.c
@@ -12,7 +12,7 @@
 
 #include "common.h"
 
-#define ACCESS_LAST LANDLOCK_SCOPED_ABSTRACT_UNIX_SOCKET
+#define ACCESS_LAST LANDLOCK_SCOPED_SIGNAL
 
 TEST(ruleset_with_unknown_scope)
 {
-- 
2.34.1

[PATCH v4 2/6] selftest/landlock: Signal restriction tests

[Tahera Fahimi]

This patch expands Landlock ABI version 6 by providing tests for signal
scoping mechanism. Base on kill(2), if the signal is 0, no signal will
be sent, but the permission of a process to send a signal will be
checked. Likewise, this test consider one signal for each signal
category (SIGTRAP, SIGURG, SIGHUP, and SIGTSTP).

Signed-off-by: Tahera Fahimi <fahimitahera@gmail.com>
---
Changes in versions:
V4:
* Refactoring by providing two sets of tests, send_sig_to_parent to
  check simple case of sending signal with scoped and non-scoped
  domains, and check_access_signal to examine access to send a signal
  with various domains.
V3:
* Using generalized scoped domain creation "create_scoped_domain"
V2:
* Moving tests from ptrace_test.c to scoped_signal_test.c
* Remove debugging statements.
* Covering all basic restriction scenarios by sending 0 as signal
V1:
* Expanding Landlock ABI version 6 by providing basic tests for
  four signals to test signal scoping mechanism.
---
 .../selftests/landlock/scoped_signal_test.c   | 225 ++++++++++++++++++
 1 file changed, 225 insertions(+)
 create mode 100644 tools/testing/selftests/landlock/scoped_signal_test.c

diff --git a/tools/testing/selftests/landlock/scoped_signal_test.c b/tools/testing/selftests/landlock/scoped_signal_test.c
new file mode 100644
index 000000000000..8df027e22324
--- /dev/null
+++ b/tools/testing/selftests/landlock/scoped_signal_test.c
@@ -0,0 +1,225 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Landlock tests - Signal Scoping
+ *
+ * Copyright © 2024 Tahera Fahimi <fahimitahera@gmail.com>
+ */
+
+#define _GNU_SOURCE
+#include <errno.h>
+#include <fcntl.h>
+#include <linux/landlock.h>
+#include <signal.h>
+#include <sys/prctl.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+#include <unistd.h>
+
+#include "common.h"
+#include "scoped_common.h"
+
+/* This variable is used for handeling several signals. */
+static volatile sig_atomic_t is_signaled;
+
+/* clang-format off */
+FIXTURE(scoping_signals) {};
+/* clang-format on */
+
+FIXTURE_VARIANT(scoping_signals)
+{
+	int sig;
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(scoping_signals, sigtrap) {
+	/* clang-format on */
+	.sig = SIGTRAP,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(scoping_signals, sigurg) {
+	/* clang-format on */
+	.sig = SIGURG,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(scoping_signals, sighup) {
+	/* clang-format on */
+	.sig = SIGHUP,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(scoping_signals, sigtstp) {
+	/* clang-format on */
+	.sig = SIGTSTP,
+};
+
+FIXTURE_SETUP(scoping_signals)
+{
+	is_signaled = 0;
+}
+
+FIXTURE_TEARDOWN(scoping_signals)
+{
+}
+
+static void scope_signal_handler(int sig, siginfo_t *info, void *ucontext)
+{
+	if (sig == SIGTRAP || sig == SIGURG || sig == SIGHUP || sig == SIGTSTP)
+		is_signaled = 1;
+}
+
+/*
+ * In this test, a child process sends a signal to parent before and
+ * after getting scoped.
+ */
+TEST_F(scoping_signals, send_sig_to_parent)
+{
+	pid_t child;
+	pid_t parent = getpid();
+	int status;
+	struct sigaction action = {
+		.sa_sigaction = scope_signal_handler,
+		.sa_flags = SA_SIGINFO,
+
+	};
+
+	ASSERT_LE(0, sigaction(variant->sig, &action, NULL));
+
+	/* The process should not have already been signaled. */
+	EXPECT_EQ(0, is_signaled);
+
+	child = fork();
+	ASSERT_LE(0, child);
+	if (child == 0) {
+		int err;
+
+		/*
+		 * The child process can send signal to parent when
+		 * domain is not scoped.
+		 */
+		err = kill(parent, variant->sig);
+		ASSERT_EQ(0, err);
+
+		create_scoped_domain(_metadata, LANDLOCK_SCOPED_SIGNAL);
+
+		/*
+		 * The child process cannot send signal to the parent
+		 * anymore.
+		 */
+		err = kill(parent, variant->sig);
+		ASSERT_EQ(-1, err);
+		ASSERT_EQ(EPERM, errno);
+
+		/*
+		 * No matter of the domain, a process should be able to
+		 * send a signal to itself.
+		 */
+		ASSERT_EQ(0, is_signaled);
+		ASSERT_EQ(0, raise(variant->sig));
+		ASSERT_EQ(1, is_signaled);
+
+		_exit(_metadata->exit_code);
+		return;
+	}
+
+	while (!is_signaled && !usleep(1))
+		;
+	ASSERT_EQ(1, is_signaled);
+
+	ASSERT_EQ(child, waitpid(child, &status, 0));
+
+	if (WIFSIGNALED(status) || !WIFEXITED(status) ||
+	    WEXITSTATUS(status) != EXIT_SUCCESS)
+		_metadata->exit_code = KSFT_FAIL;
+}
+
+/* clang-format off */
+FIXTURE(scoped_domains) {};
+/* clang-format on */
+
+#include "scoped_base_variants.h"
+
+FIXTURE_SETUP(scoped_domains)
+{
+}
+
+FIXTURE_TEARDOWN(scoped_domains)
+{
+}
+
+/*
+ * This test ensures that a scoped process cannot send signal out of
+ * scoped domain.
+ */
+TEST_F(scoped_domains, check_access_signal)
+{
+	pid_t child;
+	pid_t parent = getpid();
+	int status;
+	bool can_signal_child, can_signal_parent;
+	int pipe_parent[2], pipe_child[2];
+	int err;
+	char buf;
+
+	can_signal_parent = !variant->domain_child;
+	can_signal_child = !variant->domain_parent;
+
+	if (variant->domain_both)
+		create_scoped_domain(_metadata, LANDLOCK_SCOPED_SIGNAL);
+	ASSERT_EQ(0, pipe2(pipe_parent, O_CLOEXEC));
+	ASSERT_EQ(0, pipe2(pipe_child, O_CLOEXEC));
+
+	child = fork();
+	ASSERT_LE(0, child);
+	if (child == 0) {
+		ASSERT_EQ(0, close(pipe_child[0]));
+		ASSERT_EQ(0, close(pipe_parent[1]));
+
+		if (variant->domain_child)
+			create_scoped_domain(_metadata, LANDLOCK_SCOPED_SIGNAL);
+
+		ASSERT_EQ(1, write(pipe_child[1], ".", 1));
+
+		/* Waits for the parent to send signals. */
+		ASSERT_EQ(1, read(pipe_parent[0], &buf, 1));
+
+		err = kill(parent, 0);
+		if (can_signal_parent) {
+			ASSERT_EQ(0, err);
+		} else {
+			ASSERT_EQ(-1, err);
+			ASSERT_EQ(EPERM, errno);
+		}
+		/*
+		 * No matter of the domain, a process should be able to
+		 * send a signal to itself.
+		 */
+		ASSERT_EQ(0, raise(0));
+
+		_exit(_metadata->exit_code);
+		return;
+	}
+	ASSERT_EQ(0, close(pipe_parent[0]));
+	if (variant->domain_parent)
+		create_scoped_domain(_metadata, LANDLOCK_SCOPED_SIGNAL);
+
+	ASSERT_EQ(1, read(pipe_child[0], &buf, 1));
+
+	err = kill(child, 0);
+	if (can_signal_child) {
+		ASSERT_EQ(0, err);
+	} else {
+		ASSERT_EQ(-1, err);
+		ASSERT_EQ(EPERM, errno);
+	}
+	ASSERT_EQ(0, raise(0));
+
+	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 v4 3/6] selftest/landlock: Add signal_scoping_threads test

[Tahera Fahimi]

This patch expands the signal scoping tests with pthread_kill(3) It
tests if an scoped thread can send signal to a process in the same
scoped domain, or a non-sandboxed thread.

Signed-off-by: Tahera Fahimi <fahimitahera@gmail.com>
---
V4:
* Code improvement, and removing sleep(3) from threads.
* Commit improvement.
---
 .../selftests/landlock/scoped_signal_test.c   | 47 +++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/tools/testing/selftests/landlock/scoped_signal_test.c b/tools/testing/selftests/landlock/scoped_signal_test.c
index 8df027e22324..c71fb83b7147 100644
--- a/tools/testing/selftests/landlock/scoped_signal_test.c
+++ b/tools/testing/selftests/landlock/scoped_signal_test.c
@@ -9,6 +9,7 @@
 #include <errno.h>
 #include <fcntl.h>
 #include <linux/landlock.h>
+#include <pthread.h>
 #include <signal.h>
 #include <sys/prctl.h>
 #include <sys/types.h>
@@ -222,4 +223,50 @@ TEST_F(scoped_domains, check_access_signal)
 		_metadata->exit_code = KSFT_FAIL;
 }
 
+static int thread_pipe[2];
+
+enum thread_return {
+	THREAD_INVALID = 0,
+	THREAD_SUCCESS = 1,
+	THREAD_ERROR = 2,
+};
+
+void *thread_func(void *arg)
+{
+	char buf;
+
+	if (read(thread_pipe[0], &buf, 1) != 1)
+		return (void *)THREAD_ERROR;
+
+	return (void *)THREAD_SUCCESS;
+}
+
+TEST(signal_scoping_threads)
+{
+	pthread_t no_sandbox_thread, scoped_thread;
+	enum thread_return ret = THREAD_INVALID;
+
+	ASSERT_EQ(0, pipe2(thread_pipe, O_CLOEXEC));
+
+	ASSERT_EQ(0,
+		  pthread_create(&no_sandbox_thread, NULL, thread_func, NULL));
+	/* Restrict the domain after creating the first thread. */
+	create_scoped_domain(_metadata, LANDLOCK_SCOPED_SIGNAL);
+
+	ASSERT_EQ(EPERM, pthread_kill(no_sandbox_thread, 0));
+	ASSERT_EQ(1, write(thread_pipe[1], ".", 1));
+
+	ASSERT_EQ(0, pthread_create(&scoped_thread, NULL, thread_func, NULL));
+	ASSERT_EQ(0, pthread_kill(scoped_thread, 0));
+	ASSERT_EQ(1, write(thread_pipe[1], ".", 1));
+
+	EXPECT_EQ(0, pthread_join(no_sandbox_thread, (void **)&ret));
+	EXPECT_EQ(THREAD_SUCCESS, ret);
+	EXPECT_EQ(0, pthread_join(scoped_thread, (void **)&ret));
+	EXPECT_EQ(THREAD_SUCCESS, ret);
+
+	EXPECT_EQ(0, close(thread_pipe[0]));
+	EXPECT_EQ(0, close(thread_pipe[1]));
+}
+
 TEST_HARNESS_MAIN
-- 
2.34.1

[PATCH v4 4/6] selftest/landlock: Test file_send_sigiotask by sending out-of-bound message

[Tahera Fahimi]

This patch adds a test to verify handling the signal scoping mechanism
in file_send_sigiotask by triggering SIGURG through receiving an
out-of-bound message in UNIX sockets.

Signed-off-by: Tahera Fahimi <fahimitahera@gmail.com>
---
V4:
* Using pipe instead of Poll for synchronization.
---
 .../selftests/landlock/scoped_signal_test.c   | 99 +++++++++++++++++++
 1 file changed, 99 insertions(+)

diff --git a/tools/testing/selftests/landlock/scoped_signal_test.c b/tools/testing/selftests/landlock/scoped_signal_test.c
index c71fb83b7147..630f3a515731 100644
--- a/tools/testing/selftests/landlock/scoped_signal_test.c
+++ b/tools/testing/selftests/landlock/scoped_signal_test.c
@@ -269,4 +269,103 @@ TEST(signal_scoping_threads)
 	EXPECT_EQ(0, close(thread_pipe[1]));
 }
 
+#define SOCKET_PATH "/tmp/unix_sock_test"
+
+const short backlog = 10;
+
+static volatile sig_atomic_t signal_received;
+
+static void handle_sigurg(int sig)
+{
+	if (sig == SIGURG)
+		signal_received = 1;
+	else
+		signal_received = -1;
+}
+
+static int setup_signal_handler(int signal)
+{
+	struct sigaction sa;
+
+	sa.sa_handler = handle_sigurg;
+	sigemptyset(&sa.sa_mask);
+	sa.sa_flags = SA_SIGINFO | SA_RESTART;
+	return sigaction(SIGURG, &sa, NULL);
+}
+
+/*
+ * Sending an out of bound message will trigger the SIGURG signal
+ * through file_send_sigiotask.
+ */
+TEST(test_sigurg_socket)
+{
+	int sock_fd, recv_sock;
+	struct sockaddr_un addr, paddr;
+	socklen_t size;
+	char oob_buf, buffer;
+	int status;
+	int pipe_parent[2], pipe_child[2];
+	pid_t child;
+
+	ASSERT_EQ(0, pipe2(pipe_parent, O_CLOEXEC));
+	ASSERT_EQ(0, pipe2(pipe_child, O_CLOEXEC));
+
+	memset(&addr, 0, sizeof(addr));
+	addr.sun_family = AF_UNIX;
+	snprintf(addr.sun_path, sizeof(addr.sun_path), "%s", SOCKET_PATH);
+	unlink(SOCKET_PATH);
+	size = sizeof(addr);
+
+	child = fork();
+	ASSERT_LE(0, child);
+	if (child == 0) {
+		oob_buf = '.';
+
+		ASSERT_EQ(0, close(pipe_parent[1]));
+		ASSERT_EQ(0, close(pipe_child[0]));
+
+		sock_fd = socket(AF_UNIX, SOCK_STREAM, 0);
+		ASSERT_NE(-1, sock_fd);
+
+		ASSERT_EQ(1, read(pipe_parent[0], &buffer, 1));
+		ASSERT_EQ(0, connect(sock_fd, &addr, sizeof(addr)));
+
+		ASSERT_EQ(1, read(pipe_parent[0], &buffer, 1));
+		ASSERT_NE(-1, send(sock_fd, &oob_buf, 1, MSG_OOB));
+		ASSERT_EQ(1, write(pipe_child[1], ".", 1));
+
+		EXPECT_EQ(0, close(sock_fd));
+
+		_exit(_metadata->exit_code);
+		return;
+	}
+	ASSERT_EQ(0, close(pipe_parent[0]));
+	ASSERT_EQ(0, close(pipe_child[1]));
+
+	sock_fd = socket(AF_UNIX, SOCK_STREAM, 0);
+	ASSERT_NE(-1, sock_fd);
+	ASSERT_EQ(0, bind(sock_fd, &addr, size));
+	ASSERT_EQ(0, listen(sock_fd, backlog));
+
+	ASSERT_NE(-1, setup_signal_handler(SIGURG));
+	ASSERT_EQ(1, write(pipe_parent[1], ".", 1));
+	recv_sock = accept(sock_fd, &paddr, &size);
+	ASSERT_NE(-1, recv_sock);
+
+	create_scoped_domain(_metadata, LANDLOCK_SCOPED_SIGNAL);
+
+	ASSERT_NE(-1, fcntl(recv_sock, F_SETOWN, getpid()));
+	ASSERT_EQ(1, write(pipe_parent[1], ".", 1));
+	ASSERT_EQ(1, read(pipe_child[0], &buffer, 1));
+	ASSERT_EQ(1, recv(recv_sock, &oob_buf, 1, MSG_OOB));
+
+	ASSERT_EQ(1, signal_received);
+	EXPECT_EQ(0, close(sock_fd));
+	EXPECT_EQ(0, close(recv_sock));
+	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 v4 5/6] sample/landlock: Support sample for signal scoping restriction

[Tahera Fahimi]

A sandboxer can receive the character "s" as input from the environment
variable LL_SCOPE to restrict itself from sending a signal to a process
outside its scoped domain.

Example
=======
Create a sandboxed shell and pass the character "s" to LL_SCOPED:
LL_FS_RO=/ LL_FS_RW=. LL_SCOPED="s" ./sandboxer /bin/bash
Try to send a SIGTRAP to a process with process ID <PID> through:
kill -SIGTRAP <PID>
The sandboxed process should not be able to send the signal.

Signed-off-by: Tahera Fahimi <fahimitahera@gmail.com>
---
v4:
- Make it compatible with changes in abstract UNIX socket scoping sample
v3:
- Add a restrict approach on input of LL_SCOPED, so it only allows
  zero or one "s" to be the input.
---
 samples/landlock/sandboxer.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/samples/landlock/sandboxer.c b/samples/landlock/sandboxer.c
index 18d072c23a23..618fbf70d38f 100644
--- a/samples/landlock/sandboxer.c
+++ b/samples/landlock/sandboxer.c
@@ -191,11 +191,13 @@ static bool check_ruleset_scope(const char *const env_var,
 				struct landlock_ruleset_attr *ruleset_attr)
 {
 	bool abstract_scoping = false;
+	bool signal_scoping = false;
 	bool ret = true;
 	char *env_type_scope, *env_type_scope_next, *ipc_scoping_name;
 
 	/* scoping is not supported by Landlock ABI */
-	if (!(ruleset_attr->scoped & LANDLOCK_SCOPED_ABSTRACT_UNIX_SOCKET))
+	if (!(ruleset_attr->scoped &
+	      (LANDLOCK_SCOPED_ABSTRACT_UNIX_SOCKET | LANDLOCK_SCOPED_SIGNAL)))
 		return ret;
 
 	env_type_scope = getenv(env_var);
@@ -212,6 +214,9 @@ static bool check_ruleset_scope(const char *const env_var,
 			strsep(&env_type_scope_next, ENV_DELIMITER))) {
 		if (strcmp("a", ipc_scoping_name) == 0 && !abstract_scoping) {
 			abstract_scoping = true;
+		} else if (strcmp("s", ipc_scoping_name) == 0 &&
+			   !signal_scoping) {
+			signal_scoping = true;
 		} else {
 			fprintf(stderr, "Unsupported scoping \"%s\"\n",
 				ipc_scoping_name);
@@ -221,6 +226,8 @@ static bool check_ruleset_scope(const char *const env_var,
 	}
 	if (!abstract_scoping)
 		ruleset_attr->scoped &= ~LANDLOCK_SCOPED_ABSTRACT_UNIX_SOCKET;
+	if (!signal_scoping)
+		ruleset_attr->scoped &= ~LANDLOCK_SCOPED_SIGNAL;
 out_free_name:
 	free(env_type_scope);
 	return ret;
@@ -265,7 +272,8 @@ 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,
+		.scoped = LANDLOCK_SCOPED_ABSTRACT_UNIX_SOCKET |
+			  LANDLOCK_SCOPED_SIGNAL,
 	};
 
 	if (argc < 2) {
@@ -302,7 +310,7 @@ int main(const int argc, char *const argv[], char *const *const envp)
 			"%s=\"/dev/null:/dev/full:/dev/zero:/dev/pts:/tmp\" "
 			"%s=\"9418\" "
 			"%s=\"80:443\" "
-			"%s=\"a\" "
+			"%s=\"a:s\" "
 			"%s bash -i\n\n",
 			ENV_FS_RO_NAME, ENV_FS_RW_NAME, ENV_TCP_BIND_NAME,
 			ENV_TCP_CONNECT_NAME, ENV_SCOPED_NAME, argv[0]);
@@ -376,7 +384,8 @@ int main(const int argc, char *const argv[], char *const *const envp)
 		__attribute__((fallthrough));
 	case 5:
 		/* Removes LANDLOCK_SCOPED_ABSTRACT_UNIX_SOCKET for ABI < 6 */
-		ruleset_attr.scoped &= ~LANDLOCK_SCOPED_ABSTRACT_UNIX_SOCKET;
+		ruleset_attr.scoped &= ~(LANDLOCK_SCOPED_ABSTRACT_UNIX_SOCKET |
+					 LANDLOCK_SCOPED_SIGNAL);
 		fprintf(stderr,
 			"Hint: You should update the running kernel "
 			"to leverage Landlock features "
-- 
2.34.1

[PATCH v4 6/6] landlock: Document LANDLOCK_SCOPED_SIGNAL

[Tahera Fahimi]

Improving Landlock ABI version 6 to support signal scoping with
LANDLOCK_SCOPED_SIGNAL.

Signed-off-by: Tahera Fahimi <fahimitahera@gmail.com>
---
v3:
- update date
---
 Documentation/userspace-api/landlock.rst | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/Documentation/userspace-api/landlock.rst b/Documentation/userspace-api/landlock.rst
index c3b87755e98d..c694e9fe36fc 100644
--- a/Documentation/userspace-api/landlock.rst
+++ b/Documentation/userspace-api/landlock.rst
@@ -82,7 +82,8 @@ to be explicit about the denied-by-default access rights.
             LANDLOCK_ACCESS_NET_BIND_TCP |
             LANDLOCK_ACCESS_NET_CONNECT_TCP,
         .scoped =
-            LANDLOCK_SCOPED_ABSTRACT_UNIX_SOCKET,
+            LANDLOCK_SCOPED_ABSTRACT_UNIX_SOCKET |
+            LANDLOCK_SCOPED_SIGNAL,
     };
 
 Because we may not know on which kernel version an application will be
@@ -123,7 +124,8 @@ version, and only use the available subset of access rights:
         ruleset_attr.handled_access_fs &= ~LANDLOCK_ACCESS_FS_IOCTL_DEV;
     case 5:
         /* Removes LANDLOCK_SCOPED_ABSTRACT_UNIX_SOCKET for ABI < 6 */
-        ruleset_attr.scoped &= ~LANDLOCK_SCOPED_ABSTRACT_UNIX_SOCKET;
+        ruleset_attr.scoped &= ~(LANDLOCK_SCOPED_ABSTRACT_UNIX_SOCKET |
+                                 LANDLOCK_SCOPED_SIGNAL);
     }
 
 This enables to create an inclusive ruleset that will contain our rules.
@@ -320,11 +322,15 @@ 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``.
+``LANDLOCK_SCOPED_ABSTRACT_UNIX_SOCKET``. Moreover, if a sandboxed
+process should not be able to send a signal to a non-sandboxed process,
+we can specify this restriction with ``LANDLOCK_SCOPED_SIGNAL``.
 
 A sandboxed process can connect to a non-sandboxed process when its
 domain is not scoped. If a process's domain is scoped, it can only
 connect to sockets created by processes in the same scoped domain.
+Moreover, If a process is scoped to send signal to a non-scoped process,
+it can only send signals to processes in the same scoped domain.
 
 A connected datagram socket behaves like a stream socket when its domain
 is scoped, meaning if the domain is scoped after the socket is connected
@@ -575,12 +581,14 @@ 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.
 
-Abstract UNIX sockets Restriction  (ABI < 6)
---------------------------------------------
+Abstract Unix sockets and Signal Restriction  (ABI < 6)
+-------------------------------------------------------
 
+<<<<<<< current
 With ABI version 6, it is possible to restrict connection to an abstract
-Unix socket through ``LANDLOCK_SCOPED_ABSTRACT_UNIX_SOCKET``, thanks to
-the ``scoped`` ruleset attribute.
+:manpage:`unix(7)` socket through
+``LANDLOCK_SCOPED_ABSTRACT_UNIX_SOCKET`` and sending signal through
+``LANDLOCK_SCOPED_SIGNAL``, thanks to the ``scoped`` ruleset attribute.
 
 .. _kernel_support:
 
-- 
2.34.1

Re: [PATCH v4 4/6] selftest/landlock: Test file_send_sigiotask by sending out-of-bound message

[Mickaël Salaün]

This test does not cover hook_file_send_sigiotask(): the is_scoped
variable is never set to true.

On Fri, Sep 06, 2024 at 03:30:06PM -0600, Tahera Fahimi wrote:
> This patch adds a test to verify handling the signal scoping mechanism
> in file_send_sigiotask by triggering SIGURG through receiving an
> out-of-bound message in UNIX sockets.
> 
> Signed-off-by: Tahera Fahimi <fahimitahera@gmail.com>
> ---
> V4:
> * Using pipe instead of Poll for synchronization.
> ---
>  .../selftests/landlock/scoped_signal_test.c   | 99 +++++++++++++++++++
>  1 file changed, 99 insertions(+)
> 
> diff --git a/tools/testing/selftests/landlock/scoped_signal_test.c b/tools/testing/selftests/landlock/scoped_signal_test.c
> index c71fb83b7147..630f3a515731 100644
> --- a/tools/testing/selftests/landlock/scoped_signal_test.c
> +++ b/tools/testing/selftests/landlock/scoped_signal_test.c
> @@ -269,4 +269,103 @@ TEST(signal_scoping_threads)
>  	EXPECT_EQ(0, close(thread_pipe[1]));
>  }
>  
> +#define SOCKET_PATH "/tmp/unix_sock_test"

We must not create file on absolute paths because concurrent executions
or previous ones could interfer with the tests.  Why not use an abstract
unix socket created with set_unix_address()?

> +
> +const short backlog = 10;
> +
> +static volatile sig_atomic_t signal_received;
> +
> +static void handle_sigurg(int sig)
> +{
> +	if (sig == SIGURG)
> +		signal_received = 1;
> +	else
> +		signal_received = -1;
> +}
> +
> +static int setup_signal_handler(int signal)
> +{
> +	struct sigaction sa;
> +
> +	sa.sa_handler = handle_sigurg;
> +	sigemptyset(&sa.sa_mask);
> +	sa.sa_flags = SA_SIGINFO | SA_RESTART;
> +	return sigaction(SIGURG, &sa, NULL);
> +}
> +
> +/*
> + * Sending an out of bound message will trigger the SIGURG signal
> + * through file_send_sigiotask.
> + */
> +TEST(test_sigurg_socket)
> +{
> +	int sock_fd, recv_sock;
> +	struct sockaddr_un addr, paddr;
> +	socklen_t size;
> +	char oob_buf, buffer;
> +	int status;
> +	int pipe_parent[2], pipe_child[2];
> +	pid_t child;
> +
> +	ASSERT_EQ(0, pipe2(pipe_parent, O_CLOEXEC));
> +	ASSERT_EQ(0, pipe2(pipe_child, O_CLOEXEC));
> +
> +	memset(&addr, 0, sizeof(addr));
> +	addr.sun_family = AF_UNIX;
> +	snprintf(addr.sun_path, sizeof(addr.sun_path), "%s", SOCKET_PATH);
> +	unlink(SOCKET_PATH);
> +	size = sizeof(addr);
> +
> +	child = fork();
> +	ASSERT_LE(0, child);
> +	if (child == 0) {
> +		oob_buf = '.';
> +
> +		ASSERT_EQ(0, close(pipe_parent[1]));
> +		ASSERT_EQ(0, close(pipe_child[0]));
> +
> +		sock_fd = socket(AF_UNIX, SOCK_STREAM, 0);
> +		ASSERT_NE(-1, sock_fd);
> +
> +		ASSERT_EQ(1, read(pipe_parent[0], &buffer, 1));
> +		ASSERT_EQ(0, connect(sock_fd, &addr, sizeof(addr)));
> +
> +		ASSERT_EQ(1, read(pipe_parent[0], &buffer, 1));
> +		ASSERT_NE(-1, send(sock_fd, &oob_buf, 1, MSG_OOB));
> +		ASSERT_EQ(1, write(pipe_child[1], ".", 1));
> +
> +		EXPECT_EQ(0, close(sock_fd));
> +
> +		_exit(_metadata->exit_code);
> +		return;
> +	}
> +	ASSERT_EQ(0, close(pipe_parent[0]));
> +	ASSERT_EQ(0, close(pipe_child[1]));
> +
> +	sock_fd = socket(AF_UNIX, SOCK_STREAM, 0);
> +	ASSERT_NE(-1, sock_fd);
> +	ASSERT_EQ(0, bind(sock_fd, &addr, size));
> +	ASSERT_EQ(0, listen(sock_fd, backlog));
> +
> +	ASSERT_NE(-1, setup_signal_handler(SIGURG));
> +	ASSERT_EQ(1, write(pipe_parent[1], ".", 1));
> +	recv_sock = accept(sock_fd, &paddr, &size);
> +	ASSERT_NE(-1, recv_sock);
> +
> +	create_scoped_domain(_metadata, LANDLOCK_SCOPED_SIGNAL);
> +
> +	ASSERT_NE(-1, fcntl(recv_sock, F_SETOWN, getpid()));
> +	ASSERT_EQ(1, write(pipe_parent[1], ".", 1));
> +	ASSERT_EQ(1, read(pipe_child[0], &buffer, 1));
> +	ASSERT_EQ(1, recv(recv_sock, &oob_buf, 1, MSG_OOB));
> +
> +	ASSERT_EQ(1, signal_received);
> +	EXPECT_EQ(0, close(sock_fd));
> +	EXPECT_EQ(0, close(recv_sock));
> +	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 v4 0/6] landlock: Signal scoping support

[Mickaël Salaün]

We should also have the same tests as for scoped_vs_unscoped variants.
I renamed them from the abstract unix socket patch series, please take a
look:
https://git.kernel.org/pub/scm/linux/kernel/git/mic/linux.git/log/?h=next

I'll send more reviews tomorrow and I'll fix most of them in my -next
branch (WIP), except for the hook_file_send_sigiotask tests and these
scoped_vs_unscoped variants that you should resolve.

On Fri, Sep 06, 2024 at 03:30:02PM -0600, Tahera Fahimi wrote:
> This patch series adds scoping mechanism for signals.
> Closes: https://github.com/landlock-lsm/linux/issues/8
> 
> Problem
> =======
> 
> A sandboxed process is currently not restricted from sending signals
> (e.g. SIGKILL) to processes outside the sandbox since Landlock has no
> restriction on signals(see more details in [1]).
> 
> A simple way to apply this restriction would be to scope signals the
> same way abstract unix sockets are restricted.
> 
> [1]https://lore.kernel.org/all/20231023.ahphah4Wii4v@digikod.net/
> 
> Solution
> ========
> 
> To solve this issue, we extend the "scoped" field in the Landlock
> ruleset attribute structure by introducing "LANDLOCK_SCOPED_SIGNAL"
> field to specify that a ruleset will deny sending any signals from
> within the sandbox domain to its parent(i.e. any parent sandbox or
> non-sandbox processes).
> 
> Example
> =======
> 
> Create a sansboxed shell and pass the character "s" to LL_SCOPED:
> LL_FD_RO=/ LL_FS_RW=. LL_SCOPED="s" ./sandboxer /bin/bash
> Try to send a signal(like SIGTRAP) to a process ID <PID> through:
> kill -SIGTRAP <PID>
> The sandboxed process should not be able to send the signal.
> 
> Previous Versions
> =================
> v3:https://lore.kernel.org/all/cover.1723680305.git.fahimitahera@gmail.com/
> v2:https://lore.kernel.org/all/cover.1722966592.git.fahimitahera@gmail.com/
> v1:https://lore.kernel.org/all/cover.1720203255.git.fahimitahera@gmail.com/
> 
> Tahera Fahimi (6):
>   landlock: Add signal scoping control
>   selftest/landlock: Signal restriction tests
>   selftest/landlock: Add signal_scoping_threads test
>   selftest/landlock: Test file_send_sigiotask by sending out-of-bound
>     message
>   sample/landlock: Support sample for signal scoping restriction
>   landlock: Document LANDLOCK_SCOPED_SIGNAL
> 
>  Documentation/userspace-api/landlock.rst      |  22 +-
>  include/uapi/linux/landlock.h                 |   3 +
>  samples/landlock/sandboxer.c                  |  17 +-
>  security/landlock/fs.c                        |  17 +
>  security/landlock/fs.h                        |   6 +
>  security/landlock/limits.h                    |   2 +-
>  security/landlock/task.c                      |  59 +++
>  .../selftests/landlock/scoped_signal_test.c   | 371 ++++++++++++++++++
>  .../testing/selftests/landlock/scoped_test.c  |   2 +-
>  9 files changed, 486 insertions(+), 13 deletions(-)
>  create mode 100644 tools/testing/selftests/landlock/scoped_signal_test.c
> 
> -- 
> 2.34.1
> 

Re: [PATCH v4 0/6] landlock: Signal scoping support

[Tahera Fahimi]

On Wed, Sep 11, 2024 at 08:17:04PM +0200, Mickaël Salaün wrote:
> We should also have the same tests as for scoped_vs_unscoped variants.
Hi, 

Thanks for the review, I will add them soon.
> I renamed them from the abstract unix socket patch series, please take a
> look:
> https://git.kernel.org/pub/scm/linux/kernel/git/mic/linux.git/log/?h=next
Wonderful! Thank you :)

> I'll send more reviews tomorrow and I'll fix most of them in my -next
> branch (WIP), except for the hook_file_send_sigiotask tests and these
> scoped_vs_unscoped variants that you should resolve.
I will keep an eye on reviews. What parts of hook_file_send_sigiotask
would need changes?

> On Fri, Sep 06, 2024 at 03:30:02PM -0600, Tahera Fahimi wrote:
> > This patch series adds scoping mechanism for signals.
> > Closes: https://github.com/landlock-lsm/linux/issues/8
> > 
> > Problem
> > =======
> > 
> > A sandboxed process is currently not restricted from sending signals
> > (e.g. SIGKILL) to processes outside the sandbox since Landlock has no
> > restriction on signals(see more details in [1]).
> > 
> > A simple way to apply this restriction would be to scope signals the
> > same way abstract unix sockets are restricted.
> > 
> > [1]https://lore.kernel.org/all/20231023.ahphah4Wii4v@digikod.net/
> > 
> > Solution
> > ========
> > 
> > To solve this issue, we extend the "scoped" field in the Landlock
> > ruleset attribute structure by introducing "LANDLOCK_SCOPED_SIGNAL"
> > field to specify that a ruleset will deny sending any signals from
> > within the sandbox domain to its parent(i.e. any parent sandbox or
> > non-sandbox processes).
> > 
> > Example
> > =======
> > 
> > Create a sansboxed shell and pass the character "s" to LL_SCOPED:
> > LL_FD_RO=/ LL_FS_RW=. LL_SCOPED="s" ./sandboxer /bin/bash
> > Try to send a signal(like SIGTRAP) to a process ID <PID> through:
> > kill -SIGTRAP <PID>
> > The sandboxed process should not be able to send the signal.
> > 
> > Previous Versions
> > =================
> > v3:https://lore.kernel.org/all/cover.1723680305.git.fahimitahera@gmail.com/
> > v2:https://lore.kernel.org/all/cover.1722966592.git.fahimitahera@gmail.com/
> > v1:https://lore.kernel.org/all/cover.1720203255.git.fahimitahera@gmail.com/
> > 
> > Tahera Fahimi (6):
> >   landlock: Add signal scoping control
> >   selftest/landlock: Signal restriction tests
> >   selftest/landlock: Add signal_scoping_threads test
> >   selftest/landlock: Test file_send_sigiotask by sending out-of-bound
> >     message
> >   sample/landlock: Support sample for signal scoping restriction
> >   landlock: Document LANDLOCK_SCOPED_SIGNAL
> > 
> >  Documentation/userspace-api/landlock.rst      |  22 +-
> >  include/uapi/linux/landlock.h                 |   3 +
> >  samples/landlock/sandboxer.c                  |  17 +-
> >  security/landlock/fs.c                        |  17 +
> >  security/landlock/fs.h                        |   6 +
> >  security/landlock/limits.h                    |   2 +-
> >  security/landlock/task.c                      |  59 +++
> >  .../selftests/landlock/scoped_signal_test.c   | 371 ++++++++++++++++++
> >  .../testing/selftests/landlock/scoped_test.c  |   2 +-
> >  9 files changed, 486 insertions(+), 13 deletions(-)
> >  create mode 100644 tools/testing/selftests/landlock/scoped_signal_test.c
> > 
> > -- 
> > 2.34.1
> > 

Re: [PATCH v4 0/6] landlock: Signal scoping support

[Mickaël Salaün]

On Wed, Sep 11, 2024 at 06:15:24PM -0600, Tahera Fahimi wrote:
> On Wed, Sep 11, 2024 at 08:17:04PM +0200, Mickaël Salaün wrote:
> > We should also have the same tests as for scoped_vs_unscoped variants.
> Hi, 
> 
> Thanks for the review, I will add them soon.
> > I renamed them from the abstract unix socket patch series, please take a
> > look:
> > https://git.kernel.org/pub/scm/linux/kernel/git/mic/linux.git/log/?h=next
> Wonderful! Thank you :)
> 
> > I'll send more reviews tomorrow and I'll fix most of them in my -next
> > branch (WIP), except for the hook_file_send_sigiotask tests and these
> > scoped_vs_unscoped variants that you should resolve.
> I will keep an eye on reviews. What parts of hook_file_send_sigiotask
> would need changes?

The file_send_sigiotask hook was not fully covered.  It's OK now, I
reworked this test to fix that with four variants.

You still need to work on the scoped_vs_unscoped tests for signaling
though.

> 
> > On Fri, Sep 06, 2024 at 03:30:02PM -0600, Tahera Fahimi wrote:
> > > This patch series adds scoping mechanism for signals.
> > > Closes: https://github.com/landlock-lsm/linux/issues/8
> > > 
> > > Problem
> > > =======
> > > 
> > > A sandboxed process is currently not restricted from sending signals
> > > (e.g. SIGKILL) to processes outside the sandbox since Landlock has no
> > > restriction on signals(see more details in [1]).
> > > 
> > > A simple way to apply this restriction would be to scope signals the
> > > same way abstract unix sockets are restricted.
> > > 
> > > [1]https://lore.kernel.org/all/20231023.ahphah4Wii4v@digikod.net/
> > > 
> > > Solution
> > > ========
> > > 
> > > To solve this issue, we extend the "scoped" field in the Landlock
> > > ruleset attribute structure by introducing "LANDLOCK_SCOPED_SIGNAL"
> > > field to specify that a ruleset will deny sending any signals from
> > > within the sandbox domain to its parent(i.e. any parent sandbox or
> > > non-sandbox processes).
> > > 
> > > Example
> > > =======
> > > 
> > > Create a sansboxed shell and pass the character "s" to LL_SCOPED:
> > > LL_FD_RO=/ LL_FS_RW=. LL_SCOPED="s" ./sandboxer /bin/bash
> > > Try to send a signal(like SIGTRAP) to a process ID <PID> through:
> > > kill -SIGTRAP <PID>
> > > The sandboxed process should not be able to send the signal.
> > > 
> > > Previous Versions
> > > =================
> > > v3:https://lore.kernel.org/all/cover.1723680305.git.fahimitahera@gmail.com/
> > > v2:https://lore.kernel.org/all/cover.1722966592.git.fahimitahera@gmail.com/
> > > v1:https://lore.kernel.org/all/cover.1720203255.git.fahimitahera@gmail.com/
> > > 
> > > Tahera Fahimi (6):
> > >   landlock: Add signal scoping control
> > >   selftest/landlock: Signal restriction tests
> > >   selftest/landlock: Add signal_scoping_threads test
> > >   selftest/landlock: Test file_send_sigiotask by sending out-of-bound
> > >     message
> > >   sample/landlock: Support sample for signal scoping restriction
> > >   landlock: Document LANDLOCK_SCOPED_SIGNAL
> > > 
> > >  Documentation/userspace-api/landlock.rst      |  22 +-
> > >  include/uapi/linux/landlock.h                 |   3 +
> > >  samples/landlock/sandboxer.c                  |  17 +-
> > >  security/landlock/fs.c                        |  17 +
> > >  security/landlock/fs.h                        |   6 +
> > >  security/landlock/limits.h                    |   2 +-
> > >  security/landlock/task.c                      |  59 +++
> > >  .../selftests/landlock/scoped_signal_test.c   | 371 ++++++++++++++++++
> > >  .../testing/selftests/landlock/scoped_test.c  |   2 +-
> > >  9 files changed, 486 insertions(+), 13 deletions(-)
> > >  create mode 100644 tools/testing/selftests/landlock/scoped_signal_test.c
> > > 
> > > -- 
> > > 2.34.1
> > > 
> 
> 

Re: [PATCH v4 1/6] landlock: Add signal scoping control

[Mickaël Salaün]

On Fri, Sep 06, 2024 at 03:30:03PM -0600, Tahera Fahimi wrote:
> Currently, a sandbox process is not restricted to sending a signal (e.g.
> SIGKILL) to a process outside the sandbox environment. The ability to
> send a signal for a sandboxed process should be scoped the same way
> abstract UNIX sockets are scoped. Therefore, we extend the "scoped"
> field in a ruleset with "LANDLOCK_SCOPED_SIGNAL" to specify that a
> ruleset will deny sending any signal from within a sandbox process to
> its parent(i.e. any parent sandbox or non-sandboxed procsses).
> 
> This patch adds two new hooks, "hook_file_set_fowner" and
> "hook_file_free_security", to set and release a pointer to the file
> owner's domain. This pointer, "fown_domain" in "landlock_file_security"
> will be used in "file_send_sigiotask" to check if the process can send a
> signal. Also, it updates the function "ruleset_with_unknown_scope", to
> support the scope mask of signal "LANDLOCK_SCOPED_SIGNAL".
> 
> Signed-off-by: Tahera Fahimi <fahimitahera@gmail.com>
> ---
> Changes in versions:
> V4:
> * Merging file_send_sigiotask and task_kill patches into one.
> * Commit improvement.
> * Applying feedback of V3 on managing fown_domain pointer.
> V3:
> * Moving file_send_sigiotask to another patch.
> * Minor code refactoring.
> V2:
> * Remove signal_is_scoped function
> * Applying reviews of V1
> V1:
> * Introducing LANDLOCK_SCOPE_SIGNAL
> * Adding two hooks, hook_task_kill and hook_file_send_sigiotask for
>   signal scoping.
> ---
>  include/uapi/linux/landlock.h                 |  3 +
>  security/landlock/fs.c                        | 17 ++++++
>  security/landlock/fs.h                        |  6 ++
>  security/landlock/limits.h                    |  2 +-
>  security/landlock/task.c                      | 59 +++++++++++++++++++
>  .../testing/selftests/landlock/scoped_test.c  |  2 +-
>  6 files changed, 87 insertions(+), 2 deletions(-)
> 
> diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
> index dfd48d722834..197da0c5c264 100644
> --- a/include/uapi/linux/landlock.h
> +++ b/include/uapi/linux/landlock.h
> @@ -297,9 +297,12 @@ struct landlock_net_port_attr {
>   *   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).
> + * - %LANDLOCK_SCOPED_SIGNAL: Restrict a sandboxed process from sending
> + *   a signal to another process outside sandbox domain.
>   */
>  /* clang-format off */
>  #define LANDLOCK_SCOPED_ABSTRACT_UNIX_SOCKET		(1ULL << 0)
> +#define LANDLOCK_SCOPED_SIGNAL		                (1ULL << 1)
>  /* clang-format on*/
>  
>  #endif /* _UAPI_LINUX_LANDLOCK_H */
> diff --git a/security/landlock/fs.c b/security/landlock/fs.c
> index 7877a64cc6b8..b1207f0a8cd4 100644
> --- a/security/landlock/fs.c
> +++ b/security/landlock/fs.c
> @@ -1636,6 +1636,20 @@ static int hook_file_ioctl_compat(struct file *file, unsigned int cmd,
>  	return -EACCES;
>  }
>  
> +static void hook_file_set_fowner(struct file *file)
> +{
> +	write_lock_irq(&file->f_owner.lock);

This lock is already held.

> +	landlock_put_ruleset_deferred(landlock_file(file)->fown_domain);
> +	landlock_file(file)->fown_domain = landlock_get_current_domain();
> +	landlock_get_ruleset(landlock_file(file)->fown_domain);
> +	write_unlock_irq(&file->f_owner.lock);

Here is my new proposal:

struct landlock_ruleset *new_dom, *prev_dom;

/*
 * Lock already held by __f_setown(), see commit 26f204380a3c ("fs: Fix
 * file_set_fowner LSM hook inconsistencies").
 */
lockdep_assert_held(&file_f_owner(file)->lock);
new_dom = landlock_get_current_domain();
landlock_get_ruleset(new_dom);
prev_dom = landlock_file(file)->fown_domain;
landlock_file(file)->fown_domain = new_dom;

/* Called in an RCU read-side critical section. */
landlock_put_ruleset_deferred(prev_dom);


> +}
> +
> +static void hook_file_free_security(struct file *file)
> +{
> +	landlock_put_ruleset_deferred(landlock_file(file)->fown_domain);
> +}
> +
>  static struct security_hook_list landlock_hooks[] __ro_after_init = {
>  	LSM_HOOK_INIT(inode_free_security, hook_inode_free_security),
>  
> @@ -1660,6 +1674,9 @@ static struct security_hook_list landlock_hooks[] __ro_after_init = {
>  	LSM_HOOK_INIT(file_truncate, hook_file_truncate),
>  	LSM_HOOK_INIT(file_ioctl, hook_file_ioctl),
>  	LSM_HOOK_INIT(file_ioctl_compat, hook_file_ioctl_compat),
> +
> +	LSM_HOOK_INIT(file_set_fowner, hook_file_set_fowner),
> +	LSM_HOOK_INIT(file_free_security, hook_file_free_security),
>  };
>  
>  __init void landlock_add_fs_hooks(void)
> diff --git a/security/landlock/fs.h b/security/landlock/fs.h
> index 488e4813680a..9a97f9285b90 100644
> --- a/security/landlock/fs.h
> +++ b/security/landlock/fs.h
> @@ -52,6 +52,12 @@ struct landlock_file_security {
>  	 * needed to authorize later operations on the open file.
>  	 */
>  	access_mask_t allowed_access;
> +	/**
> +	 * @fown_domain: A pointer to a &landlock_ruleset of the process owns
> +	 * the file. This ruleset is protected by fowner_struct.lock same as
> +	 * pid, uid, euid fields in fown_struct.
> +	 */

Some rewording:

/**
 * @fown_domain: Domain of the task that set the PID that may receive a
 * signal e.g., SIGURG when writing MSG_OOB to the related socket.
 * This pointer is protected by the related file->f_owner->lock, as for
 * fown_struct's members: pid, uid, and euid.
 */


> +	struct landlock_ruleset *fown_domain;
>  };
>  
>  /**
> diff --git a/security/landlock/limits.h b/security/landlock/limits.h
> index eb01d0fb2165..fa28f9236407 100644
> --- a/security/landlock/limits.h
> +++ b/security/landlock/limits.h
> @@ -26,7 +26,7 @@
>  #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_LAST_SCOPE		LANDLOCK_SCOPED_SIGNAL
>  #define LANDLOCK_MASK_SCOPE		((LANDLOCK_LAST_SCOPE << 1) - 1)
>  #define LANDLOCK_NUM_SCOPE		__const_hweight64(LANDLOCK_MASK_SCOPE)
>  /* clang-format on */
> diff --git a/security/landlock/task.c b/security/landlock/task.c
> index b9390445d242..a72a61e7e6c3 100644
> --- a/security/landlock/task.c
> +++ b/security/landlock/task.c
> @@ -18,6 +18,7 @@
>  
>  #include "common.h"
>  #include "cred.h"
> +#include "fs.h"
>  #include "ruleset.h"
>  #include "setup.h"
>  #include "task.h"
> @@ -242,11 +243,69 @@ static int hook_unix_may_send(struct socket *const sock,
>  	return 0;
>  }
>  
> +static int hook_task_kill(struct task_struct *const p,
> +			  struct kernel_siginfo *const info, const int sig,
> +			  const struct cred *const cred)
> +{
> +	bool is_scoped;
> +	const struct landlock_ruleset *target_dom, *dom;
> +

> +	dom = landlock_get_current_domain();
> +	rcu_read_lock();
> +	target_dom = landlock_get_task_domain(p);
> +	if (cred)
> +		/* dealing with USB IO */
> +		is_scoped = domain_is_scoped(landlock_cred(cred)->domain,
> +					     target_dom,
> +					     LANDLOCK_SCOPED_SIGNAL);
> +	else
> +		is_scoped = (!dom) ? false :
> +				     domain_is_scoped(dom, target_dom,
> +						      LANDLOCK_SCOPED_SIGNAL);

Some refactoring to simplify code:

if (cred) {
	/* Dealing with USB IO. */
	dom = landlock_cred(cred)->domain;
} else {
	dom = landlock_get_current_domain();
}

/* Quick return for non-landlocked tasks. */
if (!dom)
	return 0;

rcu_read_lock();
is_scoped = domain_is_scoped(dom, landlock_get_task_domain(p),
			     LANDLOCK_SCOPE_SIGNAL);


> +	rcu_read_unlock();
> +	if (is_scoped)
> +		return -EPERM;
> +
> +	return 0;
> +}
> +
> +static int hook_file_send_sigiotask(struct task_struct *tsk,
> +				    struct fown_struct *fown, int signum)
> +{
> +	struct file *file;
> +	bool is_scoped;
> +	struct landlock_ruleset *dom;
> +
> +	/* struct fown_struct is never outside the context of a struct file */
> +	file = container_of(fown, struct file, f_owner);
> +

I rebased on these two commits that are planned for the next merge
window:
- commit 1934b212615d ("file: reclaim 24 bytes from f_owner"): replace
  container_of(fown, struct file, f_owner) with fown->file .
- commit 26f204380a3c ("fs: Fix file_set_fowner LSM hook
  inconsistencies"): lock before calling the hook.

For now, this is based on a custom merge:
https://git.kernel.org/pub/scm/linux/kernel/git/mic/linux.git/commit/?h=next&id=b1fefed2459f7bd3b1a03c2fa577d30a07130ef6

I'll rebase on top of the master branch once it will include the
required commits.  In the meantime, please use this merge commit as the
base commit for both patch series.

> +	read_lock_irq(&file->f_owner.lock);
> +	dom = landlock_file(file)->fown_domain;
> +	landlock_get_ruleset(dom);
> +	read_unlock_irq(&file->f_owner.lock);
> +	if (!dom)
> +		goto out_unlock;
> +
> +	rcu_read_lock();
> +	is_scoped = domain_is_scoped(dom, landlock_get_task_domain(tsk),
> +				     LANDLOCK_SCOPED_SIGNAL);
> +	rcu_read_unlock();
> +	if (is_scoped) {
> +		landlock_put_ruleset(dom);
> +		return -EPERM;
> +	}
> +out_unlock:
> +	landlock_put_ruleset(dom);
> +	return 0;

Here is the refactoring:

bool is_scoped = false;

/* Lock already held by send_sigio() and send_sigurg(). */
lockdep_assert_held(&fown->lock);
dom = landlock_file(fown->file)->fown_domain;

/* Quick return for unowned socket. */
if (!dom)
	return 0;

rcu_read_lock();
is_scoped = domain_is_scoped(dom, landlock_get_task_domain(tsk),
			     LANDLOCK_SCOPE_SIGNAL);
rcu_read_unlock();
if (is_scoped)
	return -EPERM;

return 0;


> +}
> +
>  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),
> +	LSM_HOOK_INIT(task_kill, hook_task_kill),
> +	LSM_HOOK_INIT(file_send_sigiotask, hook_file_send_sigiotask),
>  };
>  
>  __init void landlock_add_task_hooks(void)
> diff --git a/tools/testing/selftests/landlock/scoped_test.c b/tools/testing/selftests/landlock/scoped_test.c
> index 36d7266de9dc..237f98369b25 100644
> --- a/tools/testing/selftests/landlock/scoped_test.c
> +++ b/tools/testing/selftests/landlock/scoped_test.c
> @@ -12,7 +12,7 @@
>  
>  #include "common.h"
>  
> -#define ACCESS_LAST LANDLOCK_SCOPED_ABSTRACT_UNIX_SOCKET
> +#define ACCESS_LAST LANDLOCK_SCOPED_SIGNAL
>  
>  TEST(ruleset_with_unknown_scope)
>  {
> -- 
> 2.34.1
> 

Re: [PATCH v4 6/6] landlock: Document LANDLOCK_SCOPED_SIGNAL

[Mickaël Salaün]

On Fri, Sep 06, 2024 at 03:30:08PM -0600, Tahera Fahimi wrote:
> Improving Landlock ABI version 6 to support signal scoping with
> LANDLOCK_SCOPED_SIGNAL.
> 
> Signed-off-by: Tahera Fahimi <fahimitahera@gmail.com>
> ---
> v3:
> - update date
> ---
>  Documentation/userspace-api/landlock.rst | 22 +++++++++++++++-------
>  1 file changed, 15 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/userspace-api/landlock.rst b/Documentation/userspace-api/landlock.rst
> index c3b87755e98d..c694e9fe36fc 100644
> --- a/Documentation/userspace-api/landlock.rst
> +++ b/Documentation/userspace-api/landlock.rst
> @@ -82,7 +82,8 @@ to be explicit about the denied-by-default access rights.
>              LANDLOCK_ACCESS_NET_BIND_TCP |
>              LANDLOCK_ACCESS_NET_CONNECT_TCP,
>          .scoped =
> -            LANDLOCK_SCOPED_ABSTRACT_UNIX_SOCKET,
> +            LANDLOCK_SCOPED_ABSTRACT_UNIX_SOCKET |
> +            LANDLOCK_SCOPED_SIGNAL,
>      };
>  
>  Because we may not know on which kernel version an application will be
> @@ -123,7 +124,8 @@ version, and only use the available subset of access rights:
>          ruleset_attr.handled_access_fs &= ~LANDLOCK_ACCESS_FS_IOCTL_DEV;
>      case 5:
>          /* Removes LANDLOCK_SCOPED_ABSTRACT_UNIX_SOCKET for ABI < 6 */
> -        ruleset_attr.scoped &= ~LANDLOCK_SCOPED_ABSTRACT_UNIX_SOCKET;
> +        ruleset_attr.scoped &= ~(LANDLOCK_SCOPED_ABSTRACT_UNIX_SOCKET |
> +                                 LANDLOCK_SCOPED_SIGNAL);
>      }
>  
>  This enables to create an inclusive ruleset that will contain our rules.
> @@ -320,11 +322,15 @@ 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``.
> +``LANDLOCK_SCOPED_ABSTRACT_UNIX_SOCKET``. Moreover, if a sandboxed
> +process should not be able to send a signal to a non-sandboxed process,
> +we can specify this restriction with ``LANDLOCK_SCOPED_SIGNAL``.
>  
>  A sandboxed process can connect to a non-sandboxed process when its
>  domain is not scoped. If a process's domain is scoped, it can only
>  connect to sockets created by processes in the same scoped domain.
> +Moreover, If a process is scoped to send signal to a non-scoped process,
> +it can only send signals to processes in the same scoped domain.
>  
>  A connected datagram socket behaves like a stream socket when its domain
>  is scoped, meaning if the domain is scoped after the socket is connected
> @@ -575,12 +581,14 @@ 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.
>  
> -Abstract UNIX sockets Restriction  (ABI < 6)
> ---------------------------------------------
> +Abstract Unix sockets and Signal Restriction  (ABI < 6)
> +-------------------------------------------------------

I created a dedicated section instead of merging both.

>  
> +<<<<<<< current

I fixed that.

>  With ABI version 6, it is possible to restrict connection to an abstract
> -Unix socket through ``LANDLOCK_SCOPED_ABSTRACT_UNIX_SOCKET``, thanks to
> -the ``scoped`` ruleset attribute.
> +:manpage:`unix(7)` socket through
> +``LANDLOCK_SCOPED_ABSTRACT_UNIX_SOCKET`` and sending signal through
> +``LANDLOCK_SCOPED_SIGNAL``, thanks to the ``scoped`` ruleset attribute.

I cleaned up this fix that should be part of the other series.

>  
>  .. _kernel_support:
>  
> -- 
> 2.34.1
> 
>