boot: set LoaderPcrSMBIOS only if all structures were measured

So far the variable was set as soon as one of the SMBIOS structures
(types 1, 2 and 11) was measured, and a later failure on another only
logged.The original intent was to prevent sd-stub from measuring these
things a second time after sd-boot already did.

This PR adds a second consumer: in a confidential VM, PID 1 takes the
variable as confirmation that the SMBIOS OEM strings are covered by
PCR 1 before it imports credentials from them. Make the loader provide
that guarantee and leave the variable unset if any structure could not
be measured, which also matches how the documentation already describes
it.

The effect on the existing consumer is limited to boots with a
measurement failure: when sd-boot fails to measure some of the structs
and then chainloads sd-stub, sd-stub measures the structs sd-boot was
able to measure a second time. Previously it would have left the
incomplete measurement as is. In that situation PCR 1 already didn't
match the expected value before (events missing), and after this change
it still won't (additional events), so anything bound to it, such as a
pcrlock policy, does not match either way.

Signed-off-by: Paul Meyer <katexochen0@gmail.com>

docs/BOOT_LOADER_INTERFACE.md

@@ -191,11 +191,11 @@ Variables will be listed below using the Linux efivarfs naming,
   NUL-terminated UTF-16 string, e.g. `1`) into which the boot loader measured
   select SMBIOS structures: type 1 (system information, with the volatile
   "Wake-up Type" field zeroed out), type 2 (baseboard information) and type 11
-  (OEM strings). This is a volatile (non-persistent) variable, set only if a
-  measurement was successfully completed, and remains unset otherwise. Both
-  `systemd-boot` and `systemd-stub` perform this measurement; whichever runs
-  first sets the variable, and its presence suppresses a second measurement of
-  the same data into the same PCR during the same boot.
+  (OEM strings). This is a volatile (non-persistent) variable, set only if all
+  of these structures were measured successfully, and remains unset otherwise.
+  Both `systemd-boot` and `systemd-stub` perform this measurement; whichever
+  runs first sets the variable, and its presence suppresses a second
+  measurement of the same data into the same PCR during the same boot.
 
 * The EFI variable `LoaderInfo-4a67b082-0a4c-41cf-b6c7-440b29bb8c4f`
   contains the version string of the boot loader that was booted,

docs/TPM2_PCR_MEASUREMENTS.md

@@ -156,11 +156,12 @@ described above.
 
 This measurement is also performed by `systemd-stub` (see below), so that systems
 that boot a UKI directly, bypassing `systemd-boot`, still get it. Whichever
-component runs first performs the measurement and sets the volatile
-`LoaderPcrSMBIOS` EFI variable to the PCR index used; its presence suppresses a
-second measurement of the same data into the same PCR during the same boot. Note
-that the firmware itself typically also extends PCR 1, so its final value is not
-solely determined by this measurement.
+component runs first performs the measurement and, if all structures were
+measured successfully, sets the volatile `LoaderPcrSMBIOS` EFI variable to the
+PCR index used; its presence suppresses a second measurement of the same data
+into the same PCR during the same boot. Note that the firmware itself typically
+also extends PCR 1, so its final value is not solely determined by this
+measurement.
 
 ### PCR 5, `EV_EVENT_TAG`, `loader.conf`
 

man/systemd-boot.xml

@@ -631,9 +631,9 @@ extra       /6a9857a393724b7a981ebb5b8495b9ea/1.2.3/baz.confext.raw</programlist
         <listitem><para>The PCR register index select SMBIOS structures are measured into — type 1
         (system information, with the volatile "Wake-up Type" field zeroed out), type 2 (baseboard
         information) and type 11 (OEM strings). Formatted as decimal ASCII string (e.g.
-        <literal>1</literal>). Set by the boot loader if a measurement was successfully completed, and remains
-        unset otherwise. (Note that <command>systemd-stub</command> performs the same measurement when booted
-        directly, bypassing the boot loader.)</para>
+        <literal>1</literal>). Set by the boot loader only if all of these structures were measured
+        successfully, and remains unset otherwise. (Note that <command>systemd-stub</command> performs the
+        same measurement when booted directly, bypassing the boot loader.)</para>
 
         <xi:include href="version-info.xml" xpointer="v261"/></listitem>
       </varlistentry>

man/systemd-stub.xml

@@ -671,10 +671,10 @@
         <listitem><para>The PCR register index select SMBIOS structures are measured into — type 1
         (system information, with the volatile "Wake-up Type" field zeroed out), type 2 (baseboard
         information) and type 11 (OEM strings). Formatted as decimal ASCII string (e.g.
-        <literal>1</literal>). This variable is set if a measurement was successfully completed, and remains
-        unset otherwise. <command>systemd-stub</command> performs this measurement only if it has not already
-        been done in the same boot (e.g. by the boot loader), as indicated by this variable being set
-        already.</para>
+        <literal>1</literal>). This variable is set only if all of these structures were measured
+        successfully, and remains unset otherwise. <command>systemd-stub</command> performs this measurement
+        only if it has not already been done in the same boot (e.g. by the boot loader), as indicated by this
+        variable being set already.</para>
 
         <xi:include href="version-info.xml" xpointer="v261"/></listitem>
       </varlistentry>

src/boot/measure-smbios.c

@@ -8,19 +8,24 @@
 #include "tpm2-pcr.h"
 #include "util.h"
 
+typedef struct MeasureSmbiosContext {
+        bool measured; /* At least one structure was measured */
+        bool failed;   /* At least one measurement failed */
+} MeasureSmbiosContext;
+
 static void measure_smbios_raw(
                 const void *p,
                 size_t size,
                 uint32_t event_id,
                 const char16_t *description,
-                bool *measured) {
+                MeasureSmbiosContext *ctx) {
 
         EFI_STATUS err;
         bool m = false;
 
         assert(p);
         assert(description);
-        assert(measured);
+        assert(ctx);
 
         err = tpm_log_tagged_event(
                         TPM2_PCR_PLATFORM_CONFIG,
@@ -29,15 +34,18 @@ static void measure_smbios_raw(
                         event_id,
                         description,
                         &m);
-        if (err != EFI_SUCCESS)
-                log_error_status(err, "Unable to measure SMBIOS structure (%ls), ignoring: %m", description);
+        if (err != EFI_SUCCESS) {
+                log_error_status(err, "Unable to measure SMBIOS structure (%ls): %m", description);
+                ctx->failed = true;
+                return;
+        }
 
-        *measured = *measured || m;
+        ctx->measured = ctx->measured || m;
 }
 
-static void measure_smbios_type1(const SmbiosHeader *header, size_t size, bool *measured) {
+static void measure_smbios_type1(const SmbiosHeader *header, size_t size, MeasureSmbiosContext *ctx) {
         assert(header);
-        assert(measured);
+        assert(ctx);
 
         /* The wake-up type field varies depending on how the machine was powered on (cold boot, resume
          * from sleep, AC restore, …), which would make the measurement non-reproducible. Hence measure a
@@ -61,24 +69,24 @@ static void measure_smbios_type1(const SmbiosHeader *header, size_t size, bool *
                 p = copy;
         }
 
-        measure_smbios_raw(p, size, SMBIOS_TYPE1_EVENT_TAG_ID, u"smbios:type1", measured);
+        measure_smbios_raw(p, size, SMBIOS_TYPE1_EVENT_TAG_ID, u"smbios:type1", ctx);
 }
 
 static bool measure_smbios_object(const SmbiosHeader *header, size_t size, void *userdata) {
-        bool *measured = ASSERT_PTR(userdata);
+        MeasureSmbiosContext *ctx = ASSERT_PTR(userdata);
 
         switch (header->type) {
 
         case 1: /* System Information */
-                measure_smbios_type1(header, size, measured);
+                measure_smbios_type1(header, size, ctx);
                 break;
 
         case 2: /* Baseboard Information */
-                measure_smbios_raw(header, size, SMBIOS_TYPE2_EVENT_TAG_ID, u"smbios:type2", measured);
+                measure_smbios_raw(header, size, SMBIOS_TYPE2_EVENT_TAG_ID, u"smbios:type2", ctx);
                 break;
 
         case 11: /* OEM Strings */
-                measure_smbios_raw(header, size, SMBIOS_TYPE11_EVENT_TAG_ID, u"smbios:type11", measured);
+                measure_smbios_raw(header, size, SMBIOS_TYPE11_EVENT_TAG_ID, u"smbios:type11", ctx);
                 break;
 
         default:
@@ -89,7 +97,7 @@ static bool measure_smbios_object(const SmbiosHeader *header, size_t size, void
 }
 
 void measure_smbios(void) {
-        bool measured = false;
+        MeasureSmbiosContext ctx = {};
 
         if (!runtime_measurement_available())
                 return;
@@ -101,9 +109,9 @@ void measure_smbios(void) {
 
         /* Measure SMBIOS type 1 (system information), type 2 (baseboard information) and type 11 (OEM
          * strings) into PCR 1, in a single pass over the SMBIOS table. */
-        smbios_foreach(measure_smbios_object, &measured);
+        smbios_foreach(measure_smbios_object, &ctx);
 
-        /* If we measured something, tell the OS which PCR we used (and suppress a second pass). */
-        if (measured)
+        /* Tell the OS which PCR we used (and suppress a second pass), but only if every structure was measured. */
+        if (ctx.measured && !ctx.failed)
                 (void) efivar_set_uint64_str16(MAKE_GUID_PTR(LOADER), u"LoaderPcrSMBIOS", TPM2_PCR_PLATFORM_CONFIG, /* flags= */ 0);
 }

src/boot/measure-smbios.h

@@ -1,8 +1,9 @@
 /* SPDX-License-Identifier: LGPL-2.1-or-later */
 #pragma once
 
-/* Measures SMBIOS type 1 (system information, with the volatile "Wake-up Type" field masked) and all
- * type 11 (OEM strings) structures into PCR 1, and records the PCR index in the transient
- * LoaderPcrSMBIOS EFI variable. Called by both sd-boot and sd-stub; the presence of LoaderPcrSMBIOS
- * suppresses a redundant second measurement when both run during the same boot. */
+/* Measures SMBIOS type 1 (system information, with the volatile "Wake-up Type" field masked), type 2
+ * (baseboard information) and all type 11 (OEM strings) structures into PCR 1, and records the PCR
+ * index in the transient LoaderPcrSMBIOS EFI variable if all of them were measured successfully.
+ * Called by both sd-boot and sd-stub; the presence of LoaderPcrSMBIOS suppresses a redundant second
+ * measurement when both run during the same boot. */
 void measure_smbios(void);

src/shared/efi-loader.c

@@ -347,8 +347,9 @@ int efi_measured_smbios(int log_level) {
         if (cached >= 0)
                 return cached;
 
-        /* Whether sd-boot or sd-stub measured the SMBIOS table (types 1, 2, and 11) into PCR 1. Firmware
-         * does not do this for OEM strings: edk2's SmbiosMeasurementDxe zeros type 11 before hashing. */
+        /* Whether sd-boot or sd-stub measured the SMBIOS table (types 1, 2, and 11) into PCR 1. The loader
+         * sets the variable only if all of these structures were measured. Firmware does not do this for
+         * OEM strings: edk2's SmbiosMeasurementDxe zeros type 11 before hashing. */
 
         r = efi_get_variable_string(EFI_LOADER_VARIABLE_STR("LoaderPcrSMBIOS"), &pcr_string);
         if (r == -ENOENT)