* [PATCH v3] selftests/landlock: Add tests for io_uring openat access control with Landlock rules
@ 2024-03-30 12:48 Dorine Tipo
2024-04-02 14:47 ` Mickaël Salaün
0 siblings, 1 reply; 4+ messages in thread
From: Dorine Tipo @ 2024-03-30 12:48 UTC (permalink / raw)
To: mic, skhan, outreachy
Cc: Dorine Tipo, Linux-kernel-mentees-request, fabio.maria.de.francesco
This v3 patch expands Landlock test coverage to include io_uring
operations. It also introduces a test for IORING_OP_OPENAT with Landlock
rules, verifying allowed and disallowed access. This makes sure that
IORING_OP_OPENAT is well covered from security vulnerabilities by
ensuring Landlock controls access through io_uring.
It also updates Makefile to include -luring in the LDLIBS variable.
This ensures the test code has access to the necessary library for
io_uring operations.
The patch includes several improvements to the test_ioring_openat(),
FIXTURE_SETUP() and FIXTURE_TEARDOWN() addressing feedback received on
the initial version.
Signed-off-by: Dorine Tipo <dorine.a.tipo@gmail.com>
---
Changes since V1:
V2: - Consolidated two dependent patches in the V1 series into one patch
as suggested by Fabio.
- Updated The subject line to be more descriptive.
V3: - Added "selftests/landlock" subject prefix
- Revised wording in the commit message to accurately reflect the
test functionality as suggested by Mickaël.
- Updated the Fixture set_up and tear_down to create and remove the
necesssary files and folders for testing access.
- renamed allowed_ruleset_fd and disallowed_ruleset_fd to ruleset_fd
as suggest by Mickaël.
- Moved all variable declarations to the top of the function.
- Refactored the code to test only one allowed and one restricted
path instead of iterating through all the paths.
- Added comments to explain what is happening in different blocks
of code
- Removed the clang-format markers.
- Removed unused arguments in the function definition.
- Added a final rule struct with a null path to the allowed_rule
and disallowed_rule arrays as suggested by Fabio.
- CC'd the missing mailing lists as suggested by Shuah.
- All executables have been included in the .gitignore so no updates
are necessary.
tools/testing/selftests/landlock/Makefile | 4 +-
tools/testing/selftests/landlock/fs_test.c | 140 +++++++++++++++++++++
2 files changed, 142 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
diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
index 9a6036fbf289..7147c75b6f79 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,141 @@ TEST_F_FORK(layout3_fs, release_inodes)
ASSERT_EQ(EACCES, test_open(TMP_DIR, O_RDONLY));
}
+/* Test io_uring's IORING_OP_OPENAT access control with landlock rules */
+static int test_ioring_op_openat(struct __test_metadata *const _metadata, const __u64 access, const char **paths)
+{
+ int ruleset_fd;
+ int ret;
+ struct io_uring ring;
+ struct io_uring_sqe *sqe;
+
+ /* Test Allowed Access */
+ const struct rule allowed_rule[] = {
+ {
+ .path = paths[0],
+ .access = access,
+ },
+ {
+ .path = NULL,
+ },
+ };
+
+ ruleset_fd = create_ruleset(_metadata, allowed_rule[0].access, allowed_rule);
+
+ ASSERT_LE(0, ruleset_fd);
+
+ ret = io_uring_queue_init(32, &ring, 0);
+
+ ASSERT_EQ(0, ret);
+
+ /* Test allowed path */
+ sqe = io_uring_get_sqe(&ring);
+
+ /* Prepare SQE for the openat operation */
+ io_uring_prep_openat(sqe, AT_FDCWD, paths[0], O_RDONLY, 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 = paths[0],
+ .access = 0,
+ },
+ {
+ .path = NULL,
+ },
+ };
+
+ ruleset_fd = create_ruleset(_metadata, disallowed_rule[0].access, disallowed_rule);
+
+ ASSERT_LE(0, ruleset_fd);
+
+ /* Test disallowed path */
+ sqe = io_uring_get_sqe(&ring);
+
+ /* Prepare SQE for the openat operation */
+ io_uring_prep_openat(sqe, AT_FDCWD, paths[0], O_RDONLY, 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(0, ret);
+
+ /* Cleanup: close ruleset fds, etc. */
+ close(ruleset_fd);
+
+ return 0;
+}
+
+FIXTURE(openat_test) {
+ struct __test_metadata *metadata;
+ const char *allowed_paths[11];
+ const char *disallowed_paths[2];
+};
+
+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));
+
+ /* Create necessary directories and files */
+ for (int i = 0; i < sizeof(self->allowed_paths) / sizeof(char *); i++) {
+ create_file(self->metadata, self->allowed_paths[i]);
+ }
+
+ /* Create directories for disallowed paths */
+ for (int i = 0; i < sizeof(self->disallowed_paths) / sizeof(char *); i++) {
+ create_directory(self->metadata, self->disallowed_paths[i]);
+ }
+}
+
+FIXTURE_TEARDOWN(openat_test)
+{
+ /* Clean up test environment */
+ for (int i = 0; i < sizeof(self->allowed_paths) / sizeof(char *); i++) {
+ remove_path(self->allowed_paths[i]);
+ }
+
+ for (int i = 0; i < sizeof(self->disallowed_paths) / sizeof(char *); i++) {
+ remove_path(self->disallowed_paths[i]);
+ }
+}
+
+/* Test IORING_OP_OPENAT. */
+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);
+}
+
+TEST_F_FORK(openat_test, test_ioring_op_openat_disallowed)
+{
+ test_ioring_op_openat(self->metadata, 0, self->disallowed_paths);
+}
+
TEST_HARNESS_MAIN
--
2.25.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v3] selftests/landlock: Add tests for io_uring openat access control with Landlock rules
2024-03-30 12:48 [PATCH v3] selftests/landlock: Add tests for io_uring openat access control with Landlock rules Dorine Tipo
@ 2024-04-02 14:47 ` Mickaël Salaün
2024-04-04 12:16 ` Dorine Tipo
0 siblings, 1 reply; 4+ messages in thread
From: Mickaël Salaün @ 2024-04-02 14:47 UTC (permalink / raw)
To: Dorine Tipo
Cc: skhan, outreachy, Linux-kernel-mentees-request,
fabio.maria.de.francesco, linux-security-module, Paul Moore
On Sat, Mar 30, 2024 at 12:48:17PM +0000, Dorine Tipo wrote:
> This v3 patch expands Landlock test coverage to include io_uring
> operations. It also introduces a test for IORING_OP_OPENAT with Landlock
> rules, verifying allowed and disallowed access. This makes sure that
> IORING_OP_OPENAT is well covered from security vulnerabilities by
> ensuring Landlock controls access through io_uring.
>
> It also updates Makefile to include -luring in the LDLIBS variable.
> This ensures the test code has access to the necessary library for
> io_uring operations.
>
> The patch includes several improvements to the test_ioring_openat(),
> FIXTURE_SETUP() and FIXTURE_TEARDOWN() addressing feedback received on
> the initial version.
>
> Signed-off-by: Dorine Tipo <dorine.a.tipo@gmail.com>
> ---
> Changes since V1:
> V2: - Consolidated two dependent patches in the V1 series into one patch
> as suggested by Fabio.
> - Updated The subject line to be more descriptive.
>
> V3: - Added "selftests/landlock" subject prefix
> - Revised wording in the commit message to accurately reflect the
> test functionality as suggested by Mickaël.
> - Updated the Fixture set_up and tear_down to create and remove the
> necesssary files and folders for testing access.
> - renamed allowed_ruleset_fd and disallowed_ruleset_fd to ruleset_fd
> as suggest by Mickaël.
> - Moved all variable declarations to the top of the function.
> - Refactored the code to test only one allowed and one restricted
> path instead of iterating through all the paths.
> - Added comments to explain what is happening in different blocks
> of code
> - Removed the clang-format markers.
> - Removed unused arguments in the function definition.
> - Added a final rule struct with a null path to the allowed_rule
> and disallowed_rule arrays as suggested by Fabio.
> - CC'd the missing mailing lists as suggested by Shuah.
> - All executables have been included in the .gitignore so no updates
> are necessary.
>
> tools/testing/selftests/landlock/Makefile | 4 +-
> tools/testing/selftests/landlock/fs_test.c | 140 +++++++++++++++++++++
> 2 files changed, 142 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
> diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
> index 9a6036fbf289..7147c75b6f79 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,141 @@ TEST_F_FORK(layout3_fs, release_inodes)
> ASSERT_EQ(EACCES, test_open(TMP_DIR, O_RDONLY));
> }
>
> +/* Test io_uring's IORING_OP_OPENAT access control with landlock rules */
What is tested exactly?
> +static int test_ioring_op_openat(struct __test_metadata *const _metadata, const __u64 access, const char **paths)
> +{
> + int ruleset_fd;
> + int ret;
> + struct io_uring ring;
> + struct io_uring_sqe *sqe;
> +
> + /* Test Allowed Access */
> + const struct rule allowed_rule[] = {
> + {
> + .path = paths[0],
> + .access = access,
> + },
> + {
> + .path = NULL,
> + },
> + };
> +
> + ruleset_fd = create_ruleset(_metadata, allowed_rule[0].access, allowed_rule);
> +
> + ASSERT_LE(0, ruleset_fd);
> +
> + ret = io_uring_queue_init(32, &ring, 0);
> +
> + ASSERT_EQ(0, ret);
> +
> + /* Test allowed path */
> + sqe = io_uring_get_sqe(&ring);
> +
> + /* Prepare SQE for the openat operation */
> + io_uring_prep_openat(sqe, AT_FDCWD, paths[0], O_RDONLY, ruleset_fd);
Can you please explain what this call to io_uring_prep_openat() do?
> +
> + /* 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 = paths[0],
> + .access = 0,
> + },
> + {
> + .path = NULL,
> + },
> + };
> +
> + ruleset_fd = create_ruleset(_metadata, disallowed_rule[0].access, disallowed_rule);
> +
> + ASSERT_LE(0, ruleset_fd);
> +
> + /* Test disallowed path */
> + sqe = io_uring_get_sqe(&ring);
> +
> + /* Prepare SQE for the openat operation */
> + io_uring_prep_openat(sqe, AT_FDCWD, paths[0], O_RDONLY, 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(0, ret);
> +
> + /* Cleanup: close ruleset fds, etc. */
> + close(ruleset_fd);
> +
> + return 0;
> +}
> +
> +FIXTURE(openat_test) {
> + struct __test_metadata *metadata;
> + const char *allowed_paths[11];
> + const char *disallowed_paths[2];
> +};
> +
> +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));
> +
> + /* Create necessary directories and files */
> + for (int i = 0; i < sizeof(self->allowed_paths) / sizeof(char *); i++) {
> + create_file(self->metadata, self->allowed_paths[i]);
> + }
> +
> + /* Create directories for disallowed paths */
> + for (int i = 0; i < sizeof(self->disallowed_paths) / sizeof(char *); i++) {
> + create_directory(self->metadata, self->disallowed_paths[i]);
> + }
> +}
> +
> +FIXTURE_TEARDOWN(openat_test)
> +{
> + /* Clean up test environment */
> + for (int i = 0; i < sizeof(self->allowed_paths) / sizeof(char *); i++) {
> + remove_path(self->allowed_paths[i]);
> + }
> +
> + for (int i = 0; i < sizeof(self->disallowed_paths) / sizeof(char *); i++) {
> + remove_path(self->disallowed_paths[i]);
> + }
> +}
> +
> +/* Test IORING_OP_OPENAT. */
> +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);
> +}
> +
> +TEST_F_FORK(openat_test, test_ioring_op_openat_disallowed)
> +{
> + test_ioring_op_openat(self->metadata, 0, self->disallowed_paths);
> +}
> +
> TEST_HARNESS_MAIN
> --
> 2.25.1
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* (no subject)
2024-04-02 14:47 ` Mickaël Salaün
@ 2024-04-04 12:16 ` Dorine Tipo
2024-04-04 13:08 ` [PATCH v3] selftests/landlock: Add tests for io_uring openat access control with Landlock rules Mickaël Salaün
0 siblings, 1 reply; 4+ messages in thread
From: Dorine Tipo @ 2024-04-04 12:16 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Dorine Tipo, Linux-kernel-mentees-request,
fabio.maria.de.francesco, skhan, outreachy,
linux-security-module, Paul Moore
Subject: [PATCH v3] selftests/landlock: Add tests for io_uring openat access control with Landlock rules
> What is tested exactly?
The test_ioring_op_openat() function is testing the enforcement of
Landlock rules for read-only access.
It's specifically checking whether the Landlock rules correctly allow or
deny read-only access to the specified file when using the openat system
call with io_uring. The test does this by preparing a submission queue
entry for the openat operation with io_uring_prep_openat(), submitting
this operation to the kernel via io_uring, and then checking the result.
If the operation is successful, it means that Landlock allowed the read-only
access as expected. If the operation fails, it means that Landlock correctly
denied the access.
> +static int test_ioring_op_openat(struct __test_metadata *const _metadata, const __u64 access, const char **paths)
> +{
> + int ruleset_fd;
> + int ret;
> + struct io_uring ring;
> + struct io_uring_sqe *sqe;
> +
> + /* Test Allowed Access */
> + const struct rule allowed_rule[] = {
> + {
> + .path = paths[0],
> + .access = access,
> + },
> + {
> + .path = NULL,
> + },
> + };
> +
> + ruleset_fd = create_ruleset(_metadata, allowed_rule[0].access, allowed_rule);
> +
> + ASSERT_LE(0, ruleset_fd);
> +
> + ret = io_uring_queue_init(32, &ring, 0);
> +
> + ASSERT_EQ(0, ret);
> +
> + /* Test allowed path */
> + sqe = io_uring_get_sqe(&ring);
> +
> + /* Prepare SQE for the openat operation */
> + io_uring_prep_openat(sqe, AT_FDCWD, paths[0], O_RDONLY, ruleset_fd);
> Can you please explain what this call to io_uring_prep_openat() do?
io_uring_prep_openat() prepares the submission queue entry for the openat
system call. In my tests the call to io_uring_prep_openat() is preparing
an openat operation to open a file at a certain path with read-only access.
This operation is then submitted to the kernel via io_uring, and the test
checks whether the operation is allowed or denied based on the define
Landlock rules.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v3] selftests/landlock: Add tests for io_uring openat access control with Landlock rules
2024-04-04 12:16 ` Dorine Tipo
@ 2024-04-04 13:08 ` Mickaël Salaün
0 siblings, 0 replies; 4+ messages in thread
From: Mickaël Salaün @ 2024-04-04 13:08 UTC (permalink / raw)
To: Dorine Tipo
Cc: Linux-kernel-mentees-request, fabio.maria.de.francesco, skhan,
outreachy, linux-security-module, Paul Moore
> Subject: Re: your mail
There is an issue with the subject.
On Thu, Apr 04, 2024 at 12:16:21PM +0000, Dorine Tipo wrote:
>
> > What is tested exactly?
>
> The test_ioring_op_openat() function is testing the enforcement of
> Landlock rules for read-only access.
> It's specifically checking whether the Landlock rules correctly allow or
> deny read-only access to the specified file when using the openat system
> call with io_uring. The test does this by preparing a submission queue
> entry for the openat operation with io_uring_prep_openat(), submitting
> this operation to the kernel via io_uring, and then checking the result.
> If the operation is successful, it means that Landlock allowed the read-only
> access as expected. If the operation fails, it means that Landlock correctly
> denied the access.
That doesn't reflect the content of this patch.
>
> > +static int test_ioring_op_openat(struct __test_metadata *const _metadata, const __u64 access, const char **paths)
> > +{
> > + int ruleset_fd;
> > + int ret;
> > + struct io_uring ring;
> > + struct io_uring_sqe *sqe;
> > +
> > + /* Test Allowed Access */
> > + const struct rule allowed_rule[] = {
> > + {
> > + .path = paths[0],
> > + .access = access,
> > + },
> > + {
> > + .path = NULL,
> > + },
> > + };
> > +
> > + ruleset_fd = create_ruleset(_metadata, allowed_rule[0].access, allowed_rule);
> > +
> > + ASSERT_LE(0, ruleset_fd);
> > +
> > + ret = io_uring_queue_init(32, &ring, 0);
> > +
> > + ASSERT_EQ(0, ret);
> > +
> > + /* Test allowed path */
> > + sqe = io_uring_get_sqe(&ring);
> > +
> > + /* Prepare SQE for the openat operation */
> > + io_uring_prep_openat(sqe, AT_FDCWD, paths[0], O_RDONLY, ruleset_fd);
>
> > Can you please explain what this call to io_uring_prep_openat() do?
>
> io_uring_prep_openat() prepares the submission queue entry for the openat
> system call. In my tests the call to io_uring_prep_openat() is preparing
> an openat operation to open a file at a certain path with read-only access.
> This operation is then submitted to the kernel via io_uring, and the test
> checks whether the operation is allowed or denied based on the define
> Landlock rules.
This is not what is tested by this patch.
What is the last argument of io_uring_prep_openat()? Did you write this
code?
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-04-04 13:28 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-03-30 12:48 [PATCH v3] selftests/landlock: Add tests for io_uring openat access control with Landlock rules Dorine Tipo
2024-04-02 14:47 ` Mickaël Salaün
2024-04-04 12:16 ` Dorine Tipo
2024-04-04 13:08 ` [PATCH v3] selftests/landlock: Add tests for io_uring openat access control with Landlock rules Mickaël Salaün
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).