| Message ID | 20240904104824.1844082-8-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:12PM +0800, Mikhail Ivanov wrote: > Add test that validates behaviour of Landlock after rule with > empty access is added. > > Signed-off-by: Mikhail Ivanov <ivanov.mikhail1@huawei-partners.com> > --- > Changes since v2: > * Renames protocol.inval into protocol.rule_with_empty_access. > * Replaces ASSERT_EQ with EXPECT_EQ for landlock_add_rule(). > * Closes ruleset_fd. > * Refactors commit message and title. > * Minor fixes. > > Changes since v1: > * Refactors commit message. > --- > .../testing/selftests/landlock/socket_test.c | 33 +++++++++++++++++++ > 1 file changed, 33 insertions(+) > > diff --git a/tools/testing/selftests/landlock/socket_test.c b/tools/testing/selftests/landlock/socket_test.c > index d2fedfca7193..d323f649a183 100644 > --- a/tools/testing/selftests/landlock/socket_test.c > +++ b/tools/testing/selftests/landlock/socket_test.c > @@ -384,4 +384,37 @@ TEST_F(protocol, rule_with_unhandled_access) > ASSERT_EQ(0, close(ruleset_fd)); > } > > +TEST_F(protocol, rule_with_empty_access) > +{ > + const struct landlock_ruleset_attr ruleset_attr = { > + .handled_access_socket = LANDLOCK_ACCESS_SOCKET_CREATE > + }; > + struct landlock_socket_attr protocol_allowed = { > + .allowed_access = LANDLOCK_ACCESS_SOCKET_CREATE, > + .family = self->prot.family, > + .type = self->prot.type, > + }; > + struct landlock_socket_attr protocol_denied = { > + .allowed_access = 0, > + .family = self->prot.family, > + .type = self->prot.type, > + }; > + int ruleset_fd; > + > + ruleset_fd = > + landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0); > + ASSERT_LE(0, ruleset_fd); > + > + /* Checks zero access value. */ > + EXPECT_EQ(-1, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_SOCKET, > + &protocol_denied, 0)); > + EXPECT_EQ(ENOMSG, errno); > + > + /* Adds with legitimate value. */ > + EXPECT_EQ(0, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_SOCKET, > + &protocol_allowed, 0)); In my mind, the check with the legitimate rule is probably already done in other places and does not strictly need to be duplicated here. But up to you, it's fine either way. :) Reviewed-by: Günther Noack <gnoack@google.com> > + > + ASSERT_EQ(0, close(ruleset_fd)); > +} > + > TEST_HARNESS_MAIN > -- > 2.34.1 >
On 9/18/2024 3:42 PM, Günther Noack wrote: > On Wed, Sep 04, 2024 at 06:48:12PM +0800, Mikhail Ivanov wrote: >> Add test that validates behaviour of Landlock after rule with >> empty access is added. >> >> Signed-off-by: Mikhail Ivanov <ivanov.mikhail1@huawei-partners.com> >> --- >> Changes since v2: >> * Renames protocol.inval into protocol.rule_with_empty_access. >> * Replaces ASSERT_EQ with EXPECT_EQ for landlock_add_rule(). >> * Closes ruleset_fd. >> * Refactors commit message and title. >> * Minor fixes. >> >> Changes since v1: >> * Refactors commit message. >> --- >> .../testing/selftests/landlock/socket_test.c | 33 +++++++++++++++++++ >> 1 file changed, 33 insertions(+) >> >> diff --git a/tools/testing/selftests/landlock/socket_test.c b/tools/testing/selftests/landlock/socket_test.c >> index d2fedfca7193..d323f649a183 100644 >> --- a/tools/testing/selftests/landlock/socket_test.c >> +++ b/tools/testing/selftests/landlock/socket_test.c >> @@ -384,4 +384,37 @@ TEST_F(protocol, rule_with_unhandled_access) >> ASSERT_EQ(0, close(ruleset_fd)); >> } >> >> +TEST_F(protocol, rule_with_empty_access) >> +{ >> + const struct landlock_ruleset_attr ruleset_attr = { >> + .handled_access_socket = LANDLOCK_ACCESS_SOCKET_CREATE >> + }; >> + struct landlock_socket_attr protocol_allowed = { >> + .allowed_access = LANDLOCK_ACCESS_SOCKET_CREATE, >> + .family = self->prot.family, >> + .type = self->prot.type, >> + }; >> + struct landlock_socket_attr protocol_denied = { >> + .allowed_access = 0, >> + .family = self->prot.family, >> + .type = self->prot.type, >> + }; >> + int ruleset_fd; >> + >> + ruleset_fd = >> + landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0); >> + ASSERT_LE(0, ruleset_fd); >> + >> + /* Checks zero access value. */ >> + EXPECT_EQ(-1, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_SOCKET, >> + &protocol_denied, 0)); >> + EXPECT_EQ(ENOMSG, errno); >> + >> + /* Adds with legitimate value. */ >> + EXPECT_EQ(0, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_SOCKET, >> + &protocol_allowed, 0)); > > In my mind, the check with the legitimate rule is probably already done in other > places and does not strictly need to be duplicated here. > > But up to you, it's fine either way. :) This test is a duplicate of mini.inval from net_test.c. I thought this line can be useful to check that adding rule with zero access does not affect Landlock behavior of adding a line with legitimate value. But this is a really weak reason and I'd like to remove this line for simplicity. Thank you! > > Reviewed-by: Günther Noack <gnoack@google.com> > >> + >> + ASSERT_EQ(0, close(ruleset_fd)); >> +} >> + >> 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 d2fedfca7193..d323f649a183 100644 --- a/tools/testing/selftests/landlock/socket_test.c +++ b/tools/testing/selftests/landlock/socket_test.c @@ -384,4 +384,37 @@ TEST_F(protocol, rule_with_unhandled_access) ASSERT_EQ(0, close(ruleset_fd)); } +TEST_F(protocol, rule_with_empty_access) +{ + const struct landlock_ruleset_attr ruleset_attr = { + .handled_access_socket = LANDLOCK_ACCESS_SOCKET_CREATE + }; + struct landlock_socket_attr protocol_allowed = { + .allowed_access = LANDLOCK_ACCESS_SOCKET_CREATE, + .family = self->prot.family, + .type = self->prot.type, + }; + struct landlock_socket_attr protocol_denied = { + .allowed_access = 0, + .family = self->prot.family, + .type = self->prot.type, + }; + int ruleset_fd; + + ruleset_fd = + landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0); + ASSERT_LE(0, ruleset_fd); + + /* Checks zero access value. */ + EXPECT_EQ(-1, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_SOCKET, + &protocol_denied, 0)); + EXPECT_EQ(ENOMSG, errno); + + /* Adds with legitimate value. */ + EXPECT_EQ(0, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_SOCKET, + &protocol_allowed, 0)); + + ASSERT_EQ(0, close(ruleset_fd)); +} + TEST_HARNESS_MAIN
Add test that validates behaviour of Landlock after rule with empty access is added. Signed-off-by: Mikhail Ivanov <ivanov.mikhail1@huawei-partners.com> --- Changes since v2: * Renames protocol.inval into protocol.rule_with_empty_access. * Replaces ASSERT_EQ with EXPECT_EQ for landlock_add_rule(). * Closes ruleset_fd. * Refactors commit message and title. * Minor fixes. Changes since v1: * Refactors commit message. --- .../testing/selftests/landlock/socket_test.c | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+)