[PATCH 0/2] Add io_uring test with Landlock access control rules

[PATCH 0/2] Add io_uring test with Landlock access control rules

[Dorine Tipo]

This patch series introduces a new test for verifying Landlock access
control rules with io_uring operation, IORING_OP_OPENAT.

The first patch (Add test for io_uring openat access control with Landlock
rules) implements the actual test.

- The test defines allowed and disallowed paths for access attempts.
- It creates Landlock rulesets for both allowed and disallowed access.
- It uses io_uring_prep_openat with the rulesets to attempt opening files.
- The test verifies successful preparation for allowed access and expects
submission failures for disallowed access.

The second patch  (Add liburing dependency for io_uring test) updates the
Makefile to include the necessary liburing library.

This patch series improves Landlock's test coverage by incorporating
io_uring operations and helps ensure Landlock effectively controls access
through io_uring.
It enhances the overall security posture of the system.

The test has been successfully compiled using make run_tests -C landlock
and successfully verifies Landlock access control with io_uring.

Dorine Tipo (2):
  Add test for io_uring openat access control with Landlock rules
  Add liburing dependency for io_uring test

 tools/testing/selftests/landlock/Makefile  |   4 +-
 tools/testing/selftests/landlock/fs_test.c | 132 +++++++++++++++++++++
 2 files changed, 134 insertions(+), 2 deletions(-)

--
2.25.1

[PATCH 1/2] Add test for io_uring openat access control with Landlock rules

[Dorine Tipo]

Expand Landlock test coverage to include io_uring operations.

This commit introduces a test for IORING_OP_OPENAT with Landlock
rules, verifying allowed and disallowed access. This mitigates potential
security vulnerabilities by ensuring Landlock controls access through
io_uring.

Signed-off-by: Dorine Tipo <dorine.a.tipo@gmail.com>
---
 tools/testing/selftests/landlock/fs_test.c | 132 +++++++++++++++++++++
 1 file changed, 132 insertions(+)

diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
index 9a6036fbf289..9c8247995d42 100644
--- a/tools/testing/selftests/landlock/fs_test.c
+++ b/tools/testing/selftests/landlock/fs_test.c
@@ -21,7 +21,10 @@
 #include <sys/stat.h>
 #include <sys/sysmacros.h>
 #include <sys/vfs.h>
+#include <sys/types.h>
 #include <unistd.h>
+#include <liburing.h>
+#include <linux/io_uring.h>

 #include "common.h"

@@ -4874,4 +4877,133 @@ TEST_F_FORK(layout3_fs, release_inodes)
 	ASSERT_EQ(EACCES, test_open(TMP_DIR, O_RDONLY));
 }

+/* Test io_uring openat access control with landlock rules */
+static int test_ioring_op_openat(struct __test_metadata *const _metadata, const __u64 access, const char **paths, const int paths_size)
+{
+	struct io_uring ring;
+	struct io_uring_sqe *sqe;
+
+	const char *allowed_paths[] = {
+		file1_s1d1, file2_s1d1,
+		file1_s1d2, file2_s1d2,
+		file1_s1d3, file2_s1d3,
+		file1_s2d1, file1_s2d2,
+		file1_s2d3, file2_s2d3,
+		file1_s3d1,
+	};
+	const char *disallowed_paths[] = {
+		/* dir_s3d2 is a mount point. */
+		dir_s3d2,
+		dir_s3d3,
+	};
+
+	/* Test Allowed Access */
+	const struct rule allowed_rule[] = {
+		{
+			.path = allowed_paths[0],
+			.access = LANDLOCK_ACCESS_FS_READ_FILE |
+				  LANDLOCK_ACCESS_FS_WRITE_FILE,
+		},
+	};
+	int allowed_ruleset_fd = create_ruleset(_metadata, allowed_rule[0].access, allowed_rule);
+
+	ASSERT_LE(0, allowed_ruleset_fd);
+
+	int ret = io_uring_queue_init(32, &ring, 0);
+
+	ASSERT_EQ(0, ret);
+
+	/* Test each allowed path */
+	for (int i = 0; i < ARRAY_SIZE(allowed_paths); ++i) {
+		sqe = io_uring_get_sqe(&ring);
+		io_uring_prep_openat(sqe, AT_FDCWD, allowed_paths[i], O_RDONLY,
+				     allowed_ruleset_fd);
+		/* Verify successful SQE preparation */
+		ASSERT_EQ(0, ret);
+
+		if (ret != 0)
+			return ret;
+
+		ret = io_uring_submit(&ring);
+		/* Verify 1 submission completed */
+		ASSERT_EQ(1, ret);
+	}
+
+	/* Test Disallowed Access */
+	const struct rule disallowed_rule[] = {
+		{
+			.path = disallowed_paths[0],
+			.access = 0,
+		}
+
+	};
+	int disallowed_ruleset_fd = create_ruleset(_metadata, disallowed_rule[0].access, disallowed_rule);
+
+	ASSERT_LE(0, disallowed_ruleset_fd);
+
+	/* Test each disallowed path */
+	for (int i = 0; i < ARRAY_SIZE(disallowed_paths); ++i) {
+		sqe = io_uring_get_sqe(&ring);
+		io_uring_prep_openat(sqe, AT_FDCWD, disallowed_paths[i], O_RDONLY, disallowed_ruleset_fd);
+		/* Verify successful SQE preparation */
+		ASSERT_EQ(1, ret);
+
+		if (ret != 0)
+			return ret;
+
+		ret = io_uring_submit(&ring);
+		/* Verify 1 submission completed */
+		ASSERT_EQ(0, ret);
+	}
+
+	/*  Cleanup: close ruleset fds, etc. */
+	close(allowed_ruleset_fd);
+	close(disallowed_ruleset_fd);
+
+	return 0;
+}
+
+/* clang-format off */
+FIXTURE(openat_test) {
+	struct __test_metadata *metadata;
+	const char *allowed_paths[11];
+	const char *disallowed_paths[2];
+};
+
+/* clang-format on */
+
+FIXTURE_SETUP(openat_test)
+{
+	/* initialize metadata, allowed_paths, and disallowed_paths */
+	self->metadata = _metadata;
+	const char *temp_allowed_paths[] = {
+		file1_s1d1, file2_s1d1, file1_s1d2, file2_s1d2,
+		file1_s1d3, file2_s1d3, file1_s2d1, file1_s2d2,
+		file1_s2d3, file2_s2d3, file1_s3d1};
+
+	memcpy(self->allowed_paths, temp_allowed_paths, sizeof(temp_allowed_paths));
+
+	const char *temp_disallowed_paths[] = {dir_s3d2, dir_s3d3};
+
+	memcpy(self->disallowed_paths, temp_disallowed_paths, sizeof(temp_disallowed_paths));
+}
+
+FIXTURE_TEARDOWN(openat_test)
+{
+	/* Clean up test environment */
+}
+
+TEST_F_FORK(openat_test, test_ioring_op_openat_allowed)
+{
+	test_ioring_op_openat(self->metadata, LANDLOCK_ACCESS_FS_READ_FILE |
+			      LANDLOCK_ACCESS_FS_WRITE_FILE, self->allowed_paths,
+			      ARRAY_SIZE(self->allowed_paths));
+}
+
+TEST_F_FORK(openat_test, test_ioring_op_openat_disallowed)
+{
+	test_ioring_op_openat(self->metadata, 0, self->disallowed_paths,
+			      ARRAY_SIZE(self->disallowed_paths));
+}
+
 TEST_HARNESS_MAIN
--
2.25.1

[PATCH 2/2] Add liburing dependency for io_uring test

[Dorine Tipo]

This commit updates Makefile to include -luring in the LDLIBS variable.
This ensures the test code has access to the necessary library for
io_uring operations.

Signed-off-by: Dorine Tipo <dorine.a.tipo@gmail.com>
---
 tools/testing/selftests/landlock/Makefile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/landlock/Makefile b/tools/testing/selftests/landlock/Makefile
index 348e2dbdb4e0..ab47d1dadb62 100644
--- a/tools/testing/selftests/landlock/Makefile
+++ b/tools/testing/selftests/landlock/Makefile
@@ -13,11 +13,11 @@ TEST_GEN_PROGS := $(src_test:.c=)
 TEST_GEN_PROGS_EXTENDED := true

 # Short targets:
-$(TEST_GEN_PROGS): LDLIBS += -lcap
+$(TEST_GEN_PROGS): LDLIBS += -lcap -luring
 $(TEST_GEN_PROGS_EXTENDED): LDFLAGS += -static

 include ../lib.mk

 # Targets with $(OUTPUT)/ prefix:
-$(TEST_GEN_PROGS): LDLIBS += -lcap
+$(TEST_GEN_PROGS): LDLIBS += -lcap -luring
 $(TEST_GEN_PROGS_EXTENDED): LDFLAGS += -static
--
2.25.1

Re: [PATCH 0/2] Add io_uring test with Landlock access control rules

[Fabio M. De Francesco]

On Monday, 25 March 2024 03:32:33 CET Dorine Tipo wrote:
> This patch series introduces a new test for verifying Landlock access
> control rules with io_uring operation, IORING_OP_OPENAT.
> 
> The first patch (Add test for io_uring openat access control with
> Landlock rules) implements the actual test.
> 
> - The test defines allowed and disallowed paths for access attempts.
> - It creates Landlock rulesets for both allowed and disallowed access.
> - It uses io_uring_prep_openat with the rulesets to attempt opening
> files. - The test verifies successful preparation for allowed access and
> expects submission failures for disallowed access.
> 
> The second patch  (Add liburing dependency for io_uring test) updates the
> Makefile to include the necessary liburing library.

Hi Dorine,

If I read your message correctly, patch 1/2 depends on 2/2 because  
liburing is needed by tests in 1/2. Every single patch of a series must 
build and link successfully.  I think that patch 1/2 cannot link without 
the changes that you put in patch 2/2. Am I missing something?

The prerequisites one must meet when sending series are that (1) that  
series must be applicable to (based on) the most recent publicly available 
official version of the subsystem/driver it is meant for, (2) for all n 
patches of a series with N patches, patch n must not require any k > n  
patch (i.e., k:=(n < k <= N)), and (3) each patch must always achieve a 
logically self-contained objective.

Well, in theory you may think to reverse the order of the patches to meet 
rule 2.  But by doing so you would break rule 3 because the changes you 
make in the Makefile would only be needed to build and link another patch so 
they don't serve any self-contained purposes. 

Therefore, please make a self-contained patch that includes all the 
necessary changes to make it build and link.

Thanks,

Fabio

> This patch series improves Landlock's test coverage by incorporating
> io_uring operations and helps ensure Landlock effectively controls access
> through io_uring.
> It enhances the overall security posture of the system.
> 
> The test has been successfully compiled using make run_tests -C landlock
> and successfully verifies Landlock access control with io_uring.
> 
> Dorine Tipo (2):
>   Add test for io_uring openat access control with Landlock rules
>   Add liburing dependency for io_uring test
> 
>  tools/testing/selftests/landlock/Makefile  |   4 +-
>  tools/testing/selftests/landlock/fs_test.c | 132 +++++++++++++++++++++
>  2 files changed, 134 insertions(+), 2 deletions(-)
> 
> --
> 2.25.1