From 89d64eee9d7a01c92b94f382b6783fb82bb1c417 Mon Sep 17 00:00:00 2001 From: Tom Smeding Date: Sat, 14 May 2022 13:39:31 +0200 Subject: Prepare for multiple different modifier options Signed-off-by: Tom Smeding --- bubblewrap.c | 32 +++++++++++--------------------- 1 file changed, 11 insertions(+), 21 deletions(-) diff --git a/bubblewrap.c b/bubblewrap.c index b17ff99..24a402c 100644 --- a/bubblewrap.c +++ b/bubblewrap.c @@ -1575,25 +1575,9 @@ print_version_and_exit (void) } static int -takes_perms (const char *next_option) +is_modifier_option (const char *option) { - static const char *const options_that_take_perms[] = - { - "--bind-data", - "--dir", - "--file", - "--ro-bind-data", - "--tmpfs", - }; - size_t i; - - for (i = 0; i < N_ELEMENTS (options_that_take_perms); i++) - { - if (strcmp (options_that_take_perms[i], next_option) == 0) - return 1; - } - - return 0; + return strcmp (option, "--perms") == 0; } static void @@ -1630,9 +1614,6 @@ parse_args_recurse (int *argcp, { const char *arg = argv[0]; - if (next_perms >= 0 && !takes_perms (arg)) - die ("--perms must be followed by an option that creates a file"); - if (strcmp (arg, "--help") == 0) { usage (EXIT_SUCCESS, stdout); @@ -2383,6 +2364,9 @@ parse_args_recurse (int *argcp, if (argc < 2) die ("--perms takes an argument"); + if (next_perms != -1) + die ("--perms given twice for the same action"); + perms = strtoul (argv[1], &endptr, 8); if (argv[1][0] == '\0' @@ -2435,6 +2419,12 @@ parse_args_recurse (int *argcp, break; } + /* If --perms was set for the current action but the current action + * didn't consume the setting, apparently --perms wasn't suitable for + * this action. */ + if (!is_modifier_option(arg) && next_perms >= 0) + die ("--perms must be followed by an option that creates a file"); + argv++; argc--; } -- cgit v1.2.1 From 494b269a69bd1a6e63fbeb1316506b8f8f3feab1 Mon Sep 17 00:00:00 2001 From: Tom Smeding Date: Sat, 14 May 2022 13:41:35 +0200 Subject: Add --size option to control size of a --tmpfs Signed-off-by: Tom Smeding --- bubblewrap.c | 111 +++++++++++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 93 insertions(+), 18 deletions(-) diff --git a/bubblewrap.c b/bubblewrap.c index 24a402c..9e67eb5 100644 --- a/bubblewrap.c +++ b/bubblewrap.c @@ -23,6 +23,7 @@ #include #include #include +#include #include #include #include @@ -52,6 +53,12 @@ __result; })) #endif +/* We limit the size of a tmpfs to half the architecture's address space, + * to avoid hitting arbitrary limits in the kernel. + * For example, on at least one x86_64 machine, the actual limit seems to be + * 2^64 - 2^12. */ +#define MAX_TMPFS_BYTES ((size_t) (SIZE_MAX >> 1)) + /* Globals to avoid having to use getuid(), since the uid/gid changes during runtime */ static uid_t real_uid; static gid_t real_gid; @@ -91,6 +98,7 @@ int opt_userns_fd = -1; int opt_userns2_fd = -1; int opt_pidns_fd = -1; int next_perms = -1; +size_t next_size_arg = 0; #define CAP_TO_MASK_0(x) (1L << ((x) & 31)) #define CAP_TO_MASK_1(x) CAP_TO_MASK_0(x - 32) @@ -149,6 +157,7 @@ struct _SetupOp int fd; SetupOpFlag flags; int perms; + size_t size; /* number of bytes, zero means unset/default */ SetupOp *next; }; @@ -177,6 +186,7 @@ typedef struct uint32_t op; uint32_t flags; uint32_t perms; + size_t size_arg; uint32_t arg1_offset; uint32_t arg2_offset; } PrivSepOp; @@ -341,6 +351,7 @@ usage (int ecode, FILE *out) " --cap-add CAP Add cap CAP when running as privileged user\n" " --cap-drop CAP Drop cap CAP when running as privileged user\n" " --perms OCTAL Set permissions of next argument (--bind-data, --file, etc.)\n" + " --size BYTES Set size of next argument (only for --tmpfs)\n" " --chmod OCTAL PATH Change permissions of PATH (must already exist)\n" ); exit (ecode); @@ -1001,6 +1012,7 @@ privileged_op (int privileged_op_socket, uint32_t op, uint32_t flags, uint32_t perms, + size_t size_arg, const char *arg1, const char *arg2) { @@ -1032,6 +1044,7 @@ privileged_op (int privileged_op_socket, op_buffer->op = op; op_buffer->flags = flags; op_buffer->perms = perms; + op_buffer->size_arg = size_arg; op_buffer->arg1_offset = arg1_offset; op_buffer->arg2_offset = arg2_offset; if (arg1 != NULL) @@ -1096,7 +1109,18 @@ privileged_op (int privileged_op_socket, case PRIV_SEP_OP_TMPFS_MOUNT: { - cleanup_free char *mode = xasprintf ("mode=%#o", perms); + cleanup_free char *mode = NULL; + + /* This check should be unnecessary since we checked this when parsing + * the --size option as well. However, better be safe than sorry. */ + if (size_arg > MAX_TMPFS_BYTES) + die_with_error ("Specified tmpfs size too large (%zu > %zu)", size_arg, MAX_TMPFS_BYTES); + + if (size_arg != 0) + mode = xasprintf ("mode=%#o,size=%zu", perms, size_arg); + else + mode = xasprintf ("mode=%#o", perms); + cleanup_free char *opt = label_mount (mode, opt_file_label); if (mount ("tmpfs", arg1, "tmpfs", MS_NOSUID | MS_NODEV, opt) != 0) die_with_error ("Can't mount tmpfs on %s", arg1); @@ -1197,12 +1221,12 @@ setup_newroot (bool unshare_pid, PRIV_SEP_OP_BIND_MOUNT, (op->type == SETUP_RO_BIND_MOUNT ? BIND_READONLY : 0) | (op->type == SETUP_DEV_BIND_MOUNT ? BIND_DEVICES : 0), - 0, source, dest); + 0, 0, source, dest); break; case SETUP_REMOUNT_RO_NO_RECURSIVE: privileged_op (privileged_op_socket, - PRIV_SEP_OP_REMOUNT_RO_NO_RECURSIVE, 0, 0, NULL, dest); + PRIV_SEP_OP_REMOUNT_RO_NO_RECURSIVE, 0, 0, 0, NULL, dest); break; case SETUP_MOUNT_PROC: @@ -1213,14 +1237,14 @@ setup_newroot (bool unshare_pid, { /* Our own procfs */ privileged_op (privileged_op_socket, - PRIV_SEP_OP_PROC_MOUNT, 0, 0, + PRIV_SEP_OP_PROC_MOUNT, 0, 0, 0, dest, NULL); } else { /* Use system procfs, as we share pid namespace anyway */ privileged_op (privileged_op_socket, - PRIV_SEP_OP_BIND_MOUNT, 0, 0, + PRIV_SEP_OP_BIND_MOUNT, 0, 0, 0, "oldroot/proc", dest); } @@ -1242,7 +1266,7 @@ setup_newroot (bool unshare_pid, } privileged_op (privileged_op_socket, - PRIV_SEP_OP_BIND_MOUNT, BIND_READONLY, 0, + PRIV_SEP_OP_BIND_MOUNT, BIND_READONLY, 0, 0, subdir, subdir); } @@ -1253,7 +1277,7 @@ setup_newroot (bool unshare_pid, die_with_error ("Can't mkdir %s", op->dest); privileged_op (privileged_op_socket, - PRIV_SEP_OP_TMPFS_MOUNT, 0, 0755, + PRIV_SEP_OP_TMPFS_MOUNT, 0, 0755, 0, dest, NULL); static const char *const devnodes[] = { "null", "zero", "full", "random", "urandom", "tty" }; @@ -1264,7 +1288,7 @@ setup_newroot (bool unshare_pid, if (create_file (node_dest, 0444, NULL) != 0) die_with_error ("Can't create file %s/%s", op->dest, devnodes[i]); privileged_op (privileged_op_socket, - PRIV_SEP_OP_BIND_MOUNT, BIND_DEVICES, 0, + PRIV_SEP_OP_BIND_MOUNT, BIND_DEVICES, 0, 0, node_src, node_dest); } @@ -1298,7 +1322,7 @@ setup_newroot (bool unshare_pid, if (mkdir (pts, 0755) == -1) die_with_error ("Can't create %s/devpts", op->dest); privileged_op (privileged_op_socket, - PRIV_SEP_OP_DEVPTS_MOUNT, 0, 0, pts, NULL); + PRIV_SEP_OP_DEVPTS_MOUNT, 0, 0, 0, pts, NULL); if (symlink ("pts/ptmx", ptmx) != 0) die_with_error ("Can't make symlink at %s/ptmx", op->dest); @@ -1318,7 +1342,7 @@ setup_newroot (bool unshare_pid, die_with_error ("creating %s/console", op->dest); privileged_op (privileged_op_socket, - PRIV_SEP_OP_BIND_MOUNT, BIND_DEVICES, 0, + PRIV_SEP_OP_BIND_MOUNT, BIND_DEVICES, 0, 0, src_tty_dev, dest_console); } @@ -1333,7 +1357,7 @@ setup_newroot (bool unshare_pid, die_with_error ("Can't mkdir %s", op->dest); privileged_op (privileged_op_socket, - PRIV_SEP_OP_TMPFS_MOUNT, 0, op->perms, + PRIV_SEP_OP_TMPFS_MOUNT, 0, op->perms, op->size, dest, NULL); break; @@ -1342,7 +1366,7 @@ setup_newroot (bool unshare_pid, die_with_error ("Can't mkdir %s", op->dest); privileged_op (privileged_op_socket, - PRIV_SEP_OP_MQUEUE_MOUNT, 0, 0, + PRIV_SEP_OP_MQUEUE_MOUNT, 0, 0, 0, dest, NULL); break; @@ -1423,7 +1447,7 @@ setup_newroot (bool unshare_pid, privileged_op (privileged_op_socket, PRIV_SEP_OP_BIND_MOUNT, (op->type == SETUP_MAKE_RO_BIND_FILE ? BIND_READONLY : 0), - 0, tempfile, dest); + 0, 0, tempfile, dest); /* Remove the file so we're sure the app can't get to it in any other way. Its outside the container chroot, so it shouldn't be possible, but lets @@ -1441,7 +1465,7 @@ setup_newroot (bool unshare_pid, case SETUP_SET_HOSTNAME: assert (op->dest != NULL); /* guaranteed by the constructor */ privileged_op (privileged_op_socket, - PRIV_SEP_OP_SET_HOSTNAME, 0, 0, + PRIV_SEP_OP_SET_HOSTNAME, 0, 0, 0, op->dest, NULL); break; @@ -1450,7 +1474,7 @@ setup_newroot (bool unshare_pid, } } privileged_op (privileged_op_socket, - PRIV_SEP_OP_DONE, 0, 0, NULL, NULL); + PRIV_SEP_OP_DONE, 0, 0, 0, NULL, NULL); } /* Do not leak file descriptors already used by setup_newroot () */ @@ -1537,6 +1561,7 @@ read_priv_sec_op (int read_socket, size_t buffer_size, uint32_t *flags, uint32_t *perms, + size_t *size_arg, const char **arg1, const char **arg2) { @@ -1561,6 +1586,7 @@ read_priv_sec_op (int read_socket, *flags = op->flags; *perms = op->perms; + *size_arg = op->size_arg; *arg1 = resolve_string_offset (buffer, rec_len, op->arg1_offset); *arg2 = resolve_string_offset (buffer, rec_len, op->arg2_offset); @@ -1577,7 +1603,8 @@ print_version_and_exit (void) static int is_modifier_option (const char *option) { - return strcmp (option, "--perms") == 0; + return strcmp (option, "--perms") == 0 + || strcmp(option, "--size") == 0; } static void @@ -1871,6 +1898,13 @@ parse_args_recurse (int *argcp, op->perms = 0755; next_perms = -1; + + /* If the option is unset, next_size_arg is zero, which results in + * the default tmpfs size. This is exactly what we want. */ + op->size = next_size_arg; + + next_size_arg = 0; + argv += 1; argc -= 1; } @@ -2377,6 +2411,42 @@ parse_args_recurse (int *argcp, next_perms = (int) perms; + argv += 1; + argc -= 1; + } + else if (strcmp (arg, "--size") == 0) + { + unsigned long long size; + char *endptr = NULL; + + if (is_privileged) + die ("The --size option is not permitted in setuid mode"); + + if (argc < 2) + die ("--size takes an argument"); + + if (next_size_arg != 0) + die ("--size given twice for the same action"); + + errno = 0; /* reset errno so we can detect ERANGE from strtoull */ + + size = strtoull (argv[1], &endptr, 0); + + /* isdigit: Not only check that the first digit is not '\0', but + * simultaneously guard against negative numbers or preceding + * spaces. */ + if (errno != 0 /* from strtoull */ + || !isdigit(argv[1][0]) + || endptr == NULL + || *endptr != '\0' + || size == 0) + die ("--size takes a non-zero number of bytes"); + + if (size > MAX_TMPFS_BYTES) + die ("--size (for tmpfs) is limited to %zu", MAX_TMPFS_BYTES); + + next_size_arg = (size_t) size; + argv += 1; argc -= 1; } @@ -2425,6 +2495,10 @@ parse_args_recurse (int *argcp, if (!is_modifier_option(arg) && next_perms >= 0) die ("--perms must be followed by an option that creates a file"); + /* Similarly for --size. */ + if (!is_modifier_option(arg) && next_size_arg != 0) + die ("--size must be followed by --tmpfs"); + argv++; argc--; } @@ -3006,6 +3080,7 @@ main (int argc, int status; uint32_t buffer[2048]; /* 8k, but is int32 to guarantee nice alignment */ uint32_t op, flags, perms; + size_t size_arg; const char *arg1, *arg2; cleanup_fd int unpriv_socket = -1; @@ -3015,8 +3090,8 @@ main (int argc, do { op = read_priv_sec_op (unpriv_socket, buffer, sizeof (buffer), - &flags, &perms, &arg1, &arg2); - privileged_op (-1, op, flags, perms, arg1, arg2); + &flags, &perms, &size_arg, &arg1, &arg2); + privileged_op (-1, op, flags, perms, size_arg, arg1, arg2); if (write (unpriv_socket, buffer, 1) != 1) die ("Can't write to op_socket"); } -- cgit v1.2.1 From 906a7a75bdbf856fac7795c4440f3c170878fe3d Mon Sep 17 00:00:00 2001 From: Tom Smeding Date: Sat, 7 May 2022 10:15:39 +0200 Subject: Tests for --size and --perms Signed-off-by: Tom Smeding --- tests/test-run.sh | 59 ++++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 58 insertions(+), 1 deletion(-) diff --git a/tests/test-run.sh b/tests/test-run.sh index f25a9bc..cafc580 100755 --- a/tests/test-run.sh +++ b/tests/test-run.sh @@ -8,7 +8,7 @@ srcd=$(cd $(dirname "$0") && pwd) bn=$(basename "$0") -echo "1..54" +echo "1..57" # Test help ${BWRAP} --help > help.txt @@ -398,6 +398,29 @@ $RUN \ assert_file_has_content dir-permissions '^755$' echo "ok - tmpfs has expected permissions" +# 1048576 = 1 MiB +$RUN \ + --size 1048576 --tmpfs "$(pwd -P)" \ + df --output=size --block-size=1K "$(pwd -P)" > dir-size +assert_file_has_content dir-size '^ *1024$' +$RUN \ + --size 1048576 --perms 01777 --tmpfs "$(pwd -P)" \ + stat -c '%a' "$(pwd -P)" > dir-permissions +assert_file_has_content dir-permissions '^1777$' +$RUN \ + --size 1048576 --perms 01777 --tmpfs "$(pwd -P)" \ + df --output=size --block-size=1K "$(pwd -P)" > dir-size +assert_file_has_content dir-size '^ *1024$' +$RUN \ + --perms 01777 --size 1048576 --tmpfs "$(pwd -P)" \ + stat -c '%a' "$(pwd -P)" > dir-permissions +assert_file_has_content dir-permissions '^1777$' +$RUN \ + --perms 01777 --size 1048576 --tmpfs "$(pwd -P)" \ + df --output=size --block-size=1K "$(pwd -P)" > dir-size +assert_file_has_content dir-size '^ *1024$' +echo "ok - tmpfs has expected size" + $RUN \ --file 0 /tmp/file \ stat -c '%a' /tmp/file < /dev/null > file-permissions @@ -424,6 +447,40 @@ $RUN \ assert_file_has_content file-permissions '^640$' echo "ok - files have expected permissions" +if $RUN --size 0 --tmpfs /tmp/a true; then + assert_not_reached Zero tmpfs size allowed +fi +if $RUN --size 123bogus --tmpfs /tmp/a true; then + assert_not_reached Bogus tmpfs size allowed +fi +if $RUN --size '' --tmpfs /tmp/a true; then + assert_not_reached Empty tmpfs size allowed +fi +if $RUN --size -12345678 --tmpfs /tmp/a true; then + assert_not_reached Negative tmpfs size allowed +fi +if $RUN --size ' -12345678' --tmpfs /tmp/a true; then + assert_not_reached Negative tmpfs size with space allowed +fi +# This is 2^64 +if $RUN --size 18446744073709551616 --tmpfs /tmp/a true; then + assert_not_reached Overflowing tmpfs size allowed +fi +# This is 2^63 + 1; note that the current max size is SIZE_MAX/2 +if $RUN --size 9223372036854775809 --tmpfs /tmp/a true; then + assert_not_reached Too-large tmpfs size allowed +fi +echo "ok - bogus tmpfs size not allowed" + +if $RUN --perms 0640 --perms 0640 --tmpfs /tmp/a true; then + assert_not_reached Multiple perms options allowed +fi +if $RUN --size 1048576 --size 1048576 --tmpfs /tmp/a true; then + assert_not_reached Multiple perms options allowed +fi +echo "ok - --perms and --size only allowed once" + + FOO= BAR=baz $RUN --setenv FOO bar sh -c 'echo "$FOO$BAR"' > stdout assert_file_has_content stdout barbaz FOO=wrong BAR=baz $RUN --setenv FOO bar sh -c 'echo "$FOO$BAR"' > stdout -- cgit v1.2.1 From 7655fe2f9dd63f0e3cda07dab385af5308265786 Mon Sep 17 00:00:00 2001 From: Tom Smeding Date: Thu, 5 May 2022 00:47:24 +0200 Subject: --size: Update completions and documentation Signed-off-by: Tom Smeding --- bwrap.xml | 26 ++++++++++++++++++++++++-- completions/bash/bwrap | 1 + completions/zsh/_bwrap | 21 +++++++++++++++++++++ 3 files changed, 46 insertions(+), 2 deletions(-) diff --git a/bwrap.xml b/bwrap.xml index 2baec5d..46e2478 100644 --- a/bwrap.xml +++ b/bwrap.xml @@ -207,6 +207,9 @@ (rwxr-xr-x). However, if a option is in effect, and it sets the permissions for group or other to zero, then newly-created parent directories will also have their corresponding permission set to zero. + modifies the size of the created mount when preceding a + action; and + can be combined. @@ -217,7 +220,24 @@ Subsequent operations are not affected: for example, --perms 0700 --tmpfs /a --tmpfs /b will mount /a with permissions 0700, then return to - the default permissions for /b. + the default permissions for /b. + Note that and can be + combined: --perms 0700 --size 10485760 --tmpfs /s will apply + permissions as well as a maximum size to the created tmpfs. + + + + This option does nothing on its own, and must be followed + by --tmpfs. It sets the size in bytes for the next tmpfs. + For example, --size 10485760 --tmpfs /tmp will create a tmpfs + at /tmp of size 10MiB. Subsequent operations are not + affected: for example, + --size 10485760 --tmpfs /a --tmpfs /b will mount + /a with size 10MiB, then return to the default size for + /b. + Note that and can be + combined: --size 10485760 --perms 0700 --tmpfs /s will apply + permissions as well as a maximum size to the created tmpfs. @@ -260,7 +280,9 @@ Mount new tmpfs on DEST. If the previous option was , it sets the - mode of the tmpfs. Otherwise, the tmpfs has mode 0755. + mode of the tmpfs. Otherwise, the tmpfs has mode 0755. + If the previous option was , it sets the + size in bytes of the tmpfs. Otherwise, the tmpfs has the default size. diff --git a/completions/bash/bwrap b/completions/bash/bwrap index c57d9ab..e796be3 100644 --- a/completions/bash/bwrap +++ b/completions/bash/bwrap @@ -54,6 +54,7 @@ _bwrap() { --ro-bind --seccomp --setenv + --size --symlink --sync-fd --uid diff --git a/completions/zsh/_bwrap b/completions/zsh/_bwrap index 1e365f0..f81ffaf 100644 --- a/completions/zsh/_bwrap +++ b/completions/zsh/_bwrap @@ -1,11 +1,23 @@ #compdef bwrap +_bwrap_args_after_perms_size=( + # Please sort alphabetically (in LC_ALL=C order) by option name + '--tmpfs[Mount new tmpfs on DEST]:mount point for tmpfs:_files -/' +) + _bwrap_args_after_perms=( # Please sort alphabetically (in LC_ALL=C order) by option name '--bind-data[Copy from FD to file which is bind-mounted on DEST]: :_guard "[0-9]#" "file descriptor to read content":destination:_files' '--dir[Create dir at DEST]:directory to create:_files -/' '--file[Copy from FD to destination DEST]: :_guard "[0-9]#" "file descriptor to read content from":destination:_files' '--ro-bind-data[Copy from FD to file which is readonly bind-mounted on DEST]: :_guard "[0-9]#" "file descriptor to read content from":destination:_files' + '--size[Set size in bytes for next action argument]: :->after_perms_size' + '--tmpfs[Mount new tmpfs on DEST]:mount point for tmpfs:_files -/' +) + +_bwrap_args_after_size=( + # Please sort alphabetically (in LC_ALL=C order) by option name + '--perms[Set permissions for next action argument]: :_guard "[0-7]#" "permissions in octal": :->after_perms_size' '--tmpfs[Mount new tmpfs on DEST]:mount point for tmpfs:_files -/' ) @@ -47,6 +59,7 @@ _bwrap_args=( '--ro-bind[Bind mount the host path SRC readonly on DEST]:source:_files:destination:_files' '--seccomp[Load and use seccomp rules from FD]: :_guard "[0-9]#" "file descriptor to read seccomp rules from"' '--setenv[Set an environment variable]:variable to set:_parameters -g "*export*":value of variable: :' + '--size[Set size in bytes for next action argument]: :->after_size' '--symlink[Create symlink at DEST with target SRC]:symlink target:_files:symlink to create:_files:' '--sync-fd[Keep this fd open while sandbox is running]: :_guard "[0-9]#" "file descriptor to keep open"' '--uid[Custom uid in the sandbox (requires --unshare-user or --userns)]: :_guard "[0-9]#" "numeric group ID"' @@ -73,6 +86,14 @@ _bwrap() { _values -S ' ' 'option' $_bwrap_args_after_perms ;; + after_size) + _values -S ' ' 'option' $_bwrap_args_after_size + ;; + + after_perms_size) + _values -S ' ' 'option' $_bwrap_args_after_perms_size + ;; + caps) # $ grep -E '#define\sCAP_\w+\s+[0-9]+' /usr/include/linux/capability.h | awk '{print $2}' | xargs echo local all_caps=( -- cgit v1.2.1