Skip to content

Commit fb1c9f8

Browse files
committed
Work around non specs-compliant firmwares that reject EFI_VARIABLE_AUTHENTICATION_2 with ContentInfo
* Welp, the EDK2 screwed up their implementation of VerifyTimeBasedPayload(), in breach of the UEFI specs, to return EFI_SECURITY_VIOLATION if the PKCS#7 SignedData section contained a ContentInfo section, whereas, ever since 2.3.1, the specs have clearly stated that ContentInfo should be part of the signature data. * This was eventually fixed in tianocore/edk2@37d3eb0 but not before sways of UEFI firmwares, such as the HP ProDesk ones, were shipped with the non specs compliant code... * Since the HP firmware requires the provision of signed payloads when calling SetVariable() in Setup Mode, we now make sure that our signed EFI_VARIABLE_AUTHENTICATION_2 structures are stripped of the ContentInfo section. * Oh, and as a result of this mishap, recent UEFI specs have had to be modified to indicate that a modern compliant implementation should accept signed payloads with OR without ContentInfo... * Closes #17.
1 parent 060f986 commit fb1c9f8

File tree

1 file changed

+16
-2
lines changed

1 file changed

+16
-2
lines changed

src/pki.c

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -585,14 +585,28 @@ EFI_STATUS SignAuthVar(
585585
if (bio == NULL)
586586
ReportOpenSSLErrorAndExit(EFI_PROTOCOL_ERROR);
587587

588+
// Soooo, the UEFI 2.3.1 specs, released with the introduction of Secure Boot in 2011 CLEARLY stipulate
589+
// (https://uefi.org/sites/default/files/resources/UEFI_Spec_2_3_1.pdf, section 7.2.1) that ContentInfo
590+
// SHOULD be present in SignedData.
591+
// Yet, the EDK2's own implementation, which ended up being used as the base for the HP ProDesk 600 G1
592+
// as well as other UEFI firmwares, had the VerifyTimeBasedPayload() code report EFI_SECURITY_VIOLATION
593+
// if ContentInfo was present.
594+
// This was eventually fixed in https://github.com/tianocore/edk2/commit/37d3eb026a766b2405daae47e02094c2ec248646
595+
// Because of this screwup, recent UEFI specs have added an exception for EFI_VARIABLE_AUTHENTICATION_2
596+
// "Which shall be supported both with and without a DER-encoded ContentInfo structure". See for instance:
597+
// https://uefi.org/specs/UEFI/2.11/08_Services_Runtime_Services.html#using-the-efi-variable-authentication-2-descriptor
598+
// The end result of this, since we want to be compatible with HP hardware (that requires a workaround
599+
// where we always feed SetVariable() with signed data), is that we must ensure that ContentInfo is
600+
// removed from the signature data, which means using i2d_PKCS7_SIGNED(p7->d.sign, ...) instead of the
601+
// expected i2d_PKCS7(p7, ...).
588602
p7 = PKCS7_sign(NULL, NULL, NULL, bio, flags | PKCS7_PARTIAL);
589603
if (p7 == NULL)
590604
ReportOpenSSLErrorAndExit(EFI_PROTOCOL_ERROR);
591605
if (PKCS7_sign_add_signer(p7, (X509*)Credentials->Cert, (EVP_PKEY*)Credentials->Key, EVP_get_digestbyname("SHA256"), flags) == NULL)
592606
ReportOpenSSLErrorAndExit(EFI_PROTOCOL_ERROR);
593607
if (!PKCS7_final(p7, bio, flags))
594608
ReportOpenSSLErrorAndExit(EFI_PROTOCOL_ERROR);
595-
SignatureSize = i2d_PKCS7(p7, NULL);
609+
SignatureSize = i2d_PKCS7_SIGNED(p7->d.sign, NULL);
596610

597611
// Create the signed variable
598612
SignedVariable = AllocateZeroPool(Variable->Size + SignatureSize);
@@ -601,7 +615,7 @@ EFI_STATUS SignAuthVar(
601615
CopyMem(SignedVariable, Variable->Data, OFFSET_OF_AUTHINFO2_CERT_DATA);
602616
SignedVariable->AuthInfo.Hdr.dwLength = OFFSET_OF(WIN_CERTIFICATE_UEFI_GUID, CertData) + SignatureSize;
603617
Ptr = SignedVariable->AuthInfo.CertData;
604-
SignatureSize = i2d_PKCS7(p7, &Ptr);
618+
SignatureSize = i2d_PKCS7_SIGNED(p7->d.sign, &Ptr);
605619
Payload = (UINT8*)SignedVariable;
606620
Payload = &Payload[OFFSET_OF_AUTHINFO2_CERT_DATA + SignatureSize];
607621
CopyMem(Payload, SignableElement[PAYLOAD].Ptr, SignableElement[PAYLOAD].Size);

0 commit comments

Comments
 (0)