From 589c3f289e05454be23507767439cb9769a2264a Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Thu, 20 Feb 2025 13:51:29 -0500 Subject: Move memory attribute support to its own file. This moves the EFI Memory Attribute Protocol helper functions to their own file, since they're not related to PE things. Signed-off-by: Peter Jones --- include/memattrs.h | 15 +++++++++++++++ 1 file changed, 15 insertions(+) create mode 100644 include/memattrs.h (limited to 'include/memattrs.h') diff --git a/include/memattrs.h b/include/memattrs.h new file mode 100644 index 00000000..8fefef22 --- /dev/null +++ b/include/memattrs.h @@ -0,0 +1,15 @@ +// SPDX-License-Identifier: BSD-2-Clause-Patent +/* + * memattrs.h - EFI and DXE memory attribute helpers + * Copyright Peter Jones + */ + +#ifndef SHIM_MEMATTRS_H_ +#define SHIM_MEMATTRS_H_ + +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); + +#endif /* !SHIM_MEMATTRS_H_ */ +// vim:fenc=utf-8:tw=75:noet -- 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 'include/memattrs.h') 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 1baf1efb37e2728104765477b12b70aeef3090af Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Tue, 25 Feb 2025 10:41:41 -0500 Subject: HSI: Add decode_hsi_bits() for easier reading of the debug log This changes all the HSI bitfield operations to print a string showing the change instead of just hex values. Signed-off-by: Peter Jones --- include/memattrs.h | 1 + memattrs.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++-------- 2 files changed, 68 insertions(+), 11 deletions(-) (limited to 'include/memattrs.h') diff --git a/include/memattrs.h b/include/memattrs.h index 5c40b4cc..193da988 100644 --- a/include/memattrs.h +++ b/include/memattrs.h @@ -12,6 +12,7 @@ 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); +extern char *decode_hsi_bits(UINTN hsi); #endif /* !SHIM_MEMATTRS_H_ */ // vim:fenc=utf-8:tw=75:noet diff --git a/memattrs.c b/memattrs.c index f502805f..bc571641 100644 --- a/memattrs.c +++ b/memattrs.c @@ -400,6 +400,46 @@ get_mem_attrs(uintptr_t addr, size_t size, uint64_t *attrs) return efi_status; } +char * +decode_hsi_bits(UINTN hsi) +{ + static const struct { + UINTN bit; + char name[16]; + } bits[] = { + {.bit = SHIM_HSI_STATUS_HEAPX, .name = "HEAPX"}, + {.bit = SHIM_HSI_STATUS_STACKX, .name = "STACKX"}, + {.bit = SHIM_HSI_STATUS_ROW, .name = "ROW"}, + {.bit = SHIM_HSI_STATUS_HASMAP, .name = "HASMAP"}, + {.bit = SHIM_HSI_STATUS_HASDST, .name = "HASDST"}, + {.bit = SHIM_HSI_STATUS_HASDSTGMSD, .name = "HASDSTGMSD"}, + {.bit = SHIM_HSI_STATUS_HASDSTSMSA, .name = "HASDSTSMSA"}, + {.bit = 0, .name = ""}, + }; + static int x = 0; + static char retbufs[2][sizeof(bits)]; + char *retbuf = &retbufs[x % 2][0]; + char *prev = &retbuf[0]; + char *pos = &retbuf[0]; + + x = ( x + 1 ) % 2; + + ZeroMem(retbuf, sizeof(bits)); + + if (hsi == 0) { + prev = stpcpy(retbuf, "0"); + } else { + for (UINTN i = 0; bits[i].bit != 0; i++) { + if (hsi & bits[i].bit) { + prev = stpcpy(pos, bits[i].name); + pos = stpcpy(prev, "|"); + } + } + } + prev[0] = '\0'; + return retbuf; +} + void get_hsi_mem_info(void) { @@ -412,17 +452,30 @@ get_hsi_mem_info(void) get_efi_mem_attr_protocol(&efiproto); if (efiproto) { - hsi_status = SHIM_HSI_STATUS_HASMAP; - dprint(L"Setting HSI to 0x%lx\n", hsi_status); + dprint(L"Setting HSI from %a to %a\n", + decode_hsi_bits(hsi_status), + decode_hsi_bits(hsi_status | SHIM_HSI_STATUS_HASMAP)); + hsi_status |= SHIM_HSI_STATUS_HASMAP; } get_dxe_services_table(&dst); if (dst) { + dprint(L"Setting HSI from %a to %a\n", + decode_hsi_bits(hsi_status), + decode_hsi_bits(hsi_status | SHIM_HSI_STATUS_HASDST)); hsi_status |= SHIM_HSI_STATUS_HASDST; - if (dst->GetMemorySpaceDescriptor) + if (dst->GetMemorySpaceDescriptor) { + dprint(L"Setting HSI from %a to %a\n", + decode_hsi_bits(hsi_status), + decode_hsi_bits(hsi_status | SHIM_HSI_STATUS_HASDSTGMSD)); hsi_status |= SHIM_HSI_STATUS_HASDSTGMSD; - if (dst->SetMemorySpaceAttributes) + } + if (dst->SetMemorySpaceAttributes) { + dprint(L"Setting HSI from %a to %a\n", + decode_hsi_bits(hsi_status), + decode_hsi_bits(hsi_status | SHIM_HSI_STATUS_HASDSTSMSA)); hsi_status |= SHIM_HSI_STATUS_HASDSTSMSA; + } } if (!(hsi_status & SHIM_HSI_STATUS_HASMAP) && @@ -440,8 +493,9 @@ get_hsi_mem_info(void) } 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); + dprint(L"get_hsi_mem_info() is on a writable page: %a->%a\n", + decode_hsi_bits(hsi_status), + decode_hsi_bits(hsi_status | SHIM_HSI_STATUS_ROW)); hsi_status |= SHIM_HSI_STATUS_ROW; } @@ -453,8 +507,9 @@ get_hsi_mem_info(void) } 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); + dprint(L"Stack variable is on an executable page: %a->%a\n", + decode_hsi_bits(hsi_status), + decode_hsi_bits(hsi_status | SHIM_HSI_STATUS_STACKX)); hsi_status |= SHIM_HSI_STATUS_STACKX; } @@ -472,8 +527,9 @@ get_hsi_mem_info(void) 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); + dprint(L"Heap variable is on an executable page: %a->%a\n", + decode_hsi_bits(hsi_status), + decode_hsi_bits(hsi_status | SHIM_HSI_STATUS_HEAPX)); hsi_status |= SHIM_HSI_STATUS_HEAPX; } @@ -487,7 +543,7 @@ error: 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); + dprint(L"Setting HSI to 0x%lx due to error: %r\n", decode_hsi_bits(hsi_status), efi_status); } // vim:fenc=utf-8:tw=75:noet -- cgit v1.2.3