diff -Nru grub2-unsigned-2.06/debian/changelog grub2-unsigned-2.06/debian/changelog --- grub2-unsigned-2.06/debian/changelog 2022-03-02 10:32:51.000000000 +0000 +++ grub2-unsigned-2.06/debian/changelog 2022-03-15 13:11:13.000000000 +0000 @@ -1,9 +1,8 @@ -grub2-unsigned (2.06-2ubuntu9) jammy; urgency=medium +grub2-unsigned (2.06-2ubuntu10) jammy; urgency=medium - * Do not validate kernel file twice with shim - * and not when chainloading. + * Do not validate kernels twice. (LP: #1964943) - -- Dimitri John Ledkov Wed, 02 Mar 2022 11:32:51 +0100 + -- Dimitri John Ledkov Tue, 15 Mar 2022 13:11:13 +0000 grub2 (2.06-2ubuntu5) jammy; urgency=medium diff -Nru grub2-unsigned-2.06/debian/patches/0124-grub-core-loader-i386-efi-linux.c-do-not-validate-ke.patch grub2-unsigned-2.06/debian/patches/0124-grub-core-loader-i386-efi-linux.c-do-not-validate-ke.patch --- grub2-unsigned-2.06/debian/patches/0124-grub-core-loader-i386-efi-linux.c-do-not-validate-ke.patch 2022-03-02 10:32:51.000000000 +0000 +++ grub2-unsigned-2.06/debian/patches/0124-grub-core-loader-i386-efi-linux.c-do-not-validate-ke.patch 1970-01-01 00:00:00.000000000 +0000 @@ -1,71 +0,0 @@ -From: Dimitri John Ledkov -Date: Thu, 3 Mar 2022 13:10:56 +0100 -Subject: 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 6a90743..cbd313a 100644 ---- a/grub-core/loader/i386/efi/linux.c -+++ b/grub-core/loader/i386/efi/linux.c -@@ -27,7 +27,6 @@ - #include - #include - #include --#include - #include - - GRUB_MOD_LICENSE ("GPLv3+"); -@@ -170,7 +169,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); - -@@ -201,17 +199,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_ACCESS_DENIED, N_("%s has invalid signature"), -- argv[0]); -- goto fail; -- } -- } -- - params = grub_efi_allocate_pages_max (0x3fffffff, - BYTES_TO_PAGES(sizeof(*params))); - if (! params) diff -Nru grub2-unsigned-2.06/debian/patches/0125-grub-core-loader-efi-chainloader.c-do-not-validate-c.patch grub2-unsigned-2.06/debian/patches/0125-grub-core-loader-efi-chainloader.c-do-not-validate-c.patch --- grub2-unsigned-2.06/debian/patches/0125-grub-core-loader-efi-chainloader.c-do-not-validate-c.patch 2022-03-02 10:32:51.000000000 +0000 +++ grub2-unsigned-2.06/debian/patches/0125-grub-core-loader-efi-chainloader.c-do-not-validate-c.patch 1970-01-01 00:00:00.000000000 +0000 @@ -1,79 +0,0 @@ -From: Dimitri John Ledkov -Date: Fri, 4 Mar 2022 09:31:43 +0100 -Subject: 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 4a85f41..c6ae2fc 100644 ---- a/grub-core/loader/efi/chainloader.c -+++ b/grub-core/loader/efi/chainloader.c -@@ -910,7 +910,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")); -@@ -1080,9 +1079,7 @@ grub_cmd_chainloader (grub_command_t cmd __attribute__ ((unused)), - } - #endif - -- rc = grub_linuxefi_secure_validate((void *)((grub_addr_t) 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); - if (orig_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); -@@ -1103,7 +1100,6 @@ grub_cmd_chainloader (grub_command_t cmd __attribute__ ((unused)), - - return 0; - } -- // -1 fall-through to fail - - fail: - if (orig_dev) diff -Nru grub2-unsigned-2.06/debian/patches/linuxefi-do-not-validate-kernels-twice.patch grub2-unsigned-2.06/debian/patches/linuxefi-do-not-validate-kernels-twice.patch --- grub2-unsigned-2.06/debian/patches/linuxefi-do-not-validate-kernels-twice.patch 1970-01-01 00:00:00.000000000 +0000 +++ grub2-unsigned-2.06/debian/patches/linuxefi-do-not-validate-kernels-twice.patch 2022-03-15 13:00:37.000000000 +0000 @@ -0,0 +1,227 @@ +From: Dimitri John Ledkov +Date: Thu, 3 Mar 2022 13:10:56 +0100 +Subject: linuxefi: 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. + +This is a backport of https://github.com/rhboot/grub2/pull/97 onto +Ubuntu packaging. + +Signed-off-by: Dimitri John Ledkov +--- + grub-core/kern/arm/coreboot/coreboot.S | 2 -- + grub-core/loader/efi/chainloader.c | 8 ++----- + grub-core/loader/efi/linux.c | 12 ---------- + grub-core/loader/efi/linux_sb.c | 41 ---------------------------------- + grub-core/loader/i386/efi/linux.c | 13 ----------- + include/grub/efi/linux.h | 2 -- + 6 files changed, 2 insertions(+), 76 deletions(-) + +diff --git a/grub-core/kern/arm/coreboot/coreboot.S b/grub-core/kern/arm/coreboot/coreboot.S +index 70998c0..13e73ac 100644 +--- a/grub-core/kern/arm/coreboot/coreboot.S ++++ b/grub-core/kern/arm/coreboot/coreboot.S +@@ -42,8 +42,6 @@ FUNCTION(grub_armv7_get_timer_frequency) + mrc p15, 0, r0, c14, c0, 0 + bx lr + +-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-core/loader/efi/chainloader.c b/grub-core/loader/efi/chainloader.c +index 4a85f41..c6ae2fc 100644 +--- a/grub-core/loader/efi/chainloader.c ++++ b/grub-core/loader/efi/chainloader.c +@@ -910,7 +910,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")); +@@ -1080,9 +1079,7 @@ grub_cmd_chainloader (grub_command_t cmd __attribute__ ((unused)), + } + #endif + +- rc = grub_linuxefi_secure_validate((void *)((grub_addr_t) 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); + if (orig_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); +@@ -1103,7 +1100,6 @@ grub_cmd_chainloader (grub_command_t cmd __attribute__ ((unused)), + + return 0; + } +- // -1 fall-through to fail + + fail: + if (orig_dev) +diff --git a/grub-core/loader/efi/linux.c b/grub-core/loader/efi/linux.c +index 576d8cb..24abc0c 100644 +--- a/grub-core/loader/efi/linux.c ++++ b/grub-core/loader/efi/linux.c +@@ -433,7 +433,6 @@ grub_cmd_linux (grub_command_t cmd __attribute__ ((unused)), + struct linux_arch_kernel_header lh; + struct grub_arm64_linux_pe_header *pe; + grub_err_t err; +- int rc; + + grub_dl_ref (my_mod); + +@@ -478,17 +477,6 @@ grub_cmd_linux (grub_command_t cmd __attribute__ ((unused)), + + grub_dprintf ("linux", "kernel @ %p\n", kernel_addr); + +- if (grub_efi_get_secureboot() == GRUB_EFI_SECUREBOOT_MODE_ENABLED) +- { +- rc = grub_linuxefi_secure_validate (kernel_addr, kernel_size); +- if (rc <= 0) +- { +- grub_error (GRUB_ERR_INVALID_COMMAND, +- N_("%s has invalid signature"), argv[0]); +- goto fail; +- } +- } +- + pe = (void *)((unsigned long)kernel_addr + lh.hdr_offset); + handover_offset = pe->opt.entry_addr; + +diff --git a/grub-core/loader/efi/linux_sb.c b/grub-core/loader/efi/linux_sb.c +index a09479c..13bcb02 100644 +--- a/grub-core/loader/efi/linux_sb.c ++++ b/grub-core/loader/efi/linux_sb.c +@@ -25,47 +25,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; +- int status; +- +- grub_dprintf ("linuxefi", "Locating shim protocol\n"); +- 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(): %d\n", 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; +-} +- + typedef void (*handover_func) (void *, grub_efi_system_table_t *, void *); + + grub_err_t +diff --git a/grub-core/loader/i386/efi/linux.c b/grub-core/loader/i386/efi/linux.c +index 6a90743..cbd313a 100644 +--- a/grub-core/loader/i386/efi/linux.c ++++ b/grub-core/loader/i386/efi/linux.c +@@ -27,7 +27,6 @@ + #include + #include + #include +-#include + #include + + GRUB_MOD_LICENSE ("GPLv3+"); +@@ -170,7 +169,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); + +@@ -201,17 +199,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_ACCESS_DENIED, N_("%s has invalid signature"), +- argv[0]); +- goto fail; +- } +- } +- + params = grub_efi_allocate_pages_max (0x3fffffff, + BYTES_TO_PAGES(sizeof(*params))); + if (! params) +diff --git a/include/grub/efi/linux.h b/include/grub/efi/linux.h +index 39f5a9a..cedf20c 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 -Nru grub2-unsigned-2.06/debian/patches/series grub2-unsigned-2.06/debian/patches/series --- grub2-unsigned-2.06/debian/patches/series 2022-03-02 10:32:51.000000000 +0000 +++ grub2-unsigned-2.06/debian/patches/series 2022-03-15 13:00:37.000000000 +0000 @@ -121,5 +121,4 @@ efi-correct-struct-grub_efi_boot_services.patch efi-implement-grub_efi_run_image.patch fat-fix-listing-the-root-directory.patch -0124-grub-core-loader-i386-efi-linux.c-do-not-validate-ke.patch -0125-grub-core-loader-efi-chainloader.c-do-not-validate-c.patch +linuxefi-do-not-validate-kernels-twice.patch