Re: [PATCH v7 2/4] selftests/landlock: Abstract unix socket restriction tests

From: Mickaël Salaün
Date: Thu Jul 25 2024 - 14:53:48 EST

Next message: Evan Green: "Re: [PATCH v6 6/8] RISC-V: Detect unaligned vector accesses supported"
Previous message: Benno Lossin: "Re: [RFC PATCH] rust: types: Add explanation for ARef pattern"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On Wed, Jul 17, 2024 at 10:15:20PM -0600, Tahera Fahimi wrote:
> The patch has three types of tests:
> 1) unix_socket: base tests the scoping mechanism for a landlocked process,
> same as the ptrace test.
> 2) optional_scoping: generates three processes with different domains and
> tests if a process with a non-scoped domain can connect to other processes.
> 3) unix_sock_special_cases: since the socket's creator credentials are used
> for scoping datagram sockets, this test examines the cases where the
> socket's credentials are different from the process using it.
>
> Signed-off-by: Tahera Fahimi <fahimitahera@xxxxxxxxx>

You need to add "---" here. You can test the result by applying this
patch with `git am`: the comments after these three dashes will not be
inlcuded in the commit message.

>
> Changes in versions:
> V7:
> - Introducing landlock ABI version 6.
> - Adding some edge test cases to optional_scoping test.
> - Using `enum` for different domains in optional_scoping tests.
> - Extend unix_sock_special_cases test cases for connected(SOCK_STREAM) sockets.
> - Modifying inline comments.
> V6:
> - Introducing optional_scoping test which ensures a sandboxed process with a
> non-scoped domain can still connect to another abstract unix socket(either
> sandboxed or non-sandboxed).
> - Introducing unix_sock_special_cases test which tests examines scenarios where
> the connecting sockets have different domain than the process using them.
> V4:
> - Introducing unix_socket to evaluate the basic scoping mechanism for abstract
> unix sockets.
>
> Signed-off-by: Tahera Fahimi <fahimitahera@xxxxxxxxx>
> ---
> tools/testing/selftests/landlock/base_test.c | 2 +-
> .../testing/selftests/landlock/ptrace_test.c | 867 ++++++++++++++++++
> 2 files changed, 868 insertions(+), 1 deletion(-)

Even if these tests are similar to the ptrace's ones, they don't share
code. For maintainance reasons and test flexibility, we should have
these new tests in a "scoped_abstract_unix_test.c" file.

>
> diff --git a/tools/testing/selftests/landlock/base_test.c b/tools/testing/selftests/landlock/base_test.c
> index 3c1e9f35b531..52b00472a487 100644
> --- a/tools/testing/selftests/landlock/base_test.c
> +++ b/tools/testing/selftests/landlock/base_test.c
> @@ -75,7 +75,7 @@ TEST(abi_version)
> const struct landlock_ruleset_attr ruleset_attr = {
> .handled_access_fs = LANDLOCK_ACCESS_FS_READ_FILE,
> };
> - ASSERT_EQ(5, landlock_create_ruleset(NULL, 0,
> + ASSERT_EQ(6, landlock_create_ruleset(NULL, 0,
> LANDLOCK_CREATE_RULESET_VERSION));
>
> ASSERT_EQ(-1, landlock_create_ruleset(&ruleset_attr, 0,
> diff --git a/tools/testing/selftests/landlock/ptrace_test.c b/tools/testing/selftests/landlock/ptrace_test.c
> index a19db4d0b3bd..e7dcefda8ce0 100644
> --- a/tools/testing/selftests/landlock/ptrace_test.c
> +++ b/tools/testing/selftests/landlock/ptrace_test.c

> +/* Test UNIX_STREAM_CONNECT and UNIX_MAY_SEND for parent, child
> + * and grand child processes when they can have scoped or non-scoped
> + * domains.
> + **/
> +TEST_F(optional_scoping, unix_scoping)
> +{
> + pid_t child;
> + socklen_t addrlen;
> + int sock_len = 5;
> + int status;
> + struct sockaddr_un addr = {
> + .sun_family = AF_UNIX,
> + };
> + const char sun_path[8] = "\0test";
> + bool can_connect_to_parent, can_connect_to_child;
> + int pipe_parent[2];
> +
> + if (variant->domain_grand_child == SCOPE_SANDBOX)
> + can_connect_to_child = false;
> + else
> + can_connect_to_child = true;
> +
> + if (!can_connect_to_child || variant->domain_children == SCOPE_SANDBOX)
> + can_connect_to_parent = false;
> + else
> + can_connect_to_parent = true;
> +
> + addrlen = offsetof(struct sockaddr_un, sun_path) + sock_len;
> + memcpy(&addr.sun_path, sun_path, sock_len);
> +
> + ASSERT_EQ(0, pipe2(pipe_parent, O_CLOEXEC));
> +
> + if (variant->domain_all == OTHER_SANDBOX)
> + create_domain(_metadata);
> + else if (variant->domain_all == SCOPE_SANDBOX)
> + create_unix_domain(_metadata);
> +
> + child = fork();
> + ASSERT_LE(0, child);
> + if (child == 0) {
> + int pipe_child[2];
> + ASSERT_EQ(0, pipe2(pipe_child, O_CLOEXEC));
> + pid_t grand_child;
> + struct sockaddr_un child_addr = {
> + .sun_family = AF_UNIX,
> + };
> + const char child_sun_path[8] = "\0tsst";

To avoid potential issues with conflicting names, we should use the same
sun_path as in net_test.c:set_service(). Same for all use of sun_path.

> +
> + memcpy(&child_addr.sun_path, child_sun_path, sock_len);
> +
> + if (variant->domain_children == OTHER_SANDBOX)
> + create_domain(_metadata);
> + else if (variant->domain_children == SCOPE_SANDBOX)
> + create_unix_domain(_metadata);
> +
> + grand_child = fork();
> + ASSERT_LE(0, grand_child);
> + if (grand_child == 0) {
> + ASSERT_EQ(0, close(pipe_parent[1]));
> + ASSERT_EQ(0, close(pipe_child[1]));
> +
> + char buf1, buf2;
> + int err;
> +
> + if (variant->domain_grand_child == OTHER_SANDBOX)
> + create_domain(_metadata);
> + else if (variant->domain_grand_child == SCOPE_SANDBOX)
> + create_unix_domain(_metadata);
> +
> + self->client = socket(AF_UNIX, variant->type, 0);
> + ASSERT_NE(-1, self->client);
> +
> + ASSERT_EQ(1, read(pipe_child[0], &buf2, 1));
> + err = connect(self->client,
> + (struct sockaddr *)&child_addr, addrlen);
> + if (can_connect_to_child) {
> + EXPECT_EQ(0, err);
> + } else {
> + EXPECT_EQ(-1, err);
> + EXPECT_EQ(EPERM, errno);
> + }
> +
> + if (variant->type == SOCK_STREAM) {
> + EXPECT_EQ(0, close(self->client));
> + self->client =
> + socket(AF_UNIX, variant->type, 0);
> + ASSERT_NE(-1, self->client);
> + }
> +
> + ASSERT_EQ(1, read(pipe_parent[0], &buf1, 1));
> + err = connect(self->client, (struct sockaddr *)&addr,
> + addrlen);
> + if (can_connect_to_parent) {
> + EXPECT_EQ(0, err);
> + } else {
> + EXPECT_EQ(-1, err);
> + EXPECT_EQ(EPERM, errno);
> + }
> + EXPECT_EQ(0, close(self->client));
> +
> + _exit(_metadata->exit_code);
> + return;
> + }
> +
> + ASSERT_EQ(0, close(pipe_child[0]));
> + if (variant->domain_child == OTHER_SANDBOX)
> + create_domain(_metadata);
> + else if (variant->domain_child == SCOPE_SANDBOX)
> + create_unix_domain(_metadata);
> +
> + self->child_server = socket(AF_UNIX, variant->type, 0);
> + ASSERT_NE(-1, self->child_server);
> + ASSERT_EQ(0, bind(self->child_server,
> + (struct sockaddr *)&child_addr, addrlen));
> + if (variant->type == SOCK_STREAM)
> + ASSERT_EQ(0, listen(self->child_server, 32));
> +
> + ASSERT_EQ(1, write(pipe_child[1], ".", 1));
> + ASSERT_EQ(grand_child, waitpid(grand_child, &status, 0));
> + return;
> + }
> + ASSERT_EQ(0, close(pipe_parent[0]));
> +
> + if (variant->domain_parent == OTHER_SANDBOX)
> + create_domain(_metadata);
> + else if (variant->domain_parent == SCOPE_SANDBOX)
> + create_unix_domain(_metadata);
> +
> + self->parent_server = socket(AF_UNIX, variant->type, 0);
> + ASSERT_NE(-1, self->parent_server);
> + ASSERT_EQ(0,
> + bind(self->parent_server, (struct sockaddr *)&addr, addrlen));
> +
> + if (variant->type == SOCK_STREAM)
> + ASSERT_EQ(0, listen(self->parent_server, 32));
> +
> + ASSERT_EQ(1, write(pipe_parent[1], ".", 1));
> + ASSERT_EQ(child, waitpid(child, &status, 0));
> + if (WIFSIGNALED(status) || !WIFEXITED(status) ||
> + WEXITSTATUS(status) != EXIT_SUCCESS)
> + _metadata->exit_code = KSFT_FAIL;
> +}
> +
> +/*
> + * Since the special case of scoping only happens when the connecting socket
> + * is scoped, the client's domain is true for all the following test cases.
> + */
> +/* clang-format off */
> +FIXTURE(unix_sock_special_cases) {
> + int server_socket, client;
> + int stream_server, stream_client;
> +};
> +
> +/* clang-format on */
> +FIXTURE_VARIANT(unix_sock_special_cases)
> +{
> + const bool domain_server;
> + const bool domain_server_socket;
> + const int type;
> +};
> +
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(unix_sock_special_cases, allow_dgram_server_sock_domain) {
> + /* clang-format on */
> + .domain_server = false,
> + .domain_server_socket = true,
> + .type = SOCK_DGRAM,
> +};
> +
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(unix_sock_special_cases, deny_dgram_server_domain) {
> + /* clang-format off */
> + .domain_server = true,
> + .domain_server_socket = false,
> + .type = SOCK_DGRAM,
> +};
> +
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(unix_sock_special_cases, allow_stream_server_sock_domain) {
> + /* clang-format on */
> + .domain_server = false,
> + .domain_server_socket = true,
> + .type = SOCK_STREAM,
> +};
> +
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(unix_sock_special_cases, deny_stream_server_domain) {
> + /* clang-format off */
> + .domain_server = true,
> + .domain_server_socket = false,
> + .type = SOCK_STREAM,
> +};
> +
> +FIXTURE_SETUP(unix_sock_special_cases)
> +{
> +}
> +
> +FIXTURE_TEARDOWN(unix_sock_special_cases)
> +{
> + close(self->client);
> + close(self->server_socket);
> + close(self->stream_server);
> + close(self->stream_client);
> +}
> +
> +/* Test UNIX_STREAM_CONNECT and UNIX_MAY_SEND for parent and
> + * child processes when connecting socket has different domain
> + * than the process using it.
> + **/
> +TEST_F(unix_sock_special_cases, dgram_cases)
> +{
> + pid_t child;
> + socklen_t addrlen;
> + int sock_len = 5;
> + struct sockaddr_un addr, addr_stream = {
> + .sun_family = AF_UNIX,
> + };
> + const char sun_path[8] = "\0test";
> + const char sun_path_stream[8] = "\0strm";
> + int err, status;
> + int pipe_child[2], pipe_parent[2];
> + char buf_parent;
> +
> + ASSERT_EQ(0, pipe2(pipe_child, O_CLOEXEC));
> + ASSERT_EQ(0, pipe2(pipe_parent, O_CLOEXEC));
> +
> + addrlen = offsetof(struct sockaddr_un, sun_path) + sock_len;
> + memcpy(&addr.sun_path, sun_path, sock_len);
> +
> + child = fork();
> + ASSERT_LE(0, child);
> + if (child == 0) {
> + char buf_child;
> +
> + ASSERT_EQ(0, close(pipe_parent[1]));
> + ASSERT_EQ(0, close(pipe_child[0]));
> +
> + /* client always has domain */
> + create_unix_domain(_metadata);
> +
> + if (variant->domain_server_socket) {
> + int data_socket;
> + int fd_sock = socket(AF_UNIX, variant->type, 0);
> +
> + ASSERT_NE(-1, fd_sock);
> +
> + self->stream_server = socket(AF_UNIX, SOCK_STREAM, 0);
> +
> + ASSERT_NE(-1, self->stream_server);
> + memcpy(&addr_stream.sun_path, sun_path_stream,
> + sock_len);
> + ASSERT_EQ(0, bind(self->stream_server,
> + (struct sockaddr *)&addr_stream,
> + addrlen));
> + ASSERT_EQ(0, listen(self->stream_server, 32));
> +
> + ASSERT_EQ(1, write(pipe_child[1], ".", 1));
> +
> + data_socket = accept(self->stream_server, NULL, NULL);
> +
> + ASSERT_EQ(0, send_fd(data_socket, fd_sock));
> + ASSERT_EQ(0, close(fd_sock));
> + TH_LOG("sending completed\n");
> + }
> +
> + self->client = socket(AF_UNIX, variant->type, 0);
> + ASSERT_NE(-1, self->client);
> + /* wait for parent signal for connection */
> + ASSERT_EQ(1, read(pipe_parent[0], &buf_child, 1));
> +
> + err = connect(self->client, (struct sockaddr *)&addr, addrlen);
> + if (!variant->domain_server_socket) {
> + EXPECT_EQ(-1, err);
> + EXPECT_EQ(EPERM, errno);
> + } else {
> + EXPECT_EQ(0, err);
> + }
> + _exit(_metadata->exit_code);
> + return;
> + }
> +
> + ASSERT_EQ(0, close(pipe_child[1]));
> + ASSERT_EQ(0, close(pipe_parent[0]));
> +
> + if (!variant->domain_server_socket) {
> + self->server_socket = socket(AF_UNIX, variant->type, 0);
> + } else {
> + int cli = socket(AF_UNIX, SOCK_STREAM, 0);
> +
> + ASSERT_NE(-1, cli);
> + memcpy(&addr_stream.sun_path, sun_path_stream, sock_len);
> + ASSERT_EQ(1, read(pipe_child[0], &buf_parent, 1));
> + ASSERT_EQ(0, connect(cli, (struct sockaddr *)&addr_stream,
> + addrlen));
> +
> + self->server_socket = recv_fd(cli);
> + ASSERT_LE(0, self->server_socket);
> + }
> +
> + ASSERT_NE(-1, self->server_socket);
> +
> + if (variant->domain_server)
> + create_unix_domain(_metadata);
> +
> + ASSERT_EQ(0,
> + bind(self->server_socket, (struct sockaddr *)&addr, addrlen));
> + if (variant->type == SOCK_STREAM)
> + ASSERT_EQ(0, listen(self->server_socket, 32));
> + /* signal to child that parent is listening */
> + ASSERT_EQ(1, write(pipe_parent[1], ".", 1));
> +
> + ASSERT_EQ(child, waitpid(child, &status, 0));
> +
> + if (WIFSIGNALED(status) || !WIFEXITED(status) ||
> + WEXITSTATUS(status) != EXIT_SUCCESS)
> + _metadata->exit_code = KSFT_FAIL;
> +}
> TEST_HARNESS_MAIN
> --
> 2.34.1
>
>

Next message: Evan Green: "Re: [PATCH v6 6/8] RISC-V: Detect unaligned vector accesses supported"
Previous message: Benno Lossin: "Re: [RFC PATCH] rust: types: Add explanation for ARef pattern"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]