| Message ID | 20240904104824.1844082-12-ivanov.mikhail1@huawei-partners.com (mailing list archive) |
|---|---|
| State | RFC |
| Headers | show |
| Series | Support socket access-control | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch, async |
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 >
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@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? 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 >>
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) +{ + 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 = { + .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
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(+)