* [PATCH 1/2] Add test for io_uring openat access control with Landlock rules
2024-03-25 2:32 [PATCH 0/2] Add io_uring test with Landlock access control rules Dorine Tipo
@ 2024-03-25 2:32 ` Dorine Tipo
2024-03-25 2:32 ` [PATCH 2/2] Add liburing dependency for io_uring test Dorine Tipo
2024-03-26 10:21 ` [PATCH 0/2] Add io_uring test with Landlock access control rules Fabio M. De Francesco
2 siblings, 0 replies; 4+ messages in thread
From: Dorine Tipo @ 2024-03-25 2:32 UTC (permalink / raw)
To: mic, skhan, outreachy; +Cc: 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
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH 0/2] Add io_uring test with Landlock access control rules
2024-03-25 2:32 [PATCH 0/2] Add io_uring test with Landlock access control rules Dorine Tipo
2024-03-25 2:32 ` [PATCH 1/2] Add test for io_uring openat access control with Landlock rules Dorine Tipo
2024-03-25 2:32 ` [PATCH 2/2] Add liburing dependency for io_uring test Dorine Tipo
@ 2024-03-26 10:21 ` Fabio M. De Francesco
2 siblings, 0 replies; 4+ messages in thread
From: Fabio M. De Francesco @ 2024-03-26 10:21 UTC (permalink / raw)
To: Dorine Tipo; +Cc: mic, skhan, outreachy, Dorine Tipo
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
^ permalink raw reply [flat|nested] 4+ messages in thread