From 4c56862170889020300dec147a6e522d2c0b819c Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 25 Mar 2024 22:15:51 +0900 Subject: [PATCH 1/4] journalctl-authenticate: use is_dir() and refuse symlink for /var/log/journal I am not sure it is explicitly documented that /var/log/journal should be a directory, rather than a symlink to a directory, but the current code of journald seems not to support symlinked directory well. Let's refuse that at least here and now. --- src/journal/journalctl-authenticate.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/src/journal/journalctl-authenticate.c b/src/journal/journalctl-authenticate.c index 79f09b1fb0..de14a057ab 100644 --- a/src/journal/journalctl-authenticate.c +++ b/src/journal/journalctl-authenticate.c @@ -13,6 +13,7 @@ #include "memstream-util.h" #include "qrcode-util.h" #include "random-util.h" +#include "stat-util.h" #include "terminal-util.h" #include "tmpfile-util.h" @@ -63,21 +64,19 @@ int action_setup_keys(void) { uint8_t *mpk, *seed, *state; _cleanup_close_ int fd = -EBADF; sd_id128_t machine, boot; - struct stat st; uint64_t n; int r; assert(arg_action == ACTION_SETUP_KEYS); - r = stat("/var/log/journal", &st); - if (r < 0 && !IN_SET(errno, ENOENT, ENOTDIR)) - return log_error_errno(errno, "stat(\"%s\") failed: %m", "/var/log/journal"); - - if (r < 0 || !S_ISDIR(st.st_mode)) { - log_error("%s is not a directory, must be using persistent logging for FSS.", - "/var/log/journal"); - return r < 0 ? -errno : -ENOTDIR; - } + r = is_dir("/var/log/journal/", /* follow = */ false); + if (r == 0) + return log_error_errno(SYNTHETIC_ERRNO(ENOTDIR), + "/var/log/journal is not a directory, must be using persistent logging for FSS."); + if (r == -ENOENT) + return log_error_errno(r, "Directory /var/log/journal/ does not exist, must be using persistent logging for FSS."); + if (r < 0) + return log_error_errno(r, "Failed to check if /var/log/journal/ is a directory: %m"); r = sd_id128_get_machine(&machine); if (r < 0) From 2dac05e62b256653fabb6e555c5e348130455822 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 26 Mar 2024 22:40:43 +0900 Subject: [PATCH 2/4] journalctl-authenticate: drop unnecessary safe_close() Follow-up for 7560fffcd2531786b9c1ca657667a43e90331326. Addresses https://github.com/systemd/systemd/pull/31879#discussion_r1539063239. --- src/journal/journalctl-authenticate.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/journal/journalctl-authenticate.c b/src/journal/journalctl-authenticate.c index de14a057ab..c31c30a2db 100644 --- a/src/journal/journalctl-authenticate.c +++ b/src/journal/journalctl-authenticate.c @@ -127,7 +127,6 @@ int action_setup_keys(void) { n = now(CLOCK_REALTIME); n /= arg_interval; - safe_close(fd); fd = mkostemp_safe(k); if (fd < 0) return log_error_errno(fd, "Failed to open %s: %m", k); From ef277c961fe1fe94bf592fd684884c5c3b3d7c82 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 26 Mar 2024 23:39:43 +0900 Subject: [PATCH 3/4] journalctl-authenticate: use open_tmpfile_linkable() and link_tmpfile() This also - use path_join(), - rename variables to more descriptive names. --- src/journal/journalctl-authenticate.c | 39 ++++++++++++--------------- 1 file changed, 17 insertions(+), 22 deletions(-) diff --git a/src/journal/journalctl-authenticate.c b/src/journal/journalctl-authenticate.c index c31c30a2db..9217454b88 100644 --- a/src/journal/journalctl-authenticate.c +++ b/src/journal/journalctl-authenticate.c @@ -58,11 +58,11 @@ static int format_journal_url( } int action_setup_keys(void) { - size_t mpk_size, seed_size, state_size; - _cleanup_(unlink_and_freep) char *k = NULL; - _cleanup_free_ char *p = NULL; - uint8_t *mpk, *seed, *state; + _cleanup_(unlink_and_freep) char *tmpfile = NULL; _cleanup_close_ int fd = -EBADF; + _cleanup_free_ char *path = NULL; + size_t mpk_size, seed_size, state_size; + uint8_t *mpk, *seed, *state; sd_id128_t machine, boot; uint64_t n; int r; @@ -86,21 +86,16 @@ int action_setup_keys(void) { if (r < 0) return log_error_errno(r, "Failed to get boot ID: %m"); - if (asprintf(&p, "/var/log/journal/" SD_ID128_FORMAT_STR "/fss", - SD_ID128_FORMAT_VAL(machine)) < 0) + path = path_join("/var/log/journal/", SD_ID128_TO_STRING(machine), "/fss"); + if (!path) return log_oom(); if (arg_force) { - r = unlink(p); - if (r < 0 && errno != ENOENT) - return log_error_errno(errno, "unlink(\"%s\") failed: %m", p); - } else if (access(p, F_OK) >= 0) + if (unlink(path) < 0 && errno != ENOENT) + return log_error_errno(errno, "Failed to remove \"%s\": %m", path); + } else if (access(path, F_OK) >= 0) return log_error_errno(SYNTHETIC_ERRNO(EEXIST), - "Sealing key file %s exists already. Use --force to recreate.", p); - - if (asprintf(&k, "/var/log/journal/" SD_ID128_FORMAT_STR "/fss.tmp.XXXXXX", - SD_ID128_FORMAT_VAL(machine)) < 0) - return log_oom(); + "Sealing key file %s exists already. Use --force to recreate.", path); mpk_size = FSPRG_mskinbytes(FSPRG_RECOMMENDED_SECPAR); mpk = alloca_safe(mpk_size); @@ -123,18 +118,17 @@ int action_setup_keys(void) { FSPRG_GenState0(state, mpk, seed, seed_size); assert(arg_interval > 0); - n = now(CLOCK_REALTIME); n /= arg_interval; - fd = mkostemp_safe(k); + fd = open_tmpfile_linkable(path, O_WRONLY|O_CLOEXEC, &tmpfile); if (fd < 0) - return log_error_errno(fd, "Failed to open %s: %m", k); + return log_error_errno(fd, "Failed to open a temporary file for %s: %m", path); r = chattr_secret(fd, CHATTR_WARN_UNSUPPORTED_FLAGS); if (r < 0) log_full_errno(ERRNO_IS_NOT_SUPPORTED(r) ? LOG_DEBUG : LOG_WARNING, - r, "Failed to set file attributes on '%s', ignoring: %m", k); + r, "Failed to set file attributes on a temporary file for '%s', ignoring: %m", path); struct FSSHeader h = { .signature = { 'K', 'S', 'H', 'H', 'R', 'H', 'L', 'P' }, @@ -155,10 +149,11 @@ int action_setup_keys(void) { if (r < 0) return log_error_errno(r, "Failed to write state: %m"); - if (rename(k, p) < 0) - return log_error_errno(errno, "Failed to link file: %m"); + r = link_tmpfile(fd, tmpfile, path, /* flags = */ 0); + if (r < 0) + return log_error_errno(r, "Failed to link file: %m"); - k = mfree(k); + tmpfile = mfree(tmpfile); _cleanup_free_ char *hn = NULL, *key = NULL; From 66a0fdcaa5f865bd183b86eac655ca56e9921762 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 26 Mar 2024 23:42:21 +0900 Subject: [PATCH 4/4] journalctl-authenticate: return earlier if we are not on a TTY No functional change, just refactoring. --- src/journal/journalctl-authenticate.c | 105 ++++++++++++-------------- 1 file changed, 49 insertions(+), 56 deletions(-) diff --git a/src/journal/journalctl-authenticate.c b/src/journal/journalctl-authenticate.c index 9217454b88..40d1032388 100644 --- a/src/journal/journalctl-authenticate.c +++ b/src/journal/journalctl-authenticate.c @@ -11,35 +11,31 @@ #include "journalctl.h" #include "journalctl-authenticate.h" #include "memstream-util.h" +#include "path-util.h" #include "qrcode-util.h" #include "random-util.h" #include "stat-util.h" #include "terminal-util.h" #include "tmpfile-util.h" -static int format_journal_url( +static int format_key( const void *seed, size_t seed_size, uint64_t start, uint64_t interval, - const char *hn, - sd_id128_t machine, - bool full, - char **ret_url) { + char **ret) { _cleanup_(memstream_done) MemStream m = {}; FILE *f; assert(seed); assert(seed_size > 0); + assert(ret); f = memstream_init(&m); if (!f) return -ENOMEM; - if (full) - fputs("fss://", f); - for (size_t i = 0; i < seed_size; i++) { if (i > 0 && i % 3 == 0) fputc('-', f); @@ -48,13 +44,7 @@ static int format_journal_url( fprintf(f, "/%"PRIx64"-%"PRIx64, start, interval); - if (full) { - fprintf(f, "?machine=" SD_ID128_FORMAT_STR, SD_ID128_FORMAT_VAL(machine)); - if (hn) - fprintf(f, ";hostname=%s", hn); - } - - return memstream_finalize(&m, ret_url, NULL); + return memstream_finalize(&m, ret, NULL); } int action_setup_keys(void) { @@ -155,56 +145,59 @@ int action_setup_keys(void) { tmpfile = mfree(tmpfile); - _cleanup_free_ char *hn = NULL, *key = NULL; - - r = format_journal_url(seed, seed_size, n, arg_interval, hn, machine, false, &key); + _cleanup_free_ char *key = NULL; + r = format_key(seed, seed_size, n, arg_interval, &key); if (r < 0) return r; - if (on_tty()) { - hn = gethostname_malloc(); - if (hn) - hostname_cleanup(hn); - - fprintf(stderr, - "\nNew keys have been generated for host %s%s" SD_ID128_FORMAT_STR ".\n" - "\n" - "The %ssecret sealing key%s has been written to the following local file.\n" - "This key file is automatically updated when the sealing key is advanced.\n" - "It should not be used on multiple hosts.\n" - "\n" - "\t%s\n" - "\n" - "The sealing key is automatically changed every %s.\n" - "\n" - "Please write down the following %ssecret verification key%s. It should be stored\n" - "in a safe location and should not be saved locally on disk.\n" - "\n\t%s", - strempty(hn), hn ? "/" : "", - SD_ID128_FORMAT_VAL(machine), - ansi_highlight(), ansi_normal(), - p, - FORMAT_TIMESPAN(arg_interval, 0), - ansi_highlight(), ansi_normal(), - ansi_highlight_red()); - fflush(stderr); + if (!on_tty()) { + /* If we are not on a TTY, show only the key. */ + puts(key); + return 0; } + _cleanup_free_ char *hn = NULL; + hn = gethostname_malloc(); + if (hn) + hostname_cleanup(hn); + + fprintf(stderr, + "\nNew keys have been generated for host %s%s" SD_ID128_FORMAT_STR ".\n" + "\n" + "The %ssecret sealing key%s has been written to the following local file.\n" + "This key file is automatically updated when the sealing key is advanced.\n" + "It should not be used on multiple hosts.\n" + "\n" + "\t%s\n" + "\n" + "The sealing key is automatically changed every %s.\n" + "\n" + "Please write down the following %ssecret verification key%s. It should be stored\n" + "in a safe location and should not be saved locally on disk.\n" + "\n\t%s", + strempty(hn), hn ? "/" : "", + SD_ID128_FORMAT_VAL(machine), + ansi_highlight(), ansi_normal(), + path, + FORMAT_TIMESPAN(arg_interval, 0), + ansi_highlight(), ansi_normal(), + ansi_highlight_red()); + fflush(stderr); + puts(key); - if (on_tty()) { - fprintf(stderr, "%s", ansi_normal()); -#if HAVE_QRENCODE - _cleanup_free_ char *url = NULL; - r = format_journal_url(seed, seed_size, n, arg_interval, hn, machine, true, &url); - if (r < 0) - return r; + fputs(ansi_normal(), stderr); - (void) print_qrcode(stderr, - "To transfer the verification key to your phone scan the QR code below", - url); +#if HAVE_QRENCODE + _cleanup_free_ char *url = NULL; + url = strjoin("fss://", key, "?machine=", SD_ID128_TO_STRING(machine), hn ? ";hostname=" : "", hn); + if (!url) + return log_oom(); + + (void) print_qrcode(stderr, + "To transfer the verification key to your phone scan the QR code below", + url); #endif - } return 0; }