On Fri, May 24, 2024 at 05:30:06PM +0800, Mikhail Ivanov wrote:
> Initiate socket_test.c selftests. Add protocol fixture for tests
> with changeable family-type values. Only most common variants of
> protocols (like ipv4-tcp,ipv6-udp, unix) were added.
> Add simple socket access right checking test.
>
> Signed-off-by: Mikhail Ivanov <ivanov.mikhail1@xxxxxxxxxxxxxxxxxxx>
> ---
>
> Changes since v1:
> * Replaces test_socket_create() and socket_variant() helpers
> with test_socket().
> * Renames domain to family in protocol fixture.
> * Remove AF_UNSPEC fixture entry and add unspec_srv0 fixture field to
> check AF_UNSPEC socket creation case.
> * Formats code with clang-format.
> * Refactors commit message.
> ---
> .../testing/selftests/landlock/socket_test.c | 181 ++++++++++++++++++
> 1 file changed, 181 insertions(+)
> create mode 100644 tools/testing/selftests/landlock/socket_test.c
>
> diff --git a/tools/testing/selftests/landlock/socket_test.c b/tools/testing/selftests/landlock/socket_test.c
> new file mode 100644
> index 000000000000..4c51f89ed578
> --- /dev/null
> +++ b/tools/testing/selftests/landlock/socket_test.c
> @@ -0,0 +1,181 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Landlock tests - Socket
> + *
> + * Copyright © 2024 Huawei Tech. Co., Ltd.
> + * Copyright © 2024 Microsoft Corporation
It looked to me like these patches came from Huawei?
Was this left by accident?
> + */
> +
> +#define _GNU_SOURCE
> +
> +#include <errno.h>
> +#include <linux/landlock.h>
> +#include <sched.h>
> +#include <string.h>
> +#include <sys/prctl.h>
> +#include <sys/socket.h>
> +
> +#include "common.h"
> +
> +/* clang-format off */
> +
> +#define ACCESS_LAST LANDLOCK_ACCESS_SOCKET_CREATE
> +
> +#define ACCESS_ALL ( \
> + LANDLOCK_ACCESS_SOCKET_CREATE)
> +
> +/* clang-format on */
It does not look like clang-format would really mess up this format in a bad
way. Maybe we can remove the "clang-format off" section here and just write the
"#define"s on one line?
ACCESS_ALL is unused in this commit.
Should it be introduced in a subsequent commit instead?
> +static int test_socket(const struct service_fixture *const srv)
> +{
> + int fd;
> +
> + fd = socket(srv->protocol.family, srv->protocol.type | SOCK_CLOEXEC, 0);
> + if (fd < 0)
> + return errno;
> + /*
> + * Mixing error codes from close(2) and socket(2) should not lead to any
> + * (access type) confusion for this test.
> + */
> + if (close(fd) != 0)
> + return errno;
> + return 0;
> +}
I personally find that it helps me remember if these test helpers have the same
signature as the syscall that they are exercising. (But I don't feel very
strongly about it. Just a suggestion.)
> [...]
>
> +TEST_F(protocol, create)
> +{
> + const struct landlock_ruleset_attr ruleset_attr = {
> + .handled_access_socket = LANDLOCK_ACCESS_SOCKET_CREATE,
> + };
> + const struct landlock_socket_attr create_socket_attr = {
> + .allowed_access = LANDLOCK_ACCESS_SOCKET_CREATE,
> + .family = self->srv0.protocol.family,
> + .type = self->srv0.protocol.type,
> + };
> +
> + int ruleset_fd;
> +
> + /* Allowed create */
> + 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_SOCKET,
> + &create_socket_attr, 0));
> +
> + enforce_ruleset(_metadata, ruleset_fd);
> + EXPECT_EQ(0, close(ruleset_fd));
> +
> + ASSERT_EQ(0, test_socket(&self->srv0));
> + ASSERT_EQ(EAFNOSUPPORT, test_socket(&self->unspec_srv0));
> +
> + /* Denied create */
> + ruleset_fd =
> + landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
> + ASSERT_LE(0, ruleset_fd);
> +
> + enforce_ruleset(_metadata, ruleset_fd);
> + EXPECT_EQ(0, close(ruleset_fd));
> +
> + ASSERT_EQ(EACCES, test_socket(&self->srv0));
> + ASSERT_EQ(EAFNOSUPPORT, test_socket(&self->unspec_srv0));
Should we exhaustively try out the other combinations (other than selv->srv0)
here? I assume socket() should always fail for these?
(If you are alredy doing this in another commit that I have not looked at yet,
please ignore this comment.)
—Günther
