Skip to content

Commit

Permalink
shim: add HSIStatus feature
Browse files Browse the repository at this point in the history
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 <pjones@redhat.com>
  • Loading branch information
vathpela committed Feb 21, 2025
1 parent c2c66aa commit 8b876bc
Show file tree
Hide file tree
Showing 9 changed files with 161 additions and 0 deletions.
10 changes: 10 additions & 0 deletions MokVars.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
1 change: 1 addition & 0 deletions globals.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 2 additions & 0 deletions include/memattrs.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
10 changes: 10 additions & 0 deletions include/mok.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
6 changes: 6 additions & 0 deletions include/test-data-efivars-1.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
72 changes: 72 additions & 0 deletions memattrs.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
46 changes: 46 additions & 0 deletions mok.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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]);
Expand Down
1 change: 1 addition & 0 deletions shim.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand Down
13 changes: 13 additions & 0 deletions test-mok-mirror.c
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit 8b876bc

Please sign in to comment.