Re: [RFC PATCH v3 11/19] selftests/landlock: Test unsupported protocol restriction

["Günther Noack" <gnoack@google.com>]

On Wed, Sep 04, 2024 at 06:48:16PM +0800, Mikhail Ivanov wrote:
> Add test validating that Landlock doesn't wrongfully
> return EACCES for unsupported address family and protocol.
> 
> Signed-off-by: Mikhail Ivanov <ivanov.mikhail1@huawei-partners.com>
> ---
> Changes since v1:
> * Adds socket(2) error code check when ruleset is not established.
> * Tests unsupported family for error code consistency.
> * Renames test to `unsupported_af_and_prot`.
> * Refactors commit title and message.
> * Minor fixes.
> ---
>  .../testing/selftests/landlock/socket_test.c  | 47 +++++++++++++++++++
>  1 file changed, 47 insertions(+)
> 
> diff --git a/tools/testing/selftests/landlock/socket_test.c b/tools/testing/selftests/landlock/socket_test.c
> index 047603abc5a7..ff5ace711697 100644
> --- a/tools/testing/selftests/landlock/socket_test.c
> +++ b/tools/testing/selftests/landlock/socket_test.c
> @@ -581,4 +581,51 @@ TEST_F(prot_outside_range, add_rule)
>  	ASSERT_EQ(0, close(ruleset_fd));
>  }
>  
> +TEST(unsupported_af_and_prot)

Nit: If I am reading this test correctly, the point is to make sure that for
unsuported (EAFNOSUPPORT and ESOCKTNOSUPPORT) combinations of "family" and
"type", socket(2) returns the same error code, independent of whether that
combination is restricted with Landlock or not.  Maybe we could make it more
clear from the test name or a brief docstring that this is about error code
compatibility when calling socket() under from within a Landlock domain?

> +{
> +	const struct landlock_ruleset_attr ruleset_attr = {
> +		.handled_access_socket = LANDLOCK_ACCESS_SOCKET_CREATE,
> +	};
> +	struct landlock_socket_attr socket_af_unsupported = {
> +		.allowed_access = LANDLOCK_ACCESS_SOCKET_CREATE,
> +		.family = AF_UNSPEC,
> +		.type = SOCK_STREAM,
> +	};
> +	struct landlock_socket_attr socket_prot_unsupported = {
                                           ^^^^
Here and in the test name: Should this say "type" instead of "prot"?
It seems that the part that is unsupported here is the socket(2) "type"
argument, not the "protocol" argument?

> +		.allowed_access = LANDLOCK_ACCESS_SOCKET_CREATE,
> +		.family = AF_UNIX,
> +		.type = SOCK_PACKET,
> +	};
> +	int ruleset_fd;
> +
> +	/* Tries to create a socket when ruleset is not established. */
> +	ASSERT_EQ(EAFNOSUPPORT, test_socket(AF_UNSPEC, SOCK_STREAM, 0));
> +	ASSERT_EQ(ESOCKTNOSUPPORT, test_socket(AF_UNIX, SOCK_PACKET, 0));
> +
> +	ruleset_fd =
> +		landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
> +	ASSERT_LE(0, ruleset_fd);
> +
> +	EXPECT_EQ(0, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_SOCKET,
> +				       &socket_af_unsupported, 0));
> +	EXPECT_EQ(0, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_SOCKET,
> +				       &socket_prot_unsupported, 0));
> +	enforce_ruleset(_metadata, ruleset_fd);
> +	ASSERT_EQ(0, close(ruleset_fd));
> +
> +	/* Tries to create a socket when protocols are allowed. */
> +	EXPECT_EQ(EAFNOSUPPORT, test_socket(AF_UNSPEC, SOCK_STREAM, 0));
> +	EXPECT_EQ(ESOCKTNOSUPPORT, test_socket(AF_UNIX, SOCK_PACKET, 0));
> +
> +	ruleset_fd =
> +		landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
> +	ASSERT_LE(0, ruleset_fd);
> +	enforce_ruleset(_metadata, ruleset_fd);
> +	ASSERT_EQ(0, close(ruleset_fd));
> +
> +	/* Tries to create a socket when protocols are restricted. */
> +	EXPECT_EQ(EAFNOSUPPORT, test_socket(AF_UNSPEC, SOCK_STREAM, 0));
> +	EXPECT_EQ(ESOCKTNOSUPPORT, test_socket(AF_UNIX, SOCK_PACKET, 0));
> +}
> +
>  TEST_HARNESS_MAIN
> -- 
> 2.34.1
>