From 3cc53c788ed2c540889da8b01d681103dda39e92 Mon Sep 17 00:00:00 2001
From: Peter Jones <pjones@redhat.com>
Date: Fri, 23 Jul 2021 13:18:57 -0400
Subject: mok: move the mok_state_variables definitions to their own header

This lets us access the definitions for this structure, and the data
being used at runtime, from unit tests.

Signed-off-by: Peter Jones <pjones@redhat.com>
---
 include/mok.h | 101 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 101 insertions(+)
 create mode 100644 include/mok.h

(limited to 'include/mok.h')

diff --git a/include/mok.h b/include/mok.h
new file mode 100644
index 00000000..96da397a
--- /dev/null
+++ b/include/mok.h
@@ -0,0 +1,101 @@
+// SPDX-License-Identifier: BSD-2-Clause-Patent
+/*
+ * mok.h - structs for MoK data
+ * Copyright Peter Jones <pjones@redhat.com>
+ */
+
+#ifndef SHIM_MOK_H_
+#define SHIM_MOK_H_
+
+#include "shim.h"
+
+typedef enum {
+	VENDOR_ADDEND_DB,
+	VENDOR_ADDEND_X509,
+	VENDOR_ADDEND_NONE,
+} vendor_addend_category_t;
+
+struct mok_state_variable;
+typedef vendor_addend_category_t (vendor_addend_categorizer_t)(struct mok_state_variable *);
+
+/*
+ * MoK variables that need to have their storage validated.
+ *
+ * The order here is important, since this is where we measure for the
+ * tpm as well.
+ */
+struct mok_state_variable {
+	CHAR16 *name;	/* UCS-2 BS|NV variable name */
+	char *name8;	/* UTF-8 BS|NV variable name */
+	CHAR16 *rtname;	/* UCS-2 RT variable name */
+	char *rtname8;	/* UTF-8 RT variable name */
+	EFI_GUID *guid;	/* variable GUID */
+
+	/*
+	 * these are used during processing, they shouldn't be filled out
+	 * in the static table below.
+	 */
+	UINT8 *data;
+	UINTN data_size;
+
+	/*
+	 * addend are added to the input variable, as part of the runtime
+	 * variable, so that they're visible to the kernel.  These are
+	 * where we put vendor_cert / vendor_db / vendor_dbx
+	 *
+	 * These are indirect pointers just to make initialization saner...
+	 */
+	vendor_addend_categorizer_t *categorize_addend; /* determines format */
+	/*
+	 * we call categorize_addend() and it determines what kind of thing
+	 * this is.  That is, if this shim was built with VENDOR_CERT, for
+	 * the DB entry it'll return VENDOR_ADDEND_X509; if you used
+	 * VENDOR_DB instead, it'll return VENDOR_ADDEND_DB.  If you used
+	 * neither, it'll do VENDOR_ADDEND_NONE.
+	 *
+	 * The existing categorizers are for db and dbx; they differ
+	 * because we don't currently support a CERT for dbx.
+	 */
+	UINT8 **addend;
+	UINT32 *addend_size;
+
+	/*
+	 * build_cert is our build-time cert.  Like addend, this is added
+	 * to the input variable, as part of the runtime variable, so that
+	 * they're visible to the kernel.  This is the ephemeral cert used
+	 * for signing MokManager.efi and fallback.efi.
+	 *
+	 * These are indirect pointers just to make initialization saner...
+	 */
+	UINT8 **build_cert;
+	UINT32 *build_cert_size;
+
+	UINT32 yes_attr;	/* var attrs that must be set */
+	UINT32 no_attr;		/* var attrs that must not be set */
+	UINT32 flags;		/* flags on what and how to mirror */
+	/*
+	 * MOK_MIRROR_KEYDB	    mirror this as a key database
+	 * 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
+	 */
+	UINTN pcr;		/* PCR to measure and hash to */
+
+	/*
+	 * if this is a state value, a pointer to our internal state to be
+	 * mirrored.
+	 */
+	UINT8 *state;
+};
+
+extern size_t n_mok_state_variables;
+extern struct mok_state_variable *mok_state_variables;
+
+struct mok_variable_config_entry {
+	CHAR8 name[256];
+	UINT64 data_size;
+	UINT8 data[];
+};
+
+#endif /* !SHIM_MOK_H_ */
+// vim:fenc=utf-8:tw=75:noet
-- 
cgit v1.2.3


From 35d7378d29b9ad6f664df20efc4121e210859e65 Mon Sep 17 00:00:00 2001
From: Eric Snowberg <eric.snowberg@oracle.com>
Date: Tue, 1 Feb 2022 15:49:51 -0500
Subject: Load additional certs from a signed binary

Heavily inspired by Matthew Garrett's patch "Allow additional certificates
to be loaded from a signed binary".

Add support for loading a binary, verifying its signature, and then
scanning it for embedded certificates. This is intended to make it
possible to decouple shim builds from vendor signatures. In order to
add new signatures to shim, an EFI Signature List should be generated
and then added to the .db section of a well-formed EFI binary. This
binary should then be signed with a key that shim already trusts (either
a built-in key, one present in the platform firmware or
one present in MOK) and placed in the same directory as shim with a
filename starting "shim_certificate" (eg, "shim_certificate_oracle").

Shim will read multiple files and incorporate the signatures from all of
them. Note that each section *must* be an EFI Signature List, not a raw
certificate.

Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
---
 globals.c     |   3 ++
 include/mok.h |   3 ++
 mok.c         |   9 ++++-
 shim.c        | 127 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 shim.h        |   3 ++
 5 files changed, 144 insertions(+), 1 deletion(-)

(limited to 'include/mok.h')

diff --git a/globals.c b/globals.c
index 4a1f432f..30d10630 100644
--- a/globals.c
+++ b/globals.c
@@ -12,6 +12,9 @@ UINT8 *vendor_authorized = NULL;
 UINT32 vendor_deauthorized_size = 0;
 UINT8 *vendor_deauthorized = NULL;
 
+UINT32 user_cert_size;
+UINT8 *user_cert;
+
 #if defined(ENABLE_SHIM_CERT)
 UINT32 build_cert_size;
 UINT8 *build_cert;
diff --git a/include/mok.h b/include/mok.h
index 96da397a..6f99a105 100644
--- a/include/mok.h
+++ b/include/mok.h
@@ -59,6 +59,9 @@ struct mok_state_variable {
 	UINT8 **addend;
 	UINT32 *addend_size;
 
+	UINT8 **user_cert;
+	UINT32 *user_cert_size;
+
 	/*
 	 * build_cert is our build-time cert.  Like addend, this is added
 	 * to the input variable, as part of the runtime variable, so that
diff --git a/mok.c b/mok.c
index f4de8081..94101843 100644
--- a/mok.c
+++ b/mok.c
@@ -98,6 +98,8 @@ struct mok_state_variable mok_state_variable_data[] = {
 	 .categorize_addend = categorize_authorized,
 	 .addend = &vendor_authorized,
 	 .addend_size = &vendor_authorized_size,
+	 .user_cert = &user_cert,
+	 .user_cert_size = &user_cert_size,
 #if defined(ENABLE_SHIM_CERT)
 	 .build_cert = &build_cert,
 	 .build_cert_size = &build_cert_size,
@@ -588,7 +590,8 @@ mirror_one_mok_variable(struct mok_state_variable *v,
 			dprint(L"FullDataSize:0x%lx FullData:0x%llx\n",
 			       FullDataSize, FullData);
 		}
-
+		if (v->user_cert_size)
+			FullDataSize += *v->user_cert_size;
 	}
 
 	/*
@@ -702,6 +705,10 @@ 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 (v->user_cert_size) {
+			CopyMem(p, *v->user_cert, *v->user_cert_size);
+			p += *v->user_cert_size;
+		}
 	}
 
 	/*
diff --git a/shim.c b/shim.c
index 51e72e7e..c4f9461a 100644
--- a/shim.c
+++ b/shim.c
@@ -1391,6 +1391,126 @@ uninstall_shim_protocols(void)
 #endif
 }
 
+EFI_STATUS
+load_cert_file(EFI_HANDLE image_handle, CHAR16 *filename)
+{
+	EFI_STATUS efi_status;
+	EFI_LOADED_IMAGE *li = NULL;
+	PE_COFF_LOADER_IMAGE_CONTEXT context;
+	EFI_IMAGE_SECTION_HEADER *Section;
+	EFI_SIGNATURE_LIST *certlist;
+	CHAR16 *PathName = NULL;
+	void *pointer;
+	UINT32 original;
+	int datasize;
+	void *data;
+	int i;
+
+	efi_status = read_image(image_handle, filename, PathName,
+				&data, &datasize);
+
+	if (EFI_ERROR(efi_status))
+		return efi_status;
+
+	efi_status = verify_image(data, datasize, li, &context);
+	if (EFI_ERROR(efi_status))
+		return efi_status;
+
+	Section = context.FirstSection;
+	for (i = 0; i < context.NumberOfSections; i++, Section++) {
+		if (CompareMem(Section->Name, ".db\0\0\0\0\0", 8) == 0) {
+			original = user_cert_size;
+			if (Section->SizeOfRawData < sizeof(EFI_SIGNATURE_LIST)) {
+				continue;
+			}
+			pointer = ImageAddress(data, datasize,
+					       Section->PointerToRawData);
+			if (!pointer) {
+				continue;
+			}
+			certlist = pointer;
+			user_cert_size += certlist->SignatureListSize;;
+			user_cert = ReallocatePool(user_cert, original,
+						   user_cert_size);
+			memcpy(user_cert + original, pointer,
+			       certlist->SignatureListSize);
+		}
+	}
+	FreePool(data);
+	return EFI_SUCCESS;
+}
+
+/* Read additional certificates from files (after verifying signatures) */
+EFI_STATUS
+load_certs(EFI_HANDLE image_handle)
+{
+	EFI_STATUS efi_status;
+	EFI_LOADED_IMAGE *li = NULL;
+	CHAR16 *PathName = NULL;
+	EFI_FILE *root, *dir;
+	EFI_FILE_INFO *info;
+	EFI_HANDLE device;
+	EFI_FILE_IO_INTERFACE *drive;
+	UINTN buffersize = 0;
+	void *buffer = NULL;
+
+	efi_status = gBS->HandleProtocol(image_handle, &EFI_LOADED_IMAGE_GUID,
+					 (void **)&li);
+	if (EFI_ERROR(efi_status)) {
+		perror(L"Unable to init protocol\n");
+		return efi_status;
+	}
+
+	efi_status = generate_path_from_image_path(li, L"", &PathName);
+	if (EFI_ERROR(efi_status))
+		goto done;
+
+	device = li->DeviceHandle;
+	efi_status = gBS->HandleProtocol(device, &EFI_SIMPLE_FILE_SYSTEM_GUID,
+					 (void **)&drive);
+	if (EFI_ERROR(efi_status)) {
+		perror(L"Failed to find fs: %r\n", efi_status);
+		goto done;
+	}
+
+	efi_status = drive->OpenVolume(drive, &root);
+	if (EFI_ERROR(efi_status)) {
+		perror(L"Failed to open fs: %r\n", efi_status);
+		goto done;
+	}
+
+	efi_status = root->Open(root, &dir, PathName, EFI_FILE_MODE_READ, 0);
+	if (EFI_ERROR(efi_status)) {
+		perror(L"Failed to open %s - %r\n", PathName, efi_status);
+		goto done;
+	}
+
+	while (1) {
+		int old = buffersize;
+		efi_status = dir->Read(dir, &buffersize, buffer);
+		if (efi_status == EFI_BUFFER_TOO_SMALL) {
+			buffer = ReallocatePool(buffer, old, buffersize);
+			continue;
+		} else if (EFI_ERROR(efi_status)) {
+			perror(L"Failed to read directory %s - %r\n", PathName,
+			       efi_status);
+			goto done;
+		}
+
+		if (buffersize == 0)
+			goto done;
+
+		info = (EFI_FILE_INFO *)buffer;
+		if (StrnCaseCmp(info->FileName, L"shim_certificate", 16) == 0) {
+			load_cert_file(image_handle, info->FileName);
+		}
+	}
+done:
+	FreePool(buffer);
+	FreePool(PathName);
+	return efi_status;
+}
+
 EFI_STATUS
 shim_init(void)
 {
@@ -1626,6 +1746,13 @@ efi_main (EFI_HANDLE passed_image_handle, EFI_SYSTEM_TABLE *passed_systab)
 
 	init_openssl();
 
+	if (secure_mode()) {
+		efi_status = load_certs(global_image_handle);
+		if (EFI_ERROR(efi_status)) {
+			LogError(L"Failed to load addon certificates\n");
+		}
+	}
+
 	/*
 	 * Before we do anything else, validate our non-volatile,
 	 * boot-services-only state variables are what we think they are.
diff --git a/shim.h b/shim.h
index db264cb7..703c1145 100644
--- a/shim.h
+++ b/shim.h
@@ -248,6 +248,9 @@ extern UINT8 *vendor_authorized;
 extern UINT32 vendor_deauthorized_size;
 extern UINT8 *vendor_deauthorized;
 
+extern UINT32 user_cert_size;
+extern UINT8 *user_cert;
+
 #if defined(ENABLE_SHIM_CERT)
 extern UINT32 build_cert_size;
 extern UINT8 *build_cert;
-- 
cgit v1.2.3


From df96f48f28fa94b62d06f39a3b014133dd38def5 Mon Sep 17 00:00:00 2001
From: Peter Jones <pjones@redhat.com>
Date: Thu, 31 Mar 2022 16:19:53 -0400
Subject: 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 <pjones@redhat.com>
---
 globals.c     |  1 +
 include/mok.h |  5 +++++
 mok.c         | 13 +++++++++++++
 pe.c          |  8 +++++---
 shim.h        |  2 ++
 5 files changed, 26 insertions(+), 3 deletions(-)

(limited to 'include/mok.h')

diff --git a/globals.c b/globals.c
index 30d10630..b4e80dd3 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 6f99a105..fb19423b 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 94101843..a8c8be6b 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 9fa6fffd..5d0c6b0b 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 dc3cda73..b5272b9c 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;
-- 
cgit v1.2.3


From 49db3de08ef2c55f6dbc3c2b8e6ab7b2f22e5309 Mon Sep 17 00:00:00 2001
From: Peter Jones <pjones@redhat.com>
Date: Wed, 15 May 2024 16:13:40 -0400
Subject: 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 <pjones@redhat.com>
---
 include/mok.h |  2 ++
 mok.c         | 36 ++++++++++++++++++++++++++++--------
 2 files changed, 30 insertions(+), 8 deletions(-)

(limited to 'include/mok.h')

diff --git a/include/mok.h b/include/mok.h
index fb19423b..fe92cf03 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 5ae91244..2778cd05 100644
--- a/mok.c
+++ b/mok.c
@@ -106,11 +106,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",
@@ -834,7 +835,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,
@@ -842,7 +844,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);
 	}
@@ -938,7 +941,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);
@@ -980,6 +984,22 @@ EFI_STATUS import_one_mok_state(struct mok_state_variable *v,
 			}
 		}
 	}
+
+	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);
+		if (EFI_ERROR(efi_status)) {
+			dprint(L"Couldn't get variable \"%s\" for mirroring: %r\n",
+			       v->name, efi_status);
+			if (efi_status != EFI_NOT_FOUND)
+				return efi_status;
+			v->data = NULL;
+			v->data_size = 0;
+		}
+	}
+
 	if (delete == TRUE) {
 		perror(L"Deleting bad variable %s\n", v->name);
 		efi_status = LibDeleteVariable(v->name, v->guid);
-- 
cgit v1.2.3


From 887c0edab93c52e2558047897847166b73dc8c3a Mon Sep 17 00:00:00 2001
From: Peter Jones <pjones@redhat.com>
Date: Wed, 15 May 2024 16:01:34 -0400
Subject: 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 <pjones@redhat.com>
---
 include/mok.h | 18 ++++++++++++++++++
 mok.c         | 16 +++++++++++++++-
 2 files changed, 33 insertions(+), 1 deletion(-)

(limited to 'include/mok.h')

diff --git a/include/mok.h b/include/mok.h
index fe92cf03..c37ccba5 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)(UINT8 *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 2778cd05..67a798a3 100644
--- a/mok.c
+++ b/mok.c
@@ -985,8 +985,22 @@ 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 (!v->data && !v->data_size &&
-	    (v->flags & MOK_VARIABLE_CONFIG_ONLY)) {
+	    (v->flags & MOK_VARIABLE_CONFIG_ONLY) &&
+	    !v->format) {
 		efi_status = get_variable_attr(v->name,
 					       &v->data, &v->data_size,
 					       *v->guid, &attrs);
-- 
cgit v1.2.3


From 848667d0f3a99401d93c93b3af16b55e3fb28cea Mon Sep 17 00:00:00 2001
From: Peter Jones <pjones@redhat.com>
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 <pjones@redhat.com>
---
 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/mok.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 b216543d691050d6cdd37c3500571cf67882f1bc Mon Sep 17 00:00:00 2001
From: Peter Jones <pjones@redhat.com>
Date: Tue, 18 Feb 2025 15:10:22 -0500
Subject: Move mok state variable data flag definitions to the header.

Previously the mok mirror state flags were only used in the mok
mirroring code.  But there are other consumers of that data, namely our
variable test cases, and it's useful for them to be able to check the
flags.

Signed-off-by: Peter Jones <pjones@redhat.com>
---
 include/mok.h | 7 +++++++
 mok.c         | 7 -------
 2 files changed, 7 insertions(+), 7 deletions(-)

(limited to 'include/mok.h')

diff --git a/include/mok.h b/include/mok.h
index e6921e09..1b44217c 100644
--- a/include/mok.h
+++ b/include/mok.h
@@ -19,6 +19,13 @@ struct mok_state_variable;
 typedef vendor_addend_category_t (vendor_addend_categorizer_t)(struct mok_state_variable *);
 typedef UINTN (mok_variable_format_helper_t)(UINT8 *buf, size_t sz, struct mok_state_variable *);
 
+#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
+
 /*
  * MoK variables that need to have their storage validated.
  *
diff --git a/mok.c b/mok.c
index 5c7f9a2b..f98e36de 100644
--- a/mok.c
+++ b/mok.c
@@ -144,13 +144,6 @@ 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_VARIABLE_CONFIG_ONLY	0x20
-
 struct mok_state_variable mok_state_variable_data[] = {
 	{.name = L"MokList",
 	 .name8 = "MokList",
-- 
cgit v1.2.3


From 9269e9b0aa15ae9832f7eba6c5eeef0c5e1f4edb Mon Sep 17 00:00:00 2001
From: Peter Jones <pjones@redhat.com>
Date: Thu, 20 Feb 2025 14:44:10 -0500
Subject: Add DXE Services information to HSI

This adds three more entries to our HSI data:

has-dxe-services-table: technically only tells us if UEFI's
			LocateProtocol will give us a DXE services
			table, but practically also tells us if the
			machine is implementing DXE in any way.
has-get-memory-space-descriptor: tells us if DXE->GetMemorySpaceDescriptor
                                 is populated
has-set-memory-space-descriptor: tells us if DXE->SetMemorySpaceDescriptor
                                 is populated

Signed-off-by: Peter Jones <pjones@redhat.com>
---
 MokVars.txt                   |  6 ++++++
 include/mok.h                 |  6 ++++++
 include/test-data-efivars-1.h |  6 +++++-
 memattrs.c                    | 16 ++++++++++++++--
 mok.c                         | 11 +++++++++++
 5 files changed, 42 insertions(+), 3 deletions(-)

(limited to 'include/mok.h')

diff --git a/MokVars.txt b/MokVars.txt
index e6e68ce4..0ab81ff6 100644
--- a/MokVars.txt
+++ b/MokVars.txt
@@ -103,3 +103,9 @@ HSIStatus: Status of various security features:
                             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
+  has-dxe-services-table: 0: platform does not provide the DXE Services Table
+                          1: platform does provide the DXE Services Table
+  has-get-memory-space-descriptor: 0: platform's DST does not populate GetMemorySpaceDescriptor
+                                   1: platform's DST does populate GetMemorySpaceDescriptor
+  has-set-memory-space-descriptor: 0: platform's DST does not populate SetMemorySpaceDescriptor
+                                   1: platform's DST does populate SetMemorySpaceDescriptor
diff --git a/include/mok.h b/include/mok.h
index 1b44217c..cea4c997 100644
--- a/include/mok.h
+++ b/include/mok.h
@@ -141,6 +141,12 @@ extern UINTN hsi_status;
 #define SHIM_HSI_STATUS_ROW		0x00000004ULL
 /* platform provides the EFI Memory Attribute Protocol */
 #define SHIM_HSI_STATUS_HASMAP		0x00000008ULL
+/* platform provides DXE Services Table */
+#define SHIM_HSI_STATUS_HASDST		0x00000010ULL
+/* platform has DST->GetMemorySpaceDescriptor */
+#define SHIM_HSI_STATUS_HASDSTGMSD	0x00000020ULL
+/* platform has DST->SetMemorySpaceAttributes */
+#define SHIM_HSI_STATUS_HASDSTSMSA	0x00000040ULL
 
 #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 d97a4d6d..7a34ea70 100644
--- a/include/test-data-efivars-1.h
+++ b/include/test-data-efivars-1.h
@@ -110,7 +110,11 @@ 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";
+	"has-memory-attribute-protocol: 0\n"
+	"has-dxe-services-table: 0\n"
+	"has-get-memory-space-descriptor: 0\n"
+	"has-set-memory-space-attributes: 0\n"
+	;
 
 #endif /* !TEST_DATA_EFIVARS_1_H_ */
 // vim:fenc=utf-8:tw=75:noet
diff --git a/memattrs.c b/memattrs.c
index a2c1777c..f502805f 100644
--- a/memattrs.c
+++ b/memattrs.c
@@ -50,7 +50,7 @@ get_dxe_services_table(EFI_DXE_SERVICES_TABLE **dstp)
 	static EFI_DXE_SERVICES_TABLE *dst = NULL;
 
 	if (dst == NULL) {
-	dprint(L"Looking for configuration table " LGUID_FMT L"\n", GUID_ARGS(gEfiDxeServicesTableGuid));
+		dprint(L"Looking for configuration table " LGUID_FMT L"\n", GUID_ARGS(gEfiDxeServicesTableGuid));
 
 		for (UINTN i = 0; i < ST->NumberOfTableEntries; i++) {
 			EFI_CONFIGURATION_TABLE *ct = &ST->ConfigurationTable[i];
@@ -408,6 +408,7 @@ get_hsi_mem_info(void)
 	uint64_t attrs = 0;
 	uint32_t *tmp_alloc;
 	EFI_MEMORY_ATTRIBUTE_PROTOCOL *efiproto = NULL;
+	EFI_DXE_SERVICES_TABLE *dst = NULL;
 
 	get_efi_mem_attr_protocol(&efiproto);
 	if (efiproto) {
@@ -415,7 +416,18 @@ get_hsi_mem_info(void)
 		dprint(L"Setting HSI to 0x%lx\n", hsi_status);
 	}
 
-	if (!(hsi_status & SHIM_HSI_STATUS_HASMAP)) {
+	get_dxe_services_table(&dst);
+	if (dst) {
+		hsi_status |= SHIM_HSI_STATUS_HASDST;
+		if (dst->GetMemorySpaceDescriptor)
+			hsi_status |= SHIM_HSI_STATUS_HASDSTGMSD;
+		if (dst->SetMemorySpaceAttributes)
+			hsi_status |= SHIM_HSI_STATUS_HASDSTSMSA;
+	}
+
+	if (!(hsi_status & SHIM_HSI_STATUS_HASMAP) &&
+	    !(hsi_status & SHIM_HSI_STATUS_HASDSTGMSD &&
+	      hsi_status & SHIM_HSI_STATUS_HASDSTSMSA)) {
 		dprint(L"No memory protocol, not testing further\n");
 		return;
 	}
diff --git a/mok.c b/mok.c
index 97d4a0eb..cb70e7e2 100644
--- a/mok.c
+++ b/mok.c
@@ -42,6 +42,9 @@ format_hsi_status(UINT8 *buf, size_t sz,
 	const char stackx[] = "\nstack-is-executable: ";
 	const char row[] = "\nro-sections-are-writable: ";
 	const char hasmap[] = "\nhas-memory-attribute-protocol: ";
+	const char hasdxeservices[] = "\nhas-dxe-services-table: ";
+	const char hasdsgmsd[] = "\nhas-get-memory-space-descriptor: ";
+	const char hasdssmsa[] = "\nhas-set-memory-space-attributes: ";
 	const char finale[] = "\n";
 	char *pos;
 
@@ -51,6 +54,8 @@ format_hsi_status(UINT8 *buf, size_t sz,
 	 */
 	UINTN ret = sizeof(heapx) + sizeof(stackx) +
 		    sizeof(row) + sizeof(hasmap) +
+		    sizeof(hasdxeservices) + sizeof(hasdsgmsd) +
+		    sizeof(hasdssmsa) +
 		    sizeof(finale);
 
 	if (buf == 0 || sz < ret) {
@@ -67,6 +72,12 @@ format_hsi_status(UINT8 *buf, size_t sz,
 	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");
+	pos = stpcpy(pos, hasdxeservices);
+	pos = stpcpy(pos, (hsi_status & SHIM_HSI_STATUS_HASDST) ? "1" : "0");
+	pos = stpcpy(pos, hasdsgmsd);
+	pos = stpcpy(pos, (hsi_status & SHIM_HSI_STATUS_HASDSTGMSD) ? "1" : "0");
+	pos = stpcpy(pos, hasdssmsa);
+	pos = stpcpy(pos, (hsi_status & SHIM_HSI_STATUS_HASDSTSMSA) ? "1" : "0");
 	stpcpy(pos, finale);
 
 	return ret;
-- 
cgit v1.2.3


From 89e615081af5fbafefeae5b09def3a003e467838 Mon Sep 17 00:00:00 2001
From: Peter Jones <pjones@redhat.com>
Date: Thu, 20 Feb 2025 19:20:47 -0500
Subject: Add shim's current NX_COMPAT status to HSIStatus

hughsie asked me to also make it observable at runtime whether the shim
binary that was used to boot was set as NX_COMPAT or not.

This adds that into the HSIStatus data as "shim-has-nx-compat-set".

Signed-off-by: Peter Jones <pjones@redhat.com>
---
 MokVars.txt                   |  2 ++
 include/mok.h                 |  2 ++
 include/pe.h                  |  3 +++
 include/test-data-efivars-1.h |  1 +
 memattrs.c                    |  1 +
 mok.c                         |  5 ++++-
 pe-relocate.c                 | 37 +++++++++++++++++++++++++++++++++----
 shim.c                        |  2 ++
 8 files changed, 48 insertions(+), 5 deletions(-)

(limited to 'include/mok.h')

diff --git a/MokVars.txt b/MokVars.txt
index 0ab81ff6..6ad8ce70 100644
--- a/MokVars.txt
+++ b/MokVars.txt
@@ -109,3 +109,5 @@ HSIStatus: Status of various security features:
                                    1: platform's DST does populate GetMemorySpaceDescriptor
   has-set-memory-space-descriptor: 0: platform's DST does not populate SetMemorySpaceDescriptor
                                    1: platform's DST does populate SetMemorySpaceDescriptor
+  shim-has-nx-compat-set: 0: the running shim binary does not have NX_COMPAT bit set
+                          1: the running shim binary does have the NX_COMPAT bit set
diff --git a/include/mok.h b/include/mok.h
index cea4c997..89edf9de 100644
--- a/include/mok.h
+++ b/include/mok.h
@@ -147,6 +147,8 @@ extern UINTN hsi_status;
 #define SHIM_HSI_STATUS_HASDSTGMSD	0x00000020ULL
 /* platform has DST->SetMemorySpaceAttributes */
 #define SHIM_HSI_STATUS_HASDSTSMSA	0x00000040ULL
+/* This shim has the NX_COMPAT bit set */
+#define SHIM_HSI_STATUS_NX		0x00000100ULL
 
 #endif /* !SHIM_MOK_H_ */
 // vim:fenc=utf-8:tw=75:noet
diff --git a/include/pe.h b/include/pe.h
index a1eb8853..ea40184b 100644
--- a/include/pe.h
+++ b/include/pe.h
@@ -53,5 +53,8 @@ relocate_coff (PE_COFF_LOADER_IMAGE_CONTEXT *context,
 	       EFI_IMAGE_SECTION_HEADER *Section,
 	       void *orig, void *data);
 
+void
+get_shim_nx_capability(EFI_HANDLE image_handle);
+
 #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 7a34ea70..259558e0 100644
--- a/include/test-data-efivars-1.h
+++ b/include/test-data-efivars-1.h
@@ -114,6 +114,7 @@ static const unsigned char test_data_efivars_1_HSIStatus[] =
 	"has-dxe-services-table: 0\n"
 	"has-get-memory-space-descriptor: 0\n"
 	"has-set-memory-space-attributes: 0\n"
+	"shim-has-nx-compat-set: 0\n"
 	;
 
 #endif /* !TEST_DATA_EFIVARS_1_H_ */
diff --git a/memattrs.c b/memattrs.c
index bc571641..ed8a3ae2 100644
--- a/memattrs.c
+++ b/memattrs.c
@@ -414,6 +414,7 @@ decode_hsi_bits(UINTN hsi)
 		{.bit = SHIM_HSI_STATUS_HASDST, .name = "HASDST"},
 		{.bit = SHIM_HSI_STATUS_HASDSTGMSD, .name = "HASDSTGMSD"},
 		{.bit = SHIM_HSI_STATUS_HASDSTSMSA, .name = "HASDSTSMSA"},
+		{.bit = SHIM_HSI_STATUS_NX, .name = "NX"},
 		{.bit = 0, .name = ""},
 	};
 	static int x = 0;
diff --git a/mok.c b/mok.c
index cb70e7e2..fb4c1489 100644
--- a/mok.c
+++ b/mok.c
@@ -45,6 +45,7 @@ format_hsi_status(UINT8 *buf, size_t sz,
 	const char hasdxeservices[] = "\nhas-dxe-services-table: ";
 	const char hasdsgmsd[] = "\nhas-get-memory-space-descriptor: ";
 	const char hasdssmsa[] = "\nhas-set-memory-space-attributes: ";
+	const char shimhasnx[] = "\nshim-has-nx-compat-set: ";
 	const char finale[] = "\n";
 	char *pos;
 
@@ -55,7 +56,7 @@ format_hsi_status(UINT8 *buf, size_t sz,
 	UINTN ret = sizeof(heapx) + sizeof(stackx) +
 		    sizeof(row) + sizeof(hasmap) +
 		    sizeof(hasdxeservices) + sizeof(hasdsgmsd) +
-		    sizeof(hasdssmsa) +
+		    sizeof(hasdssmsa) + sizeof(shimhasnx) +
 		    sizeof(finale);
 
 	if (buf == 0 || sz < ret) {
@@ -78,6 +79,8 @@ format_hsi_status(UINT8 *buf, size_t sz,
 	pos = stpcpy(pos, (hsi_status & SHIM_HSI_STATUS_HASDSTGMSD) ? "1" : "0");
 	pos = stpcpy(pos, hasdssmsa);
 	pos = stpcpy(pos, (hsi_status & SHIM_HSI_STATUS_HASDSTSMSA) ? "1" : "0");
+	pos = stpcpy(pos, shimhasnx);
+	pos = stpcpy(pos, (hsi_status & SHIM_HSI_STATUS_NX) ? "1" : "0");
 	stpcpy(pos, finale);
 
 	return ret;
diff --git a/pe-relocate.c b/pe-relocate.c
index b436d3ec..4991dbe1 100644
--- a/pe-relocate.c
+++ b/pe-relocate.c
@@ -375,7 +375,6 @@ 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;
 	size_t dos_sz = 0;
 	size_t tmpsz0, tmpsz1;
 
@@ -504,17 +503,17 @@ 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;
+		context->DllCharacteristics = 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;
+		context->DllCharacteristics = PEHdr->Pe32.OptionalHeader.DllCharacteristics;
 	}
 
 	if ((mok_policy & MOK_POLICY_REQUIRE_NX) &&
-	    !(DllFlags & EFI_IMAGE_DLLCHARACTERISTICS_NX_COMPAT)) {
+	    !(context->DllCharacteristics & EFI_IMAGE_DLLCHARACTERISTICS_NX_COMPAT)) {
 		perror(L"Policy requires NX, but image does not support NX\n");
 		return EFI_UNSUPPORTED;
         }
@@ -555,4 +554,34 @@ read_header(void *data, unsigned int datasize,
 	return EFI_SUCCESS;
 }
 
+void
+get_shim_nx_capability(EFI_HANDLE image_handle)
+{
+	EFI_LOADED_IMAGE_PROTOCOL*li = NULL;
+	EFI_STATUS efi_status;
+	PE_COFF_LOADER_IMAGE_CONTEXT context;
+
+	efi_status = BS->HandleProtocol(image_handle, &gEfiLoadedImageProtocolGuid, (void **)&li);
+	if (EFI_ERROR(efi_status) || !li) {
+		dprint(L"Could not get loaded image protocol: %r\n", efi_status);
+		return;
+	}
+
+	ZeroMem(&context, sizeof(context));
+	efi_status = read_header(li->ImageBase, li->ImageSize, &context, false);
+	if (EFI_ERROR(efi_status)) {
+		dprint(L"Couldn't parse image header: %r\n", efi_status);
+		return;
+	}
+
+	dprint(L"DllCharacteristics:0x%lx\n", context.DllCharacteristics);
+	if (context.DllCharacteristics & EFI_IMAGE_DLLCHARACTERISTICS_NX_COMPAT) {
+		dprint(L"Setting HSI from %a to %a\n",
+		       decode_hsi_bits(hsi_status),
+		       decode_hsi_bits(hsi_status | SHIM_HSI_STATUS_NX));
+		hsi_status |= SHIM_HSI_STATUS_NX;
+	}
+}
+
+
 // vim:fenc=utf-8:tw=75:noet
diff --git a/shim.c b/shim.c
index 9efbde11..f85ce00e 100644
--- a/shim.c
+++ b/shim.c
@@ -1993,6 +1993,8 @@ efi_main (EFI_HANDLE passed_image_handle, EFI_SYSTEM_TABLE *passed_systab)
 	 */
 	debug_hook();
 
+	get_shim_nx_capability(image_handle);
+
 	efi_status = set_sbat_uefi_variable_internal();
 	if (EFI_ERROR(efi_status) && secure_mode()) {
 		perror(L"%s variable initialization failed\n", SBAT_VAR_NAME);
-- 
cgit v1.2.3


From 9c423e09cd9a0196888253b16fe7022deadc20fc Mon Sep 17 00:00:00 2001
From: Peter Jones <pjones@redhat.com>
Date: Tue, 25 Feb 2025 14:33:50 -0500
Subject: Some save_logs() improvements.

In d972515e608e ("Save the debug and error logs in mok-variables") had a
few deficiencies: 1) the size of the result table isn't correctly
computed when either errlog or dbglog is 0 sized (much more likely for
the former), 2) when we save the error log we leak the allocation for
the previous mok variables, and 3) original mok variables were allocated
with AllocatePages(), but the new ones were allocated with
AllocateZeroPool().  The former guarantees page alignment, which we want
here.

This fixes all three of these.

Signed-off-by: Peter Jones <pjones@redhat.com>
---
 errlog.c      | 64 ++++++++++++++++++++++++++++++++++++++++++++---------------
 globals.c     |  3 +++
 include/mok.h |  3 +++
 mok.c         |  2 ++
 4 files changed, 56 insertions(+), 16 deletions(-)

(limited to 'include/mok.h')

diff --git a/errlog.c b/errlog.c
index fc89ae0a..b43a4bc2 100644
--- a/errlog.c
+++ b/errlog.c
@@ -172,17 +172,42 @@ format_debug_log(UINT8 *dest, size_t dest_sz)
 	return debug_log_sz;
 }
 
+void
+replace_config_table(EFI_CONFIGURATION_TABLE *CT, EFI_PHYSICAL_ADDRESS new_table, UINTN new_table_pages)
+{
+	EFI_GUID bogus_guid = { 0x29f2f0db, 0xd025, 0x4aa6, { 0x99, 0x58, 0xa0, 0x21, 0x8b, 0x1d, 0xec, 0x0e }};
+	EFI_STATUS efi_status;
+
+	if (CT) {
+		CopyMem(&CT->VendorGuid, &bogus_guid, sizeof(bogus_guid));
+		if (CT->VendorTable &&
+		    CT->VendorTable == (void *)(uintptr_t)mok_config_table) {
+			BS->FreePages(mok_config_table, mok_config_table_pages);
+			CT->VendorTable = NULL;
+		}
+	}
+
+	efi_status = BS->InstallConfigurationTable(&MOK_VARIABLE_STORE,
+						   (void *)(uintptr_t)new_table);
+	if (EFI_ERROR(efi_status)) {
+		console_print(L"Could not re-install MoK configuration table: %r\n", efi_status);
+	} else {
+		mok_config_table = new_table;
+		mok_config_table_pages = new_table_pages;
+	}
+}
+
 void
 save_logs(void)
 {
 	struct mok_variable_config_entry *cfg_table = NULL;
 	struct mok_variable_config_entry *new_table = NULL;
 	struct mok_variable_config_entry *entry = NULL;
+	EFI_PHYSICAL_ADDRESS physaddr = 0;
+	UINTN new_table_pages = 0;
 	size_t new_table_sz;
 	UINTN pos = 0;
 	EFI_STATUS efi_status;
-	EFI_CONFIGURATION_TABLE *CT;
-	EFI_GUID bogus_guid = { 0x29f2f0db, 0xd025, 0x4aa6, { 0x99, 0x58, 0xa0, 0x21, 0x8b, 0x1d, 0xec, 0x0e }};
 	size_t errlog_sz, dbglog_sz;
 
 	errlog_sz = format_error_log(NULL, 0);
@@ -194,6 +219,7 @@ save_logs(void)
 	}
 
 	for (UINTN i = 0; i < ST->NumberOfTableEntries; i++) {
+		EFI_CONFIGURATION_TABLE *CT;
 		CT = &ST->ConfigurationTable[i];
 
 		if (CompareGuid(&MOK_VARIABLE_STORE, &CT->VendorGuid) == 0) {
@@ -208,18 +234,28 @@ save_logs(void)
 		size_t entry_sz;
 		entry = (struct mok_variable_config_entry *)((uintptr_t)cfg_table + pos);
 
-		entry_sz = sizeof(*entry);
-		entry_sz += entry->data_size;
-
-		if (entry->name[0] != 0)
+		if (entry->name[0] != 0) {
+			entry_sz = sizeof(*entry);
+			entry_sz += entry->data_size;
 			pos += entry_sz;
+		}
 	}
 
-	new_table_sz = pos + 3 * sizeof(*entry) + errlog_sz + dbglog_sz;
-	new_table = AllocateZeroPool(new_table_sz);
+	new_table_sz = pos +
+		(errlog_sz ? sizeof(*entry) + errlog_sz : 0) +
+		(dbglog_sz ? sizeof(*entry) + dbglog_sz : 0) +
+		sizeof(*entry);
+	new_table = NULL;
+	new_table_pages = ALIGN_UP(new_table_sz + 4*EFI_PAGE_SIZE, EFI_PAGE_SIZE) / EFI_PAGE_SIZE;
+	efi_status = BS->AllocatePages(AllocateAnyPages, EfiRuntimeServicesData, new_table_pages, &physaddr);
+	if (EFI_ERROR(efi_status)) {
+		perror(L"Couldn't allocate %llu pages\n", new_table_pages);
+		return;
+	}
+	new_table = (void *)(uintptr_t)physaddr;
 	if (!new_table)
 		return;
-
+	ZeroMem(new_table, new_table_pages * EFI_PAGE_SIZE);
 	CopyMem(new_table, cfg_table, pos);
 
 	entry = (struct mok_variable_config_entry *)((uintptr_t)new_table + pos);
@@ -237,15 +273,11 @@ save_logs(void)
 		format_debug_log(&entry->data[0], dbglog_sz);
 
 		pos += sizeof(*entry) + dbglog_sz;
-		// entry = (struct mok_variable_config_entry *)((uintptr_t)new_table + pos);
-	}
 
-	if (CT) {
-		CopyMem(&CT->VendorGuid, &bogus_guid, sizeof(bogus_guid));
+		entry = (struct mok_variable_config_entry *)((uintptr_t)new_table + pos);
 	}
-	efi_status = BS->InstallConfigurationTable(&MOK_VARIABLE_STORE, new_table);
-	if (EFI_ERROR(efi_status))
-		console_print(L"Could not re-install MoK configuration table: %r\n", efi_status);
+
+	replace_config_table((EFI_CONFIGURATION_TABLE *)cfg_table, physaddr, new_table_pages);
 }
 
 // vim:fenc=utf-8:tw=75
diff --git a/globals.c b/globals.c
index de64c44c..11197128 100644
--- a/globals.c
+++ b/globals.c
@@ -35,4 +35,7 @@ UINT8 mok_policy = 0;
 
 UINT32 verbose = 0;
 
+EFI_PHYSICAL_ADDRESS mok_config_table = 0;
+UINTN mok_config_table_pages = 0;
+
 // vim:fenc=utf-8:tw=75:noet
diff --git a/include/mok.h b/include/mok.h
index 89edf9de..f4468ab0 100644
--- a/include/mok.h
+++ b/include/mok.h
@@ -127,6 +127,9 @@ struct mok_variable_config_entry {
 	UINT8 data[];
 };
 
+extern EFI_PHYSICAL_ADDRESS mok_config_table;
+extern UINTN mok_config_table_pages;
+
 /*
  * bit definitions for MokPolicy
  */
diff --git a/mok.c b/mok.c
index fb4c1489..8367fad5 100644
--- a/mok.c
+++ b/mok.c
@@ -1305,6 +1305,8 @@ EFI_STATUS import_mok_state(EFI_HANDLE image_handle)
 			config_table = NULL;
 		} else {
 			ZeroMem(config_table, npages << EFI_PAGE_SHIFT);
+			mok_config_table = (EFI_PHYSICAL_ADDRESS)(uintptr_t)config_table;
+			mok_config_table_pages = npages;
 		}
 	}
 
-- 
cgit v1.2.3