[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <a31a0cc8-cf2f-5fe3-3efc-f817b8a5c6db@huawei.com>
Date: Fri, 18 Aug 2023 17:05:20 +0300
From: "Konstantin Meskhidze (A)" <konstantin.meskhidze@...wei.com>
To: Mickaël Salaün <mic@...ikod.net>
CC: <artem.kuzin@...wei.com>, <gnoack3000@...il.com>,
<willemdebruijn.kernel@...il.com>, <yusongping@...wei.com>,
<linux-security-module@...r.kernel.org>, <netdev@...r.kernel.org>,
<netfilter-devel@...r.kernel.org>
Subject: Re: [PATCH v11.1] selftests/landlock: Add 11 new test suites
dedicated to network
8/17/2023 6:34 PM, Mickaël Salaün пишет:
> On Thu, Aug 17, 2023 at 05:04:00PM +0300, Konstantin Meskhidze (A) wrote:
>>
>>
>> 8/17/2023 4:19 PM, Mickaël Salaün пишет:
>> > On Sun, Aug 13, 2023 at 11:09:59PM +0300, Konstantin Meskhidze (A) wrote:
>> > >
>> > >
>> > > 7/12/2023 10:02 AM, Mickaël Salaün пишет:
>> > > > > On 06/07/2023 16:55, Mickaël Salaün wrote:
>> > > > > From: Konstantin Meskhidze <konstantin.meskhidze@...wei.com>
>> > > > > > > This patch is a revamp of the v11 tests [1] with new tests
>> > > (see the
>> > > > > "Changes since v11" description). I (Mickaël) only added the following
>> > > > > todo list and the "Changes since v11" sections in this commit message.
>> > > > > I think this patch is good but it would appreciate reviews.
>> > > > > You can find the diff of my changes here but it is not really readable:
>> > > > > https://git.kernel.org/mic/c/78edf722fba5 (landlock-net-v11 branch)
>> > > > > [1] https://lore.kernel.org/all/20230515161339.631577-11-konstantin.meskhidze@huawei.com/
>> > > > > TODO:
>> > > > > - Rename all "net_service" to "net_port".
>> > > > > - Fix the two kernel bugs found with the new tests.
>> > > > > - Update this commit message with a small description of all tests.
>> > > > > [...]
>> >
>> > > > > +FIXTURE(inet)
>> > > > > +{
>> > > > > + struct service_fixture srv0, srv1;
>> > > > > +};
>> > > > > The "inet" variants are useless and should be removed. The
>> > > "inet"
>> > > > fixture can then be renamed to "ipv4_tcp".
>> > > > Maybe its better to name it "tcp". So we dont need to copy
>> > > TEST_F(tcp,
>> > > port_endianness) for ipv6 and ipv4.
>> > > What do you think?
>> >
>> > I don't see any need to test with IPv4 and IPv6, hence the "inet" name
>> > (and without variants). You can rename it to "inet_tcp" to highlight the
>> > specificities of this fixture.
>> >
>>
>> I think there was some misunderstanding from my side. So I will rename
>> inet to inet_tcp and keep all fixture variants:
>> - no_sandbox_with_ipv4.
>> - sandbox_with_ipv4.
>> - no_sandbox_with_ipv6.
>> - sandbox_with_ipv6.
>> Correct?
>
> No, you just need to remove the FIXTURE_VARIANT and the four
> FIXTURE_VARIANT_ADD blocks bellow. And according to another reply,
> "ipv4_tcp" seems more appropriate.
>
Ok. Got it. Thank you.
>
>> > > > > > +
>> > > > > +FIXTURE_VARIANT(inet)
>> > > > > +{
>> > > > > + const bool is_sandboxed;
>> > > > > + const struct protocol_variant prot;
>> > > > > +};
>> > > > > +
>> > > > > +/* clang-format off */
>> > > > > +FIXTURE_VARIANT_ADD(inet, no_sandbox_with_ipv4) {
>> > > > > + /* clang-format on */
>> > > > > + .is_sandboxed = false,
>> > > > > + .prot = {
>> > > > > + .domain = AF_INET,
>> > > > > + .type = SOCK_STREAM,
>> > > > > + },
>> > > > > +};
>> > > > > +
>> > > > > +/* clang-format off */
>> > > > > +FIXTURE_VARIANT_ADD(inet, sandbox_with_ipv4) {
>> > > > > + /* clang-format on */
>> > > > > + .is_sandboxed = true,
>> > > > > + .prot = {
>> > > > > + .domain = AF_INET,
>> > > > > + .type = SOCK_STREAM,
>> > > > > + },
>> > > > > +};
>> > > > > +
>> > > > > +/* clang-format off */
>> > > > > +FIXTURE_VARIANT_ADD(inet, no_sandbox_with_ipv6) {
>> > > > > + /* clang-format on */
>> > > > > + .is_sandboxed = false,
>> > > > > + .prot = {
>> > > > > + .domain = AF_INET6,
>> > > > > + .type = SOCK_STREAM,
>> > > > > + },
>> > > > > +};
>> > > > > +
>> > > > > +/* clang-format off */
>> > > > > +FIXTURE_VARIANT_ADD(inet, sandbox_with_ipv6) {
>> > > > > + /* clang-format on */
>> > > > > + .is_sandboxed = true,
>> > > > > + .prot = {
>> > > > > + .domain = AF_INET6,
>> > > > > + .type = SOCK_STREAM,
>> > > > > + },
>> > > > > +};
>> > > > > +
>> > > > > +FIXTURE_SETUP(inet)
>> > > > > +{
>> > > > > + const struct protocol_variant ipv4_tcp = {
>> > > > > + .domain = AF_INET,
>> > > > > + .type = SOCK_STREAM,
>> > > > > + };
>> > > > > +
>> > > > > + disable_caps(_metadata);
>> > > > > +
>> > > > > + ASSERT_EQ(0, set_service(&self->srv0, ipv4_tcp, 0));
>> > > > > + ASSERT_EQ(0, set_service(&self->srv1, ipv4_tcp, 1));
>> > > > > +
>> > > > > + setup_loopback(_metadata);
>> > > > > +};
>> > > > > +
>> > > > > +FIXTURE_TEARDOWN(inet)
>> > > > > +{
>> > > > > +}
>> > > > > +
>> > > > > +TEST_F(inet, port_endianness)
>> > > > > +{
>> > > > > + const struct landlock_ruleset_attr ruleset_attr = {
>> > > > > + .handled_access_net = LANDLOCK_ACCESS_NET_BIND_TCP |
>> > > > > + LANDLOCK_ACCESS_NET_CONNECT_TCP,
>> > > > > + };
>> > > > > + const struct landlock_net_service_attr bind_host_endian_p0 = {
>> > > > > + .allowed_access = LANDLOCK_ACCESS_NET_BIND_TCP,
>> > > > > + /* Host port format. */
>> > > > > + .port = self->srv0.port,
>> > > > > + };
>> > > > > + const struct landlock_net_service_attr connect_big_endian_p0 = {
>> > > > > + .allowed_access = LANDLOCK_ACCESS_NET_CONNECT_TCP,
>> > > > > + /* Big endian port format. */
>> > > > > + .port = htons(self->srv0.port),
>> > > > > + };
>> > > > > + const struct landlock_net_service_attr bind_connect_host_endian_p1 = {
>> > > > > + .allowed_access = LANDLOCK_ACCESS_NET_BIND_TCP |
>> > > > > + LANDLOCK_ACCESS_NET_CONNECT_TCP,
>> > > > > + /* Host port format. */
>> > > > > + .port = self->srv1.port,
>> > > > > + };
>> > > > > + const unsigned int one = 1;
>> > > > > + const char little_endian = *(const char *)&one;
>> > > > > + int ruleset_fd;
>> > > > > +
>> > > > > + ruleset_fd =
>> > > > > + landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
>> > > > > + ASSERT_LE(0, ruleset_fd);
>> > > > > + ASSERT_EQ(0, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_NET_SERVICE,
>> > > > > + &bind_host_endian_p0, 0));
>> > > > > + ASSERT_EQ(0, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_NET_SERVICE,
>> > > > > + &connect_big_endian_p0, 0));
>> > > > > + ASSERT_EQ(0, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_NET_SERVICE,
>> > > > > + &bind_connect_host_endian_p1, 0));
>> > > > > + enforce_ruleset(_metadata, ruleset_fd);
>> > > > > +
>> > > > > + /* No restriction for big endinan CPU. */
>> > > > > + test_bind_and_connect(_metadata, &self->srv0, false, little_endian);
>> > > > > +
>> > > > > + /* No restriction for any CPU. */
>> > > > > + test_bind_and_connect(_metadata, &self->srv1, false, false);
>> > > > > +}
>> > > > > +
>> > > > > +TEST_HARNESS_MAIN
>> > > > .
>> > .
> .
Powered by blists - more mailing lists