diff mbox series

[v2,1/2] selftests/landlock: Add tests to check unknown rule's access rights

Message ID 20231130093616.67340-2-mic@digikod.net (mailing list archive)
State Handled Elsewhere
Headers show
Series Extend Landlock test to improve rule's coverage | expand

Commit Message

Mickaël Salaün Nov. 30, 2023, 9:36 a.m. UTC 
Add two tests to make sure that we cannot add a rule with access
rights that are unknown:
* fs: layout0.rule_with_unknown_access
* net: mini.rule_with_unknown_access

Rename unknown_access_rights tests to ruleset_with_unknown_access .

Cc: Günther Noack <gnoack@google.com>
Cc: Konstantin Meskhidze <konstantin.meskhidze@huawei.com>
Signed-off-by: Mickaël Salaün <mic@digikod.net>
---

Changes since v1:
* Move checks into their own test/loop as suggested by Günther Noack.
* Don't change layout1.file_and_dir_access_rights
---
 tools/testing/selftests/landlock/fs_test.c  | 29 ++++++++++++++++++++-
 tools/testing/selftests/landlock/net_test.c | 27 ++++++++++++++++++-
 2 files changed, 54 insertions(+), 2 deletions(-)

Comments

Mickaël Salaün Dec. 11, 2023, 7:17 a.m. UTC | #1 
Günther, are you OK with this update?

On Thu, Nov 30, 2023 at 10:36:15AM +0100, Mickaël Salaün wrote:
> Add two tests to make sure that we cannot add a rule with access
> rights that are unknown:
> * fs: layout0.rule_with_unknown_access
> * net: mini.rule_with_unknown_access
> 
> Rename unknown_access_rights tests to ruleset_with_unknown_access .
> 
> Cc: Günther Noack <gnoack@google.com>
> Cc: Konstantin Meskhidze <konstantin.meskhidze@huawei.com>
> Signed-off-by: Mickaël Salaün <mic@digikod.net>
> ---
> 
> Changes since v1:
> * Move checks into their own test/loop as suggested by Günther Noack.
> * Don't change layout1.file_and_dir_access_rights
> ---
>  tools/testing/selftests/landlock/fs_test.c  | 29 ++++++++++++++++++++-
>  tools/testing/selftests/landlock/net_test.c | 27 ++++++++++++++++++-
>  2 files changed, 54 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
> index 18e1f86a6234..1e6c474e3d08 100644
> --- a/tools/testing/selftests/landlock/fs_test.c
> +++ b/tools/testing/selftests/landlock/fs_test.c
> @@ -589,7 +589,7 @@ TEST_F_FORK(layout1, file_and_dir_access_rights)
>  	ASSERT_EQ(0, close(ruleset_fd));
>  }
>  
> -TEST_F_FORK(layout0, unknown_access_rights)
> +TEST_F_FORK(layout0, ruleset_with_unknown_access)
>  {
>  	__u64 access_mask;
>  
> @@ -605,6 +605,33 @@ TEST_F_FORK(layout0, unknown_access_rights)
>  	}
>  }
>  
> +TEST_F_FORK(layout0, rule_with_unknown_access)
> +{
> +	__u64 access;
> +	struct landlock_path_beneath_attr path_beneath = {};
> +	const struct landlock_ruleset_attr ruleset_attr = {
> +		.handled_access_fs = ACCESS_ALL,
> +	};
> +	const int ruleset_fd =
> +		landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
> +
> +	ASSERT_LE(0, ruleset_fd);
> +
> +	path_beneath.parent_fd =
> +		open(TMP_DIR, O_PATH | O_DIRECTORY | O_CLOEXEC);
> +	ASSERT_LE(0, path_beneath.parent_fd);
> +
> +	for (access = 1ULL << 63; access != ACCESS_LAST; access >>= 1) {
> +		path_beneath.allowed_access = access;
> +		EXPECT_EQ(-1, landlock_add_rule(ruleset_fd,
> +						LANDLOCK_RULE_PATH_BENEATH,
> +						&path_beneath, 0));
> +		EXPECT_EQ(EINVAL, errno);
> +	}
> +	ASSERT_EQ(0, close(path_beneath.parent_fd));
> +	ASSERT_EQ(0, close(ruleset_fd));
> +}
> +
>  static void add_path_beneath(struct __test_metadata *const _metadata,
>  			     const int ruleset_fd, const __u64 allowed_access,
>  			     const char *const path)
> diff --git a/tools/testing/selftests/landlock/net_test.c b/tools/testing/selftests/landlock/net_test.c
> index 929e21c4db05..83d9abc3ee55 100644
> --- a/tools/testing/selftests/landlock/net_test.c
> +++ b/tools/testing/selftests/landlock/net_test.c
> @@ -1260,7 +1260,7 @@ TEST_F(mini, network_access_rights)
>  }
>  
>  /* Checks invalid attribute, out of landlock network access range. */
> -TEST_F(mini, unknown_access_rights)
> +TEST_F(mini, ruleset_with_unknown_access)
>  {
>  	__u64 access_mask;
>  
> @@ -1276,6 +1276,31 @@ TEST_F(mini, unknown_access_rights)
>  	}
>  }
>  
> +TEST_F(mini, rule_with_unknown_access)
> +{
> +	const struct landlock_ruleset_attr ruleset_attr = {
> +		.handled_access_net = ACCESS_ALL,
> +	};
> +	struct landlock_net_port_attr net_port = {
> +		.port = sock_port_start,
> +	};
> +	int ruleset_fd;
> +	__u64 access;
> +
> +	ruleset_fd =
> +		landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
> +	ASSERT_LE(0, ruleset_fd);
> +
> +	for (access = 1ULL << 63; access != ACCESS_LAST; access >>= 1) {
> +		net_port.allowed_access = access;
> +		EXPECT_EQ(-1,
> +			  landlock_add_rule(ruleset_fd, LANDLOCK_RULE_NET_PORT,
> +					    &net_port, 0));
> +		EXPECT_EQ(EINVAL, errno);
> +	}
> +	EXPECT_EQ(0, close(ruleset_fd));
> +}
> +
>  TEST_F(mini, inval)
>  {
>  	const struct landlock_ruleset_attr ruleset_attr = {
> -- 
> 2.42.1
> 
>
Günther Noack Dec. 11, 2023, 8:54 a.m. UTC | #2 
On Thu, Nov 30, 2023 at 10:36:15AM +0100, Mickaël Salaün wrote:
> Add two tests to make sure that we cannot add a rule with access
> rights that are unknown:
> * fs: layout0.rule_with_unknown_access
> * net: mini.rule_with_unknown_access
> 
> Rename unknown_access_rights tests to ruleset_with_unknown_access .
> 
> Cc: Günther Noack <gnoack@google.com>
> Cc: Konstantin Meskhidze <konstantin.meskhidze@huawei.com>
> Signed-off-by: Mickaël Salaün <mic@digikod.net>
> ---
> 
> Changes since v1:
> * Move checks into their own test/loop as suggested by Günther Noack.
> * Don't change layout1.file_and_dir_access_rights
> ---
>  tools/testing/selftests/landlock/fs_test.c  | 29 ++++++++++++++++++++-
>  tools/testing/selftests/landlock/net_test.c | 27 ++++++++++++++++++-
>  2 files changed, 54 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
> index 18e1f86a6234..1e6c474e3d08 100644
> --- a/tools/testing/selftests/landlock/fs_test.c
> +++ b/tools/testing/selftests/landlock/fs_test.c
> @@ -589,7 +589,7 @@ TEST_F_FORK(layout1, file_and_dir_access_rights)
>  	ASSERT_EQ(0, close(ruleset_fd));
>  }
>  
> -TEST_F_FORK(layout0, unknown_access_rights)
> +TEST_F_FORK(layout0, ruleset_with_unknown_access)
>  {
>  	__u64 access_mask;
>  
> @@ -605,6 +605,33 @@ TEST_F_FORK(layout0, unknown_access_rights)
>  	}
>  }
>  
> +TEST_F_FORK(layout0, rule_with_unknown_access)
> +{
> +	__u64 access;
> +	struct landlock_path_beneath_attr path_beneath = {};
> +	const struct landlock_ruleset_attr ruleset_attr = {
> +		.handled_access_fs = ACCESS_ALL,
> +	};
> +	const int ruleset_fd =
> +		landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
> +
> +	ASSERT_LE(0, ruleset_fd);
> +
> +	path_beneath.parent_fd =
> +		open(TMP_DIR, O_PATH | O_DIRECTORY | O_CLOEXEC);
> +	ASSERT_LE(0, path_beneath.parent_fd);
> +
> +	for (access = 1ULL << 63; access != ACCESS_LAST; access >>= 1) {
> +		path_beneath.allowed_access = access;
> +		EXPECT_EQ(-1, landlock_add_rule(ruleset_fd,
> +						LANDLOCK_RULE_PATH_BENEATH,
> +						&path_beneath, 0));
> +		EXPECT_EQ(EINVAL, errno);
> +	}
> +	ASSERT_EQ(0, close(path_beneath.parent_fd));
> +	ASSERT_EQ(0, close(ruleset_fd));
> +}
> +
>  static void add_path_beneath(struct __test_metadata *const _metadata,
>  			     const int ruleset_fd, const __u64 allowed_access,
>  			     const char *const path)
> diff --git a/tools/testing/selftests/landlock/net_test.c b/tools/testing/selftests/landlock/net_test.c
> index 929e21c4db05..83d9abc3ee55 100644
> --- a/tools/testing/selftests/landlock/net_test.c
> +++ b/tools/testing/selftests/landlock/net_test.c
> @@ -1260,7 +1260,7 @@ TEST_F(mini, network_access_rights)
>  }
>  
>  /* Checks invalid attribute, out of landlock network access range. */
> -TEST_F(mini, unknown_access_rights)
> +TEST_F(mini, ruleset_with_unknown_access)
>  {
>  	__u64 access_mask;
>  
> @@ -1276,6 +1276,31 @@ TEST_F(mini, unknown_access_rights)
>  	}
>  }
>  
> +TEST_F(mini, rule_with_unknown_access)
> +{
> +	const struct landlock_ruleset_attr ruleset_attr = {
> +		.handled_access_net = ACCESS_ALL,
> +	};
> +	struct landlock_net_port_attr net_port = {
> +		.port = sock_port_start,
> +	};
> +	int ruleset_fd;
> +	__u64 access;
> +
> +	ruleset_fd =
> +		landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
> +	ASSERT_LE(0, ruleset_fd);
> +
> +	for (access = 1ULL << 63; access != ACCESS_LAST; access >>= 1) {
> +		net_port.allowed_access = access;
> +		EXPECT_EQ(-1,
> +			  landlock_add_rule(ruleset_fd, LANDLOCK_RULE_NET_PORT,
> +					    &net_port, 0));
> +		EXPECT_EQ(EINVAL, errno);
> +	}
> +	EXPECT_EQ(0, close(ruleset_fd));
> +}
> +
>  TEST_F(mini, inval)
>  {
>  	const struct landlock_ruleset_attr ruleset_attr = {
> -- 
> 2.42.1
> 

Reviewed-by: Günther Noack <gnoack@google.com>

Thank you, looks good to me!
Good idea to split it up into two separate tests.

—Günther
diff mbox series

Patch

diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
index 18e1f86a6234..1e6c474e3d08 100644
--- a/tools/testing/selftests/landlock/fs_test.c
+++ b/tools/testing/selftests/landlock/fs_test.c
@@ -589,7 +589,7 @@  TEST_F_FORK(layout1, file_and_dir_access_rights)
 	ASSERT_EQ(0, close(ruleset_fd));
 }
 
-TEST_F_FORK(layout0, unknown_access_rights)
+TEST_F_FORK(layout0, ruleset_with_unknown_access)
 {
 	__u64 access_mask;
 
@@ -605,6 +605,33 @@  TEST_F_FORK(layout0, unknown_access_rights)
 	}
 }
 
+TEST_F_FORK(layout0, rule_with_unknown_access)
+{
+	__u64 access;
+	struct landlock_path_beneath_attr path_beneath = {};
+	const struct landlock_ruleset_attr ruleset_attr = {
+		.handled_access_fs = ACCESS_ALL,
+	};
+	const int ruleset_fd =
+		landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
+
+	ASSERT_LE(0, ruleset_fd);
+
+	path_beneath.parent_fd =
+		open(TMP_DIR, O_PATH | O_DIRECTORY | O_CLOEXEC);
+	ASSERT_LE(0, path_beneath.parent_fd);
+
+	for (access = 1ULL << 63; access != ACCESS_LAST; access >>= 1) {
+		path_beneath.allowed_access = access;
+		EXPECT_EQ(-1, landlock_add_rule(ruleset_fd,
+						LANDLOCK_RULE_PATH_BENEATH,
+						&path_beneath, 0));
+		EXPECT_EQ(EINVAL, errno);
+	}
+	ASSERT_EQ(0, close(path_beneath.parent_fd));
+	ASSERT_EQ(0, close(ruleset_fd));
+}
+
 static void add_path_beneath(struct __test_metadata *const _metadata,
 			     const int ruleset_fd, const __u64 allowed_access,
 			     const char *const path)
diff --git a/tools/testing/selftests/landlock/net_test.c b/tools/testing/selftests/landlock/net_test.c
index 929e21c4db05..83d9abc3ee55 100644
--- a/tools/testing/selftests/landlock/net_test.c
+++ b/tools/testing/selftests/landlock/net_test.c
@@ -1260,7 +1260,7 @@  TEST_F(mini, network_access_rights)
 }
 
 /* Checks invalid attribute, out of landlock network access range. */
-TEST_F(mini, unknown_access_rights)
+TEST_F(mini, ruleset_with_unknown_access)
 {
 	__u64 access_mask;
 
@@ -1276,6 +1276,31 @@  TEST_F(mini, unknown_access_rights)
 	}
 }
 
+TEST_F(mini, rule_with_unknown_access)
+{
+	const struct landlock_ruleset_attr ruleset_attr = {
+		.handled_access_net = ACCESS_ALL,
+	};
+	struct landlock_net_port_attr net_port = {
+		.port = sock_port_start,
+	};
+	int ruleset_fd;
+	__u64 access;
+
+	ruleset_fd =
+		landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
+	ASSERT_LE(0, ruleset_fd);
+
+	for (access = 1ULL << 63; access != ACCESS_LAST; access >>= 1) {
+		net_port.allowed_access = access;
+		EXPECT_EQ(-1,
+			  landlock_add_rule(ruleset_fd, LANDLOCK_RULE_NET_PORT,
+					    &net_port, 0));
+		EXPECT_EQ(EINVAL, errno);
+	}
+	EXPECT_EQ(0, close(ruleset_fd));
+}
+
 TEST_F(mini, inval)
 {
 	const struct landlock_ruleset_attr ruleset_attr = {