From da012db02d85ea990efcc3dbea9cf5473de85fe8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 4 Aug 2020 15:20:08 +0200 Subject: [PATCH 1/7] journalctl: reshuffle console message about fss keys We talked about the verification key, then about sealing keys, and then about the verification key again. Let's shorten things a bit, and divide the output in three paragraphs: one about the machine, one about the sealing keys, and one about verification keys and the qr code with them. --- src/journal/journal-qrcode.c | 4 ++-- src/journal/journalctl.c | 45 +++++++++++++++++------------------- 2 files changed, 23 insertions(+), 26 deletions(-) diff --git a/src/journal/journal-qrcode.c b/src/journal/journal-qrcode.c index dddbd7b381..8c8360853e 100644 --- a/src/journal/journal-qrcode.c +++ b/src/journal/journal-qrcode.c @@ -46,7 +46,7 @@ int print_qr_code( _cleanup_(dlclosep) void *dl = NULL; _cleanup_free_ char *url = NULL; _cleanup_fclose_ FILE *f = NULL; - size_t url_size = 0, i; + size_t url_size = 0; unsigned x, y; QRcode* qr; int r; @@ -79,7 +79,7 @@ int print_qr_code( fputs("fss://", f); - for (i = 0; i < seed_size; i++) { + for (size_t i = 0; i < seed_size; i++) { if (i > 0 && i % 3 == 0) fputc('-', f); fprintf(f, "%02x", ((uint8_t*) seed)[i]); diff --git a/src/journal/journalctl.c b/src/journal/journalctl.c index 79daa43494..9bca7fd782 100644 --- a/src/journal/journalctl.c +++ b/src/journal/journalctl.c @@ -1778,7 +1778,7 @@ static int add_syslog_identifier(sd_journal *j) { static int setup_keys(void) { #if HAVE_GCRYPT - size_t mpk_size, seed_size, state_size, i; + 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; @@ -1890,52 +1890,49 @@ static int setup_keys(void) { k = mfree(k); + _cleanup_free_ char *hn = NULL; + if (on_tty()) { + hn = gethostname_malloc(); + if (hn) + hostname_cleanup(hn); + + char tsb[FORMAT_TIMESPAN_MAX]; fprintf(stderr, + "\nNew keys have been generated for host %s%s" SD_ID128_FORMAT_STR ".\n" "\n" - "The new key pair has been generated. The %ssecret sealing key%s has been written to\n" - "the following local file. This key file is automatically updated when the\n" - "sealing key is advanced. It should not be used on multiple hosts.\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" - "at a safe location and should not be saved locally on disk.\n" + "in a safe location and should not be saved locally on disk.\n" "\n\t%s", + hn ?: "", hn ? "/" : "", SD_ID128_FORMAT_VAL(machine), ansi_highlight(), ansi_normal(), p, + format_timespan(tsb, sizeof(tsb), arg_interval, 0), ansi_highlight(), ansi_normal(), ansi_highlight_red()); fflush(stderr); } - for (i = 0; i < seed_size; i++) { + + for (size_t i = 0; i < seed_size; i++) { if (i > 0 && i % 3 == 0) putchar('-'); printf("%02x", ((uint8_t*) seed)[i]); } - printf("/%llx-%llx\n", (unsigned long long) n, (unsigned long long) arg_interval); if (on_tty()) { - _cleanup_free_ char *hn = NULL; - char tsb[FORMAT_TIMESPAN_MAX]; - - fprintf(stderr, - "%s\n" - "The sealing key is automatically changed every %s.\n", - ansi_normal(), - format_timespan(tsb, sizeof(tsb), arg_interval, 0)); - - hn = gethostname_malloc(); - if (hn) { - hostname_cleanup(hn); - fprintf(stderr, "\nThe keys have been generated for host %s/" SD_ID128_FORMAT_STR ".\n", hn, SD_ID128_FORMAT_VAL(machine)); - } else - fprintf(stderr, "\nThe keys have been generated for host " SD_ID128_FORMAT_STR ".\n", SD_ID128_FORMAT_VAL(machine)); - + fprintf(stderr, "%s", ansi_normal()); #if HAVE_QRENCODE (void) print_qr_code(stderr, - "\nTo transfer the verification key to your phone please scan the QR code below:\n\n", + "\nTo transfer the verification key to your phone scan the QR code below:\n", seed, seed_size, n, arg_interval, hn, machine); From 890e5a4d294a75f637a04456fa0ea01e1859f4fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 4 Aug 2020 17:47:00 +0200 Subject: [PATCH 2/7] test-nss: modernize a bit and print path to the loaded so file This makes it easier to figure out if we're looking at the right module. --- src/test/test-nss.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/test/test-nss.c b/src/test/test-nss.c index 6ba0f6887e..da122d92df 100644 --- a/src/test/test-nss.c +++ b/src/test/test-nss.c @@ -54,7 +54,7 @@ static const char* af_to_string(int family, char *buf, size_t buf_len) { return buf; } -static void* open_handle(const char* dir, const char* module, int flags) { +static void* open_handle(const char *dir, const char *module, int flags) { const char *path = NULL; void *handle; @@ -63,6 +63,7 @@ static void* open_handle(const char* dir, const char* module, int flags) { if (!path || access(path, F_OK) < 0) path = strjoina("libnss_", module, ".so.2"); + log_debug("Using %s", path); handle = dlopen(path, flags); if (!handle) log_error("Failed to load module %s: %s", module, dlerror()); @@ -70,10 +71,9 @@ static void* open_handle(const char* dir, const char* module, int flags) { } static int print_gaih_addrtuples(const struct gaih_addrtuple *tuples) { - const struct gaih_addrtuple *it; int n = 0; - for (it = tuples; it; it = it->next) { + for (const struct gaih_addrtuple *it = tuples; it; it = it->next) { _cleanup_free_ char *a = NULL; union in_addr_union u; int r; @@ -388,14 +388,13 @@ static int make_addresses(struct local_address **addresses) { return 0; } -static int test_one_module(const char* dir, +static int test_one_module(const char *dir, const char *module, char **names, struct local_address *addresses, int n_addresses) { void *handle; char **name; - int i; log_info("======== %s ========", module); @@ -406,7 +405,7 @@ static int test_one_module(const char* dir, STRV_FOREACH(name, names) test_byname(handle, module, *name); - for (i = 0; i < n_addresses; i++) + for (int i = 0; i < n_addresses; i++) test_byaddr(handle, module, &addresses[i].address, FAMILY_ADDRESS_SIZE(addresses[i].family), From 604b163a31712b7d3abd5b9627e07464adc31e38 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 3 Aug 2020 17:46:48 +0200 Subject: [PATCH 3/7] test-seccomp: minor simpification --- src/test/test-seccomp.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/test/test-seccomp.c b/src/test/test-seccomp.c index eec2779a9e..cebf5a1080 100644 --- a/src/test/test-seccomp.c +++ b/src/test/test-seccomp.c @@ -98,9 +98,6 @@ static void test_syscall_filter_set_find(void) { } static void test_filter_sets(void) { - unsigned i; - int r; - log_info("/* %s */", __func__); if (!is_seccomp_available()) { @@ -112,7 +109,7 @@ static void test_filter_sets(void) { return; } - for (i = 0; i < _SYSCALL_FILTER_SET_MAX; i++) { + for (unsigned i = 0; i < _SYSCALL_FILTER_SET_MAX; i++) { pid_t pid; log_info("Testing %s", syscall_filter_sets[i].name); @@ -121,7 +118,7 @@ static void test_filter_sets(void) { assert_se(pid >= 0); if (pid == 0) { /* Child? */ - int fd; + int fd, r; /* If we look at the default set (or one that includes it), allow-list instead of deny-list */ if (IN_SET(i, SYSCALL_FILTER_SET_DEFAULT, SYSCALL_FILTER_SET_SYSTEM_SERVICE)) From 08554d4725ff562d2465af53bba9812af525bee1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 4 Aug 2020 20:14:00 +0200 Subject: [PATCH 4/7] test-nss: do not assume all symbols are defined MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It is OK for some symbols to be missing. With this change, "test-nss sss" can be used to test nss-sss without crashing. $ build-rawhide/test-nss sss fedoraproject.org ======== sss ======== _nss_sss_gethostbyname4_r not defined _nss_sss_gethostbyname3_r not defined _nss_sss_gethostbyname3_r not defined _nss_sss_gethostbyname3_r not defined _nss_sss_gethostbyname3_r not defined _nss_sss_gethostbyname2_r("fedoraproject.org", AF_INET) → status=NSS_STATUS_NOTFOUND errno=0/--- h_errno=-1/Resolver internal error _nss_sss_gethostbyname2_r("fedoraproject.org", AF_INET6) → status=NSS_STATUS_NOTFOUND errno=0/--- h_errno=-1/Resolver internal error _nss_sss_gethostbyname2_r("fedoraproject.org", *) → status=NSS_STATUS_UNAVAIL errno=97/EAFNOSUPPORT h_errno=-1/Resolver internal error _nss_sss_gethostbyname2_r("fedoraproject.org", AF_UNIX) → status=NSS_STATUS_UNAVAIL errno=97/EAFNOSUPPORT h_errno=-1/Resolver internal error _nss_sss_gethostbyname_r("fedoraproject.org") → status=NSS_STATUS_NOTFOUND errno=0/--- h_errno=-1/Resolver internal error --- src/test/test-nss.c | 28 ++++++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/src/test/test-nss.c b/src/test/test-nss.c index da122d92df..293d7a8468 100644 --- a/src/test/test-nss.c +++ b/src/test/test-nss.c @@ -147,7 +147,10 @@ static void test_gethostbyname4_r(void *handle, const char *module, const char * fname = strjoina("_nss_", module, "_gethostbyname4_r"); f = dlsym(handle, fname); log_debug("dlsym(0x%p, %s) → 0x%p", handle, fname, f); - assert_se(f); + if (!f) { + log_info("%s not defined", fname); + return; + } status = f(name, &pat, buffer, sizeof buffer, &errno1, &errno2, &ttl); if (status == NSS_STATUS_SUCCESS) { @@ -197,7 +200,10 @@ static void test_gethostbyname3_r(void *handle, const char *module, const char * fname = strjoina("_nss_", module, "_gethostbyname3_r"); f = dlsym(handle, fname); log_debug("dlsym(0x%p, %s) → 0x%p", handle, fname, f); - assert_se(f); + if (!f) { + log_info("%s not defined", fname); + return; + } status = f(name, af, &host, buffer, sizeof buffer, &errno1, &errno2, &ttl, &canon); log_info("%s(\"%s\", %s) → status=%s%-20serrno=%d/%s h_errno=%d/%s ttl=%"PRIi32, @@ -223,7 +229,10 @@ static void test_gethostbyname2_r(void *handle, const char *module, const char * fname = strjoina("_nss_", module, "_gethostbyname2_r"); f = dlsym(handle, fname); log_debug("dlsym(0x%p, %s) → 0x%p", handle, fname, f); - assert_se(f); + if (!f) { + log_info("%s not defined", fname); + return; + } status = f(name, af, &host, buffer, sizeof buffer, &errno1, &errno2); log_info("%s(\"%s\", %s) → status=%s%-20serrno=%d/%s h_errno=%d/%s", @@ -247,7 +256,10 @@ static void test_gethostbyname_r(void *handle, const char *module, const char *n fname = strjoina("_nss_", module, "_gethostbyname_r"); f = dlsym(handle, fname); log_debug("dlsym(0x%p, %s) → 0x%p", handle, fname, f); - assert_se(f); + if (!f) { + log_info("%s not defined", fname); + return; + } status = f(name, &host, buffer, sizeof buffer, &errno1, &errno2); log_info("%s(\"%s\") → status=%s%-20serrno=%d/%s h_errno=%d/%s", @@ -279,8 +291,10 @@ static void test_gethostbyaddr2_r(void *handle, log_full_errno(f ? LOG_DEBUG : LOG_INFO, errno, "dlsym(0x%p, %s) → 0x%p: %m", handle, fname, f); - if (!f) + if (!f) { + log_info("%s not defined", fname); return; + } assert_se(in_addr_to_string(af, addr, &addr_pretty) >= 0); @@ -314,8 +328,10 @@ static void test_gethostbyaddr_r(void *handle, log_full_errno(f ? LOG_DEBUG : LOG_INFO, errno, "dlsym(0x%p, %s) → 0x%p: %m", handle, fname, f); - if (!f) + if (!f) { + log_info("%s not defined", fname); return; + } assert_se(in_addr_to_string(af, addr, &addr_pretty) >= 0); From ddd8e23dc9d6ab33b3b3c243369042a325ba618d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 5 Aug 2020 12:16:03 +0200 Subject: [PATCH 5/7] bless-boot: add missing verb to --help There is also "help" verb which duplicates the --help option. I don't think we should advertise it. --- src/boot/bless-boot.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/boot/bless-boot.c b/src/boot/bless-boot.c index b96e1f927f..0824266a80 100644 --- a/src/boot/bless-boot.c +++ b/src/boot/bless-boot.c @@ -34,6 +34,7 @@ static int help(int argc, char *argv[], void *userdata) { printf("%s [OPTIONS...] COMMAND\n" "\n%sMark the boot process as good or bad.%s\n" "\nCommands:\n" + " status Show status of current boot loader entry\n" " good Mark this boot as good\n" " bad Mark this boot as bad\n" " indeterminate Undo any marking as good or bad\n" From 4ea0782649975de63fe14359879bb710e236881c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 5 Aug 2020 19:44:37 +0200 Subject: [PATCH 6/7] docs: rework awkward sentence in AUTOMATIC_BOOT_ASSESSMENT --- docs/AUTOMATIC_BOOT_ASSESSMENT.md | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/docs/AUTOMATIC_BOOT_ASSESSMENT.md b/docs/AUTOMATIC_BOOT_ASSESSMENT.md index 83ddf28fdd..f6d63afcdf 100644 --- a/docs/AUTOMATIC_BOOT_ASSESSMENT.md +++ b/docs/AUTOMATIC_BOOT_ASSESSMENT.md @@ -10,12 +10,11 @@ systemd provides support for automatically reverting back to the previous version of the OS or kernel in case the system consistently fails to boot. This support is built into various of its components. When used together these components provide a complete solution on UEFI systems, built as add-on to the -[Boot Loader -Specification](https://systemd.io/BOOT_LOADER_SPECIFICATION). However, the -different components may also be used independently, and in combination with -other software, to implement similar schemes, for example with other boot -loaders or for non-UEFI systems. Here's a brief overview of the complete set of -components: +[Boot Loader Specification](https://systemd.io/BOOT_LOADER_SPECIFICATION). +However, the different components may also be used independently, and in +combination with other software, to implement similar schemes, for example with +other boot loaders or for non-UEFI systems. Here's a brief overview of the +complete set of components: * The [`systemd-boot(7)`](https://www.freedesktop.org/software/systemd/man/systemd-boot.html) @@ -45,11 +44,10 @@ components: * The `boot-complete.target` target unit (see [`systemd.special(7)`](https://www.freedesktop.org/software/systemd/man/systemd.special.html)) - serves as a generic extension point both for units that shall be considered - necessary to consider a boot successful on one side (example: - `systemd-boot-check-no-failures.service` as described above), and units that - want to act only if the boot is successful on the other (example: - `systemd-bless-boot.service` as described above). + serves as a generic extension point both for units that are necessary to + consider a boot successful (example: `systemd-boot-check-no-failures.service` + as described above), and units that want to act only if the boot is + successful (example: `systemd-bless-boot.service` as described above). * The [`kernel-install(8)`](https://www.freedesktop.org/software/systemd/man/kernel-install.html) From d08a529518c37734e4e77540844373249589fb78 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 5 Aug 2020 19:44:53 +0200 Subject: [PATCH 7/7] man: fix typo --- man/org.freedesktop.systemd1.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/man/org.freedesktop.systemd1.xml b/man/org.freedesktop.systemd1.xml index 6b16ae16da..8a7990ff7d 100644 --- a/man/org.freedesktop.systemd1.xml +++ b/man/org.freedesktop.systemd1.xml @@ -8778,7 +8778,7 @@ node /org/freedesktop/systemd1/unit/system_2eslice { Scope Unit Objects - All slice unit objects implement the org.freedesktop.systemd1.Scope + All scope unit objects implement the org.freedesktop.systemd1.Scope interface (described here) in addition to the generic org.freedesktop.systemd1.Unit interface (see above).