Don't verify kernels twice
Signed-off-by: Robbie Harwood <rharwood@redhat.com>
This commit is contained in:
Robbie Harwood
parent
e31fc7ca96
commit
90dacf59d0
@ -0,0 +1,73 @@
|
||||
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
|
||||
From: Dimitri John Ledkov <dimitri.ledkov@canonical.com>
|
||||
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 <dimitri.ledkov@canonical.com>
|
||||
---
|
||||
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 <grub/cpu/efi/memory.h>
|
||||
#include <grub/tpm.h>
|
||||
#include <grub/safemath.h>
|
||||
-#include <grub/efi/sb.h>
|
||||
|
||||
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);
|
||||
|
||||
@ -0,0 +1,58 @@
|
||||
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
|
||||
From: Dimitri John Ledkov <dimitri.ledkov@canonical.com>
|
||||
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 <dimitri.ledkov@canonical.com>
|
||||
---
|
||||
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 <grub/i18n.h>
|
||||
#include <grub/lib/cmdline.h>
|
||||
#include <grub/verify.h>
|
||||
-#include <grub/efi/sb.h>
|
||||
|
||||
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)
|
||||
@ -0,0 +1,80 @@
|
||||
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
|
||||
From: Dimitri John Ledkov <dimitri.ledkov@canonical.com>
|
||||
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 <dimitri.ledkov@canonical.com>
|
||||
---
|
||||
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)
|
||||
@ -0,0 +1,83 @@
|
||||
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
|
||||
From: Dimitri John Ledkov <dimitri.ledkov@canonical.com>
|
||||
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 <dimitri.ledkov@canonical.com>
|
||||
---
|
||||
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 <grub/efi/pe32.h>
|
||||
#include <grub/efi/linux.h>
|
||||
|
||||
-#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 <grub/err.h>
|
||||
#include <grub/symbol.h>
|
||||
|
||||
-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);
|
||||
@ -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
|
||||
|
||||
@ -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 <rharwood@redhat.com> - 2.06-23
|
||||
- Don't verify kernels twice
|
||||
|
||||
* Thu Mar 10 2022 Robbie Harwood <rharwood@redhat.com> - 2.06-22
|
||||
- Skip updating BLS on ostree installations
|
||||
- Resolves: #2059776
|
||||
|
||||
Loading…
Reference in New Issue
Block a user