From b446211e24f76c0c8e10698897ad155384719468 Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Wed, 15 May 2024 16:13:13 -0400 Subject: [PATCH] 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/mok.h | 12 ++++++++++- include/pe.h | 2 ++ mok.c | 48 +++++++++++++++++++++++++++++++++++++++++ pe.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++++++- 6 files changed, 130 insertions(+), 2 deletions(-) diff --git a/MokVars.txt b/MokVars.txt index baf8db9a5..320377fc9 100644 --- a/MokVars.txt +++ b/MokVars.txt @@ -88,3 +88,13 @@ to trust CA keys in the MokList. BS,NV MokListTrustedRT: A copy of MokListTrusted made available to the kernel at runtime. 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 b4e80dd3a..08d81c8fe 100644 --- a/globals.c +++ b/globals.c @@ -27,6 +27,7 @@ verification_method_t verification_method; int loader_is_participating; UINT8 user_insecure_mode; +UINTN hsi_status = 0; UINT8 ignore_db; UINT8 trust_mok_list; UINT8 mok_policy = 0; diff --git a/include/mok.h b/include/mok.h index b6b1d586f..e6921e092 100644 --- a/include/mok.h +++ b/include/mok.h @@ -17,7 +17,7 @@ typedef enum { struct mok_state_variable; typedef vendor_addend_category_t (vendor_addend_categorizer_t)(struct mok_state_variable *); -typedef UINTN (mok_variable_format_helper_t)(char *buf, size_t sz, struct mok_state_variable *); +typedef UINTN (mok_variable_format_helper_t)(UINT8 *buf, size_t sz, struct mok_state_variable *); /* * MoK variables that need to have their storage validated. @@ -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/pe.h b/include/pe.h index 9ea9eb446..31f829f9d 100644 --- a/include/pe.h +++ b/include/pe.h @@ -52,5 +52,7 @@ relocate_coff (PE_COFF_LOADER_IMAGE_CONTEXT *context, EFI_IMAGE_SECTION_HEADER *Section, void *orig, void *data); +void get_hsi_mem_info(void); + #endif /* !PE_H_ */ // vim:fenc=utf-8:tw=75:noet diff --git a/mok.c b/mok.c index 547bfd201..4738214dc 100644 --- a/mok.c +++ b/mok.c @@ -34,6 +34,46 @@ static BOOLEAN check_var(CHAR16 *varname) efi_status_; \ }) +static UINTN +format_hsi_status(UINT8 *buf, size_t sz, + struct mok_state_variable *msv UNUSED) +{ + UINT8 heapx[] = "heap-is-executable: X\n"; + UINT8 stackx[] = "stack-is-executable: X\n"; + UINT8 row[] = "ro-sections-are-writable: X\n"; + UINT8 hasmap[] = "has-memory-attribute-protocol: X\n"; + UINTN pos; + + UINTN ret = sizeof(heapx) + sizeof(stackx) + + sizeof(row) + sizeof(hasmap) - 4; + + if (buf == 0 || sz < ret) { + return ret; + } + + pos = sizeof(heapx) - 3; + heapx[pos] = (hsi_status & SHIM_HSI_STATUS_HEAPX) ? '1' : '0'; + + pos = sizeof(stackx) - 3; + stackx[pos] = (hsi_status & SHIM_HSI_STATUS_STACKX) ? '1' : '0'; + + pos = sizeof(row) - 3; + row[pos] = (hsi_status & SHIM_HSI_STATUS_ROW) ? '1' : '0'; + + pos = sizeof(hasmap) - 3; + hasmap[pos] = (hsi_status & SHIM_HSI_STATUS_HEAPX) ? '1' : '0'; + + memcpy(buf, heapx, sizeof(heapx) - 1); + pos = sizeof(heapx) - 1; + memcpy(buf+pos, stackx, sizeof(stackx) - 1); + pos += sizeof(stackx) - 1; + memcpy(buf+pos, row, sizeof(row) - 1); + pos += sizeof(row) - 1; + memcpy(buf+pos, hasmap, sizeof(hasmap) - 1); + + return ret; +} + /* * If the OS has set any of these variables we need to drop into MOK and * handle them appropriately @@ -197,6 +237,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/pe.c b/pe.c index 330560171..fd55419e7 100644 --- a/pe.c +++ b/pe.c @@ -442,8 +442,11 @@ get_mem_attrs (uintptr_t addr, size_t size, uint64_t *attrs) efi_status = LibLocateProtocol(&EFI_MEMORY_ATTRIBUTE_PROTOCOL_GUID, (VOID **)&proto); - if (EFI_ERROR(efi_status) || !proto) + if (EFI_ERROR(efi_status) || !proto) { + if (!EFI_ERROR(efi_status)) + efi_status = EFI_UNSUPPORTED; return efi_status; + } if (!IS_PAGE_ALIGNED(physaddr) || !IS_PAGE_ALIGNED(size) || size == 0 || attrs == NULL) { dprint(L"%a called on 0x%llx-0x%llx and attrs 0x%llx\n", @@ -943,4 +946,58 @@ handle_image (void *data, unsigned int datasize, return EFI_SUCCESS; } +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; + return; + } + + hsi_status = SHIM_HSI_STATUS_HASMAP; + if (attrs & MEM_ATTR_W) { + 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)) { + goto error; + } + + if (attrs & MEM_ATTR_X) { + hsi_status |= SHIM_HSI_STATUS_STACKX; + } + + tmp_alloc = AllocatePool(EFI_PAGE_SIZE); + if (!tmp_alloc) { + goto error; + } + + addr = ((uintptr_t)tmp_alloc) & ~EFI_PAGE_MASK; + efi_status = get_mem_attrs(addr, EFI_PAGE_MASK, &attrs); + FreePool(tmp_alloc); + if (EFI_ERROR(efi_status)) { + goto error; + } + if (attrs & MEM_ATTR_X) { + hsi_status |= SHIM_HSI_STATUS_HEAPX; + } +} + // vim:fenc=utf-8:tw=75:noet