WIP - refactoring doneshim4

5 files changed, 44 insertions, 11 deletions

diff --git a/src/hci/commands/shim_cmd.c b/src/hci/commands/shim_cmd.c
index 00bd0ac..9150af3 100644
--- a/src/hci/commands/shim_cmd.c
+++ b/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:
diff --git a/src/include/ipxe/efi/efi_shim.h b/src/include/ipxe/efi/efi_shim.h
index ad8d24d..21f2431 100644
--- a/src/include/ipxe/efi/efi_shim.h
+++ b/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,
diff --git a/src/include/usr/shimmgmt.h b/src/include/usr/shimmgmt.h
index 5030607..0c59f54 100644
--- a/src/include/usr/shimmgmt.h
+++ b/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 */
diff --git a/src/interface/efi/efi_shim.c b/src/interface/efi/efi_shim.c
index 6fc77e8..1d1ed75 100644
--- a/src/interface/efi/efi_shim.c
+++ b/src/interface/efi/efi_shim.c
@@ -84,6 +84,26 @@ 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",
@@ -101,8 +121,8 @@ static EFI_SET_VARIABLE efi_shim_orig_set_variable;
 /** Original GetVariable() function */
 static EFI_GET_VARIABLE efi_shim_orig_get_variable;
 
-/** Patch reads from SbatLevel variable */
-static int efi_shim_sbatlevel_patch = 1;
+/** Verify read from SbatLevel variable */
+static int efi_shim_sbatlevel_verify;
 
 /**
  * Check if variable is SbatLevel
@@ -205,7 +225,7 @@ efi_shim_set_variable ( CHAR16 *name, EFI_GUID *guid, UINT32 attrs,
 	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_patch = 0;
+		efi_shim_sbatlevel_verify = 1;
 	}
 
 	return efirc;
@@ -232,14 +252,17 @@ efi_shim_get_variable ( CHAR16 *name, EFI_GUID *guid, UINT32 *attrs,
 
 	/* Patch SbatLevel variable if applicable */
 	if ( efi_shim_is_sbatlevel ( name, guid ) && data && ( efirc == 0 ) ) {
-		if ( efi_shim_sbatlevel_patch ) {
-			DBGC ( &efi_shim, "SHIM patching read from %ls:\n",
+		if ( efi_shim_allow_sbat ) {
+			DBGC ( &efi_shim, "SHIM allowing read from %ls:\n",
 			       name );
-			value[0] = '\0';
-		} else {
+		} else if ( efi_shim_sbatlevel_verify ) {
 			DBGC ( &efi_shim, "SHIM allowing one read from %ls:\n",
 			       name );
-			efi_shim_sbatlevel_patch = 1;
+			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 );
 	}
diff --git a/src/usr/shimmgmt.c b/src/usr/shimmgmt.c
index ba9c348..6ac1ac3 100644
--- a/src/usr/shimmgmt.c
+++ b/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;
 }