From a2da05fcb8972628bec08e4adfc13abbafc319ad Mon Sep 17 00:00:00 2001 From: Chris Coulson Date: Mon, 28 Feb 2022 21:29:16 +0000 Subject: [PATCH] shim: implement SBAT verification for the shim_lock protocol This implements SBAT verification via the shim_lock protocol by moving verification inside the existing verify_buffer() function that is shared by both shim_verify() and handle_image(). The .sbat section is optional for code verified via the shim_lock protocol, unlike for code that is verified and executed directly by shim. For executables that don't have a .sbat section, verification is skipped when using the protocol. A vendor can enforce SBAT verification for code verified via the shim_lock protocol by revoking all pre-SBAT binaries via a dbx update or by using vendor_dbx and then only signing binaries that have a .sbat section from that point. Signed-off-by: Chris Coulson --- include/pe.h | 2 +- pe.c | 46 +++++++-------------------------- shim.c | 73 +++++++++++++++++++++++++++++++++++++++++++++++++--- 3 files changed, 79 insertions(+), 42 deletions(-) diff --git a/include/pe.h b/include/pe.h index 43727f5..b86e1b3 100644 --- a/include/pe.h +++ b/include/pe.h @@ -15,7 +15,7 @@ read_header(void *data, unsigned int datasize, PE_COFF_LOADER_IMAGE_CONTEXT *context); EFI_STATUS -handle_sbat(char *SBATBase, size_t SBATSize); +verify_sbat_section(char *SBATBase, size_t SBATSize); EFI_STATUS handle_image (void *data, unsigned int datasize, diff --git a/pe.c b/pe.c index 92c2804..554e77c 100644 --- a/pe.c +++ b/pe.c @@ -820,7 +820,7 @@ read_header(void *data, unsigned int datasize, } EFI_STATUS -handle_sbat(char *SBATBase, size_t SBATSize) +verify_sbat_section(char *SBATBase, size_t SBATSize) { unsigned int i; EFI_STATUS efi_status; @@ -834,7 +834,12 @@ handle_sbat(char *SBATBase, size_t SBATSize) if (SBATBase == NULL || SBATSize == 0) { dprint(L"No .sbat section data\n"); - return EFI_SECURITY_VIOLATION; + /* + * SBAT is mandatory for binaries loaded by shim, but optional + * for binaries loaded outside of shim but verified via the + * protocol. + */ + return in_protocol ? EFI_SUCCESS : EFI_SECURITY_VIOLATION; } sbat_size = SBATSize + 1; @@ -980,9 +985,6 @@ handle_image (void *data, unsigned int datasize, EFI_IMAGE_SECTION_HEADER *RelocSection = NULL; - char *SBATBase = NULL; - size_t SBATSize = 0; - /* * Copy the executable's sections to their desired offsets */ @@ -1027,33 +1029,6 @@ handle_image (void *data, unsigned int datasize, RelocBaseEnd == end) { RelocSection = Section; } - } else if (CompareMem(Section->Name, ".sbat\0\0\0", 8) == 0) { - if (SBATBase || SBATSize) { - perror(L"Image has multiple SBAT sections\n"); - return EFI_UNSUPPORTED; - } - - if (Section->NumberOfRelocations != 0 || - Section->PointerToRelocations != 0) { - perror(L"SBAT section has relocations\n"); - return EFI_UNSUPPORTED; - } - - /* The virtual size corresponds to the size of the SBAT - * metadata and isn't necessarily a multiple of the file - * alignment. The on-disk size is a multiple of the file - * alignment and is zero padded. Make sure that the - * on-disk size is at least as large as virtual size, - * and ignore the section if it isn't. */ - if (Section->SizeOfRawData && - Section->SizeOfRawData >= Section->Misc.VirtualSize && - base && end) { - SBATBase = base; - /* +1 because of size vs last byte location */ - SBATSize = end - base + 1; - dprint(L"sbat section base:0x%lx size:0x%lx\n", - SBATBase, SBATSize); - } } if (Section->Characteristics & EFI_IMAGE_SCN_MEM_DISCARDABLE) { @@ -1095,11 +1070,8 @@ handle_image (void *data, unsigned int datasize, } if (secure_mode ()) { - efi_status = handle_sbat(SBATBase, SBATSize); - - if (!EFI_ERROR(efi_status)) - efi_status = verify_buffer(data, datasize, - &context, sha256hash, sha1hash); + efi_status = verify_buffer(data, datasize, &context, sha256hash, + sha1hash); if (EFI_ERROR(efi_status)) { if (verbose) diff --git a/shim.c b/shim.c index 604c0db..6d6b1e5 100644 --- a/shim.c +++ b/shim.c @@ -559,9 +559,9 @@ verify_one_signature(WIN_CERTIFICATE_EFI_PKCS *sig, * Check that the signature is valid and matches the binary */ EFI_STATUS -verify_buffer (char *data, int datasize, - PE_COFF_LOADER_IMAGE_CONTEXT *context, - UINT8 *sha256hash, UINT8 *sha1hash) +verify_buffer_authenticode (char *data, int datasize, + PE_COFF_LOADER_IMAGE_CONTEXT *context, + UINT8 *sha256hash, UINT8 *sha1hash) { EFI_STATUS ret_efi_status; size_t size = datasize; @@ -695,6 +695,71 @@ verify_buffer (char *data, int datasize, return ret_efi_status; } +/* + * Check that the binary is permitted to load by SBAT. + */ +EFI_STATUS +verify_buffer_sbat (char *data, int datasize, + PE_COFF_LOADER_IMAGE_CONTEXT *context) +{ + int i; + EFI_IMAGE_SECTION_HEADER *Section; + char *SBATBase = NULL; + size_t SBATSize = 0; + + Section = context->FirstSection; + for (i = 0; i < context->NumberOfSections; i++, Section++) { + if (CompareMem(Section->Name, ".sbat\0\0\0", 8) != 0) + continue; + + if (SBATBase || SBATSize) { + perror(L"Image has multiple SBAT sections\n"); + return EFI_UNSUPPORTED; + } + + if (Section->NumberOfRelocations != 0 || + Section->PointerToRelocations != 0) { + perror(L"SBAT section has relocations\n"); + return EFI_UNSUPPORTED; + } + + /* The virtual size corresponds to the size of the SBAT + * metadata and isn't necessarily a multiple of the file + * alignment. The on-disk size is a multiple of the file + * alignment and is zero padded. Make sure that the + * on-disk size is at least as large as virtual size, + * and ignore the section if it isn't. */ + if (Section->SizeOfRawData && + Section->SizeOfRawData >= Section->Misc.VirtualSize) { + SBATBase = ImageAddress(data, datasize, + Section->PointerToRawData); + SBATSize = Section->SizeOfRawData; + dprint(L"sbat section base:0x%lx size:0x%lx\n", + SBATBase, SBATSize); + } + } + + return verify_sbat_section(SBATBase, SBATSize); +} + +/* + * Check that the signature is valid and matches the binary and that + * the binary is permitted to load by SBAT. + */ +EFI_STATUS +verify_buffer (char *data, int datasize, + PE_COFF_LOADER_IMAGE_CONTEXT *context, + UINT8 *sha256hash, UINT8 *sha1hash) +{ + EFI_STATUS efi_status; + + efi_status = verify_buffer_sbat(data, datasize, context); + if (EFI_ERROR(efi_status)) + return efi_status; + + return verify_buffer_authenticode(data, datasize, context, sha256hash, sha1hash); +} + static int should_use_fallback(EFI_HANDLE image_handle) { @@ -1542,7 +1607,7 @@ efi_main (EFI_HANDLE passed_image_handle, EFI_SYSTEM_TABLE *passed_systab) goto die; } - efi_status = handle_sbat(sbat_start, sbat_end - sbat_start - 1); + efi_status = verify_sbat_section(sbat_start, sbat_end - sbat_start - 1); if (EFI_ERROR(efi_status)) { perror(L"Verifiying shim SBAT data failed: %r\n", efi_status); -- 2.27.0