diff mbox series

[RFC,v3,12/19] selftests/landlock: Test that kernel space sockets are not restricted

Message ID 20240904104824.1844082-13-ivanov.mikhail1@huawei-partners.com (mailing list archive)
State RFC
Headers show
Series Support socket access-control | expand

Checks

Context Check Description
netdev/tree_selection success Not a local patch, async

Commit Message

Mikhail Ivanov Sept. 4, 2024, 10:48 a.m. UTC
Add test validating that Landlock provides restriction of user space
sockets only.

Signed-off-by: Mikhail Ivanov <ivanov.mikhail1@huawei-partners.com>
---
 .../testing/selftests/landlock/socket_test.c  | 39 ++++++++++++++++++-
 1 file changed, 38 insertions(+), 1 deletion(-)

Comments

Mikhail Ivanov Sept. 4, 2024, 12:45 p.m. UTC | #1
9/4/2024 1:48 PM, Mikhail Ivanov wrote:
> Add test validating that Landlock provides restriction of user space
> sockets only.
> 
> Signed-off-by: Mikhail Ivanov <ivanov.mikhail1@huawei-partners.com>
> ---
>   .../testing/selftests/landlock/socket_test.c  | 39 ++++++++++++++++++-
>   1 file changed, 38 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/landlock/socket_test.c b/tools/testing/selftests/landlock/socket_test.c
> index ff5ace711697..23698b8c2f4d 100644
> --- a/tools/testing/selftests/landlock/socket_test.c
> +++ b/tools/testing/selftests/landlock/socket_test.c
> @@ -7,7 +7,7 @@
>   
>   #define _GNU_SOURCE
>   
> -#include <linux/landlock.h>
> +#include "landlock.h"

typo, will be fixed

>   #include <linux/pfkeyv2.h>
>   #include <linux/kcm.h>
>   #include <linux/can.h>
> @@ -628,4 +628,41 @@ TEST(unsupported_af_and_prot)
>   	EXPECT_EQ(ESOCKTNOSUPPORT, test_socket(AF_UNIX, SOCK_PACKET, 0));
>   }
>   
> +TEST(kernel_socket)
> +{
> +	const struct landlock_ruleset_attr ruleset_attr = {
> +		.handled_access_socket = LANDLOCK_ACCESS_SOCKET_CREATE,
> +	};
> +	struct landlock_socket_attr smc_socket_create = {
> +		.allowed_access = LANDLOCK_ACCESS_SOCKET_CREATE,
> +		.family = AF_SMC,
> +		.type = SOCK_STREAM,
> +	};
> +	int ruleset_fd;
> +
> +	/*
> +	 * Checks that SMC socket is created sucessfuly without
> +	 * landlock restrictions.
> +	 */
> +	ASSERT_EQ(0, test_socket(AF_SMC, SOCK_STREAM, 0));
> +
> +	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_SOCKET,
> +				       &smc_socket_create, 0));
> +	enforce_ruleset(_metadata, ruleset_fd);
> +	ASSERT_EQ(0, close(ruleset_fd));
> +
> +	/*
> +	 * During the creation of an SMC socket, an internal service TCP socket
> +	 * is also created (Cf. smc_create_clcsk).
> +	 *
> +	 * Checks that Landlock does not restrict creation of the kernel space
> +	 * socket.
> +	 */
> +	EXPECT_EQ(0, test_socket(AF_SMC, SOCK_STREAM, 0));
> +}
> +
>   TEST_HARNESS_MAIN
Günther Noack Sept. 18, 2024, 1 p.m. UTC | #2
This is a good way to test this, IMHO. Good find.
The comment at the bottom is really valuable. :)

Out of curiosity: I suspect that a selftest with NFS or another network-backed
filesystem might be too complicated?  Have you tried that manually, by any
chance?


On Wed, Sep 04, 2024 at 06:48:17PM +0800, Mikhail Ivanov wrote:
> Add test validating that Landlock provides restriction of user space
> sockets only.
> 
> Signed-off-by: Mikhail Ivanov <ivanov.mikhail1@huawei-partners.com>
> ---
>  .../testing/selftests/landlock/socket_test.c  | 39 ++++++++++++++++++-
>  1 file changed, 38 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/landlock/socket_test.c b/tools/testing/selftests/landlock/socket_test.c
> index ff5ace711697..23698b8c2f4d 100644
> --- a/tools/testing/selftests/landlock/socket_test.c
> +++ b/tools/testing/selftests/landlock/socket_test.c
> @@ -7,7 +7,7 @@
>  
>  #define _GNU_SOURCE
>  
> -#include <linux/landlock.h>
> +#include "landlock.h"
>  #include <linux/pfkeyv2.h>
>  #include <linux/kcm.h>
>  #include <linux/can.h>
> @@ -628,4 +628,41 @@ TEST(unsupported_af_and_prot)
>  	EXPECT_EQ(ESOCKTNOSUPPORT, test_socket(AF_UNIX, SOCK_PACKET, 0));
>  }
>  
> +TEST(kernel_socket)
> +{
> +	const struct landlock_ruleset_attr ruleset_attr = {
> +		.handled_access_socket = LANDLOCK_ACCESS_SOCKET_CREATE,
> +	};
> +	struct landlock_socket_attr smc_socket_create = {
> +		.allowed_access = LANDLOCK_ACCESS_SOCKET_CREATE,
> +		.family = AF_SMC,
> +		.type = SOCK_STREAM,
> +	};
> +	int ruleset_fd;
> +
> +	/*
> +	 * Checks that SMC socket is created sucessfuly without

Typo nit: "successfully"
             ^^     ^^

> +	 * landlock restrictions.
> +	 */
> +	ASSERT_EQ(0, test_socket(AF_SMC, SOCK_STREAM, 0));
> +
> +	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_SOCKET,
> +				       &smc_socket_create, 0));
> +	enforce_ruleset(_metadata, ruleset_fd);
> +	ASSERT_EQ(0, close(ruleset_fd));
> +
> +	/*
> +	 * During the creation of an SMC socket, an internal service TCP socket
> +	 * is also created (Cf. smc_create_clcsk).
> +	 *
> +	 * Checks that Landlock does not restrict creation of the kernel space
> +	 * socket.
> +	 */
> +	EXPECT_EQ(0, test_socket(AF_SMC, SOCK_STREAM, 0));
> +}
> +
>  TEST_HARNESS_MAIN
> -- 
> 2.34.1
> 

Reviewed-by: Günther Noack <gnoack@google.com>
Mikhail Ivanov Sept. 19, 2024, 10:53 a.m. UTC | #3
On 9/18/2024 4:00 PM, Günther Noack wrote:
> This is a good way to test this, IMHO. Good find.
> The comment at the bottom is really valuable. :)
> 
> Out of curiosity: I suspect that a selftest with NFS or another network-backed
> filesystem might be too complicated?  Have you tried that manually, by any
> chance?

I haven't, just ran through a code a little bit. I think that testing
NFS is possible, but it depends on which scenario we want to test.
Simple creation of a server may require only to mount NFS with the
appropriate parameters. Anyway, I don't see any reason for implementing
such test if restriction of kernel sockets is already tested with SMC
socket creation.

> 
> 
> On Wed, Sep 04, 2024 at 06:48:17PM +0800, Mikhail Ivanov wrote:
>> Add test validating that Landlock provides restriction of user space
>> sockets only.
>>
>> Signed-off-by: Mikhail Ivanov <ivanov.mikhail1@huawei-partners.com>
>> ---
>>   .../testing/selftests/landlock/socket_test.c  | 39 ++++++++++++++++++-
>>   1 file changed, 38 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/testing/selftests/landlock/socket_test.c b/tools/testing/selftests/landlock/socket_test.c
>> index ff5ace711697..23698b8c2f4d 100644
>> --- a/tools/testing/selftests/landlock/socket_test.c
>> +++ b/tools/testing/selftests/landlock/socket_test.c
>> @@ -7,7 +7,7 @@
>>   
>>   #define _GNU_SOURCE
>>   
>> -#include <linux/landlock.h>
>> +#include "landlock.h"
>>   #include <linux/pfkeyv2.h>
>>   #include <linux/kcm.h>
>>   #include <linux/can.h>
>> @@ -628,4 +628,41 @@ TEST(unsupported_af_and_prot)
>>   	EXPECT_EQ(ESOCKTNOSUPPORT, test_socket(AF_UNIX, SOCK_PACKET, 0));
>>   }
>>   
>> +TEST(kernel_socket)
>> +{
>> +	const struct landlock_ruleset_attr ruleset_attr = {
>> +		.handled_access_socket = LANDLOCK_ACCESS_SOCKET_CREATE,
>> +	};
>> +	struct landlock_socket_attr smc_socket_create = {
>> +		.allowed_access = LANDLOCK_ACCESS_SOCKET_CREATE,
>> +		.family = AF_SMC,
>> +		.type = SOCK_STREAM,
>> +	};
>> +	int ruleset_fd;
>> +
>> +	/*
>> +	 * Checks that SMC socket is created sucessfuly without
> 
> Typo nit: "successfully"
>               ^^     ^^

Thanks, will be fixed.

> 
>> +	 * landlock restrictions.
>> +	 */
>> +	ASSERT_EQ(0, test_socket(AF_SMC, SOCK_STREAM, 0));
>> +
>> +	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_SOCKET,
>> +				       &smc_socket_create, 0));
>> +	enforce_ruleset(_metadata, ruleset_fd);
>> +	ASSERT_EQ(0, close(ruleset_fd));
>> +
>> +	/*
>> +	 * During the creation of an SMC socket, an internal service TCP socket
>> +	 * is also created (Cf. smc_create_clcsk).
>> +	 *
>> +	 * Checks that Landlock does not restrict creation of the kernel space
>> +	 * socket.
>> +	 */
>> +	EXPECT_EQ(0, test_socket(AF_SMC, SOCK_STREAM, 0));
>> +}
>> +
>>   TEST_HARNESS_MAIN
>> -- 
>> 2.34.1
>>
> 
> Reviewed-by: Günther Noack <gnoack@google.com>
diff mbox series

Patch

diff --git a/tools/testing/selftests/landlock/socket_test.c b/tools/testing/selftests/landlock/socket_test.c
index ff5ace711697..23698b8c2f4d 100644
--- a/tools/testing/selftests/landlock/socket_test.c
+++ b/tools/testing/selftests/landlock/socket_test.c
@@ -7,7 +7,7 @@ 
 
 #define _GNU_SOURCE
 
-#include <linux/landlock.h>
+#include "landlock.h"
 #include <linux/pfkeyv2.h>
 #include <linux/kcm.h>
 #include <linux/can.h>
@@ -628,4 +628,41 @@  TEST(unsupported_af_and_prot)
 	EXPECT_EQ(ESOCKTNOSUPPORT, test_socket(AF_UNIX, SOCK_PACKET, 0));
 }
 
+TEST(kernel_socket)
+{
+	const struct landlock_ruleset_attr ruleset_attr = {
+		.handled_access_socket = LANDLOCK_ACCESS_SOCKET_CREATE,
+	};
+	struct landlock_socket_attr smc_socket_create = {
+		.allowed_access = LANDLOCK_ACCESS_SOCKET_CREATE,
+		.family = AF_SMC,
+		.type = SOCK_STREAM,
+	};
+	int ruleset_fd;
+
+	/*
+	 * Checks that SMC socket is created sucessfuly without
+	 * landlock restrictions.
+	 */
+	ASSERT_EQ(0, test_socket(AF_SMC, SOCK_STREAM, 0));
+
+	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_SOCKET,
+				       &smc_socket_create, 0));
+	enforce_ruleset(_metadata, ruleset_fd);
+	ASSERT_EQ(0, close(ruleset_fd));
+
+	/*
+	 * During the creation of an SMC socket, an internal service TCP socket
+	 * is also created (Cf. smc_create_clcsk).
+	 *
+	 * Checks that Landlock does not restrict creation of the kernel space
+	 * socket.
+	 */
+	EXPECT_EQ(0, test_socket(AF_SMC, SOCK_STREAM, 0));
+}
+
 TEST_HARNESS_MAIN