On Wed, Sep 04, 2024 at 06:48:20PM +0800, Mikhail Ivanov wrote:
> It is possible to branch off an SCTP UDP association into a separate
> user space UDP socket. Add test validating that such scenario is not
> restricted by Landlock.
>
> Move setup_loopback() helper from net_test to common.h to use it to
> enable connection in this test.
>
> Signed-off-by: Mikhail Ivanov <ivanov.mikhail1@xxxxxxxxxxxxxxxxxxx>
> ---
> tools/testing/selftests/landlock/common.h | 12 +++
> tools/testing/selftests/landlock/net_test.c | 11 --
> .../testing/selftests/landlock/socket_test.c | 102 +++++++++++++++++-
> 3 files changed, 113 insertions(+), 12 deletions(-)
>
> diff --git a/tools/testing/selftests/landlock/common.h b/tools/testing/selftests/landlock/common.h
> index 28df49fa22d5..07d959a8ac7b 100644
> --- a/tools/testing/selftests/landlock/common.h
> +++ b/tools/testing/selftests/landlock/common.h
> @@ -16,6 +16,7 @@
> #include <sys/types.h>
> #include <sys/wait.h>
> #include <unistd.h>
> +#include <sched.h>
>
> #include "../kselftest_harness.h"
>
> @@ -227,3 +228,14 @@ enforce_ruleset(struct __test_metadata *const _metadata, const int ruleset_fd)
> TH_LOG("Failed to enforce ruleset: %s", strerror(errno));
> }
> }
> +
> +static void setup_loopback(struct __test_metadata *const _metadata)
> +{
> + set_cap(_metadata, CAP_SYS_ADMIN);
> + ASSERT_EQ(0, unshare(CLONE_NEWNET));
> + clear_cap(_metadata, CAP_SYS_ADMIN);
> +
> + set_ambient_cap(_metadata, CAP_NET_ADMIN);
> + ASSERT_EQ(0, system("ip link set dev lo up"));
> + clear_ambient_cap(_metadata, CAP_NET_ADMIN);
> +}
> diff --git a/tools/testing/selftests/landlock/net_test.c b/tools/testing/selftests/landlock/net_test.c
> index f21cfbbc3638..0b8386657c72 100644
> --- a/tools/testing/selftests/landlock/net_test.c
> +++ b/tools/testing/selftests/landlock/net_test.c
> @@ -103,17 +103,6 @@ static int set_service(struct service_fixture *const srv,
> return 1;
> }
>
> -static void setup_loopback(struct __test_metadata *const _metadata)
> -{
> - set_cap(_metadata, CAP_SYS_ADMIN);
> - ASSERT_EQ(0, unshare(CLONE_NEWNET));
> - clear_cap(_metadata, CAP_SYS_ADMIN);
> -
> - set_ambient_cap(_metadata, CAP_NET_ADMIN);
> - ASSERT_EQ(0, system("ip link set dev lo up"));
> - clear_ambient_cap(_metadata, CAP_NET_ADMIN);
> -}
> -
> static bool is_restricted(const struct protocol_variant *const prot,
> const enum sandbox_type sandbox)
> {
> diff --git a/tools/testing/selftests/landlock/socket_test.c b/tools/testing/selftests/landlock/socket_test.c
> index 67db0e1c1121..2ab27196fa3d 100644
> --- a/tools/testing/selftests/landlock/socket_test.c
> +++ b/tools/testing/selftests/landlock/socket_test.c
> @@ -11,8 +11,11 @@
> #include <linux/pfkeyv2.h>
> #include <linux/kcm.h>
> #include <linux/can.h>
> -#include <linux/in.h>
> +#include <sys/socket.h>
> +#include <stdint.h>
> +#include <linux/sctp.h>
> #include <sys/prctl.h>
> +#include <arpa/inet.h>
>
> #include "common.h"
>
> @@ -839,4 +842,101 @@ TEST_F(socket_creation, socketpair)
> }
> }
>
> +static const char loopback_ipv4[] = "127.0.0.1";
> +static const int backlog = 10;
> +static const int loopback_port = 1024;
> +
> +TEST_F(socket_creation, sctp_peeloff)
> +{
> + int status, ret;
> + pid_t child;
> + struct sockaddr_in addr;
> + int server_fd;
> +
> + server_fd =
> + socket(AF_INET, SOCK_SEQPACKET | SOCK_CLOEXEC, IPPROTO_SCTP);
> + ASSERT_LE(0, server_fd);
> +
> + addr.sin_family = AF_INET;
> + addr.sin_port = htons(loopback_port);
> + addr.sin_addr.s_addr = inet_addr(loopback_ipv4);
> +
> + ASSERT_EQ(0, bind(server_fd, &addr, sizeof(addr)));
> + ASSERT_EQ(0, listen(server_fd, backlog));
> +
> + child = fork();
> + ASSERT_LE(0, child);
> + if (child == 0) {
> + int client_fd;
> + sctp_peeloff_flags_arg_t peeloff;
> + socklen_t peeloff_size = sizeof(peeloff);
> + const struct landlock_ruleset_attr ruleset_attr = {
> + .handled_access_socket = LANDLOCK_ACCESS_SOCKET_CREATE,
> + };
> + struct landlock_socket_attr sctp_socket_create = {
> + .allowed_access = LANDLOCK_ACCESS_SOCKET_CREATE,
> + .family = AF_INET,
> + .type = SOCK_SEQPACKET,
> + };
> +
> + /* Closes listening socket for the child. */
> + ASSERT_EQ(0, close(server_fd));
> +
> + client_fd = socket(AF_INET, SOCK_SEQPACKET | SOCK_CLOEXEC,
> + IPPROTO_SCTP);
> + ASSERT_LE(0, client_fd);
> +
> + /*
> + * Establishes connection between sockets and
> + * gets SCTP association id.
> + */
> + ret = setsockopt(client_fd, IPPROTO_SCTP, SCTP_SOCKOPT_CONNECTX,
> + &addr, sizeof(addr));
> + ASSERT_LE(0, ret);
> +
> + if (self->sandboxed) {
> + /* Denies creation of SCTP sockets. */
> + int ruleset_fd = landlock_create_ruleset(
> + &ruleset_attr, sizeof(ruleset_attr), 0);
> + ASSERT_LE(0, ruleset_fd);
> +
> + if (self->allowed) {
> + ASSERT_EQ(0, landlock_add_rule(
> + ruleset_fd,
> + LANDLOCK_RULE_SOCKET,
> + &sctp_socket_create, 0));
> + }
> + enforce_ruleset(_metadata, ruleset_fd);
> + ASSERT_EQ(0, close(ruleset_fd));
> + }
> + /*
> + * Branches off current SCTP association into a separate socket
> + * and returns it to user space.
> + */
> + peeloff.p_arg.associd = ret;
> + ret = getsockopt(client_fd, IPPROTO_SCTP, SCTP_SOCKOPT_PEELOFF,
> + &peeloff, &peeloff_size);
> +
> + /*
> + * Creation of SCTP socket by branching off existing SCTP association
> + * should not be restricted by Landlock.
> + */
> + EXPECT_LE(0, ret);
> +
> + /* Closes peeloff socket if such was created. */
> + if (!ret) {
> + ASSERT_EQ(0, close(peeloff.p_arg.sd));
> + }
Nit: Should this check for (ret >= 0) instead?
I imagine that getsockopt returns -1 on error, normally,
and that would make it past the EXPECT_LE (even if it logs a failure).
> + ASSERT_EQ(0, close(client_fd));
> + _exit(_metadata->exit_code);
> + return;
> + }
> +
> + ASSERT_EQ(child, waitpid(child, &status, 0));
> + ASSERT_EQ(1, WIFEXITED(status));
> + ASSERT_EQ(EXIT_SUCCESS, WEXITSTATUS(status));
> +
> + ASSERT_EQ(0, close(server_fd));
> +}
> +
> TEST_HARNESS_MAIN
> --
> 2.34.1
>
Reviewed-by: Günther Noack <gnoack@xxxxxxxxxx>
