WIP - refactoring done

src/config/console.h

@@ -39,7 +39,7 @@ FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL );
 //#define	CONSOLE_SYSLOG		/* Syslog console */
 //#define	CONSOLE_SYSLOGS		/* Encrypted syslog console */
 //#define	CONSOLE_VMWARE		/* VMware logfile console */
-//#define	CONSOLE_DEBUGCON	/* Bochs/QEMU/KVM debug port console */
+#define	CONSOLE_DEBUGCON	/* Bochs/QEMU/KVM debug port console */
 //#define	CONSOLE_INT13		/* INT13 disk log console */
 
 /*

src/hci/commands/shim_cmd.c

@@ -44,6 +44,8 @@ struct shim_options {
 	int require_loader;
 	/** Allow PXE base code protocol */
 	int allow_pxe;
+	/** Allow SBAT variable access */
+	int allow_sbat;
 };
 
 /** "shim" option list */
@@ -54,6 +56,8 @@ static struct option_descriptor shim_opts[] = {
 		      struct shim_options, require_loader, parse_flag ),
 	OPTION_DESC ( "allow-pxe", 'p', no_argument,
 		      struct shim_options, allow_pxe, parse_flag ),
+	OPTION_DESC ( "allow-sbat", 's', no_argument,
+		      struct shim_options, allow_sbat, parse_flag ),
 };
 
 /** "shim" command descriptor */
@@ -94,7 +98,8 @@ static int shim_exec ( int argc, char **argv ) {
 	}
 
 	/* (Un)register as shim */
-	if ( ( rc = shim ( image, opts.require_loader, opts.allow_pxe ) ) != 0 )
+	if ( ( rc = shim ( image, opts.require_loader, opts.allow_pxe,
+			   opts.allow_sbat ) ) != 0 )
 		goto err_shim;
 
  err_shim:

src/include/ipxe/efi/efi_shim.h

@@ -14,6 +14,7 @@ FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL );
 
 extern int efi_shim_require_loader;
 extern int efi_shim_allow_pxe;
+extern int efi_shim_allow_sbat;
 extern struct image_tag efi_shim __image_tag;
 
 extern int efi_shim_install ( struct image *shim, EFI_HANDLE handle,

src/include/usr/shimmgmt.h

@@ -11,6 +11,7 @@ FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL );
 
 #include <ipxe/image.h>
 
-extern int shim ( struct image *image, int require_loader, int allow_pxe );
+extern int shim ( struct image *image, int require_loader, int allow_pxe,
+		  int allow_sbat );
 
 #endif /* _USR_SHIMMGMT_H */

src/interface/efi/efi_shim.c

@@ -84,29 +84,66 @@ int efi_shim_require_loader = 0;
  */
 int efi_shim_allow_pxe = 0;
 
+/**
+ * Allow SBAT variable access
+ *
+ * The UEFI shim implements a fairly nicely designed revocation
+ * mechanism designed around the concept of security generations.
+ * Unfortunately nobody in the shim community has thus far added the
+ * relevant metadata to the Linux kernel, with the result that current
+ * versions of shim are incapable of booting current versions of the
+ * Linux kernel.
+ *
+ * Experience shows that there is unfortunately no point in trying to
+ * get a fix for this upstreamed into shim.  We therefore default to
+ * working around this undesirable behaviour by patching accesses to
+ * the "SbatLevel" variable used to hold SBAT configuration.
+ *
+ * This option may be used to allow shim unpatched access to the
+ * "SbatLevel" variable, in case this behaviour is ever desirable.
+ */
+int efi_shim_allow_sbat = 0;
+
 /** UEFI shim image */
 struct image_tag efi_shim __image_tag = {
 	.name = "SHIM",
 };
 
 /** Original GetMemoryMap() function */
-static EFI_GET_MEMORY_MAP efi_shim_orig_map;
+static EFI_GET_MEMORY_MAP efi_shim_orig_get_memory_map;
+
+/** Original ExitBootServices() function */
+static EFI_EXIT_BOOT_SERVICES efi_shim_orig_exit_boot_services;
+
+/** Original SetVariable() function */
+static EFI_SET_VARIABLE efi_shim_orig_set_variable;
+
+/** Original GetVariable() function */
+static EFI_GET_VARIABLE efi_shim_orig_get_variable;
+
+/** Verify read from SbatLevel variable */
+static int efi_shim_sbatlevel_verify;
+
+/**
+ * Check if variable is SbatLevel
+ *
+ * @v name		Variable name
+ * @v guid		Variable namespace GUID
+ * @ret is_sbatlevel	Variable is SbatLevel
+ */
+static int efi_shim_is_sbatlevel ( const CHAR16 *name, const EFI_GUID *guid ) {
+	static CHAR16 sbatlevel[] = L"SbatLevel";
+	EFI_GUID *shimlock = &efi_shim_lock_protocol_guid;
+
+	return ( ( memcmp ( name, sbatlevel, sizeof ( sbatlevel ) ) == 0 ) &&
+		 ( memcmp ( guid, shimlock, sizeof ( *shimlock ) ) == 0 ) );
+}
 
 /**
  * Unlock UEFI shim
  *
- * @v len		Memory map size
- * @v map		Memory map
- * @v key		Memory map key
- * @v desclen		Descriptor size
- * @v descver		Descriptor version
- * @ret efirc		EFI status code
- *
  */
-static EFIAPI EFI_STATUS efi_shim_unlock ( UINTN *len,
-					   EFI_MEMORY_DESCRIPTOR *map,
-					   UINTN *key, UINTN *desclen,
-					   UINT32 *descver ) {
+static void efi_shim_unlock ( void ) {
 	EFI_BOOT_SERVICES *bs = efi_systab->BootServices;
 	uint8_t empty[0];
 	union {
@@ -121,9 +158,116 @@ static EFIAPI EFI_STATUS efi_shim_unlock ( UINTN *len,
 		u.lock->Verify ( empty, sizeof ( empty ) );
 		DBGC ( &efi_shim, "SHIM unlocked via %p\n", u.lock );
 	}
+}
+
+/**
+ * Wrap GetMemoryMap()
+ *
+ * @v len		Memory map size
+ * @v map		Memory map
+ * @v key		Memory map key
+ * @v desclen		Descriptor size
+ * @v descver		Descriptor version
+ * @ret efirc		EFI status code
+ */
+static EFIAPI EFI_STATUS efi_shim_get_memory_map ( UINTN *len,
+						   EFI_MEMORY_DESCRIPTOR *map,
+						   UINTN *key, UINTN *desclen,
+						   UINT32 *descver ) {
+
+	/* Unlock shim */
+	if ( ! efi_shim_require_loader )
+		efi_shim_unlock();
 
 	/* Hand off to original GetMemoryMap() */
-	return efi_shim_orig_map ( len, map, key, desclen, descver );
+	return efi_shim_orig_get_memory_map ( len, map, key, desclen,
+					      descver );
+}
+
+/**
+ * Wrap ExitBootServices()
+ *
+ * @v handle		Image handle
+ * @v key		Memory map key
+ * @ret efirc		EFI status code
+ */
+static EFIAPI EFI_STATUS efi_shim_exit_boot_services ( EFI_HANDLE handle,
+						       UINTN key ) {
+	EFI_RUNTIME_SERVICES *rs = efi_systab->RuntimeServices;
+
+	/* Restore original runtime services functions */
+	rs->GetVariable = efi_shim_orig_get_variable;
+	rs->SetVariable = efi_shim_orig_set_variable;
+
+	/* Hand off to original ExitBootServices() */
+	return efi_shim_orig_exit_boot_services ( handle, key );
+}
+
+/**
+ * Wrap SetVariable()
+ *
+ * @v name		Variable name
+ * @v guid		Variable namespace GUID
+ * @v attrs		Attributes
+ * @v len		Buffer size
+ * @v data		Data buffer
+ * @ret efirc		EFI status code
+ */
+static EFI_STATUS EFIAPI
+efi_shim_set_variable ( CHAR16 *name, EFI_GUID *guid, UINT32 attrs,
+			UINTN len, VOID *data ) {
+	EFI_STATUS efirc;
+
+	/* Call original SetVariable() */
+	efirc = efi_shim_orig_set_variable ( name, guid, attrs, len, data );
+
+	/* Allow verification of SbatLevel variable content */
+	if ( efi_shim_is_sbatlevel ( name, guid ) && ( efirc == 0 ) ) {
+		DBGC ( &efi_shim, "SHIM detected write to %ls:\n", name );
+		DBGC_HDA ( &efi_shim, 0, data, len );
+		efi_shim_sbatlevel_verify = 1;
+	}
+
+	return efirc;
+}
+
+/**
+ * Wrap GetVariable()
+ *
+ * @v name		Variable name
+ * @v guid		Variable namespace GUID
+ * @v attrs		Attributes to fill in
+ * @v len		Buffer size
+ * @v data		Data buffer
+ * @ret efirc		EFI status code
+ */
+static EFI_STATUS EFIAPI
+efi_shim_get_variable ( CHAR16 *name, EFI_GUID *guid, UINT32 *attrs,
+			UINTN *len, VOID *data ) {
+	char *value = data;
+	EFI_STATUS efirc;
+
+	/* Call original GetVariable() */
+	efirc = efi_shim_orig_get_variable ( name, guid, attrs, len, data );
+
+	/* Patch SbatLevel variable if applicable */
+	if ( efi_shim_is_sbatlevel ( name, guid ) && data && ( efirc == 0 ) ) {
+		if ( efi_shim_allow_sbat ) {
+			DBGC ( &efi_shim, "SHIM allowing read from %ls:\n",
+			       name );
+		} else if ( efi_shim_sbatlevel_verify ) {
+			DBGC ( &efi_shim, "SHIM allowing one read from %ls:\n",
+			       name );
+			efi_shim_sbatlevel_verify = 0;
+		} else {
+			DBGC ( &efi_shim, "SHIM patching read from %ls:\n",
+			       name );
+			value[0] = '\0';
+		}
+		DBGC_HDA ( &efi_shim, 0, data, *len );
+	}
+
+	return efirc;
 }
 
 /**
@@ -214,13 +358,9 @@ static int efi_shim_cmdline ( struct image *shim, wchar_t **cmdline ) {
 int efi_shim_install ( struct image *shim, EFI_HANDLE handle,
 		       wchar_t **cmdline ) {
 	EFI_BOOT_SERVICES *bs = efi_systab->BootServices;
+	EFI_RUNTIME_SERVICES *rs = efi_systab->RuntimeServices;
 	int rc;
 
-	/* Intercept GetMemoryMap() via boot services table */
-	efi_shim_orig_map = bs->GetMemoryMap;
-	if ( ! efi_shim_require_loader )
-		bs->GetMemoryMap = efi_shim_unlock;
-
 	/* Stop PXE base code */
 	if ( ( ! efi_shim_allow_pxe ) &&
 	     ( ( rc = efi_shim_inhibit_pxe ( handle ) ) != 0 ) ) {
@@ -231,11 +371,22 @@ int efi_shim_install ( struct image *shim, EFI_HANDLE handle,
 	if ( ( rc = efi_shim_cmdline ( shim, cmdline ) ) != 0 )
 		goto err_cmdline;
 
+	/* Record original boot and runtime services functions */
+	efi_shim_orig_get_memory_map = bs->GetMemoryMap;
+	efi_shim_orig_exit_boot_services = bs->ExitBootServices;
+	efi_shim_orig_set_variable = rs->SetVariable;
+	efi_shim_orig_get_variable = rs->GetVariable;
+
+	/* Wrap relevant boot and runtime services functions */
+	bs->GetMemoryMap = efi_shim_get_memory_map;
+	bs->ExitBootServices = efi_shim_exit_boot_services;
+	rs->SetVariable = efi_shim_set_variable;
+	rs->GetVariable = efi_shim_get_variable;
+
 	return 0;
 
  err_cmdline:
  err_inhibit_pxe:
-	bs->GetMemoryMap = efi_shim_orig_map;
 	return rc;
 }
 
@@ -245,7 +396,11 @@ int efi_shim_install ( struct image *shim, EFI_HANDLE handle,
  */
 void efi_shim_uninstall ( void ) {
 	EFI_BOOT_SERVICES *bs = efi_systab->BootServices;
+	EFI_RUNTIME_SERVICES *rs = efi_systab->RuntimeServices;
 
-	/* Restore original GetMemoryMap() */
-	bs->GetMemoryMap = efi_shim_orig_map;
+	/* Restore original boot and runtime services functions */
+	bs->GetMemoryMap = efi_shim_orig_get_memory_map;
+	bs->ExitBootServices = efi_shim_orig_exit_boot_services;
+	rs->SetVariable = efi_shim_orig_set_variable;
+	rs->GetVariable = efi_shim_orig_get_variable;
 }

src/usr/shimmgmt.c

@@ -39,9 +39,11 @@ FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL );
  * @v image		Shim image, or NULL to clear shim
  * @v require_loader	Require use of a third party loader
  * @v allow_pxe		Allow use of PXE base code
+ * @v allow_sbat	Allow SBAT variable access
  * @ret rc		Return status code
  */
-int shim ( struct image *image, int require_loader, int allow_pxe ) {
+int shim ( struct image *image, int require_loader, int allow_pxe,
+	   int allow_sbat ) {
 
 	/* Record (or clear) shim image */
 	image_tag ( image, &efi_shim );
@@ -53,6 +55,7 @@ int shim ( struct image *image, int require_loader, int allow_pxe ) {
 	/* Record configuration */
 	efi_shim_require_loader = require_loader;
 	efi_shim_allow_pxe = allow_pxe;
+	efi_shim_allow_sbat = allow_sbat;
 
 	return 0;
 }