From bfc075fa6b361b8b86e011f79939b603a5a34d42 Mon Sep 17 00:00:00 2001 From: Jan Janssen Date: Wed, 21 Sep 2022 10:42:40 +0200 Subject: [PATCH 1/6] stub: Rename image parameter This is really the parent image for the kernel that is to be run. Renaming it as such prevents confusion with any image handles that are about to be created. --- src/boot/efi/linux.c | 6 +++--- src/boot/efi/linux.h | 4 ++-- src/boot/efi/linux_x86.c | 12 ++++++------ 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/boot/efi/linux.c b/src/boot/efi/linux.c index 7771058c3a..1cebae34ec 100644 --- a/src/boot/efi/linux.c +++ b/src/boot/efi/linux.c @@ -93,7 +93,7 @@ static inline void cleanup_loaded_image(EFI_HANDLE *loaded_image_handle) { } EFI_STATUS linux_exec( - EFI_HANDLE image, + EFI_HANDLE parent, const char *cmdline, UINTN cmdline_len, const void *linux_buffer, UINTN linux_length, const void *initrd_buffer, UINTN initrd_length) { @@ -105,7 +105,7 @@ EFI_STATUS linux_exec( void *new_buffer; EFI_STATUS err; - assert(image); + assert(parent); assert(cmdline || cmdline_len == 0); assert(linux_buffer && linux_length > 0); assert(initrd_buffer || initrd_length == 0); @@ -117,7 +117,7 @@ EFI_STATUS linux_exec( /* Kernel is too old to support LINUX_INITRD_MEDIA_GUID, try the deprecated EFI handover * protocol. */ return linux_exec_efi_handover( - image, + parent, cmdline, cmdline_len, linux_buffer, diff --git a/src/boot/efi/linux.h b/src/boot/efi/linux.h index eab617e579..19e5f5c4a8 100644 --- a/src/boot/efi/linux.h +++ b/src/boot/efi/linux.h @@ -4,12 +4,12 @@ #include EFI_STATUS linux_exec( - EFI_HANDLE image, + EFI_HANDLE parent, const char *cmdline, UINTN cmdline_len, const void *linux_buffer, UINTN linux_length, const void *initrd_buffer, UINTN initrd_length); EFI_STATUS linux_exec_efi_handover( - EFI_HANDLE image, + EFI_HANDLE parent, const char *cmdline, UINTN cmdline_len, const void *linux_buffer, UINTN linux_length, const void *initrd_buffer, UINTN initrd_length); diff --git a/src/boot/efi/linux_x86.c b/src/boot/efi/linux_x86.c index c664abd461..64336ce348 100644 --- a/src/boot/efi/linux_x86.c +++ b/src/boot/efi/linux_x86.c @@ -99,10 +99,10 @@ assert_cc(sizeof(BootParams) == 4096); # define __regparm0__ #endif -typedef void (*handover_f)(void *image, EFI_SYSTEM_TABLE *table, BootParams *params) __regparm0__ +typedef void (*handover_f)(void *parent, EFI_SYSTEM_TABLE *table, BootParams *params) __regparm0__ __attribute__((sysv_abi)); -static void linux_efi_handover(EFI_HANDLE image, uintptr_t kernel, BootParams *params) { +static void linux_efi_handover(EFI_HANDLE parent, uintptr_t kernel, BootParams *params) { assert(params); kernel += (params->hdr.setup_sects + 1) * KERNEL_SECTOR_SIZE; /* 32bit entry address. */ @@ -121,16 +121,16 @@ static void linux_efi_handover(EFI_HANDLE image, uintptr_t kernel, BootParams *p * kernel to be booted from a 32bit sd-stub. */ handover_f handover = (handover_f) kernel; - handover(image, ST, params); + handover(parent, ST, params); } EFI_STATUS linux_exec_efi_handover( - EFI_HANDLE image, + EFI_HANDLE parent, const char *cmdline, UINTN cmdline_len, const void *linux_buffer, UINTN linux_length, const void *initrd_buffer, UINTN initrd_length) { - assert(image); + assert(parent); assert(cmdline || cmdline_len == 0); assert(linux_buffer); assert(initrd_buffer || initrd_length == 0); @@ -203,6 +203,6 @@ EFI_STATUS linux_exec_efi_handover( boot_params->hdr.ramdisk_size = initrd_length; boot_params->ext_ramdisk_size = ((uint64_t) initrd_length) >> 32; - linux_efi_handover(image, (uintptr_t) linux_buffer, boot_params); + linux_efi_handover(parent, (uintptr_t) linux_buffer, boot_params); return EFI_LOAD_ERROR; } From a529d8182e29d300385b742479fa7964724a6e04 Mon Sep 17 00:00:00 2001 From: Jan Janssen Date: Wed, 21 Sep 2022 11:07:53 +0200 Subject: [PATCH 2/6] stub: Use LoadImage/StartImage to start the kernel This is the proper way to start any EFI binary. The fact this even ever worked was because the kernel does not have any PE relocations. The only downside is that the embedded kernel image has to be signed and trusted by the firmware under secure boot. A future commit will try to deal with that. --- src/boot/efi/linux.c | 167 +++++++++++++++---------------------------- src/boot/efi/stub.c | 2 +- 2 files changed, 58 insertions(+), 111 deletions(-) diff --git a/src/boot/efi/linux.c b/src/boot/efi/linux.c index 1cebae34ec..5b062d409a 100644 --- a/src/boot/efi/linux.c +++ b/src/boot/efi/linux.c @@ -16,80 +16,41 @@ #include "pe.h" #include "util.h" -static EFI_LOADED_IMAGE_PROTOCOL *loaded_image_free(EFI_LOADED_IMAGE_PROTOCOL *img) { - if (!img) - return NULL; - mfree(img->LoadOptions); - return mfree(img); -} +#define STUB_PAYLOAD_GUID \ + { 0x55c5d1f8, 0x04cd, 0x46b5, { 0x8a, 0x20, 0xe5, 0x6c, 0xbb, 0x30, 0x52, 0xd0 } } -static EFI_STATUS loaded_image_register( - const char *cmdline, UINTN cmdline_len, - const void *linux_buffer, UINTN linux_length, - EFI_HANDLE *ret_image) { - - EFI_LOADED_IMAGE_PROTOCOL *loaded_image = NULL; - EFI_STATUS err; - - assert(cmdline || cmdline_len > 0); - assert(linux_buffer && linux_length > 0); +EFI_STATUS load_image(EFI_HANDLE parent, const void *source, size_t len, EFI_HANDLE *ret_image) { + assert(parent); + assert(source); assert(ret_image); - /* create and install new LoadedImage Protocol */ - loaded_image = xnew(EFI_LOADED_IMAGE_PROTOCOL, 1); - *loaded_image = (EFI_LOADED_IMAGE_PROTOCOL) { - .ImageBase = (void *) linux_buffer, - .ImageSize = linux_length + /* We could pass a NULL device path, but it's nicer to provide something. */ + struct { + VENDOR_DEVICE_PATH payload; + EFI_DEVICE_PATH end; + } _packed_ payload_device_path = { + .payload = { + .Header = { + .Type = MEDIA_DEVICE_PATH, + .SubType = MEDIA_VENDOR_DP, + .Length = { sizeof(payload_device_path.payload), 0 }, + }, + .Guid = STUB_PAYLOAD_GUID, + }, + .end = { + .Type = END_DEVICE_PATH_TYPE, + .SubType = END_ENTIRE_DEVICE_PATH_SUBTYPE, + .Length = { sizeof(payload_device_path.end), 0 }, + }, }; - /* if a cmdline is set convert it to UCS2 */ - if (cmdline) { - loaded_image->LoadOptions = xstra_to_str(cmdline); - loaded_image->LoadOptionsSize = strsize16(loaded_image->LoadOptions); - } - - /* install a new LoadedImage protocol. ret_handle is a new image handle */ - err = BS->InstallMultipleProtocolInterfaces( - ret_image, - &LoadedImageProtocol, loaded_image, - NULL); - if (err != EFI_SUCCESS) - loaded_image = loaded_image_free(loaded_image); - - return err; -} - -static EFI_STATUS loaded_image_unregister(EFI_HANDLE loaded_image_handle) { - EFI_LOADED_IMAGE_PROTOCOL *loaded_image; - EFI_STATUS err; - - if (!loaded_image_handle) - return EFI_SUCCESS; - - /* get the LoadedImage protocol that we allocated earlier */ - err = BS->OpenProtocol( - loaded_image_handle, &LoadedImageProtocol, (void **) &loaded_image, - NULL, NULL, EFI_OPEN_PROTOCOL_GET_PROTOCOL); - if (err != EFI_SUCCESS) - return err; - - /* close the handle */ - (void) BS->CloseProtocol(loaded_image_handle, &LoadedImageProtocol, NULL, NULL); - err = BS->UninstallMultipleProtocolInterfaces( - loaded_image_handle, - &LoadedImageProtocol, loaded_image, - NULL); - if (err != EFI_SUCCESS) - return err; - loaded_image_handle = NULL; - loaded_image = loaded_image_free(loaded_image); - - return EFI_SUCCESS; -} - -static inline void cleanup_loaded_image(EFI_HANDLE *loaded_image_handle) { - (void) loaded_image_unregister(*loaded_image_handle); - *loaded_image_handle = NULL; + return BS->LoadImage( + /*BootPolicy=*/false, + parent, + &payload_device_path.payload.Header, + (void *) source, + len, + ret_image); } EFI_STATUS linux_exec( @@ -98,11 +59,7 @@ EFI_STATUS linux_exec( const void *linux_buffer, UINTN linux_length, const void *initrd_buffer, UINTN initrd_length) { - _cleanup_(cleanup_initrd) EFI_HANDLE initrd_handle = NULL; - _cleanup_(cleanup_loaded_image) EFI_HANDLE loaded_image_handle = NULL; - uint32_t kernel_alignment, kernel_size_of_image, kernel_entry_address; - EFI_IMAGE_ENTRY_POINT kernel_entry; - void *new_buffer; + uint32_t kernel_alignment, kernel_size_of_image, kernel_entry_address = 0; EFI_STATUS err; assert(parent); @@ -126,46 +83,36 @@ EFI_STATUS linux_exec( initrd_length); #endif if (err != EFI_SUCCESS) - return err; - /* sanity check */ - assert(kernel_size_of_image >= linux_length); + return log_error_status_stall(err, u"Bad kernel image: %r", err); - /* Linux kernel complains if it's not loaded at a properly aligned memory address. The correct alignment - is provided by Linux as the SegmentAlignment in the PeOptionalHeader. Additionally the kernel needs to - be in a memory segment that's SizeOfImage (again from PeOptionalHeader) large, so that the Kernel has - space for its BSS section. SizeOfImage is always larger than linux_length, which is only the size of - Code, (static) Data and Headers. - - Interrestingly only ARM/Aarch64 and RISC-V kernel stubs check these assertions and can even boot (with warnings) - if they are not met. x86 and x86_64 kernel stubs don't do checks and fail if the BSS section is too small. - */ - /* allocate SizeOfImage + SectionAlignment because the new_buffer can move up to Alignment-1 bytes */ - _cleanup_pages_ Pages kernel = xmalloc_pages( - AllocateAnyPages, - EfiLoaderCode, - EFI_SIZE_TO_PAGES(ALIGN_TO(kernel_size_of_image, kernel_alignment) + kernel_alignment), - 0); - new_buffer = PHYSICAL_ADDRESS_TO_POINTER(ALIGN_TO(kernel.addr, kernel_alignment)); - if (!new_buffer) /* Silence gcc 11.2.0, assert(new_buffer) doesn't work. */ - return EFI_OUT_OF_RESOURCES; - - memcpy(new_buffer, linux_buffer, linux_length); - /* zero out rest of memory (probably not needed, but BSS section should be 0) */ - memset((uint8_t *)new_buffer + linux_length, 0, kernel_size_of_image - linux_length); - - /* get the entry point inside the relocated kernel */ - kernel_entry = (EFI_IMAGE_ENTRY_POINT) ((const uint8_t *)new_buffer + kernel_entry_address); - - /* register a LoadedImage Protocol in order to pass on the commandline */ - err = loaded_image_register(cmdline, cmdline_len, new_buffer, linux_length, &loaded_image_handle); + _cleanup_(unload_imagep) EFI_HANDLE kernel_image = NULL; + err = load_image(parent, linux_buffer, linux_length, &kernel_image); if (err != EFI_SUCCESS) - return err; + return log_error_status_stall(err, u"Error loading kernel image: %r", err); - /* register a LINUX_INITRD_MEDIA DevicePath to serve the initrd */ + EFI_LOADED_IMAGE_PROTOCOL *loaded_image; + err = BS->HandleProtocol(kernel_image, &LoadedImageProtocol, (void **) &loaded_image); + if (err != EFI_SUCCESS) + return log_error_status_stall(err, u"Error getting kernel loaded image protocol: %r", err); + + if (cmdline) { + loaded_image->LoadOptions = xstra_to_str(cmdline); + loaded_image->LoadOptionsSize = strsize16(loaded_image->LoadOptions); + } + + _cleanup_(cleanup_initrd) EFI_HANDLE initrd_handle = NULL; err = initrd_register(initrd_buffer, initrd_length, &initrd_handle); if (err != EFI_SUCCESS) - return err; + return log_error_status_stall(err, u"Error registering initrd: %r", err); - /* call the kernel */ - return kernel_entry(loaded_image_handle, ST); + err = BS->StartImage(kernel_image, NULL, NULL); + + /* Try calling the kernel compat entry point if one exists. */ + if (err == EFI_UNSUPPORTED && kernel_entry_address > 0) { + EFI_IMAGE_ENTRY_POINT compat_entry = + (EFI_IMAGE_ENTRY_POINT) ((uint8_t *) loaded_image->ImageBase + kernel_entry_address); + err = compat_entry(kernel_image, ST); + } + + return log_error_status_stall(err, u"Error starting kernel image: %r", err); } diff --git a/src/boot/efi/stub.c b/src/boot/efi/stub.c index 8bd1e985c9..a842c5c679 100644 --- a/src/boot/efi/stub.c +++ b/src/boot/efi/stub.c @@ -378,5 +378,5 @@ EFI_STATUS efi_main(EFI_HANDLE image, EFI_SYSTEM_TABLE *sys_table) { PHYSICAL_ADDRESS_TO_POINTER(linux_base), linux_size, PHYSICAL_ADDRESS_TO_POINTER(initrd_base), initrd_size); graphics_mode(false); - return log_error_status_stall(err, L"Execution of embedded linux image failed: %r", err); + return err; } From dcde6ae16551d126f8cd0c3fb9851bb11ac1b938 Mon Sep 17 00:00:00 2001 From: Jan Janssen Date: Wed, 21 Sep 2022 12:23:36 +0200 Subject: [PATCH 3/6] boot: Remove unused parameters from pe_kernel_info Only the compat entry address is used now. This also now only returns the compat entry address. If the image is native we do not need to try calling into the entry address again as we would already have done so from StartImage (and failed). --- src/boot/efi/boot.c | 11 ++++++----- src/boot/efi/linux.c | 9 ++++----- src/boot/efi/pe.c | 38 +++++++++++++------------------------- src/boot/efi/pe.h | 6 +----- 4 files changed, 24 insertions(+), 40 deletions(-) diff --git a/src/boot/efi/boot.c b/src/boot/efi/boot.c index 6b3d29ba3b..e1a5e861c5 100644 --- a/src/boot/efi/boot.c +++ b/src/boot/efi/boot.c @@ -2416,21 +2416,22 @@ static EFI_STATUS image_start( /* Try calling the kernel compat entry point if one exists. */ if (err == EFI_UNSUPPORTED && entry->type == LOADER_LINUX) { - uint32_t kernel_entry_address; + uint32_t compat_address; - err = pe_kernel_info(loaded_image->ImageBase, &kernel_entry_address, NULL, NULL); + err = pe_kernel_info(loaded_image->ImageBase, &compat_address); if (err != EFI_SUCCESS) { if (err != EFI_UNSUPPORTED) return log_error_status_stall(err, L"Error finding kernel compat entry address: %r", err); - } else { + } else if (compat_address > 0) { EFI_IMAGE_ENTRY_POINT kernel_entry = - (EFI_IMAGE_ENTRY_POINT) ((uint8_t *) loaded_image->ImageBase + kernel_entry_address); + (EFI_IMAGE_ENTRY_POINT) ((uint8_t *) loaded_image->ImageBase + compat_address); err = kernel_entry(image, ST); graphics_mode(false); if (err == EFI_SUCCESS) return EFI_SUCCESS; - } + } else + err = EFI_UNSUPPORTED; } return log_error_status_stall(err, L"Failed to execute %s (%s): %r", entry->title_show, entry->loader, err); diff --git a/src/boot/efi/linux.c b/src/boot/efi/linux.c index 5b062d409a..622869e36c 100644 --- a/src/boot/efi/linux.c +++ b/src/boot/efi/linux.c @@ -59,7 +59,7 @@ EFI_STATUS linux_exec( const void *linux_buffer, UINTN linux_length, const void *initrd_buffer, UINTN initrd_length) { - uint32_t kernel_alignment, kernel_size_of_image, kernel_entry_address = 0; + uint32_t compat_address; EFI_STATUS err; assert(parent); @@ -67,8 +67,7 @@ EFI_STATUS linux_exec( assert(linux_buffer && linux_length > 0); assert(initrd_buffer || initrd_length == 0); - /* get the necessary fields from the PE header */ - err = pe_kernel_info(linux_buffer, &kernel_entry_address, &kernel_size_of_image, &kernel_alignment); + err = pe_kernel_info(linux_buffer, &compat_address); #if defined(__i386__) || defined(__x86_64__) if (err == EFI_UNSUPPORTED) /* Kernel is too old to support LINUX_INITRD_MEDIA_GUID, try the deprecated EFI handover @@ -108,9 +107,9 @@ EFI_STATUS linux_exec( err = BS->StartImage(kernel_image, NULL, NULL); /* Try calling the kernel compat entry point if one exists. */ - if (err == EFI_UNSUPPORTED && kernel_entry_address > 0) { + if (err == EFI_UNSUPPORTED && compat_address > 0) { EFI_IMAGE_ENTRY_POINT compat_entry = - (EFI_IMAGE_ENTRY_POINT) ((uint8_t *) loaded_image->ImageBase + kernel_entry_address); + (EFI_IMAGE_ENTRY_POINT) ((uint8_t *) loaded_image->ImageBase + compat_address); err = compat_entry(kernel_image, ST); } diff --git a/src/boot/efi/pe.c b/src/boot/efi/pe.c index 051ea74d42..3d5da14d10 100644 --- a/src/boot/efi/pe.c +++ b/src/boot/efi/pe.c @@ -215,23 +215,15 @@ static uint32_t get_compatibility_entry_address(const DosFileHeader *dos, const return 0; } -EFI_STATUS pe_kernel_info( - const void *base, - uint32_t *ret_entry_point_address, - uint32_t *ret_size_of_image, - uint32_t *ret_section_alignment) { - - const DosFileHeader *dos; - const PeFileHeader *pe; - +EFI_STATUS pe_kernel_info(const void *base, uint32_t *ret_compat_address) { assert(base); - assert(ret_entry_point_address); + assert(ret_compat_address); - dos = (const DosFileHeader *) base; + const DosFileHeader *dos = (const DosFileHeader *) base; if (!verify_dos(dos)) return EFI_LOAD_ERROR; - pe = (const PeFileHeader *) ((const uint8_t *) base + dos->ExeHeader); + const PeFileHeader *pe = (const PeFileHeader *) ((const uint8_t *) base + dos->ExeHeader); if (!verify_pe(pe, /* allow_compatibility= */ true)) return EFI_LOAD_ERROR; @@ -239,21 +231,17 @@ EFI_STATUS pe_kernel_info( if (pe->OptionalHeader.MajorImageVersion < 1) return EFI_UNSUPPORTED; - uint32_t entry_address = pe->OptionalHeader.AddressOfEntryPoint; - - /* Look for a compat entry point. */ - if (pe->FileHeader.Machine != TARGET_MACHINE_TYPE) { - entry_address = get_compatibility_entry_address(dos, pe); - if (entry_address == 0) - /* Image type not supported and no compat entry found. */ - return EFI_UNSUPPORTED; + if (pe->FileHeader.Machine == TARGET_MACHINE_TYPE) { + *ret_compat_address = 0; + return EFI_SUCCESS; } - *ret_entry_point_address = entry_address; - if (ret_size_of_image) - *ret_size_of_image = pe->OptionalHeader.SizeOfImage; - if (ret_section_alignment) - *ret_section_alignment = pe->OptionalHeader.SectionAlignment; + uint32_t compat_address = get_compatibility_entry_address(dos, pe); + if (compat_address == 0) + /* Image type not supported and no compat entry found. */ + return EFI_UNSUPPORTED; + + *ret_compat_address = compat_address; return EFI_SUCCESS; } diff --git a/src/boot/efi/pe.h b/src/boot/efi/pe.h index ead2ba01cc..ff7ff479ec 100644 --- a/src/boot/efi/pe.h +++ b/src/boot/efi/pe.h @@ -17,8 +17,4 @@ EFI_STATUS pe_file_locate_sections( UINTN *offsets, UINTN *sizes); -EFI_STATUS pe_kernel_info( - const void *base, - uint32_t *ret_entry_point_address, - uint32_t *ret_size_of_image, - uint32_t *ret_section_alignment); +EFI_STATUS pe_kernel_info(const void *base, uint32_t *ret_compat_address); From 0e3c374e8c0dbf3586fa9ac0262c953585456201 Mon Sep 17 00:00:00 2001 From: Jan Janssen Date: Wed, 21 Sep 2022 12:39:46 +0200 Subject: [PATCH 4/6] boot: Use proper security arch protocol names This is how the Platform Intregration Specification defines these. --- src/boot/efi/missing_efi.h | 51 ++++++++++++++++---------------------- src/boot/efi/shim.c | 23 ++++++++++------- 2 files changed, 36 insertions(+), 38 deletions(-) diff --git a/src/boot/efi/missing_efi.h b/src/boot/efi/missing_efi.h index 4e80acca56..f9169248ec 100644 --- a/src/boot/efi/missing_efi.h +++ b/src/boot/efi/missing_efi.h @@ -309,41 +309,34 @@ typedef struct tdEFI_TCG2_PROTOCOL { {0x5568e427, 0x68fc, 0x4f3d, {0xac, 0x74, 0xca, 0x55, 0x52, 0x31, 0xcc, 0x68} } /* UEFI Platform Initialization (Vol2: DXE) */ -#ifndef SECURITY_PROTOCOL_GUID +#ifndef EFI_SECURITY_ARCH_PROTOCOL_GUID -#define SECURITY_PROTOCOL_GUID \ - &(const EFI_GUID) { 0xa46423e3, 0x4617, 0x49f1, { 0xb9, 0xff, 0xd1, 0xbf, 0xa9, 0x11, 0x58, 0x39 } } -#define SECURITY_PROTOCOL2_GUID \ - &(const EFI_GUID) { 0x94ab2f58, 0x1438, 0x4ef1, { 0x91, 0x52, 0x18, 0x94, 0x1a, 0x3a, 0x0e, 0x68 } } +#define EFI_SECURITY_ARCH_PROTOCOL_GUID \ + { 0xa46423e3, 0x4617, 0x49f1, { 0xb9, 0xff, 0xd1, 0xbf, 0xa9, 0x11, 0x58, 0x39 } } +#define EFI_SECURITY2_ARCH_PROTOCOL_GUID \ + { 0x94ab2f58, 0x1438, 0x4ef1, { 0x91, 0x52, 0x18, 0x94, 0x1a, 0x3a, 0x0e, 0x68 } } -struct _EFI_SECURITY2_PROTOCOL; -struct _EFI_SECURITY_PROTOCOL; -struct _EFI_DEVICE_PATH_PROTOCOL; +typedef struct EFI_SECURITY_ARCH_PROTOCOL EFI_SECURITY_ARCH_PROTOCOL; +typedef struct EFI_SECURITY2_ARCH_PROTOCOL EFI_SECURITY2_ARCH_PROTOCOL; -typedef struct _EFI_SECURITY2_PROTOCOL EFI_SECURITY2_PROTOCOL; -typedef struct _EFI_SECURITY_PROTOCOL EFI_SECURITY_PROTOCOL; -typedef struct _EFI_DEVICE_PATH_PROTOCOL EFI_DEVICE_PATH_PROTOCOL; +typedef EFI_STATUS (EFIAPI *EFI_SECURITY_FILE_AUTHENTICATION_STATE)( + const EFI_SECURITY_ARCH_PROTOCOL *This, + uint32_t AuthenticationStatus, + const EFI_DEVICE_PATH *File); -typedef EFI_STATUS (EFIAPI *EFI_SECURITY_FILE_AUTHENTICATION_STATE) ( - const EFI_SECURITY_PROTOCOL *This, - UINT32 AuthenticationStatus, - const EFI_DEVICE_PATH_PROTOCOL *File -); - -typedef EFI_STATUS (EFIAPI *EFI_SECURITY2_FILE_AUTHENTICATION) ( - const EFI_SECURITY2_PROTOCOL *This, - const EFI_DEVICE_PATH_PROTOCOL *DevicePath, - VOID *FileBuffer, - UINTN FileSize, - BOOLEAN BootPolicy -); - -struct _EFI_SECURITY2_PROTOCOL { - EFI_SECURITY2_FILE_AUTHENTICATION FileAuthentication; +struct EFI_SECURITY_ARCH_PROTOCOL { + EFI_SECURITY_FILE_AUTHENTICATION_STATE FileAuthenticationState; }; -struct _EFI_SECURITY_PROTOCOL { - EFI_SECURITY_FILE_AUTHENTICATION_STATE FileAuthenticationState; +typedef EFI_STATUS (EFIAPI *EFI_SECURITY2_FILE_AUTHENTICATION)( + const EFI_SECURITY2_ARCH_PROTOCOL *This, + const EFI_DEVICE_PATH *DevicePath, + void *FileBuffer, + UINTN FileSize, + BOOLEAN BootPolicy); + +struct EFI_SECURITY2_ARCH_PROTOCOL { + EFI_SECURITY2_FILE_AUTHENTICATION FileAuthentication; }; #endif diff --git a/src/boot/efi/shim.c b/src/boot/efi/shim.c index 8b0407857e..79c89c067e 100644 --- a/src/boot/efi/shim.c +++ b/src/boot/efi/shim.c @@ -69,9 +69,12 @@ static EFI_SECURITY2_FILE_AUTHENTICATION es2fa = NULL; * the SB failure code seems to vary from one implementation to another, and I * don't want to interfere with that at this time. */ -static EFIAPI EFI_STATUS security2_policy_authentication (const EFI_SECURITY2_PROTOCOL *this, - const EFI_DEVICE_PATH_PROTOCOL *device_path, - void *file_buffer, UINTN file_size, BOOLEAN boot_policy) { +static EFIAPI EFI_STATUS security2_policy_authentication( + const EFI_SECURITY2_ARCH_PROTOCOL *this, + const EFI_DEVICE_PATH *device_path, + void *file_buffer, + UINTN file_size, + BOOLEAN boot_policy) { EFI_STATUS err; assert(this); @@ -99,8 +102,10 @@ static EFIAPI EFI_STATUS security2_policy_authentication (const EFI_SECURITY2_PR * authentication failure, be it EFI_ACCESS_DENIED, EFI_SECURITY_VIOLATION, or something * else. (This seems to vary between implementations.) */ -static EFIAPI EFI_STATUS security_policy_authentication (const EFI_SECURITY_PROTOCOL *this, uint32_t authentication_status, - const EFI_DEVICE_PATH_PROTOCOL *device_path_const) { +static EFIAPI EFI_STATUS security_policy_authentication( + const EFI_SECURITY_ARCH_PROTOCOL *this, + uint32_t authentication_status, + const EFI_DEVICE_PATH *device_path_const) { EFI_STATUS err; _cleanup_free_ char16_t *dev_path_str = NULL; EFI_HANDLE h; @@ -138,8 +143,8 @@ static EFIAPI EFI_STATUS security_policy_authentication (const EFI_SECURITY_PROT } EFI_STATUS security_policy_install(void) { - EFI_SECURITY_PROTOCOL *security_protocol; - EFI_SECURITY2_PROTOCOL *security2_protocol = NULL; + EFI_SECURITY_ARCH_PROTOCOL *security_protocol; + EFI_SECURITY2_ARCH_PROTOCOL *security2_protocol = NULL; EFI_STATUS err; /* Already Installed */ @@ -151,9 +156,9 @@ EFI_STATUS security_policy_install(void) { * to fail, since SECURITY2 was introduced in PI 1.2.1. * Use security2_protocol == NULL as indicator. */ - BS->LocateProtocol((EFI_GUID*) SECURITY_PROTOCOL2_GUID, NULL, (void**) &security2_protocol); + BS->LocateProtocol(&(EFI_GUID) EFI_SECURITY2_ARCH_PROTOCOL_GUID, NULL, (void **) &security2_protocol); - err = BS->LocateProtocol((EFI_GUID*) SECURITY_PROTOCOL_GUID, NULL, (void**) &security_protocol); + err = BS->LocateProtocol(&(EFI_GUID) EFI_SECURITY_ARCH_PROTOCOL_GUID, NULL, (void**) &security_protocol); /* This one is mandatory, so there's a serious problem */ if (err != EFI_SUCCESS) return err; From 6731a102da4b5827ae10355670c34396e89e265b Mon Sep 17 00:00:00 2001 From: Jan Janssen Date: Wed, 21 Sep 2022 12:56:20 +0200 Subject: [PATCH 5/6] stub: Allow loading unsigned kernel images --- src/boot/efi/linux.c | 57 ++++++++++++++++++++++++++++++-- src/boot/efi/secure-boot.c | 67 ++++++++++++++++++++++++++++++++++++++ src/boot/efi/secure-boot.h | 23 +++++++++++++ 3 files changed, 145 insertions(+), 2 deletions(-) diff --git a/src/boot/efi/linux.c b/src/boot/efi/linux.c index 622869e36c..813e648b6b 100644 --- a/src/boot/efi/linux.c +++ b/src/boot/efi/linux.c @@ -14,17 +14,50 @@ #include "initrd.h" #include "linux.h" #include "pe.h" +#include "secure-boot.h" #include "util.h" #define STUB_PAYLOAD_GUID \ { 0x55c5d1f8, 0x04cd, 0x46b5, { 0x8a, 0x20, 0xe5, 0x6c, 0xbb, 0x30, 0x52, 0xd0 } } +static EFIAPI EFI_STATUS security_hook( + const SecurityOverride *this, uint32_t authentication_status, const EFI_DEVICE_PATH *file) { + + assert(this); + assert(this->hook == security_hook); + + if (file == this->payload_device_path) + return EFI_SUCCESS; + + return this->original_security->FileAuthenticationState( + this->original_security, authentication_status, file); +} + +static EFIAPI EFI_STATUS security2_hook( + const SecurityOverride *this, + const EFI_DEVICE_PATH *device_path, + void *file_buffer, + size_t file_size, + BOOLEAN boot_policy) { + + assert(this); + assert(this->hook == security2_hook); + + if (file_buffer == this->payload && file_size == this->payload_len && + device_path == this->payload_device_path) + return EFI_SUCCESS; + + return this->original_security2->FileAuthentication( + this->original_security2, device_path, file_buffer, file_size, boot_policy); +} + EFI_STATUS load_image(EFI_HANDLE parent, const void *source, size_t len, EFI_HANDLE *ret_image) { assert(parent); assert(source); assert(ret_image); - /* We could pass a NULL device path, but it's nicer to provide something. */ + /* We could pass a NULL device path, but it's nicer to provide something and it allows us to identify + * the loaded image from within the security hooks. */ struct { VENDOR_DEVICE_PATH payload; EFI_DEVICE_PATH end; @@ -44,13 +77,33 @@ EFI_STATUS load_image(EFI_HANDLE parent, const void *source, size_t len, EFI_HAN }, }; - return BS->LoadImage( + /* We want to support unsigned kernel images as payload, which is safe to do under secure boot + * because it is embedded in this stub loader (and since it is already running it must be trusted). */ + SecurityOverride security_override = { + .hook = security_hook, + .payload = source, + .payload_len = len, + .payload_device_path = &payload_device_path.payload.Header, + }, security2_override = { + .hook = security2_hook, + .payload = source, + .payload_len = len, + .payload_device_path = &payload_device_path.payload.Header, + }; + + install_security_override(&security_override, &security2_override); + + EFI_STATUS ret = BS->LoadImage( /*BootPolicy=*/false, parent, &payload_device_path.payload.Header, (void *) source, len, ret_image); + + uninstall_security_override(&security_override, &security2_override); + + return ret; } EFI_STATUS linux_exec( diff --git a/src/boot/efi/secure-boot.c b/src/boot/efi/secure-boot.c index cf7a464d0a..6a5c2a9bea 100644 --- a/src/boot/efi/secure-boot.c +++ b/src/boot/efi/secure-boot.c @@ -126,3 +126,70 @@ out_deallocate: return err; } + +static EFI_STATUS install_security_override_one(EFI_GUID guid, SecurityOverride *override) { + EFI_STATUS err; + + assert(override); + + _cleanup_free_ EFI_HANDLE *handles = NULL; + size_t n_handles = 0; + + err = BS->LocateHandleBuffer(ByProtocol, &guid, NULL, &n_handles, &handles); + if (err != EFI_SUCCESS) + /* No security arch protocol around? */ + return err; + + /* There should only ever be one security arch protocol instance, but let's be paranoid here. */ + assert(n_handles == 1); + + void *security = NULL; + err = BS->LocateProtocol(&guid, NULL, &security); + if (err != EFI_SUCCESS) + return log_error_status_stall(err, u"Error getting security arch protocol: %r", err); + + err = BS->ReinstallProtocolInterface(handles[0], &guid, security, override); + if (err != EFI_SUCCESS) + return log_error_status_stall(err, u"Error overriding security arch protocol: %r", err); + + override->original = security; + override->original_handle = handles[0]; + return EFI_SUCCESS; +} + +/* This replaces the platform provided security arch protocols (defined in the UEFI Platform Initialization + * Specification) with the provided override instances. If not running in secure boot or the protocols are + * not available nothing happens. The override instances are provided with the neccessary info to undo this + * in uninstall_security_override(). */ +void install_security_override(SecurityOverride *override, SecurityOverride *override2) { + assert(override); + assert(override2); + + if (!secure_boot_enabled()) + return; + + (void) install_security_override_one((EFI_GUID) EFI_SECURITY_ARCH_PROTOCOL_GUID, override); + (void) install_security_override_one((EFI_GUID) EFI_SECURITY2_ARCH_PROTOCOL_GUID, override2); +} + +void uninstall_security_override(SecurityOverride *override, SecurityOverride *override2) { + assert(override); + assert(override2); + + /* We use assert_se here to guarantee the system is not in a weird state in the unlikely case of an + * error restoring the original protocols. */ + + if (override->original_handle) + assert_se(BS->ReinstallProtocolInterface( + override->original_handle, + &(EFI_GUID) EFI_SECURITY_ARCH_PROTOCOL_GUID, + override, + override->original) == EFI_SUCCESS); + + if (override2->original_handle) + assert_se(BS->ReinstallProtocolInterface( + override2->original_handle, + &(EFI_GUID) EFI_SECURITY2_ARCH_PROTOCOL_GUID, + override2, + override2->original) == EFI_SUCCESS); +} diff --git a/src/boot/efi/secure-boot.h b/src/boot/efi/secure-boot.h index ff434ed1ad..91b6770edb 100644 --- a/src/boot/efi/secure-boot.h +++ b/src/boot/efi/secure-boot.h @@ -2,7 +2,9 @@ #pragma once #include + #include "efivars-fundamental.h" +#include "missing_efi.h" typedef enum { ENROLL_OFF, /* no Secure Boot key enrollment whatsoever, even manual entries are not generated */ @@ -14,3 +16,24 @@ bool secure_boot_enabled(void); SecureBootMode secure_boot_mode(void); EFI_STATUS secure_boot_enroll_at(EFI_FILE *root_dir, const char16_t *path); + +typedef struct { + void *hook; + + /* End of EFI_SECURITY_ARCH(2)_PROTOCOL. The rest is our own protocol instance data. */ + + EFI_HANDLE original_handle; + union { + void *original; + EFI_SECURITY_ARCH_PROTOCOL *original_security; + EFI_SECURITY2_ARCH_PROTOCOL *original_security2; + }; + + /* Used by the stub to identify the embedded image. */ + const void *payload; + size_t payload_len; + const EFI_DEVICE_PATH *payload_device_path; +} SecurityOverride; + +void install_security_override(SecurityOverride *override, SecurityOverride *override2); +void uninstall_security_override(SecurityOverride *override, SecurityOverride *override2); From 09da51f8e98c18278d27a3fddb006a6c75f3227c Mon Sep 17 00:00:00 2001 From: Jan Janssen Date: Fri, 14 Oct 2022 11:09:12 +0200 Subject: [PATCH 6/6] boot: Rework shim image verification This moves the shim security arch override to the new ReinstallProtocolInterface based interface. This also has the benefit to reduce the time window in which we have this override active and also actually removes it, which was not previously done. The shim hooks themselves are also modernized too. The upcalls should really not be neccessary if shim is happy with the provided binary. --- src/boot/efi/boot.c | 8 +-- src/boot/efi/shim.c | 116 ++++++++++++++++---------------------------- src/boot/efi/shim.h | 4 +- 3 files changed, 44 insertions(+), 84 deletions(-) diff --git a/src/boot/efi/boot.c b/src/boot/efi/boot.c index e1a5e861c5..4150b16ecf 100644 --- a/src/boot/efi/boot.c +++ b/src/boot/efi/boot.c @@ -2379,7 +2379,7 @@ static EFI_STATUS image_start( if (err != EFI_SUCCESS) return log_error_status_stall(err, L"Error preparing initrd: %r", err); - err = BS->LoadImage(false, parent_image, path, NULL, 0, &image); + err = shim_load_image(parent_image, path, &image); if (err != EFI_SUCCESS) return log_error_status_stall(err, L"Error loading %s: %r", entry->loader, err); @@ -2686,12 +2686,6 @@ EFI_STATUS efi_main(EFI_HANDLE image, EFI_SYSTEM_TABLE *sys_table) { if (err != EFI_SUCCESS) return log_error_status_stall(err, L"Unable to open root directory: %r", err); - if (secure_boot_enabled() && shim_loaded()) { - err = security_policy_install(); - if (err != EFI_SUCCESS) - return log_error_status_stall(err, L"Error installing security policy: %r", err); - } - (void) load_drivers(image, loaded_image, root_dir); config_load_all_entries(&config, loaded_image, loaded_image_path, root_dir); diff --git a/src/boot/efi/shim.c b/src/boot/efi/shim.c index 79c89c067e..3ae058cb84 100644 --- a/src/boot/efi/shim.c +++ b/src/boot/efi/shim.c @@ -13,6 +13,7 @@ #include "missing_efi.h" #include "util.h" +#include "secure-boot.h" #include "shim.h" #if defined(__x86_64__) || defined(__i386__) @@ -55,121 +56,86 @@ static bool shim_validate(void *data, uint32_t size) { return shim_lock->shim_verify(data, size) == EFI_SUCCESS; } -/* Handle to the original authenticator for security1 protocol */ -static EFI_SECURITY_FILE_AUTHENTICATION_STATE esfas = NULL; - -/* Handle to the original authenticator for security2 protocol */ -static EFI_SECURITY2_FILE_AUTHENTICATION es2fa = NULL; - -/* - * Perform shim/MOK and Secure Boot authentication on a binary that's already been - * loaded into memory. This function does the platform SB authentication first - * but preserves its return value in case of its failure, so that it can be - * returned in case of a shim/MOK authentication failure. This is done because - * the SB failure code seems to vary from one implementation to another, and I - * don't want to interfere with that at this time. - */ -static EFIAPI EFI_STATUS security2_policy_authentication( - const EFI_SECURITY2_ARCH_PROTOCOL *this, +static EFIAPI EFI_STATUS security2_hook( + const SecurityOverride *this, const EFI_DEVICE_PATH *device_path, void *file_buffer, UINTN file_size, BOOLEAN boot_policy) { - EFI_STATUS err; assert(this); - /* device_path and file_buffer may be NULL */ - - /* Chain original security policy */ - err = es2fa(this, device_path, file_buffer, file_size, boot_policy); - - /* if OK, don't bother with MOK check */ - if (err == EFI_SUCCESS) - return err; + assert(this->hook == security2_hook); if (shim_validate(file_buffer, file_size)) return EFI_SUCCESS; - return err; + return this->original_security2->FileAuthentication( + this->original_security2, device_path, file_buffer, file_size, boot_policy); } -/* - * Perform both shim/MOK and platform Secure Boot authentication. This function loads - * the file and performs shim/MOK authentication first simply to avoid double loads - * of Linux kernels, which are much more likely to be shim/MOK-signed than platform-signed, - * since kernels are big and can take several seconds to load on some computers and - * filesystems. This also has the effect of returning whatever the platform code is for - * authentication failure, be it EFI_ACCESS_DENIED, EFI_SECURITY_VIOLATION, or something - * else. (This seems to vary between implementations.) - */ -static EFIAPI EFI_STATUS security_policy_authentication( - const EFI_SECURITY_ARCH_PROTOCOL *this, +static EFIAPI EFI_STATUS security_hook( + const SecurityOverride *this, uint32_t authentication_status, - const EFI_DEVICE_PATH *device_path_const) { + const EFI_DEVICE_PATH *device_path) { + EFI_STATUS err; - _cleanup_free_ char16_t *dev_path_str = NULL; - EFI_HANDLE h; - _cleanup_free_ char *file_buffer = NULL; - UINTN file_size; assert(this); + assert(this->hook == security_hook); - if (!device_path_const) - return EFI_INVALID_PARAMETER; + if (!device_path) + return this->original_security->FileAuthenticationState( + this->original_security, authentication_status, device_path); - EFI_DEVICE_PATH *dp = (EFI_DEVICE_PATH *) device_path_const; - err = BS->LocateDevicePath(&FileSystemProtocol, &dp, &h); + EFI_HANDLE device_handle; + EFI_DEVICE_PATH *dp = (EFI_DEVICE_PATH *) device_path; + err = BS->LocateDevicePath(&FileSystemProtocol, &dp, &device_handle); if (err != EFI_SUCCESS) return err; _cleanup_(file_closep) EFI_FILE *root = NULL; - err = open_volume(h, &root); + err = open_volume(device_handle, &root); if (err != EFI_SUCCESS) return err; - err = device_path_to_str(dp, &dev_path_str); + _cleanup_free_ char16_t *dp_str = NULL; + err = device_path_to_str(dp, &dp_str); if (err != EFI_SUCCESS) return err; - err = file_read(root, dev_path_str, 0, 0, &file_buffer, &file_size); + char *file_buffer; + size_t file_size; + err = file_read(root, dp_str, 0, 0, &file_buffer, &file_size); if (err != EFI_SUCCESS) return err; if (shim_validate(file_buffer, file_size)) return EFI_SUCCESS; - /* Try using the platform's native policy.... */ - return esfas(this, authentication_status, device_path_const); + return this->original_security->FileAuthenticationState( + this->original_security, authentication_status, device_path); } -EFI_STATUS security_policy_install(void) { - EFI_SECURITY_ARCH_PROTOCOL *security_protocol; - EFI_SECURITY2_ARCH_PROTOCOL *security2_protocol = NULL; - EFI_STATUS err; +EFI_STATUS shim_load_image(EFI_HANDLE parent, const EFI_DEVICE_PATH *device_path, EFI_HANDLE *ret_image) { + assert(device_path); + assert(ret_image); - /* Already Installed */ - if (esfas) - return EFI_ALREADY_STARTED; + bool have_shim = shim_loaded(); - /* - * Don't bother with status here. The call is allowed - * to fail, since SECURITY2 was introduced in PI 1.2.1. - * Use security2_protocol == NULL as indicator. - */ - BS->LocateProtocol(&(EFI_GUID) EFI_SECURITY2_ARCH_PROTOCOL_GUID, NULL, (void **) &security2_protocol); + SecurityOverride security_override = { + .hook = security_hook, + }, security2_override = { + .hook = security2_hook, + }; - err = BS->LocateProtocol(&(EFI_GUID) EFI_SECURITY_ARCH_PROTOCOL_GUID, NULL, (void**) &security_protocol); - /* This one is mandatory, so there's a serious problem */ - if (err != EFI_SUCCESS) - return err; + if (have_shim) + install_security_override(&security_override, &security2_override); - esfas = security_protocol->FileAuthenticationState; - security_protocol->FileAuthenticationState = security_policy_authentication; + EFI_STATUS ret = BS->LoadImage( + /*BootPolicy=*/false, parent, (EFI_DEVICE_PATH *) device_path, NULL, 0, ret_image); - if (security2_protocol) { - es2fa = security2_protocol->FileAuthentication; - security2_protocol->FileAuthentication = security2_policy_authentication; - } + if (have_shim) + uninstall_security_override(&security_override, &security2_override); - return EFI_SUCCESS; + return ret; } diff --git a/src/boot/efi/shim.h b/src/boot/efi/shim.h index ddbc88c52c..6d213f5efa 100644 --- a/src/boot/efi/shim.h +++ b/src/boot/efi/shim.h @@ -10,7 +10,7 @@ #pragma once #include +#include bool shim_loaded(void); - -EFI_STATUS security_policy_install(void); +EFI_STATUS shim_load_image(EFI_HANDLE parent, const EFI_DEVICE_PATH *device_path, EFI_HANDLE *ret_image);