Disconnected files or directories can appear when they are visible and opened from a bind mount, but have been renamed or moved from the source of the bind mount in a way that makes them inaccessible from the mount point (i.e. out of scope). Previously, access rights tied to files or directories opened through a disconnected directory were collected by walking the related hierarchy down to the root of the filesystem, without taking into account the mount point because it couldn't be found. This could lead to inconsistent access results, potential access right widening, and hard-to-debug renames, especially since such paths cannot be printed. For a sandboxed task to create a disconnected directory, it needs to have write access (i.e. FS_MAKE_REG, FS_REMOVE_FILE, and FS_REFER) to the underlying source of the bind mount, and read access to the related mount point. Because a sandboxed task cannot acquire more access rights than those defined by its Landlock domain, this could lead to inconsistent access rights due to missing permissions that should be inherited from the mount point hierarchy, while inheriting permissions from the filesystem hierarchy hidden by this mount point instead. Landlock now handles files and directories opened from disconnected directories by taking into account the filesystem hierarchy when the mount point is not found in the hierarchy walk, and also always taking into account the mount point from which these disconnected directories were opened. This ensures that a rename is not allowed if it would widen access rights [1]. The rationale is that, even if disconnected hierarchies might not be visible or accessible to a sandboxed task, relying on the collected access rights from them improves the guarantee that access rights will not be widened during a rename because of the access right comparison between the source and the destination (see LANDLOCK_ACCESS_FS_REFER). It may look like this would grant more access on disconnected files and directories, but the security policies are always enforced for all the evaluated hierarchies. This new behavior should be less surprising to users and safer from an access control perspective. Remove a wrong WARN_ON_ONCE() canary in collect_domain_accesses() and fix the related comment. Because opened files have their access rights stored in the related file security properties, there is no impact for disconnected or unlinked files. Cc: Christian Brauner Cc: Günther Noack Cc: Song Liu Reported-by: Tingmao Wang Closes: https://lore.kernel.org/r/027d5190-b37a-40a8-84e9-4ccbc352bcdf@maowtm.org Closes: https://lore.kernel.org/r/09b24128f86973a6022e6aa8338945fcfb9a33e4.1749925391.git.m@maowtm.org Fixes: b91c3e4ea756 ("landlock: Add support for file reparenting with LANDLOCK_ACCESS_FS_REFER") Fixes: cb2c7d1a1776 ("landlock: Support filesystem access-control") Link: https://lore.kernel.org/r/b0f46246-f2c5-42ca-93ce-0d629702a987@maowtm.org [1] Signed-off-by: Mickaël Salaün --- Changes since v3: - Simplify the approach by checking both the filesystem hierarchy and the mount point hierarchy for disconnected directories. Remove the related snapshot mechanism. See [1]. - Update erratum. - Do not expose path_connected() anymore, and remove related Acked-by. Changes since v2: - Fix domain check mode reset, spotted by Tingmao. Replace a conditional branch (when all accesses are granted) by only looking for a rule when needed. This simplifies code and remove a call to path_connected(). Add a new is_dom_check_bkp to safely handle race conditions and move initial backup for layer_masks_parent1. - Fix layer masks parent backup initialization, spotted by Tingmao. - Add more comments, suggested by Tingmao. - Reformat erratum. Changes since v1: - Remove useless else branch in is_access_to_paths_allowed(). - Update commit message and squash "landlock: Remove warning in collect_domain_accesses()": https://lore.kernel.org/r/20250618134734.1673254-1-mic@digikod.net - Remove "extern" for path_connected() in fs.h, requested by Christian. - Add Acked-by Christian. - Fix docstring and improve doc for collect_domain_accesses(). - Remove path_connected() check for the real root. - Fix allowed_parent* resets to be consistent with their initial values infered from the evaluated domain. - Add Landlock erratum. Cc: Christian Brauner Cc: Günther Noack Cc: Song Liu Reported-by: Tingmao Wang Closes: https://lore.kernel.org/r/027d5190-b37a-40a8-84e9-4ccbc352bcdf@maowtm.org Closes: https://lore.kernel.org/r/09b24128f86973a6022e6aa8338945fcfb9a33e4.1749925391.git.m@maowtm.org Fixes: b91c3e4ea756 ("landlock: Add support for file reparenting with LANDLOCK_ACCESS_FS_REFER") Fixes: cb2c7d1a1776 ("landlock: Support filesystem access-control") Link: https://lore.kernel.org/r/b0f46246-f2c5-42ca-93ce-0d629702a987@maowtm.org [1] Signed-off-by: Mickaël Salaün --- Changes since v3: - Simplify the approach by checking both the filesystem hierarchy and the mount point hierarchy for disconnected directories. Remove the related snapshot mechanism. See [1]. - Do not expose path_connected() anymore. Changes since v2: - Fix domain check mode reset, spotted by Tingmao. Replace a conditional branch (when all accesses are granted) by only looking for a rule when needed. This simplifies code and remove a call to path_connected(). Add a new is_dom_check_bkp to safely handle race conditions and move initial backup for layer_masks_parent1. - Fix layer masks parent backup initialization, spotted by Tingmao. - Add more comments, suggested by Tingmao. - Reformat erratum. Changes since v1: - Remove useless else branch in is_access_to_paths_allowed(). - Update commit message and squash "landlock: Remove warning in collect_domain_accesses()": https://lore.kernel.org/r/20250618134734.1673254-1-mic@digikod.net - Remove "extern" for path_connected() in fs.h, requested by Christian. - Add Acked-by Christian. - Fix docstring and improve doc for collect_domain_accesses(). - Remove path_connected() check for the real root. - Fix allowed_parent* resets to be consistent with their initial values infered from the evaluated domain. - Add Landlock erratum. --- security/landlock/errata/abi-1.h | 16 +++++++++++++ security/landlock/fs.c | 40 ++++++++++++++++++++++---------- 2 files changed, 44 insertions(+), 12 deletions(-) create mode 100644 security/landlock/errata/abi-1.h diff --git a/security/landlock/errata/abi-1.h b/security/landlock/errata/abi-1.h new file mode 100644 index 000000000000..e8a2bff2e5b6 --- /dev/null +++ b/security/landlock/errata/abi-1.h @@ -0,0 +1,16 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +/** + * DOC: erratum_3 + * + * Erratum 3: Disconnected directory handling + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + * + * This fix addresses an issue with disconnected directories that occur when a + * directory is moved outside the scope of a bind mount. The change ensures + * that evaluated access rights include both those from the disconnected file + * hierarchy down to its filesystem root and those from the related mount point + * hierarchy. This prevents access right widening through rename or link + * actions. + */ +LANDLOCK_ERRATUM(3) diff --git a/security/landlock/fs.c b/security/landlock/fs.c index d9c12b993fa7..26d5c274a4c9 100644 --- a/security/landlock/fs.c +++ b/security/landlock/fs.c @@ -909,21 +909,31 @@ static bool is_access_to_paths_allowed( break; } } + if (unlikely(IS_ROOT(walker_path.dentry))) { - /* - * Stops at disconnected root directories. Only allows - * access to internal filesystems (e.g. nsfs, which is - * reachable through /proc//ns/). - */ - if (walker_path.mnt->mnt_flags & MNT_INTERNAL) { + if (likely(walker_path.mnt->mnt_flags & MNT_INTERNAL)) { + /* + * Stops and allows access when reaching disconnected root + * directories that are part of internal filesystems (e.g. nsfs, + * which is reachable through /proc//ns/). + */ allowed_parent1 = true; allowed_parent2 = true; + break; } - break; + + /* + * We reached a disconnected root directory from a bind mount. + * Let's continue the walk with the current mount root. + */ + dput(walker_path.dentry); + walker_path.dentry = walker_path.mnt->mnt_root; + dget(walker_path.dentry); + } else { + parent_dentry = dget_parent(walker_path.dentry); + dput(walker_path.dentry); + walker_path.dentry = parent_dentry; } - parent_dentry = dget_parent(walker_path.dentry); - dput(walker_path.dentry); - walker_path.dentry = parent_dentry; } path_put(&walker_path); @@ -1021,6 +1031,9 @@ static access_mask_t maybe_remove(const struct dentry *const dentry) * file. While walking from @dir to @mnt_root, we record all the domain's * allowed accesses in @layer_masks_dom. * + * Because of disconnected directories, this walk may not reach @mnt_dir. In + * this case, the walk will continue to @mnt_dir after this call. + * * This is similar to is_access_to_paths_allowed() but much simpler because it * only handles walking on the same mount point and only checks one set of * accesses. @@ -1062,8 +1075,11 @@ static bool collect_domain_accesses( break; } - /* We should not reach a root other than @mnt_root. */ - if (dir == mnt_root || WARN_ON_ONCE(IS_ROOT(dir))) + /* + * Stops at the mount point or the filesystem root for a disconnected + * directory. + */ + if (dir == mnt_root || unlikely(IS_ROOT(dir))) break; parent_dentry = dget_parent(dir); -- 2.51.0 This is now possible thanks to the disconnected directory fix. Cc: Günther Noack Cc: Song Liu Cc: Tingmao Wang Signed-off-by: Mickaël Salaün --- Changes since v3: - New patch extracted from the previous one. --- security/landlock/fs.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/security/landlock/fs.c b/security/landlock/fs.c index 26d5c274a4c9..ee2fa7382a9b 100644 --- a/security/landlock/fs.c +++ b/security/landlock/fs.c @@ -837,7 +837,6 @@ static bool is_access_to_paths_allowed( * restriction. */ while (true) { - struct dentry *parent_dentry; const struct landlock_rule *rule; /* @@ -930,7 +929,9 @@ static bool is_access_to_paths_allowed( walker_path.dentry = walker_path.mnt->mnt_root; dget(walker_path.dentry); } else { - parent_dentry = dget_parent(walker_path.dentry); + struct dentry *const parent_dentry = + dget_parent(walker_path.dentry); + dput(walker_path.dentry); walker_path.dentry = parent_dentry; } -- 2.51.0 From: Tingmao Wang This adds tests for the edge case discussed in [1], with specific ones for rename and link operations when the operands are through disconnected paths, as that go through a separate code path in Landlock. This has resulted in a warning, due to collect_domain_accesses() not expecting to reach a different root from path->mnt: # RUN layout1_bind.path_disconnected ... # OK layout1_bind.path_disconnected ok 96 layout1_bind.path_disconnected # RUN layout1_bind.path_disconnected_rename ... [..] ------------[ cut here ]------------ [..] WARNING: CPU: 3 PID: 385 at security/landlock/fs.c:1065 collect_domain_accesses [..] ... [..] RIP: 0010:collect_domain_accesses (security/landlock/fs.c:1065 (discriminator 2) security/landlock/fs.c:1031 (discriminator 2)) [..] current_check_refer_path (security/landlock/fs.c:1205) [..] ... [..] hook_path_rename (security/landlock/fs.c:1526) [..] security_path_rename (security/security.c:2026 (discriminator 1)) [..] do_renameat2 (fs/namei.c:5264) # OK layout1_bind.path_disconnected_rename ok 97 layout1_bind.path_disconnected_rename Move the const char definitions a bit above so that we can use the path for s4d1 in cleanup code. Cc: Günther Noack Cc: Song Liu Link: https://lore.kernel.org/r/027d5190-b37a-40a8-84e9-4ccbc352bcdf@maowtm.org [1] Signed-off-by: Tingmao Wang Signed-off-by: Mickaël Salaün --- Changes since v3: - Update layout1_bind.path_disconnected tests according to the semantic change of the fix. - Update documentation for the layout1_bind hierarchy with the new s4d1 directory. Changes since v1: - Integrate this patch into my patch series, and change the result for two tests with updated comments. Diff here: https://lore.kernel.org/r/20250701183812.3201231-2-mic@digikod.net - Replace most ASSERT with EXPECT, add extra checks, massage commit message and comments. - Squash Tingmao's patches: https://lore.kernel.org/r/09b24128f86973a6022e6aa8338945fcfb9a33e4.1749925391.git.m@maowtm.org https://lore.kernel.org/r/8ed0bfcd-aefa-44bd-86b6-e12583779187@maowtm.org https://lore.kernel.org/r/3080e512-64b0-42cf-b379-8f52cfeff78a@maowtm.org --- tools/testing/selftests/landlock/fs_test.c | 423 ++++++++++++++++++++- 1 file changed, 415 insertions(+), 8 deletions(-) diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c index fa0f18ec62c4..032dd5dcf5eb 100644 --- a/tools/testing/selftests/landlock/fs_test.c +++ b/tools/testing/selftests/landlock/fs_test.c @@ -4561,6 +4561,18 @@ TEST_F_FORK(ioctl, handle_file_access_file) FIXTURE(layout1_bind) {}; /* clang-format on */ +static const char bind_dir_s1d3[] = TMP_DIR "/s2d1/s2d2/s1d3"; +static const char bind_file1_s1d3[] = TMP_DIR "/s2d1/s2d2/s1d3/f1"; + +/* Move targets for disconnected path tests. */ +static const char dir_s4d1[] = TMP_DIR "/s4d1"; +static const char file1_s4d1[] = TMP_DIR "/s4d1/f1"; +static const char file2_s4d1[] = TMP_DIR "/s4d1/f2"; +static const char dir_s4d2[] = TMP_DIR "/s4d1/s4d2"; +static const char file1_s4d2[] = TMP_DIR "/s4d1/s4d2/f1"; +static const char file1_name[] = "f1"; +static const char file2_name[] = "f2"; + FIXTURE_SETUP(layout1_bind) { prepare_layout(_metadata); @@ -4576,14 +4588,14 @@ FIXTURE_TEARDOWN_PARENT(layout1_bind) { /* umount(dir_s2d2)) is handled by namespace lifetime. */ + remove_path(file1_s4d1); + remove_path(file2_s4d1); + remove_layout1(_metadata); cleanup_layout(_metadata); } -static const char bind_dir_s1d3[] = TMP_DIR "/s2d1/s2d2/s1d3"; -static const char bind_file1_s1d3[] = TMP_DIR "/s2d1/s2d2/s1d3/f1"; - /* * layout1_bind hierarchy: * @@ -4594,20 +4606,25 @@ static const char bind_file1_s1d3[] = TMP_DIR "/s2d1/s2d2/s1d3/f1"; * │   └── s1d2 * │   ├── f1 * │   ├── f2 - * │   └── s1d3 + * │   └── s1d3 [disconnected by path_disconnected] * │   ├── f1 * │   └── f2 * ├── s2d1 * │   ├── f1 - * │   └── s2d2 + * │   └── s2d2 [bind mount from s1d2] * │   ├── f1 * │   ├── f2 * │   └── s1d3 * │   ├── f1 * │   └── f2 - * └── s3d1 - * └── s3d2 - * └── s3d3 + * ├── s3d1 + * │   └── s3d2 + * │   └── s3d3 + * └── s4d1 [renamed from s1d3 by path_disconnected] + *    ├── f1 + *    ├── f2 + * └── s4d2 + * └── f1 */ TEST_F_FORK(layout1_bind, no_restriction) @@ -4806,6 +4823,396 @@ TEST_F_FORK(layout1_bind, reparent_cross_mount) ASSERT_EQ(0, rename(bind_file1_s1d3, file1_s2d2)); } +/* + * Make sure access to file through a disconnected path works as expected. + * This test moves s1d3 to s4d1. + */ +TEST_F_FORK(layout1_bind, path_disconnected) +{ + const struct rule layer1_allow_all[] = { + { + .path = TMP_DIR, + .access = ACCESS_ALL, + }, + {}, + }; + const struct rule layer2_allow_just_f1[] = { + { + .path = file1_s1d3, + .access = LANDLOCK_ACCESS_FS_READ_FILE, + }, + {}, + }; + const struct rule layer3_only_s1d2[] = { + { + .path = dir_s1d2, + .access = LANDLOCK_ACCESS_FS_READ_FILE, + }, + {}, + }; + + /* Landlock should not deny access just because it is disconnected. */ + int ruleset_fd_l1 = + create_ruleset(_metadata, ACCESS_ALL, layer1_allow_all); + + /* Creates the new ruleset now before we move the dir containing the file. */ + int ruleset_fd_l2 = + create_ruleset(_metadata, ACCESS_RW, layer2_allow_just_f1); + int ruleset_fd_l3 = + create_ruleset(_metadata, ACCESS_RW, layer3_only_s1d2); + int bind_s1d3_fd; + + ASSERT_LE(0, ruleset_fd_l1); + ASSERT_LE(0, ruleset_fd_l2); + ASSERT_LE(0, ruleset_fd_l3); + + enforce_ruleset(_metadata, ruleset_fd_l1); + EXPECT_EQ(0, close(ruleset_fd_l1)); + + bind_s1d3_fd = open(bind_dir_s1d3, O_PATH | O_CLOEXEC); + ASSERT_LE(0, bind_s1d3_fd); + + /* Tests access is possible before we move. */ + EXPECT_EQ(0, test_open_rel(bind_s1d3_fd, file1_name, O_RDONLY)); + EXPECT_EQ(0, test_open_rel(bind_s1d3_fd, file2_name, O_RDONLY)); + EXPECT_EQ(0, test_open_rel(bind_s1d3_fd, "..", O_RDONLY | O_DIRECTORY)); + + /* Makes it disconnected. */ + ASSERT_EQ(0, rename(dir_s1d3, dir_s4d1)) + { + TH_LOG("Failed to rename %s to %s: %s", dir_s1d3, dir_s4d1, + strerror(errno)); + } + + /* Tests that access is still possible. */ + EXPECT_EQ(0, test_open_rel(bind_s1d3_fd, file1_name, O_RDONLY)); + EXPECT_EQ(0, test_open_rel(bind_s1d3_fd, file2_name, O_RDONLY)); + + /* + * Tests that ".." is not possible (not because of Landlock, but just + * because it's disconnected). + */ + EXPECT_EQ(ENOENT, + test_open_rel(bind_s1d3_fd, "..", O_RDONLY | O_DIRECTORY)); + + /* This should still work with a narrower rule. */ + enforce_ruleset(_metadata, ruleset_fd_l2); + EXPECT_EQ(0, close(ruleset_fd_l2)); + + EXPECT_EQ(0, test_open(file1_s4d1, O_RDONLY)); + /* + * Accessing a file through a disconnected file descriptor can still be + * allowed by a rule tied to this file, even if it is no longer visible in + * its mount point. + */ + EXPECT_EQ(0, test_open_rel(bind_s1d3_fd, file1_name, O_RDONLY)); + EXPECT_EQ(EACCES, test_open_rel(bind_s1d3_fd, file2_name, O_RDONLY)); + + enforce_ruleset(_metadata, ruleset_fd_l3); + EXPECT_EQ(0, close(ruleset_fd_l3)); + + EXPECT_EQ(EACCES, test_open(file1_s4d1, O_RDONLY)); + /* + * Accessing a file through a disconnected file descriptor can still be + * allowed by a rule tied to the original mount point, even if it is no + * longer visible in its mount point. + */ + EXPECT_EQ(0, test_open_rel(bind_s1d3_fd, file1_name, O_RDONLY)); + EXPECT_EQ(EACCES, test_open_rel(bind_s1d3_fd, file2_name, O_RDONLY)); +} + +/* + * Test that renameat with disconnected paths works under Landlock. This test + * moves s1d3 to s4d2, so that we can have a rule allowing refers on the move + * target's immediate parent. + */ +TEST_F_FORK(layout1_bind, path_disconnected_rename) +{ + const struct rule layer1[] = { + { + .path = dir_s1d2, + .access = LANDLOCK_ACCESS_FS_REFER | + LANDLOCK_ACCESS_FS_MAKE_DIR | + LANDLOCK_ACCESS_FS_REMOVE_DIR | + LANDLOCK_ACCESS_FS_MAKE_REG | + LANDLOCK_ACCESS_FS_REMOVE_FILE | + LANDLOCK_ACCESS_FS_READ_FILE, + }, + { + .path = dir_s4d1, + .access = LANDLOCK_ACCESS_FS_REFER | + LANDLOCK_ACCESS_FS_MAKE_DIR | + LANDLOCK_ACCESS_FS_REMOVE_DIR | + LANDLOCK_ACCESS_FS_MAKE_REG | + LANDLOCK_ACCESS_FS_REMOVE_FILE | + LANDLOCK_ACCESS_FS_READ_FILE, + }, + {} + }; + + /* This layer only handles LANDLOCK_ACCESS_FS_READ_FILE. */ + const struct rule layer2_only_s1d2[] = { + { + .path = dir_s1d2, + .access = LANDLOCK_ACCESS_FS_READ_FILE, + }, + {}, + }; + int ruleset_fd_l1, ruleset_fd_l2; + pid_t child_pid; + int bind_s1d3_fd, status; + + ASSERT_EQ(0, mkdir(dir_s4d1, 0755)) + { + TH_LOG("Failed to create %s: %s", dir_s4d1, strerror(errno)); + } + ruleset_fd_l1 = create_ruleset(_metadata, ACCESS_ALL, layer1); + ruleset_fd_l2 = create_ruleset(_metadata, LANDLOCK_ACCESS_FS_READ_FILE, + layer2_only_s1d2); + ASSERT_LE(0, ruleset_fd_l1); + ASSERT_LE(0, ruleset_fd_l2); + + enforce_ruleset(_metadata, ruleset_fd_l1); + EXPECT_EQ(0, close(ruleset_fd_l1)); + + bind_s1d3_fd = open(bind_dir_s1d3, O_PATH | O_CLOEXEC); + ASSERT_LE(0, bind_s1d3_fd); + EXPECT_EQ(0, test_open_rel(bind_s1d3_fd, file1_name, O_RDONLY)); + + /* Tests ENOENT priority over EACCES for disconnected directory. */ + EXPECT_EQ(EACCES, test_open_rel(bind_s1d3_fd, "..", O_DIRECTORY)); + ASSERT_EQ(0, rename(dir_s1d3, dir_s4d2)) + { + TH_LOG("Failed to rename %s to %s: %s", dir_s1d3, dir_s4d2, + strerror(errno)); + } + EXPECT_EQ(ENOENT, test_open_rel(bind_s1d3_fd, "..", O_DIRECTORY)); + + /* + * The file is no longer under s1d2 but we should still be able to access it + * with layer 2 because its mount point is evaluated as the first valid + * directory because it was initially a parent. Do a fork to test this so + * we don't prevent ourselves from renaming it back later. + */ + child_pid = fork(); + ASSERT_LE(0, child_pid); + if (child_pid == 0) { + enforce_ruleset(_metadata, ruleset_fd_l2); + EXPECT_EQ(0, close(ruleset_fd_l2)); + EXPECT_EQ(0, test_open_rel(bind_s1d3_fd, file1_name, O_RDONLY)); + EXPECT_EQ(EACCES, test_open(file1_s4d2, O_RDONLY)); + + /* + * Tests that access widening checks indeed prevents us from renaming it + * back. + */ + EXPECT_EQ(-1, rename(dir_s4d2, dir_s1d3)); + EXPECT_EQ(EXDEV, errno); + + /* + * Including through the now disconnected fd (but it should return + * EXDEV). + */ + EXPECT_EQ(-1, renameat(bind_s1d3_fd, file1_name, AT_FDCWD, + file1_s2d2)); + EXPECT_EQ(EXDEV, errno); + _exit(_metadata->exit_code); + return; + } + + EXPECT_EQ(child_pid, waitpid(child_pid, &status, 0)); + EXPECT_EQ(1, WIFEXITED(status)); + EXPECT_EQ(EXIT_SUCCESS, WEXITSTATUS(status)); + + ASSERT_EQ(0, rename(dir_s4d2, dir_s1d3)) + { + TH_LOG("Failed to rename %s back to %s: %s", dir_s4d1, dir_s1d3, + strerror(errno)); + } + + /* Now checks that we can access it under l2. */ + child_pid = fork(); + ASSERT_LE(0, child_pid); + if (child_pid == 0) { + enforce_ruleset(_metadata, ruleset_fd_l2); + EXPECT_EQ(0, close(ruleset_fd_l2)); + EXPECT_EQ(0, test_open_rel(bind_s1d3_fd, file1_name, O_RDONLY)); + EXPECT_EQ(0, test_open(file1_s1d3, O_RDONLY)); + _exit(_metadata->exit_code); + return; + } + + EXPECT_EQ(child_pid, waitpid(child_pid, &status, 0)); + EXPECT_EQ(1, WIFEXITED(status)); + EXPECT_EQ(EXIT_SUCCESS, WEXITSTATUS(status)); + + /* + * Also test that we can rename via a disconnected path. We move the + * dir back to the disconnected place first, then we rename file1 to + * file2 through our dir fd. + */ + ASSERT_EQ(0, rename(dir_s1d3, dir_s4d2)) + { + TH_LOG("Failed to rename %s to %s: %s", dir_s1d3, dir_s4d2, + strerror(errno)); + } + ASSERT_EQ(0, + renameat(bind_s1d3_fd, file1_name, bind_s1d3_fd, file2_name)) + { + TH_LOG("Failed to rename %s to %s within disconnected %s: %s", + file1_name, file2_name, bind_dir_s1d3, strerror(errno)); + } + EXPECT_EQ(0, test_open_rel(bind_s1d3_fd, file2_name, O_RDONLY)); + ASSERT_EQ(0, renameat(bind_s1d3_fd, file2_name, AT_FDCWD, file1_s2d2)) + { + TH_LOG("Failed to rename %s to %s through disconnected %s: %s", + file2_name, file1_s2d2, bind_dir_s1d3, strerror(errno)); + } + EXPECT_EQ(0, test_open(file1_s2d2, O_RDONLY)); + EXPECT_EQ(0, test_open(file1_s1d2, O_RDONLY)); + + /* Move it back using the disconnected path as the target. */ + ASSERT_EQ(0, renameat(AT_FDCWD, file1_s2d2, bind_s1d3_fd, file1_name)) + { + TH_LOG("Failed to rename %s to %s through disconnected %s: %s", + file1_s1d2, file1_name, bind_dir_s1d3, strerror(errno)); + } + + /* Now make it connected again. */ + ASSERT_EQ(0, rename(dir_s4d2, dir_s1d3)) + { + TH_LOG("Failed to rename %s back to %s: %s", dir_s4d2, dir_s1d3, + strerror(errno)); + } + + /* Checks again that we can access it under l2. */ + enforce_ruleset(_metadata, ruleset_fd_l2); + EXPECT_EQ(0, close(ruleset_fd_l2)); + EXPECT_EQ(0, test_open_rel(bind_s1d3_fd, file1_name, O_RDONLY)); + EXPECT_EQ(0, test_open(file1_s1d3, O_RDONLY)); +} + +/* + * Test that linkat(2) with disconnected paths works under Landlock. This + * test moves s1d3 to s4d1. + */ +TEST_F_FORK(layout1_bind, path_disconnected_link) +{ + /* Ruleset to be applied after renaming s1d3 to s4d1. */ + const struct rule layer1[] = { + { + .path = dir_s4d1, + .access = LANDLOCK_ACCESS_FS_REFER | + LANDLOCK_ACCESS_FS_READ_FILE | + LANDLOCK_ACCESS_FS_MAKE_REG | + LANDLOCK_ACCESS_FS_REMOVE_FILE, + }, + { + .path = dir_s2d2, + .access = LANDLOCK_ACCESS_FS_REFER | + LANDLOCK_ACCESS_FS_READ_FILE | + LANDLOCK_ACCESS_FS_MAKE_REG | + LANDLOCK_ACCESS_FS_REMOVE_FILE, + }, + {} + }; + int ruleset_fd, bind_s1d3_fd; + + /* Removes unneeded files created by layout1, otherwise it will EEXIST. */ + ASSERT_EQ(0, unlink(file1_s1d2)); + ASSERT_EQ(0, unlink(file2_s1d3)); + + bind_s1d3_fd = open(bind_dir_s1d3, O_PATH | O_CLOEXEC); + ASSERT_LE(0, bind_s1d3_fd); + EXPECT_EQ(0, test_open_rel(bind_s1d3_fd, file1_name, O_RDONLY)); + + /* Disconnects bind_s1d3_fd. */ + ASSERT_EQ(0, rename(dir_s1d3, dir_s4d1)) + { + TH_LOG("Failed to rename %s to %s: %s", dir_s1d3, dir_s4d1, + strerror(errno)); + } + + /* Need this later to test different parent link. */ + ASSERT_EQ(0, mkdir(dir_s4d2, 0755)) + { + TH_LOG("Failed to create %s: %s", dir_s4d2, strerror(errno)); + } + + ruleset_fd = create_ruleset(_metadata, ACCESS_ALL, layer1); + ASSERT_LE(0, ruleset_fd); + enforce_ruleset(_metadata, ruleset_fd); + EXPECT_EQ(0, close(ruleset_fd)); + + /* From disconnected to connected. */ + ASSERT_EQ(0, linkat(bind_s1d3_fd, file1_name, AT_FDCWD, file1_s2d2, 0)) + { + TH_LOG("Failed to link %s to %s via disconnected %s: %s", + file1_name, file1_s2d2, bind_dir_s1d3, strerror(errno)); + } + + /* Tests that we can access via the new link... */ + EXPECT_EQ(0, test_open(file1_s2d2, O_RDONLY)) + { + TH_LOG("Failed to open newly linked %s: %s", file1_s2d2, + strerror(errno)); + } + + /* ...as well as the old one. */ + EXPECT_EQ(0, test_open(file1_s4d1, O_RDONLY)) + { + TH_LOG("Failed to open original %s: %s", file1_s4d1, + strerror(errno)); + } + + /* From connected to disconnected. */ + ASSERT_EQ(0, unlink(file1_s4d1)); + ASSERT_EQ(0, linkat(AT_FDCWD, file1_s2d2, bind_s1d3_fd, file2_name, 0)) + { + TH_LOG("Failed to link %s to %s via disconnected %s: %s", + file1_s2d2, file2_name, bind_dir_s1d3, strerror(errno)); + } + EXPECT_EQ(0, test_open(file2_s4d1, O_RDONLY)); + ASSERT_EQ(0, unlink(file1_s2d2)); + + /* From disconnected to disconnected (same parent). */ + ASSERT_EQ(0, + linkat(bind_s1d3_fd, file2_name, bind_s1d3_fd, file1_name, 0)) + { + TH_LOG("Failed to link %s to %s within disconnected %s: %s", + file2_name, file1_name, bind_dir_s1d3, strerror(errno)); + } + EXPECT_EQ(0, test_open(file1_s4d1, O_RDONLY)) + { + TH_LOG("Failed to open newly linked %s: %s", file1_s4d1, + strerror(errno)); + } + EXPECT_EQ(0, test_open_rel(bind_s1d3_fd, file1_name, O_RDONLY)) + { + TH_LOG("Failed to open %s through newly created link under disconnected path: %s", + file1_name, strerror(errno)); + } + ASSERT_EQ(0, unlink(file2_s4d1)); + + /* From disconnected to disconnected (different parent). */ + ASSERT_EQ(0, + linkat(bind_s1d3_fd, file1_name, bind_s1d3_fd, "s4d2/f1", 0)) + { + TH_LOG("Failed to link %s to %s within disconnected %s: %s", + file1_name, "s4d2/f1", bind_dir_s1d3, strerror(errno)); + } + EXPECT_EQ(0, test_open(file1_s4d2, O_RDONLY)) + { + TH_LOG("Failed to open %s after link: %s", file1_s4d2, + strerror(errno)); + } + EXPECT_EQ(0, test_open_rel(bind_s1d3_fd, "s4d2/f1", O_RDONLY)) + { + TH_LOG("Failed to open %s through disconnected path after link: %s", + "s4d2/f1", strerror(errno)); + } +} + #define LOWER_BASE TMP_DIR "/lower" #define LOWER_DATA LOWER_BASE "/data" static const char lower_fl1[] = LOWER_DATA "/fl1"; -- 2.51.0 Test disconnected directories with two test suites and 31 variants to cover the main corner cases. These tests are complementary to the previous commit. Add test_renameat() and test_exchangeat() helpers. Test coverage for security/landlock is 92.1% of 1927 lines according to LLVM 20. Cc: Günther Noack Cc: Song Liu Cc: Tingmao Wang Signed-off-by: Mickaël Salaün --- Changes since v3: - Update tests to reflect the new approach: * layout4_disconnected_leafs.s1d41_s1d42_disconnected: allow all * layout4_disconnected_leafs.s3d1_s4d1_new_parent: allow all * layout4_disconnected_leafs.f1_f2_f3: allow read * layout5_disconnected_branch.s2d3_mount1_dst_parent: allow all * layout5_disconnected_branch.s4d1_rename_parent: allow all - Update test coverage. Changes since v2: - Update test coverage. Changes since v1: - Rename layout4_disconnected to layout4_disconnected_leafs. - Fix variable names. - Add layout5_disconnected_branch test suite with 19 variants to cover potential implementation issues. --- tools/testing/selftests/landlock/fs_test.c | 912 +++++++++++++++++++++ 1 file changed, 912 insertions(+) diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c index 032dd5dcf5eb..7f9c97219367 100644 --- a/tools/testing/selftests/landlock/fs_test.c +++ b/tools/testing/selftests/landlock/fs_test.c @@ -2267,6 +2267,22 @@ static int test_exchange(const char *const oldpath, const char *const newpath) return 0; } +static int test_renameat(int olddirfd, const char *oldpath, int newdirfd, + const char *newpath) +{ + if (renameat2(olddirfd, oldpath, newdirfd, newpath, 0)) + return errno; + return 0; +} + +static int test_exchangeat(int olddirfd, const char *oldpath, int newdirfd, + const char *newpath) +{ + if (renameat2(olddirfd, oldpath, newdirfd, newpath, RENAME_EXCHANGE)) + return errno; + return 0; +} + TEST_F_FORK(layout1, rename_file) { const struct rule rules[] = { @@ -5213,6 +5229,902 @@ TEST_F_FORK(layout1_bind, path_disconnected_link) } } +/* + * layout4_disconnected_leafs with bind mount and renames: + * + * tmp + * ├── s1d1 + * │   └── s1d2 [source of the bind mount] + * │ ├── s1d31 + * │   │ └── s1d41 [now renamed beneath s3d1] + * │ │ ├── f1 + * │ │ └── f2 + * │   └── s1d32 + * │ └── s1d42 [now renamed beneath s4d1] + * │ ├── f3 + * │ └── f4 + * ├── s2d1 + * │   └── s2d2 [bind mount of s1d2] + * │ ├── s1d31 + * │   │ └── s1d41 [opened FD, now renamed beneath s3d1] + * │ │ ├── f1 + * │ │ └── f2 + * │   └── s1d32 + * │ └── s1d42 [opened FD, now renamed beneath s4d1] + * │ ├── f3 + * │ └── f4 + * ├── s3d1 + * │  └── s1d41 [renamed here] + * │ ├── f1 + * │ └── f2 + * └── s4d1 + * └── s1d42 [renamed here] + * ├── f3 + * └── f4 + */ +/* clang-format off */ +FIXTURE(layout4_disconnected_leafs) { + int s2d2_fd; +}; +/* clang-format on */ + +FIXTURE_SETUP(layout4_disconnected_leafs) +{ + prepare_layout(_metadata); + + create_file(_metadata, TMP_DIR "/s1d1/s1d2/s1d31/s1d41/f1"); + create_file(_metadata, TMP_DIR "/s1d1/s1d2/s1d31/s1d41/f2"); + create_file(_metadata, TMP_DIR "/s1d1/s1d2/s1d32/s1d42/f3"); + create_file(_metadata, TMP_DIR "/s1d1/s1d2/s1d32/s1d42/f4"); + create_directory(_metadata, TMP_DIR "/s2d1/s2d2"); + create_directory(_metadata, TMP_DIR "/s3d1"); + create_directory(_metadata, TMP_DIR "/s4d1"); + + self->s2d2_fd = + open(TMP_DIR "/s2d1/s2d2", O_DIRECTORY | O_PATH | O_CLOEXEC); + ASSERT_LE(0, self->s2d2_fd); + + set_cap(_metadata, CAP_SYS_ADMIN); + ASSERT_EQ(0, mount(TMP_DIR "/s1d1/s1d2", TMP_DIR "/s2d1/s2d2", NULL, + MS_BIND, NULL)); + clear_cap(_metadata, CAP_SYS_ADMIN); +} + +FIXTURE_TEARDOWN_PARENT(layout4_disconnected_leafs) +{ + /* umount(TMP_DIR "/s2d1") is handled by namespace lifetime. */ + + /* Removes files after renames. */ + remove_path(TMP_DIR "/s3d1/s1d41/f1"); + remove_path(TMP_DIR "/s3d1/s1d41/f2"); + remove_path(TMP_DIR "/s4d1/s1d42/f1"); + remove_path(TMP_DIR "/s4d1/s1d42/f3"); + remove_path(TMP_DIR "/s4d1/s1d42/f4"); + remove_path(TMP_DIR "/s4d1/s1d42/f5"); + + cleanup_layout(_metadata); +} + +FIXTURE_VARIANT(layout4_disconnected_leafs) +{ + /* + * Parent of the bind mount source. It should always be ignored when + * testing against files under the s1d41 or s1d42 disconnected directories. + */ + const __u64 allowed_s1d1; + /* + * Source of bind mount (to s2d2). It should always be enforced when + * testing against files under the s1d41 or s1d42 disconnected directories. + */ + const __u64 allowed_s1d2; + /* + * Original parent of s1d41. It should always be ignored when testing + * against files under the s1d41 disconnected directory. + */ + const __u64 allowed_s1d31; + /* + * Original parent of s1d42. It should always be ignored when testing + * against files under the s1d42 disconnected directory. + */ + const __u64 allowed_s1d32; + /* + * Opened and disconnected source directory. It should always be enforced + * when testing against files under the s1d41 disconnected directory. + */ + const __u64 allowed_s1d41; + /* + * Opened and disconnected source directory. It should always be enforced + * when testing against files under the s1d42 disconnected directory. + */ + const __u64 allowed_s1d42; + /* + * File in the s1d41 disconnected directory. It should always be enforced + * when testing against itself under the s1d41 disconnected directory. + */ + const __u64 allowed_f1; + /* + * File in the s1d41 disconnected directory. It should always be enforced + * when testing against itself under the s1d41 disconnected directory. + */ + const __u64 allowed_f2; + /* + * File in the s1d42 disconnected directory. It should always be enforced + * when testing against itself under the s1d42 disconnected directory. + */ + const __u64 allowed_f3; + /* + * Parent of the bind mount destination. It should always be enforced when + * testing against files under the s1d41 or s1d42 disconnected directories. + */ + const __u64 allowed_s2d1; + /* + * Directory covered by the bind mount. It should always be ignored when + * testing against files under the s1d41 or s1d42 disconnected directories. + */ + const __u64 allowed_s2d2; + /* + * New parent of the renamed s1d41. It should always be ignored when + * testing against files under the s1d41 disconnected directory. + */ + const __u64 allowed_s3d1; + /* + * New parent of the renamed s1d42. It should always be ignored when + * testing against files under the s1d42 disconnected directory. + */ + const __u64 allowed_s4d1; + + /* Expected result of the call to open([fd:s1d41]/f1, O_RDONLY). */ + const int expected_read_result; + /* Expected result of the call to renameat([fd:s1d41]/f1, [fd:s1d42]/f1). */ + const int expected_rename_result; + /* + * Expected result of the call to renameat([fd:s1d41]/f2, [fd:s1d42]/f3, + * RENAME_EXCHANGE). + */ + const int expected_exchange_result; + /* Expected result of the call to renameat([fd:s1d42]/f4, [fd:s1d42]/f5). */ + const int expected_same_dir_rename_result; +}; + +/* clang-format off */ +FIXTURE_VARIANT_ADD(layout4_disconnected_leafs, s1d1_mount_src_parent) { + /* clang-format on */ + .allowed_s1d1 = LANDLOCK_ACCESS_FS_REFER | + LANDLOCK_ACCESS_FS_READ_FILE | + LANDLOCK_ACCESS_FS_EXECUTE | + LANDLOCK_ACCESS_FS_MAKE_REG, + .expected_read_result = EACCES, + .expected_same_dir_rename_result = EACCES, + .expected_rename_result = EACCES, + .expected_exchange_result = EACCES, +}; + +/* clang-format off */ +FIXTURE_VARIANT_ADD(layout4_disconnected_leafs, s1d2_mount_src_refer) { + /* clang-format on */ + .allowed_s1d2 = LANDLOCK_ACCESS_FS_REFER | LANDLOCK_ACCESS_FS_READ_FILE, + .expected_read_result = 0, + .expected_same_dir_rename_result = EACCES, + .expected_rename_result = EACCES, + .expected_exchange_result = EACCES, +}; + +/* clang-format off */ +FIXTURE_VARIANT_ADD(layout4_disconnected_leafs, s1d2_mount_src_create) { + /* clang-format on */ + .allowed_s1d2 = LANDLOCK_ACCESS_FS_READ_FILE | + LANDLOCK_ACCESS_FS_MAKE_REG, + .expected_read_result = 0, + .expected_same_dir_rename_result = 0, + .expected_rename_result = EXDEV, + .expected_exchange_result = EXDEV, +}; + +/* clang-format off */ +FIXTURE_VARIANT_ADD(layout4_disconnected_leafs, s1d2_mount_src_rename) { + /* clang-format on */ + .allowed_s1d2 = LANDLOCK_ACCESS_FS_REFER | LANDLOCK_ACCESS_FS_MAKE_REG, + .expected_read_result = EACCES, + .expected_same_dir_rename_result = 0, + .expected_rename_result = 0, + .expected_exchange_result = 0, +}; + +/* clang-format off */ +FIXTURE_VARIANT_ADD(layout4_disconnected_leafs, s1d31_s1d32_old_parent) { + /* clang-format on */ + .allowed_s1d31 = LANDLOCK_ACCESS_FS_REFER | + LANDLOCK_ACCESS_FS_READ_FILE | + LANDLOCK_ACCESS_FS_EXECUTE | + LANDLOCK_ACCESS_FS_MAKE_REG, + .allowed_s1d32 = LANDLOCK_ACCESS_FS_REFER | + LANDLOCK_ACCESS_FS_READ_FILE | + LANDLOCK_ACCESS_FS_EXECUTE | + LANDLOCK_ACCESS_FS_MAKE_REG, + .expected_read_result = EACCES, + .expected_same_dir_rename_result = EACCES, + .expected_rename_result = EACCES, + .expected_exchange_result = EACCES, +}; + +/* clang-format off */ +FIXTURE_VARIANT_ADD(layout4_disconnected_leafs, s1d41_s1d42_disconnected) { + /* clang-format on */ + .allowed_s1d41 = LANDLOCK_ACCESS_FS_REFER | + LANDLOCK_ACCESS_FS_READ_FILE | + LANDLOCK_ACCESS_FS_EXECUTE | + LANDLOCK_ACCESS_FS_MAKE_REG, + .allowed_s1d42 = LANDLOCK_ACCESS_FS_REFER | + LANDLOCK_ACCESS_FS_READ_FILE | + LANDLOCK_ACCESS_FS_EXECUTE | + LANDLOCK_ACCESS_FS_MAKE_REG, + .expected_read_result = 0, + .expected_same_dir_rename_result = 0, + .expected_rename_result = 0, + .expected_exchange_result = 0, +}; + +/* clang-format off */ +FIXTURE_VARIANT_ADD(layout4_disconnected_leafs, s2d1_mount_dst_parent_create) { + /* clang-format on */ + .allowed_s2d1 = LANDLOCK_ACCESS_FS_READ_FILE | + LANDLOCK_ACCESS_FS_MAKE_REG, + .expected_read_result = 0, + .expected_same_dir_rename_result = 0, + .expected_rename_result = EXDEV, + .expected_exchange_result = EXDEV, +}; + +/* clang-format off */ +FIXTURE_VARIANT_ADD(layout4_disconnected_leafs, s2d1_mount_dst_parent_refer) { + /* clang-format on */ + .allowed_s2d1 = LANDLOCK_ACCESS_FS_REFER | LANDLOCK_ACCESS_FS_READ_FILE, + .expected_read_result = 0, + .expected_same_dir_rename_result = EACCES, + .expected_rename_result = EACCES, + .expected_exchange_result = EACCES, +}; + +/* clang-format off */ +FIXTURE_VARIANT_ADD(layout4_disconnected_leafs, s2d1_mount_dst_parent_mini) { + /* clang-format on */ + .allowed_s2d1 = LANDLOCK_ACCESS_FS_REFER | + LANDLOCK_ACCESS_FS_READ_FILE | + LANDLOCK_ACCESS_FS_MAKE_REG, + .expected_read_result = 0, + .expected_same_dir_rename_result = 0, + .expected_rename_result = 0, + .expected_exchange_result = 0, +}; + +/* clang-format off */ +FIXTURE_VARIANT_ADD(layout4_disconnected_leafs, s2d2_covered_by_mount) { + /* clang-format on */ + .allowed_s2d2 = LANDLOCK_ACCESS_FS_REFER | + LANDLOCK_ACCESS_FS_READ_FILE | + LANDLOCK_ACCESS_FS_EXECUTE | + LANDLOCK_ACCESS_FS_MAKE_REG, + .expected_read_result = EACCES, + .expected_same_dir_rename_result = EACCES, + .expected_rename_result = EACCES, + .expected_exchange_result = EACCES, +}; + +/* Tests collect_domain_accesses(). */ +/* clang-format off */ +FIXTURE_VARIANT_ADD(layout4_disconnected_leafs, s3d1_s4d1_new_parent) { + /* clang-format on */ + .allowed_s3d1 = LANDLOCK_ACCESS_FS_REFER | + LANDLOCK_ACCESS_FS_READ_FILE | + LANDLOCK_ACCESS_FS_EXECUTE | + LANDLOCK_ACCESS_FS_MAKE_REG | + LANDLOCK_ACCESS_FS_MAKE_REG, + .allowed_s4d1 = LANDLOCK_ACCESS_FS_REFER | + LANDLOCK_ACCESS_FS_READ_FILE | + LANDLOCK_ACCESS_FS_EXECUTE | + LANDLOCK_ACCESS_FS_MAKE_REG | + LANDLOCK_ACCESS_FS_MAKE_REG, + .expected_read_result = 0, + .expected_same_dir_rename_result = 0, + .expected_rename_result = 0, + .expected_exchange_result = 0, +}; + +/* clang-format off */ +FIXTURE_VARIANT_ADD(layout4_disconnected_leafs, f1_f2_f3) { + /* clang-format on */ + .allowed_f1 = LANDLOCK_ACCESS_FS_READ_FILE, + .allowed_f2 = LANDLOCK_ACCESS_FS_READ_FILE, + .allowed_f3 = LANDLOCK_ACCESS_FS_READ_FILE, + .expected_read_result = 0, + .expected_same_dir_rename_result = EACCES, + .expected_rename_result = EACCES, + .expected_exchange_result = EACCES, +}; + +TEST_F_FORK(layout4_disconnected_leafs, read_rename_exchange) +{ + const __u64 handled_access = + LANDLOCK_ACCESS_FS_REFER | LANDLOCK_ACCESS_FS_READ_FILE | + LANDLOCK_ACCESS_FS_EXECUTE | LANDLOCK_ACCESS_FS_MAKE_REG; + const struct rule rules[] = { + { + .path = TMP_DIR "/s1d1", + .access = variant->allowed_s1d1, + }, + { + .path = TMP_DIR "/s1d1/s1d2", + .access = variant->allowed_s1d2, + }, + { + .path = TMP_DIR "/s1d1/s1d2/s1d31", + .access = variant->allowed_s1d31, + }, + { + .path = TMP_DIR "/s1d1/s1d2/s1d32", + .access = variant->allowed_s1d32, + }, + { + .path = TMP_DIR "/s1d1/s1d2/s1d31/s1d41", + .access = variant->allowed_s1d41, + }, + { + .path = TMP_DIR "/s1d1/s1d2/s1d32/s1d42", + .access = variant->allowed_s1d42, + }, + { + .path = TMP_DIR "/s1d1/s1d2/s1d31/s1d41/f1", + .access = variant->allowed_f1, + }, + { + .path = TMP_DIR "/s1d1/s1d2/s1d31/s1d41/f2", + .access = variant->allowed_f2, + }, + { + .path = TMP_DIR "/s1d1/s1d2/s1d32/s1d42/f3", + .access = variant->allowed_f3, + }, + { + .path = TMP_DIR "/s2d1", + .access = variant->allowed_s2d1, + }, + /* s2d2_fd */ + { + .path = TMP_DIR "/s3d1", + .access = variant->allowed_s3d1, + }, + { + .path = TMP_DIR "/s4d1", + .access = variant->allowed_s4d1, + }, + {}, + }; + int ruleset_fd, s1d41_bind_fd, s1d42_bind_fd; + + ruleset_fd = create_ruleset(_metadata, handled_access, rules); + ASSERT_LE(0, ruleset_fd); + + /* Adds rule for the covered directory. */ + if (variant->allowed_s2d2) { + ASSERT_EQ(0, landlock_add_rule( + ruleset_fd, LANDLOCK_RULE_PATH_BENEATH, + &(struct landlock_path_beneath_attr){ + .parent_fd = self->s2d2_fd, + .allowed_access = + variant->allowed_s2d2, + }, + 0)); + } + EXPECT_EQ(0, close(self->s2d2_fd)); + + s1d41_bind_fd = open(TMP_DIR "/s2d1/s2d2/s1d31/s1d41", + O_DIRECTORY | O_PATH | O_CLOEXEC); + ASSERT_LE(0, s1d41_bind_fd); + s1d42_bind_fd = open(TMP_DIR "/s2d1/s2d2/s1d32/s1d42", + O_DIRECTORY | O_PATH | O_CLOEXEC); + ASSERT_LE(0, s1d42_bind_fd); + + /* Disconnects and checks source and destination directories. */ + EXPECT_EQ(0, test_open_rel(s1d41_bind_fd, "..", O_DIRECTORY)); + EXPECT_EQ(0, test_open_rel(s1d42_bind_fd, "..", O_DIRECTORY)); + /* Renames to make it accessible through s3d1/s1d41 */ + ASSERT_EQ(0, test_renameat(AT_FDCWD, TMP_DIR "/s1d1/s1d2/s1d31/s1d41", + AT_FDCWD, TMP_DIR "/s3d1/s1d41")); + /* Renames to make it accessible through s4d1/s1d42 */ + ASSERT_EQ(0, test_renameat(AT_FDCWD, TMP_DIR "/s1d1/s1d2/s1d32/s1d42", + AT_FDCWD, TMP_DIR "/s4d1/s1d42")); + EXPECT_EQ(ENOENT, test_open_rel(s1d41_bind_fd, "..", O_DIRECTORY)); + EXPECT_EQ(ENOENT, test_open_rel(s1d42_bind_fd, "..", O_DIRECTORY)); + + enforce_ruleset(_metadata, ruleset_fd); + EXPECT_EQ(0, close(ruleset_fd)); + + EXPECT_EQ(variant->expected_read_result, + test_open_rel(s1d41_bind_fd, "f1", O_RDONLY)); + + EXPECT_EQ(variant->expected_rename_result, + test_renameat(s1d41_bind_fd, "f1", s1d42_bind_fd, "f1")); + EXPECT_EQ(variant->expected_exchange_result, + test_exchangeat(s1d41_bind_fd, "f2", s1d42_bind_fd, "f3")); + + EXPECT_EQ(variant->expected_same_dir_rename_result, + test_renameat(s1d42_bind_fd, "f4", s1d42_bind_fd, "f5")); +} + +/* + * layout5_disconnected_branch before rename: + * + * tmp + * ├── s1d1 + * │   └── s1d2 [source of the first bind mount] + * │   └── s1d3 + * │   ├── s1d41 + * │   │   ├── f1 + * │   │   └── f2 + * │   └── s1d42 + * │   ├── f3 + * │   └── f4 + * ├── s2d1 + * │   └── s2d2 [source of the second bind mount] + * │   └── s2d3 + * │   └── s2d4 [first s1d2 bind mount] + * │   └── s1d3 + * │   ├── s1d41 + * │   │   ├── f1 + * │   │   └── f2 + * │   └── s1d42 + * │   ├── f3 + * │   └── f4 + * ├── s3d1 + * │   └── s3d2 [second s2d2 bind mount] + * │   └── s2d3 + * │   └── s2d4 [first s1d2 bind mount] + * │   └── s1d3 + * │   ├── s1d41 + * │   │   ├── f1 + * │   │   └── f2 + * │   └── s1d42 + * │   ├── f3 + * │   └── f4 + * └── s4d1 + * + * After rename: + * + * tmp + * ├── s1d1 + * │   └── s1d2 [source of the first bind mount] + * │   └── s1d3 + * │   ├── s1d41 + * │   │   ├── f1 + * │   │   └── f2 + * │   └── s1d42 + * │   ├── f3 + * │   └── f4 + * ├── s2d1 + * │   └── s2d2 [source of the second bind mount] + * ├── s3d1 + * │   └── s3d2 [second s2d2 bind mount] + * └── s4d1 + * └── s2d3 [renamed here] + * └── s2d4 [first s1d2 bind mount] + * └── s1d3 + * ├── s1d41 + * │   ├── f1 + * │   └── f2 + * └── s1d42 + * ├── f3 + * └── f4 + * + * Decision path: s1d3 -> s1d2 -> s2d2 -> s3d1 -> tmp + * s2d3 is ignored, as well as the directories under the mount points. + */ + +/* clang-format off */ +FIXTURE(layout5_disconnected_branch) { + int s2d4_fd, s3d2_fd; +}; +/* clang-format on */ + +FIXTURE_SETUP(layout5_disconnected_branch) +{ + prepare_layout(_metadata); + + create_file(_metadata, TMP_DIR "/s1d1/s1d2/s1d3/s1d41/f1"); + create_file(_metadata, TMP_DIR "/s1d1/s1d2/s1d3/s1d41/f2"); + create_file(_metadata, TMP_DIR "/s1d1/s1d2/s1d3/s1d42/f3"); + create_file(_metadata, TMP_DIR "/s1d1/s1d2/s1d3/s1d42/f4"); + create_directory(_metadata, TMP_DIR "/s2d1/s2d2/s2d3/s2d4"); + create_directory(_metadata, TMP_DIR "/s3d1/s3d2"); + create_directory(_metadata, TMP_DIR "/s4d1"); + + self->s2d4_fd = open(TMP_DIR "/s2d1/s2d2/s2d3/s2d4", + O_DIRECTORY | O_PATH | O_CLOEXEC); + ASSERT_LE(0, self->s2d4_fd); + + self->s3d2_fd = + open(TMP_DIR "/s3d1/s3d2", O_DIRECTORY | O_PATH | O_CLOEXEC); + ASSERT_LE(0, self->s3d2_fd); + + set_cap(_metadata, CAP_SYS_ADMIN); + ASSERT_EQ(0, mount(TMP_DIR "/s1d1/s1d2", TMP_DIR "/s2d1/s2d2/s2d3/s2d4", + NULL, MS_BIND, NULL)); + ASSERT_EQ(0, mount(TMP_DIR "/s2d1/s2d2", TMP_DIR "/s3d1/s3d2", NULL, + MS_BIND | MS_REC, NULL)); + clear_cap(_metadata, CAP_SYS_ADMIN); +} + +FIXTURE_TEARDOWN_PARENT(layout5_disconnected_branch) +{ + /* Bind mounts are handled by namespace lifetime. */ + + /* Removes files after renames. */ + remove_path(TMP_DIR "/s1d1/s1d2/s1d3/s1d41/f1"); + remove_path(TMP_DIR "/s1d1/s1d2/s1d3/s1d41/f2"); + remove_path(TMP_DIR "/s1d1/s1d2/s1d3/s1d42/f1"); + remove_path(TMP_DIR "/s1d1/s1d2/s1d3/s1d42/f3"); + remove_path(TMP_DIR "/s1d1/s1d2/s1d3/s1d42/f4"); + remove_path(TMP_DIR "/s1d1/s1d2/s1d3/s1d42/f5"); + + cleanup_layout(_metadata); +} + +FIXTURE_VARIANT(layout5_disconnected_branch) +{ + /* + * Parent of all files. It should always be enforced when testing against + * files under the s1d41 or s1d42 disconnected directories. + */ + const __u64 allowed_base; + /* + * Parent of the first bind mount source. It should always be ignored when + * testing against files under the s1d41 or s1d42 disconnected directories. + */ + const __u64 allowed_s1d1; + const __u64 allowed_s1d2; + const __u64 allowed_s1d3; + const __u64 allowed_s2d1; + const __u64 allowed_s2d2; + const __u64 allowed_s2d3; + const __u64 allowed_s2d4; + const __u64 allowed_s3d1; + const __u64 allowed_s3d2; + const __u64 allowed_s4d1; + + /* Expected result of the call to open([fd:s1d3]/s1d41/f1, O_RDONLY). */ + const int expected_read_result; + /* + * Expected result of the call to renameat([fd:s1d3]/s1d41/f1, + * [fd:s1d3]/s1d42/f1). + */ + const int expected_rename_result; + /* + * Expected result of the call to renameat([fd:s1d3]/s1d41/f2, + * [fd:s1d3]/s1d42/f3, RENAME_EXCHANGE). + */ + const int expected_exchange_result; + /* + * Expected result of the call to renameat([fd:s1d3]/s1d42/f4, + * [fd:s1d3]/s1d42/f5). + */ + const int expected_same_dir_rename_result; +}; + +/* clang-format off */ +FIXTURE_VARIANT_ADD(layout5_disconnected_branch, s1d1_mount1_src_parent) { + /* clang-format on */ + .allowed_s1d1 = LANDLOCK_ACCESS_FS_REFER | + LANDLOCK_ACCESS_FS_READ_FILE | + LANDLOCK_ACCESS_FS_EXECUTE | + LANDLOCK_ACCESS_FS_MAKE_REG, + .expected_read_result = EACCES, + .expected_same_dir_rename_result = EACCES, + .expected_rename_result = EACCES, + .expected_exchange_result = EACCES, +}; + +/* clang-format off */ +FIXTURE_VARIANT_ADD(layout5_disconnected_branch, s1d2_mount1_src_refer) { + /* clang-format on */ + .allowed_s1d2 = LANDLOCK_ACCESS_FS_REFER | LANDLOCK_ACCESS_FS_READ_FILE, + .expected_read_result = 0, + .expected_same_dir_rename_result = EACCES, + .expected_rename_result = EACCES, + .expected_exchange_result = EACCES, +}; + +/* clang-format off */ +FIXTURE_VARIANT_ADD(layout5_disconnected_branch, s1d2_mount1_src_create) { + /* clang-format on */ + .allowed_s1d2 = LANDLOCK_ACCESS_FS_READ_FILE | + LANDLOCK_ACCESS_FS_MAKE_REG, + .expected_read_result = 0, + .expected_same_dir_rename_result = 0, + .expected_rename_result = EXDEV, + .expected_exchange_result = EXDEV, +}; + +/* clang-format off */ +FIXTURE_VARIANT_ADD(layout5_disconnected_branch, s1d2_mount1_src_rename) { + /* clang-format on */ + .allowed_s1d2 = LANDLOCK_ACCESS_FS_REFER | LANDLOCK_ACCESS_FS_MAKE_REG, + .expected_read_result = EACCES, + .expected_same_dir_rename_result = 0, + .expected_rename_result = 0, + .expected_exchange_result = 0, +}; + +/* clang-format off */ +FIXTURE_VARIANT_ADD(layout5_disconnected_branch, s1d3_fd_refer) { + /* clang-format on */ + .allowed_s1d3 = LANDLOCK_ACCESS_FS_REFER | LANDLOCK_ACCESS_FS_READ_FILE, + .expected_read_result = 0, + .expected_same_dir_rename_result = EACCES, + .expected_rename_result = EACCES, + .expected_exchange_result = EACCES, +}; + +/* clang-format off */ +FIXTURE_VARIANT_ADD(layout5_disconnected_branch, s1d3_fd_create) { + /* clang-format on */ + .allowed_s1d3 = LANDLOCK_ACCESS_FS_READ_FILE | + LANDLOCK_ACCESS_FS_MAKE_REG, + .expected_read_result = 0, + .expected_same_dir_rename_result = 0, + .expected_rename_result = EXDEV, + .expected_exchange_result = EXDEV, +}; + +/* clang-format off */ +FIXTURE_VARIANT_ADD(layout5_disconnected_branch, s1d3_fd_rename) { + /* clang-format on */ + .allowed_s1d3 = LANDLOCK_ACCESS_FS_REFER | LANDLOCK_ACCESS_FS_MAKE_REG, + .expected_read_result = EACCES, + .expected_same_dir_rename_result = 0, + .expected_rename_result = 0, + .expected_exchange_result = 0, +}; + +/* clang-format off */ +FIXTURE_VARIANT_ADD(layout5_disconnected_branch, s1d3_fd_full) { + /* clang-format on */ + .allowed_s1d3 = LANDLOCK_ACCESS_FS_REFER | + LANDLOCK_ACCESS_FS_READ_FILE | + LANDLOCK_ACCESS_FS_EXECUTE | + LANDLOCK_ACCESS_FS_MAKE_REG, + .expected_read_result = 0, + .expected_same_dir_rename_result = 0, + .expected_rename_result = 0, + .expected_exchange_result = 0, +}; + +/* clang-format off */ +FIXTURE_VARIANT_ADD(layout5_disconnected_branch, s2d1_mount2_src_parent) { + /* clang-format on */ + .allowed_s2d1 = LANDLOCK_ACCESS_FS_REFER | + LANDLOCK_ACCESS_FS_READ_FILE | + LANDLOCK_ACCESS_FS_EXECUTE | + LANDLOCK_ACCESS_FS_MAKE_REG, + .expected_read_result = EACCES, + .expected_same_dir_rename_result = EACCES, + .expected_rename_result = EACCES, + .expected_exchange_result = EACCES, +}; + +/* clang-format off */ +FIXTURE_VARIANT_ADD(layout5_disconnected_branch, s2d2_mount2_src_refer) { + /* clang-format on */ + .allowed_s2d2 = LANDLOCK_ACCESS_FS_REFER | LANDLOCK_ACCESS_FS_READ_FILE, + .expected_read_result = 0, + .expected_same_dir_rename_result = EACCES, + .expected_rename_result = EACCES, + .expected_exchange_result = EACCES, +}; + +/* clang-format off */ +FIXTURE_VARIANT_ADD(layout5_disconnected_branch, s2d2_mount2_src_create) { + /* clang-format on */ + .allowed_s2d2 = LANDLOCK_ACCESS_FS_READ_FILE | + LANDLOCK_ACCESS_FS_MAKE_REG, + .expected_read_result = 0, + .expected_same_dir_rename_result = 0, + .expected_rename_result = EXDEV, + .expected_exchange_result = EXDEV, +}; + +/* clang-format off */ +FIXTURE_VARIANT_ADD(layout5_disconnected_branch, s2d2_mount2_src_rename) { + /* clang-format on */ + .allowed_s2d2 = LANDLOCK_ACCESS_FS_REFER | LANDLOCK_ACCESS_FS_MAKE_REG, + .expected_read_result = EACCES, + .expected_same_dir_rename_result = 0, + .expected_rename_result = 0, + .expected_exchange_result = 0, +}; + +/* clang-format off */ +FIXTURE_VARIANT_ADD(layout5_disconnected_branch, s2d3_mount1_dst_parent) { + /* clang-format on */ + .allowed_s2d3 = LANDLOCK_ACCESS_FS_REFER | + LANDLOCK_ACCESS_FS_READ_FILE | + LANDLOCK_ACCESS_FS_EXECUTE | + LANDLOCK_ACCESS_FS_MAKE_REG, + .expected_read_result = 0, + .expected_same_dir_rename_result = 0, + .expected_rename_result = 0, + .expected_exchange_result = 0, +}; + +/* clang-format off */ +FIXTURE_VARIANT_ADD(layout5_disconnected_branch, s2d4_mount1_dst) { + /* clang-format on */ + .allowed_s2d4 = LANDLOCK_ACCESS_FS_REFER | + LANDLOCK_ACCESS_FS_READ_FILE | + LANDLOCK_ACCESS_FS_EXECUTE | + LANDLOCK_ACCESS_FS_MAKE_REG, + .expected_read_result = EACCES, + .expected_same_dir_rename_result = EACCES, + .expected_rename_result = EACCES, + .expected_exchange_result = EACCES, +}; + +/* clang-format off */ +FIXTURE_VARIANT_ADD(layout5_disconnected_branch, s3d1_mount2_dst_parent_refer) { + /* clang-format on */ + .allowed_s3d1 = LANDLOCK_ACCESS_FS_REFER | LANDLOCK_ACCESS_FS_READ_FILE, + .expected_read_result = 0, + .expected_same_dir_rename_result = EACCES, + .expected_rename_result = EACCES, + .expected_exchange_result = EACCES, +}; + +/* clang-format off */ +FIXTURE_VARIANT_ADD(layout5_disconnected_branch, s3d1_mount2_dst_parent_create) { + /* clang-format on */ + .allowed_s3d1 = LANDLOCK_ACCESS_FS_READ_FILE | + LANDLOCK_ACCESS_FS_MAKE_REG, + .expected_read_result = 0, + .expected_same_dir_rename_result = 0, + .expected_rename_result = EXDEV, + .expected_exchange_result = EXDEV, +}; + +/* clang-format off */ +FIXTURE_VARIANT_ADD(layout5_disconnected_branch, s3d1_mount2_dst_parent_rename) { + /* clang-format on */ + .allowed_s3d1 = LANDLOCK_ACCESS_FS_REFER | LANDLOCK_ACCESS_FS_MAKE_REG, + .expected_read_result = EACCES, + .expected_same_dir_rename_result = 0, + .expected_rename_result = 0, + .expected_exchange_result = 0, +}; + +/* clang-format off */ +FIXTURE_VARIANT_ADD(layout5_disconnected_branch, s3d2_mount1_dst) { + /* clang-format on */ + .allowed_s3d2 = LANDLOCK_ACCESS_FS_REFER | + LANDLOCK_ACCESS_FS_READ_FILE | + LANDLOCK_ACCESS_FS_EXECUTE | + LANDLOCK_ACCESS_FS_MAKE_REG, + .expected_read_result = EACCES, + .expected_same_dir_rename_result = EACCES, + .expected_rename_result = EACCES, + .expected_exchange_result = EACCES, +}; + +/* clang-format off */ +FIXTURE_VARIANT_ADD(layout5_disconnected_branch, s4d1_rename_parent) { + /* clang-format on */ + .allowed_s4d1 = LANDLOCK_ACCESS_FS_REFER | + LANDLOCK_ACCESS_FS_READ_FILE | + LANDLOCK_ACCESS_FS_EXECUTE | + LANDLOCK_ACCESS_FS_MAKE_REG, + .expected_read_result = 0, + .expected_same_dir_rename_result = 0, + .expected_rename_result = 0, + .expected_exchange_result = 0, +}; + +TEST_F_FORK(layout5_disconnected_branch, read_rename_exchange) +{ + const __u64 handled_access = + LANDLOCK_ACCESS_FS_REFER | LANDLOCK_ACCESS_FS_READ_FILE | + LANDLOCK_ACCESS_FS_EXECUTE | LANDLOCK_ACCESS_FS_MAKE_REG; + const struct rule rules[] = { + { + .path = TMP_DIR "/s1d1", + .access = variant->allowed_s1d1, + }, + { + .path = TMP_DIR "/s1d1/s1d2", + .access = variant->allowed_s1d2, + }, + { + .path = TMP_DIR "/s1d1/s1d2/s1d3", + .access = variant->allowed_s1d3, + }, + { + .path = TMP_DIR "/s2d1", + .access = variant->allowed_s2d1, + }, + { + .path = TMP_DIR "/s2d1/s2d2", + .access = variant->allowed_s2d2, + }, + { + .path = TMP_DIR "/s2d1/s2d2/s2d3", + .access = variant->allowed_s2d3, + }, + /* s2d4_fd */ + { + .path = TMP_DIR "/s3d1", + .access = variant->allowed_s3d1, + }, + /* s3d2_fd */ + { + .path = TMP_DIR "/s4d1", + .access = variant->allowed_s4d1, + }, + {}, + }; + int ruleset_fd, s1d3_bind_fd; + + ruleset_fd = create_ruleset(_metadata, handled_access, rules); + ASSERT_LE(0, ruleset_fd); + + /* Adds rules for the covered directories. */ + if (variant->allowed_s2d4) { + ASSERT_EQ(0, landlock_add_rule( + ruleset_fd, LANDLOCK_RULE_PATH_BENEATH, + &(struct landlock_path_beneath_attr){ + .parent_fd = self->s2d4_fd, + .allowed_access = + variant->allowed_s2d4, + }, + 0)); + } + EXPECT_EQ(0, close(self->s2d4_fd)); + + if (variant->allowed_s3d2) { + ASSERT_EQ(0, landlock_add_rule( + ruleset_fd, LANDLOCK_RULE_PATH_BENEATH, + &(struct landlock_path_beneath_attr){ + .parent_fd = self->s3d2_fd, + .allowed_access = + variant->allowed_s3d2, + }, + 0)); + } + EXPECT_EQ(0, close(self->s3d2_fd)); + + s1d3_bind_fd = open(TMP_DIR "/s3d1/s3d2/s2d3/s2d4/s1d3", + O_DIRECTORY | O_PATH | O_CLOEXEC); + ASSERT_LE(0, s1d3_bind_fd); + + /* Disconnects and checks source and destination directories. */ + EXPECT_EQ(0, test_open_rel(s1d3_bind_fd, "../../..", O_DIRECTORY)); + /* Renames to make it accessible through s3d1/s1d41 */ + ASSERT_EQ(0, test_renameat(AT_FDCWD, TMP_DIR "/s2d1/s2d2/s2d3", + AT_FDCWD, TMP_DIR "/s4d1/s2d3")); + EXPECT_EQ(ENOENT, test_open_rel(s1d3_bind_fd, "../../..", O_DIRECTORY)); + + enforce_ruleset(_metadata, ruleset_fd); + EXPECT_EQ(0, close(ruleset_fd)); + + EXPECT_EQ(variant->expected_read_result, + test_open_rel(s1d3_bind_fd, "s1d41/f1", O_RDONLY)); + + EXPECT_EQ(variant->expected_rename_result, + test_renameat(s1d3_bind_fd, "s1d41/f1", s1d3_bind_fd, + "s1d42/f1")); + EXPECT_EQ(variant->expected_exchange_result, + test_exchangeat(s1d3_bind_fd, "s1d41/f2", s1d3_bind_fd, + "s1d42/f3")); + + EXPECT_EQ(variant->expected_same_dir_rename_result, + test_renameat(s1d3_bind_fd, "s1d42/f4", s1d3_bind_fd, + "s1d42/f5")); +} + #define LOWER_BASE TMP_DIR "/lower" #define LOWER_DATA LOWER_BASE "/data" static const char lower_fl1[] = LOWER_DATA "/fl1"; -- 2.51.0