lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Wed, 12 Jul 2023 09:02:17 +0200
From: Mickaël Salaün <mic@...ikod.net>
To: Konstantin Meskhidze <konstantin.meskhidze@...wei.com>
Cc: artem.kuzin@...wei.com, gnoack3000@...il.com,
 willemdebruijn.kernel@...il.com, yusongping@...wei.com,
 linux-security-module@...r.kernel.org, netdev@...r.kernel.org,
 netfilter-devel@...r.kernel.org
Subject: Re: [PATCH v11.1] selftests/landlock: Add 11 new test suites
 dedicated to network


On 06/07/2023 16:55, Mickaël Salaün wrote:
> From: Konstantin Meskhidze <konstantin.meskhidze@...wei.com>
> 
> This patch is a revamp of the v11 tests [1] with new tests (see the
> "Changes since v11" description).  I (Mickaël) only added the following
> todo list and the "Changes since v11" sections in this commit message.
> I think this patch is good but it would appreciate reviews.
> You can find the diff of my changes here but it is not really readable:
> https://git.kernel.org/mic/c/78edf722fba5 (landlock-net-v11 branch)
> [1] https://lore.kernel.org/all/20230515161339.631577-11-konstantin.meskhidze@huawei.com/
> TODO:
> - Rename all "net_service" to "net_port".
> - Fix the two kernel bugs found with the new tests.
> - Update this commit message with a small description of all tests.

[...]

> +FIXTURE_SETUP(ipv4)
> +{
> +	const struct protocol_variant prot = {
> +		.domain = AF_INET,
> +		.type = variant->type,
> +	};
> +
> +	disable_caps(_metadata);
> +
> +	set_service(&self->srv0, prot, 0);
> +	set_service(&self->srv1, prot, 1);
> +
> +	setup_loopback(_metadata);
> +};
> +
> +FIXTURE_TEARDOWN(ipv4)
> +{
> +}
> +
> +// Kernel FIXME: tcp_sandbox_with_tcp and tcp_sandbox_with_udp
> +TEST_F(ipv4, from_unix_to_inet)
> +{
> +	int unix_stream_fd, unix_dgram_fd;
> +
> +	if (variant->sandbox == TCP_SANDBOX) {
> +		const struct landlock_ruleset_attr ruleset_attr = {
> +			.handled_access_net = LANDLOCK_ACCESS_NET_BIND_TCP |
> +					      LANDLOCK_ACCESS_NET_CONNECT_TCP,
> +		};
> +		const struct landlock_net_service_attr tcp_bind_connect_p0 = {
> +			.allowed_access = LANDLOCK_ACCESS_NET_BIND_TCP |
> +					  LANDLOCK_ACCESS_NET_CONNECT_TCP,
> +			.port = self->srv0.port,
> +		};
> +		int ruleset_fd;
> +
> +		/* Denies connect and bind to check errno value. */
> +		ruleset_fd = landlock_create_ruleset(&ruleset_attr,
> +						     sizeof(ruleset_attr), 0);
> +		ASSERT_LE(0, ruleset_fd);
> +
> +		/* Allows connect and bind for srv0.  */
> +		ASSERT_EQ(0, landlock_add_rule(ruleset_fd,
> +					       LANDLOCK_RULE_NET_SERVICE,
> +					       &tcp_bind_connect_p0, 0));
> +
> +		enforce_ruleset(_metadata, ruleset_fd);
> +		EXPECT_EQ(0, close(ruleset_fd));
> +	}
> +
> +	unix_stream_fd = socket(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0);
> +	ASSERT_LE(0, unix_stream_fd);
> +
> +	unix_dgram_fd = socket(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0);
> +	ASSERT_LE(0, unix_dgram_fd);
> +
> +	/* Checks unix stream bind and connect for srv0. */
> +	EXPECT_EQ(-EINVAL, bind_variant(unix_stream_fd, &self->srv0));
> +	EXPECT_EQ(-EINVAL, connect_variant(unix_stream_fd, &self->srv0));
> +
> +	/* Checks unix stream bind and connect for srv1. */
> +	EXPECT_EQ(-EINVAL, bind_variant(unix_stream_fd, &self->srv1))
> +	{
> +		TH_LOG("Wrong bind error: %s", strerror(errno));
> +	}
> +	EXPECT_EQ(-EINVAL, connect_variant(unix_stream_fd, &self->srv1));
> +
> +	/* Checks unix datagram bind and connect for srv0. */
> +	EXPECT_EQ(-EINVAL, bind_variant(unix_dgram_fd, &self->srv0));
> +	EXPECT_EQ(-EINVAL, connect_variant(unix_dgram_fd, &self->srv0));
> +
> +	/* Checks unix datagram bind and connect for srv0. */
> +	EXPECT_EQ(-EINVAL, bind_variant(unix_dgram_fd, &self->srv1));
> +	EXPECT_EQ(-EINVAL, connect_variant(unix_dgram_fd, &self->srv1));
> +}

We should also add a test to make sure errno is the same with and 
without sandboxing when using port 0 for connect and consistent with 
bind (using an available port). The test fixture and variants should be 
quite similar to the "ipv4" ones, but we can also add AF_INET6 variants, 
which will result in 8 "ip" variants:

TEST_F(ip, port_zero)
{
	if (variant->sandbox == TCP_SANDBOX) {
		/* Denies any connect and bind. */
	}
	/* Checks errno for port 0. */
}

[...]

> +FIXTURE(inet)
> +{
> +	struct service_fixture srv0, srv1;
> +};

The "inet" variants are useless and should be removed. The "inet" 
fixture can then be renamed to "ipv4_tcp".


> +
> +FIXTURE_VARIANT(inet)
> +{
> +	const bool is_sandboxed;
> +	const struct protocol_variant prot;
> +};
> +
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(inet, no_sandbox_with_ipv4) {
> +	/* clang-format on */
> +	.is_sandboxed = false,
> +	.prot = {
> +		.domain = AF_INET,
> +		.type = SOCK_STREAM,
> +	},
> +};
> +
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(inet, sandbox_with_ipv4) {
> +	/* clang-format on */
> +	.is_sandboxed = true,
> +	.prot = {
> +		.domain = AF_INET,
> +		.type = SOCK_STREAM,
> +	},
> +};
> +
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(inet, no_sandbox_with_ipv6) {
> +	/* clang-format on */
> +	.is_sandboxed = false,
> +	.prot = {
> +		.domain = AF_INET6,
> +		.type = SOCK_STREAM,
> +	},
> +};
> +
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(inet, sandbox_with_ipv6) {
> +	/* clang-format on */
> +	.is_sandboxed = true,
> +	.prot = {
> +		.domain = AF_INET6,
> +		.type = SOCK_STREAM,
> +	},
> +};
> +
> +FIXTURE_SETUP(inet)
> +{
> +	const struct protocol_variant ipv4_tcp = {
> +		.domain = AF_INET,
> +		.type = SOCK_STREAM,
> +	};
> +
> +	disable_caps(_metadata);
> +
> +	ASSERT_EQ(0, set_service(&self->srv0, ipv4_tcp, 0));
> +	ASSERT_EQ(0, set_service(&self->srv1, ipv4_tcp, 1));
> +
> +	setup_loopback(_metadata);
> +};
> +
> +FIXTURE_TEARDOWN(inet)
> +{
> +}
> +
> +TEST_F(inet, port_endianness)
> +{
> +	const struct landlock_ruleset_attr ruleset_attr = {
> +		.handled_access_net = LANDLOCK_ACCESS_NET_BIND_TCP |
> +				      LANDLOCK_ACCESS_NET_CONNECT_TCP,
> +	};
> +	const struct landlock_net_service_attr bind_host_endian_p0 = {
> +		.allowed_access = LANDLOCK_ACCESS_NET_BIND_TCP,
> +		/* Host port format. */
> +		.port = self->srv0.port,
> +	};
> +	const struct landlock_net_service_attr connect_big_endian_p0 = {
> +		.allowed_access = LANDLOCK_ACCESS_NET_CONNECT_TCP,
> +		/* Big endian port format. */
> +		.port = htons(self->srv0.port),
> +	};
> +	const struct landlock_net_service_attr bind_connect_host_endian_p1 = {
> +		.allowed_access = LANDLOCK_ACCESS_NET_BIND_TCP |
> +				  LANDLOCK_ACCESS_NET_CONNECT_TCP,
> +		/* Host port format. */
> +		.port = self->srv1.port,
> +	};
> +	const unsigned int one = 1;
> +	const char little_endian = *(const char *)&one;
> +	int ruleset_fd;
> +
> +	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_NET_SERVICE,
> +				       &bind_host_endian_p0, 0));
> +	ASSERT_EQ(0, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_NET_SERVICE,
> +				       &connect_big_endian_p0, 0));
> +	ASSERT_EQ(0, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_NET_SERVICE,
> +				       &bind_connect_host_endian_p1, 0));
> +	enforce_ruleset(_metadata, ruleset_fd);
> +
> +	/* No restriction for big endinan CPU. */
> +	test_bind_and_connect(_metadata, &self->srv0, false, little_endian);
> +
> +	/* No restriction for any CPU. */
> +	test_bind_and_connect(_metadata, &self->srv1, false, false);
> +}
> +
> +TEST_HARNESS_MAIN

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ