From 90dacf59d0f5270f345a9079f3a27df614b8e17c Mon Sep 17 00:00:00 2001 From: Robbie Harwood Date: Fri, 18 Mar 2022 18:33:12 +0000 Subject: [PATCH] Don't verify kernels twice Signed-off-by: Robbie Harwood --- ...-i386-efi-linux.c-do-not-validate-ke.patch | 73 ++++++++++++++++ ...-arm64-linux.c-do-not-validate-kerne.patch | 58 +++++++++++++ ...-efi-chainloader.c-do-not-validate-c.patch | 80 ++++++++++++++++++ ...-efi-linux.c-drop-now-unused-grub_li.patch | 83 +++++++++++++++++++ grub.patches | 4 + grub2.spec | 5 +- 6 files changed, 302 insertions(+), 1 deletion(-) create mode 100644 0213-grub-core-loader-i386-efi-linux.c-do-not-validate-ke.patch create mode 100644 0214-grub-core-loader-arm64-linux.c-do-not-validate-kerne.patch create mode 100644 0215-grub-core-loader-efi-chainloader.c-do-not-validate-c.patch create mode 100644 0216-grub-core-loader-efi-linux.c-drop-now-unused-grub_li.patch diff --git a/0213-grub-core-loader-i386-efi-linux.c-do-not-validate-ke.patch b/0213-grub-core-loader-i386-efi-linux.c-do-not-validate-ke.patch new file mode 100644 index 0000000..e4a8522 --- /dev/null +++ b/0213-grub-core-loader-i386-efi-linux.c-do-not-validate-ke.patch @@ -0,0 +1,73 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Dimitri John Ledkov +Date: Thu, 3 Mar 2022 13:10:56 +0100 +Subject: [PATCH] grub-core/loader/i386/efi/linux.c: do not validate kernels + twice + +On codebases that have shim-lock-verifier built into the grub core +(like 2.06 upstream), shim-lock-verifier is in enforcing mode when +booted with secureboot. It means that grub_cmd_linux() command +attempts to perform shim validate upon opening linux kernel image, +including kernel measurement. And the verifier correctly returns file +open error when shim validate protocol is not present or shim fails to +validate the kernel. + +This makes the call to grub_linuxefi_secure_validate() redundant, but +also harmful. As validating the kernel image twice, extends the PCRs +with the same measurement twice. Which breaks existing sealing +policies when upgrading from grub2.04+rhboot+sb+linuxefi to +grub2.06+rhboot+sb+linuxefi builds. It is also incorrect to measure +the kernel twice. + +This patch must not be ported to older editions of grub code bases +that do not have verifiers framework, or it is not builtin, or +shim-lock-verifier is an optional module. + +This patch is tested to ensure that unsigned kernels are not possible +to boot in secureboot mode when shim rejects kernel, or shim protocol +is missing, and that the measurements become stable once again. The +above also ensures that CVE-2020-15705 is not reintroduced. + +Signed-off-by: Dimitri John Ledkov +--- + grub-core/loader/i386/efi/linux.c | 13 ------------- + 1 file changed, 13 deletions(-) + +diff --git a/grub-core/loader/i386/efi/linux.c b/grub-core/loader/i386/efi/linux.c +index 3cf0f9b330b..941df6400b9 100644 +--- a/grub-core/loader/i386/efi/linux.c ++++ b/grub-core/loader/i386/efi/linux.c +@@ -30,7 +30,6 @@ + #include + #include + #include +-#include + + GRUB_MOD_LICENSE ("GPLv3+"); + +@@ -278,7 +277,6 @@ grub_cmd_linux (grub_command_t cmd __attribute__ ((unused)), + grub_ssize_t start, filelen; + void *kernel = NULL; + int setup_header_end_offset; +- int rc; + + grub_dl_ref (my_mod); + +@@ -308,17 +306,6 @@ grub_cmd_linux (grub_command_t cmd __attribute__ ((unused)), + goto fail; + } + +- if (grub_efi_get_secureboot () == GRUB_EFI_SECUREBOOT_MODE_ENABLED) +- { +- rc = grub_linuxefi_secure_validate (kernel, filelen); +- if (rc <= 0) +- { +- grub_error (GRUB_ERR_INVALID_COMMAND, +- N_("%s has invalid signature"), argv[0]); +- goto fail; +- } +- } +- + lh = (struct linux_i386_kernel_header *)kernel; + grub_dprintf ("linux", "original lh is at %p\n", kernel); + diff --git a/0214-grub-core-loader-arm64-linux.c-do-not-validate-kerne.patch b/0214-grub-core-loader-arm64-linux.c-do-not-validate-kerne.patch new file mode 100644 index 0000000..9fb39a6 --- /dev/null +++ b/0214-grub-core-loader-arm64-linux.c-do-not-validate-kerne.patch @@ -0,0 +1,58 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Dimitri John Ledkov +Date: Fri, 4 Mar 2022 11:29:31 +0100 +Subject: [PATCH] grub-core/loader/arm64/linux.c: do not validate kernel twice + +Call to grub_file_open(, GRUB_FILE_TYPE_LINUX_KERNEL) already passes +the kernel file through shim-lock verifier when secureboot is on. Thus +there is no need to validate the kernel image again. And when doing so +again, duplicate PCR measurement is performed, breaking measurements +compatibility with 2.04+linuxefi. + +This patch must not be ported to older editions of grub code bases +that do not have verifiers framework, or it is not builtin, or +shim-lock-verifier is an optional module. + +Signed-off-by: Dimitri John Ledkov +--- + grub-core/loader/arm64/linux.c | 13 ------------- + 1 file changed, 13 deletions(-) + +diff --git a/grub-core/loader/arm64/linux.c b/grub-core/loader/arm64/linux.c +index f18d90bd749..d2af47c2c0a 100644 +--- a/grub-core/loader/arm64/linux.c ++++ b/grub-core/loader/arm64/linux.c +@@ -34,7 +34,6 @@ + #include + #include + #include +-#include + + GRUB_MOD_LICENSE ("GPLv3+"); + +@@ -341,7 +340,6 @@ grub_cmd_linux (grub_command_t cmd __attribute__ ((unused)), + grub_off_t filelen; + grub_uint32_t align; + void *kernel = NULL; +- int rc; + + grub_dl_ref (my_mod); + +@@ -370,17 +368,6 @@ grub_cmd_linux (grub_command_t cmd __attribute__ ((unused)), + goto fail; + } + +- if (grub_efi_get_secureboot () == GRUB_EFI_SECUREBOOT_MODE_ENABLED) +- { +- rc = grub_linuxefi_secure_validate (kernel, filelen); +- if (rc <= 0) +- { +- grub_error (GRUB_ERR_INVALID_COMMAND, +- N_("%s has invalid signature"), argv[0]); +- goto fail; +- } +- } +- + if (grub_arch_efi_linux_check_image (kernel) != GRUB_ERR_NONE) + goto fail; + if (parse_pe_header (kernel, &kernel_size, &handover_offset, &align) != GRUB_ERR_NONE) diff --git a/0215-grub-core-loader-efi-chainloader.c-do-not-validate-c.patch b/0215-grub-core-loader-efi-chainloader.c-do-not-validate-c.patch new file mode 100644 index 0000000..ad34309 --- /dev/null +++ b/0215-grub-core-loader-efi-chainloader.c-do-not-validate-c.patch @@ -0,0 +1,80 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Dimitri John Ledkov +Date: Fri, 4 Mar 2022 09:31:43 +0100 +Subject: [PATCH] grub-core/loader/efi/chainloader.c: do not validate + chainloader twice + +On secureboot systems, with shimlock verifier, call to +grub_file_open(, GRUB_FILE_TYPE_EFI_CHAINLOADED_IMAGE) will already +pass the chainloader target through shim-lock protocol verify +call. And create a TPM measurement. If verification fails, +grub_cmd_chainloader will fail at file open time. + +This makes previous code paths for negative, and zero return codes +from grub_linuxefi_secure_validate unreachable under secureboot. But +also breaking measurements compatibility with 2.04+linuxefi codebases, +as the chainloader file is passed through shim_lock->verify() twice +(via verifier & direct call to grub_linuxefi_secure_validate) +extending the PCRs twice. + +This reduces grub_loader options to perform +grub_secureboot_chainloader when secureboot is on, and otherwise +attempt grub_chainloader_boot. + +It means that booting with secureboot off, yet still with shim (which +always verifies things successfully), will stop choosing +grub_secureboot_chainloader, and opting for a more regular +loadimage/startimage codepath. If we want to use the +grub_secureboot_chainloader codepath in such scenarios we should adapt +the code to simply check for shim_lock protocol presence / +shim_lock->context() success?! But I am not sure if that is necessary. + +This patch must not be ported to older editions of grub code bases +that do not have verifiers framework, or it is not builtin, or +shim-lock-verifier is an optional module. + +Signed-off-by: Dimitri John Ledkov +--- + grub-core/loader/efi/chainloader.c | 8 ++------ + 1 file changed, 2 insertions(+), 6 deletions(-) + +diff --git a/grub-core/loader/efi/chainloader.c b/grub-core/loader/efi/chainloader.c +index 3af6b122926..644cd2e56fe 100644 +--- a/grub-core/loader/efi/chainloader.c ++++ b/grub-core/loader/efi/chainloader.c +@@ -906,7 +906,6 @@ grub_cmd_chainloader (grub_command_t cmd __attribute__ ((unused)), + grub_efi_device_path_t *dp = 0; + char *filename; + void *boot_image = 0; +- int rc; + + if (argc == 0) + return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("filename expected")); +@@ -1082,9 +1081,7 @@ grub_cmd_chainloader (grub_command_t cmd __attribute__ ((unused)), + orig_dev = 0; + } + +- rc = grub_linuxefi_secure_validate((void *)(unsigned long)address, fsize); +- grub_dprintf ("chain", "linuxefi_secure_validate: %d\n", rc); +- if (rc > 0) ++ if (grub_efi_get_secureboot () == GRUB_EFI_SECUREBOOT_MODE_ENABLED) + { + grub_file_close (file); + grub_device_close (dev); +@@ -1092,7 +1089,7 @@ grub_cmd_chainloader (grub_command_t cmd __attribute__ ((unused)), + grub_secureboot_chainloader_unload, 0); + return 0; + } +- else if (rc == 0) ++ else + { + grub_load_and_start_image(boot_image); + grub_file_close (file); +@@ -1101,7 +1098,6 @@ grub_cmd_chainloader (grub_command_t cmd __attribute__ ((unused)), + + return 0; + } +- // -1 fall-through to fail + + fail: + if (orig_dev) diff --git a/0216-grub-core-loader-efi-linux.c-drop-now-unused-grub_li.patch b/0216-grub-core-loader-efi-linux.c-drop-now-unused-grub_li.patch new file mode 100644 index 0000000..46eafc0 --- /dev/null +++ b/0216-grub-core-loader-efi-linux.c-drop-now-unused-grub_li.patch @@ -0,0 +1,83 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Dimitri John Ledkov +Date: Fri, 4 Mar 2022 11:36:09 +0100 +Subject: [PATCH] grub-core/loader/efi/linux.c: drop now unused + grub_linuxefi_secure_validate + +Drop the now unused grub_linuxefi_secure_validate() as all prior users +of this API now rely on the shim-lock-verifier codepath instead. + +This patch must not be ported to older editions of grub code bases +that do not have verifiers framework, or it is not builtin, or +shim-lock-verifier is an optional module. + +Signed-off-by: Dimitri John Ledkov +--- + grub-core/loader/efi/linux.c | 40 ---------------------------------------- + include/grub/efi/linux.h | 2 -- + 2 files changed, 42 deletions(-) + +diff --git a/grub-core/loader/efi/linux.c b/grub-core/loader/efi/linux.c +index 9260731c107..9265cf4200a 100644 +--- a/grub-core/loader/efi/linux.c ++++ b/grub-core/loader/efi/linux.c +@@ -24,46 +24,6 @@ + #include + #include + +-#define SHIM_LOCK_GUID \ +- { 0x605dab50, 0xe046, 0x4300, {0xab, 0xb6, 0x3d, 0xd8, 0x10, 0xdd, 0x8b, 0x23} } +- +-struct grub_efi_shim_lock +-{ +- grub_efi_status_t (*verify) (void *buffer, grub_uint32_t size); +-}; +-typedef struct grub_efi_shim_lock grub_efi_shim_lock_t; +- +-// Returns 1 on success, -1 on error, 0 when not available +-int +-grub_linuxefi_secure_validate (void *data, grub_uint32_t size) +-{ +- grub_efi_guid_t guid = SHIM_LOCK_GUID; +- grub_efi_shim_lock_t *shim_lock; +- grub_efi_status_t status; +- +- shim_lock = grub_efi_locate_protocol(&guid, NULL); +- grub_dprintf ("secureboot", "shim_lock: %p\n", shim_lock); +- if (!shim_lock) +- { +- grub_dprintf ("secureboot", "shim not available\n"); +- return 0; +- } +- +- grub_dprintf ("secureboot", "Asking shim to verify kernel signature\n"); +- status = shim_lock->verify (data, size); +- grub_dprintf ("secureboot", "shim_lock->verify(): %ld\n", (long int)status); +- if (status == GRUB_EFI_SUCCESS) +- { +- grub_dprintf ("secureboot", "Kernel signature verification passed\n"); +- return 1; +- } +- +- grub_dprintf ("secureboot", "Kernel signature verification failed (0x%lx)\n", +- (unsigned long) status); +- +- return -1; +-} +- + #pragma GCC diagnostic push + #pragma GCC diagnostic ignored "-Wcast-align" + +diff --git a/include/grub/efi/linux.h b/include/grub/efi/linux.h +index 0033d9305a9..887b02fd9f3 100644 +--- a/include/grub/efi/linux.h ++++ b/include/grub/efi/linux.h +@@ -22,8 +22,6 @@ + #include + #include + +-int +-EXPORT_FUNC(grub_linuxefi_secure_validate) (void *data, grub_uint32_t size); + grub_err_t + EXPORT_FUNC(grub_efi_linux_boot) (void *kernel_address, grub_off_t offset, + void *kernel_param); diff --git a/grub.patches b/grub.patches index 2818569..29c5894 100644 --- a/grub.patches +++ b/grub.patches @@ -210,3 +210,7 @@ Patch0209: 0209-Update-gnulib-version-and-drop-most-gnulib-patches.patch Patch0210: 0210-commands-search-Fix-bug-stopping-iteration-when-no-f.patch Patch0211: 0211-search-new-efidisk-only-option-on-EFI-systems.patch Patch0212: 0212-efi-new-connectefi-command.patch +Patch0213: 0213-grub-core-loader-i386-efi-linux.c-do-not-validate-ke.patch +Patch0214: 0214-grub-core-loader-arm64-linux.c-do-not-validate-kerne.patch +Patch0215: 0215-grub-core-loader-efi-chainloader.c-do-not-validate-c.patch +Patch0216: 0216-grub-core-loader-efi-linux.c-drop-now-unused-grub_li.patch diff --git a/grub2.spec b/grub2.spec index f13f7c3..34b2ccc 100644 --- a/grub2.spec +++ b/grub2.spec @@ -17,7 +17,7 @@ Name: grub2 Epoch: 1 Version: 2.06 -Release: 22%{?dist} +Release: 23%{?dist} Summary: Bootloader with support for Linux, Multiboot and more License: GPLv3+ URL: http://www.gnu.org/software/grub/ @@ -526,6 +526,9 @@ mv ${EFI_HOME}/grub.cfg.stb ${EFI_HOME}/grub.cfg %endif %changelog +* Fri Mar 18 2022 Robbie Harwood - 2.06-23 +- Don't verify kernels twice + * Thu Mar 10 2022 Robbie Harwood - 2.06-22 - Skip updating BLS on ostree installations - Resolves: #2059776