From 70def6fed3b6e1cb19aebf0fabfd7d96e2da942e Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Thu, 28 Sep 2023 11:56:27 +0100 Subject: [PATCH 1/8] mkosi: use different configs for Debian kernel package list The kernel package is named after the architecture, so builds will fail if mkosi --architecture arm64 is used --- .../{10-debian.conf => 10-debian-amd64.conf} | 1 + mkosi.presets/system/mkosi.conf.d/10-debian-arm64.conf | 10 ++++++++++ 2 files changed, 11 insertions(+) rename mkosi.presets/system/mkosi.conf.d/{10-debian.conf => 10-debian-amd64.conf} (87%) create mode 100644 mkosi.presets/system/mkosi.conf.d/10-debian-arm64.conf diff --git a/mkosi.presets/system/mkosi.conf.d/10-debian.conf b/mkosi.presets/system/mkosi.conf.d/10-debian-amd64.conf similarity index 87% rename from mkosi.presets/system/mkosi.conf.d/10-debian.conf rename to mkosi.presets/system/mkosi.conf.d/10-debian-amd64.conf index d4cd53e6f2c..d3c89f3a8c7 100644 --- a/mkosi.presets/system/mkosi.conf.d/10-debian.conf +++ b/mkosi.presets/system/mkosi.conf.d/10-debian-amd64.conf @@ -2,6 +2,7 @@ [Match] Distribution=debian +Architecture=x86-64 [Content] Packages= diff --git a/mkosi.presets/system/mkosi.conf.d/10-debian-arm64.conf b/mkosi.presets/system/mkosi.conf.d/10-debian-arm64.conf new file mode 100644 index 00000000000..76a68981113 --- /dev/null +++ b/mkosi.presets/system/mkosi.conf.d/10-debian-arm64.conf @@ -0,0 +1,10 @@ +# SPDX-License-Identifier: LGPL-2.1-or-later + +[Match] +Distribution=debian +Architecture=arm64 + +[Content] +Packages= + bpftool + linux-image-cloud-arm64 From d869ec4ab0063eca8697bc44ad655b559acb32cb Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Mon, 2 Oct 2023 01:17:58 +0100 Subject: [PATCH 2/8] efi: add EFI_TCG2_TAGGED_EVENT and helpers --- src/boot/efi/measure.c | 76 ++++++++++++++++++++++++++++++++++++++++ src/boot/efi/measure.h | 7 ++++ src/boot/efi/proto/tcg.h | 7 ++++ 3 files changed, 90 insertions(+) diff --git a/src/boot/efi/measure.c b/src/boot/efi/measure.c index f9aabb42be4..28ec6b7d825 100644 --- a/src/boot/efi/measure.c +++ b/src/boot/efi/measure.c @@ -42,6 +42,50 @@ static EFI_STATUS tpm1_measure_to_pcr_and_event_log( &event_log_last); } +static EFI_STATUS tpm2_measure_to_pcr_and_tagged_event_log( + EFI_TCG2_PROTOCOL *tcg, + uint32_t pcrindex, + EFI_PHYSICAL_ADDRESS buffer, + uint64_t buffer_size, + uint32_t event_id, + const char16_t *description) { + + _cleanup_free_ struct event { + EFI_TCG2_EVENT tcg_event; + EFI_TCG2_TAGGED_EVENT tcg_tagged_event; + } _packed_ *event = NULL; + size_t desc_len, event_size; + + assert(tcg); + assert(description); + + desc_len = strsize16(description); + event_size = offsetof(EFI_TCG2_EVENT, Event) + offsetof(EFI_TCG2_TAGGED_EVENT, Event) + desc_len; + + event = xmalloc0(event_size); + + *event = (struct event) { + .tcg_event = (EFI_TCG2_EVENT) { + .Size = event_size, + .Header.HeaderSize = sizeof(EFI_TCG2_EVENT_HEADER), + .Header.HeaderVersion = EFI_TCG2_EVENT_HEADER_VERSION, + .Header.PCRIndex = pcrindex, + .Header.EventType = EV_EVENT_TAG, + }, + .tcg_tagged_event = { + .EventId = event_id, + .EventSize = desc_len, + }, + }; + memcpy(event->tcg_tagged_event.Event, description, desc_len); + + return tcg->HashLogExtendEvent( + tcg, + 0, + buffer, buffer_size, + &event->tcg_event); +} + static EFI_STATUS tpm2_measure_to_pcr_and_event_log( EFI_TCG2_PROTOCOL *tcg, uint32_t pcrindex, @@ -185,6 +229,38 @@ EFI_STATUS tpm_log_event(uint32_t pcrindex, EFI_PHYSICAL_ADDRESS buffer, size_t return err; } +EFI_STATUS tpm_log_tagged_event( + uint32_t pcrindex, + EFI_PHYSICAL_ADDRESS buffer, + size_t buffer_size, + uint32_t event_id, + const char16_t *description, + bool *ret_measured) { + + EFI_TCG2_PROTOCOL *tpm2; + EFI_STATUS err; + + assert(description || pcrindex == UINT32_MAX); + assert(event_id > 0); + + /* If EFI_SUCCESS is returned, will initialize ret_measured to true if we actually measured + * something, or false if measurement was turned off. */ + + tpm2 = tcg2_interface_check(); + if (!tpm2 || pcrindex == UINT32_MAX) { /* PCR disabled? */ + if (ret_measured) + *ret_measured = false; + + return EFI_SUCCESS; + } + + err = tpm2_measure_to_pcr_and_tagged_event_log(tpm2, pcrindex, buffer, buffer_size, event_id, description); + if (err == EFI_SUCCESS && ret_measured) + *ret_measured = true; + + return err; +} + EFI_STATUS tpm_log_event_ascii(uint32_t pcrindex, EFI_PHYSICAL_ADDRESS buffer, size_t buffer_size, const char *description, bool *ret_measured) { _cleanup_free_ char16_t *c = NULL; diff --git a/src/boot/efi/measure.h b/src/boot/efi/measure.h index d252f0f832b..c3c4e0a9ad1 100644 --- a/src/boot/efi/measure.h +++ b/src/boot/efi/measure.h @@ -8,6 +8,7 @@ bool tpm_present(void); EFI_STATUS tpm_log_event(uint32_t pcrindex, EFI_PHYSICAL_ADDRESS buffer, size_t buffer_size, const char16_t *description, bool *ret_measured); EFI_STATUS tpm_log_event_ascii(uint32_t pcrindex, EFI_PHYSICAL_ADDRESS buffer, size_t buffer_size, const char *description, bool *ret_measured); +EFI_STATUS tpm_log_tagged_event(uint32_t pcrindex, EFI_PHYSICAL_ADDRESS buffer, size_t buffer_size, uint32_t event_id, const char16_t *description, bool *ret_measured); EFI_STATUS tpm_log_load_options(const char16_t *cmdline, bool *ret_measured); #else @@ -28,6 +29,12 @@ static inline EFI_STATUS tpm_log_event_ascii(uint32_t pcrindex, EFI_PHYSICAL_ADD return EFI_SUCCESS; } +static inline EFI_STATUS tpm_log_tagged_event(uint32_t pcrindex, EFI_PHYSICAL_ADDRESS buffer, size_t buffer_size, uint32_t event_id, const char16_t *description, bool *ret_measured) { + if (ret_measured) + *ret_measured = false; + return EFI_SUCCESS; +} + static inline EFI_STATUS tpm_log_load_options(const char16_t *cmdline, bool *ret_measured) { if (ret_measured) *ret_measured = false; diff --git a/src/boot/efi/proto/tcg.h b/src/boot/efi/proto/tcg.h index d8802ae0e33..b4b82962ef6 100644 --- a/src/boot/efi/proto/tcg.h +++ b/src/boot/efi/proto/tcg.h @@ -11,6 +11,7 @@ #define TCG_ALG_SHA 0x4 #define EFI_TCG2_EVENT_HEADER_VERSION 1 #define EV_IPL 13 +#define EV_EVENT_TAG UINT32_C(6) typedef struct { uint8_t Major; @@ -70,6 +71,12 @@ typedef struct { uint8_t Event[]; } _packed_ EFI_TCG2_EVENT; +typedef struct { + uint32_t EventId; + uint32_t EventSize; + uint8_t Event[]; +} _packed_ EFI_TCG2_TAGGED_EVENT; + typedef struct EFI_TCG_PROTOCOL EFI_TCG_PROTOCOL; struct EFI_TCG_PROTOCOL { EFI_STATUS (EFIAPI *StatusCheck)( From 3e5a499009df048a553a8d01358a1b59fb04aa15 Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Sat, 12 Aug 2023 23:14:13 +0100 Subject: [PATCH 3/8] efi: add xmemdup --- src/boot/efi/util.h | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/boot/efi/util.h b/src/boot/efi/util.h index 2f8c96d0ac6..25f35d2b5d7 100644 --- a/src/boot/efi/util.h +++ b/src/boot/efi/util.h @@ -58,6 +58,11 @@ static inline void *xrealloc(void *p, size_t old_size, size_t new_size) { return t; } +_malloc_ _alloc_(2) _returns_nonnull_ _warn_unused_result_ +static inline void* xmemdup(const void *p, size_t l) { + return memcpy(xmalloc(l), p, l); +} + #define xnew(type, n) ((type *) xmalloc_multiply(sizeof(type), (n))) typedef struct { From 3b66a6764ec738392b075a306a60eaabf2a34f21 Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Sat, 23 Sep 2023 18:29:32 +0100 Subject: [PATCH 4/8] Move CLEANUP_ARRAY to src/fundamental --- src/basic/alloc-util.h | 1 - src/basic/memory-util.h | 34 --------------------- src/fundamental/memory-util-fundamental.h | 36 +++++++++++++++++++++++ 3 files changed, 36 insertions(+), 35 deletions(-) diff --git a/src/basic/alloc-util.h b/src/basic/alloc-util.h index 3ef834aceee..4f86334d7d4 100644 --- a/src/basic/alloc-util.h +++ b/src/basic/alloc-util.h @@ -15,7 +15,6 @@ typedef void (*free_func_t)(void *p); typedef void* (*mfree_func_t)(void *p); -typedef void (*free_array_func_t)(void *p, size_t n); /* If for some reason more than 4M are allocated on the stack, let's abort immediately. It's better than * proceeding and smashing the stack limits. Note that by default RLIMIT_STACK is 8M on Linux. */ diff --git a/src/basic/memory-util.h b/src/basic/memory-util.h index c350dc1e54b..b5e3984a09a 100644 --- a/src/basic/memory-util.h +++ b/src/basic/memory-util.h @@ -104,37 +104,3 @@ static inline void erase_and_freep(void *p) { static inline void erase_char(char *p) { explicit_bzero_safe(p, sizeof(char)); } - -/* An automatic _cleanup_-like logic for destroy arrays (i.e. pointers + size) when leaving scope */ -typedef struct ArrayCleanup { - void **parray; - size_t *pn; - free_array_func_t pfunc; -} ArrayCleanup; - -static inline void array_cleanup(const ArrayCleanup *c) { - assert(c); - - assert(!c->parray == !c->pn); - - if (!c->parray) - return; - - if (*c->parray) { - assert(c->pfunc); - c->pfunc(*c->parray, *c->pn); - *c->parray = NULL; - } - - *c->pn = 0; -} - -#define CLEANUP_ARRAY(array, n, func) \ - _cleanup_(array_cleanup) _unused_ const ArrayCleanup CONCATENATE(_cleanup_array_, UNIQ) = { \ - .parray = (void**) &(array), \ - .pn = &(n), \ - .pfunc = (free_array_func_t) ({ \ - void (*_f)(typeof(array[0]) *a, size_t b) = func; \ - _f; \ - }), \ - } diff --git a/src/fundamental/memory-util-fundamental.h b/src/fundamental/memory-util-fundamental.h index 1ffacff21a9..6870f54f584 100644 --- a/src/fundamental/memory-util-fundamental.h +++ b/src/fundamental/memory-util-fundamental.h @@ -70,3 +70,39 @@ static inline void erase_varp(struct VarEraser *e) { .p = (ptr), \ .size = (sz), \ } + +typedef void (*free_array_func_t)(void *p, size_t n); + +/* An automatic _cleanup_-like logic for destroy arrays (i.e. pointers + size) when leaving scope */ +typedef struct ArrayCleanup { + void **parray; + size_t *pn; + free_array_func_t pfunc; +} ArrayCleanup; + +static inline void array_cleanup(const ArrayCleanup *c) { + assert(c); + + assert(!c->parray == !c->pn); + + if (!c->parray) + return; + + if (*c->parray) { + assert(c->pfunc); + c->pfunc(*c->parray, *c->pn); + *c->parray = NULL; + } + + *c->pn = 0; +} + +#define CLEANUP_ARRAY(array, n, func) \ + _cleanup_(array_cleanup) _unused_ const ArrayCleanup CONCATENATE(_cleanup_array_, UNIQ) = { \ + .parray = (void**) &(array), \ + .pn = &(n), \ + .pfunc = (free_array_func_t) ({ \ + void (*_f)(typeof(array[0]) *a, size_t b) = func; \ + _f; \ + }), \ + } From 68f85761e2eb1fd2243019980a64b174f07432c3 Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Mon, 7 Aug 2023 01:05:18 +0100 Subject: [PATCH 5/8] stub: add support for dtb addons Same as kernel command line addons. --- docs/TPM2_PCR_MEASUREMENTS.md | 10 + man/systemd-stub.xml | 33 ++-- src/boot/bootctl-status.c | 1 + src/boot/efi/stub.c | 260 ++++++++++++++++++++------ src/fundamental/efivars-fundamental.h | 1 + 5 files changed, 236 insertions(+), 69 deletions(-) diff --git a/docs/TPM2_PCR_MEASUREMENTS.md b/docs/TPM2_PCR_MEASUREMENTS.md index dffe2baaa02..f2893f13445 100644 --- a/docs/TPM2_PCR_MEASUREMENTS.md +++ b/docs/TPM2_PCR_MEASUREMENTS.md @@ -90,6 +90,16 @@ UTF-16. → **Measured hash** covers the literal kernel command line in UTF-16 (without any trailing NUL bytes). +### PCR 12, `EV_EVENT_TAG`, "Devicetrees" + +Devicetree addons are measured individually as a tagged event. + +→ **Event Tag** `0x6c46f751` + +→ **Description** the addon filename. + +→ **Measured hash** covers the content of the Devicetree. + ### PCR 12, `EV_IPL`, "Per-UKI Credentials initrd" → **Description** in the event log record is the constant string "Credentials diff --git a/man/systemd-stub.xml b/man/systemd-stub.xml index 1173db572b5..5650c53f021 100644 --- a/man/systemd-stub.xml +++ b/man/systemd-stub.xml @@ -167,19 +167,20 @@ ukify1 tool will add a SBAT policy by default if none is passed when building addons. For more information on SBAT see Shim's documentation. - Addons are supposed to be used to pass additional kernel command line parameters, regardless of the - kernel image being booted, for example to allow platform vendors to ship platform-specific - configuration. The loaded command line addon files are sorted, loaded, measured into TPM PCR 12 (if a - TPM is present) and appended to the kernel command line. UKI command line options are listed first, - then options from addons in /loader/addons/*.addon.efi are appended next, and - finally UKI-specific addons are appended last. Addons are always loaded in the same order based on the - filename, so that, given the same set of addons, the same set of measurements can be expected in - PCR12, however note that the filename is not protected by the PE signature, and as such an attacker - with write access to the ESP could potentially rename these files to change the order in which they - are loaded, in a way that could alter the functionality of the kernel, as some options might be order - dependent. If you sign such addons, you should pay attention to the PCR12 values and make use of an - attestation service so that improper use of your signed addons can be detected and dealt with using - one of the aforementioned revocation mechanisms. + Addons are supposed to be used to pass additional kernel command line parameters or Devicetree blobs, + regardless of the kernel image being booted, for example to allow platform vendors to ship + platform-specific configuration. The loaded command line addon files are sorted, loaded, and measured + into TPM PCR 12 (if a TPM is present) and appended to the kernel command line. UKI command line options + are listed first, then options from addons in /loader/addons/*.addon.efi, and + finally UKI-specific addons. Device tree blobs are loaded and measured following the same algorithm. + Addons are always loaded in the same order based on the filename, so that, given the same set of + addons, the same set of measurements can be expected in PCR12. However, note that the filename is not + protected by the PE signature, and as such an attacker with write access to the ESP could potentially + rename these files to change the order in which they are loaded, in a way that could alter the + functionality of the kernel, as some options might be order dependent. If you sign such addons, you + should pay attention to the PCR12 values and make use of an attestation service so that improper use + of your signed addons can be detected and dealt with using one of the aforementioned revocation + mechanisms. Files /loader/credentials/*.cred are packed up in a cpio archive and placed in the /.extra/global_credentials/ @@ -188,9 +189,9 @@ measured into TPM PCR 12 (if a TPM is present). Additionally, files /loader/addons/*.addon.efi are loaded and - verified as PE binaries, and a .cmdline section is parsed from them. This is - supposed to be used to pass additional command line parameters to the kernel, regardless of the kernel - being booted. + verified as PE binaries, and .cmdline and/or .dtb sections are + parsed from them. This is supposed to be used to pass additional command line parameters or Devicetree + blobs to the kernel, regardless of the kernel being booted. These mechanisms may be used to parameterize and extend trusted (i.e. signed), immutable initrd diff --git a/src/boot/bootctl-status.c b/src/boot/bootctl-status.c index 033abb05665..a1fe3364331 100644 --- a/src/boot/bootctl-status.c +++ b/src/boot/bootctl-status.c @@ -384,6 +384,7 @@ int verb_status(int argc, char *argv[], void *userdata) { { EFI_STUB_FEATURE_RANDOM_SEED, "Support for passing random seed to OS" }, { EFI_STUB_FEATURE_CMDLINE_ADDONS, "Pick up .cmdline from addons" }, { EFI_STUB_FEATURE_CMDLINE_SMBIOS, "Pick up .cmdline from SMBIOS Type 11" }, + { EFI_STUB_FEATURE_DEVICETREE_ADDONS, "Pick up .dtb from addons" }, }; _cleanup_free_ char *fw_type = NULL, *fw_info = NULL, *loader = NULL, *loader_path = NULL, *stub = NULL; sd_id128_t loader_part_uuid = SD_ID128_NULL; diff --git a/src/boot/efi/stub.c b/src/boot/efi/stub.c index 0f30f13d921..dd3d017cce8 100644 --- a/src/boot/efi/stub.c +++ b/src/boot/efi/stub.c @@ -26,6 +26,8 @@ DECLARE_NOALLOC_SECTION(".sdmagic", "#### LoaderInfo: systemd-stub " GIT_VERSION DECLARE_SBAT(SBAT_STUB_SECTION_TEXT); +#define ADDON_FILENAME_EVENT_TAG_ID UINT32_C(0x6c46f751) + static EFI_STATUS combine_initrd( EFI_PHYSICAL_ADDRESS initrd_base, size_t initrd_size, const void * const extra_initrds[], const size_t extra_initrd_sizes[], size_t n_extra_initrds, @@ -95,6 +97,7 @@ static void export_variables(EFI_LOADED_IMAGE_PROTOCOL *loaded_image) { EFI_STUB_FEATURE_RANDOM_SEED | /* We pass a random seed to the kernel */ EFI_STUB_FEATURE_CMDLINE_ADDONS | /* We pick up .cmdline addons */ EFI_STUB_FEATURE_CMDLINE_SMBIOS | /* We support extending kernel cmdline from SMBIOS Type #11 */ + EFI_STUB_FEATURE_DEVICETREE_ADDONS | /* We pick up .dtb addons */ 0; assert(loaded_image); @@ -253,29 +256,123 @@ static EFI_STATUS load_addons_from_dir( return EFI_SUCCESS; } -static EFI_STATUS cmdline_append_and_measure_addons( +static void cmdline_append_and_measure_addons( + char16_t *cmdline, + char16_t **cmdline_append, + bool *ret_parameters_measured) { + + _cleanup_free_ char16_t *tmp = NULL; + bool m = false; + + assert(cmdline_append); + assert(ret_parameters_measured); + + mangle_stub_cmdline(cmdline); + + if (isempty(cmdline)) + return; + + (void) tpm_log_load_options(cmdline, &m); + *ret_parameters_measured = m; + + tmp = TAKE_PTR(*cmdline_append); + *cmdline_append = xasprintf("%ls%ls%ls", strempty(tmp), isempty(tmp) ? u"" : u" ", cmdline); +} + +static void dtb_install_addons( + struct devicetree_state *dt_state, + void **dt_bases, + size_t *dt_sizes, + char16_t **dt_filenames, + size_t n_dts, + bool *ret_parameters_measured) { + + int parameters_measured = -1; + EFI_STATUS err; + + assert(dt_state); + assert(n_dts == 0 || (dt_bases && dt_sizes && dt_filenames)); + assert(ret_parameters_measured); + + for (size_t i = 0; i < n_dts; ++i) { + err = devicetree_install_from_memory(dt_state, dt_bases[i], dt_sizes[i]); + if (err != EFI_SUCCESS) + log_error_status(err, "Error loading addon devicetree, ignoring: %m"); + else { + bool m = false; + + err = tpm_log_tagged_event( + TPM2_PCR_KERNEL_CONFIG, + POINTER_TO_PHYSICAL_ADDRESS(dt_bases[i]), + dt_sizes[i], + ADDON_FILENAME_EVENT_TAG_ID, + dt_filenames[i], + &m); + if (err != EFI_SUCCESS) + return (void) log_error_status( + err, + "Unable to add measurement of DTB addon #%zu to PCR %i: %m", + i, + TPM2_PCR_KERNEL_CONFIG); + + parameters_measured = parameters_measured < 0 ? m : (parameters_measured && m); + } + } + + *ret_parameters_measured = parameters_measured; +} + +static void dt_bases_free(void **dt_bases, size_t n_dt) { + assert(dt_bases || n_dt == 0); + + for (size_t i = 0; i < n_dt; ++i) + free(dt_bases[i]); + + free(dt_bases); +} + +static void dt_filenames_free(char16_t **dt_filenames, size_t n_dt) { + assert(dt_filenames || n_dt == 0); + + for (size_t i = 0; i < n_dt; ++i) + free(dt_filenames[i]); + + free(dt_filenames); +} + +static EFI_STATUS load_addons( EFI_HANDLE stub_image, EFI_LOADED_IMAGE_PROTOCOL *loaded_image, const char16_t *prefix, const char *uname, - bool *ret_parameters_measured, - char16_t **cmdline_append) { + char16_t **ret_cmdline, + void ***ret_dt_bases, + size_t **ret_dt_sizes, + char16_t ***ret_dt_filenames, + size_t *ret_n_dt) { + _cleanup_free_ size_t *dt_sizes = NULL; _cleanup_(strv_freep) char16_t **items = NULL; _cleanup_(file_closep) EFI_FILE *root = NULL; - _cleanup_free_ char16_t *buffer = NULL; - size_t n_items = 0, n_allocated = 0; + _cleanup_free_ char16_t *cmdline = NULL; + size_t n_items = 0, n_allocated = 0, n_dt = 0; + char16_t **dt_filenames = NULL; + void **dt_bases = NULL; EFI_STATUS err; assert(stub_image); assert(loaded_image); assert(prefix); - assert(ret_parameters_measured); - assert(cmdline_append); + assert(!!ret_dt_bases == !!ret_dt_sizes); + assert(!!ret_dt_bases == !!ret_n_dt); + assert(!!ret_dt_filenames == !!ret_n_dt); if (!loaded_image->DeviceHandle) return EFI_SUCCESS; + CLEANUP_ARRAY(dt_bases, n_dt, dt_bases_free); + CLEANUP_ARRAY(dt_filenames, n_dt, dt_filenames_free); + err = open_volume(loaded_image->DeviceHandle, &root); if (err == EFI_UNSUPPORTED) /* Error will be unsupported if the bootloader doesn't implement the file system protocol on @@ -325,11 +422,12 @@ static EFI_STATUS cmdline_append_and_measure_addons( return log_error_status(err, "Failed to find protocol in %ls: %m", items[i]); err = pe_memory_locate_sections(loaded_addon->ImageBase, unified_sections, addrs, szs); - if (err != EFI_SUCCESS || szs[UNIFIED_SECTION_CMDLINE] == 0) { + if (err != EFI_SUCCESS || + (szs[UNIFIED_SECTION_CMDLINE] == 0 && szs[UNIFIED_SECTION_DTB] == 0)) { if (err == EFI_SUCCESS) err = EFI_NOT_FOUND; log_error_status(err, - "Unable to locate embedded .cmdline section in %ls, ignoring: %m", + "Unable to locate embedded .cmdline/.dtb sections in %ls, ignoring: %m", items[i]); continue; } @@ -350,22 +448,42 @@ static EFI_STATUS cmdline_append_and_measure_addons( continue; } - _cleanup_free_ char16_t *tmp = TAKE_PTR(buffer), - *extra16 = xstrn8_to_16((char *)loaded_addon->ImageBase + addrs[UNIFIED_SECTION_CMDLINE], - szs[UNIFIED_SECTION_CMDLINE]); - buffer = xasprintf("%ls%ls%ls", strempty(tmp), isempty(tmp) ? u"" : u" ", extra16); + if (ret_cmdline && szs[UNIFIED_SECTION_CMDLINE] > 0) { + _cleanup_free_ char16_t *tmp = TAKE_PTR(cmdline), + *extra16 = xstrn8_to_16((char *)loaded_addon->ImageBase + addrs[UNIFIED_SECTION_CMDLINE], + szs[UNIFIED_SECTION_CMDLINE]); + cmdline = xasprintf("%ls%ls%ls", strempty(tmp), isempty(tmp) ? u"" : u" ", extra16); + } + + if (ret_dt_bases && szs[UNIFIED_SECTION_DTB] > 0) { + dt_sizes = xrealloc(dt_sizes, + n_dt * sizeof(size_t), + (n_dt + 1) * sizeof(size_t)); + dt_sizes[n_dt] = szs[UNIFIED_SECTION_DTB]; + + dt_bases = xrealloc(dt_bases, + n_dt * sizeof(void *), + (n_dt + 1) * sizeof(void *)); + dt_bases[n_dt] = xmemdup((uint8_t*)loaded_addon->ImageBase + addrs[UNIFIED_SECTION_DTB], + dt_sizes[n_dt]); + + dt_filenames = xrealloc(dt_filenames, + n_dt * sizeof(char16_t *), + (n_dt + 1) * sizeof(char16_t *)); + dt_filenames[n_dt] = xstrdup16(items[i]); + + ++n_dt; + } } - mangle_stub_cmdline(buffer); + if (ret_cmdline && !isempty(cmdline)) + *ret_cmdline = TAKE_PTR(cmdline); - if (!isempty(buffer)) { - _cleanup_free_ char16_t *tmp = TAKE_PTR(*cmdline_append); - bool m = false; - - (void) tpm_log_load_options(buffer, &m); - *ret_parameters_measured = m; - - *cmdline_append = xasprintf("%ls%ls%ls", strempty(tmp), isempty(tmp) ? u"" : u" ", buffer); + if (ret_n_dt && n_dt > 0) { + *ret_dt_filenames = TAKE_PTR(dt_filenames); + *ret_dt_bases = TAKE_PTR(dt_bases); + *ret_dt_sizes = TAKE_PTR(dt_sizes); + *ret_n_dt = n_dt; } return EFI_SUCCESS; @@ -374,12 +492,15 @@ static EFI_STATUS cmdline_append_and_measure_addons( static EFI_STATUS run(EFI_HANDLE image) { _cleanup_free_ void *credential_initrd = NULL, *global_credential_initrd = NULL, *sysext_initrd = NULL, *pcrsig_initrd = NULL, *pcrpkey_initrd = NULL; size_t credential_initrd_size = 0, global_credential_initrd_size = 0, sysext_initrd_size = 0, pcrsig_initrd_size = 0, pcrpkey_initrd_size = 0; - size_t linux_size, initrd_size, dt_size; + void **dt_bases_addons_global = NULL, **dt_bases_addons_uki = NULL; + char16_t **dt_filenames_addons_global = NULL, **dt_filenames_addons_uki = NULL; + _cleanup_free_ size_t *dt_sizes_addons_global = NULL, *dt_sizes_addons_uki = NULL; + size_t linux_size, initrd_size, dt_size, n_dts_addons_global = 0, n_dts_addons_uki = 0; EFI_PHYSICAL_ADDRESS linux_base, initrd_base, dt_base; _cleanup_(devicetree_cleanup) struct devicetree_state dt_state = {}; EFI_LOADED_IMAGE_PROTOCOL *loaded_image; size_t addrs[_UNIFIED_SECTION_MAX] = {}, szs[_UNIFIED_SECTION_MAX] = {}; - _cleanup_free_ char16_t *cmdline = NULL; + _cleanup_free_ char16_t *cmdline = NULL, *cmdline_addons_global = NULL, *cmdline_addons_uki = NULL; int sections_measured = -1, parameters_measured = -1; _cleanup_free_ char *uname = NULL; bool sysext_measured = false, m; @@ -406,6 +527,40 @@ static EFI_STATUS run(EFI_HANDLE image) { return log_error_status(err, "Unable to locate embedded .linux section: %m"); } + CLEANUP_ARRAY(dt_bases_addons_global, n_dts_addons_global, dt_bases_free); + CLEANUP_ARRAY(dt_bases_addons_uki, n_dts_addons_uki, dt_bases_free); + CLEANUP_ARRAY(dt_filenames_addons_global, n_dts_addons_global, dt_filenames_free); + CLEANUP_ARRAY(dt_filenames_addons_uki, n_dts_addons_uki, dt_filenames_free); + + /* Now that we have the UKI sections loaded, also load global first and then local (per-UKI) + * addons. The data is loaded at once, and then used later. */ + err = load_addons( + image, + loaded_image, + u"\\loader\\addons", + uname, + &cmdline_addons_global, + &dt_bases_addons_global, + &dt_sizes_addons_global, + &dt_filenames_addons_global, + &n_dts_addons_global); + if (err != EFI_SUCCESS) + log_error_status(err, "Error loading global addons, ignoring: %m"); + + _cleanup_free_ char16_t *dropin_dir = get_extra_dir(loaded_image->FilePath); + err = load_addons( + image, + loaded_image, + dropin_dir, + uname, + &cmdline_addons_uki, + &dt_bases_addons_uki, + &dt_sizes_addons_uki, + &dt_filenames_addons_uki, + &n_dts_addons_uki); + if (err != EFI_SUCCESS) + log_error_status(err, "Error loading UKI-specific addons, ignoring: %m"); + /* Measure all "payload" of this PE image into a separate PCR (i.e. where nothing else is written * into so far), so that we have one PCR that we can nicely write policies against because it * contains all static data of this image, and thus can be easily be pre-calculated. */ @@ -471,27 +626,10 @@ static EFI_STATUS run(EFI_HANDLE image) { * measure the additions separately, after the embedded options, but before the smbios ones, * so that the order is reversed from "most hardcoded" to "most dynamic". The global addons are * loaded first, and the image-specific ones later, for the same reason. */ - err = cmdline_append_and_measure_addons( - image, - loaded_image, - u"\\loader\\addons", - uname, - &m, - &cmdline); - if (err != EFI_SUCCESS) - log_error_status(err, "Error loading global addons, ignoring: %m"); + cmdline_append_and_measure_addons(cmdline_addons_global, &cmdline, &m); parameters_measured = parameters_measured < 0 ? m : (parameters_measured && m); - _cleanup_free_ char16_t *dropin_dir = get_extra_dir(loaded_image->FilePath); - err = cmdline_append_and_measure_addons( - image, - loaded_image, - dropin_dir, - uname, - &m, - &cmdline); - if (err != EFI_SUCCESS) - log_error_status(err, "Error loading UKI-specific addons, ignoring: %m"); + cmdline_append_and_measure_addons(cmdline_addons_uki, &cmdline, &m); parameters_measured = parameters_measured < 0 ? m : (parameters_measured && m); /* SMBIOS OEM Strings data is controlled by the host admin and not covered @@ -552,6 +690,32 @@ static EFI_STATUS run(EFI_HANDLE image) { &m) == EFI_SUCCESS) sysext_measured = m; + dt_size = szs[UNIFIED_SECTION_DTB]; + dt_base = dt_size != 0 ? POINTER_TO_PHYSICAL_ADDRESS(loaded_image->ImageBase) + addrs[UNIFIED_SECTION_DTB] : 0; + + /* First load the base device tree, then fix it up using addons - global first, then per-UKI. */ + if (dt_size > 0) { + err = devicetree_install_from_memory( + &dt_state, PHYSICAL_ADDRESS_TO_POINTER(dt_base), dt_size); + if (err != EFI_SUCCESS) + log_error_status(err, "Error loading embedded devicetree: %m"); + } + + dtb_install_addons(&dt_state, + dt_bases_addons_global, + dt_sizes_addons_global, + dt_filenames_addons_global, + n_dts_addons_global, + &m); + parameters_measured = parameters_measured < 0 ? m : (parameters_measured && m); + dtb_install_addons(&dt_state, + dt_bases_addons_uki, + dt_sizes_addons_uki, + dt_filenames_addons_uki, + n_dts_addons_uki, + &m); + parameters_measured = parameters_measured < 0 ? m : (parameters_measured && m); + if (parameters_measured > 0) (void) efivar_set_uint_string(MAKE_GUID_PTR(LOADER), u"StubPcrKernelParameters", TPM2_PCR_KERNEL_CONFIG, 0); if (sysext_measured) @@ -600,9 +764,6 @@ static EFI_STATUS run(EFI_HANDLE image) { initrd_size = szs[UNIFIED_SECTION_INITRD]; initrd_base = initrd_size != 0 ? POINTER_TO_PHYSICAL_ADDRESS(loaded_image->ImageBase) + addrs[UNIFIED_SECTION_INITRD] : 0; - dt_size = szs[UNIFIED_SECTION_DTB]; - dt_base = dt_size != 0 ? POINTER_TO_PHYSICAL_ADDRESS(loaded_image->ImageBase) + addrs[UNIFIED_SECTION_DTB] : 0; - _cleanup_pages_ Pages initrd_pages = {}; if (credential_initrd || global_credential_initrd || sysext_initrd || pcrsig_initrd || pcrpkey_initrd) { /* If we have generated initrds dynamically, let's combine them with the built-in initrd. */ @@ -637,13 +798,6 @@ static EFI_STATUS run(EFI_HANDLE image) { pcrpkey_initrd = mfree(pcrpkey_initrd); } - if (dt_size > 0) { - err = devicetree_install_from_memory( - &dt_state, PHYSICAL_ADDRESS_TO_POINTER(dt_base), dt_size); - if (err != EFI_SUCCESS) - log_error_status(err, "Error loading embedded devicetree: %m"); - } - err = linux_exec(image, cmdline, PHYSICAL_ADDRESS_TO_POINTER(linux_base), linux_size, PHYSICAL_ADDRESS_TO_POINTER(initrd_base), initrd_size); diff --git a/src/fundamental/efivars-fundamental.h b/src/fundamental/efivars-fundamental.h index 569f5eeceb1..8c9e7ddef28 100644 --- a/src/fundamental/efivars-fundamental.h +++ b/src/fundamental/efivars-fundamental.h @@ -31,6 +31,7 @@ #define EFI_STUB_FEATURE_RANDOM_SEED (UINT64_C(1) << 4) #define EFI_STUB_FEATURE_CMDLINE_ADDONS (UINT64_C(1) << 5) #define EFI_STUB_FEATURE_CMDLINE_SMBIOS (UINT64_C(1) << 6) +#define EFI_STUB_FEATURE_DEVICETREE_ADDONS (UINT64_C(1) << 7) typedef enum SecureBootMode { SECURE_BOOT_UNSUPPORTED, From 3e6f010e03a81082ba8aa39606dd34e00f410af7 Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Sun, 24 Sep 2023 00:34:58 +0100 Subject: [PATCH 6/8] stub: measure all cmdline addons together --- docs/TPM2_PCR_MEASUREMENTS.md | 7 +++---- src/boot/efi/stub.c | 30 ++++++++++++++++++------------ 2 files changed, 21 insertions(+), 16 deletions(-) diff --git a/docs/TPM2_PCR_MEASUREMENTS.md b/docs/TPM2_PCR_MEASUREMENTS.md index f2893f13445..f92b8a5ed5b 100644 --- a/docs/TPM2_PCR_MEASUREMENTS.md +++ b/docs/TPM2_PCR_MEASUREMENTS.md @@ -77,12 +77,11 @@ PE section order, as per the UKI specification, see above. ### PCR 12, `EV_IPL`, "Kernel Command Line" -Might happen up to four times, for kernel command lines from: +Might happen up to three times, for kernel command lines from: 1. Passed cmdline - 2. System cmdline add-ons (one measurement covering all add-ons combined) - 3. Per-UKI cmdline add-ons (one measurement covering all add-ons combined) - 2. SMBIOS cmdline + 2. System and per-UKI cmdline add-ons (one measurement covering all add-ons combined) + 3. SMBIOS cmdline → **Description** in the event log record is the literal kernel command line in UTF-16. diff --git a/src/boot/efi/stub.c b/src/boot/efi/stub.c index dd3d017cce8..2366dfd42b6 100644 --- a/src/boot/efi/stub.c +++ b/src/boot/efi/stub.c @@ -257,26 +257,35 @@ static EFI_STATUS load_addons_from_dir( } static void cmdline_append_and_measure_addons( - char16_t *cmdline, + char16_t *cmdline_global, + char16_t *cmdline_uki, char16_t **cmdline_append, bool *ret_parameters_measured) { - _cleanup_free_ char16_t *tmp = NULL; + _cleanup_free_ char16_t *tmp = NULL, *merged = NULL; bool m = false; assert(cmdline_append); assert(ret_parameters_measured); - mangle_stub_cmdline(cmdline); - - if (isempty(cmdline)) + if (isempty(cmdline_global) && isempty(cmdline_uki)) return; - (void) tpm_log_load_options(cmdline, &m); + merged = xasprintf("%ls%ls%ls", + strempty(cmdline_global), + isempty(cmdline_global) || isempty(cmdline_uki) ? u"" : u" ", + strempty(cmdline_uki)); + + mangle_stub_cmdline(merged); + + if (isempty(merged)) + return; + + (void) tpm_log_load_options(merged, &m); *ret_parameters_measured = m; tmp = TAKE_PTR(*cmdline_append); - *cmdline_append = xasprintf("%ls%ls%ls", strempty(tmp), isempty(tmp) ? u"" : u" ", cmdline); + *cmdline_append = xasprintf("%ls%ls%ls", strempty(tmp), isempty(tmp) ? u"" : u" ", merged); } static void dtb_install_addons( @@ -623,13 +632,10 @@ static EFI_STATUS run(EFI_HANDLE image) { } /* If we have any extra command line to add via PE addons, load them now and append, and - * measure the additions separately, after the embedded options, but before the smbios ones, + * measure the additions together, after the embedded options, but before the smbios ones, * so that the order is reversed from "most hardcoded" to "most dynamic". The global addons are * loaded first, and the image-specific ones later, for the same reason. */ - cmdline_append_and_measure_addons(cmdline_addons_global, &cmdline, &m); - parameters_measured = parameters_measured < 0 ? m : (parameters_measured && m); - - cmdline_append_and_measure_addons(cmdline_addons_uki, &cmdline, &m); + cmdline_append_and_measure_addons(cmdline_addons_global, cmdline_addons_uki, &cmdline, &m); parameters_measured = parameters_measured < 0 ? m : (parameters_measured && m); /* SMBIOS OEM Strings data is controlled by the host admin and not covered From 12de4ed1ca8c4c023678d1022de48aecc656b862 Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Fri, 6 Oct 2023 23:41:10 +0100 Subject: [PATCH 7/8] boot: measure loader.conf in PCR5 Results in: - EventNum: 26 PCRIndex: 5 EventType: EV_EVENT_TAG DigestCount: 4 Digests: - AlgorithmId: sha1 Digest: 155fb999ca61ba8c7b1f1d87cee821f772ef084a - AlgorithmId: sha256 Digest: 4c26adf231603613afc00bb3d5cad046aec6a525ca01262417c7085caab452b5 - AlgorithmId: sha384 Digest: 3e0758cb6605ac274e55d747bf29ee3474fc4413cd5e7a451d1375219cd7f08a30fc915a8df7131657ca78b82b9ccec8 - AlgorithmId: sha512 Digest: e32d905b9092c543802f386db9a397d9b6593bdb8360fb747a6d23e491a09595fec8699184cc790d0873a3d52ed16d045538f0c73ece48278fae0fb6ed9b4ed6 EventSize: 32 Event: 2a58bcf5180000006c006f0061006400650072002e0063006f006e0066000000 --- docs/TPM2_PCR_MEASUREMENTS.md | 15 +++++++++++++-- src/boot/efi/boot.c | 19 ++++++++++++++++--- 2 files changed, 29 insertions(+), 5 deletions(-) diff --git a/docs/TPM2_PCR_MEASUREMENTS.md b/docs/TPM2_PCR_MEASUREMENTS.md index f92b8a5ed5b..7601c155313 100644 --- a/docs/TPM2_PCR_MEASUREMENTS.md +++ b/docs/TPM2_PCR_MEASUREMENTS.md @@ -16,8 +16,8 @@ measurements listed below are (by default) only done if a system is booted with to systemd's UEFI-mode measurements, and if the latter are not done the former aren't made either. -systemd will measure to PCRs 11 (`kernel-boot`), 12 (`kernel-config`), 13 -(`sysexts`), 15 (`system-identity`). +systemd will measure to PCRs 5 (`boot-loader-config`), 11 (`kernel-boot`), +12 (`kernel-config`), 13 (`sysexts`), 15 (`system-identity`). Currently, four components will issue TPM2 PCR measurements: @@ -31,6 +31,17 @@ maintained in `/run/log/systemd/tpm2-measure.log`. ## PCR Measurements Made by `systemd-boot` (UEFI) +### PCS 5, `EV_EVENT_TAG`, "loader.conf" + +The content of `systemd-boot`'s configuration file, `loader/loader.conf`, is +measured as a tagged event. + +→ **Event Tag** `0xf5bc582a` + +→ **Description** in the event log record is the file name, `loader.conf`. + +→ **Measured hash** covers the content of `loader.conf` as it is read from the ESP. + ### PCR 12, `EV_IPL`, "Kernel Command Line" If the kernel command line was specified explicitly (by the user or in a Boot diff --git a/src/boot/efi/boot.c b/src/boot/efi/boot.c index de801ceb37d..c28ec019753 100644 --- a/src/boot/efi/boot.c +++ b/src/boot/efi/boot.c @@ -21,6 +21,7 @@ #include "secure-boot.h" #include "shim.h" #include "ticks.h" +#include "tpm2-pcr.h" #include "util.h" #include "version.h" #include "vmm.h" @@ -38,6 +39,8 @@ DECLARE_NOALLOC_SECTION( DECLARE_SBAT(SBAT_BOOT_SECTION_TEXT); +#define LOADER_CONF_CONTENT_EVENT_TAG_ID UINT32_C(0xf5bc582a) + typedef enum LoaderType { LOADER_UNDEFINED, LOADER_AUTO, @@ -1621,7 +1624,7 @@ static EFI_STATUS efivar_get_timeout(const char16_t *var, uint32_t *ret_value) { static void config_load_defaults(Config *config, EFI_FILE *root_dir) { _cleanup_free_ char *content = NULL; - size_t value = 0; /* avoid false maybe-uninitialized warning */ + size_t content_size, value = 0; /* avoid false maybe-uninitialized warning */ EFI_STATUS err; assert(root_dir); @@ -1638,9 +1641,19 @@ static void config_load_defaults(Config *config, EFI_FILE *root_dir) { .timeout_sec_efivar = TIMEOUT_UNSET, }; - err = file_read(root_dir, u"\\loader\\loader.conf", 0, 0, &content, NULL); - if (err == EFI_SUCCESS) + err = file_read(root_dir, u"\\loader\\loader.conf", 0, 0, &content, &content_size); + if (err == EFI_SUCCESS) { config_defaults_load_from_file(config, content); + err = tpm_log_tagged_event( + TPM2_PCR_BOOT_LOADER_CONFIG, + POINTER_TO_PHYSICAL_ADDRESS(content), + content_size, + LOADER_CONF_CONTENT_EVENT_TAG_ID, + u"loader.conf", + /* ret_measured= */ NULL); + if (err != EFI_SUCCESS) + log_error_status(err, "Error measuring loader.conf into TPM: %m"); + } err = efivar_get_timeout(u"LoaderConfigTimeout", &config->timeout_sec_efivar); if (err == EFI_SUCCESS) From 375991c0b5cb23585f767950aa082bcf34a27fba Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Fri, 6 Oct 2023 23:42:27 +0100 Subject: [PATCH 8/8] Update TODO --- TODO | 3 --- 1 file changed, 3 deletions(-) diff --git a/TODO b/TODO index fde2013c834..81fc1f3cae1 100644 --- a/TODO +++ b/TODO @@ -233,9 +233,6 @@ Features: 2nd key derived from volume key of the user, with which to wrap all keys. maintain keys in kernel keyring if possible. -* sd-boot should probably measure its configuration file to PCR 5 at boot, as - per TCG PC Client Platform Firmware Profile Spec. - * use sd-event ratelimit feature optionally for .socket units to "pause" overly busy sockets temporarily. (as a less drastic version of the trigger ratelimit)