From 47fb161e3a72c65071038eddbd84108504155b77 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 17 Dec 2021 16:06:51 +0100 Subject: [PATCH 1/2] =?UTF-8?q?bootctl:=20write=20KERNEL=5FINSTALL=5FLAYOU?= =?UTF-8?q?T=3Dbls=20and=20KERNEL=5FINSTALL=5FMACHINE=5FID=3D=E2=80=A6?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is a natural extension of d6bce6e224: if we are installing sd-boot, we want to use the sd-boot layout, so let's write the appropriate KERNEL_INSTALL_LAYOUT setting. Effectively, if we do 'booctl install', kernel-install will not autodetect the layout anymore. And 357376d0bb added support for KERNEL_INSTALL_MACHINE_ID. We need to support it here too. We both read it, so that we create the right directories, and also write it if it wasn't written yet and we created some directories using it, so that kernel-install that is executed later knows the machine-id that matches the directories we crated. The code is changed in some places to fail if we can't figure out the current status. When installing the boot loader it's probably better not to guess. --- src/boot/bootctl.c | 193 +++++++++++++++++++++++++++++++++------- src/sysusers/sysusers.c | 6 +- 2 files changed, 166 insertions(+), 33 deletions(-) diff --git a/src/boot/bootctl.c b/src/boot/bootctl.c index 1a940f9462..8f89a309e8 100644 --- a/src/boot/bootctl.c +++ b/src/boot/bootctl.c @@ -19,6 +19,7 @@ #include "dirent-util.h" #include "efi-loader.h" #include "efivars.h" +#include "env-file.h" #include "env-util.h" #include "escape.h" #include "fd-util.h" @@ -40,6 +41,7 @@ #include "sync-util.h" #include "terminal-util.h" #include "tmpfile-util.h" +#include "tmpfile-util-label.h" #include "umask-util.h" #include "utf8.h" #include "util.h" @@ -54,9 +56,12 @@ static bool arg_touch_variables = true; static PagerFlags arg_pager_flags = 0; static bool arg_graceful = false; static int arg_make_machine_id_directory = -1; +static sd_id128_t arg_machine_id = SD_ID128_NULL; +static char *arg_install_layout = NULL; STATIC_DESTRUCTOR_REGISTER(arg_esp_path, freep); STATIC_DESTRUCTOR_REGISTER(arg_xbootldr_path, freep); +STATIC_DESTRUCTOR_REGISTER(arg_install_layout, freep); static const char *arg_dollar_boot_path(void) { /* $BOOT shall be the XBOOTLDR partition if it exists, and otherwise the ESP */ @@ -119,17 +124,68 @@ static int acquire_xbootldr(bool unprivileged_mode, sd_id128_t *ret_uuid) { return 1; } -static void settle_make_machine_id_directory(void) { +static int load_install_machine_id_and_layout(void) { + /* Figure out the right machine-id for operations. If KERNEL_INSTALL_MACHINE_ID is configured in + * /etc/machine-info, let's use that. Otherwise, just use the real machine-id. + * + * Also load KERNEL_INSTALL_LAYOUT. + */ + _cleanup_free_ char *s = NULL, *layout = NULL; int r; - if (arg_make_machine_id_directory >= 0) - return; + r = parse_env_file(NULL, "/etc/machine-info", + "KERNEL_INSTALL_LAYOUT", &layout, + "KERNEL_INSTALL_MACHINE_ID", &s); + if (r < 0 && r != -ENOENT) + return log_error_errno(r, "Failed to parse /etc/machine-info: %m"); - r = path_is_temporary_fs("/etc/machine-id"); + if (isempty(s)) { + r = sd_id128_get_machine(&arg_machine_id); + if (r < 0) + return log_error_errno(r, "Failed to get machine-id: %m"); + } else { + r = sd_id128_from_string(s, &arg_machine_id); + if (r < 0) + return log_error_errno(r, "Failed to parse KERNEL_INSTALL_MACHINE_ID=%s in /etc/machine-info: %m", s); + + } + log_debug("Using KERNEL_INSTALL_MACHINE_ID=%s from %s.", + SD_ID128_TO_STRING(arg_machine_id), + isempty(s) ? "/etc/machine_id" : "KERNEL_INSTALL_MACHINE_ID in /etc/machine-info"); + + if (!isempty(layout)) { + log_debug("KERNEL_INSTALL_LAYOUT=%s is specified in /etc/machine-info.", layout); + arg_install_layout = TAKE_PTR(layout); + } + + return 0; +} + +static int settle_install_machine_id(void) { + int r; + + r = load_install_machine_id_and_layout(); if (r < 0) - log_debug_errno(r, "Couldn't determine whether /etc/machine-id is on a temporary file system, assuming so."); + return r; - arg_make_machine_id_directory = r == 0; + bool layout_non_bls = arg_install_layout && !streq(arg_install_layout, "bls"); + if (arg_make_machine_id_directory < 0) { + if (layout_non_bls) + arg_make_machine_id_directory = 0; + else { + r = path_is_temporary_fs("/etc/machine-id"); + if (r < 0) + return log_debug_errno(r, "Couldn't determine whether /etc/machine-id is on a temporary file system: %m"); + arg_make_machine_id_directory = r == 0; + } + } + + if (arg_make_machine_id_directory > 0 && layout_non_bls) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), + "KERNEL_INSTALL_LAYOUT=%s is configured, but bls directory creation was requested.", + arg_install_layout); + + return 0; } /* search for "#### LoaderInfo: systemd-boot 218 ####" string inside the binary */ @@ -924,20 +980,14 @@ static int remove_subdirs(const char *root, const char *const *subdirs) { } static int remove_machine_id_directory(const char *root) { - sd_id128_t machine_id; - int r; - assert(root); assert(arg_make_machine_id_directory >= 0); + assert(!sd_id128_is_null(arg_machine_id)); if (!arg_make_machine_id_directory) return 0; - r = sd_id128_get_machine(&machine_id); - if (r < 0) - return log_error_errno(r, "Failed to get machine id: %m"); - - return rmdir_one(root, SD_ID128_TO_STRING(machine_id)); + return rmdir_one(root, SD_ID128_TO_STRING(arg_machine_id)); } static int remove_binaries(const char *esp_path) { @@ -1047,13 +1097,8 @@ static int install_loader_config(const char *esp_path) { "#console-mode keep\n"); if (arg_make_machine_id_directory) { - sd_id128_t machine_id; - - r = sd_id128_get_machine(&machine_id); - if (r < 0) - return log_error_errno(r, "Failed to get machine id: %m"); - - fprintf(f, "default %s-*\n", SD_ID128_TO_STRING(machine_id)); + assert(!sd_id128_is_null(arg_machine_id)); + fprintf(f, "default %s-*\n", SD_ID128_TO_STRING(arg_machine_id)); } r = fflush_sync_and_check(f); @@ -1071,20 +1116,100 @@ static int install_loader_config(const char *esp_path) { } static int install_machine_id_directory(const char *root) { - sd_id128_t machine_id; - int r; - assert(root); assert(arg_make_machine_id_directory >= 0); if (!arg_make_machine_id_directory) return 0; - r = sd_id128_get_machine(&machine_id); - if (r < 0) - return log_error_errno(r, "Failed to get machine id: %m"); + assert(!sd_id128_is_null(arg_machine_id)); + return mkdir_one(root, SD_ID128_TO_STRING(arg_machine_id)); +} - return mkdir_one(root, SD_ID128_TO_STRING(machine_id)); +static int install_machine_info_config(void) { + _cleanup_free_ char *contents = NULL; + size_t length; + bool need_install_layout = true, need_machine_id; + int r; + + assert(arg_make_machine_id_directory >= 0); + + /* We only want to save the machine-id if we created any directories using it. */ + need_machine_id = arg_make_machine_id_directory; + + _cleanup_fclose_ FILE *orig = fopen("/etc/machine-info", "re"); + if (!orig && errno != ENOENT) + return log_error_errno(errno, "Failed to open /etc/machine-info: %m"); + + if (orig) { + _cleanup_free_ char *install_layout = NULL, *machine_id = NULL; + + r = parse_env_file(orig, "/etc/machine-info", + "KERNEL_INSTALL_LAYOUT", &install_layout, + "KERNEL_INSTALL_MACHINE_ID", &machine_id); + if (r < 0) + return log_error_errno(r, "Failed to parse /etc/machine-info: %m"); + + rewind(orig); + + if (!isempty(install_layout)) + need_install_layout = false; + + if (!isempty(machine_id)) + need_machine_id = false; + + if (!need_install_layout && !need_machine_id) { + log_debug("/etc/machine-info already has KERNEL_INSTALL_MACHINE_ID=%s and KERNEL_INSTALL_LAYOUT=%s.", + machine_id, install_layout); + return 0; + } + + r = read_full_stream(orig, &contents, &length); + if (r < 0) + return log_error_errno(r, "Failed to read /etc/machine-info: %m"); + } + + _cleanup_(unlink_and_freep) char *dst_tmp = NULL; + _cleanup_fclose_ FILE *dst = NULL; + r = fopen_temporary_label("/etc/machine-info", /* The path for which to the look up the label */ + "/etc/machine-info", /* Where we want the file actually to end up */ + &dst, /* The temporary file we write to */ + &dst_tmp); + if (r < 0) + return log_debug_errno(r, "Failed to open temporary copy of /etc/machine-info: %m"); + + if (contents) + fwrite_unlocked(contents, 1, length, dst); + + bool no_newline = !contents || contents[length - 1] == '\n'; + + if (need_install_layout) { + const char *line = "\nKERNEL_INSTALL_LAYOUT=bls\n" + no_newline; + fwrite_unlocked(line, 1, strlen(line), dst); + no_newline = false; + } + + const char *mid_string = SD_ID128_TO_STRING(arg_machine_id); + if (need_machine_id) + fprintf(dst, "%sKERNEL_INSTALL_MACHINE_ID=%s\n", + no_newline ? "" : "\n", + mid_string); + + r = fflush_and_check(dst); + if (r < 0) + return log_error_errno(r, "Failed to write temporary copy of /etc/machine-info: %m"); + if (fchmod(fileno(dst), 0644) < 0) + return log_debug_errno(errno, "Failed to fchmod %s: %m", dst_tmp); + + if (rename(dst_tmp, "/etc/machine-info") < 0) + return log_error_errno(errno, "Failed to replace /etc/machine-info: %m"); + + log_info("%s /etc/machine-info with%s%s%s", + orig ? "Updated" : "Created", + need_install_layout ? " KERNEL_INSTALL_LAYOUT=bls" : "", + need_machine_id ? " KERNEL_INSTALL_MACHINE_ID=" : "", + need_machine_id ? mid_string : ""); + return 0; } static int help(int argc, char *argv[], void *userdata) { @@ -1637,7 +1762,9 @@ static int verb_install(int argc, char *argv[], void *userdata) { if (r < 0) return r; - settle_make_machine_id_directory(); + r = settle_install_machine_id(); + if (r < 0) + return r; RUN_WITH_UMASK(0002) { if (install) { @@ -1666,6 +1793,10 @@ static int verb_install(int argc, char *argv[], void *userdata) { if (r < 0) return r; + r = install_machine_info_config(); + if (r < 0) + return r; + r = install_random_seed(arg_esp_path); if (r < 0) return r; @@ -1695,7 +1826,9 @@ static int verb_remove(int argc, char *argv[], void *userdata) { if (r < 0) return r; - settle_make_machine_id_directory(); + r = settle_install_machine_id(); + if (r < 0) + return r; r = remove_binaries(arg_esp_path); diff --git a/src/sysusers/sysusers.c b/src/sysusers/sysusers.c index c3a9e54f3f..015e3a9ae4 100644 --- a/src/sysusers/sysusers.c +++ b/src/sysusers/sysusers.c @@ -269,10 +269,10 @@ static int make_backup(const char *target, const char *x) { return -errno; r = fopen_temporary_label( - target, /* The path for which to the lookup the label */ + target, /* The path for which to the look up the label */ x, /* Where we want the file actually to end up */ - &dst, - &dst_tmp /* The temporary file we write to */); + &dst, /* The temporary file we write to */ + &dst_tmp); if (r < 0) return r; From 1ff493d5c1155ed791a1ae2e0264b5f2213c5765 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 17 Dec 2021 18:37:08 +0100 Subject: [PATCH 2/2] bootctl: deprecate --make-machine-id-directory=auto Now that kernel-install creates the machine-id directory, we don't need to do this is 'bootctl install', and in fact it's better not to do this since it might never be necessary. So let's change the default behaviour to 'no'. I kept support for 'auto' to maintain backwards compatibility, even though the default was changed. Previous behaviour can be requested by specifying --make-machine-id-directory=auto. --- man/bootctl.xml | 7 ++----- src/boot/bootctl.c | 6 +++--- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/man/bootctl.xml b/man/bootctl.xml index 91dbb3a6e8..16c4985796 100644 --- a/man/bootctl.xml +++ b/man/bootctl.xml @@ -258,14 +258,11 @@ - + Control creation and deletion of the top-level machine ID directory on the file system containing boot loader entries (i.e. beneath the file system returned by the option, see above) during and - , respectively. auto is equivalent to - yes if /etc/machine-id resides on a filesystem other than - tmpfs and no otherwise (in the latter case the machine ID is likely transient and - hence should not be used persistently in the ESP). Defaults to auto. See + , respectively. Defaults to no. See machine-id5 for details about the machine ID concept and file. diff --git a/src/boot/bootctl.c b/src/boot/bootctl.c index 8f89a309e8..e1e56cf923 100644 --- a/src/boot/bootctl.c +++ b/src/boot/bootctl.c @@ -55,7 +55,7 @@ static bool arg_print_dollar_boot_path = false; static bool arg_touch_variables = true; static PagerFlags arg_pager_flags = 0; static bool arg_graceful = false; -static int arg_make_machine_id_directory = -1; +static int arg_make_machine_id_directory = 0; static sd_id128_t arg_machine_id = SD_ID128_NULL; static char *arg_install_layout = NULL; @@ -1347,8 +1347,8 @@ static int parse_argv(int argc, char *argv[]) { break; case ARG_MAKE_MACHINE_ID_DIRECTORY: - if (streq(optarg, "auto")) - arg_make_machine_id_directory = -1; /* default */ + if (streq(optarg, "auto")) /* retained for backwards compatibility */ + arg_make_machine_id_directory = -1; /* yes if machine-id is permanent */ else { r = parse_boolean_argument("--make-machine-id-directory=", optarg, &b); if (r < 0)