From 5aca36c849555d5d706d2ba8783d28098dce1f87 Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Thu, 2 Dec 2021 17:14:51 -0500 Subject: [PATCH 1/6] post-process-pe: there is no 's' argument. There is no 's' argument to post-process-pe, so we shouldn't tell getopt that there is. This patch takes the 's' out of the getopt short option list. Signed-off-by: Peter Jones --- post-process-pe.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/post-process-pe.c b/post-process-pe.c index daacf5ef7..e848f0094 100644 --- a/post-process-pe.c +++ b/post-process-pe.c @@ -474,7 +474,7 @@ int main(int argc, char **argv) }; int longindex = -1; - while ((i = getopt_long(argc, argv, "hqsv", options, &longindex)) != -1) { + while ((i = getopt_long(argc, argv, "hqv", options, &longindex)) != -1) { switch (i) { case 'h': case '?': From 7abca5ebc0f7ec6add2bc79004b9ba9fdc0ffe5a Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Thu, 2 Dec 2021 17:04:23 -0500 Subject: [PATCH 2/6] gnu-efi: Add EFI_MEMORY_ATTRIBUTE protocol This patch adds the EFI_MEMORY_ATTRIBUTE protocol to gnu-efi, as well as some associated missing definitions. Signed-off-by: Peter Jones From a6b9a0589ca1fcab560a2c623b33006fc51b28d1 Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Thu, 2 Dec 2021 17:43:29 -0500 Subject: [PATCH 3/6] Add some missing PE image flag definitions This patch adds some missing definitions for PE header flags. We don't use all of them, but it's less confusing with the list matching the spec, except where the spec is obviously wrong. Signed-off-by: Peter Jones --- include/peimage.h | 54 +++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 47 insertions(+), 7 deletions(-) diff --git a/include/peimage.h b/include/peimage.h index 3b3f01a79..4bcb940d5 100644 --- a/include/peimage.h +++ b/include/peimage.h @@ -236,6 +236,24 @@ typedef struct { EFI_IMAGE_DATA_DIRECTORY DataDirectory[EFI_IMAGE_NUMBER_OF_DIRECTORY_ENTRIES]; } EFI_IMAGE_OPTIONAL_HEADER64; +#define EFI_IMAGE_DLLCHARACTERISTICS_RESERVED_0001 0x0001 +#define EFI_IMAGE_DLLCHARACTERISTICS_RESERVED_0002 0x0002 +#define EFI_IMAGE_DLLCHARACTERISTICS_RESERVED_0004 0x0004 +#define EFI_IMAGE_DLLCHARACTERISTICS_RESERVED_0008 0x0008 +#if 0 /* This is not in the PE spec. */ +#define EFI_IMAGE_DLLCHARACTERISTICS_RESERVED_0010 0x0010 +#endif +#define EFI_IMAGE_DLLCHARACTERISTICS_HIGH_ENTROPY_VA 0x0020 +#define EFI_IMAGE_DLLCHARACTERISTICS_DYNAMIC_BASE 0x0040 +#define EFI_IMAGE_DLLCHARACTERISTICS_FORCE_INTEGRITY 0x0080 +#define EFI_IMAGE_DLLCHARACTERISTICS_NX_COMPAT 0x0100 +#define EFI_IMAGE_DLLCHARACTERISTICS_NO_ISOLATION 0x0200 +#define EFI_IMAGE_DLLCHARACTERISTICS_NO_SEH 0x0400 +#define EFI_IMAGE_DLLCHARACTERISTICS_NO_BIND 0x0800 +#define EFI_IMAGE_DLLCHARACTERISTICS_APPCONTAINER 0x1000 +#define EFI_IMAGE_DLLCHARACTERISTICS_WDM_DRIVER 0x2000 +#define EFI_IMAGE_DLLCHARACTERISTICS_GUARD_CF 0x4000 +#define EFI_IMAGE_DLLCHARACTERISTICS_TERMINAL_SERVER_AWARE 0x8000 /// /// @attention @@ -303,16 +321,31 @@ typedef struct { // // Section Flags Values // -#define EFI_IMAGE_SCN_TYPE_NO_PAD 0x00000008 ///< Reserved. +#define EFI_IMAGE_SCN_RESERVED_00000000 0x00000000 +#define EFI_IMAGE_SCN_RESERVED_00000001 0x00000001 +#define EFI_IMAGE_SCN_RESERVED_00000002 0x00000002 +#define EFI_IMAGE_SCN_RESERVED_00000004 0x00000004 +#define EFI_IMAGE_SCN_TYPE_NO_PAD 0x00000008 +#define EFI_IMAGE_SCN_RESERVED_00000010 0x00000010 #define EFI_IMAGE_SCN_CNT_CODE 0x00000020 #define EFI_IMAGE_SCN_CNT_INITIALIZED_DATA 0x00000040 #define EFI_IMAGE_SCN_CNT_UNINITIALIZED_DATA 0x00000080 - -#define EFI_IMAGE_SCN_LNK_OTHER 0x00000100 ///< Reserved. -#define EFI_IMAGE_SCN_LNK_INFO 0x00000200 ///< Section contains comments or some other type of information. -#define EFI_IMAGE_SCN_LNK_REMOVE 0x00000800 ///< Section contents will not become part of image. +#define EFI_IMAGE_SCN_LNK_OTHER 0x00000100 +#define EFI_IMAGE_SCN_LNK_INFO 0x00000200 +#define EFI_IMAGE_SCN_RESERVED_00000400 0x00000400 +#define EFI_IMAGE_SCN_LNK_REMOVE 0x00000800 #define EFI_IMAGE_SCN_LNK_COMDAT 0x00001000 - +#define EFI_IMAGE_SCN_RESERVED_00002000 0x00002000 +#define EFI_IMAGE_SCN_RESERVED_00004000 0x00004000 +#define EFI_IMAGE_SCN_GPREL 0x00008000 +/* + * PE 9.3 says both IMAGE_SCN_MEM_PURGEABLE and IMAGE_SCN_MEM_16BIT are + * 0x00020000, but I think it's wrong. --pjones + */ +#define EFI_IMAGE_SCN_MEM_PURGEABLE 0x00010000 // "Reserved for future use." +#define EFI_IMAGE_SCN_MEM_16BIT 0x00020000 // "Reserved for future use." +#define EFI_IMAGE_SCN_MEM_LOCKED 0x00040000 // "Reserved for future use." +#define EFI_IMAGE_SCN_MEM_PRELOAD 0x00080000 // "Reserved for future use." #define EFI_IMAGE_SCN_ALIGN_1BYTES 0x00100000 #define EFI_IMAGE_SCN_ALIGN_2BYTES 0x00200000 #define EFI_IMAGE_SCN_ALIGN_4BYTES 0x00300000 @@ -320,7 +353,14 @@ typedef struct { #define EFI_IMAGE_SCN_ALIGN_16BYTES 0x00500000 #define EFI_IMAGE_SCN_ALIGN_32BYTES 0x00600000 #define EFI_IMAGE_SCN_ALIGN_64BYTES 0x00700000 - +#define EFI_IMAGE_SCN_ALIGN_128BYTES 0x00800000 +#define EFI_IMAGE_SCN_ALIGN_256BYTES 0x00900000 +#define EFI_IMAGE_SCN_ALIGN_512BYTES 0x00a00000 +#define EFI_IMAGE_SCN_ALIGN_1024BYTES 0x00b00000 +#define EFI_IMAGE_SCN_ALIGN_2048BYTES 0x00c00000 +#define EFI_IMAGE_SCN_ALIGN_4096BYTES 0x00d00000 +#define EFI_IMAGE_SCN_ALIGN_8192BYTES 0x00e00000 +#define EFI_IMAGE_SCN_LNK_NRELOC_OVFL 0x01000000 #define EFI_IMAGE_SCN_MEM_DISCARDABLE 0x02000000 #define EFI_IMAGE_SCN_MEM_NOT_CACHED 0x04000000 #define EFI_IMAGE_SCN_MEM_NOT_PAGED 0x08000000 From 825d99361b4aaa16144392dc6cea43e24c8472ae Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Thu, 2 Dec 2021 18:29:50 -0500 Subject: [PATCH 4/6] PE Loader: support and require NX This adds support in our PE loader for NX support utilizing the EFI_MEMORY_ATTRIBUTE protocol. Specifically, it changes the loader such that: - binaries without the EFI_IMAGE_DLLCHARACTERISTICS_NX_COMPAT flag set in the Optional Header are rejected as EFI_UNSUPPORTED - binaries with non-discardable sections that have both the EFI_SCN_MEM_WRITE and EFI_SCN_MEM_EXECUTE flags set are rejected as EFI_UNSUPPORTED - if the EFI_MEMORY_ATTRIBUTE protocol is installed, then: - sections without the EFI_SCN_MEM_READ flag set will be marked with EFI_MEMORY_RP - sections without the EFI_SCN_MEM_WRITE flag set will be marked with EFI_MEMORY_RO - sections without the EFI_SCN_MEM_EXECUTE flag set will be marked with EFI_MEMORY_XP Signed-off-by: Peter Jones --- include/guid.h | 2 +- lib/guid.c | 2 +- pe.c | 203 +++++++++++++++++++++++++++++++++++++++++++++++++ shim.h | 4 + 4 files changed, 209 insertions(+), 2 deletions(-) diff --git a/include/guid.h b/include/guid.h index 07a19a915..d9910ff1a 100644 --- a/include/guid.h +++ b/include/guid.h @@ -33,8 +33,8 @@ extern EFI_GUID EFI_SECURE_BOOT_DB_GUID; extern EFI_GUID EFI_SIMPLE_FILE_SYSTEM_GUID; extern EFI_GUID SECURITY_PROTOCOL_GUID; extern EFI_GUID SECURITY2_PROTOCOL_GUID; +extern EFI_GUID EFI_MEMORY_ATTRIBUTE_PROTOCOL_GUID; extern EFI_GUID SHIM_LOCK_GUID; - extern EFI_GUID MOK_VARIABLE_STORE; #endif /* SHIM_GUID_H */ diff --git a/lib/guid.c b/lib/guid.c index 143e0bbd3..e100c92ed 100644 --- a/lib/guid.c +++ b/lib/guid.c @@ -32,6 +32,6 @@ EFI_GUID EFI_SECURE_BOOT_DB_GUID = { 0xd719b2cb, 0x3d3a, 0x4596, { 0xa3, 0xbc, EFI_GUID EFI_SIMPLE_FILE_SYSTEM_GUID = SIMPLE_FILE_SYSTEM_PROTOCOL; EFI_GUID SECURITY_PROTOCOL_GUID = { 0xA46423E3, 0x4617, 0x49f1, {0xB9, 0xFF, 0xD1, 0xBF, 0xA9, 0x11, 0x58, 0x39 } }; EFI_GUID SECURITY2_PROTOCOL_GUID = { 0x94ab2f58, 0x1438, 0x4ef1, {0x91, 0x52, 0x18, 0x94, 0x1a, 0x3a, 0x0e, 0x68 } }; - +EFI_GUID EFI_MEMORY_ATTRIBUTE_PROTOCOL_GUID = { 0xf4560cf6, 0x40ec, 0x4b4a, {0xa1, 0x92, 0xbf, 0x1d, 0x57, 0xd0, 0xb1, 0x89} }; EFI_GUID SHIM_LOCK_GUID = {0x605dab50, 0xe046, 0x4300, {0xab, 0xb6, 0x3d, 0xd8, 0x10, 0xdd, 0x8b, 0x23 } }; EFI_GUID MOK_VARIABLE_STORE = {0xc451ed2b, 0x9694, 0x45d3, {0xba, 0xba, 0xed, 0x9f, 0x89, 0x88, 0xa3, 0x89} }; diff --git a/pe.c b/pe.c index 535d463a7..9fa6fffdc 100644 --- a/pe.c +++ b/pe.c @@ -696,6 +696,7 @@ read_header(void *data, unsigned int datasize, EFI_IMAGE_OPTIONAL_HEADER_UNION *PEHdr = data; unsigned long HeaderWithoutDataDir, SectionHeaderOffset, OptHeaderSize; unsigned long FileAlignment = 0; + UINT16 DllFlags; if (datasize < sizeof (PEHdr->Pe32)) { perror(L"Invalid image\n"); @@ -790,13 +791,20 @@ read_header(void *data, unsigned int datasize, context->EntryPoint = PEHdr->Pe32Plus.OptionalHeader.AddressOfEntryPoint; context->RelocDir = &PEHdr->Pe32Plus.OptionalHeader.DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_BASERELOC]; context->SecDir = &PEHdr->Pe32Plus.OptionalHeader.DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_SECURITY]; + DllFlags = PEHdr->Pe32Plus.OptionalHeader.DllCharacteristics; } else { context->ImageAddress = PEHdr->Pe32.OptionalHeader.ImageBase; context->EntryPoint = PEHdr->Pe32.OptionalHeader.AddressOfEntryPoint; context->RelocDir = &PEHdr->Pe32.OptionalHeader.DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_BASERELOC]; context->SecDir = &PEHdr->Pe32.OptionalHeader.DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_SECURITY]; + DllFlags = PEHdr->Pe32.OptionalHeader.DllCharacteristics; } + if (!(DllFlags & EFI_IMAGE_DLLCHARACTERISTICS_NX_COMPAT)) { + perror(L"Image does not support NX\n"); + return EFI_UNSUPPORTED; + } + context->FirstSection = (EFI_IMAGE_SECTION_HEADER *)((char *)PEHdr + PEHdr->Pe32.FileHeader.SizeOfOptionalHeader + sizeof(UINT32) + sizeof(EFI_IMAGE_FILE_HEADER)); if (context->ImageSize < context->SizeOfHeaders) { @@ -878,6 +886,139 @@ verify_sbat_section(char *SBATBase, size_t SBATSize) return efi_status; } +static inline uint64_t +shim_mem_attrs_to_uefi_mem_attrs (uint64_t attrs) +{ + uint64_t ret = EFI_MEMORY_RP | + EFI_MEMORY_RO | + EFI_MEMORY_XP; + + if (attrs & MEM_ATTR_R) + ret &= ~EFI_MEMORY_RP; + + if (attrs & MEM_ATTR_W) + ret &= ~EFI_MEMORY_RO; + + if (attrs & MEM_ATTR_X) + ret &= ~EFI_MEMORY_XP; + + return ret; +} + +static inline uint64_t +uefi_mem_attrs_to_shim_mem_attrs (uint64_t attrs) +{ + uint64_t ret = MEM_ATTR_R | + MEM_ATTR_W | + MEM_ATTR_X; + + if (attrs & EFI_MEMORY_RP) + ret &= ~MEM_ATTR_R; + + if (attrs & EFI_MEMORY_RO) + ret &= ~MEM_ATTR_W; + + if (attrs & EFI_MEMORY_XP) + ret &= ~MEM_ATTR_X; + + return ret; +} + +static EFI_STATUS +get_mem_attrs (uintptr_t addr, size_t size, uint64_t *attrs) +{ + EFI_MEMORY_ATTRIBUTE_PROTOCOL *proto = NULL; + EFI_PHYSICAL_ADDRESS physaddr = addr; + EFI_STATUS efi_status; + + efi_status = LibLocateProtocol(&EFI_MEMORY_ATTRIBUTE_PROTOCOL_GUID, + (VOID **)&proto); + if (EFI_ERROR(efi_status) || !proto) + return efi_status; + + if (physaddr & 0xfff || size & 0xfff || size == 0 || attrs == NULL) { + dprint(L"%a called on 0x%llx-0x%llx and attrs 0x%llx\n", + __func__, (unsigned long long)physaddr, + (unsigned long long)(physaddr+size-1), + attrs); + return EFI_SUCCESS; + } + + efi_status = proto->GetMemoryAttributes(proto, physaddr, size, attrs); + *attrs = uefi_mem_attrs_to_shim_mem_attrs (*attrs); + + return efi_status; +} + +static EFI_STATUS +update_mem_attrs(uintptr_t addr, uint64_t size, + uint64_t set_attrs, uint64_t clear_attrs) +{ + EFI_MEMORY_ATTRIBUTE_PROTOCOL *proto = NULL; + EFI_PHYSICAL_ADDRESS physaddr = addr; + EFI_STATUS efi_status, ret; + uint64_t before = 0, after = 0, uefi_set_attrs, uefi_clear_attrs; + + efi_status = LibLocateProtocol(&EFI_MEMORY_ATTRIBUTE_PROTOCOL_GUID, + (VOID **)&proto); + if (EFI_ERROR(efi_status) || !proto) + return efi_status; + + efi_status = get_mem_attrs (addr, size, &before); + if (EFI_ERROR(efi_status)) + dprint(L"get_mem_attrs(0x%llx, 0x%llx, 0x%llx) -> 0x%lx\n", + (unsigned long long)addr, (unsigned long long)size, + &before, efi_status); + + if (physaddr & 0xfff || size & 0xfff || size == 0) { + dprint(L"%a called on 0x%llx-0x%llx (size 0x%llx) +%a%a%a -%a%a%a\n", + __func__, (unsigned long long)physaddr, + (unsigned long long)(physaddr + size - 1), + (unsigned long long)size, + (set_attrs & MEM_ATTR_R) ? "r" : "", + (set_attrs & MEM_ATTR_W) ? "w" : "", + (set_attrs & MEM_ATTR_X) ? "x" : "", + (clear_attrs & MEM_ATTR_R) ? "r" : "", + (clear_attrs & MEM_ATTR_W) ? "w" : "", + (clear_attrs & MEM_ATTR_X) ? "x" : ""); + return 0; + } + + uefi_set_attrs = shim_mem_attrs_to_uefi_mem_attrs (set_attrs); + dprint("translating set_attrs from 0x%lx to 0x%lx\n", set_attrs, uefi_set_attrs); + uefi_clear_attrs = shim_mem_attrs_to_uefi_mem_attrs (clear_attrs); + dprint("translating clear_attrs from 0x%lx to 0x%lx\n", clear_attrs, uefi_clear_attrs); + efi_status = EFI_SUCCESS; + if (uefi_set_attrs) + efi_status = proto->SetMemoryAttributes(proto, physaddr, size, uefi_set_attrs); + if (!EFI_ERROR(efi_status) && uefi_clear_attrs) + efi_status = proto->ClearMemoryAttributes(proto, physaddr, size, uefi_clear_attrs); + ret = efi_status; + + efi_status = get_mem_attrs (addr, size, &after); + if (EFI_ERROR(efi_status)) + dprint(L"get_mem_attrs(0x%llx, %llu, 0x%llx) -> 0x%lx\n", + (unsigned long long)addr, (unsigned long long)size, + &after, efi_status); + + dprint(L"set +%a%a%a -%a%a%a on 0x%llx-0x%llx before:%c%c%c after:%c%c%c\n", + (set_attrs & MEM_ATTR_R) ? "r" : "", + (set_attrs & MEM_ATTR_W) ? "w" : "", + (set_attrs & MEM_ATTR_X) ? "x" : "", + (clear_attrs & MEM_ATTR_R) ? "r" : "", + (clear_attrs & MEM_ATTR_W) ? "w" : "", + (clear_attrs & MEM_ATTR_X) ? "x" : "", + (unsigned long long)addr, (unsigned long long)(addr + size - 1), + (before & MEM_ATTR_R) ? 'r' : '-', + (before & MEM_ATTR_W) ? 'w' : '-', + (before & MEM_ATTR_X) ? 'x' : '-', + (after & MEM_ATTR_R) ? 'r' : '-', + (after & MEM_ATTR_W) ? 'w' : '-', + (after & MEM_ATTR_X) ? 'x' : '-'); + + return ret; +} + EFI_STATUS verify_image(void *data, unsigned int datasize, EFI_LOADED_IMAGE *li, PE_COFF_LOADER_IMAGE_CONTEXT *context) @@ -1013,6 +1154,11 @@ handle_image (void *data, unsigned int datasize, } buffer = (void *)ALIGN_VALUE((unsigned long)*alloc_address, alignment); + dprint(L"Loading 0x%llx bytes at 0x%llx\n", + (unsigned long long)context.ImageSize, + (unsigned long long)(uintptr_t)buffer); + update_mem_attrs((uintptr_t)buffer, alloc_size, MEM_ATTR_R|MEM_ATTR_W, + MEM_ATTR_X); CopyMem(buffer, data, context.SizeOfHeaders); @@ -1049,6 +1195,19 @@ handle_image (void *data, unsigned int datasize, !Section->Misc.VirtualSize) continue; + /* + * Skip sections that aren't marked readable. + */ + if (!(Section->Characteristics & EFI_IMAGE_SCN_MEM_READ)) + continue; + + if (!(Section->Characteristics & EFI_IMAGE_SCN_MEM_DISCARDABLE) && + (Section->Characteristics & EFI_IMAGE_SCN_MEM_WRITE) && + (Section->Characteristics & EFI_IMAGE_SCN_MEM_EXECUTE)) { + perror(L"Section %d is writable and executable\n", i); + return EFI_UNSUPPORTED; + } + base = ImageAddress (buffer, context.ImageSize, Section->VirtualAddress); end = ImageAddress (buffer, context.ImageSize, @@ -1159,6 +1318,50 @@ handle_image (void *data, unsigned int datasize, } } + /* + * Now set the page permissions appropriately. + */ + Section = context.FirstSection; + for (i = 0; i < context.NumberOfSections; i++, Section++) { + uint64_t set_attrs = MEM_ATTR_R; + uint64_t clear_attrs = MEM_ATTR_W|MEM_ATTR_X; + uintptr_t addr; + uint64_t length; + + /* + * Skip discardable sections with zero size + */ + if ((Section->Characteristics & EFI_IMAGE_SCN_MEM_DISCARDABLE) && + !Section->Misc.VirtualSize) + continue; + + /* + * Skip sections that aren't marked readable. + */ + if (!(Section->Characteristics & EFI_IMAGE_SCN_MEM_READ)) + continue; + + base = ImageAddress (buffer, context.ImageSize, + Section->VirtualAddress); + end = ImageAddress (buffer, context.ImageSize, + Section->VirtualAddress + + Section->Misc.VirtualSize - 1); + + addr = (uintptr_t)base; + length = (uintptr_t)end - (uintptr_t)base + 1; + + if (Section->Characteristics & EFI_IMAGE_SCN_MEM_WRITE) { + set_attrs |= MEM_ATTR_W; + clear_attrs &= ~MEM_ATTR_W; + } + if (Section->Characteristics & EFI_IMAGE_SCN_MEM_EXECUTE) { + set_attrs |= MEM_ATTR_X; + clear_attrs &= ~MEM_ATTR_X; + } + update_mem_attrs(addr, length, set_attrs, clear_attrs); + } + + /* * grub needs to know its location and size in memory, so fix up * the loaded image protocol values diff --git a/shim.h b/shim.h index 703c11455..dc3cda731 100644 --- a/shim.h +++ b/shim.h @@ -195,6 +195,10 @@ #include "Cryptlib/Include/OpenSslSupport.h" #endif +#define MEM_ATTR_R 4 +#define MEM_ATTR_W 2 +#define MEM_ATTR_X 1 + INTERFACE_DECL(_SHIM_LOCK); typedef From c84000e85d01f012084cbfc67800df696a8bb830 Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Thu, 31 Mar 2022 16:19:53 -0400 Subject: [PATCH 5/6] 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(-) diff --git a/globals.c b/globals.c index 30d10630e..b4e80dd3a 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 6f99a1059..fb19423b6 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 941018435..a8c8be6b5 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 9fa6fffdc..5d0c6b0ba 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 dc3cda731..b5272b9c9 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; From 99a8d19326f69665e0b86bcfa6a59d554f662fba Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Thu, 2 Dec 2021 15:51:00 -0500 Subject: [PATCH 6/6] post-process-pe: set EFI_IMAGE_DLLCHARACTERISTICS_NX_COMPAT Currently, system firmware has no means to discover that an EFI Application is compatible with the security feature variously known as NX or w^x. Since at least Revision 8.1, the PE spec supports setting a flag the Optional Header's DllCharacteristics field to inform loaders that an application supports being loaded with NX enabled. In the case of UEFI, there are several things that should be enabled if this flag is set: - EFI_BOOT_SERVICES.AllocatePages() with MemoryType = EfiLoaderCode, EfiBootServicesCode, EfiRuntimeServicesCode, etc, currently must set memory as rwx. This flag set implies that rw- is appropriate, and that the application knows how to use the EFI_MEMORY_ATTRIBUTE protocol to change that to r-x. - EFI_BOOT_SERVICES.AllocatePool() - same as AllocatePages() - EFI_BOOT_SERVICES.LoadImage() - currently must set the stack as rwx. This flag states that it is allowed to be rw- - currently a binary can probably have writable PLTs? This flag allows the loader to not set them writable - I have heard that some firmwares have the 0 page mapped rwx. Obviously this should not be done. Signed-off-by: Peter Jones --- post-process-pe.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 47 insertions(+), 1 deletion(-) diff --git a/post-process-pe.c b/post-process-pe.c index e848f0094..de8f4a38b 100644 --- a/post-process-pe.c +++ b/post-process-pe.c @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include @@ -41,6 +42,8 @@ static int verbosity; 0; \ }) +static bool set_nx_compat = false; + typedef uint8_t UINT8; typedef uint16_t UINT16; typedef uint32_t UINT32; @@ -330,6 +333,33 @@ load_pe(const char *const file, void *const data, const size_t datasize, errx(1, "%s: Security directory extends past end", file); } +static void +set_dll_characteristics(PE_COFF_LOADER_IMAGE_CONTEXT *ctx) +{ + uint16_t oldflags, newflags; + + if (image_is_64_bit(ctx->PEHdr)) { + oldflags = ctx->PEHdr->Pe32Plus.OptionalHeader.DllCharacteristics; + } else { + oldflags = ctx->PEHdr->Pe32.OptionalHeader.DllCharacteristics; + } + + if (set_nx_compat) + newflags = oldflags | EFI_IMAGE_DLLCHARACTERISTICS_NX_COMPAT; + else + newflags = oldflags & ~(uint16_t)EFI_IMAGE_DLLCHARACTERISTICS_NX_COMPAT; + if (oldflags == newflags) + return; + + debug(INFO, "Updating DLL Characteristics from 0x%04hx to 0x%04hx\n", + oldflags, newflags); + if (image_is_64_bit(ctx->PEHdr)) { + ctx->PEHdr->Pe32Plus.OptionalHeader.DllCharacteristics = newflags; + } else { + ctx->PEHdr->Pe32.OptionalHeader.DllCharacteristics = newflags; + } +} + static void fix_timestamp(PE_COFF_LOADER_IMAGE_CONTEXT *ctx) { @@ -417,6 +447,8 @@ handle_one(char *f) load_pe(f, map, sz, &ctx); + set_dll_characteristics(&ctx); + fix_timestamp(&ctx); fix_checksum(&ctx, map, sz); @@ -449,6 +481,8 @@ static void __attribute__((__noreturn__)) usage(int status) fprintf(out, "Options:\n"); fprintf(out, " -q Be more quiet\n"); fprintf(out, " -v Be more verbose\n"); + fprintf(out, " -N Disable the NX compatibility flag\n"); + fprintf(out, " -n Enable the NX compatibility flag\n"); fprintf(out, " -h Print this help text and exit\n"); exit(status); @@ -464,6 +498,12 @@ int main(int argc, char **argv) {.name = "usage", .val = '?', }, + {.name = "disable-nx-compat", + .val = 'N', + }, + {.name = "enable-nx-compat", + .val = 'n', + }, {.name = "quiet", .val = 'q', }, @@ -474,12 +514,18 @@ int main(int argc, char **argv) }; int longindex = -1; - while ((i = getopt_long(argc, argv, "hqv", options, &longindex)) != -1) { + while ((i = getopt_long(argc, argv, "hNnqv", options, &longindex)) != -1) { switch (i) { case 'h': case '?': usage(longindex == -1 ? 1 : 0); break; + case 'N': + set_nx_compat = false; + break; + case 'n': + set_nx_compat = true; + break; case 'q': verbosity = MAX(verbosity - 1, MIN_VERBOSITY); break;