On 9/18/2024 3:54 PM, Günther Noack wrote:
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@xxxxxxxxxxxxxxxxxxx>
---
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?
Agreed, thanks for the nit! I think that docstring would be more
appropriate here (similar to the kernel_socket test).
+{
+ 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?
You're right, this naming is more more suitable for the EPROTONOSUPPORT.
I'll extend this test by adding a separate check for this error code.
+ .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
