From e7b3598311c4b002417ac6364093cfab218ced88 Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Fri, 30 Jun 2023 13:24:57 -0400 Subject: Move some stuff around This moves some things around to help with loader protocol changes: - Move replacements.c to loader-proto.c - likewise with replacements.h - move the SHIM_IMAGE_LOADER decl to loader-proto.h - move the LoadImage / StartImage interface setup to an init function - move shim_load_image() / shim_start_image() to loader-proto.c Signed-off-by: Peter Jones --- loader-proto.c | 325 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 325 insertions(+) create mode 100644 loader-proto.c (limited to 'loader-proto.c') diff --git a/loader-proto.c b/loader-proto.c new file mode 100644 index 00000000..a61a91db --- /dev/null +++ b/loader-proto.c @@ -0,0 +1,325 @@ +// SPDX-License-Identifier: BSD-2-Clause-Patent +/* + * loader-proto.c - shim's loader protocol + * + * Copyright Red Hat, Inc + */ + +/* Chemical agents lend themselves to covert use in sabotage against + * which it is exceedingly difficult to visualize any really effective + * defense... I will not dwell upon this use of CBW because, as one + * pursues the possibilities of such covert uses, one discovers that the + * scenarios resemble that in which the components of a nuclear weapon + * are smuggled into New York City and assembled in the basement of the + * Empire State Building. + * In other words, once the possibility is recognized to exist, about + * all that one can do is worry about it. + * -- Dr. Ivan L Bennett, Jr., testifying before the Subcommittee on + * National Security Policy and Scientific Developments, November 20, + * 1969. + */ +#include "shim.h" + +static EFI_SYSTEM_TABLE *systab; + +EFI_SYSTEM_TABLE * +get_active_systab(void) +{ + if (systab) + return systab; + return ST; +} + +static typeof(systab->BootServices->LoadImage) system_load_image; +static typeof(systab->BootServices->StartImage) system_start_image; +static typeof(systab->BootServices->Exit) system_exit; +#if !defined(DISABLE_EBS_PROTECTION) +static typeof(systab->BootServices->ExitBootServices) system_exit_boot_services; +#endif /* !defined(DISABLE_EBS_PROTECTION) */ + +static EFI_HANDLE last_loaded_image; + +void +unhook_system_services(void) +{ + if (!systab) + return; + + systab->BootServices->LoadImage = system_load_image; + systab->BootServices->StartImage = system_start_image; +#if !defined(DISABLE_EBS_PROTECTION) + systab->BootServices->ExitBootServices = system_exit_boot_services; +#endif /* !defined(DISABLE_EBS_PROTECTION) */ + BS = systab->BootServices; +} + +static EFI_STATUS EFIAPI +shim_load_image(BOOLEAN BootPolicy, EFI_HANDLE ParentImageHandle, + EFI_DEVICE_PATH *FilePath, VOID *SourceBuffer, + UINTN SourceSize, EFI_HANDLE *ImageHandle) +{ + SHIM_LOADED_IMAGE *image; + EFI_STATUS efi_status; + + (void)FilePath; + + if (BootPolicy || !SourceBuffer || !SourceSize) + return EFI_UNSUPPORTED; + + image = AllocatePool(sizeof(*image)); + if (!image) + return EFI_OUT_OF_RESOURCES; + + SetMem(image, sizeof(*image), 0); + + image->li.Revision = 0x1000; + image->li.ParentHandle = ParentImageHandle; + image->li.SystemTable = systab; + + efi_status = handle_image(SourceBuffer, SourceSize, &image->li, + &image->entry_point, &image->alloc_address, + &image->alloc_pages); + if (EFI_ERROR(efi_status)) + goto free_image; + + *ImageHandle = NULL; + efi_status = BS->InstallMultipleProtocolInterfaces(ImageHandle, + &SHIM_LOADED_IMAGE_GUID, image, + &EFI_LOADED_IMAGE_GUID, &image->li, + NULL); + if (EFI_ERROR(efi_status)) + goto free_alloc; + + return EFI_SUCCESS; + +free_alloc: + BS->FreePages(image->alloc_address, image->alloc_pages); +free_image: + FreePool(image); + return efi_status; +} + +static EFI_STATUS EFIAPI +shim_start_image(IN EFI_HANDLE ImageHandle, OUT UINTN *ExitDataSize, + OUT CHAR16 **ExitData OPTIONAL) +{ + SHIM_LOADED_IMAGE *image; + EFI_STATUS efi_status; + + efi_status = BS->HandleProtocol(ImageHandle, &SHIM_LOADED_IMAGE_GUID, + (void **)&image); + if (EFI_ERROR(efi_status) || image->started) + return EFI_INVALID_PARAMETER; + + if (!setjmp(image->longjmp_buf)) { + image->started = true; + efi_status = + image->entry_point(ImageHandle, image->li.SystemTable); + } else { + if (ExitData) { + *ExitDataSize = image->exit_data_size; + *ExitData = (CHAR16 *)image->exit_data; + } + efi_status = image->exit_status; + } + + // + // We only support EFI applications, so we can unload and free the + // image unconditionally. + // + BS->UninstallMultipleProtocolInterfaces(ImageHandle, + &EFI_LOADED_IMAGE_GUID, image, + &SHIM_LOADED_IMAGE_GUID, &image->li, + NULL); + + BS->FreePages(image->alloc_address, image->alloc_pages); + FreePool(image); + + return efi_status; +} + +static EFI_STATUS EFIAPI +load_image(BOOLEAN BootPolicy, EFI_HANDLE ParentImageHandle, + EFI_DEVICE_PATH *DevicePath, VOID *SourceBuffer, + UINTN SourceSize, EFI_HANDLE *ImageHandle) +{ + EFI_STATUS efi_status; + + unhook_system_services(); + efi_status = BS->LoadImage(BootPolicy, ParentImageHandle, DevicePath, + SourceBuffer, SourceSize, ImageHandle); + hook_system_services(systab); + if (EFI_ERROR(efi_status)) + last_loaded_image = NULL; + else + last_loaded_image = *ImageHandle; + return efi_status; +} + +static EFI_STATUS EFIAPI +replacement_start_image(EFI_HANDLE image_handle, UINTN *exit_data_size, CHAR16 **exit_data) +{ + EFI_STATUS efi_status; + unhook_system_services(); + + if (image_handle == last_loaded_image) { + UINT8 retain_protocol = 0; + UINTN retain_protocol_size = sizeof(retain_protocol); + UINT32 retain_protocol_attrs = 0; + + loader_is_participating = 1; + + /* If a boot component asks us, keep our protocol around - it will be used to + * validate further PE payloads (e.g.: by the UKI stub, before the kernel is booted). + * But also check that the variable was set by a boot component, to ensure that + * nobody at runtime can attempt to change shim's behaviour. */ + efi_status = RT->GetVariable(SHIM_RETAIN_PROTOCOL_VAR_NAME, + &SHIM_LOCK_GUID, + &retain_protocol_attrs, + &retain_protocol_size, + &retain_protocol); + if (EFI_ERROR(efi_status) || + (retain_protocol_attrs & EFI_VARIABLE_NON_VOLATILE) || + !(retain_protocol_attrs & EFI_VARIABLE_BOOTSERVICE_ACCESS) || + retain_protocol_size != sizeof(retain_protocol) || + retain_protocol == 0) + uninstall_shim_protocols(); + } + efi_status = BS->StartImage(image_handle, exit_data_size, exit_data); + if (EFI_ERROR(efi_status)) { + if (image_handle == last_loaded_image) { + EFI_STATUS efi_status2 = install_shim_protocols(); + + if (EFI_ERROR(efi_status2)) { + console_print(L"Something has gone seriously wrong: %r\n", + efi_status2); + console_print(L"shim cannot continue, sorry.\n"); + usleep(5000000); + RT->ResetSystem(EfiResetShutdown, + EFI_SECURITY_VIOLATION, + 0, NULL); + } + } + hook_system_services(systab); + loader_is_participating = 0; + } + return efi_status; +} + +#if !defined(DISABLE_EBS_PROTECTION) +static EFI_STATUS EFIAPI +exit_boot_services(EFI_HANDLE image_key, UINTN map_key) +{ + if (loader_is_participating || + verification_method == VERIFIED_BY_HASH) { + unhook_system_services(); + EFI_STATUS efi_status; + efi_status = BS->ExitBootServices(image_key, map_key); + if (EFI_ERROR(efi_status)) + hook_system_services(systab); + return efi_status; + } + + console_print(L"Bootloader has not verified loaded image.\n"); + console_print(L"System is compromised. halting.\n"); + usleep(5000000); + RT->ResetSystem(EfiResetShutdown, EFI_SECURITY_VIOLATION, 0, NULL); + return EFI_SECURITY_VIOLATION; +} +#endif /* !defined(DISABLE_EBS_PROTECTION) */ + +static EFI_STATUS EFIAPI +do_exit(EFI_HANDLE ImageHandle, EFI_STATUS ExitStatus, + UINTN ExitDataSize, CHAR16 *ExitData) +{ + EFI_STATUS efi_status; + SHIM_LOADED_IMAGE *image; + + efi_status = BS->HandleProtocol(ImageHandle, &SHIM_LOADED_IMAGE_GUID, + (void **)&image); + if (!EFI_ERROR(efi_status)) { + image->exit_status = ExitStatus; + image->exit_data_size = ExitDataSize; + image->exit_data = ExitData; + + longjmp(image->longjmp_buf, 1); + } + + shim_fini(); + + restore_loaded_image(); + + efi_status = BS->Exit(ImageHandle, ExitStatus, + ExitDataSize, ExitData); + if (EFI_ERROR(efi_status)) { + EFI_STATUS efi_status2 = shim_init(); + + if (EFI_ERROR(efi_status2)) { + console_print(L"Something has gone seriously wrong: %r\n", + efi_status2); + console_print(L"shim cannot continue, sorry.\n"); + usleep(5000000); + RT->ResetSystem(EfiResetShutdown, + EFI_SECURITY_VIOLATION, 0, NULL); + } + } + return efi_status; +} + +void +init_image_loader(void) +{ + shim_image_loader_interface.LoadImage = shim_load_image; + shim_image_loader_interface.StartImage = shim_start_image; +} + +void +hook_system_services(EFI_SYSTEM_TABLE *local_systab) +{ + systab = local_systab; + BS = systab->BootServices; + + /* We need to hook various calls to make this work... */ + + /* We need LoadImage() hooked so that fallback.c can load shim + * without having to fake LoadImage as well. This allows it + * to call the system LoadImage(), and have us track the output + * and mark loader_is_participating in replacement_start_image. This + * means anything added by fallback has to be verified by the system + * db, which we want to preserve anyway, since that's all launching + * through BDS gives us. */ + system_load_image = systab->BootServices->LoadImage; + systab->BootServices->LoadImage = load_image; + + /* we need StartImage() so that we can allow chain booting to an + * image trusted by the firmware */ + system_start_image = systab->BootServices->StartImage; + systab->BootServices->StartImage = replacement_start_image; + +#if !defined(DISABLE_EBS_PROTECTION) + /* we need to hook ExitBootServices() so a) we can enforce the policy + * and b) we can unwrap when we're done. */ + system_exit_boot_services = systab->BootServices->ExitBootServices; + systab->BootServices->ExitBootServices = exit_boot_services; +#endif /* defined(DISABLE_EBS_PROTECTION) */ +} + +void +unhook_exit(void) +{ + systab->BootServices->Exit = system_exit; + BS = systab->BootServices; +} + +void +hook_exit(EFI_SYSTEM_TABLE *local_systab) +{ + systab = local_systab; + BS = local_systab->BootServices; + + /* we need to hook Exit() so that we can allow users to quit the + * bootloader and still e.g. start a new one or run an internal + * shell. */ + system_exit = systab->BootServices->Exit; + systab->BootServices->Exit = do_exit; +} -- cgit v1.2.3 From 0322e10ecc0eb6a4acbea3f83f71b19a559aaec6 Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Fri, 30 Jun 2023 14:48:17 -0400 Subject: Implement the rest of the loader protocol functions This adds an implementation of Exit() and UnloadImage(), removes the whole "loader_is_participating" mechanism and its supporting code, and removes DISABLE_EBS_PROTECTION. Signed-off-by: Peter Jones --- BUILDING | 9 --- Make.defaults | 4 - globals.c | 1 - include/loader-proto.h | 3 +- loader-proto.c | 202 +++++++++++++++++-------------------------------- shim.c | 4 - 6 files changed, 70 insertions(+), 153 deletions(-) (limited to 'loader-proto.c') diff --git a/BUILDING b/BUILDING index bdd98dd4..5005868a 100644 --- a/BUILDING +++ b/BUILDING @@ -37,15 +37,6 @@ Variables you could set to customize the build: debugger only on the development branch and not the OS you need to boot to scp in a new development build. Likewise, we look for SHIM_DEVEL_VERBOSE rather than SHIM_VERBOSE. -- DISABLE_EBS_PROTECTION - On systems where a second stage bootloader is not used, and the Linux - Kernel is embedded in the same EFI image as shim and booted directly - from shim, shim's ExitBootServices() hook can cause problems as the - kernel never calls the shim's verification protocol. In this case - calling the shim verification protocol is unnecessary and redundant as - shim has already verified the kernel when shim loaded the kernel as the - second stage loader. In such a case, and only in this case, you should - use DISABLE_EBS_PROTECTION=y to build. - DISABLE_REMOVABLE_LOAD_OPTIONS Do not parse load options when invoked as boot*.efi. This prevents boot failures because of unexpected data in boot entries automatically generated diff --git a/Make.defaults b/Make.defaults index ab11e838..c5fa32be 100644 --- a/Make.defaults +++ b/Make.defaults @@ -149,10 +149,6 @@ ifneq ($(origin REQUIRE_TPM), undefined) DEFINES += -DREQUIRE_TPM endif -ifneq ($(origin DISABLE_EBS_PROTECTION), undefined) - DEFINES += -DDISABLE_EBS_PROTECTION -endif - ifneq ($(origin DISABLE_REMOVABLE_LOAD_OPTIONS), undefined) DEFINES += -DDISABLE_REMOVABLE_LOAD_OPTIONS endif diff --git a/globals.c b/globals.c index 91916e98..d574df16 100644 --- a/globals.c +++ b/globals.c @@ -24,7 +24,6 @@ UINT8 *build_cert; * indicator of how an image has been verified */ verification_method_t verification_method; -int loader_is_participating; SHIM_IMAGE_LOADER shim_image_loader_interface; diff --git a/include/loader-proto.h b/include/loader-proto.h index d3afa2f5..db8e670e 100644 --- a/include/loader-proto.h +++ b/include/loader-proto.h @@ -16,7 +16,6 @@ typedef enum { } verification_method_t; extern verification_method_t verification_method; -extern int loader_is_participating; extern void hook_system_services(EFI_SYSTEM_TABLE *local_systab); extern void unhook_system_services(void); @@ -27,6 +26,8 @@ extern void unhook_exit(void); typedef struct _SHIM_IMAGE_LOADER { EFI_IMAGE_LOAD LoadImage; EFI_IMAGE_START StartImage; + EFI_EXIT Exit; + EFI_IMAGE_UNLOAD UnloadImage; } SHIM_IMAGE_LOADER; extern SHIM_IMAGE_LOADER shim_image_loader_interface; diff --git a/loader-proto.c b/loader-proto.c index a61a91db..f0df122c 100644 --- a/loader-proto.c +++ b/loader-proto.c @@ -32,12 +32,8 @@ get_active_systab(void) static typeof(systab->BootServices->LoadImage) system_load_image; static typeof(systab->BootServices->StartImage) system_start_image; +static typeof(systab->BootServices->UnloadImage) system_unload_image; static typeof(systab->BootServices->Exit) system_exit; -#if !defined(DISABLE_EBS_PROTECTION) -static typeof(systab->BootServices->ExitBootServices) system_exit_boot_services; -#endif /* !defined(DISABLE_EBS_PROTECTION) */ - -static EFI_HANDLE last_loaded_image; void unhook_system_services(void) @@ -47,9 +43,8 @@ unhook_system_services(void) systab->BootServices->LoadImage = system_load_image; systab->BootServices->StartImage = system_start_image; -#if !defined(DISABLE_EBS_PROTECTION) - systab->BootServices->ExitBootServices = system_exit_boot_services; -#endif /* !defined(DISABLE_EBS_PROTECTION) */ + systab->BootServices->Exit = system_exit; + systab->BootServices->UnloadImage = system_unload_image; BS = systab->BootServices; } @@ -108,6 +103,14 @@ shim_start_image(IN EFI_HANDLE ImageHandle, OUT UINTN *ExitDataSize, efi_status = BS->HandleProtocol(ImageHandle, &SHIM_LOADED_IMAGE_GUID, (void **)&image); + + /* + * This image didn't come from shim_load_image(), so it must have come + * from something before shim was involved. + */ + if (efi_status == EFI_UNSUPPORTED) + return system_start_image(ImageHandle, ExitDataSize, ExitData); + if (EFI_ERROR(efi_status) || image->started) return EFI_INVALID_PARAMETER; @@ -139,131 +142,54 @@ shim_start_image(IN EFI_HANDLE ImageHandle, OUT UINTN *ExitDataSize, } static EFI_STATUS EFIAPI -load_image(BOOLEAN BootPolicy, EFI_HANDLE ParentImageHandle, - EFI_DEVICE_PATH *DevicePath, VOID *SourceBuffer, - UINTN SourceSize, EFI_HANDLE *ImageHandle) +shim_unload_image(EFI_HANDLE ImageHandle) { + SHIM_LOADED_IMAGE *image; EFI_STATUS efi_status; - unhook_system_services(); - efi_status = BS->LoadImage(BootPolicy, ParentImageHandle, DevicePath, - SourceBuffer, SourceSize, ImageHandle); - hook_system_services(systab); - if (EFI_ERROR(efi_status)) - last_loaded_image = NULL; - else - last_loaded_image = *ImageHandle; - return efi_status; -} + efi_status = BS->HandleProtocol(ImageHandle, &SHIM_LOADED_IMAGE_GUID, + (void **)&image); -static EFI_STATUS EFIAPI -replacement_start_image(EFI_HANDLE image_handle, UINTN *exit_data_size, CHAR16 **exit_data) -{ - EFI_STATUS efi_status; - unhook_system_services(); - - if (image_handle == last_loaded_image) { - UINT8 retain_protocol = 0; - UINTN retain_protocol_size = sizeof(retain_protocol); - UINT32 retain_protocol_attrs = 0; - - loader_is_participating = 1; - - /* If a boot component asks us, keep our protocol around - it will be used to - * validate further PE payloads (e.g.: by the UKI stub, before the kernel is booted). - * But also check that the variable was set by a boot component, to ensure that - * nobody at runtime can attempt to change shim's behaviour. */ - efi_status = RT->GetVariable(SHIM_RETAIN_PROTOCOL_VAR_NAME, - &SHIM_LOCK_GUID, - &retain_protocol_attrs, - &retain_protocol_size, - &retain_protocol); - if (EFI_ERROR(efi_status) || - (retain_protocol_attrs & EFI_VARIABLE_NON_VOLATILE) || - !(retain_protocol_attrs & EFI_VARIABLE_BOOTSERVICE_ACCESS) || - retain_protocol_size != sizeof(retain_protocol) || - retain_protocol == 0) - uninstall_shim_protocols(); - } - efi_status = BS->StartImage(image_handle, exit_data_size, exit_data); - if (EFI_ERROR(efi_status)) { - if (image_handle == last_loaded_image) { - EFI_STATUS efi_status2 = install_shim_protocols(); - - if (EFI_ERROR(efi_status2)) { - console_print(L"Something has gone seriously wrong: %r\n", - efi_status2); - console_print(L"shim cannot continue, sorry.\n"); - usleep(5000000); - RT->ResetSystem(EfiResetShutdown, - EFI_SECURITY_VIOLATION, - 0, NULL); - } - } - hook_system_services(systab); - loader_is_participating = 0; - } - return efi_status; -} + if (efi_status == EFI_UNSUPPORTED) + return system_unload_image(ImageHandle); -#if !defined(DISABLE_EBS_PROTECTION) -static EFI_STATUS EFIAPI -exit_boot_services(EFI_HANDLE image_key, UINTN map_key) -{ - if (loader_is_participating || - verification_method == VERIFIED_BY_HASH) { - unhook_system_services(); - EFI_STATUS efi_status; - efi_status = BS->ExitBootServices(image_key, map_key); - if (EFI_ERROR(efi_status)) - hook_system_services(systab); - return efi_status; - } + BS->FreePages(image->alloc_address, image->alloc_pages); + FreePool(image); - console_print(L"Bootloader has not verified loaded image.\n"); - console_print(L"System is compromised. halting.\n"); - usleep(5000000); - RT->ResetSystem(EfiResetShutdown, EFI_SECURITY_VIOLATION, 0, NULL); - return EFI_SECURITY_VIOLATION; + return EFI_SUCCESS; } -#endif /* !defined(DISABLE_EBS_PROTECTION) */ static EFI_STATUS EFIAPI -do_exit(EFI_HANDLE ImageHandle, EFI_STATUS ExitStatus, - UINTN ExitDataSize, CHAR16 *ExitData) +shim_exit(EFI_HANDLE ImageHandle, + EFI_STATUS ExitStatus, + UINTN ExitDataSize, + CHAR16 *ExitData) { EFI_STATUS efi_status; SHIM_LOADED_IMAGE *image; efi_status = BS->HandleProtocol(ImageHandle, &SHIM_LOADED_IMAGE_GUID, (void **)&image); - if (!EFI_ERROR(efi_status)) { - image->exit_status = ExitStatus; - image->exit_data_size = ExitDataSize; - image->exit_data = ExitData; - longjmp(image->longjmp_buf, 1); + /* + * If this happens, something above us on the stack of running + * applications called Exit(), and we're getting aborted along with + * it. + */ + if (efi_status == EFI_UNSUPPORTED) { + shim_fini(); + return system_exit(ImageHandle, ExitStatus, ExitDataSize, + ExitData); } - shim_fini(); - - restore_loaded_image(); + if (EFI_ERROR(efi_status)) + return efi_status; - efi_status = BS->Exit(ImageHandle, ExitStatus, - ExitDataSize, ExitData); - if (EFI_ERROR(efi_status)) { - EFI_STATUS efi_status2 = shim_init(); + image->exit_status = ExitStatus; + image->exit_data_size = ExitDataSize; + image->exit_data = ExitData; - if (EFI_ERROR(efi_status2)) { - console_print(L"Something has gone seriously wrong: %r\n", - efi_status2); - console_print(L"shim cannot continue, sorry.\n"); - usleep(5000000); - RT->ResetSystem(EfiResetShutdown, - EFI_SECURITY_VIOLATION, 0, NULL); - } - } - return efi_status; + longjmp(image->longjmp_buf, 1); } void @@ -271,6 +197,8 @@ init_image_loader(void) { shim_image_loader_interface.LoadImage = shim_load_image; shim_image_loader_interface.StartImage = shim_start_image; + shim_image_loader_interface.Exit = shim_exit; + shim_image_loader_interface.UnloadImage = shim_unload_image; } void @@ -281,27 +209,31 @@ hook_system_services(EFI_SYSTEM_TABLE *local_systab) /* We need to hook various calls to make this work... */ - /* We need LoadImage() hooked so that fallback.c can load shim - * without having to fake LoadImage as well. This allows it - * to call the system LoadImage(), and have us track the output - * and mark loader_is_participating in replacement_start_image. This - * means anything added by fallback has to be verified by the system - * db, which we want to preserve anyway, since that's all launching - * through BDS gives us. */ + /* + * We need LoadImage() hooked so that we can guarantee everything is + * verified. + */ system_load_image = systab->BootServices->LoadImage; - systab->BootServices->LoadImage = load_image; + systab->BootServices->LoadImage = shim_load_image; - /* we need StartImage() so that we can allow chain booting to an - * image trusted by the firmware */ + /* + * We need StartImage() hooked because the system's StartImage() + * doesn't know about our structure layout. + */ system_start_image = systab->BootServices->StartImage; - systab->BootServices->StartImage = replacement_start_image; - -#if !defined(DISABLE_EBS_PROTECTION) - /* we need to hook ExitBootServices() so a) we can enforce the policy - * and b) we can unwrap when we're done. */ - system_exit_boot_services = systab->BootServices->ExitBootServices; - systab->BootServices->ExitBootServices = exit_boot_services; -#endif /* defined(DISABLE_EBS_PROTECTION) */ + systab->BootServices->StartImage = shim_start_image; + + /* + * We need Exit() hooked so that we make sure to use the right jmp_buf + * when an application calls Exit(), but that happens in a separate + * function. + */ + + /* + * We need UnloadImage() to match our LoadImage() + */ + system_unload_image = systab->BootServices->UnloadImage; + systab->BootServices->UnloadImage = shim_unload_image; } void @@ -317,9 +249,11 @@ hook_exit(EFI_SYSTEM_TABLE *local_systab) systab = local_systab; BS = local_systab->BootServices; - /* we need to hook Exit() so that we can allow users to quit the + /* + * We need to hook Exit() so that we can allow users to quit the * bootloader and still e.g. start a new one or run an internal - * shell. */ + * shell. + */ system_exit = systab->BootServices->Exit; - systab->BootServices->Exit = do_exit; + systab->BootServices->Exit = shim_exit; } diff --git a/shim.c b/shim.c index 98462aa0..bf0c9e6c 100644 --- a/shim.c +++ b/shim.c @@ -959,7 +959,6 @@ EFI_STATUS shim_verify (void *buffer, UINT32 size) if ((INT32)size < 0) return EFI_INVALID_PARAMETER; - loader_is_participating = 1; in_protocol = 1; efi_status = read_header(buffer, size, &context); @@ -1180,8 +1179,6 @@ EFI_STATUS start_image(EFI_HANDLE image_handle, CHAR16 *ImagePath) goto restore; } - loader_is_participating = 0; - /* * The binary is trusted and relocated. Run it */ @@ -1799,7 +1796,6 @@ shim_init(void) * validation of the next image. */ hook_system_services(systab); - loader_is_participating = 0; } } -- cgit v1.2.3 From 2bff46034aeefe4b266b6d6dd7d6cd771c1bf4de Mon Sep 17 00:00:00 2001 From: Mate Kukri Date: Thu, 18 Apr 2024 10:58:30 +0100 Subject: loader-proto: Add support for loading files from disk to LoadImage() Currently the EFI_SIMPLE_FILE_SYSTEM_PROTOCOL and EFI_LOAD_FILE2_PROTOCOL are supported. Signed-off-by: Mate Kukri --- loader-proto.c | 205 ++++++++++++++++++++++++++++++++++++++++++++++++++++----- shim.h | 1 + 2 files changed, 188 insertions(+), 18 deletions(-) (limited to 'loader-proto.c') diff --git a/loader-proto.c b/loader-proto.c index f0df122c..dcfa1a68 100644 --- a/loader-proto.c +++ b/loader-proto.c @@ -3,21 +3,9 @@ * loader-proto.c - shim's loader protocol * * Copyright Red Hat, Inc + * Copyright Canonical, Ltd */ -/* Chemical agents lend themselves to covert use in sabotage against - * which it is exceedingly difficult to visualize any really effective - * defense... I will not dwell upon this use of CBW because, as one - * pursues the possibilities of such covert uses, one discovers that the - * scenarios resemble that in which the components of a nuclear weapon - * are smuggled into New York City and assembled in the basement of the - * Empire State Building. - * In other words, once the possibility is recognized to exist, about - * all that one can do is worry about it. - * -- Dr. Ivan L Bennett, Jr., testifying before the Subcommittee on - * National Security Policy and Scientific Developments, November 20, - * 1969. - */ #include "shim.h" static EFI_SYSTEM_TABLE *systab; @@ -48,6 +36,133 @@ unhook_system_services(void) BS = systab->BootServices; } +typedef struct { + EFI_HANDLE hnd; + EFI_DEVICE_PATH *dp; + void *buffer; + size_t size; +} buffer_properties_t; + +static EFI_STATUS +try_load_from_sfs(EFI_DEVICE_PATH *dp, buffer_properties_t *bprop) +{ + EFI_STATUS status = EFI_SUCCESS; + EFI_SIMPLE_FILE_SYSTEM_PROTOCOL *sfs = NULL; + EFI_FILE_HANDLE root = NULL; + EFI_FILE_HANDLE file = NULL; + UINT64 tmpsz = 0; + + bprop->buffer = NULL; + + /* look for a handle with SFS support from the input DP */ + bprop->dp = dp; + status = BS->LocateDevicePath(&EFI_SIMPLE_FILE_SYSTEM_GUID, &bprop->dp, &bprop->hnd); + if (EFI_ERROR(status)) { + goto out; + } + + /* make sure the remaining DP portion is really a file path */ + if (DevicePathType(bprop->dp) != MEDIA_DEVICE_PATH || + DevicePathSubType(bprop->dp) != MEDIA_FILEPATH_DP) { + status = EFI_LOAD_ERROR; + goto out; + } + + /* find protocol, open the root directory, then open file */ + status = BS->HandleProtocol(bprop->hnd, &EFI_SIMPLE_FILE_SYSTEM_GUID, (void **)&sfs); + if (EFI_ERROR(status)) + goto out; + status = sfs->OpenVolume(sfs, &root); + if (EFI_ERROR(status)) + goto out; + status = root->Open(root, &file, ((FILEPATH_DEVICE_PATH *) bprop->dp)->PathName, EFI_FILE_MODE_READ, 0); + if (EFI_ERROR(status)) + goto out; + + /* get file size */ + status = file->SetPosition(file, -1ULL); + if (EFI_ERROR(status)) + goto out; + status = file->GetPosition(file, &tmpsz); + if (EFI_ERROR(status)) + goto out; + bprop->size = (size_t)tmpsz; + status = file->SetPosition(file, 0); + if (EFI_ERROR(status)) + goto out; + + /* allocate buffer */ + bprop->buffer = AllocatePool(bprop->size); + if (bprop->buffer == NULL) { + status = EFI_OUT_OF_RESOURCES; + goto out; + } + + /* read file */ + status = file->Read(file, &bprop->size, bprop->buffer); + +out: + if (EFI_ERROR(status) && bprop->buffer) + FreePool(bprop->buffer); + if (file) + file->Close(file); + if (root) + root->Close(root); + return status; +} + + +static EFI_STATUS +try_load_from_lf2(EFI_DEVICE_PATH *dp, buffer_properties_t *bprop) +{ + EFI_STATUS status = EFI_SUCCESS; + EFI_LOAD_FILE2_PROTOCOL *lf2 = NULL; + + bprop->buffer = NULL; + + /* look for a handle with LF2 support from the input DP */ + bprop->dp = dp; + status = BS->LocateDevicePath(&gEfiLoadFile2ProtocolGuid, &bprop->dp, &bprop->hnd); + if (EFI_ERROR(status)) + goto out; + + /* find protocol */ + status = BS->HandleProtocol(bprop->hnd, &gEfiLoadFile2ProtocolGuid, (void **) &lf2); + if (EFI_ERROR(status)) + goto out; + + /* get file size */ + bprop->size = 0; /* this shouldn't be read when Buffer=NULL but better be safe */ + status = lf2->LoadFile(lf2, bprop->dp, /*BootPolicy=*/false, &bprop->size, NULL); + /* + * NOTE: the spec is somewhat ambiguous what is the correct return + * status code when asking for the buffer size with Buffer=NULL. I am + * assuming EFI_SUCCESS and EFI_BUFFER_TOO_SMALL are the only + * reasonable interpretations. + */ + if (EFI_ERROR(status) && status != EFI_BUFFER_TOO_SMALL) { + status = EFI_LOAD_ERROR; + goto out; + } + + /* allocate buffer */ + bprop->buffer = AllocatePool(bprop->size); + if (!bprop->buffer) { + status = EFI_OUT_OF_RESOURCES; + goto out; + } + + /* read file */ + status = lf2->LoadFile(lf2, bprop->dp, /*BootPolicy=*/false, &bprop->size, bprop->buffer); + if (EFI_ERROR(status)) + goto out; + +out: + if (EFI_ERROR(status) && bprop->buffer) + FreePool(bprop->buffer); + return status; +} + static EFI_STATUS EFIAPI shim_load_image(BOOLEAN BootPolicy, EFI_HANDLE ParentImageHandle, EFI_DEVICE_PATH *FilePath, VOID *SourceBuffer, @@ -55,21 +170,59 @@ shim_load_image(BOOLEAN BootPolicy, EFI_HANDLE ParentImageHandle, { SHIM_LOADED_IMAGE *image; EFI_STATUS efi_status; + buffer_properties_t bprop; - (void)FilePath; - - if (BootPolicy || !SourceBuffer || !SourceSize) + if (BootPolicy) return EFI_UNSUPPORTED; + if (!SourceBuffer || !SourceSize) { + if (try_load_from_sfs(FilePath, &bprop) == EFI_SUCCESS) + ; + else if (try_load_from_lf2(FilePath, &bprop) == EFI_SUCCESS) + ; + else + /* no buffer given and we cannot load from this device */ + return EFI_LOAD_ERROR; + + SourceBuffer = bprop.buffer; + SourceSize = bprop.size; + } else { + bprop.buffer = NULL; + /* + * Even if we are using a buffer, try populating the + * device_handle and file_path fields the best we can + */ + bprop.dp = FilePath; + efi_status = BS->LocateDevicePath(&gEfiDevicePathProtocolGuid, + &bprop.dp, + &bprop.hnd); + if (efi_status != EFI_SUCCESS) { + /* can't seem to pull apart this DP */ + bprop.dp = FilePath; + bprop.hnd = NULL; + } + + } + image = AllocatePool(sizeof(*image)); - if (!image) - return EFI_OUT_OF_RESOURCES; + if (!image) { + efi_status = EFI_OUT_OF_RESOURCES; + goto free_buffer; + } SetMem(image, sizeof(*image), 0); image->li.Revision = 0x1000; image->li.ParentHandle = ParentImageHandle; image->li.SystemTable = systab; + image->li.DeviceHandle = bprop.hnd; + image->li.FilePath = DuplicateDevicePath(bprop.dp); + image->loaded_image_device_path = DuplicateDevicePath(FilePath); + if (!image->li.FilePath || + !image->loaded_image_device_path) { + efi_status = EFI_OUT_OF_RESOURCES; + goto free_image; + } efi_status = handle_image(SourceBuffer, SourceSize, &image->li, &image->entry_point, &image->alloc_address, @@ -81,16 +234,28 @@ shim_load_image(BOOLEAN BootPolicy, EFI_HANDLE ParentImageHandle, efi_status = BS->InstallMultipleProtocolInterfaces(ImageHandle, &SHIM_LOADED_IMAGE_GUID, image, &EFI_LOADED_IMAGE_GUID, &image->li, + &gEfiLoadedImageDevicePathProtocolGuid, + image->loaded_image_device_path, NULL); if (EFI_ERROR(efi_status)) goto free_alloc; + if (bprop.buffer) + FreePool(bprop.buffer); + return EFI_SUCCESS; free_alloc: BS->FreePages(image->alloc_address, image->alloc_pages); free_image: + if (image->loaded_image_device_path) + FreePool(image->loaded_image_device_path); + if (image->li.FilePath) + FreePool(image->li.FilePath); FreePool(image); +free_buffer: + if (bprop.buffer) + FreePool(bprop.buffer); return efi_status; } @@ -133,9 +298,13 @@ shim_start_image(IN EFI_HANDLE ImageHandle, OUT UINTN *ExitDataSize, BS->UninstallMultipleProtocolInterfaces(ImageHandle, &EFI_LOADED_IMAGE_GUID, image, &SHIM_LOADED_IMAGE_GUID, &image->li, + &gEfiLoadedImageDevicePathProtocolGuid, + image->loaded_image_device_path, NULL); BS->FreePages(image->alloc_address, image->alloc_pages); + BS->FreePool(image->li.FilePath); + BS->FreePool(image->loaded_image_device_path); FreePool(image); return efi_status; diff --git a/shim.h b/shim.h index a3f8a505..09d23780 100644 --- a/shim.h +++ b/shim.h @@ -341,6 +341,7 @@ typedef struct { UINTN exit_data_size; jmp_buf longjmp_buf; BOOLEAN started; + EFI_DEVICE_PATH *loaded_image_device_path; } SHIM_LOADED_IMAGE; #endif /* SHIM_H_ */ -- cgit v1.2.3 From 5d172787d5fa7faafcaf5fe62ad36819bb51ba54 Mon Sep 17 00:00:00 2001 From: Mate Kukri Date: Fri, 24 May 2024 10:54:12 +0100 Subject: loader-proto: Mark load_image()'s handle_image() call as "in_protocol" When verifying an image, if we're "in" a shim protocol call, we require the binary have an SBAT section. If it's not present we raise an EFI_SECURITY_VIOLATION error code. Currently loader protocol's load_image() is not marked as in protocol, so it instead will return EFI_SUCCESS when verifying the SBAT section. This patch changes that to be in protocol, so that SBAT will be required on any images loaded with shim's loader protocol. This will bring SBAT enforcement in-line with the shim_lock protocol. Signed-off-by: Mate Kukri --- loader-proto.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'loader-proto.c') diff --git a/loader-proto.c b/loader-proto.c index dcfa1a68..f899e594 100644 --- a/loader-proto.c +++ b/loader-proto.c @@ -224,9 +224,11 @@ shim_load_image(BOOLEAN BootPolicy, EFI_HANDLE ParentImageHandle, goto free_image; } + in_protocol = 1; efi_status = handle_image(SourceBuffer, SourceSize, &image->li, &image->entry_point, &image->alloc_address, &image->alloc_pages); + in_protocol = 0; if (EFI_ERROR(efi_status)) goto free_image; -- cgit v1.2.3 From c57af36e673b34a9b24309f76f105371316c45be Mon Sep 17 00:00:00 2001 From: Mate Kukri Date: Fri, 24 May 2024 11:41:04 +0100 Subject: loader-proto: Respect optional DevicePath parameter to load_image() load_image() takes an optional parameter, DevicePath, in addition to the SourceBuffer. Currently in shim_load_image() we don't check to see if it's provided in the case where there's no SourceBuffer, even though it can't work without it. This adds that test and errors in that case, as well as avoiding duplicating it when it's not present. Signed-off-by: Mate Kukri --- loader-proto.c | 55 +++++++++++++++++++++++++++++++++++-------------------- 1 file changed, 35 insertions(+), 20 deletions(-) (limited to 'loader-proto.c') diff --git a/loader-proto.c b/loader-proto.c index f899e594..53d35701 100644 --- a/loader-proto.c +++ b/loader-proto.c @@ -165,7 +165,7 @@ out: static EFI_STATUS EFIAPI shim_load_image(BOOLEAN BootPolicy, EFI_HANDLE ParentImageHandle, - EFI_DEVICE_PATH *FilePath, VOID *SourceBuffer, + EFI_DEVICE_PATH *DevicePath, VOID *SourceBuffer, UINTN SourceSize, EFI_HANDLE *ImageHandle) { SHIM_LOADED_IMAGE *image; @@ -176,9 +176,12 @@ shim_load_image(BOOLEAN BootPolicy, EFI_HANDLE ParentImageHandle, return EFI_UNSUPPORTED; if (!SourceBuffer || !SourceSize) { - if (try_load_from_sfs(FilePath, &bprop) == EFI_SUCCESS) + if (!DevicePath) /* Both SourceBuffer and DevicePath are NULL */ + return EFI_NOT_FOUND; + + if (try_load_from_sfs(DevicePath, &bprop) == EFI_SUCCESS) ; - else if (try_load_from_lf2(FilePath, &bprop) == EFI_SUCCESS) + else if (try_load_from_lf2(DevicePath, &bprop) == EFI_SUCCESS) ; else /* no buffer given and we cannot load from this device */ @@ -192,16 +195,19 @@ shim_load_image(BOOLEAN BootPolicy, EFI_HANDLE ParentImageHandle, * Even if we are using a buffer, try populating the * device_handle and file_path fields the best we can */ - bprop.dp = FilePath; - efi_status = BS->LocateDevicePath(&gEfiDevicePathProtocolGuid, - &bprop.dp, - &bprop.hnd); - if (efi_status != EFI_SUCCESS) { - /* can't seem to pull apart this DP */ - bprop.dp = FilePath; - bprop.hnd = NULL; - } + bprop.dp = DevicePath; + + if (bprop.dp) { + efi_status = BS->LocateDevicePath(&gEfiDevicePathProtocolGuid, + &bprop.dp, + &bprop.hnd); + if (efi_status != EFI_SUCCESS) { + /* can't seem to pull apart this DP */ + bprop.dp = DevicePath; + bprop.hnd = NULL; + } + } } image = AllocatePool(sizeof(*image)); @@ -216,12 +222,19 @@ shim_load_image(BOOLEAN BootPolicy, EFI_HANDLE ParentImageHandle, image->li.ParentHandle = ParentImageHandle; image->li.SystemTable = systab; image->li.DeviceHandle = bprop.hnd; - image->li.FilePath = DuplicateDevicePath(bprop.dp); - image->loaded_image_device_path = DuplicateDevicePath(FilePath); - if (!image->li.FilePath || - !image->loaded_image_device_path) { - efi_status = EFI_OUT_OF_RESOURCES; - goto free_image; + if (bprop.dp) { + image->li.FilePath = DuplicateDevicePath(bprop.dp); + if (!image->li.FilePath) { + efi_status = EFI_OUT_OF_RESOURCES; + goto free_image; + } + } + if (DevicePath) { + image->loaded_image_device_path = DuplicateDevicePath(DevicePath); + if (!image->loaded_image_device_path) { + efi_status = EFI_OUT_OF_RESOURCES; + goto free_image; + } } in_protocol = 1; @@ -305,8 +318,10 @@ shim_start_image(IN EFI_HANDLE ImageHandle, OUT UINTN *ExitDataSize, NULL); BS->FreePages(image->alloc_address, image->alloc_pages); - BS->FreePool(image->li.FilePath); - BS->FreePool(image->loaded_image_device_path); + if (image->li.FilePath) + BS->FreePool(image->li.FilePath); + if (image->loaded_image_device_path) + BS->FreePool(image->loaded_image_device_path); FreePool(image); return efi_status; -- cgit v1.2.3 From db0432183680121df2fcaba2b697025fef9db6ac Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Wed, 12 Mar 2025 16:00:30 -0400 Subject: shim_load_image(): initialize the buffer fully scan-build notes that we assign bprop.hnd, an EFI_HANDLE for the device path protocol, to our loaded_image->li.DeviceHandle, and it thinks since bprop is uninitialized that means it can be NULL or garbage. I don't think that's actually true, because every path to that requires either returning an error or doing some variety of: status = BS->LocateDevicePath(&gEfiDevicePathProtocolGuid, &bp, &hnd) and checking its error, but only one of those paths explicitly sets a value, and static checkers can't tell what BS->LocateDevicePath does with the pointer. This patch avoids the issue by initializing the whole bprop structure to begin with. Signed-off-by: Peter Jones --- loader-proto.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'loader-proto.c') diff --git a/loader-proto.c b/loader-proto.c index 53d35701..4581664b 100644 --- a/loader-proto.c +++ b/loader-proto.c @@ -170,7 +170,7 @@ shim_load_image(BOOLEAN BootPolicy, EFI_HANDLE ParentImageHandle, { SHIM_LOADED_IMAGE *image; EFI_STATUS efi_status; - buffer_properties_t bprop; + buffer_properties_t bprop = { NULL, NULL, NULL, 0 }; if (BootPolicy) return EFI_UNSUPPORTED; -- cgit v1.2.3