[RFC PATCH v3 15/19] selftests/landlock: Test SCTP peeloff restriction

Mikhail Ivanov ivanov.mikhail1 at huawei-partners.com
Thu Oct 3 12:15:25 UTC 2024


On 9/27/2024 5:35 PM, Günther Noack wrote:
> 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 at huawei-partners.com>
>> ---
>>   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?

Ofc, thank you

> 
> 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 at google.com>



More information about the Linux-security-module-archive mailing list