From a5974373eed912a0fbf807d8509798a01c12398f Mon Sep 17 00:00:00 2001 From: Jan Setje-Eilers Date: Fri, 12 Jul 2024 17:07:45 -0700 Subject: [PATCH 1/4] Suppress file open failures for some netboot cases Reading files during a netboot comes with the caveat that fetching files from a network does not support anything like listing a directory. In the past this has meant that we do not try to open optional files during a netboot. However at least the revocation.efi file is now tested during a netboot, which will print an error when it is not found. Since that error is spurious we should allow for those errors to be suppressed. This is also desirable since we will likely go looking for additional files in the near future. --- shim.c | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/shim.c b/shim.c index 87202f7ff..c2e94bb84 100644 --- a/shim.c +++ b/shim.c @@ -54,6 +54,8 @@ extern struct { #define EFI_IMAGE_SECURITY_DATABASE_GUID { 0xd719b2cb, 0x3d3a, 0x4596, { 0xa3, 0xbc, 0xda, 0xd0, 0x0e, 0x67, 0x65, 0x6f }} +#define SUPPRESS_NETBOOT_OPEN_FAILURE_NOISE 1 + typedef enum { DATA_FOUND, DATA_NOT_FOUND, @@ -1091,7 +1093,8 @@ str16_to_str8(CHAR16 *str16, CHAR8 **str8) * Load and run an EFI executable */ EFI_STATUS read_image(EFI_HANDLE image_handle, CHAR16 *ImagePath, - CHAR16 **PathName, void **data, int *datasize) + CHAR16 **PathName, void **data, int *datasize, + int flags) { EFI_STATUS efi_status; void *sourcebuffer = NULL; @@ -1130,8 +1133,9 @@ EFI_STATUS read_image(EFI_HANDLE image_handle, CHAR16 *ImagePath, efi_status = FetchNetbootimage(image_handle, &sourcebuffer, &sourcesize); if (EFI_ERROR(efi_status)) { - perror(L"Unable to fetch TFTP image: %r\n", - efi_status); + if (~flags & SUPPRESS_NETBOOT_OPEN_FAILURE_NOISE) + perror(L"Unable to fetch TFTP image: %r\n", + efi_status); return efi_status; } *data = sourcebuffer; @@ -1143,8 +1147,9 @@ EFI_STATUS read_image(EFI_HANDLE image_handle, CHAR16 *ImagePath, &sourcesize, netbootname); if (EFI_ERROR(efi_status)) { - perror(L"Unable to fetch HTTP image %a: %r\n", - netbootname, efi_status); + if (~flags & SUPPRESS_NETBOOT_OPEN_FAILURE_NOISE) + perror(L"Unable to fetch HTTP image %a: %r\n", + netbootname, efi_status); return efi_status; } *data = sourcebuffer; @@ -1183,7 +1188,7 @@ EFI_STATUS start_image(EFI_HANDLE image_handle, CHAR16 *ImagePath) int datasize = 0; efi_status = read_image(image_handle, ImagePath, &PathName, &data, - &datasize); + &datasize, 0); if (EFI_ERROR(efi_status)) goto done; @@ -1460,7 +1465,8 @@ load_revocations_file(EFI_HANDLE image_handle, CHAR16 *PathName) uint8_t *sspv_latest = NULL; efi_status = read_image(image_handle, L"revocations.efi", &PathName, - &data, &datasize); + &data, &datasize, + SUPPRESS_NETBOOT_OPEN_FAILURE_NOISE); if (EFI_ERROR(efi_status)) return efi_status; @@ -1523,7 +1529,8 @@ load_cert_file(EFI_HANDLE image_handle, CHAR16 *filename, CHAR16 *PathName) int i; efi_status = read_image(image_handle, filename, &PathName, - &data, &datasize); + &data, &datasize, + SUPPRESS_NETBOOT_OPEN_FAILURE_NOISE); if (EFI_ERROR(efi_status)) return efi_status; From 224e914e08006288ec1f57921f843974989fc18c Mon Sep 17 00:00:00 2001 From: Jan Setje-Eilers Date: Fri, 16 Aug 2024 15:06:43 -0700 Subject: [PATCH 2/4] Allow indepdent SkuSi and SBAT revocation updates While a revocations.efi binary can contain either SBAT revocations, SkuSi revocations, or both, it is desirable to package them separately so that higher level tools such as fwupd can decide which ones to put in place at a given moment. This changes revocations.efi to revocations_sbat.efi and revocations_sku.efi XXX: Retain support a common revocations.efi file? Constrain the sections to match the file? --- shim.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/shim.c b/shim.c index c2e94bb84..e32218728 100644 --- a/shim.c +++ b/shim.c @@ -1464,7 +1464,10 @@ load_revocations_file(EFI_HANDLE image_handle, CHAR16 *PathName) uint8_t *ssps_latest = NULL; uint8_t *sspv_latest = NULL; - efi_status = read_image(image_handle, L"revocations.efi", &PathName, + efi_status = read_image(image_handle, L"revocations_sbat.efi", &PathName, + &data, &datasize, + SUPPRESS_NETBOOT_OPEN_FAILURE_NOISE); + efi_status = read_image(image_handle, L"revocations_skusi.efi", &PathName, &data, &datasize, SUPPRESS_NETBOOT_OPEN_FAILURE_NOISE); if (EFI_ERROR(efi_status)) From 3412508a682b747e554a2cfde71e4959e488efc6 Mon Sep 17 00:00:00 2001 From: Jan Setje-Eilers Date: Fri, 16 Aug 2024 15:21:52 -0700 Subject: [PATCH 3/4] netboot: process revocations.efi as revocations not shim_certificate Bugfix: In the netboot case revocations.efi files were read, but processed as shim_certificate.efi files which is simply wrong. --- shim.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/shim.c b/shim.c index e32218728..12b467c7f 100644 --- a/shim.c +++ b/shim.c @@ -1602,11 +1602,11 @@ load_unbundled_trust(EFI_HANDLE image_handle) efi_status); /* * Network boot cases do not support reading a directory. Try - * to read revocations.efi to pull in any unbundled SBATLevel + * to read revocations to pull in any unbundled SBATLevel * updates unconditionally in those cases. This may produce * console noise when the file is not present. */ - load_cert_file(image_handle, REVOCATIONFILE, PathName); + load_revocations_file(image_handle, PathName); goto done; } From e9aef686dbce4296a160bc58b230e5975aeb5c8f Mon Sep 17 00:00:00 2001 From: Jan Setje-Eilers Date: Fri, 16 Aug 2024 16:07:47 -0700 Subject: [PATCH 4/4] netboot can try to load shim_certificate_[0..9].efi Since we can't read the directory, we can try to load shim_certificate_[0..9].efi explicitly and give up after the first one that fails to load. XXX: should we just bring in snprintf()? support more than 10? nameing scheme --- shim.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/shim.c b/shim.c index 12b467c7f..031e1f412 100644 --- a/shim.c +++ b/shim.c @@ -32,6 +32,7 @@ #include #include +#include #define OID_EKU_MODSIGN "1.3.6.1.4.1.2312.16.1.2" @@ -1519,7 +1520,8 @@ load_revocations_file(EFI_HANDLE image_handle, CHAR16 *PathName) } EFI_STATUS -load_cert_file(EFI_HANDLE image_handle, CHAR16 *filename, CHAR16 *PathName) +load_cert_file(EFI_HANDLE image_handle, CHAR16 *filename, CHAR16 *PathName, + int flags) { EFI_STATUS efi_status; PE_COFF_LOADER_IMAGE_CONTEXT context; @@ -1532,8 +1534,7 @@ load_cert_file(EFI_HANDLE image_handle, CHAR16 *filename, CHAR16 *PathName) int i; efi_status = read_image(image_handle, filename, &PathName, - &data, &datasize, - SUPPRESS_NETBOOT_OPEN_FAILURE_NOISE); + &data, &datasize, flags); if (EFI_ERROR(efi_status)) return efi_status; @@ -1575,6 +1576,7 @@ load_unbundled_trust(EFI_HANDLE image_handle) EFI_STATUS efi_status; EFI_LOADED_IMAGE *li = NULL; CHAR16 *PathName = NULL; + static CHAR16 FileName[] = L"shim_certificate_0.efi"; EFI_FILE *root, *dir; EFI_FILE_INFO *info; EFI_HANDLE device; @@ -1582,6 +1584,7 @@ load_unbundled_trust(EFI_HANDLE image_handle) UINTN buffersize = 0; void *buffer = NULL; BOOLEAN search_revocations = TRUE; + int i = 0; efi_status = gBS->HandleProtocol(image_handle, &EFI_LOADED_IMAGE_GUID, (void **)&li); @@ -1693,13 +1696,18 @@ load_unbundled_trust(EFI_HANDLE image_handle) if (EFI_ERROR(efi_status)) { perror(L"Failed to open %s - %r\n", PathName, efi_status); + while (load_cert_file(image_handle, FileName, PathName, + SUPPRESS_NETBOOT_OPEN_FAILURE_NOISE) + == EFI_SUCCESS && i++ < 10) { + FileName[17]++; + } goto done; } } if (!search_revocations && StrnCaseCmp(info->FileName, L"shim_certificate", 16) == 0) { - load_cert_file(image_handle, info->FileName, PathName); + load_cert_file(image_handle, info->FileName, PathName, 0); } } done: