From: "Günther Noack" <gnoack@google.com>
To: Mikhail Ivanov <ivanov.mikhail1@huawei-partners.com>
Cc: mic@digikod.net, willemdebruijn.kernel@gmail.com,
linux-security-module@vger.kernel.org, netdev@vger.kernel.org,
netfilter-devel@vger.kernel.org, yusongping@huawei.com,
artem.kuzin@huawei.com, konstantin.meskhidze@huawei.com
Subject: Re: [RFC PATCH v1 2/2] selftests/landlock: Test non-TCP INET connection-based protocols
Date: Thu, 3 Oct 2024 17:59:14 +0200 [thread overview]
Message-ID: <Zv6_Uitud0OzxKTn@google.com> (raw)
In-Reply-To: <20241003143932.2431249-3-ivanov.mikhail1@huawei-partners.com>
On Thu, Oct 03, 2024 at 10:39:32PM +0800, Mikhail Ivanov wrote:
> Extend protocol fixture with test suits for MPTCP, SCTP and SMC protocols.
> Add all options required by this protocols in config.
>
> Extend protocol_variant structure with protocol field (Cf. socket(2)).
>
> Refactor is_restricted() helper and add few helpers to check struct
> protocol_variant on specific protocols.
>
> Signed-off-by: Mikhail Ivanov <ivanov.mikhail1@huawei-partners.com>
> ---
> tools/testing/selftests/landlock/common.h | 1 +
> tools/testing/selftests/landlock/config | 5 +
> tools/testing/selftests/landlock/net_test.c | 212 ++++++++++++++++++--
> 3 files changed, 198 insertions(+), 20 deletions(-)
>
> diff --git a/tools/testing/selftests/landlock/common.h b/tools/testing/selftests/landlock/common.h
> index 61056fa074bb..40a2def50b83 100644
> --- a/tools/testing/selftests/landlock/common.h
> +++ b/tools/testing/selftests/landlock/common.h
> @@ -234,6 +234,7 @@ enforce_ruleset(struct __test_metadata *const _metadata, const int ruleset_fd)
> struct protocol_variant {
> int domain;
> int type;
> + int protocol;
> };
>
> struct service_fixture {
> diff --git a/tools/testing/selftests/landlock/config b/tools/testing/selftests/landlock/config
> index 29af19c4e9f9..73b01d7d0881 100644
> --- a/tools/testing/selftests/landlock/config
> +++ b/tools/testing/selftests/landlock/config
> @@ -1,8 +1,12 @@
> CONFIG_CGROUPS=y
> CONFIG_CGROUP_SCHED=y
> CONFIG_INET=y
> +CONFIG_INFINIBAND=y
> +CONFIG_IP_SCTP=y
> CONFIG_IPV6=y
> CONFIG_KEYS=y
> +CONFIG_MPTCP=y
> +CONFIG_MPTCP_IPV6=y
> CONFIG_NET=y
> CONFIG_NET_NS=y
> CONFIG_OVERLAY_FS=y
> @@ -10,6 +14,7 @@ CONFIG_PROC_FS=y
> CONFIG_SECURITY=y
> CONFIG_SECURITY_LANDLOCK=y
> CONFIG_SHMEM=y
> +CONFIG_SMC=y
> CONFIG_SYSFS=y
> CONFIG_TMPFS=y
> CONFIG_TMPFS_XATTR=y
> diff --git a/tools/testing/selftests/landlock/net_test.c b/tools/testing/selftests/landlock/net_test.c
> index 4e0aeb53b225..dbe77d436281 100644
> --- a/tools/testing/selftests/landlock/net_test.c
> +++ b/tools/testing/selftests/landlock/net_test.c
> @@ -36,6 +36,17 @@ enum sandbox_type {
> TCP_SANDBOX,
> };
>
> +/* Checks if IPPROTO_SMC is present for compatibility reasons. */
> +#if !defined(__alpha__) && defined(IPPROTO_SMC)
> +#define SMC_SUPPORTED 1
> +#else
> +#define SMC_SUPPORTED 0
> +#endif
> +
> +#ifndef IPPROTO_SMC
> +#define IPPROTO_SMC 256
> +#endif
> +
> static int set_service(struct service_fixture *const srv,
> const struct protocol_variant prot,
> const unsigned short index)
> @@ -85,19 +96,37 @@ static void setup_loopback(struct __test_metadata *const _metadata)
> clear_ambient_cap(_metadata, CAP_NET_ADMIN);
> }
>
> +static bool prot_is_inet_stream(const struct protocol_variant *const prot)
> +{
> + return (prot->domain == AF_INET || prot->domain == AF_INET6) &&
> + prot->type == SOCK_STREAM;
> +}
> +
> +static bool prot_is_tcp(const struct protocol_variant *const prot)
> +{
> + return prot_is_inet_stream(prot) &&
> + (prot->protocol == IPPROTO_TCP || prot->protocol == IPPROTO_IP);
> +}
> +
> +static bool prot_is_sctp(const struct protocol_variant *const prot)
> +{
> + return prot_is_inet_stream(prot) && prot->protocol == IPPROTO_SCTP;
> +}
> +
> +static bool prot_is_smc(const struct protocol_variant *const prot)
> +{
> + return prot_is_inet_stream(prot) && prot->protocol == IPPROTO_SMC;
> +}
> +
> +static bool prot_is_unix_stream(const struct protocol_variant *const prot)
> +{
> + return prot->domain == AF_UNIX && prot->type == SOCK_STREAM;
> +}
> +
> static bool is_restricted(const struct protocol_variant *const prot,
> const enum sandbox_type sandbox)
> {
> - switch (prot->domain) {
> - case AF_INET:
> - case AF_INET6:
> - switch (prot->type) {
> - case SOCK_STREAM:
> - return sandbox == TCP_SANDBOX;
> - }
> - break;
> - }
> - return false;
> + return prot_is_tcp(prot) && sandbox == TCP_SANDBOX;
> }
>
> static int socket_variant(const struct service_fixture *const srv)
> @@ -105,7 +134,7 @@ static int socket_variant(const struct service_fixture *const srv)
> int ret;
>
> ret = socket(srv->protocol.domain, srv->protocol.type | SOCK_CLOEXEC,
> - 0);
> + srv->protocol.protocol);
> if (ret < 0)
> return -errno;
> return ret;
> @@ -124,7 +153,7 @@ static socklen_t get_addrlen(const struct service_fixture *const srv,
> return sizeof(srv->ipv4_addr);
>
> case AF_INET6:
> - if (minimal)
> + if (minimal && !prot_is_sctp(&srv->protocol))
> return SIN6_LEN_RFC2133;
> return sizeof(srv->ipv6_addr);
>
> @@ -271,6 +300,11 @@ FIXTURE_SETUP(protocol)
> .type = SOCK_STREAM,
> };
>
> +#if !SMC_SUPPORTED
> + if (prot_is_smc(&variant->prot))
> + SKIP(return, "SMC protocol is not supported.");
> +#endif
> +
> disable_caps(_metadata);
>
> ASSERT_EQ(0, set_service(&self->srv0, variant->prot, 0));
> @@ -299,6 +333,39 @@ FIXTURE_VARIANT_ADD(protocol, no_sandbox_with_ipv4_tcp) {
> },
> };
>
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(protocol, no_sandbox_with_ipv4_mptcp) {
> + /* clang-format on */
> + .sandbox = NO_SANDBOX,
> + .prot = {
> + .domain = AF_INET,
> + .type = SOCK_STREAM,
> + .protocol = IPPROTO_MPTCP,
> + },
> +};
> +
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(protocol, no_sandbox_with_ipv4_sctp) {
> + /* clang-format on */
> + .sandbox = NO_SANDBOX,
> + .prot = {
> + .domain = AF_INET,
> + .type = SOCK_STREAM,
> + .protocol = IPPROTO_SCTP,
> + },
> +};
> +
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(protocol, no_sandbox_with_ipv4_smc) {
> + /* clang-format on */
> + .sandbox = NO_SANDBOX,
> + .prot = {
> + .domain = AF_INET,
> + .type = SOCK_STREAM,
> + .protocol = IPPROTO_SMC,
> + },
> +};
> +
> /* clang-format off */
> FIXTURE_VARIANT_ADD(protocol, no_sandbox_with_ipv6_tcp) {
> /* clang-format on */
> @@ -309,6 +376,39 @@ FIXTURE_VARIANT_ADD(protocol, no_sandbox_with_ipv6_tcp) {
> },
> };
>
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(protocol, no_sandbox_with_ipv6_mptcp) {
> + /* clang-format on */
> + .sandbox = NO_SANDBOX,
> + .prot = {
> + .domain = AF_INET6,
> + .type = SOCK_STREAM,
> + .protocol = IPPROTO_MPTCP,
> + },
> +};
> +
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(protocol, no_sandbox_with_ipv6_sctp) {
> + /* clang-format on */
> + .sandbox = NO_SANDBOX,
> + .prot = {
> + .domain = AF_INET6,
> + .type = SOCK_STREAM,
> + .protocol = IPPROTO_SCTP,
> + },
> +};
> +
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(protocol, no_sandbox_with_ipv6_smc) {
> + /* clang-format on */
> + .sandbox = NO_SANDBOX,
> + .prot = {
> + .domain = AF_INET6,
> + .type = SOCK_STREAM,
> + .protocol = IPPROTO_SMC,
> + },
> +};
> +
> /* clang-format off */
> FIXTURE_VARIANT_ADD(protocol, no_sandbox_with_ipv4_udp) {
> /* clang-format on */
> @@ -359,6 +459,39 @@ FIXTURE_VARIANT_ADD(protocol, tcp_sandbox_with_ipv4_tcp) {
> },
> };
>
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(protocol, tcp_sandbox_with_ipv4_mptcp) {
> + /* clang-format on */
> + .sandbox = TCP_SANDBOX,
> + .prot = {
> + .domain = AF_INET,
> + .type = SOCK_STREAM,
> + .protocol = IPPROTO_MPTCP,
> + },
> +};
> +
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(protocol, tcp_sandbox_with_ipv4_sctp) {
> + /* clang-format on */
> + .sandbox = TCP_SANDBOX,
> + .prot = {
> + .domain = AF_INET,
> + .type = SOCK_STREAM,
> + .protocol = IPPROTO_SCTP,
> + },
> +};
> +
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(protocol, tcp_sandbox_with_ipv4_smc) {
> + /* clang-format on */
> + .sandbox = TCP_SANDBOX,
> + .prot = {
> + .domain = AF_INET,
> + .type = SOCK_STREAM,
> + .protocol = IPPROTO_SMC,
> + },
> +};
> +
> /* clang-format off */
> FIXTURE_VARIANT_ADD(protocol, tcp_sandbox_with_ipv6_tcp) {
> /* clang-format on */
> @@ -369,6 +502,39 @@ FIXTURE_VARIANT_ADD(protocol, tcp_sandbox_with_ipv6_tcp) {
> },
> };
>
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(protocol, tcp_sandbox_with_ipv6_mptcp) {
> + /* clang-format on */
> + .sandbox = TCP_SANDBOX,
> + .prot = {
> + .domain = AF_INET6,
> + .type = SOCK_STREAM,
> + .protocol = IPPROTO_MPTCP,
> + },
> +};
> +
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(protocol, tcp_sandbox_with_ipv6_sctp) {
> + /* clang-format on */
> + .sandbox = TCP_SANDBOX,
> + .prot = {
> + .domain = AF_INET6,
> + .type = SOCK_STREAM,
> + .protocol = IPPROTO_SCTP,
> + },
> +};
> +
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(protocol, tcp_sandbox_with_ipv6_smc) {
> + /* clang-format on */
> + .sandbox = TCP_SANDBOX,
> + .prot = {
> + .domain = AF_INET6,
> + .type = SOCK_STREAM,
> + .protocol = IPPROTO_SMC,
> + },
> +};
> +
> /* clang-format off */
> FIXTURE_VARIANT_ADD(protocol, tcp_sandbox_with_ipv4_udp) {
> /* clang-format on */
> @@ -663,7 +829,7 @@ TEST_F(protocol, bind_unspec)
>
> /* Allowed bind on AF_UNSPEC/INADDR_ANY. */
> ret = bind_variant(bind_fd, &self->unspec_any0);
> - if (variant->prot.domain == AF_INET) {
> + if (variant->prot.domain == AF_INET && !prot_is_sctp(&variant->prot)) {
> EXPECT_EQ(0, ret)
> {
> TH_LOG("Failed to bind to unspec/any socket: %s",
> @@ -689,7 +855,7 @@ TEST_F(protocol, bind_unspec)
>
> /* Denied bind on AF_UNSPEC/INADDR_ANY. */
> ret = bind_variant(bind_fd, &self->unspec_any0);
> - if (variant->prot.domain == AF_INET) {
> + if (variant->prot.domain == AF_INET && !prot_is_sctp(&variant->prot)) {
> if (is_restricted(&variant->prot, variant->sandbox)) {
> EXPECT_EQ(-EACCES, ret);
> } else {
> @@ -727,6 +893,10 @@ TEST_F(protocol, connect_unspec)
> int bind_fd, client_fd, status;
> pid_t child;
>
> + if (prot_is_smc(&variant->prot))
> + SKIP(return, "SMC does not properly handles disconnect "
> + "in the case of fallback to TCP");
> +
> /* Specific connection tests. */
> bind_fd = socket_variant(&self->srv0);
> ASSERT_LE(0, bind_fd);
> @@ -769,17 +939,18 @@ TEST_F(protocol, connect_unspec)
>
> /* Disconnects already connected socket, or set peer. */
> ret = connect_variant(connect_fd, &self->unspec_any0);
> - if (self->srv0.protocol.domain == AF_UNIX &&
> - self->srv0.protocol.type == SOCK_STREAM) {
> + if (prot_is_unix_stream(&variant->prot)) {
> EXPECT_EQ(-EINVAL, ret);
> + } else if (prot_is_sctp(&variant->prot)) {
> + EXPECT_EQ(-EOPNOTSUPP, ret);
> } else {
> EXPECT_EQ(0, ret);
> }
>
> /* Tries to reconnect, or set peer. */
> ret = connect_variant(connect_fd, &self->srv0);
> - if (self->srv0.protocol.domain == AF_UNIX &&
> - self->srv0.protocol.type == SOCK_STREAM) {
> + if (prot_is_unix_stream(&variant->prot) ||
> + prot_is_sctp(&variant->prot)) {
> EXPECT_EQ(-EISCONN, ret);
> } else {
> EXPECT_EQ(0, ret);
> @@ -796,9 +967,10 @@ TEST_F(protocol, connect_unspec)
> }
>
> ret = connect_variant(connect_fd, &self->unspec_any0);
> - if (self->srv0.protocol.domain == AF_UNIX &&
> - self->srv0.protocol.type == SOCK_STREAM) {
> + if (prot_is_unix_stream(&variant->prot)) {
> EXPECT_EQ(-EINVAL, ret);
> + } else if (prot_is_sctp(&variant->prot)) {
> + EXPECT_EQ(-EOPNOTSUPP, ret);
> } else {
> /* Always allowed to disconnect. */
> EXPECT_EQ(0, ret);
> --
> 2.34.1
>
Looks good.
Reviewed-by: Günther Noack <gnoack@google.com>
next prev parent reply other threads:[~2024-10-03 15:59 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-10-03 14:39 [RFC PATCH v1 0/2] Fix non-TCP sockets restriction Mikhail Ivanov
2024-10-03 14:39 ` [RFC PATCH v1 1/2] landlock: " Mikhail Ivanov
2024-10-03 15:57 ` Günther Noack
2024-10-03 17:45 ` Mickaël Salaün
2024-10-03 21:30 ` Mikhail Ivanov
2024-10-04 10:13 ` Mickaël Salaün
2024-10-04 18:16 ` Mikhail Ivanov
2024-10-05 15:49 ` Mickaël Salaün
2024-10-05 15:55 ` Mickaël Salaün
2024-10-07 11:06 ` Mikhail Ivanov
2024-10-07 11:58 ` Mikhail Ivanov
2024-10-07 13:35 ` Mickaël Salaün
2024-10-03 21:48 ` Mikhail Ivanov
2024-10-03 14:39 ` [RFC PATCH v1 2/2] selftests/landlock: Test non-TCP INET connection-based protocols Mikhail Ivanov
2024-10-03 15:59 ` Günther Noack [this message]
2024-10-03 17:45 ` Mickaël Salaün
2024-10-03 21:22 ` Mikhail Ivanov
2024-10-04 10:14 ` Mickaël Salaün
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Zv6_Uitud0OzxKTn@google.com \
--to=gnoack@google.com \
--cc=artem.kuzin@huawei.com \
--cc=ivanov.mikhail1@huawei-partners.com \
--cc=konstantin.meskhidze@huawei.com \
--cc=linux-security-module@vger.kernel.org \
--cc=mic@digikod.net \
--cc=netdev@vger.kernel.org \
--cc=netfilter-devel@vger.kernel.org \
--cc=willemdebruijn.kernel@gmail.com \
--cc=yusongping@huawei.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.