From f234a56db597c04b7c923469962a54637c381bb9 Mon Sep 17 00:00:00 2001 From: Jan Janssen Date: Wed, 3 Aug 2022 11:05:12 +0200 Subject: [PATCH 1/3] boot: Follow-up fixes for #20255 --- man/loader.conf.xml | 4 ++-- src/boot/efi/boot.c | 4 ++-- src/boot/efi/secure-boot.c | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/man/loader.conf.xml b/man/loader.conf.xml index acddb193d1..7aa1ed7254 100644 --- a/man/loader.conf.xml +++ b/man/loader.conf.xml @@ -248,9 +248,9 @@ and PK.esl. Note that these files need to be authenticated UEFI variables. See below for an example of how to generate them from regular X.509 keys. - uuid=$(systemd-id128 new --) + uuid=$(systemd-id128 new --uuid) for key in PK KEK db; do - openssl req -new -x509 -subj "/CN=${key}/ -keyout "${key}.key" -out "${key}.crt" + openssl req -new -x509 -subj "/CN=${key}/" -keyout "${key}.key" -out "${key}.crt" openssl x509 -outform DER -in "${key}.crt" -out "${key}.cer" cert-to-efi-sig-list -g "${uuid}" "${key}.crt" "${key}.tmp" done diff --git a/src/boot/efi/boot.c b/src/boot/efi/boot.c index db0bbab0f2..87771c477d 100644 --- a/src/boot/efi/boot.c +++ b/src/boot/efi/boot.c @@ -2690,8 +2690,8 @@ EFI_STATUS efi_main(EFI_HANDLE image, EFI_SYSTEM_TABLE *sys_table) { /* if auto enrollment is activated, we try to load keys for the given entry. */ if (entry->type == LOADER_SECURE_BOOT_KEYS && config.secure_boot_enroll != ENROLL_OFF) { err = secure_boot_enroll_at(root_dir, entry->path); - if (err == EFI_SUCCESS) - return EFI_SUCCESS; + if (err != EFI_SUCCESS) + return err; continue; } diff --git a/src/boot/efi/secure-boot.c b/src/boot/efi/secure-boot.c index 1da1026df5..aabe521eba 100644 --- a/src/boot/efi/secure-boot.c +++ b/src/boot/efi/secure-boot.c @@ -43,7 +43,7 @@ EFI_STATUS secure_boot_enroll_at(EFI_FILE *root_dir, const char16_t *path) { clear_screen(COLOR_NORMAL); - Print(L"Enrolling secure boot keys from directory: \\loader\\keys\\%s\n" + Print(L"Enrolling secure boot keys from directory: %s\n" L"Warning: Enrolling custom Secure Boot keys might soft-brick your machine!\n", path); From adb9485acb2b7f429a72ce6db024724a809b6ec1 Mon Sep 17 00:00:00 2001 From: Jan Janssen Date: Wed, 3 Aug 2022 11:27:38 +0200 Subject: [PATCH 2/3] man: Add instructions for Microsoft secure boot keys Adding Microsoft keys by default is recommended because firmware drivers might be signed by it. This also changes the file ending from .esl to .auth as that is used by sign-efi-sig-list manpage and other sources. --- man/loader.conf.xml | 30 ++++++++++++++++++++++++------ man/systemd-boot.xml | 2 +- src/boot/efi/secure-boot.c | 6 +++--- 3 files changed, 28 insertions(+), 10 deletions(-) diff --git a/man/loader.conf.xml b/man/loader.conf.xml index 7aa1ed7254..3ee42cdb73 100644 --- a/man/loader.conf.xml +++ b/man/loader.conf.xml @@ -244,20 +244,38 @@ EDK2 documentation. - A complete set of UEFI variable includes db.esl, KEK.esl - and PK.esl. Note that these files need to be authenticated UEFI variables. See + A complete set of UEFI variable includes db.auth, KEK.auth + and PK.auth. Note that these files need to be authenticated UEFI variables. See below for an example of how to generate them from regular X.509 keys. uuid=$(systemd-id128 new --uuid) for key in PK KEK db; do openssl req -new -x509 -subj "/CN=${key}/" -keyout "${key}.key" -out "${key}.crt" openssl x509 -outform DER -in "${key}.crt" -out "${key}.cer" - cert-to-efi-sig-list -g "${uuid}" "${key}.crt" "${key}.tmp" + cert-to-efi-sig-list -g "${uuid}" "${key}.crt" "${key}.esl" done -sign-efi-sig-list -c PK.crt -k PK.key PK PK.tmp PK.esl -sign-efi-sig-list -c PK.crt -k PK.key KEK KEK.tmp KEK.esl -sign-efi-sig-list -c KEK.crt -k KEK.key db db.tmp db.esl +for key in MicWinProPCA2011_2011-10-19.crt MicCorUEFCA2011_2011-06-27.crt MicCorKEKCA2011_2011-06-24.crt; do + curl "https://www.microsoft.com/pkiops/certs/${key}" --output "${key}" + sbsiglist --owner 77fa9abd-0359-4d32-bd60-28f4e78f784b --type x509 --output "${key%crt}esl" "${key}" +done + +# Optionally add Microsoft Windows Production CA 2011 (needed to boot into Windows). +cat MicWinProPCA2011_2011-10-19.esl >> db.esl + +# Optionally add Microsoft Corporation UEFI CA 2011 (for firmware drivers / option ROMs +# and third-party boot loaders (including shim). This is highly recommended on real +# hardware as not including this may soft-brick your device (see next paragraph). +cat MicCorUEFCA2011_2011-06-27.esl >> db.esl + +# Optionally add Microsoft Corporation KEK CA 2011. Recommended if either of the +# Microsoft keys is used as the official UEFI revocation database is signed with this +# key. The revocation database can be updated with fwupdmgr1. +cat MicCorKEKCA2011_2011-06-24.esl >> KEK.esl + +sign-efi-sig-list -c PK.crt -k PK.key PK PK.esl PK.auth +sign-efi-sig-list -c PK.crt -k PK.key KEK KEK.esl KEK.auth +sign-efi-sig-list -c KEK.crt -k KEK.key db db.esl db.auth This feature is considered dangerous because even if all the required files are signed with the diff --git a/man/systemd-boot.xml b/man/systemd-boot.xml index 02790370df..99a749b327 100644 --- a/man/systemd-boot.xml +++ b/man/systemd-boot.xml @@ -319,7 +319,7 @@ similar, to extend the native firmware support. Enrollment of Secure Boot variables can be performed manually or automatically if files are available - under /keys/NAME/{db,KEK,PK}.esl, NAME + under /keys/NAME/{db,KEK,PK}.auth, NAME being the display name for the set of variables in the menu. If one of the sets is named auto then it might be enrolled automatically depending on whether secure-boot-enroll is set to force or not. diff --git a/src/boot/efi/secure-boot.c b/src/boot/efi/secure-boot.c index aabe521eba..854825abdb 100644 --- a/src/boot/efi/secure-boot.c +++ b/src/boot/efi/secure-boot.c @@ -81,9 +81,9 @@ EFI_STATUS secure_boot_enroll_at(EFI_FILE *root_dir, const char16_t *path) { char *buffer; size_t size; } sb_vars[] = { - { u"db", u"db.esl", EFI_IMAGE_SECURITY_DATABASE_VARIABLE, NULL, 0 }, - { u"KEK", u"KEK.esl", EFI_GLOBAL_VARIABLE, NULL, 0 }, - { u"PK", u"PK.esl", EFI_GLOBAL_VARIABLE, NULL, 0 }, + { u"db", u"db.auth", EFI_IMAGE_SECURITY_DATABASE_VARIABLE, NULL, 0 }, + { u"KEK", u"KEK.auth", EFI_GLOBAL_VARIABLE, NULL, 0 }, + { u"PK", u"PK.auth", EFI_GLOBAL_VARIABLE, NULL, 0 }, }; /* Make sure all keys files exist before we start enrolling them by loading them from the disk first. */ From bafc594528767192bc4b3c0081a992a9e2647d50 Mon Sep 17 00:00:00 2001 From: Jan Janssen Date: Thu, 4 Aug 2022 10:21:15 +0200 Subject: [PATCH 3/3] boot: Skip safety countdown when running in a VM --- src/boot/efi/secure-boot.c | 5 +++++ src/boot/efi/ticks.c | 26 ++++---------------------- src/boot/efi/util.c | 17 +++++++++++++++++ src/boot/efi/util.h | 8 ++++++++ 4 files changed, 34 insertions(+), 22 deletions(-) diff --git a/src/boot/efi/secure-boot.c b/src/boot/efi/secure-boot.c index 854825abdb..cf7a464d0a 100644 --- a/src/boot/efi/secure-boot.c +++ b/src/boot/efi/secure-boot.c @@ -49,6 +49,11 @@ EFI_STATUS secure_boot_enroll_at(EFI_FILE *root_dir, const char16_t *path) { unsigned timeout_sec = 15; for(;;) { + /* Enrolling secure boot keys is safe to do in virtualized environments as there is nothing + * we can brick there. */ + if (in_hypervisor()) + break; + PrintAt(0, ST->ConOut->Mode->CursorRow, L"Enrolling in %2u s, press any key to abort.", timeout_sec); uint64_t key; diff --git a/src/boot/efi/ticks.c b/src/boot/efi/ticks.c index 16e488c958..1b74ba15d0 100644 --- a/src/boot/efi/ticks.c +++ b/src/boot/efi/ticks.c @@ -2,35 +2,17 @@ #include #include -#if defined(__i386__) || defined(__x86_64__) -#include -#endif -#include #include "ticks.h" - -#if defined(__i386__) || defined(__x86_64__) -static bool in_hypervisor(void) { - uint32_t eax, ebx, ecx, edx; - - /* The TSC might or might not be virtualized in VMs (and thus might not be accurate or start at zero - * at boot), depending on hypervisor and CPU functionality. If it's not virtualized it's not useful - * for keeping time, hence don't attempt to use it. - * - * This is a dumbed down version of src/basic/virt.c's detect_vm() that safely works in the UEFI - * environment. */ - - if (__get_cpuid(1, &eax, &ebx, &ecx, &edx) == 0) - return false; - - return !!(ecx & 0x80000000U); -} -#endif +#include "util.h" #ifdef __x86_64__ static uint64_t ticks_read(void) { uint64_t a, d; + /* The TSC might or might not be virtualized in VMs (and thus might not be accurate or start at zero + * at boot), depending on hypervisor and CPU functionality. If it's not virtualized it's not useful + * for keeping time, hence don't attempt to use it. */ if (in_hypervisor()) return 0; diff --git a/src/boot/efi/util.c b/src/boot/efi/util.c index 6fcf9b3121..a41dbaa43e 100644 --- a/src/boot/efi/util.c +++ b/src/boot/efi/util.c @@ -2,6 +2,9 @@ #include #include +#if defined(__i386__) || defined(__x86_64__) +# include +#endif #include "ticks.h" #include "util.h" @@ -768,3 +771,17 @@ EFI_STATUS make_file_device_path(EFI_HANDLE device, const char16_t *file, EFI_DE SetDevicePathEndNode(dp); return EFI_SUCCESS; } + +#if defined(__i386__) || defined(__x86_64__) +bool in_hypervisor(void) { + uint32_t eax, ebx, ecx, edx; + + /* This is a dumbed down version of src/basic/virt.c's detect_vm() that safely works in the UEFI + * environment. */ + + if (__get_cpuid(1, &eax, &ebx, &ecx, &edx) == 0) + return false; + + return !!(ecx & 0x80000000U); +} +#endif diff --git a/src/boot/efi/util.h b/src/boot/efi/util.h index bb4bb64e0e..afbc217d53 100644 --- a/src/boot/efi/util.h +++ b/src/boot/efi/util.h @@ -179,3 +179,11 @@ static inline void beep(UINTN beep_count) {} EFI_STATUS open_volume(EFI_HANDLE device, EFI_FILE **ret_file); EFI_STATUS make_file_device_path(EFI_HANDLE device, const char16_t *file, EFI_DEVICE_PATH **ret_dp); + +#if defined(__i386__) || defined(__x86_64__) +bool in_hypervisor(void); +#else +static inline bool in_hypervisor(void) { + return false; +} +#endif