2/1/2022 9:31 PM, Mickaël Salaün пишет:
On 24/01/2022 09:02, Konstantin Meskhidze wrote:Support 4 tests for bind and connect networks actions:Good to see such tests!1. bind() a socket with no landlock restrictions. 2. bind() sockets with landllock restrictions.You can leverage the FIXTURE_VARIANT helpers to factor out this kind of tests (see ptrace_test.c).3. connect() a socket to listening one with no landlock restricitons. 4. connect() sockets with landlock restrictions.Same here, you can factor out code. I guess you could create helpers for client and server parts.We also need to test with IPv4, IPv6 and the AF_UNSPEC tricks.Please provide the kernel test coverage and explain why the uncovered code cannot be covered: https://www.kernel.org/doc/html/latest/dev-tools/gcov.html
Hi Mickaёl! Could you please provide the example of your test coverage build process? Cause as I undersatand there is no need to get coverage data for the entire kernel, just for landlock files.
You'll probably see that there are a multiple parts of the kernel that are not covered. For instance, it is important to test different combinations of layered network rules (see layout1/ruleset_overlap, layer_rule_unions, non_overlapping_accesses, interleaved_masked_accesses… in fs_test.c). Tests in fs_test.c are more complex because handling file system rules is more complex, but you can get some inspiration in it, especially the edge cases.We also need to test invalid user space supplied data (see layout1/inval test in fs_test.c).Signed-off-by: Konstantin Meskhidze <konstantin.meskhidze@xxxxxxxxxx> --- .../testing/selftests/landlock/network_test.c | 346 ++++++++++++++++++ 1 file changed, 346 insertions(+) create mode 100644 tools/testing/selftests/landlock/network_test.cdiff --git a/tools/testing/selftests/landlock/network_test.c b/tools/testing/selftests/landlock/network_test.cnew file mode 100644 index 000000000000..9dfe37a2fb20 --- /dev/null +++ b/tools/testing/selftests/landlock/network_test.c @@ -0,0 +1,346 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Landlock tests - Common user space base + * + * Copyright © 2017-2020 Mickaël Salaün <mic@xxxxxxxxxxx> + * Copyright © 2019-2020 ANSSIYou need to update this header with an appropriate description and the copyright holder (your employer).+ */ + +#define _GNU_SOURCE +#include <errno.h> +#include <fcntl.h> +#include <linux/landlock.h> +#include <string.h> +#include <sys/prctl.h> +#include <sys/socket.h> +#include <sys/types.h> +#include <netinet/in.h> +#include <arpa/inet.h>To make it determinisitic (and ease patching/diff/merging), you should sort all the included files (in tests and in the kernel code).+ +#include "common.h" + +#define SOCK_PORT_1 3470 +#define SOCK_PORT_2 3480 +#define SOCK_PORT_3 3490To avoid port collision and create a clean and stable test environement (to avoid flaky tests), you should create a network namespace with FIXTURE_SETUP, test with TEST_F_FORK (to not polute the parent process, and which works with test variants), and use the set_cap and clear_cap helpers (see fs_test.c).+ +#define IP_ADDRESS "127.0.0.1" + +/* Number pending connections queue tobe hold */ +#define BACKLOG 10 + +TEST(socket_bind_no_restrictions) { + + int sockfd; + struct sockaddr_in addr; + const int one = 1; + + /* Create a socket */ + sockfd = socket(AF_INET, SOCK_STREAM, 0); + ASSERT_LE(0, sockfd); + /* Allow reuse of local addresses */+ ASSERT_EQ(0, setsockopt(sockfd, SOL_SOCKET, SO_REUSEADDR, &one, sizeof(one)));With a dedicated namespace, SO_REUSEADDR should not be required.+ + /* Set socket address parameters */ + addr.sin_family = AF_INET; + addr.sin_port = htons(SOCK_PORT_1); + addr.sin_addr.s_addr = inet_addr(IP_ADDRESS); + memset(&(addr.sin_zero), '\0', 8); + + /* Bind the socket to IP address */ + ASSERT_EQ(0, bind(sockfd, (struct sockaddr *)&addr, sizeof(addr))); +} + +TEST(sockets_bind_with_restrictions) { + + int sockfd_1, sockfd_2, sockfd_3; + struct sockaddr_in addr_1, addr_2, addr_3; + const int one = 1; + + struct landlock_ruleset_attr ruleset_attr = { + .handled_access_net = LANDLOCK_ACCESS_NET_BIND_TCP | + LANDLOCK_ACCESS_NET_CONNECT_TCP, + }; + struct landlock_net_service_attr net_service_1 = { + .allowed_access = LANDLOCK_ACCESS_NET_BIND_TCP | + LANDLOCK_ACCESS_NET_CONNECT_TCP, + .port = SOCK_PORT_1, + }; + struct landlock_net_service_attr net_service_2 = { + .allowed_access = LANDLOCK_ACCESS_NET_CONNECT_TCP, + .port = SOCK_PORT_2, + }; + struct landlock_net_service_attr net_service_3 = { + .allowed_access = 0, + .port = SOCK_PORT_3, + };Good to have these three different rules!+ + const int ruleset_fd = landlock_create_ruleset(&ruleset_attr, + sizeof(ruleset_attr), 0); + ASSERT_LE(0, ruleset_fd); ++ /* Allow connect and bind operations to the SOCK_PORT_1 socket "object" */You can omit "object" but use full sentences at the third person (because it explains what do the next lines).+ ASSERT_EQ(0, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_NET_SERVICE,+ &net_service_1, 0));+ /* Allow connect and deny bind operations to the SOCK_PORT_2 socket "object" */ + ASSERT_EQ(0, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_NET_SERVICE,+ &net_service_2, 0));+ /* Empty allowed_access (i.e. deny rules) are ignored in network actions+ * for SOCK_PORT_3 socket "object" + */+ ASSERT_EQ(-1, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_NET_SERVICE,+ &net_service_3, 0)); + ASSERT_EQ(ENOMSG, errno); + + /* Enforces the ruleset. */ + ASSERT_EQ(0, prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0)); + ASSERT_EQ(0, landlock_restrict_self(ruleset_fd, 0)); + ASSERT_EQ(0, close(ruleset_fd)); + + /* Create a socket 1 */ + sockfd_1 = socket(AF_INET, SOCK_STREAM, 0);Please create all FD with SOCK_CLOEXEC and also close them when not needed. This could also reduce the number of FD.+ ASSERT_LE(0, sockfd_1); + /* Allow reuse of local addresses */+ ASSERT_EQ(0, setsockopt(sockfd_1, SOL_SOCKET, SO_REUSEADDR, &one, sizeof(one)));+ + /* Set socket 1 address parameters */ + addr_1.sin_family = AF_INET; + addr_1.sin_port = htons(SOCK_PORT_1); + addr_1.sin_addr.s_addr = inet_addr(IP_ADDRESS); + memset(&(addr_1.sin_zero), '\0', 8); + /* Bind the socket 1 to IP address */+ ASSERT_EQ(0, bind(sockfd_1, (struct sockaddr *)&addr_1, sizeof(addr_1)));+ + /* Create a socket 2 */ + sockfd_2 = socket(AF_INET, SOCK_STREAM, 0); + ASSERT_LE(0, sockfd_2); + /* Allow reuse of local addresses */+ ASSERT_EQ(0, setsockopt(sockfd_2, SOL_SOCKET, SO_REUSEADDR, &one, sizeof(one)));+ + /* Set socket 2 address parameters */ + addr_2.sin_family = AF_INET; + addr_2.sin_port = htons(SOCK_PORT_2); + addr_2.sin_addr.s_addr = inet_addr(IP_ADDRESS); + memset(&(addr_2.sin_zero), '\0', 8);These part could be factored out with helpers or/and test variants.+ /* Bind the socket 2 to IP address */+ ASSERT_EQ(-1, bind(sockfd_2, (struct sockaddr *)&addr_2, sizeof(addr_2)));+ ASSERT_EQ(EACCES, errno); + + /* Create a socket 3 */ + sockfd_3 = socket(AF_INET, SOCK_STREAM, 0); + ASSERT_LE(0, sockfd_3); + /* Allow reuse of local addresses */+ ASSERT_EQ(0, setsockopt(sockfd_3, SOL_SOCKET, SO_REUSEADDR, &one, sizeof(one)));+ + /* Set socket 3 address parameters */ + addr_3.sin_family = AF_INET; + addr_3.sin_port = htons(SOCK_PORT_3); + addr_3.sin_addr.s_addr = inet_addr(IP_ADDRESS); + memset(&(addr_3.sin_zero), '\0', 8); + /* Bind the socket 3 to IP address */+ ASSERT_EQ(0, bind(sockfd_3, (struct sockaddr *)&addr_3, sizeof(addr_3)));Why is it allowed to bind to SOCK_PORT_3 whereas net_service_3 forbids it?+} + +TEST(socket_connect_no_restrictions) { + + int sockfd, new_fd; + struct sockaddr_in addr; + pid_t child; + int status; + const int one = 1; + + /* Create a server socket */ + sockfd = socket(AF_INET, SOCK_STREAM, 0); + ASSERT_LE(0, sockfd); + /* Allow reuse of local addresses */+ ASSERT_EQ(0, setsockopt(sockfd, SOL_SOCKET, SO_REUSEADDR, &one, sizeof(one)));+ + /* Set socket address parameters */ + addr.sin_family = AF_INET; + addr.sin_port = htons(SOCK_PORT_1); + addr.sin_addr.s_addr = inet_addr(IP_ADDRESS); + memset(&(addr.sin_zero), '\0', 8); + + /* Bind the socket to IP address */ + ASSERT_EQ(0, bind(sockfd, (struct sockaddr *)&addr, sizeof(addr))); + + /* Make listening socket */ + ASSERT_EQ(0, listen(sockfd, BACKLOG)); + + child = fork(); + ASSERT_LE(0, child); + if (child == 0) { + int child_sockfd; + struct sockaddr_in connect_addr; + + /* Close listening socket for the child */ + ASSERT_EQ(0, close(sockfd)); + /* Create a stream client socket */ + child_sockfd = socket(AF_INET, SOCK_STREAM, 0); + ASSERT_LE(0, child_sockfd); + + /* Set server's socket address parameters*/ + connect_addr.sin_family = AF_INET; + connect_addr.sin_port = htons(SOCK_PORT_1); + connect_addr.sin_addr.s_addr = htonl(INADDR_ANY); + memset(&(connect_addr.sin_zero), '\0', 8); + + /* Make connection to the listening socket */+ ASSERT_EQ(0, connect(child_sockfd, (struct sockaddr *)&connect_addr,+ sizeof(struct sockaddr))); + _exit(_metadata->passed ? EXIT_SUCCESS : EXIT_FAILURE); + return; + } + /* Accept connection from the child */ + new_fd = accept(sockfd, NULL, 0); + ASSERT_LE(0, new_fd); + + /* Close connection */ + ASSERT_EQ(0, close(new_fd)); + + ASSERT_EQ(child, waitpid(child, &status, 0)); + ASSERT_EQ(1, WIFEXITED(status)); + ASSERT_EQ(EXIT_SUCCESS, WEXITSTATUS(status)); +} + +TEST(sockets_connect_with_restrictions) { + + int new_fd; + int sockfd_1, sockfd_2; + struct sockaddr_in addr_1, addr_2; + pid_t child_1, child_2; + int status; + const int one = 1; + + struct landlock_ruleset_attr ruleset_attr = { + .handled_access_net = LANDLOCK_ACCESS_NET_BIND_TCP | + LANDLOCK_ACCESS_NET_CONNECT_TCP, + }; + struct landlock_net_service_attr net_service_1 = { + .allowed_access = LANDLOCK_ACCESS_NET_BIND_TCP | + LANDLOCK_ACCESS_NET_CONNECT_TCP, + .port = SOCK_PORT_1, + }; + struct landlock_net_service_attr net_service_2 = { + .allowed_access = LANDLOCK_ACCESS_NET_BIND_TCP, + .port = SOCK_PORT_2, + }; + + const int ruleset_fd = landlock_create_ruleset(&ruleset_attr, + sizeof(ruleset_attr), 0); + ASSERT_LE(0, ruleset_fd); ++ /* Allow connect and bind operations to the SOCK_PORT_1 socket "object" */ + ASSERT_EQ(0, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_NET_SERVICE,+ &net_service_1, 0));+ /* Allow connect and deny bind operations to the SOCK_PORT_2 socket "object" */ + ASSERT_EQ(0, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_NET_SERVICE,+ &net_service_2, 0)); + + /* Enforces the ruleset. */ + ASSERT_EQ(0, prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0)); + ASSERT_EQ(0, landlock_restrict_self(ruleset_fd, 0)); + ASSERT_EQ(0, close(ruleset_fd)); + + /* Create a server socket 1 */ + sockfd_1 = socket(AF_INET, SOCK_STREAM, 0); + ASSERT_LE(0, sockfd_1); + /* Allow reuse of local addresses */+ ASSERT_EQ(0, setsockopt(sockfd_1, SOL_SOCKET, SO_REUSEADDR, &one, sizeof(one)));+ + /* Set socket 1 address parameters */ + addr_1.sin_family = AF_INET; + addr_1.sin_port = htons(SOCK_PORT_1); + addr_1.sin_addr.s_addr = inet_addr(IP_ADDRESS); + memset(&(addr_1.sin_zero), '\0', 8); + + /* Bind the socket 1 to IP address */+ ASSERT_EQ(0, bind(sockfd_1, (struct sockaddr *)&addr_1, sizeof(addr_1)));+ + /* Make listening socket 1 */ + ASSERT_EQ(0, listen(sockfd_1, BACKLOG)); + + child_1 = fork(); + ASSERT_LE(0, child_1); + if (child_1 == 0) { + int child_sockfd; + struct sockaddr_in connect_addr; + + /* Close listening socket for the child */ + ASSERT_EQ(0, close(sockfd_1)); + /* Create a stream client socket */ + child_sockfd = socket(AF_INET, SOCK_STREAM, 0); + ASSERT_LE(0, child_sockfd); + + /* Set server's socket 1 address parameters*/ + connect_addr.sin_family = AF_INET; + connect_addr.sin_port = htons(SOCK_PORT_1); + connect_addr.sin_addr.s_addr = htonl(INADDR_ANY); + memset(&(connect_addr.sin_zero), '\0', 8); + + /* Make connection to the listening socket 1 */+ ASSERT_EQ(0, connect(child_sockfd, (struct sockaddr *)&connect_addr,+ sizeof(struct sockaddr))); + _exit(_metadata->passed ? EXIT_SUCCESS : EXIT_FAILURE); + return; + } + /* Accept connection from the child 1 */ + new_fd = accept(sockfd_1, NULL, 0); + ASSERT_LE(0, new_fd); + + /* Close connection */ + ASSERT_EQ(0, close(new_fd)); + + ASSERT_EQ(child_1, waitpid(child_1, &status, 0)); + ASSERT_EQ(1, WIFEXITED(status)); + ASSERT_EQ(EXIT_SUCCESS, WEXITSTATUS(status)); + + /* Create a server socket 2 */ + sockfd_2 = socket(AF_INET, SOCK_STREAM, 0); + ASSERT_LE(0, sockfd_2); + /* Allow reuse of local addresses */+ ASSERT_EQ(0, setsockopt(sockfd_2, SOL_SOCKET, SO_REUSEADDR, &one, sizeof(one)));+ + /* Set socket 2 address parameters */ + addr_2.sin_family = AF_INET; + addr_2.sin_port = htons(SOCK_PORT_2); + addr_2.sin_addr.s_addr = inet_addr(IP_ADDRESS); + memset(&(addr_2.sin_zero), '\0', 8); + + /* Bind the socket 2 to IP address */+ ASSERT_EQ(0, bind(sockfd_2, (struct sockaddr *)&addr_2, sizeof(addr_2)));+ + /* Make listening socket 2 */ + ASSERT_EQ(0, listen(sockfd_2, BACKLOG)); + + child_2 = fork(); + ASSERT_LE(0, child_2); + if (child_2 == 0) { + int child_sockfd; + struct sockaddr_in connect_addr; + + /* Close listening socket for the child */ + ASSERT_EQ(0, close(sockfd_2)); + /* Create a stream client socket */ + child_sockfd = socket(AF_INET, SOCK_STREAM, 0); + ASSERT_LE(0, child_sockfd); + + /* Set server's socket address parameters*/ + connect_addr.sin_family = AF_INET; + connect_addr.sin_port = htons(SOCK_PORT_2); + connect_addr.sin_addr.s_addr = htonl(INADDR_ANY); + memset(&(connect_addr.sin_zero), '\0', 8); + + /* Make connection to the listening socket */+ ASSERT_EQ(-1, connect(child_sockfd, (struct sockaddr *)&connect_addr,+ sizeof(struct sockaddr))); + ASSERT_EQ(EACCES, errno); + _exit(_metadata->passed ? EXIT_SUCCESS : EXIT_FAILURE); + return; + } + + ASSERT_EQ(child_2, waitpid(child_2, &status, 0)); + ASSERT_EQ(1, WIFEXITED(status)); + ASSERT_EQ(EXIT_SUCCESS, WEXITSTATUS(status)); +} + +TEST_HARNESS_MAIN.
