From 0b2aa2064f93cc154e704b262713de4fc395ce56 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 31 May 2023 09:32:52 +0200 Subject: [PATCH 1/2] find-esp: change "unprivileged_mode" parameter to be tristate Previously, unprivileged mode for find_esp_and_warn() and find_xbootldr_and_warn() could be enabled or disabled. With this change it can also be set to negative in which case the functions will enable it automatically if found to be executing without privileges. This just moves te geteuid() check we often do for the param inside of the functions. At the same time internally in the functions we also pass around the VerifyESPFlags field across the various functions instead of booleans. Both changes are just refactoring. No changes in behaviour. --- src/boot/bootctl-status.c | 8 +++--- src/boot/bootctl.c | 4 +-- src/boot/bootctl.h | 4 +-- src/shared/find-esp.c | 60 +++++++++++++++++++++++++-------------- src/shared/find-esp.h | 8 +++--- 5 files changed, 50 insertions(+), 34 deletions(-) diff --git a/src/boot/bootctl-status.c b/src/boot/bootctl-status.c index 3da64782590..b51bd2681e0 100644 --- a/src/boot/bootctl-status.c +++ b/src/boot/bootctl-status.c @@ -319,7 +319,7 @@ int verb_status(int argc, char *argv[], void *userdata) { dev_t esp_devid = 0, xbootldr_devid = 0; int r, k; - r = acquire_esp(/* unprivileged_mode= */ geteuid() != 0, /* graceful= */ false, NULL, NULL, NULL, &esp_uuid, &esp_devid); + r = acquire_esp(/* unprivileged_mode= */ -1, /* graceful= */ false, NULL, NULL, NULL, &esp_uuid, &esp_devid); if (arg_print_esp_path) { if (r == -EACCES) /* If we couldn't acquire the ESP path, log about access errors (which is the only * error the find_esp_and_warn() won't log on its own) */ @@ -330,7 +330,7 @@ int verb_status(int argc, char *argv[], void *userdata) { puts(arg_esp_path); } - r = acquire_xbootldr(/* unprivileged_mode= */ geteuid() != 0, &xbootldr_uuid, &xbootldr_devid); + r = acquire_xbootldr(/* unprivileged_mode= */ -1, &xbootldr_uuid, &xbootldr_devid); if (arg_print_dollar_boot_path) { if (r == -EACCES) return log_error_errno(r, "Failed to determine XBOOTLDR partition: %m"); @@ -770,13 +770,13 @@ int verb_list(int argc, char *argv[], void *userdata) { * Here we're interested in the latter but not the former, hence request the mode, and log about * EACCES. */ - r = acquire_esp(/* unprivileged_mode= */ geteuid() != 0, /* graceful= */ false, NULL, NULL, NULL, NULL, &esp_devid); + r = acquire_esp(/* unprivileged_mode= */ -1, /* graceful= */ false, NULL, NULL, NULL, NULL, &esp_devid); if (r == -EACCES) /* We really need the ESP path for this call, hence also log about access errors */ return log_error_errno(r, "Failed to determine ESP location: %m"); if (r < 0) return r; - r = acquire_xbootldr(/* unprivileged_mode= */ geteuid() != 0, NULL, &xbootldr_devid); + r = acquire_xbootldr(/* unprivileged_mode= */ -1, NULL, &xbootldr_devid); if (r == -EACCES) return log_error_errno(r, "Failed to determine XBOOTLDR partition: %m"); if (r < 0) diff --git a/src/boot/bootctl.c b/src/boot/bootctl.c index 65608f5e833..da55ad8e0d5 100644 --- a/src/boot/bootctl.c +++ b/src/boot/bootctl.c @@ -64,7 +64,7 @@ STATIC_DESTRUCTOR_REGISTER(arg_efi_boot_option_description, freep); STATIC_DESTRUCTOR_REGISTER(arg_image_policy, image_policy_freep); int acquire_esp( - bool unprivileged_mode, + int unprivileged_mode, bool graceful, uint32_t *ret_part, uint64_t *ret_pstart, @@ -101,7 +101,7 @@ int acquire_esp( } int acquire_xbootldr( - bool unprivileged_mode, + int unprivileged_mode, sd_id128_t *ret_uuid, dev_t *ret_devid) { diff --git a/src/boot/bootctl.h b/src/boot/bootctl.h index dd98b959c29..e395b3324ad 100644 --- a/src/boot/bootctl.h +++ b/src/boot/bootctl.h @@ -42,5 +42,5 @@ static inline const char *arg_dollar_boot_path(void) { return arg_xbootldr_path ?: arg_esp_path; } -int acquire_esp(bool unprivileged_mode, bool graceful, uint32_t *ret_part, uint64_t *ret_pstart, uint64_t *ret_psize, sd_id128_t *ret_uuid, dev_t *ret_devid); -int acquire_xbootldr(bool unprivileged_mode, sd_id128_t *ret_uuid, dev_t *ret_devid); +int acquire_esp(int unprivileged_mode, bool graceful, uint32_t *ret_part, uint64_t *ret_pstart, uint64_t *ret_psize, sd_id128_t *ret_uuid, dev_t *ret_devid); +int acquire_xbootldr(int unprivileged_mode, sd_id128_t *ret_uuid, dev_t *ret_devid); diff --git a/src/shared/find-esp.c b/src/shared/find-esp.c index c4cf5085171..7cd149b29b4 100644 --- a/src/shared/find-esp.c +++ b/src/shared/find-esp.c @@ -31,7 +31,7 @@ typedef enum VerifyESPFlags { static int verify_esp_blkid( dev_t devid, - bool searching, + VerifyESPFlags flags, uint32_t *ret_part, uint64_t *ret_pstart, uint64_t *ret_psize, @@ -44,6 +44,7 @@ static int verify_esp_blkid( #if HAVE_BLKID _cleanup_(blkid_free_probep) blkid_probe b = NULL; _cleanup_free_ char *node = NULL; + bool searching = FLAGS_SET(flags, VERIFY_ESP_SEARCHING); const char *v; int r; @@ -146,12 +147,13 @@ static int verify_esp_blkid( static int verify_esp_udev( dev_t devid, - bool searching, + VerifyESPFlags flags, uint32_t *ret_part, uint64_t *ret_pstart, uint64_t *ret_psize, sd_id128_t *ret_uuid) { + bool searching = FLAGS_SET(flags, VERIFY_ESP_SEARCHING); _cleanup_(sd_device_unrefp) sd_device *d = NULL; sd_id128_t uuid = SD_ID128_NULL; uint64_t pstart = 0, psize = 0; @@ -240,10 +242,11 @@ static int verify_esp_udev( static int verify_fsroot_dir( int dir_fd, const char *path, - bool searching, - bool unprivileged_mode, + VerifyESPFlags flags, dev_t *ret_dev) { + bool searching = FLAGS_SET(flags, VERIFY_ESP_SEARCHING), + unprivileged_mode = FLAGS_SET(flags, VERIFY_ESP_UNPRIVILEGED_MODE); _cleanup_free_ char *f = NULL; STRUCT_NEW_STATX_DEFINE(sxa); STRUCT_NEW_STATX_DEFINE(sxb); @@ -377,7 +380,7 @@ static int verify_esp( relax_checks || detect_container() > 0; - r = verify_fsroot_dir(pfd, p, searching, unprivileged_mode, relax_checks ? NULL : &devid); + r = verify_fsroot_dir(pfd, p, flags, relax_checks ? NULL : &devid); if (r < 0) return r; @@ -392,9 +395,9 @@ static int verify_esp( * however blkid can't work if we have no privileges to access block devices directly, which is why * we use udev in that case. */ if (unprivileged_mode) - r = verify_esp_udev(devid, searching, ret_part, ret_pstart, ret_psize, ret_uuid); + r = verify_esp_udev(devid, flags, ret_part, ret_pstart, ret_psize, ret_uuid); else - r = verify_esp_blkid(devid, searching, ret_part, ret_pstart, ret_psize, ret_uuid); + r = verify_esp_blkid(devid, flags, ret_part, ret_pstart, ret_psize, ret_uuid); if (r < 0) return r; @@ -425,7 +428,7 @@ finish: int find_esp_and_warn_at( int rfd, const char *path, - bool unprivileged_mode, + int unprivileged_mode, char **ret_path, uint32_t *ret_part, uint64_t *ret_pstart, @@ -433,7 +436,7 @@ int find_esp_and_warn_at( sd_id128_t *ret_uuid, dev_t *ret_devid) { - VerifyESPFlags flags = (unprivileged_mode ? VERIFY_ESP_UNPRIVILEGED_MODE : 0); + VerifyESPFlags flags; int r; /* This logs about all errors except: @@ -444,6 +447,10 @@ int find_esp_and_warn_at( assert(rfd >= 0 || rfd == AT_FDCWD); + if (unprivileged_mode < 0) + unprivileged_mode = geteuid() != 0; + flags = unprivileged_mode > 0 ? VERIFY_ESP_UNPRIVILEGED_MODE : 0; + r = dir_fd_is_root_or_cwd(rfd); if (r < 0) return log_error_errno(r, "Failed to check if directory file descriptor is root: %m"); @@ -509,7 +516,7 @@ int find_esp_and_warn_at( int find_esp_and_warn( const char *root, const char *path, - bool unprivileged_mode, + int unprivileged_mode, char **ret_path, uint32_t *ret_part, uint64_t *ret_pstart, @@ -560,12 +567,13 @@ int find_esp_and_warn( static int verify_xbootldr_blkid( dev_t devid, - bool searching, + VerifyESPFlags flags, sd_id128_t *ret_uuid) { sd_id128_t uuid = SD_ID128_NULL; #if HAVE_BLKID + bool searching = FLAGS_SET(flags, VERIFY_ESP_SEARCHING); _cleanup_(blkid_free_probep) blkid_probe b = NULL; _cleanup_free_ char *node = NULL; const char *type, *v; @@ -644,9 +652,10 @@ static int verify_xbootldr_blkid( static int verify_xbootldr_udev( dev_t devid, - bool searching, + VerifyESPFlags flags, sd_id128_t *ret_uuid) { + bool searching = FLAGS_SET(flags, VERIFY_ESP_SEARCHING); _cleanup_(sd_device_unrefp) sd_device *d = NULL; sd_id128_t uuid = SD_ID128_NULL; const char *node, *type, *v; @@ -718,15 +727,16 @@ static int verify_xbootldr_udev( static int verify_xbootldr( int rfd, const char *path, - bool searching, - bool unprivileged_mode, + VerifyESPFlags flags, char **ret_path, sd_id128_t *ret_uuid, dev_t *ret_devid) { _cleanup_free_ char *p = NULL; _cleanup_close_ int pfd = -EBADF; - bool relax_checks; + bool searching = FLAGS_SET(flags, VERIFY_ESP_SEARCHING), + unprivileged_mode = FLAGS_SET(flags, VERIFY_ESP_UNPRIVILEGED_MODE), + relax_checks; dev_t devid = 0; int r; @@ -743,7 +753,7 @@ static int verify_xbootldr( getenv_bool("SYSTEMD_RELAX_XBOOTLDR_CHECKS") > 0 || detect_container() > 0; - r = verify_fsroot_dir(pfd, p, searching, unprivileged_mode, relax_checks ? NULL : &devid); + r = verify_fsroot_dir(pfd, p, flags, relax_checks ? NULL : &devid); if (r < 0) return r; @@ -751,9 +761,9 @@ static int verify_xbootldr( goto finish; if (unprivileged_mode) - r = verify_xbootldr_udev(devid, searching, ret_uuid); + r = verify_xbootldr_udev(devid, flags, ret_uuid); else - r = verify_xbootldr_blkid(devid, searching, ret_uuid); + r = verify_xbootldr_blkid(devid, flags, ret_uuid); if (r < 0) return r; @@ -778,19 +788,25 @@ finish: int find_xbootldr_and_warn_at( int rfd, const char *path, - bool unprivileged_mode, + int unprivileged_mode, char **ret_path, sd_id128_t *ret_uuid, dev_t *ret_devid) { + VerifyESPFlags flags = 0; int r; /* Similar to find_esp_and_warn(), but finds the XBOOTLDR partition. Returns the same errors. */ assert(rfd >= 0 || rfd == AT_FDCWD); + if (unprivileged_mode < 0) + unprivileged_mode = geteuid() != 0; + if (unprivileged_mode) + flags |= VERIFY_ESP_UNPRIVILEGED_MODE; + if (path) - return verify_xbootldr(rfd, path, /* searching= */ false, unprivileged_mode, ret_path, ret_uuid, ret_devid); + return verify_xbootldr(rfd, path, flags, ret_path, ret_uuid, ret_devid); path = getenv("SYSTEMD_XBOOTLDR_PATH"); if (path) { @@ -822,7 +838,7 @@ int find_xbootldr_and_warn_at( return 0; } - r = verify_xbootldr(rfd, "/boot", /* searching= */ true, unprivileged_mode, ret_path, ret_uuid, ret_devid); + r = verify_xbootldr(rfd, "/boot", flags | VERIFY_ESP_SEARCHING, ret_path, ret_uuid, ret_devid); if (r < 0) { if (!IN_SET(r, -ENOENT, -EADDRNOTAVAIL, -ENOTDIR)) /* This one is not it */ return r; @@ -836,7 +852,7 @@ int find_xbootldr_and_warn_at( int find_xbootldr_and_warn( const char *root, const char *path, - bool unprivileged_mode, + int unprivileged_mode, char **ret_path, sd_id128_t *ret_uuid, dev_t *ret_devid) { diff --git a/src/shared/find-esp.h b/src/shared/find-esp.h index 94f320195bf..2e132a74aa0 100644 --- a/src/shared/find-esp.h +++ b/src/shared/find-esp.h @@ -8,8 +8,8 @@ #include "sd-id128.h" -int find_esp_and_warn_at(int rfd, const char *path, bool unprivileged_mode, char **ret_path, uint32_t *ret_part, uint64_t *ret_pstart, uint64_t *ret_psize, sd_id128_t *ret_uuid, dev_t *ret_devid); -int find_esp_and_warn(const char *root, const char *path, bool unprivileged_mode, char **ret_path, uint32_t *ret_part, uint64_t *ret_pstart, uint64_t *ret_psize, sd_id128_t *ret_uuid, dev_t *ret_devid); +int find_esp_and_warn_at(int rfd, const char *path, int unprivileged_mode, char **ret_path, uint32_t *ret_part, uint64_t *ret_pstart, uint64_t *ret_psize, sd_id128_t *ret_uuid, dev_t *ret_devid); +int find_esp_and_warn(const char *root, const char *path, int unprivileged_mode, char **ret_path, uint32_t *ret_part, uint64_t *ret_pstart, uint64_t *ret_psize, sd_id128_t *ret_uuid, dev_t *ret_devid); -int find_xbootldr_and_warn_at(int rfd, const char *path, bool unprivileged_mode, char **ret_path, sd_id128_t *ret_uuid, dev_t *ret_devid); -int find_xbootldr_and_warn(const char *root, const char *path, bool unprivileged_mode, char **ret_path, sd_id128_t *ret_uuid, dev_t *ret_devid); +int find_xbootldr_and_warn_at(int rfd, const char *path, int unprivileged_mode, char **ret_path, sd_id128_t *ret_uuid, dev_t *ret_devid); +int find_xbootldr_and_warn(const char *root, const char *path, int unprivileged_mode, char **ret_path, sd_id128_t *ret_uuid, dev_t *ret_devid); From 14e5c99236139abc2362bd9b671f19ff375263af Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 31 May 2023 09:37:24 +0200 Subject: [PATCH 2/2] find-esp: drop some redundant 'else' --- src/shared/find-esp.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/shared/find-esp.c b/src/shared/find-esp.c index 7cd149b29b4..d9336f4431d 100644 --- a/src/shared/find-esp.c +++ b/src/shared/find-esp.c @@ -66,9 +66,9 @@ static int verify_esp_blkid( r = blkid_do_safeprobe(b); if (r == -2) return log_error_errno(SYNTHETIC_ERRNO(ENODEV), "File system \"%s\" is ambiguous.", node); - else if (r == 1) + if (r == 1) return log_error_errno(SYNTHETIC_ERRNO(ENODEV), "File system \"%s\" does not contain a label.", node); - else if (r != 0) + if (r != 0) return log_error_errno(errno ?: SYNTHETIC_ERRNO(EIO), "Failed to probe file system \"%s\": %m", node); r = blkid_probe_lookup_value(b, "TYPE", &v, NULL);