From 2f8268dc27e01e268a72363e99f79aa611a54653 Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Wed, 15 May 2024 16:13:40 -0400 Subject: [PATCH 1/3] mok: add MOK_VARIABLE_CONFIG_ONLY This adds a mok variable flag "MOK_VARIABLE_CONFIG_ONLY" to specify that the data should be added to our UEFI config table, but shim should not create a legacy UEFI variable. Signed-off-by: Peter Jones --- include/mok.h | 2 ++ mok.c | 20 ++++++++++++-------- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/include/mok.h b/include/mok.h index fb19423b6..fe92cf033 100644 --- a/include/mok.h +++ b/include/mok.h @@ -81,6 +81,8 @@ struct mok_state_variable { * MOK_MIRROR_DELETE_FIRST delete any existing variable first * MOK_VARIABLE_MEASURE extend PCR 7 and log the hash change * MOK_VARIABLE_LOG measure into whatever .pcr says and log + * MOK_VARIABLE_CONFIG_ONLY don't create a UEFI variable, only add + * it to the config space variables. */ UINTN pcr; /* PCR to measure and hash to */ diff --git a/mok.c b/mok.c index 0ac341581..a3f9f50b0 100644 --- a/mok.c +++ b/mok.c @@ -80,11 +80,12 @@ categorize_deauthorized(struct mok_state_variable *v) return VENDOR_ADDEND_DB; } -#define MOK_MIRROR_KEYDB 0x01 -#define MOK_MIRROR_DELETE_FIRST 0x02 -#define MOK_VARIABLE_MEASURE 0x04 -#define MOK_VARIABLE_LOG 0x08 -#define MOK_VARIABLE_INVERSE 0x10 +#define MOK_MIRROR_KEYDB 0x01 +#define MOK_MIRROR_DELETE_FIRST 0x02 +#define MOK_VARIABLE_MEASURE 0x04 +#define MOK_VARIABLE_LOG 0x08 +#define MOK_VARIABLE_INVERSE 0x10 +#define MOK_VARIABLE_CONFIG_ONLY 0x20 struct mok_state_variable mok_state_variable_data[] = { {.name = L"MokList", @@ -767,7 +768,8 @@ 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 (FullDataSize && v->flags & MOK_MIRROR_KEYDB) { + if (FullDataSize && v->flags & MOK_MIRROR_KEYDB && + !(v->flags & MOK_VARIABLE_CONFIG_ONLY)) { dprint(L"calling mirror_mok_db(\"%s\", datasz=%lu)\n", v->rtname, FullDataSize); efi_status = mirror_mok_db(v->rtname, (CHAR8 *)v->rtname8, v->guid, @@ -775,7 +777,8 @@ mirror_one_mok_variable(struct mok_state_variable *v, only_first); dprint(L"mirror_mok_db(\"%s\", datasz=%lu) returned %r\n", v->rtname, FullDataSize, efi_status); - } else if (FullDataSize && only_first) { + } else if (FullDataSize && only_first && + !(v->flags & MOK_VARIABLE_CONFIG_ONLY)) { efi_status = SetVariable(v->rtname, v->guid, attrs, FullDataSize, FullData); } @@ -871,7 +874,8 @@ EFI_STATUS import_one_mok_state(struct mok_state_variable *v, dprint(L"importing mok state for \"%s\"\n", v->name); - if (!v->data && !v->data_size) { + if (!v->data && !v->data_size && + !(v->flags & MOK_VARIABLE_CONFIG_ONLY)) { efi_status = get_variable_attr(v->name, &v->data, &v->data_size, *v->guid, &attrs); From 1774e0f05de5a15671a21d0cc5365aff3e9dc8ac Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Wed, 15 May 2024 16:01:34 -0400 Subject: [PATCH 2/3] mok variables: add a format callback This adds a member to the mok_state_variable struct to provide a callback function for formatting external data. It basically has snprintf()-like semantics for filling the buffer, but without the actual printf-like formatting bits. Signed-off-by: Peter Jones --- include/mok.h | 18 ++++++++++++++++++ mok.c | 14 ++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/include/mok.h b/include/mok.h index fe92cf033..b6b1d586f 100644 --- a/include/mok.h +++ b/include/mok.h @@ -17,6 +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 *); /* * MoK variables that need to have their storage validated. @@ -91,6 +92,23 @@ struct mok_state_variable { * mirrored. */ UINT8 *state; + + /* + * If this is non-NULL, this function will be called during the + * "import" phase to format the variable data. It'll get called + * twice, once as: + * + * sz = format(NULL, 0, ptr); + * + * a buffer of size sz will then be allocated, and it'll be called + * again to fill the buffer: + * + * format(buf, sz, ptr); + * + * Note that as an implementation detail data and data_size must be + * NULL and 0 respectively for this entry. + */ + mok_variable_format_helper_t *format; }; extern size_t n_mok_state_variables; diff --git a/mok.c b/mok.c index a3f9f50b0..547bfd201 100644 --- a/mok.c +++ b/mok.c @@ -917,6 +917,20 @@ EFI_STATUS import_one_mok_state(struct mok_state_variable *v, } } } + + if (v->format) { + v->data_size = v->format(NULL, 0, v); + if (v->data_size > 0) { + v->data = AllocatePool(v->data_size); + if (!v->data) { + perror(L"Could not allocate %lu bytes for %s\n", + v->data_size, v->name); + return EFI_OUT_OF_RESOURCES; + } + } + v->format(v->data, v->data_size, v); + } + if (delete == TRUE) { perror(L"Deleting bad variable %s\n", v->name); efi_status = LibDeleteVariable(v->name, v->guid); From d20c801842538f8ebaf007d1cbb24916c0542479 Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Wed, 15 May 2024 16:13:13 -0400 Subject: [PATCH 3/3] 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 ++ include/test-data-efivars-1.h | 6 ++++ mok.c | 50 +++++++++++++++++++++++++++++ pe.c | 59 ++++++++++++++++++++++++++++++++++- test-mok-mirror.c | 4 +++ 8 files changed, 142 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/include/test-data-efivars-1.h b/include/test-data-efivars-1.h index 2831bd231..d97a4d6da 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/mok.c b/mok.c index 547bfd201..1759ed9f7 100644 --- a/mok.c +++ b/mok.c @@ -34,6 +34,48 @@ 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) - 3; + + 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); + pos += sizeof(hasmap) - 1; + buf[pos] = '\0'; + + return ret; +} + /* * If the OS has set any of these variables we need to drop into MOK and * handle them appropriately @@ -197,6 +239,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 diff --git a/test-mok-mirror.c b/test-mok-mirror.c index f34f62a97..eb8069f87 100644 --- a/test-mok-mirror.c +++ b/test-mok-mirror.c @@ -188,6 +188,10 @@ test_mok_mirror_0(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,