From 5c396a01104ba3de5e4e72825c019873f50bc626 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 3 Sep 2025 11:31:30 +0200 Subject: [PATCH 1/8] bootctl: switch a few getenv() calls to secure_getenv() Following the rule that we should always prefer the secure flavour over the regular one unless there's a clear reason for the regular one, let's switch this over. Better safe than sorry. --- src/bootctl/bootctl-install.c | 4 ++-- src/bootctl/bootctl-random-seed.c | 2 +- src/bootctl/bootctl-util.c | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/bootctl/bootctl-install.c b/src/bootctl/bootctl-install.c index 197c9d5bd6..0d07f54478 100644 --- a/src/bootctl/bootctl-install.c +++ b/src/bootctl/bootctl-install.c @@ -102,7 +102,7 @@ static int load_kernel_install_layout(void) { int r; r = load_kernel_install_conf(arg_root, - getenv("KERNEL_INSTALL_CONF_ROOT"), + secure_getenv("KERNEL_INSTALL_CONF_ROOT"), /* ret_machine_id= */ NULL, /* ret_boot_root= */ NULL, &layout, @@ -578,7 +578,7 @@ static int install_entry_token(void) { if (!arg_make_entry_directory && arg_entry_token_type == BOOT_ENTRY_TOKEN_MACHINE_ID) return 0; - p = path_join(arg_root, getenv("KERNEL_INSTALL_CONF_ROOT") ?: "/etc/kernel/", "entry-token"); + p = path_join(arg_root, secure_getenv("KERNEL_INSTALL_CONF_ROOT") ?: "/etc/kernel/", "entry-token"); if (!p) return log_oom(); diff --git a/src/bootctl/bootctl-random-seed.c b/src/bootctl/bootctl-random-seed.c index 7cd39624c1..c4811aace3 100644 --- a/src/bootctl/bootctl-random-seed.c +++ b/src/bootctl/bootctl-random-seed.c @@ -61,7 +61,7 @@ static int set_system_token(void) { if (!touch_variables()) return 0; - r = getenv_bool("SYSTEMD_WRITE_SYSTEM_TOKEN"); + r = secure_getenv_bool("SYSTEMD_WRITE_SYSTEM_TOKEN"); if (r < 0) { if (r != -ENXIO) log_warning_errno(r, "Failed to parse $SYSTEMD_WRITE_SYSTEM_TOKEN, ignoring."); diff --git a/src/bootctl/bootctl-util.c b/src/bootctl/bootctl-util.c index 381c6f8af6..f5642827ac 100644 --- a/src/bootctl/bootctl-util.c +++ b/src/bootctl/bootctl-util.c @@ -124,7 +124,7 @@ int settle_entry_token(void) { r = boot_entry_token_ensure( arg_root, - getenv("KERNEL_INSTALL_CONF_ROOT"), + secure_getenv("KERNEL_INSTALL_CONF_ROOT"), arg_machine_id, /* machine_id_is_random = */ false, &arg_entry_token_type, From f7570222946f9a59d07d2feaf1496cabaca9681d Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 3 Sep 2025 11:45:22 +0200 Subject: [PATCH 2/8] bootctl: use RET_GATHER() all over the place --- src/bootctl/bootctl-install.c | 89 +++++++++-------------------------- 1 file changed, 21 insertions(+), 68 deletions(-) diff --git a/src/bootctl/bootctl-install.c b/src/bootctl/bootctl-install.c index 0d07f54478..4eefc715d5 100644 --- a/src/bootctl/bootctl-install.c +++ b/src/bootctl/bootctl-install.c @@ -1161,7 +1161,7 @@ static int rmdir_one(const char *prefix, const char *suffix) { } static int remove_subdirs(const char *root, const char *const *subdirs) { - int r, q; + int r; /* We use recursion here to destroy the directories in reverse order. Which should be safe given how * short the array is. */ @@ -1170,9 +1170,7 @@ static int remove_subdirs(const char *root, const char *const *subdirs) { return 0; r = remove_subdirs(root, subdirs + 1); - q = rmdir_one(root, subdirs[0]); - - return r < 0 ? r : q; + return RET_GATHER(r, rmdir_one(root, subdirs[0])); } static int remove_entry_directory(const char *root) { @@ -1186,19 +1184,14 @@ static int remove_entry_directory(const char *root) { } static int remove_binaries(const char *esp_path) { - int r, q; + int r; _cleanup_free_ char *p = path_join(esp_path, "/EFI/systemd"); if (!p) return log_oom(); r = rm_rf(p, REMOVE_ROOT|REMOVE_PHYSICAL); - - q = remove_boot_efi(esp_path); - if (q < 0 && r == 0) - r = q; - - return r; + return RET_GATHER(r, remove_boot_efi(esp_path)); } static int remove_file(const char *root, const char *file) { @@ -1258,11 +1251,9 @@ static int remove_loader_variables(void) { q = efi_set_variable(var, NULL, 0); if (q == -ENOENT) continue; - if (q < 0) { - log_warning_errno(q, "Failed to remove EFI variable %s: %m", var); - if (r >= 0) - r = q; - } else + if (q < 0) + RET_GATHER(r, log_warning_errno(q, "Failed to remove EFI variable %s: %m", var)); + else log_info("Removed EFI variable %s.", var); } @@ -1271,7 +1262,7 @@ static int remove_loader_variables(void) { int verb_remove(int argc, char *argv[], void *userdata) { sd_id128_t uuid = SD_ID128_NULL; - int r, q; + int r; r = acquire_esp(/* unprivileged_mode= */ false, /* graceful= */ false, NULL, NULL, NULL, &uuid, NULL); if (r < 0) @@ -1286,59 +1277,28 @@ int verb_remove(int argc, char *argv[], void *userdata) { return r; r = remove_binaries(arg_esp_path); - - q = remove_file(arg_esp_path, "/loader/loader.conf"); - if (q < 0 && r >= 0) - r = q; - - q = remove_file(arg_esp_path, "/loader/random-seed"); - if (q < 0 && r >= 0) - r = q; - - q = remove_file(arg_esp_path, "/loader/entries.srel"); - if (q < 0 && r >= 0) - r = q; + RET_GATHER(r, remove_file(arg_esp_path, "/loader/loader.conf")); + RET_GATHER(r, remove_file(arg_esp_path, "/loader/random-seed")); + RET_GATHER(r, remove_file(arg_esp_path, "/loader/entries.srel")); FOREACH_STRING(db, "PK.auth", "KEK.auth", "db.auth") { _cleanup_free_ char *p = path_join("/loader/keys/auto", db); if (!p) return log_oom(); - q = remove_file(arg_esp_path, p); - if (q < 0 && r >= 0) - r = q; + RET_GATHER(r, remove_file(arg_esp_path, p)); } - q = rmdir_one(arg_esp_path, "/loader/keys/auto"); - if (q < 0 && r >= 0) - r = q; - - q = remove_subdirs(arg_esp_path, esp_subdirs); - if (q < 0 && r >= 0) - r = q; - - q = remove_subdirs(arg_esp_path, dollar_boot_subdirs); - if (q < 0 && r >= 0) - r = q; - - q = remove_entry_directory(arg_esp_path); - if (q < 0 && r >= 0) - r = q; + RET_GATHER(r, rmdir_one(arg_esp_path, "/loader/keys/auto")); + RET_GATHER(r, remove_subdirs(arg_esp_path, esp_subdirs)); + RET_GATHER(r, remove_subdirs(arg_esp_path, dollar_boot_subdirs)); + RET_GATHER(r, remove_entry_directory(arg_esp_path)); if (arg_xbootldr_path) { /* Remove a subset of these also from the XBOOTLDR partition if it exists */ - - q = remove_file(arg_xbootldr_path, "/loader/entries.srel"); - if (q < 0 && r >= 0) - r = q; - - q = remove_subdirs(arg_xbootldr_path, dollar_boot_subdirs); - if (q < 0 && r >= 0) - r = q; - - q = remove_entry_directory(arg_xbootldr_path); - if (q < 0 && r >= 0) - r = q; + RET_GATHER(r, remove_file(arg_xbootldr_path, "/loader/entries.srel")); + RET_GATHER(r, remove_subdirs(arg_xbootldr_path, dollar_boot_subdirs)); + RET_GATHER(r, remove_entry_directory(arg_xbootldr_path)); } (void) sync_everything(); @@ -1352,15 +1312,8 @@ int verb_remove(int argc, char *argv[], void *userdata) { } char *path = strjoina("/EFI/systemd/systemd-boot", get_efi_arch(), ".efi"); - q = remove_variables(uuid, path, true); - if (q < 0 && r >= 0) - r = q; - - q = remove_loader_variables(); - if (q < 0 && r >= 0) - r = q; - - return r; + RET_GATHER(r, remove_variables(uuid, path, /* in_order= */ true)); + return RET_GATHER(r, remove_loader_variables()); } int verb_is_installed(int argc, char *argv[], void *userdata) { From 83d0b6597c208818412b7c60a90f9229a4407a3b Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 3 Sep 2025 11:48:39 +0200 Subject: [PATCH 3/8] bootctl: normalize some enum definitions --- src/bootctl/bootctl-install.c | 8 ++++---- src/bootctl/bootctl.c | 10 +++++----- src/bootctl/bootctl.h | 8 +++++--- src/shared/boot-entry.h | 2 ++ 4 files changed, 16 insertions(+), 12 deletions(-) diff --git a/src/bootctl/bootctl-install.c b/src/bootctl/bootctl-install.c index 4eefc715d5..3605b43ef0 100644 --- a/src/bootctl/bootctl-install.c +++ b/src/bootctl/bootctl-install.c @@ -381,7 +381,7 @@ static int update_efi_boot_binaries(const char *esp_path, const char *source_pat } static int copy_one_file(const char *esp_path, const char *name, bool force) { - char *root = IN_SET(arg_install_source, ARG_INSTALL_SOURCE_AUTO, ARG_INSTALL_SOURCE_IMAGE) ? arg_root : NULL; + char *root = IN_SET(arg_install_source, INSTALL_SOURCE_AUTO, INSTALL_SOURCE_IMAGE) ? arg_root : NULL; _cleanup_free_ char *source_path = NULL, *dest_path = NULL, *p = NULL, *q = NULL; const char *e; char *dest_name, *s; @@ -398,7 +398,7 @@ static int copy_one_file(const char *esp_path, const char *name, bool force) { r = chase(p, root, CHASE_PREFIX_ROOT|CHASE_PROHIBIT_SYMLINKS|CHASE_TRIGGER_AUTOFS, &source_path, NULL); /* If we had a root directory to try, we didn't find it and we are in auto mode, retry on the host */ - if (r == -ENOENT && root && arg_install_source == ARG_INSTALL_SOURCE_AUTO) + if (r == -ENOENT && root && arg_install_source == INSTALL_SOURCE_AUTO) r = chase(p, NULL, CHASE_PREFIX_ROOT|CHASE_PROHIBIT_SYMLINKS|CHASE_TRIGGER_AUTOFS, &source_path, NULL); if (r < 0) return log_error_errno(r, @@ -444,14 +444,14 @@ static int copy_one_file(const char *esp_path, const char *name, bool force) { } static int install_binaries(const char *esp_path, const char *arch, bool force) { - char *root = IN_SET(arg_install_source, ARG_INSTALL_SOURCE_AUTO, ARG_INSTALL_SOURCE_IMAGE) ? arg_root : NULL; + char *root = IN_SET(arg_install_source, INSTALL_SOURCE_AUTO, INSTALL_SOURCE_IMAGE) ? arg_root : NULL; _cleanup_closedir_ DIR *d = NULL; _cleanup_free_ char *path = NULL; int r; r = chase_and_opendir(BOOTLIBDIR, root, CHASE_PREFIX_ROOT|CHASE_PROHIBIT_SYMLINKS|CHASE_TRIGGER_AUTOFS, &path, &d); /* If we had a root directory to try, we didn't find it and we are in auto mode, retry on the host */ - if (r == -ENOENT && root && arg_install_source == ARG_INSTALL_SOURCE_AUTO) + if (r == -ENOENT && root && arg_install_source == INSTALL_SOURCE_AUTO) r = chase_and_opendir(BOOTLIBDIR, NULL, CHASE_PREFIX_ROOT|CHASE_PROHIBIT_SYMLINKS|CHASE_TRIGGER_AUTOFS, &path, &d); if (r == -ENOENT && arg_graceful() != ARG_GRACEFUL_NO) { log_debug("Source directory does not exist, ignoring."); diff --git a/src/bootctl/bootctl.c b/src/bootctl/bootctl.c index 0ef17ba501..86a51f8f41 100644 --- a/src/bootctl/bootctl.c +++ b/src/bootctl/bootctl.c @@ -67,7 +67,7 @@ sd_json_format_flags_t arg_json_format_flags = SD_JSON_FORMAT_OFF; bool arg_arch_all = false; char *arg_root = NULL; char *arg_image = NULL; -InstallSource arg_install_source = ARG_INSTALL_SOURCE_AUTO; +InstallSource arg_install_source = INSTALL_SOURCE_AUTO; char *arg_efi_boot_option_description = NULL; bool arg_dry_run = false; ImagePolicy *arg_image_policy = NULL; @@ -483,11 +483,11 @@ static int parse_argv(int argc, char *argv[]) { case ARG_INSTALL_SOURCE: if (streq(optarg, "auto")) - arg_install_source = ARG_INSTALL_SOURCE_AUTO; + arg_install_source = INSTALL_SOURCE_AUTO; else if (streq(optarg, "image")) - arg_install_source = ARG_INSTALL_SOURCE_IMAGE; + arg_install_source = INSTALL_SOURCE_IMAGE; else if (streq(optarg, "host")) - arg_install_source = ARG_INSTALL_SOURCE_HOST; + arg_install_source = INSTALL_SOURCE_HOST; else return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Unexpected parameter for --install-source=: %s", optarg); @@ -648,7 +648,7 @@ static int parse_argv(int argc, char *argv[]) { if (arg_root && arg_image) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Please specify either --root= or --image=, the combination of both is not supported."); - if (arg_install_source != ARG_INSTALL_SOURCE_AUTO && !arg_root && !arg_image) + if (arg_install_source != INSTALL_SOURCE_AUTO && !arg_root && !arg_image) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "--install-from-host is only supported with --root= or --image=."); if (arg_dry_run && argv[optind] && !STR_IN_SET(argv[optind], "unlink", "cleanup")) diff --git a/src/bootctl/bootctl.h b/src/bootctl/bootctl.h index 7504d5715c..4f052e67bc 100644 --- a/src/bootctl/bootctl.h +++ b/src/bootctl/bootctl.h @@ -4,9 +4,11 @@ #include "forward.h" typedef enum InstallSource { - ARG_INSTALL_SOURCE_IMAGE, - ARG_INSTALL_SOURCE_HOST, - ARG_INSTALL_SOURCE_AUTO, + INSTALL_SOURCE_IMAGE, + INSTALL_SOURCE_HOST, + INSTALL_SOURCE_AUTO, + _INSTALL_SOURCE_MAX, + _INSTALL_SOURCE_INVALID = -EINVAL, } InstallSource; typedef enum GracefulMode { diff --git a/src/shared/boot-entry.h b/src/shared/boot-entry.h index 927802e5ee..71260d4ff7 100644 --- a/src/shared/boot-entry.h +++ b/src/shared/boot-entry.h @@ -11,6 +11,8 @@ typedef enum BootEntryTokenType { BOOT_ENTRY_TOKEN_OS_ID, BOOT_ENTRY_TOKEN_LITERAL, BOOT_ENTRY_TOKEN_AUTO, + _BOOT_ENTRY_TOKEN_TYPE_MAX, + _BOOT_ENTRY_TOKEN_TYPE_INVALID = -EINVAL, } BootEntryTokenType; bool boot_entry_token_valid(const char *p); From c95d72913a0faf94bba7cd7692aa4ad85aa93761 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Sat, 30 Aug 2025 22:50:59 +0200 Subject: [PATCH 4/8] bootctl: split out varlink setup into a helper call of its own --- src/bootctl/bootctl.c | 61 ++++++++++++++++++++++++------------------- 1 file changed, 34 insertions(+), 27 deletions(-) diff --git a/src/bootctl/bootctl.c b/src/bootctl/bootctl.c index 86a51f8f41..1341036ced 100644 --- a/src/bootctl/bootctl.c +++ b/src/bootctl/bootctl.c @@ -697,6 +697,38 @@ static int bootctl_main(int argc, char *argv[]) { return dispatch_verb(argc, argv, verbs, NULL); } +static int vl_server(void) { + _cleanup_(sd_varlink_server_unrefp) sd_varlink_server *varlink_server = NULL; + int r; + + /* Invocation as Varlink service */ + + r = varlink_server_new( + &varlink_server, + SD_VARLINK_SERVER_ROOT_ONLY, + /* userdata= */ NULL); + if (r < 0) + return log_error_errno(r, "Failed to allocate Varlink server: %m"); + + r = sd_varlink_server_add_interface(varlink_server, &vl_interface_io_systemd_BootControl); + if (r < 0) + return log_error_errno(r, "Failed to add Varlink interface: %m"); + + r = sd_varlink_server_bind_method_many( + varlink_server, + "io.systemd.BootControl.ListBootEntries", vl_method_list_boot_entries, + "io.systemd.BootControl.SetRebootToFirmware", vl_method_set_reboot_to_firmware, + "io.systemd.BootControl.GetRebootToFirmware", vl_method_get_reboot_to_firmware); + if (r < 0) + return log_error_errno(r, "Failed to bind Varlink methods: %m"); + + r = sd_varlink_server_loop_auto(varlink_server); + if (r < 0) + return log_error_errno(r, "Failed to run Varlink event loop: %m"); + + return 0; +} + static int run(int argc, char *argv[]) { _cleanup_(loop_device_unrefp) LoopDevice *loop_device = NULL; _cleanup_(umount_and_freep) char *mounted_dir = NULL; @@ -708,33 +740,8 @@ static int run(int argc, char *argv[]) { if (r <= 0) return r; - if (arg_varlink) { - _cleanup_(sd_varlink_server_unrefp) sd_varlink_server *varlink_server = NULL; - - /* Invocation as Varlink service */ - - r = varlink_server_new(&varlink_server, SD_VARLINK_SERVER_ROOT_ONLY, NULL); - if (r < 0) - return log_error_errno(r, "Failed to allocate Varlink server: %m"); - - r = sd_varlink_server_add_interface(varlink_server, &vl_interface_io_systemd_BootControl); - if (r < 0) - return log_error_errno(r, "Failed to add Varlink interface: %m"); - - r = sd_varlink_server_bind_method_many( - varlink_server, - "io.systemd.BootControl.ListBootEntries", vl_method_list_boot_entries, - "io.systemd.BootControl.SetRebootToFirmware", vl_method_set_reboot_to_firmware, - "io.systemd.BootControl.GetRebootToFirmware", vl_method_get_reboot_to_firmware); - if (r < 0) - return log_error_errno(r, "Failed to bind Varlink methods: %m"); - - r = sd_varlink_server_loop_auto(varlink_server); - if (r < 0) - return log_error_errno(r, "Failed to run Varlink event loop: %m"); - - return EXIT_SUCCESS; - } + if (arg_varlink) + return vl_server(); if (arg_print_root_device > 0) { _cleanup_free_ char *path = NULL; From b6f4f85c395def2703c6a3501538fd7948bbba69 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 2 Sep 2025 22:41:48 +0200 Subject: [PATCH 5/8] bootctl: downgrade messages about foreign EFI files Given that we iterate through $ESP/EFI/BOOT/*.EFI these days this is a pretty common case, hence it's not really noteworthy, hence downgrade these log messages from LOG_NOTICE to LOG_INFO. --- src/bootctl/bootctl-install.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/bootctl/bootctl-install.c b/src/bootctl/bootctl-install.c index 3605b43ef0..df5065b798 100644 --- a/src/bootctl/bootctl-install.c +++ b/src/bootctl/bootctl-install.c @@ -210,13 +210,12 @@ static int version_check(int fd_from, const char *from, int fd_to, const char *t r = get_file_version(fd_to, &b); if (r == -ESRCH) - return log_notice_errno(r, "Skipping \"%s\", it's owned by another boot loader (no version info found).", - to); + return log_info_errno(r, "Skipping \"%s\", it's owned by another boot loader (no version info found).", to); if (r < 0) return r; if (compare_product(a, b) != 0) - return log_notice_errno(SYNTHETIC_ERRNO(ESRCH), - "Skipping \"%s\", it's owned by another boot loader.", to); + return log_info_errno(SYNTHETIC_ERRNO(ESRCH), + "Skipping \"%s\", it's owned by another boot loader.", to); r = compare_version(a, b); log_debug("Comparing versions: \"%s\" %s \"%s\"", a, comparison_operator(r), b); From 7dd55c83b8e4cbed3632838711d78e0ebbfd9f3c Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 2 Sep 2025 22:44:35 +0200 Subject: [PATCH 6/8] bootctl: don't update $ESP/EFI/BOOTX64.EFI twice We update BOOTX64.EFI explicitly once (because we know that it's the main entry point of UEFI) and then a second time when we update everything in $ESP/EFI/*.EFI. That's redundant and pretty ugly/confusing in the log output. Hence exclude the file we already updated explicitly from the 2nd run. --- src/bootctl/bootctl-install.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/bootctl/bootctl-install.c b/src/bootctl/bootctl-install.c index df5065b798..89a82192f8 100644 --- a/src/bootctl/bootctl-install.c +++ b/src/bootctl/bootctl-install.c @@ -334,7 +334,11 @@ static int create_subdirs(const char *root, const char * const *subdirs) { return 0; } -static int update_efi_boot_binaries(const char *esp_path, const char *source_path) { +static int update_efi_boot_binaries( + const char *esp_path, + const char *source_path, + const char *ignore_filename) { + _cleanup_closedir_ DIR *d = NULL; _cleanup_free_ char *p = NULL; int r, ret = 0; @@ -354,6 +358,9 @@ static int update_efi_boot_binaries(const char *esp_path, const char *source_pat if (!endswith_no_case(de->d_name, ".efi")) continue; + if (strcaseeq_ptr(ignore_filename, de->d_name)) + continue; + fd = xopenat_full(dirfd(d), de->d_name, O_RDONLY|O_CLOEXEC|O_NONBLOCK|O_NOCTTY|O_NOFOLLOW, XO_REGULAR, /* mode= */ 0); if (fd < 0) return log_error_errno(fd, "Failed to open \"%s/%s\" for reading: %m", p, de->d_name); @@ -425,7 +432,7 @@ static int copy_one_file(const char *esp_path, const char *name, bool force) { /* Create the EFI default boot loader name (specified for removable devices) */ v = strjoina("/EFI/BOOT/BOOT", e); - ascii_strupper(strrchr(v, '/') + 1); + const char *boot_dot_efi = ascii_strupper(strrchr(v, '/') + 1); r = chase(v, esp_path, CHASE_PREFIX_ROOT|CHASE_PROHIBIT_SYMLINKS|CHASE_NONEXISTENT|CHASE_TRIGGER_AUTOFS, &default_dest_path, NULL); if (r < 0) @@ -433,10 +440,10 @@ static int copy_one_file(const char *esp_path, const char *name, bool force) { RET_GATHER(ret, copy_file_with_version_check(source_path, default_dest_path, force)); - /* If we were installed under any other name in /EFI/BOOT, make sure we update those binaries + /* If we were installed under any other name in /EFI/BOOT/, make sure we update those binaries * as well. */ if (!force) - RET_GATHER(ret, update_efi_boot_binaries(esp_path, source_path)); + RET_GATHER(ret, update_efi_boot_binaries(esp_path, source_path, boot_dot_efi)); } return ret; From 407139ae92e527ff66a7302165d3690eb24ac3f9 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 3 Sep 2025 10:38:59 +0200 Subject: [PATCH 7/8] bootctl: output a more precise log message when updating existing EFI vars --- src/bootctl/bootctl-install.c | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/src/bootctl/bootctl-install.c b/src/bootctl/bootctl-install.c index 89a82192f8..b70e945ca7 100644 --- a/src/bootctl/bootctl-install.c +++ b/src/bootctl/bootctl-install.c @@ -906,10 +906,17 @@ static int install_variables( return graceful ? 0 : r; } - if (first || r == 0) { - r = efi_add_boot_option(slot, pick_efi_boot_option_description(), - part, pstart, psize, - uuid, path); + bool existing = r > 0; + + if (first || !existing) { + r = efi_add_boot_option( + slot, + pick_efi_boot_option_description(), + part, + pstart, + psize, + uuid, + path); if (r < 0) { int level = graceful ? arg_quiet ? LOG_DEBUG : LOG_INFO : LOG_ERR; const char *skip = graceful ? ", skipping" : ""; @@ -919,7 +926,9 @@ static int install_variables( return graceful ? 0 : r; } - log_info("Created EFI boot entry \"%s\".", pick_efi_boot_option_description()); + log_info("%s EFI boot entry \"%s\".", + existing ? "Updated" : "Created", + pick_efi_boot_option_description()); } return insert_into_order(slot, first); From a86a366eb07294357c34122114a11ee131c25e72 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 3 Sep 2025 12:20:32 +0200 Subject: [PATCH 8/8] bootctl: split out auto-enroll cert load code into function of its own --- src/bootctl/bootctl-install.c | 104 +++++++++++++++++++++------------- 1 file changed, 64 insertions(+), 40 deletions(-) diff --git a/src/bootctl/bootctl-install.c b/src/bootctl/bootctl-install.c index b70e945ca7..f31c55d36a 100644 --- a/src/bootctl/bootctl-install.c +++ b/src/bootctl/bootctl-install.c @@ -963,10 +963,67 @@ static int are_we_installed(const char *esp_path) { return r == 0; } -int verb_install(int argc, char *argv[], void *userdata) { +static int load_secure_boot_auto_enroll( + X509 **ret_certificate, + EVP_PKEY **ret_private_key) { + + int r; + + assert(ret_certificate); + assert(ret_private_key); + + if (!arg_secure_boot_auto_enroll) { + *ret_certificate = NULL; + *ret_private_key = NULL; + return 0; + } + + if (arg_certificate_source_type == OPENSSL_CERTIFICATE_SOURCE_FILE) { + r = parse_path_argument(arg_certificate, /*suppress_root=*/ false, &arg_certificate); + if (r < 0) + return r; + } + _cleanup_(X509_freep) X509 *certificate = NULL; - _cleanup_(openssl_ask_password_ui_freep) OpenSSLAskPasswordUI *ui = NULL; + r = openssl_load_x509_certificate( + arg_certificate_source_type, + arg_certificate_source, + arg_certificate, + &certificate); + if (r < 0) + return log_error_errno(r, "Failed to load X.509 certificate from %s: %m", arg_certificate); + + if (arg_private_key_source_type == OPENSSL_KEY_SOURCE_FILE) { + r = parse_path_argument(arg_private_key, /* suppress_root= */ false, &arg_private_key); + if (r < 0) + return log_error_errno(r, "Failed to parse private key path %s: %m", arg_private_key); + } + _cleanup_(EVP_PKEY_freep) EVP_PKEY *private_key = NULL; + r = openssl_load_private_key( + arg_private_key_source_type, + arg_private_key_source, + arg_private_key, + &(AskPasswordRequest) { + .tty_fd = -EBADF, + .id = "bootctl-private-key-pin", + .keyring = arg_private_key, + .credential = "bootctl.private-key-pin", + .until = USEC_INFINITY, + .hup_fd = -EBADF, + }, + &private_key, + /* ret_user_interface= */ NULL); + if (r < 0) + return log_error_errno(r, "Failed to load private key from %s: %m", arg_private_key); + + *ret_certificate = TAKE_PTR(certificate); + *ret_private_key = TAKE_PTR(private_key); + + return 0; +} + +int verb_install(int argc, char *argv[], void *userdata) { sd_id128_t uuid = SD_ID128_NULL; uint64_t pstart = 0, psize = 0; uint32_t part = 0; @@ -980,44 +1037,11 @@ int verb_install(int argc, char *argv[], void *userdata) { /* Support graceful mode only for updates, unless forcibly enabled in chroot environments */ graceful = arg_graceful() == ARG_GRACEFUL_FORCE || (!install && arg_graceful() != ARG_GRACEFUL_NO); - if (arg_secure_boot_auto_enroll) { - if (arg_certificate_source_type == OPENSSL_CERTIFICATE_SOURCE_FILE) { - r = parse_path_argument(arg_certificate, /*suppress_root=*/ false, &arg_certificate); - if (r < 0) - return r; - } - - r = openssl_load_x509_certificate( - arg_certificate_source_type, - arg_certificate_source, - arg_certificate, - &certificate); - if (r < 0) - return log_error_errno(r, "Failed to load X.509 certificate from %s: %m", arg_certificate); - - if (arg_private_key_source_type == OPENSSL_KEY_SOURCE_FILE) { - r = parse_path_argument(arg_private_key, /* suppress_root= */ false, &arg_private_key); - if (r < 0) - return log_error_errno(r, "Failed to parse private key path %s: %m", arg_private_key); - } - - r = openssl_load_private_key( - arg_private_key_source_type, - arg_private_key_source, - arg_private_key, - &(AskPasswordRequest) { - .tty_fd = -EBADF, - .id = "bootctl-private-key-pin", - .keyring = arg_private_key, - .credential = "bootctl.private-key-pin", - .until = USEC_INFINITY, - .hup_fd = -EBADF, - }, - &private_key, - &ui); - if (r < 0) - return log_error_errno(r, "Failed to load private key from %s: %m", arg_private_key); - } + _cleanup_(EVP_PKEY_freep) EVP_PKEY *private_key = NULL; + _cleanup_(X509_freep) X509 *certificate = NULL; + r = load_secure_boot_auto_enroll(&certificate, &private_key); + if (r < 0) + return r; r = acquire_esp(/* unprivileged_mode= */ false, graceful, &part, &pstart, &psize, &uuid, NULL); if (graceful && r == -ENOKEY)