From 4e169f6be09a5846e5ed2c2c0b8be55f504bb8dcf9b7a7440bf32f0e712d5b55 Mon Sep 17 00:00:00 2001 From: Gary Ching-Pang Lin Date: Fri, 21 Aug 2020 06:24:52 +0000 Subject: [PATCH 1/2] Accepting request 828385 from home:gary_lin:branches:devel:openSUSE:Factory fix the TPM2 measurement (bsc#1175509) OBS-URL: https://build.opensuse.org/request/show/828385 OBS-URL: https://build.opensuse.org/package/show/devel:openSUSE:Factory/shim?expand=0&rev=163 --- shim-bsc1175509-tpm2-fixes.patch | 205 +++++++++++++++++++++++++++++++ shim.changes | 6 + shim.spec | 3 + 3 files changed, 214 insertions(+) create mode 100644 shim-bsc1175509-tpm2-fixes.patch diff --git a/shim-bsc1175509-tpm2-fixes.patch b/shim-bsc1175509-tpm2-fixes.patch new file mode 100644 index 0000000..83d0ea9 --- /dev/null +++ b/shim-bsc1175509-tpm2-fixes.patch @@ -0,0 +1,205 @@ +From 551bab0a7c3199cad3bd1273d57e98e54bdf2ce9 Mon Sep 17 00:00:00 2001 +From: Matthew Garrett +Date: Tue, 11 Dec 2018 15:25:44 -0800 +Subject: [PATCH 1/4] Remove call to TPM2 get_event_log() + +Calling the TPM2 get_event_log causes the firmware to start logging +events to the final events table, but implementations may also continue +logging to the boot services event log. Any OS that wishes to +reconstruct the full PCR state must already look at both the final +events log and the boot services event log, so if this call is made +anywhere other than immediately before ExitBootServices() then the OS +must deduplicate events that occur in both, complicating things +immensely. + +Linux already has support for copying up the boot services event log +across the ExitBootServices() boundary, so there's no reason to make +this call. Remove it. + +Signed-off-by: Matthew Garrett +(cherry picked from commit fd7c3bd920ba39082cb7c619afb7203d150a4cd3) +--- + tpm.c | 46 ---------------------------------------------- + 1 file changed, 46 deletions(-) + +diff --git a/tpm.c b/tpm.c +index 674e69b..f07362c 100644 +--- a/tpm.c ++++ b/tpm.c +@@ -70,41 +70,6 @@ static BOOLEAN tpm2_present(EFI_TCG2_BOOT_SERVICE_CAPABILITY *caps, + return FALSE; + } + +-static inline EFI_TCG2_EVENT_LOG_BITMAP +-tpm2_get_supported_logs(efi_tpm2_protocol_t *tpm, +- EFI_TCG2_BOOT_SERVICE_CAPABILITY *caps, +- BOOLEAN old_caps) +-{ +- if (old_caps) +- return ((TREE_BOOT_SERVICE_CAPABILITY *)caps)->SupportedEventLogs; +- +- return caps->SupportedEventLogs; +-} +- +-/* +- * 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(efi_tpm2_protocol_t *tpm2, +- EFI_TCG2_EVENT_LOG_BITMAP supported_logs) +-{ +- EFI_TCG2_EVENT_LOG_FORMAT log_fmt; +- EFI_PHYSICAL_ADDRESS start; +- EFI_PHYSICAL_ADDRESS end; +- BOOLEAN truncated; +- +- 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->get_event_log(tpm2, log_fmt, &start, &end, &truncated); +-} +- + static EFI_STATUS tpm_locate_protocol(efi_tpm_protocol_t **tpm, + efi_tpm2_protocol_t **tpm2, + BOOLEAN *old_caps_p, +@@ -166,17 +131,6 @@ static EFI_STATUS tpm_log_event_raw(EFI_PHYSICAL_ADDRESS buf, UINTN size, + #endif + } else if (tpm2) { + EFI_TCG2_EVENT *event; +- EFI_TCG2_EVENT_LOG_BITMAP supported_logs; +- +- supported_logs = tpm2_get_supported_logs(tpm2, &caps, old_caps); +- +- efi_status = trigger_tcg2_final_events_table(tpm2, +- supported_logs); +- if (EFI_ERROR(efi_status)) { +- perror(L"Unable to trigger tcg2 final events table: %r\n", +- efi_status); +- return efi_status; +- } + + event = AllocatePool(sizeof(*event) + logsize); + if (!event) { +-- +2.28.0 + + +From 03cb410a51e808179e9d991057fb94a526ac269a Mon Sep 17 00:00:00 2001 +From: Chris Coulson +Date: Sat, 22 Jun 2019 15:33:03 +0100 +Subject: [PATCH 2/4] tpm: Fix off-by-one error when calculating event size + +tpm_log_event_raw() allocates a buffer for the EFI_TCG2_EVENT structure +that is one byte larger than necessary, and sets event->Size accordingly. +The result of this is that the event data recorded in the log differs +from the data that is measured to the TPM (it has an extra zero byte +at the end). + +(cherry picked from commit 8a27a4809a6a2b40fb6a4049071bf96d6ad71b50) +--- + tpm.c | 6 ++++-- + 1 file changed, 4 insertions(+), 2 deletions(-) + +diff --git a/tpm.c b/tpm.c +index f07362c..516fb87 100644 +--- a/tpm.c ++++ b/tpm.c +@@ -131,8 +131,10 @@ static EFI_STATUS tpm_log_event_raw(EFI_PHYSICAL_ADDRESS buf, UINTN size, + #endif + } else if (tpm2) { + EFI_TCG2_EVENT *event; ++ UINTN event_size = sizeof(*event) - sizeof(event->Event) + ++ logsize; + +- event = AllocatePool(sizeof(*event) + logsize); ++ event = AllocatePool(event_size); + if (!event) { + perror(L"Unable to allocate event structure\n"); + return EFI_OUT_OF_RESOURCES; +@@ -142,7 +144,7 @@ static EFI_STATUS tpm_log_event_raw(EFI_PHYSICAL_ADDRESS buf, UINTN size, + event->Header.HeaderVersion = 1; + event->Header.PCRIndex = pcr; + event->Header.EventType = type; +- event->Size = sizeof(*event) - sizeof(event->Event) + logsize + 1; ++ event->Size = event_size; + CopyMem(event->Event, (VOID *)log, logsize); + if (hash) { + /* TPM 2 systems will generate the appropriate hash +-- +2.28.0 + + +From 6b57ed99e1925728166017863ad849408cddf55d Mon Sep 17 00:00:00 2001 +From: Chris Coulson +Date: Sat, 22 Jun 2019 15:37:29 +0100 +Subject: [PATCH 3/4] tpm: Define EFI_VARIABLE_DATA_TREE as packed + +tpm_measure_variable() calculates VarLogSize by adding the size of VarName +and VarData to the size of EFI_VARIABLE_DATA_TREE, and then subtracting +the size of the UnicodeName and VariableData members. This results in a +calculation that is 5 bytes larger than necessary because it doesn't take +in to account the padding of these members. The effect of this is that +shim measures an additional 5 zero bytes when measuring UEFI variables +(at least on 64-bit architectures). + +Byte packing EFI_VARIABLE_DATA_TREE fixes this. + +(cherry picked from commit 7e4d3f1c8c730a5d3f40729cb285b5d8c7b241af) +--- + tpm.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/tpm.c b/tpm.c +index 516fb87..c0617bb 100644 +--- a/tpm.c ++++ b/tpm.c +@@ -233,7 +233,7 @@ typedef struct { + UINT64 VariableDataLength; + CHAR16 UnicodeName[1]; + INT8 VariableData[1]; +-} EFI_VARIABLE_DATA_TREE; ++} __attribute__ ((packed)) EFI_VARIABLE_DATA_TREE; + + static BOOLEAN tpm_data_measured(CHAR16 *VarName, EFI_GUID VendorGuid, UINTN VarSize, VOID *VarData) + { +-- +2.28.0 + + +From 85a8c568dde4d608a7c9cc5b0283bdc36e677947 Mon Sep 17 00:00:00 2001 +From: Chris Coulson +Date: Thu, 26 Sep 2019 20:01:01 +0100 +Subject: [PATCH 4/4] tpm: Don't log duplicate identical events + +According to the comment in tpm_measure_variable ("Don't measure something that we've already measured"), shim +shouldn't measure duplicate events if they are identical, which also aligns with section 2.3.4.8 of the TCG PC +Client Platform Firmware Profile Specification ("If it has been measured previously, it MUST NOT be measured +again"). This is currently broken because tpm_data_measured() uses the return value of CompareGuid() incorrectly. + +(cherry picked from commit 103adc89ce578a23cbdbd195c5dc5e329b85b854) +--- + tpm.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/tpm.c b/tpm.c +index c0617bb..196b93c 100644 +--- a/tpm.c ++++ b/tpm.c +@@ -241,7 +241,7 @@ static BOOLEAN tpm_data_measured(CHAR16 *VarName, EFI_GUID VendorGuid, UINTN Var + + for (i=0; i + +- Add shim-bsc1175509-tpm2-fixes.patch to fix the TPM2 measurement + (bsc#1175509) + ------------------------------------------------------------------- Thu Aug 6 09:43:19 UTC 2020 - Gary Ching-Pang Lin diff --git a/shim.spec b/shim.spec index 13f7ad1..cb62d05 100644 --- a/shim.spec +++ b/shim.spec @@ -79,6 +79,8 @@ Patch6: gcc9-fix-warnings.patch Patch7: shim-fix-gnu-efi-3.0.11.patch # PATCH-FIX-UPSTREAM shim-bsc1173411-only-check-efi-var-on-sb.patch bsc#1173411 glin@suse.com -- Make EFI variable copying check only fatal on SB systems Patch8: shim-bsc1173411-only-check-efi-var-on-sb.patch +# PATCH-FIX-UPSTREAM shim-bsc1175509-tpm2-fixes.patch bsc#1175509 glin@suse.com -- Upstream fixes for the TPM2 measurement +Patch9: shim-bsc1175509-tpm2-fixes.patch # PATCH-FIX-OPENSUSE shim-opensuse-cert-prompt.patch glin@suse.com -- Show the prompt to ask whether the user trusts openSUSE certificate or not Patch100: shim-opensuse-cert-prompt.patch BuildRequires: gnu-efi >= 3.0.3 @@ -128,6 +130,7 @@ The source code of UEFI shim loader %patch6 -p1 %patch7 -p1 %patch8 -p1 +%patch9 -p1 %if 0%{?is_opensuse} == 1 %patch100 -p1 %endif From d1e5e5e18a72d475a887bd9c0bf9749ad33f93a2f07dfb9ae37d55a30d494e0c Mon Sep 17 00:00:00 2001 From: Gary Ching-Pang Lin Date: Mon, 24 Aug 2020 08:28:08 +0000 Subject: [PATCH 2/2] Accepting request 828865 from home:gary_lin:branches:devel:openSUSE:Factory install MokManager to \EFI\boot to process the pending MOK request (bsc#1175626, bsc#1175656) OBS-URL: https://build.opensuse.org/request/show/828865 OBS-URL: https://build.opensuse.org/package/show/devel:openSUSE:Factory/shim?expand=0&rev=164 --- shim-install | 7 +++++++ shim.changes | 6 ++++++ 2 files changed, 13 insertions(+) diff --git a/shim-install b/shim-install index 212e5a8..85c494e 100644 --- a/shim-install +++ b/shim-install @@ -260,6 +260,8 @@ if test "$clean" = "yes"; then if test "$update_boot" = "yes"; then rm -f "${efibootdir}/${def_boot_efi}" rm -f "${efibootdir}/fallback.efi" + # bsc#1175626, bsc#1175656 also clean up MokManager + rm -f "${efibootdir}/MokManager.efi" fi if test "$no_nvram" = no && test -n "$bootloader_id"; then # Delete old entries from the same distributor. @@ -296,6 +298,11 @@ if test "$update_boot" = "yes"; then cp "${source_dir}/shim.efi" "${efibootdir}/${def_boot_efi}" if test "$removable" = "no"; then cp "${source_dir}/fallback.efi" "${efibootdir}" + # bsc#1175626, bsc#1175656 Since shim 15, loading MokManager becomes + # mandatory if a MOK request exists. Copy MokManager to \EFI\boot so + # that boot*.efi can load MokManager to process the request instead + # of shutting down the system immediately. + cp "${source_dir}/MokManager.efi" "${efibootdir}" fi fi diff --git a/shim.changes b/shim.changes index 176efb1..86eb3ff 100644 --- a/shim.changes +++ b/shim.changes @@ -1,3 +1,9 @@ +------------------------------------------------------------------- +Mon Aug 24 03:20:52 UTC 2020 - Gary Ching-Pang Lin + +- shim-install: install MokManager to \EFI\boot to process the + pending MOK request (bsc#1175626, bsc#1175656) + ------------------------------------------------------------------- Fri Aug 21 04:00:39 UTC 2020 - Gary Ching-Pang Lin