diff mbox series

[v12,11/12] samples/landlock: Add network demo

Message ID 20230920092641.832134-12-konstantin.meskhidze@huawei.com (mailing list archive)
State Handled Elsewhere
Headers show
Series Network support for Landlock | expand

Commit Message

Konstantin Meskhidze (A) Sept. 20, 2023, 9:26 a.m. UTC
This commit adds network demo. It's possible to allow a sandboxer to
bind/connect to a list of particular ports restricting network
actions to the rest of ports.

Signed-off-by: Konstantin Meskhidze <konstantin.meskhidze@huawei.com>
---

Changes since v11:
* Changes ENV_PATH_TOKEN to ENV_DELIMITER.
* Refactors populate_ruleset_net():
  - Deletes parse_port_num() helper.
  - Uses strsep() instead of strtok().
* Fixes wrong printf format.

Changes since v10:
* Refactors populate_ruleset_net() helper.
* Code style minor fix.

Changes since v9:
* Deletes ports converting.
* Minor fixes.

Changes since v8:
* Convert ports to __be16.
* Minor fixes.

Changes since v7:
* Removes network support if ABI < 4.
* Removes network support if not set by a user.

Changes since v6:
* Removes network support if ABI < 3.

Changes since v5:
* Makes network ports sandboxing optional.
* Fixes some logic errors.
* Formats code with clang-format-14.

Changes since v4:
* Adds ENV_TCP_BIND_NAME "LL_TCP_BIND" and
ENV_TCP_CONNECT_NAME "LL_TCP_CONNECT" variables
to insert TCP ports.
* Renames populate_ruleset() to populate_ruleset_fs().
* Adds populate_ruleset_net() and parse_port_num() helpers.
* Refactors main() to support network sandboxing.

---
 samples/landlock/sandboxer.c | 114 ++++++++++++++++++++++++++++++-----
 1 file changed, 99 insertions(+), 15 deletions(-)

--
2.25.1

Comments

Geert Uytterhoeven Oct. 3, 2023, 1:23 p.m. UTC | #1
Hi Mickaël,

On Tue, Oct 3, 2023 at 3:15 PM Mickaël Salaün <mic@digikod.net> wrote:
> On Tue, Oct 03, 2023 at 02:27:37PM +1100, Stephen Rothwell wrote:
> > After merging the landlock tree, today's linux-next build (powerpc
> > allyesconfig) produced this warning:
> >
> > samples/landlock/sandboxer.c: In function 'populate_ruleset_net':
> > samples/landlock/sandboxer.c:170:78: warning: format '%llu' expects argument of type 'long long unsigned int', but argument 3 has type '__u64' {aka 'long unsigned int'} [-Wformat=]
> >   170 |                                 "Failed to update the ruleset with port \"%llu\": %s\n",
> >       |                                                                           ~~~^
> >       |                                                                              |
> >       |                                                                              long long unsigned int
> >       |                                                                           %lu
> >   171 |                                 net_port.port, strerror(errno));
> >       |                                 ~~~~~~~~~~~~~
> >       |                                         |
> >       |                                         __u64 {aka long unsigned int}
> >
> > Introduced by commit
> >
> >   24889e7a2079 ("samples/landlock: Add network demo")
>
> PowerPC-64 follows the LP64 data model and then uses int-l64.h (instead of
> int-ll64.h like most architectures) for user space code.
>
> Here is the same code with the (suggested) "%lu" token on x86_86:
>
>   samples/landlock/sandboxer.c: In function ‘populate_ruleset_net’:
>   samples/landlock/sandboxer.c:170:77: error: format ‘%lu’ expects argument of type ‘long unsigned int’, but argument 3 has type ‘__u64’ {aka ‘long long unsigned int’} [-Werror=format=]
>     170 |                                 "Failed to update the ruleset with port \"%lu\": %s\n",
>         |                                                                           ~~^
>         |                                                                             |
>         |                                                                             long unsigned int
>         |                                                                           %llu
>     171 |                                 net_port.port, strerror(errno));
>         |                                 ~~~~~~~~~~~~~
>         |                                         |
>         |                                         __u64 {aka long long unsigned int}
>
>
> We would then need to cast __u64 to unsigned long long to avoid this warning,
> which may look useless, of even buggy, for people taking a look at this sample.

In userspace code, you are supposed to #include <inttypes.h>
and use PRIu64.

> Anyway, it makes more sense to cast it to __u16 because it is the
> expected type for a TCP port. I'm updating the patch with that.
> Konstantin, please take this fix for the next series:
> https://git.kernel.org/mic/c/fc9de206a61a

Until someone passes a too large number, and it becomes truncated...

Gr{oetje,eeting}s,

                        Geert
Mickaël Salaün Oct. 4, 2023, 11:01 a.m. UTC | #2
On Tue, Oct 03, 2023 at 03:23:22PM +0200, Geert Uytterhoeven wrote:
> Hi Mickaël,
> 
> On Tue, Oct 3, 2023 at 3:15 PM Mickaël Salaün <mic@digikod.net> wrote:
> > On Tue, Oct 03, 2023 at 02:27:37PM +1100, Stephen Rothwell wrote:
> > > After merging the landlock tree, today's linux-next build (powerpc
> > > allyesconfig) produced this warning:
> > >
> > > samples/landlock/sandboxer.c: In function 'populate_ruleset_net':
> > > samples/landlock/sandboxer.c:170:78: warning: format '%llu' expects argument of type 'long long unsigned int', but argument 3 has type '__u64' {aka 'long unsigned int'} [-Wformat=]
> > >   170 |                                 "Failed to update the ruleset with port \"%llu\": %s\n",
> > >       |                                                                           ~~~^
> > >       |                                                                              |
> > >       |                                                                              long long unsigned int
> > >       |                                                                           %lu
> > >   171 |                                 net_port.port, strerror(errno));
> > >       |                                 ~~~~~~~~~~~~~
> > >       |                                         |
> > >       |                                         __u64 {aka long unsigned int}
> > >
> > > Introduced by commit
> > >
> > >   24889e7a2079 ("samples/landlock: Add network demo")
> >
> > PowerPC-64 follows the LP64 data model and then uses int-l64.h (instead of
> > int-ll64.h like most architectures) for user space code.
> >
> > Here is the same code with the (suggested) "%lu" token on x86_86:
> >
> >   samples/landlock/sandboxer.c: In function ‘populate_ruleset_net’:
> >   samples/landlock/sandboxer.c:170:77: error: format ‘%lu’ expects argument of type ‘long unsigned int’, but argument 3 has type ‘__u64’ {aka ‘long long unsigned int’} [-Werror=format=]
> >     170 |                                 "Failed to update the ruleset with port \"%lu\": %s\n",
> >         |                                                                           ~~^
> >         |                                                                             |
> >         |                                                                             long unsigned int
> >         |                                                                           %llu
> >     171 |                                 net_port.port, strerror(errno));
> >         |                                 ~~~~~~~~~~~~~
> >         |                                         |
> >         |                                         __u64 {aka long long unsigned int}
> >
> >
> > We would then need to cast __u64 to unsigned long long to avoid this warning,
> > which may look useless, of even buggy, for people taking a look at this sample.
> 
> In userspace code, you are supposed to #include <inttypes.h>
> and use PRIu64.

Thanks for these tips!

> 
> > Anyway, it makes more sense to cast it to __u16 because it is the
> > expected type for a TCP port. I'm updating the patch with that.
> > Konstantin, please take this fix for the next series:
> > https://git.kernel.org/mic/c/fc9de206a61a
> 
> Until someone passes a too large number, and it becomes truncated...

That should not happen because it is checked by the kernel (for this
specific case), but let's make it simple and print the 64-bit value.

> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> -- 
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds
diff mbox series

Patch

diff --git a/samples/landlock/sandboxer.c b/samples/landlock/sandboxer.c
index e2056c8b902c..bd1794039078 100644
--- a/samples/landlock/sandboxer.c
+++ b/samples/landlock/sandboxer.c
@@ -8,6 +8,7 @@ 
  */

 #define _GNU_SOURCE
+#include <arpa/inet.h>
 #include <errno.h>
 #include <fcntl.h>
 #include <linux/landlock.h>
@@ -51,7 +52,9 @@  static inline int landlock_restrict_self(const int ruleset_fd,

 #define ENV_FS_RO_NAME "LL_FS_RO"
 #define ENV_FS_RW_NAME "LL_FS_RW"
-#define ENV_PATH_TOKEN ":"
+#define ENV_TCP_BIND_NAME "LL_TCP_BIND"
+#define ENV_TCP_CONNECT_NAME "LL_TCP_CONNECT"
+#define ENV_DELIMITER ":"

 static int parse_path(char *env_path, const char ***const path_list)
 {
@@ -60,13 +63,13 @@  static int parse_path(char *env_path, const char ***const path_list)
 	if (env_path) {
 		num_paths++;
 		for (i = 0; env_path[i]; i++) {
-			if (env_path[i] == ENV_PATH_TOKEN[0])
+			if (env_path[i] == ENV_DELIMITER[0])
 				num_paths++;
 		}
 	}
 	*path_list = malloc(num_paths * sizeof(**path_list));
 	for (i = 0; i < num_paths; i++)
-		(*path_list)[i] = strsep(&env_path, ENV_PATH_TOKEN);
+		(*path_list)[i] = strsep(&env_path, ENV_DELIMITER);

 	return num_paths;
 }
@@ -81,8 +84,8 @@  static int parse_path(char *env_path, const char ***const path_list)

 /* clang-format on */

-static int populate_ruleset(const char *const env_var, const int ruleset_fd,
-			    const __u64 allowed_access)
+static int populate_ruleset_fs(const char *const env_var, const int ruleset_fd,
+			       const __u64 allowed_access)
 {
 	int num_paths, i, ret = 1;
 	char *env_path_name;
@@ -143,6 +146,39 @@  static int populate_ruleset(const char *const env_var, const int ruleset_fd,
 	return ret;
 }

+static int populate_ruleset_net(const char *const env_var, const int ruleset_fd,
+				const __u64 allowed_access)
+{
+	int ret = 1;
+	char *env_port_name, *strport;
+	struct landlock_net_port_attr net_port = {
+		.allowed_access = allowed_access,
+		.port = 0,
+	};
+
+	env_port_name = getenv(env_var);
+	if (!env_port_name)
+		return 0;
+	env_port_name = strdup(env_port_name);
+	unsetenv(env_var);
+
+	while ((strport = strsep(&env_port_name, ENV_DELIMITER))) {
+		net_port.port = atoi(strport);
+		if (landlock_add_rule(ruleset_fd, LANDLOCK_RULE_NET_PORT,
+				      &net_port, 0)) {
+			fprintf(stderr,
+				"Failed to update the ruleset with port \"%llu\": %s\n",
+				net_port.port, strerror(errno));
+			goto out_free_name;
+		}
+	}
+	ret = 0;
+
+out_free_name:
+	free(env_port_name);
+	return ret;
+}
+
 /* clang-format off */

 #define ACCESS_FS_ROUGHLY_READ ( \
@@ -166,39 +202,58 @@  static int populate_ruleset(const char *const env_var, const int ruleset_fd,

 /* clang-format on */

-#define LANDLOCK_ABI_LAST 3
+#define LANDLOCK_ABI_LAST 4

 int main(const int argc, char *const argv[], char *const *const envp)
 {
 	const char *cmd_path;
 	char *const *cmd_argv;
 	int ruleset_fd, abi;
+	char *env_port_name;
 	__u64 access_fs_ro = ACCESS_FS_ROUGHLY_READ,
 	      access_fs_rw = ACCESS_FS_ROUGHLY_READ | ACCESS_FS_ROUGHLY_WRITE;
+
 	struct landlock_ruleset_attr ruleset_attr = {
 		.handled_access_fs = access_fs_rw,
+		.handled_access_net = LANDLOCK_ACCESS_NET_BIND_TCP |
+				      LANDLOCK_ACCESS_NET_CONNECT_TCP,
 	};

 	if (argc < 2) {
 		fprintf(stderr,
-			"usage: %s=\"...\" %s=\"...\" %s <cmd> [args]...\n\n",
-			ENV_FS_RO_NAME, ENV_FS_RW_NAME, argv[0]);
+			"usage: %s=\"...\" %s=\"...\" %s=\"...\" %s=\"...\"%s "
+			"<cmd> [args]...\n\n",
+			ENV_FS_RO_NAME, ENV_FS_RW_NAME, ENV_TCP_BIND_NAME,
+			ENV_TCP_CONNECT_NAME, argv[0]);
 		fprintf(stderr,
 			"Launch a command in a restricted environment.\n\n");
-		fprintf(stderr, "Environment variables containing paths, "
-				"each separated by a colon:\n");
+		fprintf(stderr,
+			"Environment variables containing paths and ports "
+			"each separated by a colon:\n");
 		fprintf(stderr,
 			"* %s: list of paths allowed to be used in a read-only way.\n",
 			ENV_FS_RO_NAME);
 		fprintf(stderr,
-			"* %s: list of paths allowed to be used in a read-write way.\n",
+			"* %s: list of paths allowed to be used in a read-write way.\n\n",
 			ENV_FS_RW_NAME);
+		fprintf(stderr,
+			"Environment variables containing ports are optional "
+			"and could be skipped.\n");
+		fprintf(stderr,
+			"* %s: list of ports allowed to bind (server).\n",
+			ENV_TCP_BIND_NAME);
+		fprintf(stderr,
+			"* %s: list of ports allowed to connect (client).\n",
+			ENV_TCP_CONNECT_NAME);
 		fprintf(stderr,
 			"\nexample:\n"
 			"%s=\"/bin:/lib:/usr:/proc:/etc:/dev/urandom\" "
 			"%s=\"/dev/null:/dev/full:/dev/zero:/dev/pts:/tmp\" "
+			"%s=\"9418\" "
+			"%s=\"80:443\" "
 			"%s bash -i\n\n",
-			ENV_FS_RO_NAME, ENV_FS_RW_NAME, argv[0]);
+			ENV_FS_RO_NAME, ENV_FS_RW_NAME, ENV_TCP_BIND_NAME,
+			ENV_TCP_CONNECT_NAME, argv[0]);
 		fprintf(stderr,
 			"This sandboxer can use Landlock features "
 			"up to ABI version %d.\n",
@@ -255,7 +310,12 @@  int main(const int argc, char *const argv[], char *const *const envp)
 	case 2:
 		/* Removes LANDLOCK_ACCESS_FS_TRUNCATE for ABI < 3 */
 		ruleset_attr.handled_access_fs &= ~LANDLOCK_ACCESS_FS_TRUNCATE;
-
+		__attribute__((fallthrough));
+	case 3:
+		/* Removes network support for ABI < 4 */
+		ruleset_attr.handled_access_net &=
+			~(LANDLOCK_ACCESS_NET_BIND_TCP |
+			  LANDLOCK_ACCESS_NET_CONNECT_TCP);
 		fprintf(stderr,
 			"Hint: You should update the running kernel "
 			"to leverage Landlock features "
@@ -274,18 +334,42 @@  int main(const int argc, char *const argv[], char *const *const envp)
 	access_fs_ro &= ruleset_attr.handled_access_fs;
 	access_fs_rw &= ruleset_attr.handled_access_fs;

+	/* Removes bind access attribute if not supported by a user. */
+	env_port_name = getenv(ENV_TCP_BIND_NAME);
+	if (!env_port_name) {
+		ruleset_attr.handled_access_net &=
+			~LANDLOCK_ACCESS_NET_BIND_TCP;
+	}
+	/* Removes connect access attribute if not supported by a user. */
+	env_port_name = getenv(ENV_TCP_CONNECT_NAME);
+	if (!env_port_name) {
+		ruleset_attr.handled_access_net &=
+			~LANDLOCK_ACCESS_NET_CONNECT_TCP;
+	}
+
 	ruleset_fd =
 		landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
 	if (ruleset_fd < 0) {
 		perror("Failed to create a ruleset");
 		return 1;
 	}
-	if (populate_ruleset(ENV_FS_RO_NAME, ruleset_fd, access_fs_ro)) {
+
+	if (populate_ruleset_fs(ENV_FS_RO_NAME, ruleset_fd, access_fs_ro)) {
+		goto err_close_ruleset;
+	}
+	if (populate_ruleset_fs(ENV_FS_RW_NAME, ruleset_fd, access_fs_rw)) {
 		goto err_close_ruleset;
 	}
-	if (populate_ruleset(ENV_FS_RW_NAME, ruleset_fd, access_fs_rw)) {
+
+	if (populate_ruleset_net(ENV_TCP_BIND_NAME, ruleset_fd,
+				 LANDLOCK_ACCESS_NET_BIND_TCP)) {
+		goto err_close_ruleset;
+	}
+	if (populate_ruleset_net(ENV_TCP_CONNECT_NAME, ruleset_fd,
+				 LANDLOCK_ACCESS_NET_CONNECT_TCP)) {
 		goto err_close_ruleset;
 	}
+
 	if (prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0)) {
 		perror("Failed to restrict privileges");
 		goto err_close_ruleset;