Skip to content

Commit

Permalink
shim: implement SBAT verification for the shim_lock protocol
Browse files Browse the repository at this point in the history 
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 <chris.coulson@canonical.com>
  • Loading branch information
@chrisccoulson @vathpela
chrisccoulson authored and vathpela committed Apr 5, 2022
1 parent 448f096 commit a2da05f
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 42 deletions.
2 changes: 1 addition & 1 deletion include/pe.h
View file
Expand Up @@ -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,
Expand Down
46 changes: 9 additions & 37 deletions pe.c
View file
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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
*/
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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)
Expand Down
73 changes: 69 additions & 4 deletions shim.c
View file
Expand Up @@ -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;
Expand Down Expand Up @@ -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)
{
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit a2da05f

Please sign in to comment.