[PATCH 0/2] measure the loaded EFI kernel image and its options

[Cedric Hombourger]

Re-use systemd code to measure binary blobs to get both the loaded EFI
image and the kernel options measured into the TPM PCR registers 4 and
8. The --disable-tpm and --options-pcr options were added to the
configure script. The changes were tested on x86 platforms with and
without a TPM (just like systemd-boot, the absence of a TPM does not
cause any harm even when the ENABLE_TPM pre-processing directive is in
effect).

Cedric Hombourger (2):
measure: import systemd code to perform measurements into the TPM
main: measure the kernel command line into the TPM

Makefile.am | 1 +
configure.ac | 16 +++
main.c | 12 ++
measure.c | 365 +++++++++++++++++++++++++++++++++++++++++++++++++++
measure.h | 5 +
5 files changed, 399 insertions(+)
create mode 100644 measure.c
create mode 100644 measure.h

--
2.30.2

[Cedric Hombourger]

Import systemd code to get the loaded image and options measures (which will
be done in a subsequent commit). This commit brings the code authored by
har...@redhat.com and will be used (later) when ENABLE_TPM is in effect.

Signed-off-by: Cedric Hombourger <Cedric_H...@mentor.com>
---
Makefile.am | 1 +
measure.c | 365 ++++++++++++++++++++++++++++++++++++++++++++++++++++
measure.h | 5 +
3 files changed, 371 insertions(+)

create mode 100644 measure.c
create mode 100644 measure.h

diff --git a/Makefile.am b/Makefile.am
index 8fb8652..90f2959 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -135,6 +135,7 @@ efi_sources = \
env/fatvars.c \
utils.c \
bootguard.c \
+ measure.c \
main.c

efi_cppflags = \
diff --git a/measure.c b/measure.c
new file mode 100644
index 0000000..95dc30e
--- /dev/null
+++ b/measure.c
@@ -0,0 +1,365 @@
+/* Copied verbatim from systemd/src/boot/efi/measured.c - authored by har...@redhat.com */
+/* SPDX-License-Identifier: LGPL-2.1+ */
+
+#if ENABLE_TPM
+
+#include <efi.h>
+#include <efilib.h>
+#include "measure.h"
+
+#define EFI_TCG_PROTOCOL_GUID { 0xf541796d, 0xa62e, 0x4954, {0xa7, 0x75, 0x95, 0x84, 0xf6, 0x1b, 0x9c, 0xdd} }
+
+typedef struct _TCG_VERSION {
+ UINT8 Major;
+ UINT8 Minor;
+ UINT8 RevMajor;
+ UINT8 RevMinor;
+} TCG_VERSION;
+
+typedef struct tdEFI_TCG2_VERSION {
+ UINT8 Major;
+ UINT8 Minor;
+} EFI_TCG2_VERSION;
+
+typedef struct _TCG_BOOT_SERVICE_CAPABILITY {
+ UINT8 Size;
+ struct _TCG_VERSION StructureVersion;
+ struct _TCG_VERSION ProtocolSpecVersion;
+ UINT8 HashAlgorithmBitmap;
+ BOOLEAN TPMPresentFlag;
+ BOOLEAN TPMDeactivatedFlag;
+} TCG_BOOT_SERVICE_CAPABILITY;
+
+typedef struct tdTREE_BOOT_SERVICE_CAPABILITY {
+ UINT8 Size;
+ EFI_TCG2_VERSION StructureVersion;
+ EFI_TCG2_VERSION ProtocolVersion;
+ UINT32 HashAlgorithmBitmap;
+ UINT32 SupportedEventLogs;
+ BOOLEAN TrEEPresentFlag;
+ UINT16 MaxCommandSize;
+ UINT16 MaxResponseSize;
+ UINT32 ManufacturerID;
+} TREE_BOOT_SERVICE_CAPABILITY;
+
+typedef UINT32 TCG_ALGORITHM_ID;
+#define TCG_ALG_SHA 0x00000004 // The SHA1 algorithm
+
+#define SHA1_DIGEST_SIZE 20
+
+typedef struct _TCG_DIGEST {
+ UINT8 Digest[SHA1_DIGEST_SIZE];
+} TCG_DIGEST;
+
+#define EV_IPL 13
+
+typedef struct _TCG_PCR_EVENT {
+ UINT32 PCRIndex;
+ UINT32 EventType;
+ struct _TCG_DIGEST digest;
+ UINT32 EventSize;
+ UINT8 Event[1];
+} TCG_PCR_EVENT;
+
+INTERFACE_DECL(_EFI_TCG);
+
+typedef EFI_STATUS(EFIAPI * EFI_TCG_STATUS_CHECK) (IN struct _EFI_TCG * This,
+ OUT struct _TCG_BOOT_SERVICE_CAPABILITY * ProtocolCapability,
+ OUT UINT32 * TCGFeatureFlags,
+ OUT EFI_PHYSICAL_ADDRESS * EventLogLocation,
+ OUT EFI_PHYSICAL_ADDRESS * EventLogLastEntry);
+
+typedef EFI_STATUS(EFIAPI * EFI_TCG_HASH_ALL) (IN struct _EFI_TCG * This,
+ IN UINT8 * HashData,
+ IN UINT64 HashDataLen,
+ IN TCG_ALGORITHM_ID AlgorithmId,
+ IN OUT UINT64 * HashedDataLen, IN OUT UINT8 ** HashedDataResult);
+
+typedef EFI_STATUS(EFIAPI * EFI_TCG_LOG_EVENT) (IN struct _EFI_TCG * This,
+ IN struct _TCG_PCR_EVENT * TCGLogData,
+ IN OUT UINT32 * EventNumber, IN UINT32 Flags);
+
+typedef EFI_STATUS(EFIAPI * EFI_TCG_PASS_THROUGH_TO_TPM) (IN struct _EFI_TCG * This,
+ IN UINT32 TpmInputParameterBlockSize,
+ IN UINT8 * TpmInputParameterBlock,
+ IN UINT32 TpmOutputParameterBlockSize,
+ IN UINT8 * TpmOutputParameterBlock);
+
+typedef EFI_STATUS(EFIAPI * EFI_TCG_HASH_LOG_EXTEND_EVENT) (IN struct _EFI_TCG * This,
+ IN EFI_PHYSICAL_ADDRESS HashData,
+ IN UINT64 HashDataLen,
+ IN TCG_ALGORITHM_ID AlgorithmId,
+ IN struct _TCG_PCR_EVENT * TCGLogData,
+ IN OUT UINT32 * EventNumber,
+ OUT EFI_PHYSICAL_ADDRESS * EventLogLastEntry);
+
+typedef struct _EFI_TCG {
+ EFI_TCG_STATUS_CHECK StatusCheck;
+ EFI_TCG_HASH_ALL HashAll;
+ EFI_TCG_LOG_EVENT LogEvent;
+ EFI_TCG_PASS_THROUGH_TO_TPM PassThroughToTPM;
+ EFI_TCG_HASH_LOG_EXTEND_EVENT HashLogExtendEvent;
+} EFI_TCG;
+
+#define EFI_TCG2_PROTOCOL_GUID {0x607f766c, 0x7455, 0x42be, { 0x93, 0x0b, 0xe4, 0xd7, 0x6d, 0xb2, 0x72, 0x0f }}
+
+typedef struct tdEFI_TCG2_PROTOCOL EFI_TCG2_PROTOCOL;
+
+typedef UINT32 EFI_TCG2_EVENT_LOG_BITMAP;
+typedef UINT32 EFI_TCG2_EVENT_LOG_FORMAT;
+typedef UINT32 EFI_TCG2_EVENT_ALGORITHM_BITMAP;
+
+#define EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2 0x00000001
+#define EFI_TCG2_EVENT_LOG_FORMAT_TCG_2 0x00000002
+
+typedef struct tdEFI_TCG2_BOOT_SERVICE_CAPABILITY {
+ UINT8 Size;
+ EFI_TCG2_VERSION StructureVersion;
+ EFI_TCG2_VERSION ProtocolVersion;
+ EFI_TCG2_EVENT_ALGORITHM_BITMAP HashAlgorithmBitmap;
+ EFI_TCG2_EVENT_LOG_BITMAP SupportedEventLogs;
+ BOOLEAN TPMPresentFlag;
+ UINT16 MaxCommandSize;
+ UINT16 MaxResponseSize;
+ UINT32 ManufacturerID;
+ UINT32 NumberOfPCRBanks;
+ EFI_TCG2_EVENT_ALGORITHM_BITMAP ActivePcrBanks;
+} EFI_TCG2_BOOT_SERVICE_CAPABILITY;
+
+#define EFI_TCG2_EVENT_HEADER_VERSION 1
+
+typedef struct {
+ UINT32 HeaderSize;
+ UINT16 HeaderVersion;
+ UINT32 PCRIndex;
+ UINT32 EventType;
+} __attribute__((packed)) EFI_TCG2_EVENT_HEADER;
+
+typedef struct tdEFI_TCG2_EVENT {
+ UINT32 Size;
+ EFI_TCG2_EVENT_HEADER Header;
+ UINT8 Event[1];
+} __attribute__((packed)) EFI_TCG2_EVENT;
+
+typedef EFI_STATUS(EFIAPI * EFI_TCG2_GET_CAPABILITY) (IN EFI_TCG2_PROTOCOL * This,
+ IN OUT EFI_TCG2_BOOT_SERVICE_CAPABILITY * ProtocolCapability);
+
+typedef EFI_STATUS(EFIAPI * EFI_TCG2_GET_EVENT_LOG) (IN EFI_TCG2_PROTOCOL * This,
+ IN EFI_TCG2_EVENT_LOG_FORMAT EventLogFormat,
+ OUT EFI_PHYSICAL_ADDRESS * EventLogLocation,
+ OUT EFI_PHYSICAL_ADDRESS * EventLogLastEntry,
+ OUT BOOLEAN * EventLogTruncated);
+
+typedef EFI_STATUS(EFIAPI * EFI_TCG2_HASH_LOG_EXTEND_EVENT) (IN EFI_TCG2_PROTOCOL * This,
+ IN UINT64 Flags,
+ IN EFI_PHYSICAL_ADDRESS DataToHash,
+ IN UINT64 DataToHashLen, IN EFI_TCG2_EVENT * EfiTcgEvent);
+
+typedef EFI_STATUS(EFIAPI * EFI_TCG2_SUBMIT_COMMAND) (IN EFI_TCG2_PROTOCOL * This,
+ IN UINT32 InputParameterBlockSize,
+ IN UINT8 * InputParameterBlock,
+ IN UINT32 OutputParameterBlockSize, IN UINT8 * OutputParameterBlock);
+
+typedef EFI_STATUS(EFIAPI * EFI_TCG2_GET_ACTIVE_PCR_BANKS) (IN EFI_TCG2_PROTOCOL * This, OUT UINT32 * ActivePcrBanks);
+
+typedef EFI_STATUS(EFIAPI * EFI_TCG2_SET_ACTIVE_PCR_BANKS) (IN EFI_TCG2_PROTOCOL * This, IN UINT32 ActivePcrBanks);
+
+typedef EFI_STATUS(EFIAPI * EFI_TCG2_GET_RESULT_OF_SET_ACTIVE_PCR_BANKS) (IN EFI_TCG2_PROTOCOL * This,
+ OUT UINT32 * OperationPresent, OUT UINT32 * Response);
+
+typedef struct tdEFI_TCG2_PROTOCOL {
+ EFI_TCG2_GET_CAPABILITY GetCapability;
+ EFI_TCG2_GET_EVENT_LOG GetEventLog;
+ EFI_TCG2_HASH_LOG_EXTEND_EVENT HashLogExtendEvent;
+ EFI_TCG2_SUBMIT_COMMAND SubmitCommand;
+ EFI_TCG2_GET_ACTIVE_PCR_BANKS GetActivePcrBanks;
+ EFI_TCG2_SET_ACTIVE_PCR_BANKS SetActivePcrBanks;
+ EFI_TCG2_GET_RESULT_OF_SET_ACTIVE_PCR_BANKS GetResultOfSetActivePcrBanks;
+} EFI_TCG2;
+
+static EFI_STATUS tpm1_measure_to_pcr_and_event_log(const EFI_TCG *tcg, UINT32 pcrindex, const EFI_PHYSICAL_ADDRESS buffer,
+ UINTN buffer_size, const CHAR16 *description) {
+ EFI_STATUS status;
+ TCG_PCR_EVENT *tcg_event;
+ UINT32 event_number;
+ EFI_PHYSICAL_ADDRESS event_log_last;
+ UINTN desc_len;
+
+ desc_len = (StrLen(description) + 1) * sizeof(CHAR16);
+
+ tcg_event = AllocateZeroPool(desc_len + sizeof(TCG_PCR_EVENT));
+
+ if (!tcg_event)
+ return EFI_OUT_OF_RESOURCES;
+
+ tcg_event->EventSize = desc_len;
+ CopyMem((VOID *) & tcg_event->Event[0], (VOID *) description, desc_len);
+
+ tcg_event->PCRIndex = pcrindex;
+ tcg_event->EventType = EV_IPL;
+
+ event_number = 1;
+ status = uefi_call_wrapper(tcg->HashLogExtendEvent, 7,
+ (EFI_TCG *) tcg, buffer, buffer_size, TCG_ALG_SHA, tcg_event, &event_number, &event_log_last);
+
+ if (EFI_ERROR(status))
+ return status;
+
+ uefi_call_wrapper(BS->FreePool, 1, tcg_event);
+
+ return EFI_SUCCESS;
+}
+
+/*
+ * According to TCG EFI Protocol Specification for TPM 2.0 family,
+ * all events generated after the invocation of EFI_TCG2_GET_EVENT_LOG
+ * shall be stored in an instance of an EFI_CONFIGURATION_TABLE aka
+ * EFI TCG 2.0 final events table. Hence, it is necessary to trigger the
+ * internal switch through calling get_event_log() in order to allow
+ * to retrieve the logs from OS runtime.
+ */
+static EFI_STATUS trigger_tcg2_final_events_table(const EFI_TCG2 *tcg, EFI_TCG2_EVENT_LOG_FORMAT log_fmt)
+{
+ EFI_PHYSICAL_ADDRESS loc;
+ EFI_PHYSICAL_ADDRESS last_loc;
+ BOOLEAN truncated;
+ return uefi_call_wrapper(tcg->GetEventLog, 5, (EFI_TCG2 *) tcg,
+ log_fmt, &loc, &last_loc, &truncated);
+}
+
+static EFI_STATUS tpm2_measure_to_pcr_and_event_log(const EFI_TCG2 *tcg, UINT32 pcrindex, const EFI_PHYSICAL_ADDRESS buffer,
+ UINT64 buffer_size, const CHAR16 *description, EFI_TCG2_EVENT_LOG_FORMAT log_fmt) {
+ EFI_STATUS status;
+ EFI_TCG2_EVENT *tcg_event;
+ UINTN desc_len;
+ static BOOLEAN triggered = FALSE;
+
+ if (triggered == FALSE) {
+ status = trigger_tcg2_final_events_table(tcg, log_fmt);
+ if (EFI_ERROR(status))
+ return status;
+
+ triggered = TRUE;
+ }
+
+ if ((buffer == NULL) && (buffer_size == 0)) {
+ return EFI_SUCCESS;
+ }
+
+ desc_len = StrLen(description) * sizeof(CHAR16);
+
+ tcg_event = AllocateZeroPool(sizeof(*tcg_event) - sizeof(tcg_event->Event) + desc_len + 1);
+
+ if (!tcg_event)
+ return EFI_OUT_OF_RESOURCES;
+
+ tcg_event->Size = sizeof(*tcg_event) - sizeof(tcg_event->Event) + desc_len + 1;
+ tcg_event->Header.HeaderSize = sizeof(EFI_TCG2_EVENT_HEADER);
+ tcg_event->Header.HeaderVersion = EFI_TCG2_EVENT_HEADER_VERSION;
+ tcg_event->Header.PCRIndex = pcrindex;
+ tcg_event->Header.EventType = EV_IPL;
+
+ CopyMem((VOID *) tcg_event->Event, (VOID *) description, desc_len);
+
+ status = uefi_call_wrapper(tcg->HashLogExtendEvent, 5, (EFI_TCG2 *) tcg, 0, buffer, (UINT64) buffer_size, tcg_event);
+
+ uefi_call_wrapper(BS->FreePool, 1, tcg_event);
+
+ if (EFI_ERROR(status))
+ return status;
+
+ return EFI_SUCCESS;
+}
+
+static EFI_TCG * tcg1_interface_check(void) {
+ EFI_GUID tpm_guid = EFI_TCG_PROTOCOL_GUID;
+ EFI_STATUS status;
+ EFI_TCG *tcg;
+ TCG_BOOT_SERVICE_CAPABILITY capability;
+ UINT32 features;
+ EFI_PHYSICAL_ADDRESS event_log_location;
+ EFI_PHYSICAL_ADDRESS event_log_last_entry;
+
+ status = LibLocateProtocol(&tpm_guid, (void **) &tcg);
+
+ if (EFI_ERROR(status))
+ return NULL;
+
+ capability.Size = (UINT8) sizeof(capability);
+ status = uefi_call_wrapper(tcg->StatusCheck, 5, tcg, &capability, &features, &event_log_location, &event_log_last_entry);
+
+ if (EFI_ERROR(status))
+ return NULL;
+
+ if (capability.TPMDeactivatedFlag)
+ return NULL;
+
+ if (!capability.TPMPresentFlag)
+ return NULL;
+
+ return tcg;
+}
+
+static EFI_TCG2 * tcg2_interface_check(EFI_TCG2_BOOT_SERVICE_CAPABILITY *caps) {
+ EFI_GUID tpm2_guid = EFI_TCG2_PROTOCOL_GUID;
+ EFI_STATUS status;
+ EFI_TCG2 *tcg;
+
+ status = LibLocateProtocol(&tpm2_guid, (void **) &tcg);
+
+ if (EFI_ERROR(status))
+ return NULL;
+
+ caps->Size = (UINT8) sizeof(EFI_TCG2_BOOT_SERVICE_CAPABILITY);
+ status = uefi_call_wrapper(tcg->GetCapability, 2, tcg, caps);
+
+ if (EFI_ERROR(status))
+ return NULL;
+
+ if (caps->StructureVersion.Major == 1 &&
+ caps->StructureVersion.Minor == 0) {
+ TCG_BOOT_SERVICE_CAPABILITY *caps_1_0;
+ caps_1_0 = (TCG_BOOT_SERVICE_CAPABILITY *)caps;
+ if (caps_1_0->TPMPresentFlag)
+ return tcg;
+ }
+
+ if (!caps->TPMPresentFlag)
+ return NULL;
+
+ return tcg;
+}
+
+EFI_STATUS tpm_log_event(UINT32 pcrindex, const EFI_PHYSICAL_ADDRESS buffer, UINTN buffer_size, const CHAR16 *description) {
+ EFI_TCG *tpm1;
+ EFI_TCG2 *tpm2;
+ EFI_TCG2_BOOT_SERVICE_CAPABILITY caps;
+
+ tpm2 = tcg2_interface_check(&caps);
+ if (tpm2) {
+ EFI_TCG2_EVENT_LOG_BITMAP supported_logs;
+ EFI_TCG2_EVENT_LOG_FORMAT log_fmt;
+
+ if (caps.StructureVersion.Major == 1 &&
+ caps.StructureVersion.Minor == 0)
+ supported_logs = ((TREE_BOOT_SERVICE_CAPABILITY *)&caps)->SupportedEventLogs;
+ else
+ supported_logs = caps.SupportedEventLogs;
+
+ if (supported_logs & EFI_TCG2_EVENT_LOG_FORMAT_TCG_2)
+ log_fmt = EFI_TCG2_EVENT_LOG_FORMAT_TCG_2;
+ else
+ log_fmt = EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2;
+
+ return tpm2_measure_to_pcr_and_event_log(tpm2, pcrindex, buffer, buffer_size, description, log_fmt);
+ }
+
+ tpm1 = tcg1_interface_check();
+ if (tpm1)
+ return tpm1_measure_to_pcr_and_event_log(tpm1, pcrindex, buffer, buffer_size, description);
+
+ /* No active TPM found, so don't return an error */
+ return EFI_SUCCESS;
+}
+
+#endif
diff --git a/measure.h b/measure.h
new file mode 100644
index 0000000..5d41ed5
--- /dev/null
+++ b/measure.h
@@ -0,0 +1,5 @@
+/* Copied verbatim from systemd/src/boot/efi/measured.c - authored by har...@redhat.com */
+/* SPDX-License-Identifier: LGPL-2.1+ */
+#pragma once
+
+EFI_STATUS tpm_log_event(UINT32 pcrindex, const EFI_PHYSICAL_ADDRESS buffer, UINTN buffer_size, const CHAR16 *description);
--
2.30.2

[Cedric Hombourger]

Make use of systemd code to measure the kernel command line for the
selected configuration into PCR#8. This also causes the firmware
to add the measurement for the loaded EFI image into PCR#4 (as per
UEFI specs). With this change both the loaded kernel and command
line options are measured. Use of the TPM may be disabled by passing
--disable-tpm to the configure script.

Signed-off-by: Cedric Hombourger <Cedric_H...@mentor.com>
---

configure.ac | 16 ++++++++++++++++
main.c | 12 ++++++++++++
2 files changed, 28 insertions(+)

diff --git a/configure.ac b/configure.ac
index b8d2b1c..2d434d5 100644
--- a/configure.ac
+++ b/configure.ac
@@ -77,6 +77,22 @@ AC_ARG_WITH([gnuefi-lib-dir],
AC_SUBST(GNUEFI_LIB_DIR)
AC_DEFINE_UNQUOTED(GNUEFI_LIB_DIR, ["$GNUEFI_LIB_DIR"], [The lib directory for gnuefi])

+AC_ARG_ENABLE([tpm],
+ AS_HELP_STRING([--disable-tpm], [Disable TPM support]))
+
+AS_IF([test "x$enable_tpm" != "xno"], [
+ AC_DEFINE([ENABLE_TPM], [1], [TPM support])
+])
+
+AC_ARG_WITH([options-pcr],
+ AS_HELP_STRING([--with-options-pcr=NUMBER],
+ [specify the PCR register to use for options, defaults to 8]),
+ [OPTIONS_PCR="$withval"],
+ [OPTIONS_PCR="8"])
+
+AC_SUBST(OPTIONS_PCR)
+AC_DEFINE_UNQUOTED(OPTIONS_PCR, [$OPTIONS_PCR], [The PCR register for options])
+
dnl Define ARCH_<NAME> conditionals
SET_ARCH(I586, i586*)
SET_ARCH(I686, i686*)
diff --git a/main.c b/main.c
index 7949218..dc388a4 100644
--- a/main.c
+++ b/main.c
@@ -22,6 +22,7 @@
#include <configuration.h>
#include "version.h"
#include "utils.h"
+#include "measure.h"

extern const unsigned long init_array_start[];
extern const unsigned long init_array_end[];
@@ -192,6 +193,17 @@ EFI_STATUS efi_main(EFI_HANDLE image_handle, EFI_SYSTEM_TABLE *system_table)
loaded_image->LoadOptionsSize =
(StrLen(bg_loader_params.payload_options) + 1) * sizeof(CHAR16);

+#ifdef ENABLE_TPM
+ /* Try to log any options to the TPM */
+ status = tpm_log_event(OPTIONS_PCR,
+ (EFI_PHYSICAL_ADDRESS) (UINTN) loaded_image->LoadOptions,
+ loaded_image->LoadOptionsSize, loaded_image->LoadOptions);
+ /* Try to log any options to the TPM, especially manually edited options */
+ if (EFI_ERROR(status)) {
+ WARNING(L"Could not measure options (%x)\n", status);
+ }
+#endif
+
INFO(L"Starting %s with watchdog set to %d seconds ...\n",
bg_loader_params.payload_path, bg_loader_params.timeout);

--
2.30.2

[Christian Storm]

Hi Cedric,

> Make use of systemd code

This is licensed differently (LGPL-2.1+) than EFI Boot Guard.
Can you elaborate on the license implications of importing this?

> to measure the kernel command line for the
> selected configuration into PCR#8. This also causes the firmware
> to add the measurement for the loaded EFI image into PCR#4 (as per
> UEFI specs).

Hm, can you point me to the according Spec section please? Maybe I have
missed it that measuring something to PCR #8 automatically causes measuring
the bootloader's chain-callee EFI binary (e.g. the UEFI-stub'd Kernel)
to PCR #4....

Here you measure the chain-loaded kernel command line, *not* the kernel
binary itself. See above.

> + /* Try to log any options to the TPM, especially manually edited options */

This comment is misleading for the following action:

> + if (EFI_ERROR(status)) {
> + WARNING(L"Could not measure options (%x)\n", status);
> + }
> +#endif
> +
> INFO(L"Starting %s with watchdog set to %d seconds ...\n",
> bg_loader_params.payload_path, bg_loader_params.timeout);
>
> --
> 2.30.2
>

Kind regards,
Christian

--
Dr. Christian Storm
Siemens AG, Technology, T RDA IOT SES-DE
Otto-Hahn-Ring 6, 81739 München, Germany

[Cedric Hombourger]

Hi Christian,

On 6/28/2021 3:54 PM, Christian Storm wrote:
> Hi Cedric,
>
>> Make use of systemd code
> This is licensed differently (LGPL-2.1+) than EFI Boot Guard.
> Can you elaborate on the license implications of importing this?

To the best of my knowledge, this use-case is covered by the LGPL:

"When a program is linked with a library, whether statically or using a
shared library, the combination of the two is
legally speaking a combined work, a derivative of the original library.
The ordinary General Public License therefore
permits such linking only if the entire combination fits its criteria of
freedom. The Lesser General Public License permits
more lax criteria for linking other code with the library"

and

"You may opt to apply the terms of the ordinary GNU General Public
License instead of this License to a given copy of the Library. To do
this, you must alter all the notices that refer to this License, so that
they refer to the ordinary GNU General Public License, version 2,
instead of to this License. (If a newer version than version 2 of the
ordinary GNU General Public License has appeared, then you can specify
that version instead if you wish.) Do not make any other change in these
notices."

It is my understanding that this clause allows for direct reuse of
LGPLed code in GPLed libraries and applications provided that the
derivative work (i.e. the copy) is re-licensed under the GPL (I should
therefore amend the SPDX license headers).

>> to measure the kernel command line for the
>> selected configuration into PCR#8. This also causes the firmware
>> to add the measurement for the loaded EFI image into PCR#4 (as per
>> UEFI specs).
> Hm, can you point me to the according Spec section please? Maybe I have
> missed it that measuring something to PCR #8 automatically causes measuring
> the bootloader's chain-callee EFI binary (e.g. the UEFI-stub'd Kernel)
> to PCR #4....

Page 13 of the TCG EFI Platform Spec:

    What the “measure before applying relocations” described below
practically means is that the EFI implementation will
    perform “LoadImage ( )” actions (e.g., copying PE/COFF to memory,
etc), measurement, and then relocation application,
    and finally, the EFI service “StartImage ( ).” As such, EFI
implementations of these services MUST punctuate their flow with
    this measurement action.

It's not really the measurement of our command line that causes the
measurement of our EFI application to be logged but rather the fact that
systemd's trigger_tcg2_final_events_table() that causes the pre-OS event
log to be finalized (as I understand it).

[Jan Kiszka]

So, all this is just for measuring the parameters passed to the kernel
image, right? Does it actually make sense in secure scenarios to use
that feature? We are currently using unified kernel images, with the
kernel parameters baked in. That permits to reject booting at all and
automatically measures the options as well.

Jan

--
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux

[Cedric Hombourger]

So are we but systemd's measure module will do two things (1) finalize
the event table (only when called for the 1st time) (2) measure the
provided buffer (options both in their case and in this patch-series).
My understanding is that tables need to be finalized for the measurement
produced by LoadImage() to extend PCR[4]. At least that's my observation
on the different pieces of hardware (and VMs) I was able to test this
change on.

The specs clearly state that LoadImage() must measure the PE/COFF image
before it gets relocated and that makes sense. They are not very clear
as to why the events table need to be finalized for PCR[4] to be
extended with the loaded image.

>
> Jan
>

[Jan Kiszka]

Means the above measurement of options is primarily an "end marker" to
the loading of images? Sorry, I'm not yet familiar with that measurement
concept of UEFI.

[Christian Storm]

Hi,

> > [...]

> > So, all this is just for measuring the parameters passed to the kernel
> > image, right? Does it actually make sense in secure scenarios to use
> > that feature? We are currently using unified kernel images, with the
> > kernel parameters baked in. That permits to reject booting at all and
> > automatically measures the options as well.

Yes, correct for the secure boot case as you're not allowed to change
the parameters then by the implementation of systemd-boot's stub which
is used to bake unified kernel images, c.f.
https://github.com/systemd/systemd/blob/main/src/boot/efi/stub.c#L58
A different implementation may allow you to do this though....

Apart from that, measured boot is not tied to secure boot and so it does
have its use in the non-secure boot case IMO. Whether we want to support
that is a different question though.

> So are we but systemd's measure module will do two things (1) finalize the
> event table (only when called for the 1st time)

What version (sha) did you actually import from systemd-boot? This
functionality you're referring to here has been removed at Jun 3, 2019,
i.e., one year ago, with this commit

https://github.com/systemd/systemd/commit/f8e54bf31970d9988bf05e70f75a3e05187f4e30

as the reasoning for adding it in the first place was wrong.

[Cedric Hombourger]

Re-use systemd code to measure binary blobs to get both the loaded EFI
image and the kernel options measured into the TPM PCR registers 4 and
8. The --disable-tpm and --options-pcr options were added to the

configure script. The changes were tested on x86 platforms with TPM
versions 1.2 and 2 but also without a TPM (just like systemd-boot, the

absence of a TPM does not cause any harm even when the ENABLE_TPM
pre-processing directive is in effect).

Changes in v2:
- Pull the latest version of the systemd code
- Re-license the LGPL code under the GPL (as permitted by the LGPL)

Cedric Hombourger (2):
measure: import systemd code to perform measurements into the TPM
main: measure the kernel command line into the TPM

Makefile.am | 1 +
configure.ac | 16 +++
main.c | 12 ++

measure.c | 317 +++++++++++++++++++++++++++++++++++++++++++++++++++
measure.h | 6 +
5 files changed, 352 insertions(+)

create mode 100644 measure.c
create mode 100644 measure.h

--
2.30.2

[Cedric Hombourger]

Import systemd code (fac6511e49) to get the loaded image and options

measures (which will be done in a subsequent commit). This commit brings
the code authored by har...@redhat.com and will be used (later) when
ENABLE_TPM is in effect.

Update SPDX license headers from LGPL 2.1 (or later) to GPL 2.1 as
permitted by the LGPL when importing LGPL code into a GPL application.

Signed-off-by: Cedric Hombourger <Cedric_H...@mentor.com>
---

Makefile.am | 1 +
measure.c | 317 ++++++++++++++++++++++++++++++++++++++++++++++++++++
measure.h | 6 +
3 files changed, 324 insertions(+)

create mode 100644 measure.c
create mode 100644 measure.h

diff --git a/Makefile.am b/Makefile.am
index 8fb8652..90f2959 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -135,6 +135,7 @@ efi_sources = \
env/fatvars.c \
utils.c \
bootguard.c \
+ measure.c \
main.c

efi_cppflags = \
diff --git a/measure.c b/measure.c
new file mode 100644

index 0000000..2806134
--- /dev/null
+++ b/measure.c
@@ -0,0 +1,317 @@
+/* SPDX-License-Identifier: GPL-2.1-or-later */

+
+#if ENABLE_TPM
+
+#include <efi.h>
+#include <efilib.h>
+

+#include "measure.h"
+
+#define EFI_TCG_GUID \
+ &(EFI_GUID) { 0xf541796d, 0xa62e, 0x4954, { 0xa7, 0x75, 0x95, 0x84, 0xf6, 0x1b, 0x9c, 0xdd } }

+#define EFI_TCG2_GUID \
+ &(EFI_GUID) { 0x607f766c, 0x7455, 0x42be, { 0x93, 0x0b, 0xe4, 0xd7, 0x6d, 0xb2, 0x72, 0x0f } }

+
+typedef struct tdEFI_TCG2_PROTOCOL EFI_TCG2_PROTOCOL;
+
+typedef UINT32 EFI_TCG2_EVENT_LOG_BITMAP;
+typedef UINT32 EFI_TCG2_EVENT_LOG_FORMAT;
+typedef UINT32 EFI_TCG2_EVENT_ALGORITHM_BITMAP;
+

+static EFI_STATUS tpm2_measure_to_pcr_and_event_log(const EFI_TCG2 *tcg, UINT32 pcrindex, const EFI_PHYSICAL_ADDRESS buffer,

+ UINT64 buffer_size, const CHAR16 *description) {
+ EFI_STATUS status;

+ EFI_TCG2_EVENT *tcg_event;
+ UINTN desc_len;
+

+ EFI_STATUS status;
+ EFI_TCG *tcg;
+ TCG_BOOT_SERVICE_CAPABILITY capability;
+ UINT32 features;
+ EFI_PHYSICAL_ADDRESS event_log_location;
+ EFI_PHYSICAL_ADDRESS event_log_last_entry;
+

+ status = LibLocateProtocol(EFI_TCG_GUID, (void **) &tcg);

+
+ if (EFI_ERROR(status))
+ return NULL;
+
+ capability.Size = (UINT8) sizeof(capability);
+ status = uefi_call_wrapper(tcg->StatusCheck, 5, tcg, &capability, &features, &event_log_location, &event_log_last_entry);
+
+ if (EFI_ERROR(status))
+ return NULL;
+
+ if (capability.TPMDeactivatedFlag)
+ return NULL;
+
+ if (!capability.TPMPresentFlag)
+ return NULL;
+
+ return tcg;
+}
+

+static EFI_TCG2 * tcg2_interface_check(void) {

+ EFI_STATUS status;
+ EFI_TCG2 *tcg;

+ EFI_TCG2_BOOT_SERVICE_CAPABILITY capability;
+
+ status = LibLocateProtocol(EFI_TCG2_GUID, (void **) &tcg);

+
+ if (EFI_ERROR(status))
+ return NULL;
+

+ capability.Size = (UINT8) sizeof(EFI_TCG2_BOOT_SERVICE_CAPABILITY);
+ status = uefi_call_wrapper(tcg->GetCapability, 2, tcg, &capability);

+
+ if (EFI_ERROR(status))
+ return NULL;
+

+ if (capability.StructureVersion.Major == 1 &&
+ capability.StructureVersion.Minor == 0) {
+ TCG_BOOT_SERVICE_CAPABILITY *caps_1_0;
+ caps_1_0 = (TCG_BOOT_SERVICE_CAPABILITY *)&capability;

+ if (caps_1_0->TPMPresentFlag)
+ return tcg;
+ }
+

+ if (!capability.TPMPresentFlag)
+ return NULL;
+
+ return tcg;
+}
+

+EFI_STATUS tpm_log_event(UINT32 pcrindex, const EFI_PHYSICAL_ADDRESS buffer, UINTN buffer_size, const CHAR16 *description) {
+ EFI_TCG *tpm1;
+ EFI_TCG2 *tpm2;
+

+ tpm2 = tcg2_interface_check();
+ if (tpm2)
+ return tpm2_measure_to_pcr_and_event_log(tpm2, pcrindex, buffer, buffer_size, description);

+
+ tpm1 = tcg1_interface_check();
+ if (tpm1)
+ return tpm1_measure_to_pcr_and_event_log(tpm1, pcrindex, buffer, buffer_size, description);
+
+ /* No active TPM found, so don't return an error */
+ return EFI_SUCCESS;
+}
+
+#endif
diff --git a/measure.h b/measure.h
new file mode 100644

index 0000000..65c7a9d
--- /dev/null
+++ b/measure.h
@@ -0,0 +1,6 @@
+/* SPDX-License-Identifier: GPL-2.1-or-later */
+#pragma once
+
+#include <efi.h>

[Cedric Hombourger]

Make use of systemd code to measure the kernel command line for the
selected configuration into PCR#8. This also causes the firmware
to add the measurement for the loaded EFI image into PCR#4 (as per
UEFI specs). With this change both the loaded kernel and command
line options are measured. Use of the TPM may be disabled by passing
--disable-tpm to the configure script.

Signed-off-by: Cedric Hombourger <Cedric_H...@mentor.com>
---

--
2.30.2

[Christian Storm]

Hi Cedric,

> Make use of systemd code to measure the kernel command line for the
> selected configuration into PCR#8. This also causes the firmware
> to add the measurement for the loaded EFI image into PCR#4 (as per
> UEFI specs). With this change both the loaded kernel and command
> line options are measured.

Measurement to PCR #4 is done by the EFI firmware for each LoadImage()'d
EFI binary. According to the spec, "UEFI Boot Manager Code (usually the
MBR) and Boot Attempts" is supposed to be measured to PCR #4. Now, an
OS kernel doesn't exactly qualify for this semantically, right?
Probably, PCRs #8 - #15 "Defined for use by the Static OS" (according to
the spec) are a better match for measuring the OS kernel, additionally.
You're measuring the kernel command line to this range already.

That said, what's the overarching concept behind this? Just measuring
"something" is not an end in itself. How is that supposed to be sensibly
used (e.g., by the OS) to actually leverage the measurements? According
to this answer, the measurement(s) should be performed by EFI Boot Guard
to support the intended idea/concept.
As it is now, you have to poke and replay PCR #4 to assess the OS kernel
as the OS kernel and the bootloader are mashed up in PCR #4. Otherwise,
you can just tell that *some* OS kernel has been booted with the same or
different kernel command lines (via PCR #8).

> Use of the TPM may be disabled by passing --disable-tpm to the
> configure script.

Why this distinction? It doesn't harm to have it enabled unconditionally
with measurement simply failing in case no TPM is available, does it?
What is the reasoning behind making in compile-time conditional?

[Cedric Hombourger]

Hi Christian

On 7/1/2021 9:02 PM, Christian Storm wrote:
> Hi Cedric,
>
>> Make use of systemd code to measure the kernel command line for the
>> selected configuration into PCR#8. This also causes the firmware
>> to add the measurement for the loaded EFI image into PCR#4 (as per
>> UEFI specs). With this change both the loaded kernel and command
>> line options are measured.
> Measurement to PCR #4 is done by the EFI firmware for each LoadImage()'d
> EFI binary. According to the spec, "UEFI Boot Manager Code (usually the
> MBR) and Boot Attempts" is supposed to be measured to PCR #4.

Correct. More specifically, the target PCR is selected as follows:

EFI_IMAGE_SUBSYSTEM_EFI_APPLICATION => 4
EFI_IMAGE_SUBSYSTEM_EFI_BOOT_SERVICE_DRIVER => 2
EFI_IMAGE_SUBSYSTEM_EFI_RUNTIME_DRIVER => 2

The type of the image is encoded into the PE/COFF header

> Now, an
> OS kernel doesn't exactly qualify for this semantically, right?

Correct but LoadImage() does not seem to provide a mean to select a PCR

> Probably, PCRs #8 - #15 "Defined for use by the Static OS" (according to
> the spec) are a better match for measuring the OS kernel, additionally.
> You're measuring the kernel command line to this range already.

This would have been my preference as well but again LoadImage() isn't
giving an option to select a different PCR (unless I missed it). What we
could do is side-load the UEFI image and HashLogExtend it ourselves to
PCR[8] but LoadImage would still be needed and would also measure it and
extend PCR[4]. I'll check again what other boot-loaders do (we may not
be the only ones load a Linux kernel image packaged as an EFI binary)

>
> That said, what's the overarching concept behind this? Just measuring
> "something" is not an end in itself. How is that supposed to be sensibly
> used (e.g., by the OS) to actually leverage the measurements? According
> to this answer, the measurement(s) should be performed by EFI Boot Guard
> to support the intended idea/concept.

We are wanting to get boot artifacts measured so we can locally attest
that we have booted into a well-known system and may safely unseal a
secret sealed into the TPM. Our secret would be sealed with at least
PCR[4] and PCR[8]. In our case, we don't really need to say which
element was tampered, we just want to prevent the bad guy from reading
our secret.

> As it is now, you have to poke and replay PCR #4 to assess the OS kernel
> as the OS kernel and the bootloader are mashed up in PCR #4. Otherwise,
> you can just tell that *some* OS kernel has been booted with the same or
> different kernel command lines (via PCR #8).

Only if your goal is to provide full attestation and needing to provide
details and what was tampered.

>
>> Use of the TPM may be disabled by passing --disable-tpm to the
>> configure script.
> Why this distinction? It doesn't harm to have it enabled unconditionally
> with measurement simply failing in case no TPM is available, does it?
> What is the reasoning behind making in compile-time conditional?

I would be happy to remove this and all the #ifdef ENABLE_TPM. I was
just worried that some people may not want this code enabled/built even
though I see no harm in always having it.

>
> Kind regards,
> Christian
>

[Christian Storm]

Hi Cedric,

> > > Make use of systemd code to measure the kernel command line for the
> > > selected configuration into PCR#8. This also causes the firmware
> > > to add the measurement for the loaded EFI image into PCR#4 (as per
> > > UEFI specs). With this change both the loaded kernel and command
> > > line options are measured.
> > Measurement to PCR #4 is done by the EFI firmware for each LoadImage()'d
> > EFI binary. According to the spec, "UEFI Boot Manager Code (usually the
> > MBR) and Boot Attempts" is supposed to be measured to PCR #4.
>
> Correct. More specifically, the target PCR is selected as follows:
>
> EFI_IMAGE_SUBSYSTEM_EFI_APPLICATION => 4

This is what EFI Boot Guard is supposed to be.

> EFI_IMAGE_SUBSYSTEM_EFI_BOOT_SERVICE_DRIVER => 2
> EFI_IMAGE_SUBSYSTEM_EFI_RUNTIME_DRIVER => 2
>
> The type of the image is encoded into the PE/COFF header
>
> > Now, an OS kernel doesn't exactly qualify for this semantically,
> > right?
> Correct but LoadImage() does not seem to provide a mean to select a PCR

True. It's always PCR #4 but that doesn't hinder you from measuring the
binary to some other and more fitting PCR(s) as well ― matching your use
case.

> > Probably, PCRs #8 - #15 "Defined for use by the Static OS" (according to
> > the spec) are a better match for measuring the OS kernel, additionally.
> > You're measuring the kernel command line to this range already.
> This would have been my preference as well but again LoadImage() isn't
> giving an option to select a different PCR (unless I missed it).

No, that's not foreseen, but see above.

> What we could do is side-load the UEFI image and HashLogExtend it
> ourselves to PCR[8] but LoadImage would still be needed and would also
> measure it and extend PCR[4].

Yes, you can measure the binary to PCR #8 as well, then however mashing
up the kernel binary and the kernel's cmdline into one PCR. Whether
that's sensible or not is the question I asked for referring to the
overarching concept you'll want to follow here, i.e., what information /
deduction does it give you and what is it you're after (as just
measuring is not an end in itself)?

> I'll check again what other boot-loaders do (we may not be the only ones load
> a Linux kernel image packaged as an EFI binary)

Certainly not :) No need, here it is:

- systemd-boot measures kernel command line options to PCR #8:
https://github.com/systemd/systemd/blob/main/src/boot/efi/boot.c#L2201
https://github.com/systemd/systemd/blob/main/src/boot/efi/stub.c#L70
- Trusted Grub measures its core to PCR #9 and chain-loadees to PCR #10
https://github.com/Rohde-Schwarz/TrustedGRUB2/blob/master/README.md#13-measurements-in-short
- rhboot/grub2 measures Strings to PCR #8 and Binaries to PCR #9
https://github.com/rhboot/grub2/blob/master/include/grub/tpm.h#L23

You surely can find some more but this will give you an idea of how
others do it...

> > That said, what's the overarching concept behind this? Just measuring
> > "something" is not an end in itself. How is that supposed to be sensibly
> > used (e.g., by the OS) to actually leverage the measurements? According
> > to this answer, the measurement(s) should be performed by EFI Boot Guard
> > to support the intended idea/concept.
> We are wanting to get boot artifacts measured so we can locally attest that we
> have booted into a well-known system and may safely unseal a secret sealed
> into the TPM. Our secret would be sealed with at least PCR[4] and PCR[8]. In
> our case, we don't really need to say which element was tampered, we just want
> to prevent the bad guy from reading our secret.

OK, but that ties you to this particular kernel binary (PCR#4) and this
particular cmdline (PCR#8). If you now change something, think firmware
update, you have to re-seal the secret which is a delicate operation to
perform in the context of a firmware update transaction...

That said, if you have Secure Boot enabled and use unified kernel
images, what does measuring the cmdline to PCR#8 give you in addition to
just sealing the secret with ― probably among others ― PCR#4 and thereby
bind it to (at least) the unified kernel image ― which includes the
cmdline as PE/COFF section?

> > > Use of the TPM may be disabled by passing --disable-tpm to the
> > > configure script.
> > Why this distinction? It doesn't harm to have it enabled unconditionally
> > with measurement simply failing in case no TPM is available, does it?
> > What is the reasoning behind making in compile-time conditional?
>
> I would be happy to remove this and all the #ifdef ENABLE_TPM. I was just
> worried that some people may not want this code enabled/built even though I
> see no harm in always having it.

I'm in favor of enabling it unconditionally so that it measures if a TPM
is available and fails (gracefully) in case it is not.

[Cedric Hombourger]

Hi Christian,

On 7/2/2021 3:50 PM, Christian Storm wrote:
> Hi Cedric,
>
>>>> Make use of systemd code to measure the kernel command line for the
>>>> selected configuration into PCR#8. This also causes the firmware
>>>> to add the measurement for the loaded EFI image into PCR#4 (as per
>>>> UEFI specs). With this change both the loaded kernel and command
>>>> line options are measured.
>>> Measurement to PCR #4 is done by the EFI firmware for each LoadImage()'d
>>> EFI binary. According to the spec, "UEFI Boot Manager Code (usually the
>>> MBR) and Boot Attempts" is supposed to be measured to PCR #4.
>> Correct. More specifically, the target PCR is selected as follows:
>>
>> EFI_IMAGE_SUBSYSTEM_EFI_APPLICATION => 4
> This is what EFI Boot Guard is supposed to be.
>
>> EFI_IMAGE_SUBSYSTEM_EFI_BOOT_SERVICE_DRIVER => 2
>> EFI_IMAGE_SUBSYSTEM_EFI_RUNTIME_DRIVER => 2
>>
>> The type of the image is encoded into the PE/COFF header
>>
>>> Now, an OS kernel doesn't exactly qualify for this semantically,
>>> right?
>> Correct but LoadImage() does not seem to provide a mean to select a PCR
> True. It's always PCR #4 but that doesn't hinder you from measuring the
> binary to some other and more fitting PCR(s) as well ― matching your use
> case.

I can send a v3 that does that. Here are the measurements I collected
from the TPM event log

/* efibootguard measured in PCR[4] */
PCR 4 -- Event 80000003 <TpmEventType.EFI_BOOT_SERVICES_APPLICATION>
Extend (SHA1): 3877b5f89f59bc5d934ba4b26672fea0ffb0806c
Path vector:
  * ACPIDevice           ACPI b'\xd0A\x03\n\x00\x00\x00\x00'
  * HardwareDevice       PCI                  b'\x00\x10'
  * MessagingDevice      SCSI                 b'\x00\x00\x00\x00'
  * MediaDevice          HardDrive
b'\x01\x00\x00\x00\x00\x08\x00\x00\x00\x00\x00\x00\x00\x00\x01\x00\x00\x00\x00\x00\\&\x0b\xa9\xa7\xee\xf1C\xb4".\xd3M\xf6\xe6Y\x02\x02'
  * MediaDevice          FilePath \EFI\BOOT\BOOTX64.EFI
  * End                  255                  b''

/* our measurement of the specified kernel image in PCR[8]
 * note: this is a raw measurement (i.e. SHA1 of the entire file
without interpreting e.g PE/COFF sections)
 */
PCR 8 -- Event 0000000D <TpmEventType.IPL>
Extend (SHA1): e42ee38fd3cf1f7d310f00e4331628b0a188ef3a
0x00000000: 4D 5A 90 00 03 00 00 |MZ.....         |
-- not touching non-systemd IPL event --
--> after this event, PCR 8 contains value
58add887d84dd4be97a32a6ba6cb5535f84a7663
--> after reboot, PCR 8 will contain value
58add887d84dd4be97a32a6ba6cb5535f84a7663


/* LoadImage() still measures our kernel image in PCR[4]
 * note: EFI knows it is a PE/COFF binary and only relevant sections
are hashed
 */
PCR 4 -- Event 80000003 <TpmEventType.EFI_BOOT_SERVICES_APPLICATION>
Extend (SHA1): a9823f0033e9cb2c30019b4bab7a9ee31071debf
Path vector:
  * HardwareDevice       MemoryMapped
b'\x02\x00\x00\x00\x18\xc0\x10\x0b\x00\x00\x00\x00\x10\xcd\xbd\r\x00\x00\x00\x00'
  * End                  255                  b''

/* our measurement of the kernel command line into PCR[9] */
PCR 9 -- Event 0000000D <TpmEventType.IPL>
Extend (SHA1): 1985ccaadef02e21d2f161311bd35ea4ce89676e
0x00000000: 72 00 6F 00 6F 00 74 00 77 00 61 00 69 00 74 00
|r.o.o.t.w.a.i.t.|
0x00000010: 20 00 71 00 75 00 69 00 65 00 74 00 20 00 6E 00
|..q.u.i.e.t...n.|
0x00000020: 6F 00 68 00 7A 00 3D 00 6F 00 66 00 66 00 20 00
|o.h.z.=.o.f.f...|
0x00000030: 73 00 65 00 63 00 75 00 72 00 69 00 74 00 79 00
|s.e.c.u.r.i.t.y.|
(125 more bytes)

So as noted before, the kernel image gets measured twice...

>
>>> Probably, PCRs #8 - #15 "Defined for use by the Static OS" (according to
>>> the spec) are a better match for measuring the OS kernel, additionally.
>>> You're measuring the kernel command line to this range already.
>> This would have been my preference as well but again LoadImage() isn't
>> giving an option to select a different PCR (unless I missed it).
> No, that's not foreseen, but see above.
>
>> What we could do is side-load the UEFI image and HashLogExtend it
>> ourselves to PCR[8] but LoadImage would still be needed and would also
>> measure it and extend PCR[4].
> Yes, you can measure the binary to PCR #8 as well, then however mashing
> up the kernel binary and the kernel's cmdline into one PCR. Whether
> that's sensible or not is the question I asked for referring to the
> overarching concept you'll want to follow here, i.e., what information /
> deduction does it give you and what is it you're after (as just
> measuring is not an end in itself)?

Our concept is twofold: (1) use SecureBoot to make sure we are booted
from a trusted boot-loader + trusted Linux kernel (both are signed with
keys loaded into the trust store) (2) extend the root of trust by
mounting a LUKS encrypted volume with the key in the TPM and sealed with
selected PCRs. In an A/B scheme, we may have the same encryption key
placed both in slot[A] and slot[B] but sealed as follows:

 * for A: PCR[4] == HashOf(efibootguard.efi) + PCR[8] ==
HashOf(kernelA) + PCR[9] == HashOf(optionsA) + other relevant PCRs (e.g.
BIOS, EFI variables, etc.)
 * for B: PCR[4] == HashOf(efibootguard.efi) + PCR[8] ==
HashOf(kernelB) + PCR[9] == HashOf(optionsB) + other relevant PCRs (e.g.
BIOS, EFI variables, etc.)

Note that for our purpose (and I understand we may now want restrict
ourselves to that specific use-case), the following sealing would have
worked:

 * for A: PCR[4] == Extend(HashOf(efibootguard.efi), HashOf(kernelA)) +
PCR[9] == HashOf(optionsA) + other relevant PCRs (e.g. BIOS, EFI
variables, etc.)
 * for B: PCR[4] == Extend(HashOf(efibootguard.efi), HashOf(kernelB)) +
PCR[9] == HashOf(optionsB) + other relevant PCRs (e.g. BIOS, EFI
variables, etc.)

since we don't really need/want to know which binary was tampered but
simply to maintain the root of trust ("yes the booted kernel is the one
I expected, I may mount my root file-system and run my user-land stack)

or seal the same encryption key using a different slot in your TPM so
you don't change anything in your current configuration. If something in
your new system wasn't done right, all pieces to revert to the old
system should still be in place

>
> That said, if you have Secure Boot enabled and use unified kernel
> images, what does measuring the cmdline to PCR#8 give you in addition to
> just sealing the secret with ― probably among others ― PCR#4 and thereby
> bind it to (at least) the unified kernel image ― which includes the
> cmdline as PE/COFF section?

in my experiments, I was able to override the kernel command line with
bg_setenv -a. this is probably a "feature" of systemd's stub (maybe this
would not happen when directly building the kernel as an EFI binary but
I am guessing that efibootguard should not assume a specific unified
kernel image implementation?)

>
>
>>>> Use of the TPM may be disabled by passing --disable-tpm to the
>>>> configure script.
>>> Why this distinction? It doesn't harm to have it enabled unconditionally
>>> with measurement simply failing in case no TPM is available, does it?
>>> What is the reasoning behind making in compile-time conditional?
>> I would be happy to remove this and all the #ifdef ENABLE_TPM. I was just
>> worried that some people may not want this code enabled/built even though I
>> see no harm in always having it.
> I'm in favor of enabling it unconditionally so that it measures if a TPM
> is available and fails (gracefully) in case it is not.

ok - thanks for the guidance
>
>
>
> Kind regards,
> Christian
>

[Cedric Hombourger]

Re-use systemd code to measure binary blobs to get both the loaded EFI

image and the kernel options measured into the TPM PCR registers 8 and
9. The --kernel-pcr and --options-pcr options were added to the

configure script. The changes were tested on x86 platforms with TPM
versions 1.2 and 2 but also without a TPM (just like systemd-boot, the

absence of a TPM does not cause any harm or errors).

Changes in v3:
- load kernel into memory using introduced load_from_device_path()
- make LoadImage() relocate the image in memory instead of loading it
from disk
- remove --disable-tpm from configure.ac and "#if ENABLE_TPM" from
main.c and measure.c
- add --kernel-pcr to configure.ac (defines KERNEL_PCR)

Changes in v2:
- Pull the latest version of the systemd code
- Re-license the LGPL code under the GPL (as permitted by the LGPL)

Cedric Hombourger (3):
loader: add load_from_device_path() to load a binary into memory

measure: import systemd code to perform measurements into the TPM

main: measure kernel image and options

Makefile.am | 4 +-
configure.ac | 18 +++
include/loader.h | 21 ++++
loader.c | 155 +++++++++++++++++++++++
main.c | 25 +++-
measure.c | 313 +++++++++++++++++++++++++++++++++++++++++++++++
measure.h | 6 +
7 files changed, 538 insertions(+), 4 deletions(-)
create mode 100644 include/loader.h
create mode 100644 loader.c

create mode 100644 measure.c
create mode 100644 measure.h

--
2.30.2

[Cedric Hombourger]

In order to measure kernel images to a PCR of our choice, we need to
load ourselves the selected image from the specified device to memory.
We may then call LoadImage() with our memory buffer instead of our
device path (this will cause the loaded image to be relocated and
ready for StartImage()).

Signed-off-by: Cedric Hombourger <Cedric_H...@mentor.com>
---

Makefile.am | 3 +-
include/loader.h | 21 +++++++
loader.c | 146 +++++++++++++++++++++++++++++++++++++++++++++++
main.c | 15 ++++-
4 files changed, 181 insertions(+), 4 deletions(-)

create mode 100644 include/loader.h
create mode 100644 loader.c

diff --git a/Makefile.am b/Makefile.am
index 8fb8652..fb907ff 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -135,7 +135,8 @@ efi_sources = \
env/fatvars.c \
utils.c \
bootguard.c \
- main.c
+ main.c \
+ loader.c

efi_cppflags = \
-I$(top_builddir) -include config.h \
diff --git a/include/loader.h b/include/loader.h
new file mode 100644
index 0000000..5cf639e
--- /dev/null
+++ b/include/loader.h
@@ -0,0 +1,21 @@
+/*
+ * EFI Boot Guard
+ *
+ * Copyright (C) 2021 Mentor Graphics, A Siemens business
+ *
+ * Authors:
+ * Cedric Hombourger <Cedric_H...@mentor.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See
+ * the COPYING file in the top-level directory.
+ *
+ * SPDX-License-Identifier: GPL-2.0
+ */
+

+#pragma once
+
+#include <efi.h>

+#include <efilib.h>
+
+EFI_STATUS
+load_from_device_path (IN OUT EFI_DEVICE_PATH **device_path, OUT VOID **data, OUT UINTN *size);
diff --git a/loader.c b/loader.c
new file mode 100644
index 0000000..6029781
--- /dev/null
+++ b/loader.c
@@ -0,0 +1,146 @@
+/*
+ * EFI Boot Guard
+ *
+ * Copyright (C) 2021 Mentor Graphics, A Siemens business
+ *
+ * Authors:
+ * Cedric Hombourger <Cedric_H...@mentor.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See
+ * the COPYING file in the top-level directory.
+ *
+ * SPDX-License-Identifier: GPL-2.0
+ */

+
+#include <efi.h>
+#include <efilib.h>

+#include "loader.h"
+
+static EFI_STATUS
+opened_file_size (EFI_FILE_HANDLE file_handle, UINT64 *size) {
+ UINT64 position;
+ EFI_STATUS status;
+
+ /* Get current position so we can restore it (on success only) */
+ status = uefi_call_wrapper(file_handle->GetPosition, 2, file_handle, &position);

+ if (EFI_ERROR(status)) {
+ return status;
+ }
+

+ /* Seek to the end of the file */
+ status = uefi_call_wrapper(file_handle->SetPosition, 2, file_handle, -1);

+ if (EFI_ERROR(status)) {
+ return status;
+ }
+

+ /* Get position at end of file */
+ status = uefi_call_wrapper(file_handle->GetPosition, 2, file_handle, size);

+ if (EFI_ERROR(status)) {
+ return status;
+ }
+

+ /* Restore original position */
+ status = uefi_call_wrapper(file_handle->SetPosition, 2, file_handle, position);

+ return status;
+}
+

+EFI_STATUS
+load_from_device_path (IN OUT EFI_DEVICE_PATH **device_path, OUT VOID **data, OUT UINTN *size)
+{
+ EFI_DEVICE_PATH *file_path;
+ MEMMAP_DEVICE_PATH *memory_path = NULL;
+ FILEPATH_DEVICE_PATH *node;
+ EFI_HANDLE device_handle;
+ EFI_FILE_HANDLE file_handle = NULL, last_handle;
+ UINT64 file_size;
+ void *file_data = NULL;
+ EFI_STATUS status;
+
+ file_path = *device_path;
+ status = uefi_call_wrapper(BS->LocateDevicePath, 3, &FileSystemProtocol, &file_path, &device_handle);
+ if (EFI_ERROR(status)) {
+ goto exit;
+ }
+
+ file_handle = LibOpenRoot (device_handle);
+ if (file_handle == NULL) {
+ /* Unable to open root device */
+ status = EFI_UNSUPPORTED;
+ goto exit;
+ }
+
+ /* Open each node from the device path until we get to the leaf node: our file */
+ node = (FILEPATH_DEVICE_PATH *)file_path;
+ while (!IsDevicePathEnd(&node->Header)) {
+ if (DevicePathType(&node->Header) != MEDIA_DEVICE_PATH || DevicePathSubType(&node->Header) != MEDIA_FILEPATH_DP) {
+ status = EFI_UNSUPPORTED;
+ goto close_file;
+ }
+
+ last_handle = file_handle;
+ file_handle = NULL;
+
+ status = uefi_call_wrapper(last_handle->Open, 5, last_handle, &file_handle, node->PathName, EFI_FILE_MODE_READ, 0);
+ if (EFI_ERROR(status)) {
+ goto exit;
+ }
+ uefi_call_wrapper(last_handle->Close, 1, last_handle);
+ node = (FILEPATH_DEVICE_PATH *) NextDevicePathNode(&node->Header);
+ }
+
+ /* Get size of the file to be loaded */
+ status = opened_file_size(file_handle, &file_size);
+ if (EFI_ERROR(status)) {
+ goto close_file;
+ }
+
+ /* Allocate memory to hold it in memory */
+ file_data = AllocatePool(file_size);
+ if (file_data == NULL) {
+ status = EFI_OUT_OF_RESOURCES;
+ goto close_file;
+ }
+
+ /* And now load it entirely */
+ status = uefi_call_wrapper(file_handle->Read, 3, file_handle, &file_size, file_data);
+ if (EFI_ERROR(status)) {
+ goto free_data;
+ }
+
+ memory_path = AllocatePool (2 * sizeof (MEMMAP_DEVICE_PATH));
+ if (memory_path == NULL) {
+ status = EFI_OUT_OF_RESOURCES;
+ goto free_data;
+ }
+
+ /* Make a memory map device path */
+ SetDevicePathNodeLength(&memory_path[0].Header, sizeof(*memory_path));
+ memory_path[0].Header.Type = HARDWARE_DEVICE_PATH;
+ memory_path[0].Header.SubType = HW_MEMMAP_DP;
+ memory_path[0].MemoryType = EfiLoaderData;
+ memory_path[0].StartingAddress = (EFI_PHYSICAL_ADDRESS) file_data;
+ memory_path[0].EndingAddress = memory_path[0].StartingAddress + file_size;
+
+ /* End list of device path elements with an "end" node */
+ SetDevicePathEndNode(&memory_path[1].Header);
+
+ /* Provide the address and size of the loaded file */
+ *data = file_data;
+ *size = (UINTN) file_size;
+
+ /* Provide new device path to caller */
+ FreePool(*device_path);
+ *device_path = (EFI_DEVICE_PATH *)memory_path;
+ goto close_file;
+
+free_data:
+ if (file_data != NULL) {
+ FreePool(file_data);
+ }
+close_file:
+ if (file_handle != NULL) {
+ uefi_call_wrapper(file_handle->Close, 1, file_handle);
+ }
+exit:
+ return status;
+}
diff --git a/main.c b/main.c
index 7949218..c47af74 100644
--- a/main.c
+++ b/main.c
@@ -20,6 +20,7 @@
#include <pci/header.h>
#include <bootguard.h>
#include <configuration.h>
+#include "loader.h"
#include "version.h"
#include "utils.h"

@@ -102,6 +103,8 @@ EFI_STATUS efi_main(EFI_HANDLE image_handle, EFI_SYSTEM_TABLE *system_table)
EFI_DEVICE_PATH *payload_dev_path;
EFI_LOADED_IMAGE *loaded_image;
EFI_HANDLE payload_handle;
+ VOID *payload_data;
+ UINTN payload_size;
EFI_STATUS status;
BG_STATUS bg_status;
BG_LOADER_PARAMS bg_loader_params;
@@ -169,11 +172,17 @@ EFI_STATUS efi_main(EFI_HANDLE image_handle, EFI_SYSTEM_TABLE *system_table)
}
}

+ /* Load specified kernel image to memory */
+ status = load_from_device_path (&payload_dev_path, &payload_data, &payload_size);
+ if (EFI_ERROR(status)) {
+ error_exit(L"Cannot load specified kernel image to memory", status);
+ }
+
/* Load and start image */
- status = uefi_call_wrapper(BS->LoadImage, 6, TRUE, this_image,
- payload_dev_path, NULL, 0, &payload_handle);
+ status = uefi_call_wrapper(BS->LoadImage, 6, FALSE, this_image,
+ payload_dev_path, payload_data, payload_size, &payload_handle);
if (EFI_ERROR(status)) {
- error_exit(L"Cannot load specified kernel image", status);
+ error_exit(L"Cannot load specified kernel image from memory", status);
}

FreePool(payload_dev_path);
--
2.30.2

[Cedric Hombourger]

Import systemd code (fac6511e49) which will be used to measure the
kernel image and options to selected PCRs.

Update SPDX license headers from LGPL 2.1 (or later) to GPL 2.1 as
permitted by the LGPL when importing LGPL code into a GPL application.

Signed-off-by: Cedric Hombourger <Cedric_H...@mentor.com>
---
Makefile.am | 3 +-

measure.c | 313 ++++++++++++++++++++++++++++++++++++++++++++++++++++
measure.h | 6 +
3 files changed, 321 insertions(+), 1 deletion(-)

create mode 100644 measure.c
create mode 100644 measure.h

diff --git a/Makefile.am b/Makefile.am
index fb907ff..8124d21 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -136,7 +136,8 @@ efi_sources = \
utils.c \
bootguard.c \
main.c \
- loader.c
+ loader.c \
+ measure.c

efi_cppflags = \
-I$(top_builddir) -include config.h \

diff --git a/measure.c b/measure.c
new file mode 100644
index 0000000..6c0c4df
--- /dev/null
+++ b/measure.c
@@ -0,0 +1,313 @@
+/* SPDX-License-Identifier: GPL-2.1-or-later */

+
+#include <efi.h>
+#include <efilib.h>

+ if (EFI_ERROR(status))
+ return status;
+

+ if (EFI_ERROR(status))
+ return status;
+

+ return EFI_SUCCESS;
+}
+
+static EFI_TCG * tcg1_interface_check(void) {
+ EFI_STATUS status;
+ EFI_TCG *tcg;
+ TCG_BOOT_SERVICE_CAPABILITY capability;
+ UINT32 features;
+ EFI_PHYSICAL_ADDRESS event_log_location;
+ EFI_PHYSICAL_ADDRESS event_log_last_entry;
+
+ status = LibLocateProtocol(EFI_TCG_GUID, (void **) &tcg);
+

+ if (EFI_ERROR(status))

+ return NULL;
+
+ capability.Size = (UINT8) sizeof(capability);
+ status = uefi_call_wrapper(tcg->StatusCheck, 5, tcg, &capability, &features, &event_log_location, &event_log_last_entry);
+

+ if (EFI_ERROR(status))

+ return NULL;
+
+ if (capability.TPMDeactivatedFlag)
+ return NULL;
+
+ if (!capability.TPMPresentFlag)
+ return NULL;
+
+ return tcg;
+}
+
+static EFI_TCG2 * tcg2_interface_check(void) {
+ EFI_STATUS status;
+ EFI_TCG2 *tcg;
+ EFI_TCG2_BOOT_SERVICE_CAPABILITY capability;
+
+ status = LibLocateProtocol(EFI_TCG2_GUID, (void **) &tcg);
+

+ if (EFI_ERROR(status))

+ return NULL;
+
+ capability.Size = (UINT8) sizeof(EFI_TCG2_BOOT_SERVICE_CAPABILITY);
+ status = uefi_call_wrapper(tcg->GetCapability, 2, tcg, &capability);
+

+ if (EFI_ERROR(status))

diff --git a/measure.h b/measure.h
new file mode 100644

index 0000000..65c7a9d
--- /dev/null
+++ b/measure.h
@@ -0,0 +1,6 @@
+/* SPDX-License-Identifier: GPL-2.1-or-later */

+#pragma once
+
+#include <efi.h>

[Cedric Hombourger]

Use systemd code introduced with 0c4aa25 to measure the kernel image
into PCR[8] and options in PCR[9].

Signed-off-by: Cedric Hombourger <Cedric_H...@mentor.com>
---

configure.ac | 18 ++++++++++++++++++
include/loader.h | 2 +-
loader.c | 11 ++++++++++-
main.c | 12 +++++++++++-
4 files changed, 40 insertions(+), 3 deletions(-)

diff --git a/configure.ac b/configure.ac
index b8d2b1c..6440ef2 100644
--- a/configure.ac
+++ b/configure.ac
@@ -77,6 +77,24 @@ AC_ARG_WITH([gnuefi-lib-dir],

AC_SUBST(GNUEFI_LIB_DIR)
AC_DEFINE_UNQUOTED(GNUEFI_LIB_DIR, ["$GNUEFI_LIB_DIR"], [The lib directory for gnuefi])

+AC_ARG_WITH([kernel-pcr],
+ AS_HELP_STRING([--with-kernel-pcr=NUMBER],
+ [specify the PCR register to use for the kernel image, defaults to 8]),
+ [KERNEL_PCR="$withval"],
+ [KERNEL_PCR="8"])
+
+AC_SUBST(KERNEL_PCR)
+AC_DEFINE_UNQUOTED(KERNEL_PCR, [$KERNEL_PCR], [The PCR register for the kernel image])

+
+AC_ARG_WITH([options-pcr],
+ AS_HELP_STRING([--with-options-pcr=NUMBER],

+ [specify the PCR register to use for options, defaults to 9]),
+ [OPTIONS_PCR="$withval"],
+ [OPTIONS_PCR="9"])

+
+AC_SUBST(OPTIONS_PCR)
+AC_DEFINE_UNQUOTED(OPTIONS_PCR, [$OPTIONS_PCR], [The PCR register for options])
+
dnl Define ARCH_<NAME> conditionals
SET_ARCH(I586, i586*)
SET_ARCH(I686, i686*)

diff --git a/include/loader.h b/include/loader.h
index 5cf639e..5ccb9ba 100644
--- a/include/loader.h
+++ b/include/loader.h
@@ -18,4 +18,4 @@
#include <efilib.h>

EFI_STATUS
-load_from_device_path (IN OUT EFI_DEVICE_PATH **device_path, OUT VOID **data, OUT UINTN *size);
+load_from_device_path (IN OUT EFI_DEVICE_PATH **device_path, IN UINTN pcr, OUT VOID **data, OUT UINTN *size);
diff --git a/loader.c b/loader.c
index 6029781..69b2570 100644
--- a/loader.c
+++ b/loader.c
@@ -15,6 +15,7 @@
#include <efi.h>
#include <efilib.h>
#include "loader.h"
+#include "measure.h"

static EFI_STATUS
opened_file_size (EFI_FILE_HANDLE file_handle, UINT64 *size) {
@@ -45,7 +46,7 @@ opened_file_size (EFI_FILE_HANDLE file_handle, UINT64 *size) {
}

EFI_STATUS
-load_from_device_path (IN OUT EFI_DEVICE_PATH **device_path, OUT VOID **data, OUT UINTN *size)
+load_from_device_path (IN OUT EFI_DEVICE_PATH **device_path, UINTN pcr, OUT VOID **data, OUT UINTN *size)
{
EFI_DEVICE_PATH *file_path;
MEMMAP_DEVICE_PATH *memory_path = NULL;
@@ -107,6 +108,14 @@ load_from_device_path (IN OUT EFI_DEVICE_PATH **device_path, OUT VOID **data, OU
goto free_data;
}

+ /* Measure the loaded binary */
+ if (pcr > 0) {
+ status = tpm_log_event(pcr, (EFI_PHYSICAL_ADDRESS) (UINTN) file_data, file_size, file_data);

+ if (EFI_ERROR(status)) {
+ goto free_data;
+ }
+ }
+
memory_path = AllocatePool (2 * sizeof (MEMMAP_DEVICE_PATH));

if (memory_path == NULL) {
status = EFI_OUT_OF_RESOURCES;
diff --git a/main.c b/main.c
index c47af74..b5ab5c2 100644
--- a/main.c
+++ b/main.c
@@ -23,6 +23,7 @@

#include "loader.h"
#include "version.h"
#include "utils.h"

+#include "measure.h"

extern const unsigned long init_array_start[];
extern const unsigned long init_array_end[];

@@ -173,7 +174,7 @@ EFI_STATUS efi_main(EFI_HANDLE image_handle, EFI_SYSTEM_TABLE *system_table)

}

/* Load specified kernel image to memory */

- status = load_from_device_path (&payload_dev_path, &payload_data, &payload_size);
+ status = load_from_device_path (&payload_dev_path, KERNEL_PCR, &payload_data, &payload_size);
if (EFI_ERROR(status)) {

error_exit(L"Cannot load specified kernel image to memory", status);
}

@@ -201,6 +202,15 @@ EFI_STATUS efi_main(EFI_HANDLE image_handle, EFI_SYSTEM_TABLE *system_table)
loaded_image->LoadOptionsSize =
(StrLen(bg_loader_params.payload_options) + 1) * sizeof(CHAR16);

+ /* Try to log any options to the TPM */
+ status = tpm_log_event(OPTIONS_PCR,
+ (EFI_PHYSICAL_ADDRESS) (UINTN) loaded_image->LoadOptions,
+ loaded_image->LoadOptionsSize, loaded_image->LoadOptions);
+ /* Try to log any options to the TPM, especially manually edited options */
+ if (EFI_ERROR(status)) {
+ WARNING(L"Could not measure options (%x)\n", status);
+ }

[Jan Kiszka]

On 04.07.21 11:03, Cedric Hombourger wrote:
> Our concept is twofold: (1) use SecureBoot to make sure we are booted
> from a trusted boot-loader + trusted Linux kernel (both are signed with
> keys loaded into the trust store) (2) extend the root of trust by
> mounting a LUKS encrypted volume with the key in the TPM and sealed with
> selected PCRs. In an A/B scheme, we may have the same encryption key
> placed both in slot[A] and slot[B] but sealed as follows:

I do get the point of using measurements to keep the platform open for
non-secure boots but protect the secretes to define boot chains. But I
do not get the use case of booting only well-defined secure images and
then additionally measuring them on top to unlock secrets. What's the
added value? The downside is the need to maintain the measuring values
in addition, right?

[Cedric Hombourger]

Our concept is kind of similar to Microsoft's BitLocker

They use SecureBoot to make sure you only boot something they have signed

They use MeasuredBoot and place all measurements of their boot
components into PCR[7] where the BitLocker key is sealed against PCR[7]
(they don't seem to bother tracking which boot component was tampered
since a single PCR is used while they most likely have multiple boot
components)

See
https://docs.microsoft.com/en-us/windows-hardware/test/hlk/testref/trusted-execution-environment-efi-protocol
and
https://docs.microsoft.com/en-us/windows-hardware/design/device-experiences/oem-bitlocker

In their case, they seem to suspend BitLocker when they apply firmware
updates

In our case, we have an A/B scheme and use different non-volatile slots
in the TPM for each system (A/B) to store our encryption key. We have
them sealed against PCR measurements of their respective boot components.

Once we have our LUKS volumes mounted, we also poisoned PCRs that were
used to seal our secret

Does that help?

We can certainly have a discussion if more clarifications are needed or
if you feel we could achieve something somewhat similar by some other means

Cedric

>
> Jan
>

[Christian Storm]

Hi Cedric,

> [...] Here are the measurements I collected from the

> TPM event log
>
> /* efibootguard measured in PCR[4] */
> PCR 4 -- Event 80000003 <TpmEventType.EFI_BOOT_SERVICES_APPLICATION>
> Extend (SHA1): 3877b5f89f59bc5d934ba4b26672fea0ffb0806c
> Path vector:
>   * ACPIDevice           ACPI b'\xd0A\x03\n\x00\x00\x00\x00'
>   * HardwareDevice       PCI                  b'\x00\x10'
>   * MessagingDevice      SCSI                 b'\x00\x00\x00\x00'
>   * MediaDevice          HardDrive b'\x01\x00\x00\x00\x00\x08\x00\x00\x00\x00\x00\x00\x00\x00\x01\x00\x00\x00\x00\x00\\&\x0b\xa9\xa7\xee\xf1C\xb4".\xd3M\xf6\xe6Y\x02\x02'
>   * MediaDevice          FilePath \EFI\BOOT\BOOTX64.EFI
>   * End                  255                  b''
>
> /* our measurement of the specified kernel image in PCR[8]
>  * note: this is a raw measurement (i.e. SHA1 of the entire file without
> interpreting e.g PE/COFF sections)
>  */
> PCR 8 -- Event 0000000D <TpmEventType.IPL>

This event type is deprecated, see, e.g., "TCG PC Client Platform
Firmware Profile Specification", Family "2.0", Level 00 Revision 00.49
from 2017 or any later revision.

> Extend (SHA1): e42ee38fd3cf1f7d310f00e4331628b0a188ef3a
> 0x00000000: 4D 5A 90 00 03 00 00 |MZ.....         |
> -- not touching non-systemd IPL event --
> --> after this event, PCR 8 contains value
> 58add887d84dd4be97a32a6ba6cb5535f84a7663
> --> after reboot, PCR 8 will contain value
> 58add887d84dd4be97a32a6ba6cb5535f84a7663
>
>
> /* LoadImage() still measures our kernel image in PCR[4]
>  * note: EFI knows it is a PE/COFF binary and only relevant sections are
> hashed
>  */
> PCR 4 -- Event 80000003 <TpmEventType.EFI_BOOT_SERVICES_APPLICATION>
> Extend (SHA1): a9823f0033e9cb2c30019b4bab7a9ee31071debf
> Path vector:
>   * HardwareDevice       MemoryMapped b'\x02\x00\x00\x00\x18\xc0\x10\x0b\x00\x00\x00\x00\x10\xcd\xbd\r\x00\x00\x00\x00'
>   * End                  255                  b''
>
> /* our measurement of the kernel command line into PCR[9] */

Why did you chose PCR #9 here? According to my shared list of who
measures to what register(s), there's at least systemd-boot and
rhboot/grub2 measuring Strings to PCR #8. Any reason to deviate?

If the (unified) kernel (image) has booted and you're in initramfs, then
you've booted an eligible kernel ― otherwise you have more severe
problems such as compromised keys. What you're doing here is tying the
kernel to the firmware/hardware so that only this specific kernel (out
of a set of binary-different but properly signed and eligible kernels)
can be booted. Is this by intention?

Next is to find out whether the root filesystem is eligible as well,
i.e., assert the root filesystem integrity. This you infer from it being
unlocked. You could've used dm-verity with a kernel image embedded root
hash for this as well.

So, your use case is entirely solvable without involving measurement or
the TPM at all.

> > > > That said, what's the overarching concept behind this? Just measuring
> > > > "something" is not an end in itself. How is that supposed to be sensibly
> > > > used (e.g., by the OS) to actually leverage the measurements? According
> > > > to this answer, the measurement(s) should be performed by EFI Boot Guard
> > > > to support the intended idea/concept.
> > > We are wanting to get boot artifacts measured so we can locally attest that we
> > > have booted into a well-known system and may safely unseal a secret sealed
> > > into the TPM. Our secret would be sealed with at least PCR[4] and PCR[8]. In
> > > our case, we don't really need to say which element was tampered, we just want
> > > to prevent the bad guy from reading our secret.
> > OK, but that ties you to this particular kernel binary (PCR#4) and this
> > particular cmdline (PCR#8). If you now change something, think firmware
> > update, you have to re-seal the secret which is a delicate operation to
> > perform in the context of a firmware update transaction...
> or seal the same encryption key using a different slot in your TPM so you
> don't change anything in your current configuration. If something in your new
> system wasn't done right, all pieces to revert to the old system should still
> be in place

This would require logics to detect which one to actually use to unseal
the secret and at least two slots in the TPM, or just trial and error.
Still, you have to seal the new secret while a firmware update transaction.
This way demands that you can under all circumstances rule out any side
effects to the current boot path in case something goes wrong.

> > That said, if you have Secure Boot enabled and use unified kernel
> > images, what does measuring the cmdline to PCR#8 give you in addition to
> > just sealing the secret with ― probably among others ― PCR#4 and thereby
> > bind it to (at least) the unified kernel image ― which includes the
> > cmdline as PE/COFF section?
>
> in my experiments, I was able to override the kernel command line with
> bg_setenv -a. this is probably a "feature" of systemd's stub (maybe this would
> not happen when directly building the kernel as an EFI binary but I am
> guessing that efibootguard should not assume a specific unified kernel image
> implementation?)

It doesn't. EFI Boot Guard supplies the command line from its
environment to the callee, unconditionally. The callee can then decide
whether to honor it or not. EFI Boot Guard is also not concerned with
the callee's format or shape as long as it's a valid PE/COFF UEFI binary
that passes Secure Boot checks if that's enabled. After all, it just
uses standard UEFI mechanisms to load the callee and start it. So, you
can also load other OSs or the EFI Shell or whatever as EFI Boot Guard's
callee. It just has to be a valid PE/COFF UEFI binary and there is
nothing else imposed on it.

The Linux kernel itself has an EFI stub loader ― enabled if compiled for
UEFI as target ― that makes it a valid PE/COFF UEFI binary. Unified
Kernel Images are just needed to pack everything in one fat binary to
allow Secure Boot checks for the stuff typically not included in the
Kernel binary itself, read: command line, splash, initramfs, ... You can
bake those into the Kernel itself at build-time but that would demand
a rebuild of, e.g., a distro's kernel to do so and that is precisely
why systemd has stub.c: to allow for simple repackaging instead of
rebuilding.

Unified Kernel Images built with systemd's stub loader only accept
external, read command line options *not* from its PE's built-in
.cmdline section, if Secure Boot is not enabled and there are actually
external while not internal command line options, see:
https://github.com/systemd/systemd/blob/main/src/boot/efi/stub.c#L58

So you may double-check whether you've done your experiments with
Secure Boot enabled or not and whether you have a baked in .cmdline
section or not...

[Christian Storm]

Hi Cedric,

> > > Our concept is twofold: (1) use SecureBoot to make sure we are booted
> > > from a trusted boot-loader + trusted Linux kernel (both are signed with
> > > keys loaded into the trust store) (2) extend the root of trust by
> > > mounting a LUKS encrypted volume with the key in the TPM and sealed with
> > > selected PCRs. In an A/B scheme, we may have the same encryption key
> > > placed both in slot[A] and slot[B] but sealed as follows:
> > I do get the point of using measurements to keep the platform open for
> > non-secure boots but protect the secretes to define boot chains. But I
> > do not get the use case of booting only well-defined secure images and
> > then additionally measuring them on top to unlock secrets. What's the
> > added value? The downside is the need to maintain the measuring values
> > in addition, right?
>
> Our concept is kind of similar to Microsoft's BitLocker

Note that this is however primarily targeted to machines having an HMI
where a human operator can jump in and interactively click on buttons to
recover the machine in case of failure. This is probably not what one
can expect from (deeply) embedded systems having more qualified
robustness requirements... Either way, the concept needs to match the
intended use case.

> They use SecureBoot to make sure you only boot something they have signed

... which is the same purpose we're using Secure Boot for, so far so aligned :)

> They use MeasuredBoot and place all measurements of their boot components into
> PCR[7] where the BitLocker key is sealed against PCR[7]
> (they don't seem to bother tracking which boot component was tampered since a
> single PCR is used while they most likely have multiple boot components)

They are then apparently anchoring it all to Secure Boot and Secure Boot's PCR #7:
If Secure Boot (or related measurements) gets compromised / changed (even
by intention), then no disk unlock will happen and hence no Windows OS
would be booted ― until the re-seal disk unlock secret operation, that is.

> See https://docs.microsoft.com/en-us/windows-hardware/test/hlk/testref/trusted-execution-environment-efi-protocol
> and https://docs.microsoft.com/en-us/windows-hardware/design/device-experiences/oem-bitlocker

Quote: "Suspend BitLocker (required for devices bound to PCR[07] only
if the firmware update changes the Secure Boot policy)."

> In their case, they seem to suspend BitLocker when they apply firmware updates

They consequently have to since any change related to Secure Boot or its
related measurements such as, e.g., a Windows Bootloader or Kernel
update requires a re-sealing ― either prior to rebooting or after the
fact by human consent. Otherwise, the machine naturally doesn't boot.

> In our case, we have an A/B scheme and use different non-volatile slots in the
> TPM for each system (A/B) to store our encryption key. We have them sealed
> against PCR measurements of their respective boot components.

Which means you have to update them while any firmware update
transaction prior to the due reboot into the new firmware plus
handling any rollback logics in the initramfs, right?
It's a direct port of Microsoft's BitLocker to an A/B deployment,
isn't it?

> Does that help?

Yes, but I'm with Jan here, in particular regarding maintaining the
measuring values, quoting:

> > But I do not get the use case of booting only well-defined secure
> > images and then additionally measuring them on top to unlock
> > secrets. What's the added value? The downside is the need to
> > maintain the measuring values in addition, right?