diff options
-rw-r--r-- | src/basic/umask-util.h | 3 | ||||
-rw-r--r-- | src/core/namespace.c | 49 | ||||
-rw-r--r-- | src/nspawn/nspawn.c | 9 | ||||
-rw-r--r-- | src/shared/dev-setup.c | 3 | ||||
-rw-r--r-- | src/test/test-execute.c | 5 | ||||
-rw-r--r-- | src/test/test-fs-util.c | 3 | ||||
-rw-r--r-- | test/test-execute/exec-umask-namespace.service | 12 |
7 files changed, 51 insertions, 33 deletions
diff --git a/src/basic/umask-util.h b/src/basic/umask-util.h index bd7c2bdb8c..90d18f70ba 100644 --- a/src/basic/umask-util.h +++ b/src/basic/umask-util.h @@ -24,3 +24,6 @@ assert_cc((S_IFMT & 0777) == 0); for (_cleanup_umask_ mode_t _saved_umask_ = umask(mask) | S_IFMT; \ FLAGS_SET(_saved_umask_, S_IFMT); \ _saved_umask_ &= 0777) + +#define BLOCK_WITH_UMASK(mask) \ + _unused_ _cleanup_umask_ mode_t _saved_umask_ = umask(mask); diff --git a/src/core/namespace.c b/src/core/namespace.c index 68704dff06..c01975b9de 100644 --- a/src/core/namespace.c +++ b/src/core/namespace.c @@ -806,8 +806,7 @@ static int clone_device_node( *make_devnode = false; } - /* We're about to fall back to bind-mounting the device - * node. So create a dummy bind-mount target. + /* We're about to fall back to bind-mounting the device node. So create a dummy bind-mount target. * Do not prepare device-node SELinux label (see issue 13762) */ r = mknod(dn, S_IFREG, 0); if (r < 0 && errno != EEXIST) @@ -853,13 +852,10 @@ static int mount_private_dev(MountEntry *m) { char temporary_mount[] = "/tmp/namespace-dev-XXXXXX"; const char *d, *dev = NULL, *devpts = NULL, *devshm = NULL, *devhugepages = NULL, *devmqueue = NULL, *devlog = NULL, *devptmx = NULL; bool can_mknod = true; - _unused_ _cleanup_umask_ mode_t u; int r; assert(m); - u = umask(0000); - if (!mkdtemp(temporary_mount)) return log_debug_errno(errno, "Failed to create temporary directory '%s': %m", temporary_mount); @@ -930,10 +926,8 @@ static int mount_private_dev(MountEntry *m) { if (r < 0) log_debug_errno(r, "Failed to set up basic device tree at '%s', ignoring: %m", temporary_mount); - /* Create the /dev directory if missing. It is more likely to be - * missing when the service is started with RootDirectory. This is - * consistent with mount units creating the mount points when missing. - */ + /* Create the /dev directory if missing. It is more likely to be missing when the service is started + * with RootDirectory. This is consistent with mount units creating the mount points when missing. */ (void) mkdir_p_label(mount_entry_path(m), 0755); /* Unmount everything in old /dev */ @@ -975,8 +969,8 @@ static int mount_bind_dev(const MountEntry *m) { assert(m); - /* Implements the little brother of mount_private_dev(): simply bind mounts the host's /dev into the service's - * /dev. This is only used when RootDirectory= is set. */ + /* Implements the little brother of mount_private_dev(): simply bind mounts the host's /dev into the + * service's /dev. This is only used when RootDirectory= is set. */ (void) mkdir_p_label(mount_entry_path(m), 0755); @@ -1085,7 +1079,8 @@ static int mount_tmpfs(const MountEntry *m) { entry_path = mount_entry_path(m); inner_path = mount_entry_unprefixed_path(m); - /* First, get rid of everything that is below if there is anything. Then, overmount with our new tmpfs */ + /* First, get rid of everything that is below if there is anything. Then, overmount with our new + * tmpfs */ (void) mkdir_p_label(entry_path, 0755); (void) umount_recursive(entry_path, 0); @@ -1900,6 +1895,10 @@ int setup_namespace( assert(ns_info); + /* Make sure that all mknod(), mkdir() calls we do are unaffected by the umask, and the access modes + * we configure take effect */ + BLOCK_WITH_UMASK(0000); + if (!isempty(propagate_dir) && !isempty(incoming_dir)) setup_propagate = true; @@ -1972,11 +1971,11 @@ int setup_namespace( * we create it if it doesn't already exist. */ (void) mkdir_p_label("/run/systemd", 0755); - /* Always create the mount namespace in a temporary directory, instead of operating - * directly in the root. The temporary directory prevents any mounts from being - * potentially obscured my other mounts we already applied. - * We use the same mount point for all images, which is safe, since they all live - * in their own namespaces after all, and hence won't see each other. */ + /* Always create the mount namespace in a temporary directory, instead of operating directly + * in the root. The temporary directory prevents any mounts from being potentially obscured + * my other mounts we already applied. We use the same mount point for all images, which is + * safe, since they all live in their own namespaces after all, and hence won't see each + * other. */ root = "/run/systemd/unit-root"; (void) mkdir_label(root, 0700); @@ -2240,8 +2239,8 @@ int setup_namespace( (void) mkdir_p(propagate_dir, 0600); if (n_extension_images > 0) - /* ExtensionImages mountpoint directories will be created - * while parsing the mounts to create, so have the parent ready */ + /* ExtensionImages mountpoint directories will be created while parsing the mounts to create, + * so have the parent ready */ (void) mkdir_p(extension_dir, 0600); /* Remount / as SLAVE so that nothing now mounted in the namespace @@ -2509,7 +2508,8 @@ static int make_tmp_prefix(const char *prefix) { if (errno != ENOENT) return -errno; - r = mkdir_parents(prefix, 0755); + RUN_WITH_UMASK(000) + r = mkdir_parents(prefix, 0755); if (r < 0) return r; @@ -2517,7 +2517,8 @@ static int make_tmp_prefix(const char *prefix) { if (r < 0) return r; - if (mkdir(t, 0777) < 0) + if (mkdir(t, 0777) < 0) /* umask will corrupt this access mode, but that doesn't matter, we need to + * call chmod() anyway for the suid bit, below. */ return -errno; if (chmod(t, 01777) < 0) { @@ -2575,10 +2576,9 @@ static int setup_one_tmp_dir(const char *id, const char *prefix, char **path, ch if (!y) return -ENOMEM; - RUN_WITH_UMASK(0000) { + RUN_WITH_UMASK(0000) if (mkdir(y, 0777 | S_ISVTX) < 0) return -errno; - } r = label_fix_container(y, prefix, 0); if (r < 0) @@ -2590,7 +2590,8 @@ static int setup_one_tmp_dir(const char *id, const char *prefix, char **path, ch /* Trouble: we failed to create the directory. Instead of failing, let's simulate /tmp being * read-only. This way the service will get the EROFS result as if it was writing to the real * file system. */ - r = mkdir_p(RUN_SYSTEMD_EMPTY, 0500); + RUN_WITH_UMASK(0000) + r = mkdir_p(RUN_SYSTEMD_EMPTY, 0500); if (r < 0) return r; diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c index 66daeb4f64..b85b8c3d43 100644 --- a/src/nspawn/nspawn.c +++ b/src/nspawn/nspawn.c @@ -2216,13 +2216,12 @@ static int copy_devnodes(const char *dest) { "tty\0" "net/tun\0"; - _unused_ _cleanup_umask_ mode_t u; const char *d; int r = 0; assert(dest); - u = umask(0000); + BLOCK_WITH_UMASK(0000); /* Create /dev/net, so that we can create /dev/net/tun in it */ if (userns_mkdir(dest, "/dev/net", 0755, 0, 0) < 0) @@ -2299,11 +2298,10 @@ static int copy_devnodes(const char *dest) { } static int make_extra_nodes(const char *dest) { - _unused_ _cleanup_umask_ mode_t u; size_t i; int r; - u = umask(0000); + BLOCK_WITH_UMASK(0000); for (i = 0; i < arg_n_extra_nodes; i++) { _cleanup_free_ char *path = NULL; @@ -2500,12 +2498,11 @@ static int setup_kmsg(int kmsg_socket) { _cleanup_(unlink_and_freep) char *from = NULL; _cleanup_free_ char *fifo = NULL; _cleanup_close_ int fd = -1; - _unused_ _cleanup_umask_ mode_t u; int r; assert(kmsg_socket >= 0); - u = umask(0000); + BLOCK_WITH_UMASK(0000); /* We create the kmsg FIFO as as temporary file in /run, but immediately delete it after bind mounting it to * /proc/kmsg. While FIFOs on the reading side behave very similar to /proc/kmsg, their writing side behaves diff --git a/src/shared/dev-setup.c b/src/shared/dev-setup.c index c3e717ae11..0390abbfdc 100644 --- a/src/shared/dev-setup.c +++ b/src/shared/dev-setup.c @@ -81,13 +81,12 @@ int make_inaccessible_nodes( { "inaccessible/blk", S_IFBLK | 0000 }, }; - _unused_ _cleanup_umask_ mode_t u; int r; if (!parent_dir) parent_dir = "/run/systemd"; - u = umask(0000); + BLOCK_WITH_UMASK(0000); /* Set up inaccessible (and empty) file nodes of all types. This are used to as mount sources for over-mounting * ("masking") file nodes that shall become inaccessible and empty for specific containers or services. We try diff --git a/src/test/test-execute.c b/src/test/test-execute.c index c81842a0d0..5818adbb55 100644 --- a/src/test/test-execute.c +++ b/src/test/test-execute.c @@ -1109,6 +1109,10 @@ static void test_exec_condition(Manager *m) { test_service(m, "exec-condition-skip.service", SERVICE_SKIP_CONDITION); } +static void test_exec_umask_namespace(Manager *m) { + test(m, "exec-umask-namespace.service", can_unshare ? 0 : EXIT_NAMESPACE, CLD_EXITED); +} + typedef struct test_entry { test_function_t f; const char *name; @@ -1191,6 +1195,7 @@ int main(int argc, char *argv[]) { entry(test_exec_specifier), entry(test_exec_execsearchpath_specifier), entry(test_exec_systemcallfilter_system), + entry(test_exec_umask_namespace), {}, }; int r; diff --git a/src/test/test-fs-util.c b/src/test/test-fs-util.c index 41ddec4783..d8273bc846 100644 --- a/src/test/test-fs-util.c +++ b/src/test/test-fs-util.c @@ -765,7 +765,6 @@ static void test_rename_noreplace(void) { static void test_chmod_and_chown(void) { _cleanup_(rm_rf_physical_and_freep) char *d = NULL; - _unused_ _cleanup_umask_ mode_t u = umask(0000); struct stat st; const char *p; @@ -774,6 +773,8 @@ static void test_chmod_and_chown(void) { log_info("/* %s */", __func__); + BLOCK_WITH_UMASK(0000); + assert_se(mkdtemp_malloc(NULL, &d) >= 0); p = strjoina(d, "/reg"); diff --git a/test/test-execute/exec-umask-namespace.service b/test/test-execute/exec-umask-namespace.service new file mode 100644 index 0000000000..8419c86c9a --- /dev/null +++ b/test/test-execute/exec-umask-namespace.service @@ -0,0 +1,12 @@ +# SPDX-License-Identifier: LGPL-2.1-or-later +[Unit] +Description=Test for UMask= + namespacing + +[Service] +ExecStart=/bin/ls -lahd /tmp/subdir +Type=oneshot +User=65534 +Group=65534 +TemporaryFileSystem=/tmp:ro +BindPaths=/etc:/tmp/subdir/subsub +UMask=0007 |