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 <[email protected]>
jsjgruber · Apr 5, 2022 · a2da05f · a2da05f
1 parent 448f096
commit a2da05f
Show file tree

Hide file tree

Showing 3 changed files with 79 additions and 42 deletions.
diff --git 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
@@ -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
@@ -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
 is_removable_media_path(EFI_LOADED_IMAGE *li)
 {
@@ -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);