From 49fb6e97d2ffeb026829e6e6d780816abea407ae Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Mon, 23 Jan 2023 14:41:33 +0100 Subject: [PATCH 1/3] Revert "repart: Make sure all files in the image are owned by root" This reverts commit d2ac7698cb43807a2dd0af727599db486180ebf1. --- src/partition/repart.c | 26 +++++++++----------------- 1 file changed, 9 insertions(+), 17 deletions(-) diff --git a/src/partition/repart.c b/src/partition/repart.c index 7c4a1ee01c..12ed0b02f5 100644 --- a/src/partition/repart.c +++ b/src/partition/repart.c @@ -3744,12 +3744,7 @@ static int context_copy_blocks(Context *context) { return 0; } -static int do_copy_files( - Partition *p, - const char *root, - uid_t override_uid, - gid_t override_gid, - const Set *denylist) { +static int do_copy_files(Partition *p, const char *root, const Set *denylist) { int r; @@ -3799,14 +3794,14 @@ static int do_copy_files( r = copy_tree_at( sfd, ".", pfd, fn, - override_uid, override_gid, + getuid(), getgid(), COPY_REFLINK|COPY_HOLES|COPY_MERGE|COPY_REPLACE|COPY_SIGINT|COPY_HARDLINKS|COPY_ALL_XATTRS|COPY_GRACEFUL_WARN, denylist); } else r = copy_tree_at( sfd, ".", tfd, ".", - override_uid, override_gid, + getuid(), getgid(), COPY_REFLINK|COPY_HOLES|COPY_MERGE|COPY_REPLACE|COPY_SIGINT|COPY_HARDLINKS|COPY_ALL_XATTRS|COPY_GRACEFUL_WARN, denylist); if (r < 0) @@ -3844,9 +3839,6 @@ static int do_copy_files( if (r < 0) return log_error_errno(r, "Failed to copy '%s' to '%s%s': %m", *source, strempty(arg_root), *target); - if (fchown(tfd, override_uid, override_gid) < 0) - return log_error_errno(r, "Failed to change ownership of %s", *target); - (void) copy_xattr(sfd, tfd, COPY_ALL_XATTRS); (void) copy_access(sfd, tfd); (void) copy_times(sfd, tfd, 0); @@ -3856,7 +3848,7 @@ static int do_copy_files( return 0; } -static int do_make_directories(Partition *p, uid_t override_uid, gid_t override_gid, const char *root) { +static int do_make_directories(Partition *p, const char *root) { int r; assert(p); @@ -3864,7 +3856,7 @@ static int do_make_directories(Partition *p, uid_t override_uid, gid_t override_ STRV_FOREACH(d, p->make_directories) { - r = mkdir_p_root(root, *d, override_uid, override_gid, 0755); + r = mkdir_p_root(root, *d, getuid(), getgid(), 0755); if (r < 0) return log_error_errno(r, "Failed to create directory '%s' in file system: %m", *d); } @@ -3891,11 +3883,11 @@ static int partition_populate_directory(Partition *p, const Set *denylist, char if (fchmod(rfd, 0755) < 0) return log_error_errno(errno, "Failed to change mode of temporary directory: %m"); - r = do_copy_files(p, root, getuid(), getgid(), denylist); + r = do_copy_files(p, root, denylist); if (r < 0) return r; - r = do_make_directories(p, getuid(), getgid(), root); + r = do_make_directories(p, root); if (r < 0) return r; @@ -3931,10 +3923,10 @@ static int partition_populate_filesystem(Partition *p, const char *node, const S if (mount_nofollow_verbose(LOG_ERR, node, fs, p->format, MS_NOATIME|MS_NODEV|MS_NOEXEC|MS_NOSUID, NULL) < 0) _exit(EXIT_FAILURE); - if (do_copy_files(p, fs, 0, 0, denylist) < 0) + if (do_copy_files(p, fs, denylist) < 0) _exit(EXIT_FAILURE); - if (do_make_directories(p, 0, 0, fs) < 0) + if (do_make_directories(p, fs) < 0) _exit(EXIT_FAILURE); r = syncfs_path(AT_FDCWD, fs); From ff1b55ffdf7ba7294e9e9d00393acbac426295c9 Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Mon, 23 Jan 2023 14:47:00 +0100 Subject: [PATCH 2/3] Revert "repart: Ensure files end up owned by root in generated filesystems" This reverts commit e59678b2cf42e4206ddabc959d3cf9a5a865ecdc. We also modify the repart integration tests to make them pass with the changes in this commit. In short, we have to make sure every file is owned by the user executing repart. We use tee instead of cat since it makes that easier. This also has the benefit of improving debugability as seeing the config file contents on stdout makes it easier to know which test is failing. --- src/partition/repart.c | 7 ++-- src/shared/mkfs-util.c | 51 +---------------------- test/units/testsuite-58.sh | 84 +++++++++++++++++++------------------- 3 files changed, 47 insertions(+), 95 deletions(-) diff --git a/src/partition/repart.c b/src/partition/repart.c index 12ed0b02f5..c95b1d601d 100644 --- a/src/partition/repart.c +++ b/src/partition/repart.c @@ -3745,7 +3745,6 @@ static int context_copy_blocks(Context *context) { } static int do_copy_files(Partition *p, const char *root, const Set *denylist) { - int r; assert(p); @@ -3794,14 +3793,14 @@ static int do_copy_files(Partition *p, const char *root, const Set *denylist) { r = copy_tree_at( sfd, ".", pfd, fn, - getuid(), getgid(), + UID_INVALID, GID_INVALID, COPY_REFLINK|COPY_HOLES|COPY_MERGE|COPY_REPLACE|COPY_SIGINT|COPY_HARDLINKS|COPY_ALL_XATTRS|COPY_GRACEFUL_WARN, denylist); } else r = copy_tree_at( sfd, ".", tfd, ".", - getuid(), getgid(), + UID_INVALID, GID_INVALID, COPY_REFLINK|COPY_HOLES|COPY_MERGE|COPY_REPLACE|COPY_SIGINT|COPY_HARDLINKS|COPY_ALL_XATTRS|COPY_GRACEFUL_WARN, denylist); if (r < 0) @@ -3856,7 +3855,7 @@ static int do_make_directories(Partition *p, const char *root) { STRV_FOREACH(d, p->make_directories) { - r = mkdir_p_root(root, *d, getuid(), getgid(), 0755); + r = mkdir_p_root(root, *d, UID_INVALID, GID_INVALID, 0755); if (r < 0) return log_error_errno(r, "Failed to create directory '%s' in file system: %m", *d); } diff --git a/src/shared/mkfs-util.c b/src/shared/mkfs-util.c index 11ae92290d..d64ef0d47a 100644 --- a/src/shared/mkfs-util.c +++ b/src/shared/mkfs-util.c @@ -98,41 +98,11 @@ static int mangle_fat_label(const char *s, char **ret) { return 0; } -static int setup_userns(uid_t uid, gid_t gid) { - int r; - - /* mkfs programs tend to keep ownership intact when bootstrapping themselves from a root directory. - * However, we'd like for the files to be owned by root instead, so we fork off a user namespace and - * inside of it, map the uid/gid of the root directory to root in the user namespace. mkfs programs - * will pick up on this and the files will be owned by root in the generated filesystem. */ - - r = write_string_filef("/proc/self/uid_map", WRITE_STRING_FILE_DISABLE_BUFFER, - UID_FMT " " UID_FMT " " UID_FMT, 0u, uid, 1u); - if (r < 0) - return log_error_errno(r, - "Failed to write mapping for "UID_FMT" to /proc/self/uid_map: %m", - uid); - - r = write_string_file("/proc/self/setgroups", "deny", WRITE_STRING_FILE_DISABLE_BUFFER); - if (r < 0) - return log_error_errno(r, "Failed to write 'deny' to /proc/self/setgroups: %m"); - - r = write_string_filef("/proc/self/gid_map", WRITE_STRING_FILE_DISABLE_BUFFER, - GID_FMT " " GID_FMT " " GID_FMT, 0u, gid, 1u); - if (r < 0) - return log_error_errno(r, - "Failed to write mapping for "GID_FMT" to /proc/self/gid_map: %m", - gid); - - return 0; -} - static int do_mcopy(const char *node, const char *root) { _cleanup_free_ char *mcopy = NULL; _cleanup_strv_free_ char **argv = NULL; _cleanup_close_ int rfd = -EBADF; _cleanup_free_ DirectoryEntries *de = NULL; - struct stat st; int r; assert(node); @@ -182,17 +152,10 @@ static int do_mcopy(const char *node, const char *root) { if (strv_extend(&argv, "::") < 0) return log_oom(); - if (fstat(rfd, &st) < 0) - return log_error_errno(errno, "Failed to stat '%s': %m", root); - - r = safe_fork("(mcopy)", FORK_RESET_SIGNALS|FORK_RLIMIT_NOFILE_SAFE|FORK_DEATHSIG|FORK_LOG|FORK_WAIT|FORK_STDOUT_TO_STDERR|FORK_NEW_USERNS|FORK_CLOSE_ALL_FDS, NULL); + r = safe_fork("(mcopy)", FORK_RESET_SIGNALS|FORK_RLIMIT_NOFILE_SAFE|FORK_DEATHSIG|FORK_LOG|FORK_WAIT|FORK_STDOUT_TO_STDERR|FORK_CLOSE_ALL_FDS, NULL); if (r < 0) return r; if (r == 0) { - r = setup_userns(st.st_uid, st.st_gid); - if (r < 0) - _exit(EXIT_FAILURE); - /* Avoid failures caused by mismatch in expectations between mkfs.vfat and mcopy by disabling * the stricter mcopy checks using MTOOLS_SKIP_CHECK. */ execve(mcopy, argv, STRV_MAKE("MTOOLS_SKIP_CHECK=1")); @@ -308,7 +271,6 @@ int make_filesystem( _cleanup_strv_free_ char **argv = NULL; _cleanup_(unlink_and_freep) char *protofile = NULL; char vol_id[CONST_MAX(SD_ID128_UUID_STRING_MAX, 8U + 1U)] = {}; - struct stat st; int r; assert(node); @@ -527,21 +489,12 @@ int make_filesystem( if (extra_mkfs_args && strv_extend_strv(&argv, extra_mkfs_args, false) < 0) return log_oom(); - if (root && stat(root, &st) < 0) - return log_error_errno(errno, "Failed to stat %s: %m", root); - - r = safe_fork("(mkfs)", FORK_RESET_SIGNALS|FORK_RLIMIT_NOFILE_SAFE|FORK_DEATHSIG|FORK_LOG|FORK_WAIT|FORK_STDOUT_TO_STDERR|FORK_CLOSE_ALL_FDS|(root ? FORK_NEW_USERNS : 0), NULL); + r = safe_fork("(mkfs)", FORK_RESET_SIGNALS|FORK_RLIMIT_NOFILE_SAFE|FORK_DEATHSIG|FORK_LOG|FORK_WAIT|FORK_STDOUT_TO_STDERR|FORK_CLOSE_ALL_FDS, NULL); if (r < 0) return r; if (r == 0) { /* Child */ - if (root) { - r = setup_userns(st.st_uid, st.st_gid); - if (r < 0) - _exit(EXIT_FAILURE); - } - execvp(mkfs, argv); log_error_errno(errno, "Failed to execute %s: %m", mkfs); diff --git a/test/units/testsuite-58.sh b/test/units/testsuite-58.sh index cf1007f69a..e83df97067 100755 --- a/test/units/testsuite-58.sh +++ b/test/units/testsuite-58.sh @@ -119,21 +119,21 @@ last-lba: 2097118" # 2. Testing with root, root2, home, and swap - cat >"$defs/root.conf" <"$defs/home.conf" <"$defs/swap.conf" <"$defs/swap.conf" <"$defs/extra.conf" <"$defs/extra2.conf" <"$defs/extra3.conf" <"$defs/root.conf" <"$defs/root.conf.d/override1.conf" <"$defs/root.conf.d/override2.conf" <"$defs/1/root1.conf" <"$defs/2/root2.conf" <"$defs/esp.conf" <"$defs/usr.conf" <"$defs/root.conf" <"$defs/esp.conf" <"$defs/usr.conf" <"$defs/root.conf" <"$defs/root.conf" <"$defs/test.conf" <"$defs/root.conf" <"$imgs/partscript" <"$defs/root.conf" <"$defs/usr.conf" <"$defs/root.conf" <"$defs/verity-data.conf" <"$defs/verity-hash.conf" <"$defs/verity-sig.conf" <> "$defs/verity.openssl.cnf" < "$defs/verity.openssl.cnf" <"$defs/00-root.conf" <"$defs/10-usr.conf" <"$defs/root-$format.conf" </dev/null; then - cat >"$defs/root-squashfs.conf" < "$defs/a.conf" < "$defs/b.conf" < "$defs/c.conf" < Date: Mon, 23 Jan 2023 16:43:58 +0100 Subject: [PATCH 3/3] repart: Add note about UIDs/GIDs of copied files and directories --- man/repart.d.xml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/man/repart.d.xml b/man/repart.d.xml index 846e1984e5..b786202905 100644 --- a/man/repart.d.xml +++ b/man/repart.d.xml @@ -425,6 +425,12 @@ target filesystem (e.g symlinks, fifos, sockets and devices on vfat). When an unsupported file type is encountered, repart will skip copying this file and write a log message about it. + Note that systemd-repart does not change the UIDs/GIDs of any copied files + and directories. When running systemd-repart as an unprivileged user to build an + image of files and directories owned by the same user, you can run systemd-repart + in a user namespace with the current user mapped to the root user to make sure the files and + directories in the image are owned by the root user. + This option cannot be combined with CopyBlocks=. When