From cae5e2f7c100bc9e8f07de62353021d6737a50ee Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Fri, 23 Jul 2021 14:18:06 -0400 Subject: shim/mm/fb: move global state to its own source file This moves the globals from shim.c (and lib/console.c) into their own file, to make it so that unit tests can more easily link against code that uses that state. Signed-off-by: Peter Jones --- globals.c | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 globals.c (limited to 'globals.c') diff --git a/globals.c b/globals.c new file mode 100644 index 00000000..476e2e9c --- /dev/null +++ b/globals.c @@ -0,0 +1,31 @@ +// SPDX-License-Identifier: BSD-2-Clause-Patent +/* + * globals.c - global shim state + * Copyright Peter Jones + */ + +#include "shim.h" + +UINT32 vendor_authorized_size = 0; +UINT8 *vendor_authorized = NULL; + +UINT32 vendor_deauthorized_size = 0; +UINT8 *vendor_deauthorized = NULL; + +#if defined(ENABLE_SHIM_CERT) +UINT32 build_cert_size; +UINT8 *build_cert; +#endif /* defined(ENABLE_SHIM_CERT) */ + +/* + * indicator of how an image has been verified + */ +verification_method_t verification_method; +int loader_is_participating; + +UINT8 user_insecure_mode; +UINT8 ignore_db; + +UINT32 verbose = 0; + +// vim:fenc=utf-8:tw=75:noet -- cgit v1.2.3 From 4e513405b4f1641710115780d19dcec130c5208f Mon Sep 17 00:00:00 2001 From: Eric Snowberg Date: Tue, 5 Oct 2021 12:06:05 -0400 Subject: Introduce a new MOK variable called MokListTrustedRT Introduce a new MOK variable called MokListTrustedRT. It allows an end-user to decide if they want to trust MOKList keys within the soon to be booted Linux kernel. This variable does not change any functionality within shim itself. When Linux boots, if MokListTrustedRT is set and EFI_VARIABLE_NON_VOLATILE is not set, keys in MokListRT are loaded into the .machine keyring instead of the .platform keyring. Signed-off-by: Eric Snowberg --- MokManager.c | 167 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-- MokVars.txt | 6 +++ globals.c | 1 + mok.c | 17 +++++- shim.h | 1 + 5 files changed, 187 insertions(+), 5 deletions(-) (limited to 'globals.c') diff --git a/MokManager.c b/MokManager.c index 08b15d6d..c195cadc 100644 --- a/MokManager.c +++ b/MokManager.c @@ -40,6 +40,12 @@ typedef struct { CHAR16 Password[SB_PASSWORD_LEN]; } __attribute__ ((packed)) MokDBvar; +typedef struct { + UINT32 MokTMLState; + UINT32 PWLen; + CHAR16 Password[SB_PASSWORD_LEN]; +} __attribute__ ((packed)) MokTMLvar; + typedef struct { INT32 Timeout; } __attribute__ ((packed)) MokTimeoutvar; @@ -1678,6 +1684,121 @@ static EFI_STATUS mok_db_prompt(void *MokDB, UINTN MokDBSize) return EFI_SUCCESS; } +static EFI_STATUS mok_tml_prompt(void *MokTML, UINTN MokTMLSize) +{ + EFI_STATUS efi_status; + SIMPLE_TEXT_OUTPUT_MODE SavedMode; + MokTMLvar *var = MokTML; + CHAR16 *message[4]; + CHAR16 pass1, pass2, pass3; + CHAR16 *str; + UINT8 fail_count = 0; + UINT8 dbval = 1; + UINT8 pos1, pos2, pos3; + int ret; + CHAR16 *untrust_tml[] = { L"Do not trust the MOK list", NULL }; + CHAR16 *trust_tml[] = { L"Trust the MOK list", NULL }; + + if (MokTMLSize != sizeof(MokTMLvar)) { + console_notify(L"Invalid MokTML variable contents"); + return EFI_INVALID_PARAMETER; + } + + clear_screen(); + + message[0] = L"Change Trusted MOK List Keyring state"; + message[1] = NULL; + + console_save_and_set_mode(&SavedMode); + console_print_box_at(message, -1, 0, 0, -1, -1, 1, 1); + console_restore_mode(&SavedMode); + + while (fail_count < 3) { + RandomBytes(&pos1, sizeof(pos1)); + pos1 = (pos1 % var->PWLen); + + do { + RandomBytes(&pos2, sizeof(pos2)); + pos2 = (pos2 % var->PWLen); + } while (pos2 == pos1); + + do { + RandomBytes(&pos3, sizeof(pos3)); + pos3 = (pos3 % var->PWLen); + } while (pos3 == pos2 || pos3 == pos1); + + str = PoolPrint(L"Enter password character %d: ", pos1 + 1); + if (!str) { + console_errorbox(L"Failed to allocate buffer"); + return EFI_OUT_OF_RESOURCES; + } + pass1 = get_password_charater(str); + FreePool(str); + + str = PoolPrint(L"Enter password character %d: ", pos2 + 1); + if (!str) { + console_errorbox(L"Failed to allocate buffer"); + return EFI_OUT_OF_RESOURCES; + } + pass2 = get_password_charater(str); + FreePool(str); + + str = PoolPrint(L"Enter password character %d: ", pos3 + 1); + if (!str) { + console_errorbox(L"Failed to allocate buffer"); + return EFI_OUT_OF_RESOURCES; + } + pass3 = get_password_charater(str); + FreePool(str); + + if (pass1 != var->Password[pos1] || + pass2 != var->Password[pos2] || + pass3 != var->Password[pos3]) { + console_print(L"Invalid character\n"); + fail_count++; + } else { + break; + } + } + + if (fail_count >= 3) { + console_notify(L"Password limit reached"); + return EFI_ACCESS_DENIED; + } + + if (var->MokTMLState == 0) + ret = console_yes_no(trust_tml); + else + ret = console_yes_no(untrust_tml); + + if (ret == 0) { + LibDeleteVariable(L"MokListTrustedNew", &SHIM_LOCK_GUID); + return EFI_ABORTED; + } + + if (var->MokTMLState == 0) { + efi_status = RT->SetVariable(L"MokListTrusted", &SHIM_LOCK_GUID, + EFI_VARIABLE_NON_VOLATILE | + EFI_VARIABLE_BOOTSERVICE_ACCESS, + 1, &dbval); + if (EFI_ERROR(efi_status)) { + console_notify(L"Failed to set MokListTrusted state"); + return efi_status; + } + } else { + efi_status = RT->SetVariable(L"MokListTrusted", &SHIM_LOCK_GUID, + EFI_VARIABLE_NON_VOLATILE | + EFI_VARIABLE_BOOTSERVICE_ACCESS, + 0, NULL); + if (EFI_ERROR(efi_status)) { + console_notify(L"Failed to delete MokListTrusted state"); + return efi_status; + } + } + + return EFI_SUCCESS; +} + static EFI_STATUS mok_pw_prompt(void *MokPW, UINTN MokPWSize) { EFI_STATUS efi_status; @@ -2076,7 +2197,8 @@ typedef enum { MOK_SET_PW, MOK_CHANGE_DB, MOK_KEY_ENROLL, - MOK_HASH_ENROLL + MOK_HASH_ENROLL, + MOK_CHANGE_TML } mok_menu_item; static void free_menu(mok_menu_item * menu_item, CHAR16 ** menu_strings) @@ -2095,7 +2217,8 @@ static EFI_STATUS enter_mok_menu(EFI_HANDLE image_handle UNUSED, void *MokPW, UINTN MokPWSize, void *MokDB, UINTN MokDBSize, void *MokXNew, UINTN MokXNewSize, - void *MokXDel, UINTN MokXDelSize) + void *MokXDel, UINTN MokXDelSize, + void *MokTML, UINTN MokTMLSize) { CHAR16 **menu_strings = NULL; mok_menu_item *menu_item = NULL; @@ -2171,6 +2294,9 @@ static EFI_STATUS enter_mok_menu(EFI_HANDLE image_handle UNUSED, if (MokDB) menucount++; + if (MokTML) + menucount++; + menu_strings = AllocateZeroPool(sizeof(CHAR16 *) * (menucount + 1)); if (!menu_strings) @@ -2242,6 +2368,12 @@ static EFI_STATUS enter_mok_menu(EFI_HANDLE image_handle UNUSED, i++; } + if (MokTML) { + menu_strings[i] = L"Change MOK List Trusted State"; + menu_item[i] = MOK_CHANGE_TML; + i++; + } + menu_strings[i] = L"Enroll key from disk"; menu_item[i] = MOK_KEY_ENROLL; i++; @@ -2352,6 +2484,17 @@ static EFI_STATUS enter_mok_menu(EFI_HANDLE image_handle UNUSED, case MOK_HASH_ENROLL: efi_status = mok_hash_enroll(); break; + case MOK_CHANGE_TML: + if (!MokTML) { + console_print(L"MokManager: internal error: %s", + L"MokListTrusted was ! NULL bs is now NULL\n"); + ret = EFI_ABORTED; + goto out; + } + efi_status = mok_tml_prompt(MokTML, MokTMLSize); + if (!EFI_ERROR(efi_status)) + MokTML = NULL; + break; } if (!EFI_ERROR(efi_status)) @@ -2376,7 +2519,7 @@ out: static EFI_STATUS check_mok_request(EFI_HANDLE image_handle) { UINTN MokNewSize = 0, MokDelSize = 0, MokSBSize = 0, MokPWSize = 0; - UINTN MokDBSize = 0, MokXNewSize = 0, MokXDelSize = 0; + UINTN MokDBSize = 0, MokXNewSize = 0, MokXDelSize = 0, MokTMLSize = 0; void *MokNew = NULL; void *MokDel = NULL; void *MokSB = NULL; @@ -2384,6 +2527,7 @@ static EFI_STATUS check_mok_request(EFI_HANDLE image_handle) void *MokDB = NULL; void *MokXNew = NULL; void *MokXDel = NULL; + void *MokTML = NULL; EFI_STATUS efi_status; efi_status = get_variable(L"MokNew", (UINT8 **) & MokNew, &MokNewSize, @@ -2436,6 +2580,18 @@ static EFI_STATUS check_mok_request(EFI_HANDLE image_handle) console_error(L"Could not retrieve MokDB", efi_status); } + efi_status = get_variable(L"MokListTrustedNew", (UINT8 **) & MokTML, + &MokTMLSize, SHIM_LOCK_GUID); + if (!EFI_ERROR(efi_status)) { + efi_status = LibDeleteVariable(L"MokListTrustedNew", + &SHIM_LOCK_GUID); + if (EFI_ERROR(efi_status)) + console_notify(L"Failed to delete MokListTrustedNew"); + } else if (EFI_ERROR(efi_status) && efi_status != EFI_NOT_FOUND) { + console_error(L"Could not retrieve MokListTrustedNew", + efi_status); + } + efi_status = get_variable(L"MokXNew", (UINT8 **) & MokXNew, &MokXNewSize, SHIM_LOCK_GUID); if (!EFI_ERROR(efi_status)) { @@ -2458,7 +2614,7 @@ static EFI_STATUS check_mok_request(EFI_HANDLE image_handle) enter_mok_menu(image_handle, MokNew, MokNewSize, MokDel, MokDelSize, MokSB, MokSBSize, MokPW, MokPWSize, MokDB, MokDBSize, - MokXNew, MokXNewSize, MokXDel, MokXDelSize); + MokXNew, MokXNewSize, MokXDel, MokXDelSize, MokTML, MokTMLSize); if (MokNew) FreePool(MokNew); @@ -2481,6 +2637,9 @@ static EFI_STATUS check_mok_request(EFI_HANDLE image_handle) if (MokXDel) FreePool(MokXDel); + if (MokTML) + FreePool(MokTML); + LibDeleteVariable(L"MokAuth", &SHIM_LOCK_GUID); LibDeleteVariable(L"MokDelAuth", &SHIM_LOCK_GUID); LibDeleteVariable(L"MokXAuth", &SHIM_LOCK_GUID); diff --git a/MokVars.txt b/MokVars.txt index 4b80a413..cdfec2c8 100644 --- a/MokVars.txt +++ b/MokVars.txt @@ -77,3 +77,9 @@ or not to import DB certs for its own verification purposes. MokPWStore: A SHA-256 representation of the password set by the user via MokPW. The user will be prompted to enter this password in order to interact with MokManager. + +MokListTrusted: An 8-bit unsigned integer. If 1, it signifies to Linux +to trust CA keys in the MokList. BS,NV + +MokListTrustedRT: A copy of MokListTrusted made available to the kernel +at runtime. RT diff --git a/globals.c b/globals.c index 476e2e9c..4a1f432f 100644 --- a/globals.c +++ b/globals.c @@ -25,6 +25,7 @@ int loader_is_participating; UINT8 user_insecure_mode; UINT8 ignore_db; +UINT8 trust_mok_list; UINT32 verbose = 0; diff --git a/mok.c b/mok.c index 7755eea9..52dffc3e 100644 --- a/mok.c +++ b/mok.c @@ -46,7 +46,7 @@ static EFI_STATUS check_mok_request(EFI_HANDLE image_handle) check_var(L"MokPW") || check_var(L"MokAuth") || check_var(L"MokDel") || check_var(L"MokDB") || check_var(L"MokXNew") || check_var(L"MokXDel") || - check_var(L"MokXAuth")) { + check_var(L"MokXAuth") || check_var(L"MokListTrustedNew")) { efi_status = start_image(image_handle, MOK_MANAGER); if (EFI_ERROR(efi_status)) { @@ -166,6 +166,20 @@ struct mok_state_variable mok_state_variable_data[] = { MOK_VARIABLE_MEASURE, .pcr = 7, }, + {.name = L"MokListTrusted", + .name8 = "MokListTrusted", + .rtname = L"MokListTrustedRT", + .rtname8 = "MokListTrustedRT", + .guid = &SHIM_LOCK_GUID, + .yes_attr = EFI_VARIABLE_BOOTSERVICE_ACCESS | + EFI_VARIABLE_NON_VOLATILE, + .no_attr = EFI_VARIABLE_RUNTIME_ACCESS, + .flags = MOK_MIRROR_DELETE_FIRST | + MOK_VARIABLE_MEASURE | + MOK_VARIABLE_LOG, + .pcr = 14, + .state = &trust_mok_list, + }, { NULL, } }; size_t n_mok_state_variables = sizeof(mok_state_variable_data) / sizeof(mok_state_variable_data[0]); @@ -897,6 +911,7 @@ EFI_STATUS import_mok_state(EFI_HANDLE image_handle) user_insecure_mode = 0; ignore_db = 0; + trust_mok_list = 0; UINT64 config_sz = 0; UINT8 *config_table = NULL; diff --git a/shim.h b/shim.h index 5e1ab36b..69442da3 100644 --- a/shim.h +++ b/shim.h @@ -255,6 +255,7 @@ extern UINT8 *build_cert; extern UINT8 user_insecure_mode; extern UINT8 ignore_db; +extern UINT8 trust_mok_list; extern UINT8 in_protocol; extern void *load_options; extern UINT32 load_options_size; -- cgit v1.2.3 From 35d7378d29b9ad6f664df20efc4121e210859e65 Mon Sep 17 00:00:00 2001 From: Eric Snowberg Date: Tue, 1 Feb 2022 15:49:51 -0500 Subject: Load additional certs from a signed binary Heavily inspired by Matthew Garrett's patch "Allow additional certificates to be loaded from a signed binary". Add support for loading a binary, verifying its signature, and then scanning it for embedded certificates. This is intended to make it possible to decouple shim builds from vendor signatures. In order to add new signatures to shim, an EFI Signature List should be generated and then added to the .db section of a well-formed EFI binary. This binary should then be signed with a key that shim already trusts (either a built-in key, one present in the platform firmware or one present in MOK) and placed in the same directory as shim with a filename starting "shim_certificate" (eg, "shim_certificate_oracle"). Shim will read multiple files and incorporate the signatures from all of them. Note that each section *must* be an EFI Signature List, not a raw certificate. Signed-off-by: Eric Snowberg --- globals.c | 3 ++ include/mok.h | 3 ++ mok.c | 9 ++++- shim.c | 127 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ shim.h | 3 ++ 5 files changed, 144 insertions(+), 1 deletion(-) (limited to 'globals.c') diff --git a/globals.c b/globals.c index 4a1f432f..30d10630 100644 --- a/globals.c +++ b/globals.c @@ -12,6 +12,9 @@ UINT8 *vendor_authorized = NULL; UINT32 vendor_deauthorized_size = 0; UINT8 *vendor_deauthorized = NULL; +UINT32 user_cert_size; +UINT8 *user_cert; + #if defined(ENABLE_SHIM_CERT) UINT32 build_cert_size; UINT8 *build_cert; diff --git a/include/mok.h b/include/mok.h index 96da397a..6f99a105 100644 --- a/include/mok.h +++ b/include/mok.h @@ -59,6 +59,9 @@ struct mok_state_variable { UINT8 **addend; UINT32 *addend_size; + UINT8 **user_cert; + UINT32 *user_cert_size; + /* * build_cert is our build-time cert. Like addend, this is added * to the input variable, as part of the runtime variable, so that diff --git a/mok.c b/mok.c index f4de8081..94101843 100644 --- a/mok.c +++ b/mok.c @@ -98,6 +98,8 @@ struct mok_state_variable mok_state_variable_data[] = { .categorize_addend = categorize_authorized, .addend = &vendor_authorized, .addend_size = &vendor_authorized_size, + .user_cert = &user_cert, + .user_cert_size = &user_cert_size, #if defined(ENABLE_SHIM_CERT) .build_cert = &build_cert, .build_cert_size = &build_cert_size, @@ -588,7 +590,8 @@ mirror_one_mok_variable(struct mok_state_variable *v, dprint(L"FullDataSize:0x%lx FullData:0x%llx\n", FullDataSize, FullData); } - + if (v->user_cert_size) + FullDataSize += *v->user_cert_size; } /* @@ -702,6 +705,10 @@ mirror_one_mok_variable(struct mok_state_variable *v, dprint(L"FullDataSize:%lu FullData:0x%llx p:0x%llx pos:%lld\n", FullDataSize, FullData, p, p-(uintptr_t)FullData); } + if (v->user_cert_size) { + CopyMem(p, *v->user_cert, *v->user_cert_size); + p += *v->user_cert_size; + } } /* diff --git a/shim.c b/shim.c index 51e72e7e..c4f9461a 100644 --- a/shim.c +++ b/shim.c @@ -1391,6 +1391,126 @@ uninstall_shim_protocols(void) #endif } +EFI_STATUS +load_cert_file(EFI_HANDLE image_handle, CHAR16 *filename) +{ + EFI_STATUS efi_status; + EFI_LOADED_IMAGE *li = NULL; + PE_COFF_LOADER_IMAGE_CONTEXT context; + EFI_IMAGE_SECTION_HEADER *Section; + EFI_SIGNATURE_LIST *certlist; + CHAR16 *PathName = NULL; + void *pointer; + UINT32 original; + int datasize; + void *data; + int i; + + efi_status = read_image(image_handle, filename, PathName, + &data, &datasize); + + if (EFI_ERROR(efi_status)) + return efi_status; + + efi_status = verify_image(data, datasize, li, &context); + if (EFI_ERROR(efi_status)) + return efi_status; + + Section = context.FirstSection; + for (i = 0; i < context.NumberOfSections; i++, Section++) { + if (CompareMem(Section->Name, ".db\0\0\0\0\0", 8) == 0) { + original = user_cert_size; + if (Section->SizeOfRawData < sizeof(EFI_SIGNATURE_LIST)) { + continue; + } + pointer = ImageAddress(data, datasize, + Section->PointerToRawData); + if (!pointer) { + continue; + } + certlist = pointer; + user_cert_size += certlist->SignatureListSize;; + user_cert = ReallocatePool(user_cert, original, + user_cert_size); + memcpy(user_cert + original, pointer, + certlist->SignatureListSize); + } + } + FreePool(data); + return EFI_SUCCESS; +} + +/* Read additional certificates from files (after verifying signatures) */ +EFI_STATUS +load_certs(EFI_HANDLE image_handle) +{ + EFI_STATUS efi_status; + EFI_LOADED_IMAGE *li = NULL; + CHAR16 *PathName = NULL; + EFI_FILE *root, *dir; + EFI_FILE_INFO *info; + EFI_HANDLE device; + EFI_FILE_IO_INTERFACE *drive; + UINTN buffersize = 0; + void *buffer = NULL; + + efi_status = gBS->HandleProtocol(image_handle, &EFI_LOADED_IMAGE_GUID, + (void **)&li); + if (EFI_ERROR(efi_status)) { + perror(L"Unable to init protocol\n"); + return efi_status; + } + + efi_status = generate_path_from_image_path(li, L"", &PathName); + if (EFI_ERROR(efi_status)) + goto done; + + device = li->DeviceHandle; + efi_status = gBS->HandleProtocol(device, &EFI_SIMPLE_FILE_SYSTEM_GUID, + (void **)&drive); + if (EFI_ERROR(efi_status)) { + perror(L"Failed to find fs: %r\n", efi_status); + goto done; + } + + efi_status = drive->OpenVolume(drive, &root); + if (EFI_ERROR(efi_status)) { + perror(L"Failed to open fs: %r\n", efi_status); + goto done; + } + + efi_status = root->Open(root, &dir, PathName, EFI_FILE_MODE_READ, 0); + if (EFI_ERROR(efi_status)) { + perror(L"Failed to open %s - %r\n", PathName, efi_status); + goto done; + } + + while (1) { + int old = buffersize; + efi_status = dir->Read(dir, &buffersize, buffer); + if (efi_status == EFI_BUFFER_TOO_SMALL) { + buffer = ReallocatePool(buffer, old, buffersize); + continue; + } else if (EFI_ERROR(efi_status)) { + perror(L"Failed to read directory %s - %r\n", PathName, + efi_status); + goto done; + } + + if (buffersize == 0) + goto done; + + info = (EFI_FILE_INFO *)buffer; + if (StrnCaseCmp(info->FileName, L"shim_certificate", 16) == 0) { + load_cert_file(image_handle, info->FileName); + } + } +done: + FreePool(buffer); + FreePool(PathName); + return efi_status; +} + EFI_STATUS shim_init(void) { @@ -1626,6 +1746,13 @@ efi_main (EFI_HANDLE passed_image_handle, EFI_SYSTEM_TABLE *passed_systab) init_openssl(); + if (secure_mode()) { + efi_status = load_certs(global_image_handle); + if (EFI_ERROR(efi_status)) { + LogError(L"Failed to load addon certificates\n"); + } + } + /* * Before we do anything else, validate our non-volatile, * boot-services-only state variables are what we think they are. diff --git a/shim.h b/shim.h index db264cb7..703c1145 100644 --- a/shim.h +++ b/shim.h @@ -248,6 +248,9 @@ extern UINT8 *vendor_authorized; extern UINT32 vendor_deauthorized_size; extern UINT8 *vendor_deauthorized; +extern UINT32 user_cert_size; +extern UINT8 *user_cert; + #if defined(ENABLE_SHIM_CERT) extern UINT32 build_cert_size; extern UINT8 *build_cert; -- cgit v1.2.3 From df96f48f28fa94b62d06f39a3b014133dd38def5 Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Thu, 31 Mar 2022 16:19:53 -0400 Subject: Add MokPolicy variable and MOK_POLICY_REQUIRE_NX This adds a new MoK variable, MokPolicy (&MokPolicyRT) that's intended as a bitmask of machine owner policy choices, and the bit MOK_POLICY_REQUIRE_NX. This bit specifies whether it is permissible to load binaries which do not support NX mitigations, and it currently defaults to allowing such binaries to be loaded. The broader intention here is to migrate all of the MoK policy variables that are really just on/off flags to this variable. Signed-off-by: Peter Jones --- globals.c | 1 + include/mok.h | 5 +++++ mok.c | 13 +++++++++++++ pe.c | 8 +++++--- shim.h | 2 ++ 5 files changed, 26 insertions(+), 3 deletions(-) (limited to 'globals.c') diff --git a/globals.c b/globals.c index 30d10630..b4e80dd3 100644 --- a/globals.c +++ b/globals.c @@ -29,6 +29,7 @@ int loader_is_participating; UINT8 user_insecure_mode; UINT8 ignore_db; UINT8 trust_mok_list; +UINT8 mok_policy = 0; UINT32 verbose = 0; diff --git a/include/mok.h b/include/mok.h index 6f99a105..fb19423b 100644 --- a/include/mok.h +++ b/include/mok.h @@ -100,5 +100,10 @@ struct mok_variable_config_entry { UINT8 data[]; }; +/* + * bit definitions for MokPolicy + */ +#define MOK_POLICY_REQUIRE_NX 1 + #endif /* !SHIM_MOK_H_ */ // vim:fenc=utf-8:tw=75:noet diff --git a/mok.c b/mok.c index 94101843..a8c8be6b 100644 --- a/mok.c +++ b/mok.c @@ -184,6 +184,19 @@ struct mok_state_variable mok_state_variable_data[] = { .pcr = 14, .state = &trust_mok_list, }, + {.name = L"MokPolicy", + .name8 = "MokPolicy", + .rtname = L"MokPolicyRT", + .rtname8 = "MokPolicyRT", + .guid = &SHIM_LOCK_GUID, + .yes_attr = EFI_VARIABLE_BOOTSERVICE_ACCESS | + EFI_VARIABLE_NON_VOLATILE, + .no_attr = EFI_VARIABLE_RUNTIME_ACCESS, + .flags = MOK_MIRROR_DELETE_FIRST | + MOK_VARIABLE_LOG, + .pcr = 14, + .state = &mok_policy, + }, { NULL, } }; size_t n_mok_state_variables = sizeof(mok_state_variable_data) / sizeof(mok_state_variable_data[0]); diff --git a/pe.c b/pe.c index 9fa6fffd..5d0c6b0b 100644 --- a/pe.c +++ b/pe.c @@ -800,8 +800,9 @@ read_header(void *data, unsigned int datasize, DllFlags = PEHdr->Pe32.OptionalHeader.DllCharacteristics; } - if (!(DllFlags & EFI_IMAGE_DLLCHARACTERISTICS_NX_COMPAT)) { - perror(L"Image does not support NX\n"); + if ((mok_policy & MOK_POLICY_REQUIRE_NX) && + !(DllFlags & EFI_IMAGE_DLLCHARACTERISTICS_NX_COMPAT)) { + perror(L"Policy requires NX, but image does not support NX\n"); return EFI_UNSUPPORTED; } @@ -1203,7 +1204,8 @@ handle_image (void *data, unsigned int datasize, if (!(Section->Characteristics & EFI_IMAGE_SCN_MEM_DISCARDABLE) && (Section->Characteristics & EFI_IMAGE_SCN_MEM_WRITE) && - (Section->Characteristics & EFI_IMAGE_SCN_MEM_EXECUTE)) { + (Section->Characteristics & EFI_IMAGE_SCN_MEM_EXECUTE) && + (mok_policy & MOK_POLICY_REQUIRE_NX)) { perror(L"Section %d is writable and executable\n", i); return EFI_UNSUPPORTED; } diff --git a/shim.h b/shim.h index dc3cda73..b5272b9c 100644 --- a/shim.h +++ b/shim.h @@ -263,6 +263,8 @@ extern UINT8 *build_cert; extern UINT8 user_insecure_mode; extern UINT8 ignore_db; extern UINT8 trust_mok_list; +extern UINT8 mok_policy; + extern UINT8 in_protocol; extern void *load_options; extern UINT32 load_options_size; -- cgit v1.2.3 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 --- Makefile | 4 +- globals.c | 2 + include/loader-proto.h | 35 ++++++ include/replacements.h | 30 ----- loader-proto.c | 325 +++++++++++++++++++++++++++++++++++++++++++++++++ replacements.c | 233 ----------------------------------- shim.c | 91 +------------- shim.h | 11 +- 8 files changed, 371 insertions(+), 360 deletions(-) create mode 100644 include/loader-proto.h delete mode 100644 include/replacements.h create mode 100644 loader-proto.c delete mode 100644 replacements.c (limited to 'globals.c') diff --git a/Makefile b/Makefile index 1c3190b9..bf339fab 100644 --- a/Makefile +++ b/Makefile @@ -38,9 +38,9 @@ CFLAGS += -DENABLE_SHIM_CERT else TARGETS += $(MMNAME) $(FBNAME) endif -OBJS = shim.o globals.o mok.o netboot.o cert.o dp.o replacements.o tpm.o version.o errlog.o sbat.o sbat_data.o sbat_var.o pe.o pe-relocate.o httpboot.o csv.o load-options.o utils.o +OBJS = shim.o globals.o mok.o netboot.o cert.o dp.o loader-proto.o tpm.o version.o errlog.o sbat.o sbat_data.o sbat_var.o pe.o pe-relocate.o httpboot.o csv.o load-options.o utils.o KEYS = shim_cert.h ocsp.* ca.* shim.crt shim.csr shim.p12 shim.pem shim.key shim.cer -ORIG_SOURCES = shim.c globals.c mok.c netboot.c dp.c replacements.c tpm.c errlog.c sbat.c pe.c pe-relocate.c httpboot.c shim.h version.h $(wildcard include/*.h) cert.S sbat_var.S +ORIG_SOURCES = shim.c globals.c mok.c netboot.c dp.c loader-proto.c tpm.c errlog.c sbat.c pe.c pe-relocate.c httpboot.c shim.h version.h $(wildcard include/*.h) cert.S sbat_var.S MOK_OBJS = MokManager.o PasswordCrypt.o crypt_blowfish.o errlog.o sbat_data.o globals.o dp.o ORIG_MOK_SOURCES = MokManager.c PasswordCrypt.c crypt_blowfish.c shim.h $(wildcard include/*.h) FALLBACK_OBJS = fallback.o tpm.o errlog.o sbat_data.o globals.o utils.o diff --git a/globals.c b/globals.c index b4e80dd3..91916e98 100644 --- a/globals.c +++ b/globals.c @@ -26,6 +26,8 @@ UINT8 *build_cert; verification_method_t verification_method; int loader_is_participating; +SHIM_IMAGE_LOADER shim_image_loader_interface; + UINT8 user_insecure_mode; UINT8 ignore_db; UINT8 trust_mok_list; diff --git a/include/loader-proto.h b/include/loader-proto.h new file mode 100644 index 00000000..d3afa2f5 --- /dev/null +++ b/include/loader-proto.h @@ -0,0 +1,35 @@ +// SPDX-License-Identifier: BSD-2-Clause-Patent + +/* + * Copyright Red Hat, Inc + * Copyright Peter Jones + */ +#ifndef SHIM_REPLACEMENTS_H +#define SHIM_REPLACEMENTS_H + +extern EFI_SYSTEM_TABLE *get_active_systab(void); + +typedef enum { + VERIFIED_BY_NOTHING, + VERIFIED_BY_CERT, + VERIFIED_BY_HASH +} 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); + +extern void hook_exit(EFI_SYSTEM_TABLE *local_systab); +extern void unhook_exit(void); + +typedef struct _SHIM_IMAGE_LOADER { + EFI_IMAGE_LOAD LoadImage; + EFI_IMAGE_START StartImage; +} SHIM_IMAGE_LOADER; + +extern SHIM_IMAGE_LOADER shim_image_loader_interface; +extern void init_image_loader(void); + +#endif /* SHIM_REPLACEMENTS_H */ diff --git a/include/replacements.h b/include/replacements.h deleted file mode 100644 index 8b35c857..00000000 --- a/include/replacements.h +++ /dev/null @@ -1,30 +0,0 @@ -// SPDX-License-Identifier: BSD-2-Clause-Patent - -/* - * Copyright Red Hat, Inc - * Copyright Peter Jones - */ -#ifndef SHIM_REPLACEMENTS_H -#define SHIM_REPLACEMENTS_H - -extern EFI_SYSTEM_TABLE *get_active_systab(void); - -typedef enum { - VERIFIED_BY_NOTHING, - VERIFIED_BY_CERT, - VERIFIED_BY_HASH -} 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); - -extern void hook_exit(EFI_SYSTEM_TABLE *local_systab); -extern void unhook_exit(void); - -extern EFI_STATUS install_shim_protocols(void); -extern void uninstall_shim_protocols(void); - -#endif /* SHIM_REPLACEMENTS_H */ 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; +} diff --git a/replacements.c b/replacements.c deleted file mode 100644 index d840616f..00000000 --- a/replacements.c +++ /dev/null @@ -1,233 +0,0 @@ -// SPDX-License-Identifier: BSD-2-Clause-Patent -/* - * shim - trivial UEFI first-stage bootloader - * - * 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 -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 -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; -} diff --git a/shim.c b/shim.c index 60b5e720..98462aa0 100644 --- a/shim.c +++ b/shim.c @@ -1314,7 +1314,6 @@ init_openssl(void) } static SHIM_LOCK shim_lock_interface; -static SHIM_IMAGE_LOADER shim_image_loader_interface; static EFI_HANDLE shim_lock_handle; EFI_STATUS @@ -1344,6 +1343,8 @@ install_shim_protocols(void) if (!EFI_ERROR(efi_status)) uninstall_shim_protocols(); + init_image_loader(); + /* * Install the protocol */ @@ -1915,91 +1916,6 @@ devel_egress(devel_egress_action action UNUSED) #endif } -static EFI_STATUS EFIAPI -shim_load_image(IN BOOLEAN BootPolicy, IN EFI_HANDLE ParentImageHandle, - IN EFI_DEVICE_PATH *FilePath, IN VOID *SourceBuffer, - IN UINTN SourceSize, OUT 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; -} - EFI_STATUS efi_main (EFI_HANDLE passed_image_handle, EFI_SYSTEM_TABLE *passed_systab) { @@ -2043,9 +1959,6 @@ efi_main (EFI_HANDLE passed_image_handle, EFI_SYSTEM_TABLE *passed_systab) shim_lock_interface.Hash = shim_hash; shim_lock_interface.Context = shim_read_header; - shim_image_loader_interface.LoadImage = shim_load_image; - shim_image_loader_interface.StartImage = shim_start_image; - systab = passed_systab; image_handle = global_image_handle = passed_image_handle; diff --git a/shim.h b/shim.h index 704e34ea..a3f8a505 100644 --- a/shim.h +++ b/shim.h @@ -174,12 +174,12 @@ #include "include/ip4config2.h" #include "include/ip6config.h" #include "include/load-options.h" +#include "include/loader-proto.h" #include "include/mok.h" #include "include/netboot.h" #include "include/passwordcrypt.h" #include "include/peimage.h" #include "include/pe.h" -#include "include/replacements.h" #include "include/sbat.h" #include "include/sbat_var_defs.h" #include "include/ssp.h" @@ -238,11 +238,6 @@ typedef struct _SHIM_LOCK { EFI_SHIM_LOCK_CONTEXT Context; } SHIM_LOCK; -typedef struct _SHIM_IMAGE_LOADER { - EFI_IMAGE_LOAD LoadImage; - EFI_IMAGE_START StartImage; -} SHIM_IMAGE_LOADER; - extern EFI_STATUS shim_init(void); extern void shim_fini(void); extern EFI_STATUS EFIAPI LogError_(const char *file, int line, const char *func, @@ -256,6 +251,8 @@ extern VOID ClearErrors(VOID); extern VOID restore_loaded_image(VOID); extern EFI_STATUS start_image(EFI_HANDLE image_handle, CHAR16 *ImagePath); extern EFI_STATUS import_mok_state(EFI_HANDLE image_handle); +extern EFI_STATUS install_shim_protocols(void); +extern void uninstall_shim_protocols(void); extern UINT32 vendor_authorized_size; extern UINT8 *vendor_authorized; @@ -332,6 +329,8 @@ verify_buffer (char *data, int datasize, char *translate_slashes(char *out, const char *str); +#include + typedef struct { EFI_LOADED_IMAGE li; EFI_IMAGE_ENTRY_POINT entry_point; -- 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 'globals.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 848667d0f3a99401d93c93b3af16b55e3fb28cea Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Wed, 15 May 2024 16:13:13 -0400 Subject: shim: add HSIStatus feature hughsie asked me if I can make shim tell userland what kinds of accesses are allowed to the heap, stack, and allocations on the running platform, so that these could be reported up through fwupd's Host Security ID program (see https://fwupd.github.io/libfwupdplugin/hsi.html ). This adds a new config-only (i.e. not a UEFI variable) variable generated during boot, "/sys/firmware/efi/mok-variables/HSIStatus", which tells us those properties as well as if the EFI Memory Attribute Protocol is present. Signed-off-by: Peter Jones --- MokVars.txt | 10 ++++++ globals.c | 1 + include/memattrs.h | 2 ++ include/mok.h | 10 ++++++ include/test-data-efivars-1.h | 6 ++++ memattrs.c | 72 +++++++++++++++++++++++++++++++++++++++++++ mok.c | 46 +++++++++++++++++++++++++++ shim.c | 1 + test-mok-mirror.c | 13 ++++++++ 9 files changed, 161 insertions(+) (limited to 'globals.c') diff --git a/MokVars.txt b/MokVars.txt index 71b42c82..e6e68ce4 100644 --- a/MokVars.txt +++ b/MokVars.txt @@ -93,3 +93,13 @@ to trust CA keys in the MokList. BS,NV MokListTrustedRT: A copy of MokListTrusted made available to the kernel at runtime. BS,RT + +HSIStatus: Status of various security features: + heap-is-executable: 0: heap allocations are not executable by default + 1: heap allocations are executable + stack-is-executable: 0: UEFI stack is not executable + 1: UEFI stack is executable + ro-sections-are-writable: 0: read-only sections are not writable + 1: read-only sections are writable + has-memory-attribute-protocol: 0: platform does not provide the EFI Memory Attribute Protocol + 1: platform does provide the EFI Memory Attribute Protocol diff --git a/globals.c b/globals.c index d574df16..de64c44c 100644 --- a/globals.c +++ b/globals.c @@ -28,6 +28,7 @@ verification_method_t verification_method; SHIM_IMAGE_LOADER shim_image_loader_interface; UINT8 user_insecure_mode; +UINTN hsi_status = 0; UINT8 ignore_db; UINT8 trust_mok_list; UINT8 mok_policy = 0; diff --git a/include/memattrs.h b/include/memattrs.h index 8fefef22..5c40b4cc 100644 --- a/include/memattrs.h +++ b/include/memattrs.h @@ -11,5 +11,7 @@ extern EFI_STATUS get_mem_attrs (uintptr_t addr, size_t size, uint64_t *attrs); extern EFI_STATUS update_mem_attrs(uintptr_t addr, uint64_t size, uint64_t set_attrs, uint64_t clear_attrs); +extern void get_hsi_mem_info(void); + #endif /* !SHIM_MEMATTRS_H_ */ // vim:fenc=utf-8:tw=75:noet diff --git a/include/mok.h b/include/mok.h index c37ccba5..e6921e09 100644 --- a/include/mok.h +++ b/include/mok.h @@ -125,5 +125,15 @@ struct mok_variable_config_entry { */ #define MOK_POLICY_REQUIRE_NX 1 +extern UINTN hsi_status; +/* heap is executable */ +#define SHIM_HSI_STATUS_HEAPX 0x00000001ULL +/* stack is executable */ +#define SHIM_HSI_STATUS_STACKX 0x00000002ULL +/* read-only sections are writable */ +#define SHIM_HSI_STATUS_ROW 0x00000004ULL +/* platform provides the EFI Memory Attribute Protocol */ +#define SHIM_HSI_STATUS_HASMAP 0x00000008ULL + #endif /* !SHIM_MOK_H_ */ // vim:fenc=utf-8:tw=75:noet diff --git a/include/test-data-efivars-1.h b/include/test-data-efivars-1.h index 2831bd23..d97a4d6d 100644 --- a/include/test-data-efivars-1.h +++ b/include/test-data-efivars-1.h @@ -106,5 +106,11 @@ static const unsigned char test_data_efivars_1_MokListTrustedRT[] ={ 0x01 }; +static const unsigned char test_data_efivars_1_HSIStatus[] = + "heap-is-executable: 0\n" + "stack-is-executable: 0\n" + "ro-sections-are-writable: 0\n" + "has-memory-attribute-protocol: 0\n"; + #endif /* !TEST_DATA_EFIVARS_1_H_ */ // vim:fenc=utf-8:tw=75:noet diff --git a/memattrs.c b/memattrs.c index 99268cd1..988459c3 100644 --- a/memattrs.c +++ b/memattrs.c @@ -164,4 +164,76 @@ update_mem_attrs(uintptr_t addr, uint64_t size, return ret; } +void +get_hsi_mem_info(void) +{ + EFI_STATUS efi_status; + uintptr_t addr; + uint64_t attrs = 0; + uint32_t *tmp_alloc; + + addr = ((uintptr_t)&get_hsi_mem_info) & ~EFI_PAGE_MASK; + + efi_status = get_mem_attrs(addr, EFI_PAGE_SIZE, &attrs); + if (EFI_ERROR(efi_status)) { +error: + /* + * In this case we can't actually tell anything, so assume + * and report the worst case scenario. + */ + hsi_status = SHIM_HSI_STATUS_HEAPX | + SHIM_HSI_STATUS_STACKX | + SHIM_HSI_STATUS_ROW; + dprint(L"Setting HSI to 0x%lx due to error: %r\n", hsi_status, efi_status); + return; + } else { + hsi_status = SHIM_HSI_STATUS_HASMAP; + dprint(L"Setting HSI to 0x%lx\n", hsi_status); + } + + if (!(hsi_status & SHIM_HSI_STATUS_HASMAP)) { + dprint(L"No memory protocol, not testing further\n"); + return; + } + + hsi_status = SHIM_HSI_STATUS_HASMAP; + if (attrs & MEM_ATTR_W) { + dprint(L"get_hsi_mem_info() is on a writable page: 0x%x->0x%x\n", + hsi_status, hsi_status | SHIM_HSI_STATUS_ROW); + hsi_status |= SHIM_HSI_STATUS_ROW; + } + + addr = ((uintptr_t)&addr) & ~EFI_PAGE_MASK; + efi_status = get_mem_attrs(addr, EFI_PAGE_SIZE, &attrs); + if (EFI_ERROR(efi_status)) { + dprint(L"get_mem_attrs(0x%016llx, 0x%x, &attrs) failed.\n", addr, EFI_PAGE_SIZE); + goto error; + } + + if (attrs & MEM_ATTR_X) { + dprint(L"Stack variable is on an executable page: 0x%x->0x%x\n", + hsi_status, hsi_status | SHIM_HSI_STATUS_STACKX); + hsi_status |= SHIM_HSI_STATUS_STACKX; + } + + tmp_alloc = AllocatePool(EFI_PAGE_SIZE); + if (!tmp_alloc) { + dprint(L"Failed to allocate heap variable.\n"); + goto error; + } + + addr = ((uintptr_t)tmp_alloc) & ~EFI_PAGE_MASK; + efi_status = get_mem_attrs(addr, EFI_PAGE_SIZE, &attrs); + FreePool(tmp_alloc); + if (EFI_ERROR(efi_status)) { + dprint(L"get_mem_attrs(0x%016llx, 0x%x, &attrs) failed.\n", addr, EFI_PAGE_SIZE); + goto error; + } + if (attrs & MEM_ATTR_X) { + dprint(L"Heap variable is on an executable page: 0x%x->0x%x\n", + hsi_status, hsi_status | SHIM_HSI_STATUS_HEAPX); + hsi_status |= SHIM_HSI_STATUS_HEAPX; + } +} + // vim:fenc=utf-8:tw=75:noet diff --git a/mok.c b/mok.c index 67a798a3..5c7f9a2b 100644 --- a/mok.c +++ b/mok.c @@ -34,6 +34,44 @@ static BOOLEAN check_var(CHAR16 *varname) efi_status_; \ }) +static UINTN +format_hsi_status(UINT8 *buf, size_t sz, + struct mok_state_variable *msv UNUSED) +{ + const char heapx[] = "heap-is-executable: "; + const char stackx[] = "\nstack-is-executable: "; + const char row[] = "\nro-sections-are-writable: "; + const char hasmap[] = "\nhas-memory-attribute-protocol: "; + const char finale[] = "\n"; + char *pos; + + /* + * sizeof includes the trailing NUL which is where our 0 or 1 value + * fits + */ + UINTN ret = sizeof(heapx) + sizeof(stackx) + + sizeof(row) + sizeof(hasmap) + + sizeof(finale); + + if (buf == 0 || sz < ret) { + return ret; + } + + buf[0] = 0; + pos = (char *)buf; + pos = stpcpy(pos, heapx); + pos = stpcpy(pos, (hsi_status & SHIM_HSI_STATUS_HEAPX) ? "1" : "0"); + pos = stpcpy(pos, stackx); + pos = stpcpy(pos, (hsi_status & SHIM_HSI_STATUS_STACKX) ? "1" : "0"); + pos = stpcpy(pos, row); + pos = stpcpy(pos, (hsi_status & SHIM_HSI_STATUS_ROW) ? "1" : "0"); + pos = stpcpy(pos, hasmap); + pos = stpcpy(pos, (hsi_status & SHIM_HSI_STATUS_HASMAP) ? "1" : "0"); + stpcpy(pos, finale); + + return ret; +} + /* * If the OS has set any of these variables we need to drop into MOK and * handle them appropriately @@ -223,6 +261,14 @@ struct mok_state_variable mok_state_variable_data[] = { .pcr = 14, .state = &mok_policy, }, + {.name = L"HSIStatus", + .name8 = "HSIStatus", + .rtname = L"HSIStatus", + .rtname8 = "HSIStatus", + .guid = &SHIM_LOCK_GUID, + .flags = MOK_VARIABLE_CONFIG_ONLY, + .format = format_hsi_status, + }, { NULL, } }; size_t n_mok_state_variables = sizeof(mok_state_variable_data) / sizeof(mok_state_variable_data[0]); diff --git a/shim.c b/shim.c index fb86589b..6afb35c5 100644 --- a/shim.c +++ b/shim.c @@ -2033,6 +2033,7 @@ efi_main (EFI_HANDLE passed_image_handle, EFI_SYSTEM_TABLE *passed_systab) } init_openssl(); + get_hsi_mem_info(); efi_status = load_unbundled_trust(global_image_handle); if (EFI_ERROR(efi_status)) { diff --git a/test-mok-mirror.c b/test-mok-mirror.c index 316424ad..38b7ed97 100644 --- a/test-mok-mirror.c +++ b/test-mok-mirror.c @@ -398,6 +398,11 @@ test_mok_mirror_with_enough_space(void) EFI_VARIABLE_RUNTIME_ACCESS, .ops = { NONE, }, }, + {.guid = SHIM_LOCK_GUID, + .name = L"HSIStatus", + .attrs = 0, + .ops = { NONE, }, + }, {.guid = { 0, }, .name = NULL, } @@ -424,6 +429,10 @@ test_mok_mirror_with_enough_space(void) .data_size = sizeof(test_data_efivars_1_MokListTrustedRT), .data = test_data_efivars_1_MokListTrustedRT }, + {.name = "HSIStatus", + .data_size = sizeof(test_data_efivars_1_HSIStatus), + .data = test_data_efivars_1_HSIStatus + }, {.name = { 0, }, .data_size = 0, .data = NULL, @@ -614,6 +623,10 @@ test_mok_mirror_setvar_out_of_resources(void) .data_size = sizeof(test_data_efivars_1_MokListTrustedRT), .data = test_data_efivars_1_MokListTrustedRT }, + {.name = "HSIStatus", + .data_size = sizeof(test_data_efivars_1_HSIStatus), + .data = test_data_efivars_1_HSIStatus + }, {.name = { 0, }, .data_size = 0, .data = NULL, -- cgit v1.2.3 From 9c423e09cd9a0196888253b16fe7022deadc20fc Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Tue, 25 Feb 2025 14:33:50 -0500 Subject: Some save_logs() improvements. In d972515e608e ("Save the debug and error logs in mok-variables") had a few deficiencies: 1) the size of the result table isn't correctly computed when either errlog or dbglog is 0 sized (much more likely for the former), 2) when we save the error log we leak the allocation for the previous mok variables, and 3) original mok variables were allocated with AllocatePages(), but the new ones were allocated with AllocateZeroPool(). The former guarantees page alignment, which we want here. This fixes all three of these. Signed-off-by: Peter Jones --- errlog.c | 64 ++++++++++++++++++++++++++++++++++++++++++++--------------- globals.c | 3 +++ include/mok.h | 3 +++ mok.c | 2 ++ 4 files changed, 56 insertions(+), 16 deletions(-) (limited to 'globals.c') diff --git a/errlog.c b/errlog.c index fc89ae0a..b43a4bc2 100644 --- a/errlog.c +++ b/errlog.c @@ -172,17 +172,42 @@ format_debug_log(UINT8 *dest, size_t dest_sz) return debug_log_sz; } +void +replace_config_table(EFI_CONFIGURATION_TABLE *CT, EFI_PHYSICAL_ADDRESS new_table, UINTN new_table_pages) +{ + EFI_GUID bogus_guid = { 0x29f2f0db, 0xd025, 0x4aa6, { 0x99, 0x58, 0xa0, 0x21, 0x8b, 0x1d, 0xec, 0x0e }}; + EFI_STATUS efi_status; + + if (CT) { + CopyMem(&CT->VendorGuid, &bogus_guid, sizeof(bogus_guid)); + if (CT->VendorTable && + CT->VendorTable == (void *)(uintptr_t)mok_config_table) { + BS->FreePages(mok_config_table, mok_config_table_pages); + CT->VendorTable = NULL; + } + } + + efi_status = BS->InstallConfigurationTable(&MOK_VARIABLE_STORE, + (void *)(uintptr_t)new_table); + if (EFI_ERROR(efi_status)) { + console_print(L"Could not re-install MoK configuration table: %r\n", efi_status); + } else { + mok_config_table = new_table; + mok_config_table_pages = new_table_pages; + } +} + void save_logs(void) { struct mok_variable_config_entry *cfg_table = NULL; struct mok_variable_config_entry *new_table = NULL; struct mok_variable_config_entry *entry = NULL; + EFI_PHYSICAL_ADDRESS physaddr = 0; + UINTN new_table_pages = 0; size_t new_table_sz; UINTN pos = 0; EFI_STATUS efi_status; - EFI_CONFIGURATION_TABLE *CT; - EFI_GUID bogus_guid = { 0x29f2f0db, 0xd025, 0x4aa6, { 0x99, 0x58, 0xa0, 0x21, 0x8b, 0x1d, 0xec, 0x0e }}; size_t errlog_sz, dbglog_sz; errlog_sz = format_error_log(NULL, 0); @@ -194,6 +219,7 @@ save_logs(void) } for (UINTN i = 0; i < ST->NumberOfTableEntries; i++) { + EFI_CONFIGURATION_TABLE *CT; CT = &ST->ConfigurationTable[i]; if (CompareGuid(&MOK_VARIABLE_STORE, &CT->VendorGuid) == 0) { @@ -208,18 +234,28 @@ save_logs(void) size_t entry_sz; entry = (struct mok_variable_config_entry *)((uintptr_t)cfg_table + pos); - entry_sz = sizeof(*entry); - entry_sz += entry->data_size; - - if (entry->name[0] != 0) + if (entry->name[0] != 0) { + entry_sz = sizeof(*entry); + entry_sz += entry->data_size; pos += entry_sz; + } } - new_table_sz = pos + 3 * sizeof(*entry) + errlog_sz + dbglog_sz; - new_table = AllocateZeroPool(new_table_sz); + new_table_sz = pos + + (errlog_sz ? sizeof(*entry) + errlog_sz : 0) + + (dbglog_sz ? sizeof(*entry) + dbglog_sz : 0) + + sizeof(*entry); + new_table = NULL; + new_table_pages = ALIGN_UP(new_table_sz + 4*EFI_PAGE_SIZE, EFI_PAGE_SIZE) / EFI_PAGE_SIZE; + efi_status = BS->AllocatePages(AllocateAnyPages, EfiRuntimeServicesData, new_table_pages, &physaddr); + if (EFI_ERROR(efi_status)) { + perror(L"Couldn't allocate %llu pages\n", new_table_pages); + return; + } + new_table = (void *)(uintptr_t)physaddr; if (!new_table) return; - + ZeroMem(new_table, new_table_pages * EFI_PAGE_SIZE); CopyMem(new_table, cfg_table, pos); entry = (struct mok_variable_config_entry *)((uintptr_t)new_table + pos); @@ -237,15 +273,11 @@ save_logs(void) format_debug_log(&entry->data[0], dbglog_sz); pos += sizeof(*entry) + dbglog_sz; - // entry = (struct mok_variable_config_entry *)((uintptr_t)new_table + pos); - } - if (CT) { - CopyMem(&CT->VendorGuid, &bogus_guid, sizeof(bogus_guid)); + entry = (struct mok_variable_config_entry *)((uintptr_t)new_table + pos); } - efi_status = BS->InstallConfigurationTable(&MOK_VARIABLE_STORE, new_table); - if (EFI_ERROR(efi_status)) - console_print(L"Could not re-install MoK configuration table: %r\n", efi_status); + + replace_config_table((EFI_CONFIGURATION_TABLE *)cfg_table, physaddr, new_table_pages); } // vim:fenc=utf-8:tw=75 diff --git a/globals.c b/globals.c index de64c44c..11197128 100644 --- a/globals.c +++ b/globals.c @@ -35,4 +35,7 @@ UINT8 mok_policy = 0; UINT32 verbose = 0; +EFI_PHYSICAL_ADDRESS mok_config_table = 0; +UINTN mok_config_table_pages = 0; + // vim:fenc=utf-8:tw=75:noet diff --git a/include/mok.h b/include/mok.h index 89edf9de..f4468ab0 100644 --- a/include/mok.h +++ b/include/mok.h @@ -127,6 +127,9 @@ struct mok_variable_config_entry { UINT8 data[]; }; +extern EFI_PHYSICAL_ADDRESS mok_config_table; +extern UINTN mok_config_table_pages; + /* * bit definitions for MokPolicy */ diff --git a/mok.c b/mok.c index fb4c1489..8367fad5 100644 --- a/mok.c +++ b/mok.c @@ -1305,6 +1305,8 @@ EFI_STATUS import_mok_state(EFI_HANDLE image_handle) config_table = NULL; } else { ZeroMem(config_table, npages << EFI_PAGE_SHIFT); + mok_config_table = (EFI_PHYSICAL_ADDRESS)(uintptr_t)config_table; + mok_config_table_pages = npages; } } -- cgit v1.2.3