linux-kernel - Re: [PATCH v4 12/30] selftests/landlock: Add test to check partial access in a mount tree

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250110.Ohthee1ohbah@digikod.net>
Date: Fri, 10 Jan 2025 12:24:30 +0100
From: Mickaël Salaün <mic@...ikod.net>
To: Eric Paris <eparis@...hat.com>, Paul Moore <paul@...l-moore.com>, 
	Günther Noack <gnoack@...gle.com>, "Serge E . Hallyn" <serge@...lyn.com>
Cc: Ben Scarlato <akhna@...gle.com>, 
	Casey Schaufler <casey@...aufler-ca.com>, Charles Zaffery <czaffery@...lox.com>, 
	Daniel Burgener <dburgener@...ux.microsoft.com>, Francis Laniel <flaniel@...ux.microsoft.com>, 
	James Morris <jmorris@...ei.org>, Jann Horn <jannh@...gle.com>, Jeff Xu <jeffxu@...gle.com>, 
	Jorge Lucangeli Obes <jorgelo@...gle.com>, Kees Cook <kees@...nel.org>, 
	Konstantin Meskhidze <konstantin.meskhidze@...wei.com>, Matt Bobrowski <mattbobrowski@...gle.com>, 
	Mikhail Ivanov <ivanov.mikhail1@...wei-partners.com>, Phil Sutter <phil@....cc>, 
	Praveen K Paladugu <prapal@...ux.microsoft.com>, Robert Salvet <robert.salvet@...lox.com>, 
	Shervin Oloumi <enlightened@...gle.com>, Song Liu <song@...nel.org>, 
	Tahera Fahimi <fahimitahera@...il.com>, Tyler Hicks <code@...icks.com>, audit@...r.kernel.org, 
	linux-kernel@...r.kernel.org, linux-security-module@...r.kernel.org
Subject: Re: [PATCH v4 12/30] selftests/landlock: Add test to check partial
 access in a mount tree

On Wed, Jan 08, 2025 at 04:43:20PM +0100, Mickaël Salaün wrote:
> Add layout1.refer_part_mount_tree_is_allowed to test the masked logical
> issue regarding collect_domain_accesses() calls followed by the
> is_access_to_paths_allowed() check in current_check_refer_path().  See
> previous commit.
> 
> This test should work without the previous fix as well, but it enables
> us to make sure future changes will not have impact regarding this
> behavior.
> 
> Cc: Günther Noack <gnoack@...gle.com>
> Signed-off-by: Mickaël Salaün <mic@...ikod.net>
> Link: https://lore.kernel.org/r/20250108154338.1129069-13-mic@digikod.net

Pushed in my next tree to simplify next patch series.

> ---
> 
> Changes since v2:
> - New patch.
> ---
>  tools/testing/selftests/landlock/fs_test.c | 54 ++++++++++++++++++++--
>  1 file changed, 50 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
> index 6788762188fe..42ce1e79ba82 100644
> --- a/tools/testing/selftests/landlock/fs_test.c
> +++ b/tools/testing/selftests/landlock/fs_test.c
> @@ -85,6 +85,9 @@ static const char file1_s3d1[] = TMP_DIR "/s3d1/f1";
>  /* dir_s3d2 is a mount point. */
>  static const char dir_s3d2[] = TMP_DIR "/s3d1/s3d2";
>  static const char dir_s3d3[] = TMP_DIR "/s3d1/s3d2/s3d3";
> +static const char file1_s3d3[] = TMP_DIR "/s3d1/s3d2/s3d3/f1";
> +static const char dir_s3d4[] = TMP_DIR "/s3d1/s3d2/s3d4";
> +static const char file1_s3d4[] = TMP_DIR "/s3d1/s3d2/s3d4/f1";
>  
>  /*
>   * layout1 hierarchy:
> @@ -108,8 +111,11 @@ static const char dir_s3d3[] = TMP_DIR "/s3d1/s3d2/s3d3";
>   * │           └── f2
>   * └── s3d1
>   *     ├── f1
> - *     └── s3d2
> - *         └── s3d3
> + *     └── s3d2 [mount point]
> + *         ├── s3d3
> + *         │   └── f1
> + *         └── s3d4
> + *             └── f1
>   */
>  
>  static bool fgrep(FILE *const inf, const char *const str)
> @@ -358,7 +364,8 @@ static void create_layout1(struct __test_metadata *const _metadata)
>  	ASSERT_EQ(0, mount_opt(&mnt_tmp, dir_s3d2));
>  	clear_cap(_metadata, CAP_SYS_ADMIN);
>  
> -	ASSERT_EQ(0, mkdir(dir_s3d3, 0700));
> +	create_file(_metadata, file1_s3d3);
> +	create_file(_metadata, file1_s3d4);
>  }
>  
>  static void remove_layout1(struct __test_metadata *const _metadata)
> @@ -378,7 +385,8 @@ static void remove_layout1(struct __test_metadata *const _metadata)
>  	EXPECT_EQ(0, remove_path(dir_s2d2));
>  
>  	EXPECT_EQ(0, remove_path(file1_s3d1));
> -	EXPECT_EQ(0, remove_path(dir_s3d3));
> +	EXPECT_EQ(0, remove_path(file1_s3d3));
> +	EXPECT_EQ(0, remove_path(file1_s3d4));
>  	set_cap(_metadata, CAP_SYS_ADMIN);
>  	umount(dir_s3d2);
>  	clear_cap(_metadata, CAP_SYS_ADMIN);
> @@ -2444,6 +2452,44 @@ TEST_F_FORK(layout1, refer_mount_root_deny)
>  	EXPECT_EQ(0, close(root_fd));
>  }
>  
> +TEST_F_FORK(layout1, refer_part_mount_tree_is_allowed)
> +{
> +	const struct rule layer1[] = {
> +		{
> +			/* Parent mount point. */
> +			.path = dir_s3d1,
> +			.access = LANDLOCK_ACCESS_FS_REFER |
> +				  LANDLOCK_ACCESS_FS_MAKE_REG,
> +		},
> +		{
> +			/*
> +			 * Removing the source file is allowed because its
> +			 * access rights are already a superset of the
> +			 * destination.
> +			 */
> +			.path = dir_s3d4,
> +			.access = LANDLOCK_ACCESS_FS_REFER |
> +				  LANDLOCK_ACCESS_FS_MAKE_REG |
> +				  LANDLOCK_ACCESS_FS_REMOVE_FILE,
> +		},
> +		{},
> +	};
> +	int ruleset_fd;
> +
> +	ASSERT_EQ(0, unlink(file1_s3d3));
> +	ruleset_fd = create_ruleset(_metadata,
> +				    LANDLOCK_ACCESS_FS_REFER |
> +					    LANDLOCK_ACCESS_FS_MAKE_REG |
> +					    LANDLOCK_ACCESS_FS_REMOVE_FILE,
> +				    layer1);
> +
> +	ASSERT_LE(0, ruleset_fd);
> +	enforce_ruleset(_metadata, ruleset_fd);
> +	ASSERT_EQ(0, close(ruleset_fd));
> +
> +	ASSERT_EQ(0, rename(file1_s3d4, file1_s3d3));
> +}
> +
>  TEST_F_FORK(layout1, reparent_link)
>  {
>  	const struct rule layer1[] = {
> -- 
> 2.47.1
> 
>