diff --git a/src/basic/user-util.h b/src/basic/user-util.h index 40979d1080..e1692c4f66 100644 --- a/src/basic/user-util.h +++ b/src/basic/user-util.h @@ -67,6 +67,19 @@ int take_etc_passwd_lock(const char *root); #define UID_NOBODY ((uid_t) 65534U) #define GID_NOBODY ((gid_t) 65534U) +/* If REMOUNT_IDMAP_HOST_ROOT is set for remount_idmap() we'll include a mapping here that maps the host root + * user accessing the idmapped mount to the this user ID on the backing fs. This is the last valid UID in the + * *signed* 32bit range. You might wonder why precisely use this specific UID for this purpose? Well, we + * definitely cannot use the first 0…65536 UIDs for that, since in most cases that's precisely the file range + * we intend to map to some high UID range, and since UID mappings have to be bijective we thus cannot use + * them at all. Furthermore the UID range beyond INT32_MAX (i.e. the range above the signed 32bit range) is + * icky, since many APIs cannot use it (example: setfsuid() returns the old UID as signed integer). Following + * our usual logic of assigning a 16bit UID range to each container, so that the upper 16bit of a 32bit UID + * value indicate kind of a "container ID" and the lower 16bit map directly to the intended user you can read + * this specific UID as the "nobody" user of the container with ID 0x7FFF, which is kinda nice. */ +#define UID_MAPPED_ROOT ((uid_t) (INT32_MAX-1)) +#define GID_MAPPED_ROOT ((gid_t) (INT32_MAX-1)) + #define ETC_PASSWD_LOCK_PATH "/etc/.pwd.lock" /* The following macros add 1 when converting things, since UID 0 is a valid UID, while the pointer diff --git a/src/home/homework-mount.c b/src/home/homework-mount.c index 35645e292e..3095a10ff9 100644 --- a/src/home/homework-mount.c +++ b/src/home/homework-mount.c @@ -216,6 +216,12 @@ static int make_userns(uid_t stored_uid, uid_t exposed_uid) { if (r < 0) return log_oom(); + /* Map nspawn's mapped root UID as identity mapping so that people can run nspawn uidmap mounted + * containers off $HOME, if they want. */ + r = strextendf(&text, UID_FMT " " UID_FMT " " UID_FMT "\n", UID_MAPPED_ROOT, UID_MAPPED_ROOT, 1); + if (r < 0) + return log_oom(); + /* Leave everything else unmapped, starting from UID_NOBODY itself. Specifically, this means the * whole space outside of 16bit remains unmapped */ diff --git a/src/nspawn/nspawn-mount.c b/src/nspawn/nspawn-mount.c index 40773d90c1..f2fad0f462 100644 --- a/src/nspawn/nspawn-mount.c +++ b/src/nspawn/nspawn-mount.c @@ -780,7 +780,7 @@ static int mount_bind(const char *dest, CustomMount *m, uid_t uid_shift, uid_t u } if (idmapped) { - r = remount_idmap(where, uid_shift, uid_range); + r = remount_idmap(where, uid_shift, uid_range, REMOUNT_IDMAP_HOST_ROOT); if (r < 0) return log_error_errno(r, "Failed to map ids for bind mount %s: %m", where); } diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c index 5102c16438..7fe061899c 100644 --- a/src/nspawn/nspawn.c +++ b/src/nspawn/nspawn.c @@ -3563,7 +3563,7 @@ static int inner_child( static int setup_notify_child(void) { _cleanup_close_ int fd = -1; - union sockaddr_union sa = { + static const union sockaddr_union sa = { .un.sun_family = AF_UNIX, .un.sun_path = NSPAWN_NOTIFY_SOCKET_PATH, }; @@ -3616,10 +3616,11 @@ static int outer_child( ssize_t l; int r; - /* This is the "outer" child process, i.e the one forked off by the container manager itself. It already has - * its own CLONE_NEWNS namespace (which was created by the clone()). It still lives in the host's CLONE_NEWPID, - * CLONE_NEWUTS, CLONE_NEWIPC, CLONE_NEWUSER and CLONE_NEWNET namespaces. After it completed a number of - * initializations a second child (the "inner" one) is forked off it, and it exits. */ + /* This is the "outer" child process, i.e the one forked off by the container manager itself. It + * already has its own CLONE_NEWNS namespace (which was created by the clone()). It still lives in + * the host's CLONE_NEWPID, CLONE_NEWUTS, CLONE_NEWIPC, CLONE_NEWUSER and CLONE_NEWNET + * namespaces. After it completed a number of initializations a second child (the "inner" one) is + * forked off it, and it exits. */ assert(barrier); assert(directory); @@ -3649,10 +3650,10 @@ static int outer_child( return r; if (dissected_image) { - /* If we are operating on a disk image, then mount its root directory now, but leave out the rest. We - * can read the UID shift from it if we need to. Further down we'll mount the rest, but then with the - * uid shift known. That way we can mount VFAT file systems shifted to the right place right away. This - * makes sure ESP partitions and userns are compatible. */ + /* If we are operating on a disk image, then mount its root directory now, but leave out the + * rest. We can read the UID shift from it if we need to. Further down we'll mount the rest, + * but then with the uid shift known. That way we can mount VFAT file systems shifted to the + * right place right away. This makes sure ESP partitions and userns are compatible. */ r = dissected_image_mount_and_warn( dissected_image, @@ -3682,9 +3683,9 @@ static int outer_child( "Short write while sending UID shift."); if (arg_userns_mode == USER_NAMESPACE_PICK) { - /* When we are supposed to pick the UID shift, the parent will check now whether the UID shift - * we just read from the image is available. If yes, it will send the UID shift back to us, if - * not it will pick a different one, and send it back to us. */ + /* When we are supposed to pick the UID shift, the parent will check now whether the + * UID shift we just read from the image is available. If yes, it will send the UID + * shift back to us, if not it will pick a different one, and send it back to us. */ l = recv(uid_shift_socket, &arg_uid_shift, sizeof(arg_uid_shift), 0); if (l < 0) @@ -3740,7 +3741,8 @@ static int outer_child( return r; if (arg_userns_mode != USER_NAMESPACE_NO && bind_user_context) { - /* Send the user maps we determined to the parent, so that it installs it in our user namespace UID map table */ + /* Send the user maps we determined to the parent, so that it installs it in our user + * namespace UID map table */ for (size_t i = 0; i < bind_user_context->n_data; i++) { uid_t map[] = { @@ -3779,7 +3781,7 @@ static int outer_child( IN_SET(arg_userns_ownership, USER_NAMESPACE_OWNERSHIP_MAP, USER_NAMESPACE_OWNERSHIP_AUTO) && arg_uid_shift != 0) { - r = remount_idmap(directory, arg_uid_shift, arg_uid_range); + r = remount_idmap(directory, arg_uid_shift, arg_uid_range, REMOUNT_IDMAP_HOST_ROOT); if (r == -EINVAL || ERRNO_IS_NOT_SUPPORTED(r)) { /* This might fail because the kernel or file system doesn't support idmapping. We * can't really distinguish this nicely, nor do we have any guarantees about the @@ -3833,15 +3835,13 @@ static int outer_child( unified_cgroup_hierarchy_socket = safe_close(unified_cgroup_hierarchy_socket); } - /* Mark everything as shared so our mounts get propagated down. This is - * required to make new bind mounts available in systemd services - * inside the container that create a new mount namespace. - * See https://github.com/systemd/systemd/issues/3860 - * Further submounts (such as /dev) done after this will inherit the - * shared propagation mode. + /* Mark everything as shared so our mounts get propagated down. This is required to make new bind + * mounts available in systemd services inside the container that create a new mount namespace. See + * https://github.com/systemd/systemd/issues/3860 Further submounts (such as /dev) done after this + * will inherit the shared propagation mode. * - * IMPORTANT: Do not overmount the root directory anymore from now on to - * enable moving the root directory mount to root later on. + * IMPORTANT: Do not overmount the root directory anymore from now on to enable moving the root + * directory mount to root later on. * https://github.com/systemd/systemd/issues/3847#issuecomment-562735251 */ r = mount_nofollow_verbose(LOG_ERR, NULL, directory, NULL, MS_SHARED|MS_REC, NULL); @@ -5612,31 +5612,36 @@ static int run(int argc, char *argv[]) { } if (arg_start_mode == START_BOOT) { + _cleanup_free_ char *b = NULL; const char *p; - if (arg_pivot_root_new) - p = prefix_roota(arg_directory, arg_pivot_root_new); - else + if (arg_pivot_root_new) { + b = path_join(arg_directory, arg_pivot_root_new); + if (!b) + return log_oom(); + + p = b; + } else p = arg_directory; if (path_is_os_tree(p) <= 0) { - log_error("Directory %s doesn't look like an OS root directory (os-release file is missing). Refusing.", p); - r = -EINVAL; + r = log_error_errno(SYNTHETIC_ERRNO(EINVAL), + "Directory %s doesn't look like an OS root directory (os-release file is missing). Refusing.", p); goto finish; } } else { - const char *p, *q; + _cleanup_free_ char *p = NULL; if (arg_pivot_root_new) - p = prefix_roota(arg_directory, arg_pivot_root_new); + p = path_join(arg_directory, arg_pivot_root_new, "/usr/"); else - p = arg_directory; + p = path_join(arg_directory, "/usr/"); + if (!p) + return log_oom(); - q = strjoina(p, "/usr/"); - - if (laccess(q, F_OK) < 0) { - log_error("Directory %s doesn't look like it has an OS tree. Refusing.", p); - r = -EINVAL; + if (laccess(p, F_OK) < 0) { + r = log_error_errno(SYNTHETIC_ERRNO(EINVAL), + "Directory %s doesn't look like it has an OS tree (/usr/ directory is missing). Refusing.", arg_directory); goto finish; } } diff --git a/src/shared/base-filesystem.c b/src/shared/base-filesystem.c index 5f5328c8cf..3a77f37a69 100644 --- a/src/shared/base-filesystem.c +++ b/src/shared/base-filesystem.c @@ -176,7 +176,7 @@ int base_filesystem_create(const char *root, uid_t uid, gid_t gid) { return -errno; } - if (uid != UID_INVALID || gid != UID_INVALID) + if (uid_is_valid(uid) || gid_is_valid(gid)) if (fchownat(fd, table[i].dir, uid, gid, AT_SYMLINK_NOFOLLOW) < 0) return log_error_errno(errno, "Failed to chown directory at %s/%s: %m", root, table[i].dir); } diff --git a/src/shared/dissect-image.c b/src/shared/dissect-image.c index 14519ead70..14361a0e5a 100644 --- a/src/shared/dissect-image.c +++ b/src/shared/dissect-image.c @@ -1807,7 +1807,7 @@ static int mount_partition( (void) fs_grow(node, p); if (remap_uid_gid) { - r = remount_idmap(p, uid_shift, uid_range); + r = remount_idmap(p, uid_shift, uid_range, REMOUNT_IDMAP_HOST_ROOT); if (r < 0) return r; } diff --git a/src/shared/mount-util.c b/src/shared/mount-util.c index 7f88a982bd..ae1f2f0b80 100644 --- a/src/shared/mount-util.c +++ b/src/shared/mount-util.c @@ -1049,14 +1049,31 @@ int make_mount_point(const char *path) { return 1; } -static int make_userns(uid_t uid_shift, uid_t uid_range) { - char line[DECIMAL_STR_MAX(uid_t)*3+3+1]; +static int make_userns(uid_t uid_shift, uid_t uid_range, RemountIdmapFlags flags) { _cleanup_close_ int userns_fd = -1; + _cleanup_free_ char *line = NULL; /* Allocates a userns file descriptor with the mapping we need. For this we'll fork off a child * process whose only purpose is to give us a new user namespace. It's killed when we got it. */ - xsprintf(line, UID_FMT " " UID_FMT " " UID_FMT "\n", 0, uid_shift, uid_range); + if (asprintf(&line, UID_FMT " " UID_FMT " " UID_FMT "\n", 0, uid_shift, uid_range) < 0) + return log_oom_debug(); + + /* If requested we'll include an entry in the mapping so that the host root user can make changes to + * the uidmapped mount like it normally would. Specifically, we'll map the user with UID_HOST_ROOT on + * the backing fs to UID 0. This is useful, since nspawn code wants to create various missing inodes + * in the OS tree before booting into it, and this becomes very easy and straightforward to do if it + * can just do it under its own regular UID. Note that in that case the container's runtime uidmap + * (i.e. the one the container payload processes run in) will leave this UID unmapped, i.e. if we + * accidentally leave files owned by host root in the already uidmapped tree around they'll show up + * as owned by 'nobody', which is safe. (Of course, we shouldn't leave such inodes around, but always + * chown() them to the container's own UID range, but it's good to have a safety net, in case we + * forget it.) */ + if (flags & REMOUNT_IDMAP_HOST_ROOT) + if (strextendf(&line, + UID_FMT " " UID_FMT " " UID_FMT "\n", + UID_MAPPED_ROOT, 0, 1) < 0) + return log_oom_debug(); /* We always assign the same UID and GID ranges */ userns_fd = userns_acquire(line, line); @@ -1069,7 +1086,8 @@ static int make_userns(uid_t uid_shift, uid_t uid_range) { int remount_idmap( const char *p, uid_t uid_shift, - uid_t uid_range) { + uid_t uid_range, + RemountIdmapFlags flags) { _cleanup_close_ int mount_fd = -1, userns_fd = -1; int r; @@ -1085,7 +1103,7 @@ int remount_idmap( return log_debug_errno(errno, "Failed to open tree of mounted filesystem '%s': %m", p); /* Create a user namespace mapping */ - userns_fd = make_userns(uid_shift, uid_range); + userns_fd = make_userns(uid_shift, uid_range, flags); if (userns_fd < 0) return userns_fd; diff --git a/src/shared/mount-util.h b/src/shared/mount-util.h index 9caf92308a..960d586ba9 100644 --- a/src/shared/mount-util.h +++ b/src/shared/mount-util.h @@ -112,7 +112,18 @@ int mount_image_in_namespace(pid_t target, const char *propagate_path, const cha int make_mount_point(const char *path); -int remount_idmap(const char *p, uid_t uid_shift, uid_t uid_range); +typedef enum RemountIdmapFlags { + /* Include a mapping from UID_MAPPED_ROOT (i.e. UID 2^31-2) on the backing fs to UID 0 on the + * uidmapped fs. This is useful to ensure that the host root user can safely add inodes to the + * uidmapped fs (which otherwise wouldn't work as the host root user is not defined on the uidmapped + * mount and any attempts to create inodes will then be refused with EOVERFLOW). The idea is that + * these inodes are quickly re-chown()ed to more suitable UIDs/GIDs. Any code that intends to be able + * to add inodes to file systems mapped this way should set this flag, but given it comes with + * certain security implications defaults to off, and requires explicit opt-in. */ + REMOUNT_IDMAP_HOST_ROOT = 1 << 0, +} RemountIdmapFlags; + +int remount_idmap(const char *p, uid_t uid_shift, uid_t uid_range, RemountIdmapFlags flags); /* Creates a mount point (not parents) based on the source path or stat - ie, a file or a directory */ int make_mount_point_inode_from_stat(const struct stat *st, const char *dest, mode_t mode);