Unverified Commit a18ee3f3 authored by Tingmao Wang's avatar Tingmao Wang Committed by Mickaël Salaün
Browse files

selftests/landlock: Add tests for access through disconnected paths

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 <gnoack@google.com>
Cc: Song Liu <song@kernel.org>
Link: https://lore.kernel.org/r/027d5190-b37a-40a8-84e9-4ccbc352bcdf@maowtm.org

 [1]
Signed-off-by: default avatarTingmao Wang <m@maowtm.org>
Link: https://lore.kernel.org/r/20251128172200.760753-4-mic@digikod.net


Signed-off-by: default avatarMickaël Salaün <mic@digikod.net>
parent f7ef7de6
Loading
Loading
Loading
Loading
+415 −8
Original line number Diff line number Diff line
@@ -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";