Always use signed authvars to update variables

* Some poorly designed UEFI firmwares (HP ProDesk 600 G1 yet again) do *not* accept
  anything that is not signed when writing the Secure Boot variables in Setup Mode.
* This means that, with the existing code, only the DBX and PK variables can be
  written, whereas DB and KEK (which we build as unsigned ESLs from the certs) will
  fail with EFI_SECURITY_VIOLATION error.
* To work around this, we now always sign unsigned variables with the PK credentials
  we autogenerate.
* Addresses part of #17.

Author: pbatard

src/mosby.c

@@ -42,13 +42,8 @@ STATIC EFI_GUID gEfiShimLockGuid =
 EFI_GUID gEfiMicrosoftGuid =
 	{ 0x77FA9ABD, 0x0359, 0x4D32, { 0xBD, 0x60, 0x28, 0xF4, 0xE7, 0x8F, 0x78, 0x4B } };
 
-/* Attributes for the "key" types we support */
-STATIC struct {
-	CHAR8 *DisplayName;
-	CHAR16 *OptionName;
-	CHAR16 *VariableName;
-	EFI_GUID *VariableGuid;
-} KeyInfo[MAX_TYPES] = {
+/* Populate the key type attributes */
+MOSBY_KEY_INFO KeyInfo[MAX_TYPES] = {
 	[PK] = {
 		.DisplayName  = "PK:  ",
 		.OptionName   = L"-pk",
@@ -229,7 +224,7 @@ EFI_STATUS EFIAPI efi_main(
 	UINT16* SystemSSPV = NULL;
 	UINT32 SystemSBatVer = 0, InstallSBatVer = 0;
 	INTN Argc, Type, Sel, LastEntry;
-	MOSBY_CRED Cred = { 0 };
+	MOSBY_CRED PkCred = { 0 }, DbCred = { 0 };
 	CHAR8 DbSubject[80], PkSubject[80], *SBat = NULL, *SBatLine = NULL;
 	CHAR16 **Argv = NULL, **ArgvCopy, MosbyKeyPath[MAX_PATH];
 	MOSBY_LIST List;
@@ -414,6 +409,17 @@ EFI_STATUS EFIAPI efi_main(
 	}
 
 process_binaries:
+
+	/* Generate the credentials, which we use both to sign the authvars as well as PK */
+	Status = gRT->GetTime(&Time, NULL);
+	if (EFI_ERROR(Status))
+		ReportErrorAndExit(L"Failed to get time - Aborting\n");
+	AsciiSPrint(PkSubject, sizeof(PkSubject), "Mosby Generated PK [%04d.%02d.%02d]",
+		Time.Year, Time.Month, Time.Day);
+	Status = GenerateCredentials(PkSubject, &PkCred);
+	if (EFI_ERROR(Status))
+		ReportErrorAndExit(L"Failed to generate signing credentials - Aborting\n");
+
 	/* Process binaries that have been provided to the application */
 	for (i = 0; i < List.Size; i++) {
 		if (List.Entry[i].Variable.Data != NULL)
@@ -435,7 +441,7 @@ EFI_STATUS EFIAPI efi_main(
 				List.Entry[i].Attrs = UEFI_VAR_NV_BS;
 				break;
 			default:
-				Status = PopulateAuthVar(&List.Entry[i]);
+				Status = PopulateAuthVar(&List.Entry[i], &PkCred);
 				if (EFI_ERROR(Status))
 					ReportErrorAndExit(L"Failed to create variable - Aborting\n");
 				break;
@@ -492,58 +498,44 @@ EFI_STATUS EFIAPI efi_main(
 		i = List.Size;
 		RecallPrint(L"Generating Secure Boot signing credentials...\n");
 		List.Entry[i].Type = DB;
-		Status = gRT->GetTime(&Time, NULL);
-		if (EFI_ERROR(Status))
-			ReportErrorAndExit(L"Failed to get time - Aborting\n");
 		AsciiSPrint(DbSubject, sizeof(DbSubject), "%a [%04d.%02d.%02d]",
 			MOSBY_CRED_NAME, Time.Year, Time.Month, Time.Day);
 		List.Entry[i].Description = DbSubject;
-		Status = GenerateCredentials(DbSubject, &Cred);
+		Status = GenerateCredentials(DbSubject, &DbCred);
 		if (EFI_ERROR(Status))
 			goto exit;
-		Status = CertToAuthVar(Cred.Cert, &List.Entry[i].Variable, FALSE);
-		if (EFI_ERROR(Status)) {
-			FreeCredentials(&Cred);
+		Status = CertToAuthVar(DbCred.Cert, &List.Entry[i].Variable, FALSE);
+		if (EFI_ERROR(Status))
 			goto exit;
-		}
-		List.Entry[i].Attrs = UEFI_VAR_NV_BS_RT_AT_AP;
-		Status = SaveCredentials(WIDEN(MOSBY_CRED_NAME), &Cred);
+		Status = SaveCredentials(WIDEN(MOSBY_CRED_NAME), &DbCred);
 		if (EFI_ERROR(Status))
 			goto exit;
 		RecallPrint(L"Saved Secure Boot signing credentials as '%a'\n", MOSBY_CRED_NAME);
-		FreeCredentials(&Cred);
+
+		List.Entry[i].Attrs = UEFI_VAR_NV_BS_RT_AT_AP;
+		Status = SignAuthVar(KeyInfo[DB].VariableName, KeyInfo[DB].VariableGuid,
+			List.Entry[i].Attrs, &List.Entry[i].Variable, &PkCred);
+		if (EFI_ERROR(Status))
+			ReportErrorAndExit(L"Failed to sign DB\n");
 		List.Size++;
 	}
 
-	/* Generate a keyless PK cert if none was specified */
+	/* Set up the PK if none was specified */
 	LastEntry = RemoveDuplicates(PK, &List);
 	if (LastEntry < 0) {
 		if (List.Size >= MOSBY_MAX_LIST_SIZE)
 			Abort(EFI_OUT_OF_RESOURCES, L"List size is too small\n");
 		RecallPrint(L"Generating PK certificate...\n");
 		i = List.Size;
 		List.Entry[i].Type = PK;
-		if (!GenDBCred) {
-			Status = gRT->GetTime(&Time, NULL);
-			if (EFI_ERROR(Status))
-				ReportErrorAndExit(L"Failed to get time - Aborting\n");
-		}
-		AsciiSPrint(PkSubject, sizeof(PkSubject), "Mosby Generated PK [%04d.%02d.%02d]",
-			Time.Year, Time.Month, Time.Day);
 		List.Entry[i].Description = PkSubject;
-		Status = GenerateCredentials(PkSubject, &Cred);
+		Status = CertToAuthVar(PkCred.Cert, &List.Entry[i].Variable, FALSE);
 		if (EFI_ERROR(Status))
 			goto exit;
-		Status = CertToAuthVar(Cred.Cert, &List.Entry[i].Variable, FALSE);
-		if (EFI_ERROR(Status)) {
-			FreeCredentials(&Cred);
-			goto exit;
-		}
 		// PK must be signed
 		List.Entry[i].Attrs = UEFI_VAR_NV_BS_RT_AT;
-		Status = SignToAuthVar(KeyInfo[PK].VariableName, KeyInfo[PK].VariableGuid,
-			List.Entry[i].Attrs, &List.Entry[i].Variable, &Cred);
-		FreeCredentials(&Cred);
+		Status = SignAuthVar(KeyInfo[PK].VariableName, KeyInfo[PK].VariableGuid,
+			List.Entry[i].Attrs, &List.Entry[i].Variable, &PkCred);
 		if (EFI_ERROR(Status)) {
 			SafeFree(List.Entry[i].Variable.Data);
 			goto exit;
@@ -554,9 +546,10 @@ EFI_STATUS EFIAPI efi_main(
 #if defined(_M_X64) || defined(__x86_64__) || defined(_M_IX86) || defined(__i386__)
 	/*
 	 * There appears to be a whole sway of AMI UEFI firmwares with a rather unfortunate bug,
-	 * that prevents appending to an existing KEK store. Which means that, on the affected
+	 * that prevents appending to an existing KEK store (or even creating the initial KEK
+	 * variable if it is done with the append flag set). Which means that, on the affected
 	 * systems, if we try to write more than one KEK, using multiple SetVariable() calls,
-	 * only the first call succeeds and all subsequent ones fail with EFI_INVALID_PARAMETER.
+	 * only the first can succeed and all subsequent ones fail with EFI_INVALID_PARAMETER.
 	 * To work around this and since any unsigned KEK we processed above should already have
 	 * been converted to an ESL (embedded in an EFI_VARIABLE_AUTHENTICATION_2 struct), we
 	 * concatenate all these ESLs into a single array, which can then be written through a
@@ -581,8 +574,9 @@ EFI_STATUS EFIAPI efi_main(
 			RecallPrint(L"Adding '%s' to Merged KEK List\n", List.Entry[i].Path);
 		/* Get the ESL data from EFI_VARIABLE_AUTHENTICATION_2 (at .AuthInfo.CertData) */
 		Esl[EslIndex++] = (EFI_SIGNATURE_LIST*)&((UINT8*)List.Entry[i].Variable.Data)[
+			((List.Entry[i].Variable.Data->AuthInfo.CertData[2] << 8) | List.Entry[i].Variable.Data->AuthInfo.CertData[3]) +
 			OFFSET_OF(EFI_VARIABLE_AUTHENTICATION_2, AuthInfo) +
-			OFFSET_OF(WIN_CERTIFICATE_UEFI_GUID, CertData)];
+			OFFSET_OF(WIN_CERTIFICATE_UEFI_GUID, CertData) + 4];
 		/* Remove the individual KEK from our installation list, since it will be merged */
 		List.Entry[i].Flags |= NO_INSTALL;
 	}
@@ -604,7 +598,7 @@ EFI_STATUS EFIAPI efi_main(
 		List.Entry[List.Size].Attrs = UEFI_VAR_NV_BS_RT_AT;
 		List.Entry[List.Size].Description = "Merged KEK List";
 		List.Entry[List.Size].Path = L"Merged KEK List";
-		Status = PopulateAuthVar(&List.Entry[List.Size]);
+		Status = PopulateAuthVar(&List.Entry[List.Size], &PkCred);
 		if (EFI_ERROR(Status))
 			ReportErrorAndExit(L"Failed to create merged KEK variable - Aborting\n");
 		List.Size++;
@@ -644,6 +638,8 @@ EFI_STATUS EFIAPI efi_main(
 exit:
 	for (i = 0; i < List.Size; i++)
 		FreePool(List.Entry[i].Variable.Data);
+	FreeCredentials(&DbCred);
+	FreeCredentials(&PkCred);
 	FreePool(Argv);
 	RecallPrintFree();
 	CloseLogger();

src/mosby.h

@@ -114,6 +114,14 @@ enum {
 	MAX_TYPES
 };
 
+/* Attributes for the "key" types we support */
+typedef struct {
+	CHAR8 *DisplayName;
+	CHAR16 *OptionName;
+	CHAR16 *VariableName;
+	EFI_GUID *VariableGuid;
+} MOSBY_KEY_INFO;
+
 /* Mosby buffer struct */
 typedef struct {
 	UINTN Size;

src/pki.c

@@ -50,6 +50,8 @@
 	ERR_print_errors_cb(OpenSSLErrorCallback, _ErrMsg); goto exit;  \
 	} while(0)
 
+extern MOSBY_KEY_INFO KeyInfo[MAX_TYPES];
+
 STATIC EFI_TIME mTime = { 0 };
 
 /* For OpenSSL error reporting */
@@ -406,15 +408,16 @@ EFI_STATUS CertToAuthVar(
 }
 
 EFI_STATUS PopulateAuthVar(
-	IN OUT MOSBY_ENTRY *Entry
+	IN OUT MOSBY_ENTRY *Entry,
+	IN MOSBY_CRED *Credentials
 )
 {
 	EFI_STATUS Status = EFI_INVALID_PARAMETER;
 	UINTN HeaderSize;
 	CONST UINT8 *Ptr;
 	EFI_SIGNATURE_LIST *Esl = NULL;
 	MOSBY_CRED Cred = { 0 };
-	EFI_VARIABLE_AUTHENTICATION_2 *SignedEsl = NULL;
+	EFI_VARIABLE_AUTHENTICATION_2 *AuthVar = NULL;
 	PKCS12 *p12 = NULL;
 	BIO *bio = NULL;
 
@@ -429,15 +432,15 @@ EFI_STATUS PopulateAuthVar(
 		Entry->Attrs = (Entry->Type == MOK) ? UEFI_VAR_NV_BS_AP : UEFI_VAR_NV_BS_RT_AT_AP;
 
 	// Check for signed ESL (PKCS#7 only)
-	SignedEsl = (EFI_VARIABLE_AUTHENTICATION_2*)Entry->Buffer.Data;
+	AuthVar = (EFI_VARIABLE_AUTHENTICATION_2*)Entry->Buffer.Data;
 	if (Entry->Buffer.Size > sizeof(EFI_VARIABLE_AUTHENTICATION_2) &&
-		SignedEsl->AuthInfo.Hdr.dwLength < Entry->Buffer.Size &&
-		SignedEsl->AuthInfo.Hdr.wRevision == 0x0200 &&
-		SignedEsl->AuthInfo.Hdr.wCertificateType == WIN_CERT_TYPE_EFI_GUID &&
-		CompareGuid(&SignedEsl->AuthInfo.CertType, &gEfiCertPkcs7Guid)) {
-		if (SignedEsl->AuthInfo.CertData[0] != 0x30 || SignedEsl->AuthInfo.CertData[1] != 0x82)
+		AuthVar->AuthInfo.Hdr.dwLength < Entry->Buffer.Size &&
+		AuthVar->AuthInfo.Hdr.wRevision == 0x0200 &&
+		AuthVar->AuthInfo.Hdr.wCertificateType == WIN_CERT_TYPE_EFI_GUID &&
+		CompareGuid(&AuthVar->AuthInfo.CertType, &gEfiCertPkcs7Guid)) {
+		if (AuthVar->AuthInfo.CertData[0] != 0x30 || AuthVar->AuthInfo.CertData[1] != 0x82)
 			ReportErrorAndExit(L"Invalid signed ESL '%s'\n", Entry->Path);
-		HeaderSize = (SignedEsl->AuthInfo.CertData[2] << 8) | SignedEsl->AuthInfo.CertData[3];
+		HeaderSize = (AuthVar->AuthInfo.CertData[2] << 8) | AuthVar->AuthInfo.CertData[3];
 		HeaderSize += OFFSET_OF(EFI_VARIABLE_AUTHENTICATION_2, AuthInfo);
 		HeaderSize += OFFSET_OF(WIN_CERTIFICATE_UEFI_GUID, CertData);
 		HeaderSize += 4;	// For the 4 extra bytes above
@@ -454,11 +457,11 @@ EFI_STATUS PopulateAuthVar(
 		if ((UINTN)Esl != (UINTN)&Entry->Buffer.Data[Entry->Buffer.Size])
 			ReportErrorAndExit(L"Invalid signed ESL '%s'\n", Entry->Path);
 		Entry->Variable.Size = Entry->Buffer.Size;
-		Entry->Variable.Data = SignedEsl;
+		Entry->Variable.Data = AuthVar;
 		Entry->Flags |= ALLOW_UPDATE;
 		Entry->Buffer.Data = NULL;	// Don't double free our data
-		Status = EFI_SUCCESS;
-		goto exit;
+		// TODO: Do we want to validate the signature too?
+		return EFI_SUCCESS;
 	}
 
 	// Check for a DER encoded X509 certificate
@@ -518,11 +521,14 @@ EFI_STATUS PopulateAuthVar(
 		Entry->Attrs = 0;
 		Entry->Variable.Size = 0;
 		SafeFree(Entry->Variable.Data);
+		return Status;
 	}
-	return Status;
+	// Sign the authvar
+	return SignAuthVar(KeyInfo[Entry->Type].VariableName, KeyInfo[Entry->Type].VariableGuid,
+			Entry->Attrs, &Entry->Variable, Credentials);
 }
 
-EFI_STATUS SignToAuthVar(
+EFI_STATUS SignAuthVar(
 	IN CONST CHAR16 *VariableName,
 	IN CONST EFI_GUID *VariableGuid,
 	IN CONST UINT32 Attributes,

src/pki.h

@@ -1,6 +1,6 @@
 /*
  * MSSB (More Secure Secure Boot -- "Mosby") PKI/OpenSSL functions
- * Copyright 2024-2025 Pete Batard <pete@akeo.ie>
+ * Copyright 2024-2026 Pete Batard <pete@akeo.ie>
  *
  * This program is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -51,10 +51,11 @@ EFI_STATUS CertToAuthVar(
 );
 
 EFI_STATUS PopulateAuthVar(
-	IN OUT MOSBY_ENTRY *Entry
+	IN OUT MOSBY_ENTRY *Entry,
+	IN MOSBY_CRED *Credentials
 );
 
-EFI_STATUS SignToAuthVar(
+EFI_STATUS SignAuthVar(
 	IN CONST CHAR16 *VariableName,
 	IN CONST EFI_GUID *VendorGuid,
 	IN CONST UINT32 Attributes,