From 3daeef088410cdddef622007f95b0a1b4a439532 Mon Sep 17 00:00:00 2001 From: "Jason A. Donenfeld" Date: Wed, 16 Nov 2022 19:27:50 +0100 Subject: [PATCH 1/3] random-seed: handle post-merge review nits These are various misc things that came up after merging. --- src/boot/bootctl.c | 2 +- src/boot/efi/random-seed.c | 6 ++-- src/boot/efi/util.h | 4 +-- src/random-seed/random-seed.c | 61 ++++++++++++++++++----------------- 4 files changed, 38 insertions(+), 35 deletions(-) diff --git a/src/boot/bootctl.c b/src/boot/bootctl.c index e04424b379..afda914d52 100644 --- a/src/boot/bootctl.c +++ b/src/boot/bootctl.c @@ -1975,7 +1975,7 @@ static int verb_list(int argc, char *argv[], void *userdata) { static int install_random_seed(const char *esp) { _cleanup_(unlink_and_freep) char *tmp = NULL; - unsigned char buffer[RANDOM_EFI_SEED_SIZE]; + uint8_t buffer[RANDOM_EFI_SEED_SIZE]; _cleanup_free_ char *path = NULL; _cleanup_close_ int fd = -1; size_t token_size; diff --git a/src/boot/efi/random-seed.c b/src/boot/efi/random-seed.c index 04bfd526f8..02f4dfbc7f 100644 --- a/src/boot/efi/random-seed.c +++ b/src/boot/efi/random-seed.c @@ -59,7 +59,6 @@ static EFI_STATUS acquire_system_token(void **ret, UINTN *ret_size) { assert(ret); assert(ret_size); - *ret_size = 0; err = efivar_get_raw(LOADER_GUID, L"LoaderSystemToken", &data, &size); if (err != EFI_SUCCESS) { if (err != EFI_NOT_FOUND) @@ -187,6 +186,7 @@ EFI_STATUS process_random_seed(EFI_FILE *root_dir, RandomSeedMode mode) { /* Get some system specific seed that the installer might have placed in an EFI variable. We include * it in our hash. This is protection against golden master image sloppiness, and it remains on the * system, even when disk images are duplicated or swapped out. */ + size = 0; err = acquire_system_token(&system_token, &size); if (mode != RANDOM_SEED_ALWAYS && (err != EFI_SUCCESS || size < DESIRED_SEED_SIZE) && !seeded_by_efi) return err; @@ -246,6 +246,7 @@ EFI_STATUS process_random_seed(EFI_FILE *root_dir, RandomSeedMode mode) { size = sizeof(uefi_monotonic_counter); sha256_process_bytes(&size, sizeof(size), &hash); sha256_process_bytes(&uefi_monotonic_counter, size, &hash); + err = RT->GetTime(&now, NULL); size = err == EFI_SUCCESS ? sizeof(now) : 0; /* Known to be flaky, so don't bark on error. */ sha256_process_bytes(&size, sizeof(size), &hash); @@ -295,7 +296,8 @@ EFI_STATUS process_random_seed(EFI_FILE *root_dir, RandomSeedMode mode) { if (err != EFI_SUCCESS) return log_error_status_stall(err, L"Failed to flush random seed file: %r", err); - err = BS->AllocatePool(EfiACPIReclaimMemory, sizeof(*new_seed_table) + DESIRED_SEED_SIZE, + err = BS->AllocatePool(EfiACPIReclaimMemory, + offsetof(struct linux_efi_random_seed, seed) + DESIRED_SEED_SIZE, (void **) &new_seed_table); if (err != EFI_SUCCESS) return log_error_status_stall(err, L"Failed to allocate EFI table for random seed: %r", err); diff --git a/src/boot/efi/util.h b/src/boot/efi/util.h index 15dd87f774..eed28c0342 100644 --- a/src/boot/efi/util.h +++ b/src/boot/efi/util.h @@ -20,7 +20,7 @@ __attribute__((noreturn)) extern void __assert_cl_failure__(void) __attribute__( __attribute__((noreturn)) extern void __assert_cl_failure__(void); #endif /* assert_cl generates a later-stage compile-time assertion when constant folding occurs. */ -#define assert_cl(condition) if (!(condition)) __assert_cl_failure__() +#define assert_cl(condition) ({ if (!(condition)) __assert_cl_failure__(); }) #else #define assert_cl(condition) assert(condition) #endif @@ -60,7 +60,7 @@ static inline void freep(void *p) { static __always_inline void erase_obj(void *p) { size_t l; - assert_cl(p != NULL); + assert_cl(p); l = __builtin_object_size(p, 0); assert_cl(l != (size_t) -1); explicit_bzero_safe(p, l); diff --git a/src/random-seed/random-seed.c b/src/random-seed/random-seed.c index ccb8e47e19..020840e0df 100644 --- a/src/random-seed/random-seed.c +++ b/src/random-seed/random-seed.c @@ -320,9 +320,10 @@ static int refresh_boot_seed(void) { struct sha256_ctx hash_state; _cleanup_free_ void *seed_file_bytes = NULL; _cleanup_free_ char *esp_path = NULL; - _cleanup_close_ int seed_fd = -1; + _cleanup_close_ int seed_fd = -1, dir_fd = -1; size_t len; - ssize_t r; + ssize_t n; + int r; assert_cc(RANDOM_EFI_SEED_SIZE == SHA256_DIGEST_SIZE); @@ -336,48 +337,48 @@ static int refresh_boot_seed(void) { return r; /* find_esp_and_warn() already logged */ } - seed_fd = chase_symlinks_and_open("/loader/random-seed", esp_path, - CHASE_PREFIX_ROOT|CHASE_PROHIBIT_SYMLINKS, - O_RDWR|O_CLOEXEC|O_NOCTTY, NULL); - if (seed_fd == -ENOENT) { + r = chase_symlinks("/loader", esp_path, CHASE_PREFIX_ROOT|CHASE_PROHIBIT_SYMLINKS, NULL, &dir_fd); + if (r < 0) { + if (r == -ENOENT) { + log_debug_errno(r, "Couldn't find ESP loader directory, so not updating ESP random seed."); + return 0; + } + return log_error_errno(r, "Failed to open ESP loader directory: %m"); + } + seed_fd = openat(dir_fd, "random-seed", O_NOFOLLOW|O_RDWR|O_CLOEXEC|O_NOCTTY); + if (seed_fd < 0 && errno == ENOENT) { uint64_t features; - r = efi_loader_get_features(&features); - if (r == 0 && FLAGS_SET(features, EFI_LOADER_FEATURE_RANDOM_SEED)) { - int dir_fd = chase_symlinks_and_open("/loader", esp_path, - CHASE_PREFIX_ROOT|CHASE_PROHIBIT_SYMLINKS, - O_DIRECTORY|O_CLOEXEC|O_NOCTTY, NULL); - if (dir_fd >= 0) { - seed_fd = openat(dir_fd, "random-seed", O_CREAT|O_EXCL|O_RDWR|O_CLOEXEC|O_NOCTTY, 0600); - close(dir_fd); - } + if (r == 0 && FLAGS_SET(features, EFI_LOADER_FEATURE_RANDOM_SEED)) + seed_fd = openat(dir_fd, "random-seed", O_CREAT|O_EXCL|O_RDWR|O_CLOEXEC|O_NOCTTY, 0600); + else { + log_debug_errno(seed_fd, "Couldn't find ESP random seed, and not booted with systemd-boot, so not updating ESP random seed."); + return 0; } } - if (seed_fd < 0) { - log_debug_errno(seed_fd, "Failed to open EFI seed path: %m"); - return 0; - } + if (seed_fd < 0) + return log_error_errno(errno, "Failed to open EFI seed path: %m"); r = random_seed_size(seed_fd, &len); if (r < 0) return log_error_errno(r, "Failed to determine EFI seed path length: %m"); seed_file_bytes = malloc(len); if (!seed_file_bytes) return log_oom(); - r = loop_read(seed_fd, seed_file_bytes, len, false); - if (r < 0) - return log_error_errno(r, "Failed to read EFI seed file: %m"); + n = loop_read(seed_fd, seed_file_bytes, len, false); + if (n < 0) + return log_error_errno(n, "Failed to read EFI seed file: %m"); /* Hash the old seed in so that we never regress in entropy. */ sha256_init_ctx(&hash_state); - sha256_process_bytes(&r, sizeof(r), &hash_state); - sha256_process_bytes(seed_file_bytes, r, &hash_state); + sha256_process_bytes(&n, sizeof(n), &hash_state); + sha256_process_bytes(seed_file_bytes, n, &hash_state); /* We're doing this opportunistically, so if the seeding dance before didn't manage to initialize the * RNG, there's no point in doing it here. Secondly, getrandom(GRND_NONBLOCK) has been around longer * than EFI seeding anyway, so there's no point in having non-getrandom() fallbacks here. So if this * fails, just return early to cut our losses. */ - r = getrandom(buffer, sizeof(buffer), GRND_NONBLOCK); - if (r < 0) { + n = getrandom(buffer, sizeof(buffer), GRND_NONBLOCK); + if (n < 0) { if (errno == EAGAIN) { log_debug_errno(errno, "Random pool not initialized yet, so skipping EFI seed update"); return 0; @@ -388,11 +389,11 @@ static int refresh_boot_seed(void) { } return log_error_errno(errno, "Failed to generate random bytes for EFI seed: %m"); } - assert(r == sizeof(buffer)); + assert(n == sizeof(buffer)); /* Hash the new seed into the state containing the old one to generate our final seed. */ - sha256_process_bytes(&r, sizeof(r), &hash_state); - sha256_process_bytes(buffer, r, &hash_state); + sha256_process_bytes(&n, sizeof(n), &hash_state); + sha256_process_bytes(buffer, n, &hash_state); sha256_finish_ctx(&hash_state, buffer); if (lseek(seed_fd, 0, SEEK_SET) < 0) @@ -404,7 +405,7 @@ static int refresh_boot_seed(void) { return log_error_errno(errno, "Failed to truncate EFI seed file: %m"); r = fsync_full(seed_fd); if (r < 0) - return log_error_errno(errno, "Failed to fsync EFI seed file: %m"); + return log_error_errno(r, "Failed to fsync EFI seed file: %m"); log_debug("Updated random seed in ESP"); return 0; From 5d29d07b342397a8ecc4bea96f53595a03dd94f1 Mon Sep 17 00:00:00 2001 From: "Jason A. Donenfeld" Date: Wed, 16 Nov 2022 19:34:53 +0100 Subject: [PATCH 2/3] boot: do not truncate random seed file There are concerns about the FAT file system driver exploding if we try to do this, so just leave the bytes zeroed out instead. --- src/boot/efi/random-seed.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/boot/efi/random-seed.c b/src/boot/efi/random-seed.c index 02f4dfbc7f..e6a317860d 100644 --- a/src/boot/efi/random-seed.c +++ b/src/boot/efi/random-seed.c @@ -263,7 +263,7 @@ EFI_STATUS process_random_seed(EFI_FILE *root_dir, RandomSeedMode mode) { sha256_finish_ctx(&hash, random_bytes); size = sizeof(random_bytes); - /* If the file size is too large, zero out the remaining bytes on disk, and then truncate. */ + /* If the file size is too large, zero out the remaining bytes on disk. */ if (size < info->FileSize) { err = handle->SetPosition(handle, size); if (err != EFI_SUCCESS) @@ -280,10 +280,17 @@ EFI_STATUS process_random_seed(EFI_FILE *root_dir, RandomSeedMode mode) { err = handle->SetPosition(handle, 0); if (err != EFI_SUCCESS) return log_error_status_stall(err, L"Failed to seek to beginning of random seed file: %r", err); - info->FileSize = size; - err = handle->SetInfo(handle, &GenericFileInfo, info->Size, info); - if (err != EFI_SUCCESS) - return log_error_status_stall(err, L"Failed to truncate random seed file: %r", err); + + /* We could truncate the file here with something like: + * + * info->FileSize = size; + * err = handle->SetInfo(handle, &GenericFileInfo, info->Size, info); + * if (err != EFI_SUCCESS) + * return log_error_status_stall(err, L"Failed to truncate random seed file: %r", err); + * + * But this is considered slightly risky, because EFI filesystem drivers are a little bit + * flimsy. So instead we rely on userspace eventually truncating this when it writes a new + * seed. For now the best we do is zero it. */ } /* Update the random seed on disk before we use it */ wsize = size; From 1a2b1e10bdbdc0ed5e2bb4168b63a3b16f9d1848 Mon Sep 17 00:00:00 2001 From: "Jason A. Donenfeld" Date: Wed, 16 Nov 2022 19:46:41 +0100 Subject: [PATCH 3/3] boot: only use __builtin_object_size with -O>0 __builtin_object_size() returns -1 with -O0, so disable this and warn about it instead. --- src/boot/efi/util.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/boot/efi/util.h b/src/boot/efi/util.h index eed28c0342..4c5b6cab13 100644 --- a/src/boot/efi/util.h +++ b/src/boot/efi/util.h @@ -10,7 +10,6 @@ #define UINTN_MAX (~(UINTN)0) #define INTN_MAX ((INTN)(UINTN_MAX>>1)) -#ifdef __OPTIMIZE__ #ifndef __has_attribute #define __has_attribute(x) 0 #endif @@ -21,9 +20,6 @@ __attribute__((noreturn)) extern void __assert_cl_failure__(void); #endif /* assert_cl generates a later-stage compile-time assertion when constant folding occurs. */ #define assert_cl(condition) ({ if (!(condition)) __assert_cl_failure__(); }) -#else -#define assert_cl(condition) assert(condition) -#endif /* gnu-efi format specifiers for integers are fixed to either 64bit with 'l' and 32bit without a size prefix. * We rely on %u/%d/%x to format regular ints, so ensure the size is what we expect. At the same time, we also @@ -59,11 +55,15 @@ static inline void freep(void *p) { #define _cleanup_free_ _cleanup_(freep) static __always_inline void erase_obj(void *p) { +#ifdef __OPTIMIZE__ size_t l; assert_cl(p); l = __builtin_object_size(p, 0); assert_cl(l != (size_t) -1); explicit_bzero_safe(p, l); +#else +#warning "Object will not be erased with -O0; do not release to production." +#endif } #define _cleanup_erase_ _cleanup_(erase_obj)