[PATCH 0/3] Implement LANDLOCK_ADD_RULE_NO_INHERIT

[PATCH 0/3] Implement LANDLOCK_ADD_RULE_NO_INHERIT

[Justin Suess]

Hi,

This patch builds on version 3 of the the "quiet flag" series by Tingmao Wang.

v3: https://lore.kernel.org/linux-security-module/cover.1761511023.git.m@maowtm.org/

It implements a new flag that prevents inheriting access rights from parent
objects within a single landlock layer. This is useful for policies
where a parent directory requires looser access grants that its
children.

For example, within a single ruleset / layer, given:
    /a = rw
    /a/b = ro

Under the current featureset, /a/b recieves rw permissions because it
inherits the w permission from /a

To solve this, I add a new flag LANDLOCK_RULE_ADD_NO_INHERIT which
suppresses parent permissions.

For example:
    /a = rw
    /a/b = ro + LANDLOCK_RULE_ADD_NO_INHERIT

This grants /a/b only read permissions.

Design:
- When this flag is added to a rule, the landlock_unmask_layers function will 
  track encounters of this flag in the rule_flags as it traverses up the fs tree.
  When this flag is encountered, the access grants of the current rule will be allowed, 
  but further access grants by rules within that layer will be suppressed.
- Access grants made by rules in other layers will continue until the access requirements are satisfied.

Demo:
~ # LL_FS_RW="/" LL_FS_RO="" LL_FS_RO_NO_INHERIT="/tmp" landlock-sandboxer touch fi
    Executing the sandboxed command...
    ~ # ls
    bin   dev   etc   fi    init  proc  root  sbin  sys   tmp   usr
    ~ # LL_FS_RW="/" LL_FS_RO="" LL_FS_RO_NO_INHERIT="/tmp" landlock-sandboxer touch /tmp/fi
    Executing the sandboxed command...
    touch: /tmp/fi: Permission denied
    ~ # LL_FS_RW="/" LL_FS_RO="" LL_FS_RO_NO_INHERIT="/tmp" landlock-sandboxer sh
    Executing the sandboxed command...
    sh: can't access tty; job control turned off
    ~ # cd tmp
    /tmp # LL_FS_RW="/" LL_FS_RO="" LL_FS_RO_NO_INHERIT="/tmp" landlock-sandboxer ls
    Executing the sandboxed command...
    /tmp # LL_FS_RW="/" LL_FS_RO="" LL_FS_RO_NO_INHERIT="/tmp" landlock-sandboxer touch fifi
    Executing the sandboxed command...
    touch: fifi: Permission denied
    /tmp # LL_FS_RW="/" LL_FS_RO=""  landlock-sandboxer touch fifi
    Executing the sandboxed command...
    touch: fifi: Permission denied
    /tmp #

This is my first patch/contribution to the LSM subsystem (and the linux
kernel as a whole), so any feedback and corrections on mailing list
ettiquite would be appreciated. 

Very Respectfully,
Justin Suess

Justin Suess (3):
  landlock: Add flag to supress access rule inheritence within a layer
  samples/landlock: Add no inherit support to sandboxer
  selftests/landlock: Add test for new no inherit flag

 include/uapi/linux/landlock.h              |  9 ++++
 samples/landlock/sandboxer.c               | 39 ++++++++++++----
 security/landlock/ruleset.c                |  8 ++++
 security/landlock/ruleset.h                | 10 ++++
 security/landlock/syscalls.c               |  3 +-
 tools/testing/selftests/landlock/fs_test.c | 53 +++++++++++++++++-----
 6 files changed, 99 insertions(+), 23 deletions(-)

base-commit: 77903de728f2a1ef40a31a3babf861b8fbf9530f
--
2.51.0

[PATCH 1/3] landlock: Add flag to supress access rule inheritence within a layer

[Justin Suess]

Creates a new flag that prevents inheriting access rights from parent
objects within a single landlock layer. For example /a/b = ro + no inherit
and /a = rw results in /a/b recieving ro and not rw permissions.

Signed-off-by: Justin Suess <utilityemal77@gmail.com>
---
 include/uapi/linux/landlock.h |  9 +++++++++
 security/landlock/ruleset.c   |  8 ++++++++
 security/landlock/ruleset.h   | 10 ++++++++++
 security/landlock/syscalls.c  |  3 ++-
 4 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
index 50f0806b7e33..d9daef551d96 100644
--- a/include/uapi/linux/landlock.h
+++ b/include/uapi/linux/landlock.h
@@ -127,10 +127,19 @@ struct landlock_ruleset_attr {
  *     allowed_access in the passed in rule_attr.  When this flag is
  *     present, the caller is also allowed to pass in an empty
  *     allowed_access.
+ * %LANDLOCK_ADD_RULE_NO_INHERIT
+ *     When this flag is set while adding a rule to a ruleset, the rule
+ *     will not inherit allowed accesses from rules on parent objects
+ *     within the same layer. (currently only applies to filesystem objects)
+ *     By default, Landlock rules added to a ruleset inherit allowed accesses
+ *     from parent objects, meaning that if a parent directory has been granted
+ *     certain access rights, those rights will also apply to its child objects.
+ *     This flag prevents such inheritance for the specific rule being added.
  */

 /* clang-format off */
 #define LANDLOCK_ADD_RULE_QUIET			(1U << 0)
+#define LANDLOCK_ADD_RULE_NO_INHERIT		(1U << 1)
 /* clang-format on */

 /**
diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c
index e0bb7e795574..8fab8222fc30 100644
--- a/security/landlock/ruleset.c
+++ b/security/landlock/ruleset.c
@@ -315,6 +315,7 @@ int landlock_insert_rule(struct landlock_ruleset *const ruleset,
 		.level = 0,
 		.flags = {
 			.quiet = !!(flags & LANDLOCK_ADD_RULE_QUIET),
+			.no_inherit = !!(flags & LANDLOCK_ADD_RULE_NO_INHERIT),
 		}
 	} };

@@ -660,9 +661,16 @@ bool landlock_unmask_layers(const struct landlock_rule *const rule,
 		unsigned long access_bit;
 		bool is_empty;

+		/* Skip layers that already have no inherit flags. */
+		if (rule_flags &&
+		    (rule_flags->no_inherit_masks & layer_bit))
+			continue;
+
 		/* Collect rule flags for each layer. */
 		if (rule_flags && layer->flags.quiet)
 			rule_flags->quiet_masks |= layer_bit;
+		if (rule_flags && layer->flags.no_inherit)
+			rule_flags->no_inherit_masks |= layer_bit;

 		/*
 		 * Records in @layer_masks which layer grants access to each requested
diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h
index cd0434d8dc63..7759151ce727 100644
--- a/security/landlock/ruleset.h
+++ b/security/landlock/ruleset.h
@@ -40,6 +40,12 @@ struct landlock_layer {
 		 * down the file hierarchy.
 		 */
 		bool quiet:1;
+		/**
+		 * @no_inherit: When set, this layer's rule does not inherit
+		 * allowed accesses from parent objects within the same layer.
+		 * (currently only applies to filesystem objects)
+		 */
+		bool no_inherit:1;
 	} flags;
 	/**
 	 * @access: Bitfield of allowed actions on the kernel object.  They are
@@ -56,6 +62,10 @@ struct collected_rule_flags {
 	 * @quiet_masks: Layers for which the quiet flag is effective.
 	 */
 	layer_mask_t quiet_masks;
+	/**
+	 * @no_inherit_masks: Layers for which the no_inherit flag is effective.
+	 */
+	layer_mask_t no_inherit_masks;
 };

 /**
diff --git a/security/landlock/syscalls.c b/security/landlock/syscalls.c
index 93396bfc1500..ed7304d53894 100644
--- a/security/landlock/syscalls.c
+++ b/security/landlock/syscalls.c
@@ -463,7 +463,8 @@ SYSCALL_DEFINE4(landlock_add_rule, const int, ruleset_fd,
 	if (!is_initialized())
 		return -EOPNOTSUPP;

-	if (flags && flags != LANDLOCK_ADD_RULE_QUIET)
+	if (flags && flags & ~(LANDLOCK_ADD_RULE_QUIET | \
+		LANDLOCK_ADD_RULE_NO_INHERIT))
 		return -EINVAL;

 	/* Gets and checks the ruleset. */
--
2.51.0

[PATCH 2/3] samples/landlock: Add no inherit support to sandboxer

[Justin Suess]

Adds two new environment variables LL_FS_RO_NO_INHERIT and LL_FS_RW_NO_INHERIT
to test the new LANDLOCK_RULE_ADD_NO_INHERIT flag

Signed-off-by: Justin Suess <utilityemal77@gmail.com>
---
 samples/landlock/sandboxer.c | 39 +++++++++++++++++++++++++++---------
 1 file changed, 29 insertions(+), 10 deletions(-)

diff --git a/samples/landlock/sandboxer.c b/samples/landlock/sandboxer.c
index 2d8e3e94b77b..2b40b2df83b4 100644
--- a/samples/landlock/sandboxer.c
+++ b/samples/landlock/sandboxer.c
@@ -58,6 +58,8 @@ static inline int landlock_restrict_self(const int ruleset_fd,

 #define ENV_FS_RO_NAME "LL_FS_RO"
 #define ENV_FS_RW_NAME "LL_FS_RW"
+#define ENV_FS_RO_NO_INHERIT_NAME "LL_FS_RO_NO_INHERIT"
+#define ENV_FS_RW_NO_INHERIT_NAME "LL_FS_RW_NO_INHERIT"
 #define ENV_FS_QUIET_NAME "LL_FS_QUIET"
 #define ENV_FS_QUIET_ACCESS_NAME "LL_FS_QUIET_ACCESS"
 #define ENV_TCP_BIND_NAME "LL_TCP_BIND"
@@ -121,7 +123,8 @@ static int parse_path(char *env_path, const char ***const path_list)
 /* clang-format on */

 static int populate_ruleset_fs(const char *const env_var, const int ruleset_fd,
-			       const __u64 allowed_access, bool quiet)
+		       const __u64 allowed_access,
+		       __u32 add_rule_flags, bool mandatory)
 {
 	int num_paths, i, ret = 1;
 	char *env_path_name;
@@ -132,9 +135,13 @@ static int populate_ruleset_fs(const char *const env_var, const int ruleset_fd,

 	env_path_name = getenv(env_var);
 	if (!env_path_name) {
-		/* Prevents users to forget a setting. */
-		fprintf(stderr, "Missing environment variable %s\n", env_var);
-		return 1;
+		if (mandatory) {
+			/* Prevents users to forget a setting. */
+			fprintf(stderr, "Missing environment variable %s\n",
+				env_var);
+			return 1;
+		}
+		return 0;
 	}
 	env_path_name = strdup(env_path_name);
 	unsetenv(env_var);
@@ -171,8 +178,7 @@ static int populate_ruleset_fs(const char *const env_var, const int ruleset_fd,
 		if (!S_ISDIR(statbuf.st_mode))
 			path_beneath.allowed_access &= ACCESS_FILE;
 		if (landlock_add_rule(ruleset_fd, LANDLOCK_RULE_PATH_BENEATH,
-				      &path_beneath,
-				      quiet ? LANDLOCK_ADD_RULE_QUIET : 0)) {
+			      &path_beneath, add_rule_flags)) {
 			fprintf(stderr,
 				"Failed to update the ruleset with \"%s\": %s\n",
 				path_list[i], strerror(errno));
@@ -375,6 +381,8 @@ static const char help[] =
 	"Optional settings (when not set, their associated access check "
 	"is always allowed, which is different from an empty string which "
 	"means an empty list):\n"
+	"* " ENV_FS_RO_NO_INHERIT_NAME ": read-only paths without rule inheritance\n"
+	"* " ENV_FS_RW_NO_INHERIT_NAME ": read-write paths without rule inheritance\n"
 	"* " ENV_TCP_BIND_NAME ": ports allowed to bind (server)\n"
 	"* " ENV_TCP_CONNECT_NAME ": ports allowed to connect (client)\n"
 	"* " ENV_SCOPED_NAME ": actions denied on the outside of the landlock domain\n"
@@ -596,17 +604,28 @@ int main(const int argc, char *const argv[], char *const *const envp)
 	}

 	if (populate_ruleset_fs(ENV_FS_RO_NAME, ruleset_fd, access_fs_ro,
-				false)) {
+			0, true)) {
 		goto err_close_ruleset;
 	}
 	if (populate_ruleset_fs(ENV_FS_RW_NAME, ruleset_fd, access_fs_rw,
-				false)) {
+			0, true)) {
+		goto err_close_ruleset;
+	}
+	/* Optional no-inherit rules mirror the regular read-only/read-write sets. */
+	if (populate_ruleset_fs(ENV_FS_RO_NO_INHERIT_NAME, ruleset_fd,
+			access_fs_ro, LANDLOCK_ADD_RULE_NO_INHERIT,
+			false)) {
+		goto err_close_ruleset;
+	}
+	if (populate_ruleset_fs(ENV_FS_RW_NO_INHERIT_NAME, ruleset_fd,
+			access_fs_rw, LANDLOCK_ADD_RULE_NO_INHERIT,
+			false)) {
 		goto err_close_ruleset;
 	}
 	/* Don't require this env to be present. */
-	if (quiet_supported && getenv(ENV_FS_QUIET_NAME)) {
+	if (quiet_supported) {
 		if (populate_ruleset_fs(ENV_FS_QUIET_NAME, ruleset_fd, 0,
-					true)) {
+				LANDLOCK_ADD_RULE_QUIET, false)) {
 			goto err_close_ruleset;
 		}
 	}
--
2.51.0

[PATCH 3/3] selftests/landlock: Add test for new no inherit flag

[Justin Suess]

Adds tests for the new no inherit flag, validating the expected
supression of permission inheritence within a layer. Also updates
the add_path_beneath function to take a __u32 (instead of bool quiet)
for the flags, since LANDLOCK_ADD_RULE_QUIET isn't the only
flag anymore.

Signed-off-by: Justin Suess <utilityemal77@gmail.com>
---
 tools/testing/selftests/landlock/fs_test.c | 53 +++++++++++++++++-----
 1 file changed, 41 insertions(+), 12 deletions(-)

diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
index d4819ff44230..a7ebe4ec1b5a 100644
--- a/tools/testing/selftests/landlock/fs_test.c
+++ b/tools/testing/selftests/landlock/fs_test.c
@@ -717,16 +717,12 @@ TEST_F_FORK(layout1, rule_with_unhandled_access)
 }

 static void add_path_beneath(struct __test_metadata *const _metadata,
-			     const int ruleset_fd, const __u64 allowed_access,
-			     const char *const path, bool quiet)
+			 const int ruleset_fd, const __u64 allowed_access,
+			 const char *const path, __u32 flags)
 {
 	struct landlock_path_beneath_attr path_beneath = {
 		.allowed_access = allowed_access,
 	};
-	__u32 flags = 0;
-
-	if (quiet)
-		flags |= LANDLOCK_ADD_RULE_QUIET;

 	path_beneath.parent_fd = open(path, O_PATH | O_CLOEXEC);
 	ASSERT_LE(0, path_beneath.parent_fd)
@@ -790,7 +786,7 @@ static int create_ruleset(struct __test_metadata *const _metadata,
 			continue;

 		add_path_beneath(_metadata, ruleset_fd, rules[i].access,
-				 rules[i].path, false);
+				 rules[i].path, 0);
 	}
 	return ruleset_fd;
 }
@@ -1368,7 +1364,7 @@ TEST_F_FORK(layout1, inherit_subset)
 	 * ANDed with the previous ones.
 	 */
 	add_path_beneath(_metadata, ruleset_fd, LANDLOCK_ACCESS_FS_WRITE_FILE,
-			 dir_s1d2, false);
+			 dir_s1d2, 0);
 	/*
 	 * According to ruleset_fd, dir_s1d2 should now have the
 	 * LANDLOCK_ACCESS_FS_READ_FILE and LANDLOCK_ACCESS_FS_WRITE_FILE
@@ -1400,7 +1396,7 @@ TEST_F_FORK(layout1, inherit_subset)
 	 * Try to get more privileges by adding new access rights to the parent
 	 * directory: dir_s1d1.
 	 */
-	add_path_beneath(_metadata, ruleset_fd, ACCESS_RW, dir_s1d1, false);
+	add_path_beneath(_metadata, ruleset_fd, ACCESS_RW, dir_s1d1, 0);
 	enforce_ruleset(_metadata, ruleset_fd);

 	/* Same tests and results as above. */
@@ -1423,7 +1419,7 @@ TEST_F_FORK(layout1, inherit_subset)
 	 * that there was no rule tied to it before.
 	 */
 	add_path_beneath(_metadata, ruleset_fd, LANDLOCK_ACCESS_FS_WRITE_FILE,
-			 dir_s1d3, false);
+			 dir_s1d3, 0);
 	enforce_ruleset(_metadata, ruleset_fd);
 	ASSERT_EQ(0, close(ruleset_fd));

@@ -1476,7 +1472,7 @@ TEST_F_FORK(layout1, inherit_superset)
 	add_path_beneath(_metadata, ruleset_fd,
 			 LANDLOCK_ACCESS_FS_READ_FILE |
 				 LANDLOCK_ACCESS_FS_READ_DIR,
-			 dir_s1d2, false);
+			 dir_s1d2, 0);
 	enforce_ruleset(_metadata, ruleset_fd);
 	ASSERT_EQ(0, close(ruleset_fd));

@@ -1488,6 +1484,39 @@ TEST_F_FORK(layout1, inherit_superset)
 	ASSERT_EQ(0, test_open(file1_s1d3, O_RDONLY));
 }

+TEST_F_FORK(layout1, inherit_no_inherit_flag)
+{
+	struct landlock_ruleset_attr ruleset_attr = {
+		.handled_access_fs = ACCESS_RW,
+	};
+	int ruleset_fd;
+
+	ruleset_fd =
+		landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
+	ASSERT_LE(0, ruleset_fd);
+
+	add_path_beneath(_metadata, ruleset_fd, ACCESS_RW, dir_s1d1, 0);
+	add_path_beneath(_metadata, ruleset_fd, ACCESS_RO, dir_s1d2,
+			       LANDLOCK_ADD_RULE_NO_INHERIT);
+
+	enforce_ruleset(_metadata, ruleset_fd);
+	ASSERT_EQ(0, close(ruleset_fd));
+
+	/* Parent directory still grants write access to its direct children. */
+	EXPECT_EQ(0, test_open(dir_s1d1, O_RDONLY | O_DIRECTORY));
+	EXPECT_EQ(0, test_open(file1_s1d1, O_WRONLY));
+
+	/* dir_s1d2 gets only its explicit read-only access rights. */
+	EXPECT_EQ(0, test_open(dir_s1d2, O_RDONLY | O_DIRECTORY));
+	EXPECT_EQ(0, test_open(file1_s1d2, O_RDONLY));
+	EXPECT_EQ(EACCES, test_open(file1_s1d2, O_WRONLY));
+
+	/* Descendants of dir_s1d2 inherit the reduced access mask. */
+	EXPECT_EQ(0, test_open(dir_s1d3, O_RDONLY | O_DIRECTORY));
+	EXPECT_EQ(0, test_open(file1_s1d3, O_RDONLY));
+	EXPECT_EQ(EACCES, test_open(file1_s1d3, O_WRONLY));
+}
+
 TEST_F_FORK(layout0, max_layers)
 {
 	int i, err;
@@ -7647,7 +7676,7 @@ static int apply_a_layer(struct __test_metadata *const _metadata,
 			continue;

 		add_path_beneath(_metadata, rs_fd, r->access, r->path,
-				 r->quiet);
+				 r->quiet ? LANDLOCK_ADD_RULE_QUIET : 0);
 	}

 	ASSERT_EQ(0, prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0));
--
2.51.0