diff --git a/CryptoPkg/Library/OpensslLib/openssl b/CryptoPkg/Library/OpensslLib/openssl index 52c587d60b..e2e09d9fba 160000 --- a/CryptoPkg/Library/OpensslLib/openssl +++ b/CryptoPkg/Library/OpensslLib/openssl @@ -1 +1 @@ -Subproject commit 52c587d60be67c337364b830dd3fdc15404a2f04 +Subproject commit e2e09d9fba1187f8d6aafaa34d4172f56f1ffb72 diff --git a/SecurityPkg/Library/SecureBootVariableProvisionLib/SecureBootVariableProvisionLib.c b/SecurityPkg/Library/SecureBootVariableProvisionLib/SecureBootVariableProvisionLib.c index 848f7ce929..bad24dd447 100644 --- a/SecurityPkg/Library/SecureBootVariableProvisionLib/SecureBootVariableProvisionLib.c +++ b/SecurityPkg/Library/SecureBootVariableProvisionLib/SecureBootVariableProvisionLib.c @@ -19,6 +19,7 @@ #include #include #include +#include /** Enroll a key/certificate based on a default variable. @@ -111,6 +112,7 @@ SecureBootInitPKDefault ( } if (EFI_ERROR (Status) && (Status != EFI_NOT_FOUND)) { + DEBUG ((DEBUG_INFO, "Variable %s read error.\n", EFI_PK_DEFAULT_VARIABLE_NAME)); return Status; } @@ -259,10 +261,10 @@ SecureBootInitDbxDefault ( IN VOID ) { - EFI_SIGNATURE_LIST *EfiSig; - UINTN SigListsSize; + UINTN Size; EFI_STATUS Status; UINT8 *Data; + VOID *Buffer; UINTN DataSize; // @@ -284,7 +286,13 @@ SecureBootInitDbxDefault ( // DEBUG ((DEBUG_INFO, "Variable %s does not exist.\n", EFI_DBX_DEFAULT_VARIABLE_NAME)); - Status = SecureBootFetchData (&gDefaultdbxFileGuid, &SigListsSize, &EfiSig); + Status = GetSectionFromAnyFv ( + &gDefaultdbxFileGuid, + EFI_SECTION_RAW, + 0, + &Buffer, + &Size + ); if (EFI_ERROR (Status)) { DEBUG ((DEBUG_INFO, "Content for %s not found\n", EFI_DBX_DEFAULT_VARIABLE_NAME)); return Status; @@ -294,15 +302,13 @@ SecureBootInitDbxDefault ( EFI_DBX_DEFAULT_VARIABLE_NAME, &gEfiGlobalVariableGuid, EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_BOOTSERVICE_ACCESS, - SigListsSize, - (VOID *)EfiSig + Size, + (VOID *)Buffer ); if (EFI_ERROR (Status)) { DEBUG ((DEBUG_INFO, "Failed to set %s\n", EFI_DBX_DEFAULT_VARIABLE_NAME)); } - FreePool (EfiSig); - return Status; } diff --git a/UefiPayloadPkg/SecureBootEnrollDefaultKeys/SecureBootSetup.c b/UefiPayloadPkg/SecureBootEnrollDefaultKeys/SecureBootSetup.c index 142ac01e06..089b1c19e6 100644 --- a/UefiPayloadPkg/SecureBootEnrollDefaultKeys/SecureBootSetup.c +++ b/UefiPayloadPkg/SecureBootEnrollDefaultKeys/SecureBootSetup.c @@ -22,302 +22,8 @@ #include #include #include - -#define EFI_MICROSOFT_KEK_CERT_GUID \ - { 0xA23665E3, 0xACA6, 0x4F6D, {0x80, 0xCC, 0x34, 0x1E, 0x7D, 0x7B, 0x8C, 0xC6} } - -#define EFI_SECUREBOOT_PK_CERT_GUID \ - { 0xF8104268, 0xA364, 0x45F5, {0x8E, 0x00, 0xAB, 0xA3, 0xFD, 0xEA, 0x12, 0xBE} } - -#define EFI_MICROSOFT_DB1_CERT_GUID \ - { 0x26A517B0, 0xE3FD, 0x46C2, {0x89, 0x32, 0xE9, 0x26, 0xBF, 0x98, 0x94, 0x1F} } - -#define EFI_MICROSOFT_DB2_CERT_GUID \ - { 0x91D2E32B, 0x0134, 0x4306, {0xBA, 0x90, 0x54, 0xED, 0xCB, 0xF3, 0x49, 0xCA} } - -#define EFI_MICROSOFT_DBX_GUID \ - { 0x74BB6E72, 0x2A56, 0x4D0E, {0xA5, 0xB3, 0x5D, 0x39, 0xFC, 0x2E, 0xE3, 0x46} } - -#define EFI_MICROSOFT_OWNER_GUID \ - { 0x77FA9ABD, 0x0359, 0x4D32, {0xBD, 0x60, 0x28, 0xF4, 0xE7, 0x8F, 0x78, 0x4B} } - -EFI_GUID gEfiSecureBootDb1CertGuid = EFI_MICROSOFT_DB1_CERT_GUID; -EFI_GUID gEfiSecureBootDb2CertGuid = EFI_MICROSOFT_DB2_CERT_GUID; -EFI_GUID gEfiSecureBootDbxCrlGuid = EFI_MICROSOFT_DBX_GUID; -EFI_GUID gEfiSecureBootKekCertGuid = EFI_MICROSOFT_KEK_CERT_GUID; -EFI_GUID gEfiSecureBootPkCertGuid = EFI_SECUREBOOT_PK_CERT_GUID; -EFI_GUID gEfiMicrosoftOwnerGuid = EFI_MICROSOFT_OWNER_GUID; - -// -// The most important thing about the variable payload is that it is a list of -// lists, where the element size of any given *inner* list is constant. -// -// Since X509 certificates vary in size, each of our *inner* lists will contain -// one element only (one X.509 certificate). This is explicitly mentioned in -// the UEFI specification, in "28.4.1 Signature Database", in a Note. -// -// The list structure looks as follows: -// -// struct EFI_VARIABLE_AUTHENTICATION_2 { | -// struct EFI_TIME { | -// UINT16 Year; | -// UINT8 Month; | -// UINT8 Day; | -// UINT8 Hour; | -// UINT8 Minute; | -// UINT8 Second; | -// UINT8 Pad1; | -// UINT32 Nanosecond; | -// INT16 TimeZone; | -// UINT8 Daylight; | -// UINT8 Pad2; | -// } TimeStamp; | -// | -// struct WIN_CERTIFICATE_UEFI_GUID { | | -// struct WIN_CERTIFICATE { | | -// UINT32 dwLength; ----------------------------------------+ | -// UINT16 wRevision; | | -// UINT16 wCertificateType; | | -// } Hdr; | +- DataSize -// | | -// EFI_GUID CertType; | | -// UINT8 CertData[1] = { <--- "struct hack" | | -// struct EFI_SIGNATURE_LIST { | | | -// EFI_GUID SignatureType; | | | -// UINT32 SignatureListSize; -------------------------+ | | -// UINT32 SignatureHeaderSize; | | | -// UINT32 SignatureSize; ---------------------------+ | | | -// UINT8 SignatureHeader[SignatureHeaderSize]; | | | | -// v | | | -// struct EFI_SIGNATURE_DATA { | | | | -// EFI_GUID SignatureOwner; | | | | -// UINT8 SignatureData[1] = { <--- "struct hack" | | | | -// X.509 payload | | | | -// } | | | | -// } Signatures[]; | | | -// } SigLists[]; | | -// }; | | -// } AuthInfo; | | -// }; | -// -// Given that the "struct hack" invokes undefined behavior (which is why C99 -// introduced the flexible array member), and because subtracting those pesky -// sizes of 1 is annoying, and because the format is fully specified in the -// UEFI specification, we'll introduce two matching convenience structures that -// are customized for our X.509 purposes. -// - -#pragma pack(1) -typedef struct { - EFI_TIME TimeStamp; - - // - // dwLength covers data below - // - UINT32 dwLength; - UINT16 wRevision; - UINT16 wCertificateType; - EFI_GUID CertType; -} SINGLE_HEADER; - -typedef struct { - // - // SignatureListSize covers data below - // - EFI_GUID SignatureType; - UINT32 SignatureListSize; - UINT32 SignatureHeaderSize; // constant 0 - UINT32 SignatureSize; - - // - // SignatureSize covers data below - // - EFI_GUID SignatureOwner; - - // - // X.509 certificate follows - // -} REPEATING_HEADER; -#pragma pack() - -/** - Enroll a set of certificates in a global variable, overwriting it. - - The variable will be rewritten with NV+BS+RT+AT attributes. - - @param[in] VariableName The name of the variable to overwrite. - - @param[in] VendorGuid The namespace (ie. vendor GUID) of the variable to - overwrite. - - @param[in] CertType The GUID determining the type of all the - certificates in the set that is passed in. For - example, gEfiCertX509Guid stands for DER-encoded - X.509 certificates, while gEfiCertSha256Guid stands - for SHA256 image hashes. - - @param[in] ... A list of - - IN CONST UINT8 *Cert, - IN UINTN CertSize, - IN CONST EFI_GUID *OwnerGuid - - triplets. If the first component of a triplet is - NULL, then the other two components are not - accessed, and processing is terminated. The list of - certificates is enrolled in the variable specified, - overwriting it. The OwnerGuid component identifies - the agent installing the certificate. - - @retval EFI_INVALID_PARAMETER The triplet list is empty (ie. the first Cert - value is NULL), or one of the CertSize values - is 0, or one of the CertSize values would - overflow the accumulated UINT32 data size. - - @retval EFI_OUT_OF_RESOURCES Out of memory while formatting variable - payload. - - @retval EFI_SUCCESS Enrollment successful; the variable has been - overwritten (or created). - - @return Error codes from gRT->GetTime() and - gRT->SetVariable(). - **/ -STATIC -EFI_STATUS -EFIAPI -EnrollListOfCerts ( - IN CHAR16 *VariableName, - IN EFI_GUID *VendorGuid, - IN EFI_GUID *CertType, - ... - ) -{ - UINTN DataSize; - SINGLE_HEADER *SingleHeader; - REPEATING_HEADER *RepeatingHeader; - VA_LIST Marker; - CONST UINT8 *Cert; - EFI_STATUS Status; - UINT8 *Data; - UINT8 *Position; - - Status = EFI_SUCCESS; - - // - // compute total size first, for UINT32 range check, and allocation - // - DataSize = sizeof *SingleHeader; - VA_START (Marker, CertType); - for (Cert = VA_ARG (Marker, CONST UINT8 *); - Cert != NULL; - Cert = VA_ARG (Marker, CONST UINT8 *)) { - UINTN CertSize; - - CertSize = VA_ARG (Marker, UINTN); - (VOID)VA_ARG (Marker, CONST EFI_GUID *); - - if (CertSize == 0 || - CertSize > MAX_UINT32 - sizeof *RepeatingHeader || - DataSize > MAX_UINT32 - sizeof *RepeatingHeader - CertSize) { - Status = EFI_INVALID_PARAMETER; - break; - } - DataSize += sizeof *RepeatingHeader + CertSize; - } - VA_END (Marker); - - if (DataSize == sizeof *SingleHeader) { - Status = EFI_INVALID_PARAMETER; - } - if (EFI_ERROR (Status)) { - DEBUG ((EFI_D_ERROR, "SecureBootSetup: Invalid certificate parameters\n")); - goto Out; - } - - Data = AllocatePool (DataSize); - if (Data == NULL) { - Status = EFI_OUT_OF_RESOURCES; - goto Out; - } - - Position = Data; - - SingleHeader = (SINGLE_HEADER *)Position; - Status = gRT->GetTime (&SingleHeader->TimeStamp, NULL); - if (EFI_ERROR (Status)) { - DEBUG ((EFI_D_INFO, "SecureBootSetup: GetTime failed\n")); - // Fill in dummy values - SingleHeader->TimeStamp.Year = 2018; - SingleHeader->TimeStamp.Month = 1; - SingleHeader->TimeStamp.Day = 1; - SingleHeader->TimeStamp.Hour = 0; - SingleHeader->TimeStamp.Minute = 0; - SingleHeader->TimeStamp.Second = 0; - Status = EFI_SUCCESS; - } - SingleHeader->TimeStamp.Pad1 = 0; - SingleHeader->TimeStamp.Nanosecond = 0; - SingleHeader->TimeStamp.TimeZone = 0; - SingleHeader->TimeStamp.Daylight = 0; - SingleHeader->TimeStamp.Pad2 = 0; - - // - // This looks like a bug in edk2. According to the UEFI specification, - // dwLength is "The length of the entire certificate, including the length of - // the header, in bytes". That shouldn't stop right after CertType -- it - // should include everything below it. - // - SingleHeader->dwLength = sizeof *SingleHeader - sizeof SingleHeader->TimeStamp; - SingleHeader->wRevision = 0x0200; - SingleHeader->wCertificateType = WIN_CERT_TYPE_EFI_GUID; - CopyGuid (&SingleHeader->CertType, &gEfiCertPkcs7Guid); - Position += sizeof *SingleHeader; - - VA_START (Marker, CertType); - for (Cert = VA_ARG (Marker, CONST UINT8 *); - Cert != NULL; - Cert = VA_ARG (Marker, CONST UINT8 *)) { - UINTN CertSize; - CONST EFI_GUID *OwnerGuid; - - CertSize = VA_ARG (Marker, UINTN); - OwnerGuid = VA_ARG (Marker, CONST EFI_GUID *); - - RepeatingHeader = (REPEATING_HEADER *)Position; - CopyGuid (&RepeatingHeader->SignatureType, CertType); - RepeatingHeader->SignatureListSize = - (UINT32)(sizeof *RepeatingHeader + CertSize); - RepeatingHeader->SignatureHeaderSize = 0; - RepeatingHeader->SignatureSize = - (UINT32)(sizeof RepeatingHeader->SignatureOwner + CertSize); - CopyGuid (&RepeatingHeader->SignatureOwner, OwnerGuid); - Position += sizeof *RepeatingHeader; - - CopyMem (Position, Cert, CertSize); - Position += CertSize; - } - VA_END (Marker); - - ASSERT (Data + DataSize == Position); - - Status = gRT->SetVariable (VariableName, VendorGuid, - (EFI_VARIABLE_NON_VOLATILE | - EFI_VARIABLE_RUNTIME_ACCESS | - EFI_VARIABLE_BOOTSERVICE_ACCESS | - EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS), - DataSize, Data); - - FreePool (Data); - -Out: - if (EFI_ERROR (Status)) { - DEBUG ((EFI_D_ERROR, "SecureBootSetup: %a(\"%s\", %g): %r\n", __FUNCTION__, VariableName, - VendorGuid, Status)); - } - return Status; -} - +#include +#include STATIC EFI_STATUS @@ -434,17 +140,6 @@ InstallSecureBootHook ( VOID *Protocol; SETTINGS Settings; - UINT8 *MicrosoftPCA = 0; - UINTN MicrosoftPCASize; - UINT8 *MicrosoftUefiCA = 0; - UINTN MicrosoftUefiCASize; - UINT8 *MicrosoftKEK = 0; - UINTN MicrosoftKEKSize; - UINT8 *SecureBootPk = 0; - UINTN SecureBootPkSize; - UINT8 *MicrosoftDbx = 0; - UINTN MicrosoftDbxSize; - Status = gBS->LocateProtocol (&gEfiVariableWriteArchProtocolGuid, NULL, (VOID **)&Protocol); if (EFI_ERROR (Status)) { return; @@ -481,68 +176,39 @@ InstallSecureBootHook ( } } - Status = GetSectionFromAnyFv(&gEfiSecureBootDb1CertGuid, EFI_SECTION_RAW, 0, (void **)&MicrosoftPCA, &MicrosoftPCASize); - ASSERT_EFI_ERROR (Status); - - Status = GetSectionFromAnyFv(&gEfiSecureBootDb2CertGuid, EFI_SECTION_RAW, 0, (void **)&MicrosoftUefiCA, &MicrosoftUefiCASize); - ASSERT_EFI_ERROR (Status); - - Status = GetSectionFromAnyFv(&gEfiSecureBootKekCertGuid, EFI_SECTION_RAW, 0, (void **)&MicrosoftKEK, &MicrosoftKEKSize); - ASSERT_EFI_ERROR (Status); - - Status = GetSectionFromAnyFv(&gEfiSecureBootPkCertGuid, EFI_SECTION_RAW, 0, (void **)&SecureBootPk, &SecureBootPkSize); - ASSERT_EFI_ERROR (Status); - - Status = GetSectionFromAnyFv(&gEfiSecureBootDbxCrlGuid, EFI_SECTION_RAW, 0, (void **)&MicrosoftDbx, &MicrosoftDbxSize); - ASSERT_EFI_ERROR (Status); - - Status = gRT->SetVariable (EFI_IMAGE_SECURITY_DATABASE1, &gEfiImageSecurityDatabaseGuid, - (EFI_VARIABLE_NON_VOLATILE | - EFI_VARIABLE_BOOTSERVICE_ACCESS | - EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS), - MicrosoftDbxSize, MicrosoftDbx); - ASSERT_EFI_ERROR (Status); - - Status = EnrollListOfCerts ( - EFI_IMAGE_SECURITY_DATABASE, - &gEfiImageSecurityDatabaseGuid, - &gEfiCertX509Guid, - MicrosoftPCA, MicrosoftPCASize, &gEfiMicrosoftOwnerGuid, - MicrosoftUefiCA, MicrosoftUefiCASize, &gEfiMicrosoftOwnerGuid, - NULL); - ASSERT_EFI_ERROR (Status); - - Status = EnrollListOfCerts ( - EFI_KEY_EXCHANGE_KEY_NAME, - &gEfiGlobalVariableGuid, - &gEfiCertX509Guid, - SecureBootPk, SecureBootPkSize, &gEfiCallerIdGuid, - MicrosoftKEK, MicrosoftKEKSize, &gEfiMicrosoftOwnerGuid, - NULL); - ASSERT_EFI_ERROR (Status); - - Status = EnrollListOfCerts ( - EFI_PLATFORM_KEY_NAME, - &gEfiGlobalVariableGuid, - &gEfiCertX509Guid, - SecureBootPk, SecureBootPkSize, &gEfiGlobalVariableGuid, - NULL); - ASSERT_EFI_ERROR (Status); - - FreePool (MicrosoftPCA); - FreePool (MicrosoftUefiCA); - FreePool (MicrosoftKEK); - FreePool (SecureBootPk); - FreePool (MicrosoftDbx); - - Settings.CustomMode = STANDARD_SECURE_BOOT_MODE; - Status = gRT->SetVariable (EFI_CUSTOM_MODE_NAME, &gEfiCustomModeEnableGuid, - EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS, - sizeof Settings.CustomMode, &Settings.CustomMode); + // Enroll all the keys from default variables + Status = EnrollDbFromDefault (); if (EFI_ERROR (Status)) { - DEBUG ((EFI_D_ERROR, "SecureBootSetup: SetVariable(\"%s\", %g): %r\n", EFI_CUSTOM_MODE_NAME, - &gEfiCustomModeEnableGuid, Status)); - ASSERT_EFI_ERROR (Status); + DEBUG ((DEBUG_ERROR, "Cannot enroll db: %r\n", Status)); + goto error; + } + + Status = EnrollDbxFromDefault (); + if (EFI_ERROR (Status)) { + DEBUG ((DEBUG_ERROR, "Cannot enroll dbx: %r\n", Status)); + } + + Status = EnrollDbtFromDefault (); + if (EFI_ERROR (Status)) { + DEBUG ((DEBUG_ERROR, "Cannot enroll dbt: %r\n", Status)); + } + + Status = EnrollKEKFromDefault (); + if (EFI_ERROR (Status)) { + DEBUG ((DEBUG_ERROR, "Cannot enroll KEK: %r\n", Status)); + goto cleardbs; + } + + Status = EnrollPKFromDefault (); + if (EFI_ERROR (Status)) { + DEBUG ((DEBUG_ERROR, "Cannot enroll PK: %r\n", Status)); + goto clearKEK; + } + + Status = SetSecureBootMode (STANDARD_SECURE_BOOT_MODE); + if (EFI_ERROR (Status)) { + DEBUG ((DEBUG_ERROR, "Cannot set CustomMode to STANDARD_SECURE_BOOT_MODE\n" + "Please do it manually, otherwise system can be easily compromised\n")); } // FIXME: Force SecureBoot to ON. The AuthService will do this if authenticated variables @@ -613,6 +279,21 @@ InstallSecureBootHook ( } DEBUG ((EFI_D_INFO, "SecureBootSetup: SecureBoot enabled\n")); + return; + +clearKEK: + DeleteKEK (); + +cleardbs: + DeleteDbt (); + DeleteDbx (); + DeleteDb (); + +error: + if (SetSecureBootMode (STANDARD_SECURE_BOOT_MODE) != EFI_SUCCESS) { + DEBUG ((DEBUG_ERROR, "Cannot set mode to Secure: %r\n", Status)); + } + DEBUG ((EFI_D_ERROR, "SecureBootSetup: disabled\n")); } EFI_STATUS diff --git a/UefiPayloadPkg/SecureBootEnrollDefaultKeys/SecureBootSetup.inf b/UefiPayloadPkg/SecureBootEnrollDefaultKeys/SecureBootSetup.inf index 513fa77615..88a246c3e8 100644 --- a/UefiPayloadPkg/SecureBootEnrollDefaultKeys/SecureBootSetup.inf +++ b/UefiPayloadPkg/SecureBootEnrollDefaultKeys/SecureBootSetup.inf @@ -45,6 +45,8 @@ UefiDriverEntryPoint DxeServicesLib UefiBootServicesTableLib + SecureBootVariableProvisionLib + SecureBootVariableLib [Protocols] gEfiTcgProtocolGuid ## CONSUMES diff --git a/UefiPayloadPkg/UefiPayloadPkg.dsc b/UefiPayloadPkg/UefiPayloadPkg.dsc index c2fd0a570c..5fa546eb76 100644 --- a/UefiPayloadPkg/UefiPayloadPkg.dsc +++ b/UefiPayloadPkg/UefiPayloadPkg.dsc @@ -537,6 +537,7 @@ !if $(SECURE_BOOT_ENABLE) == TRUE SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf SecurityPkg/VariableAuthenticated/SecureBootDefaultKeysDxe/SecureBootDefaultKeysDxe.inf + UefiPayloadPkg/SecureBootEnrollDefaultKeys/SecureBootSetup.inf !endif UefiCpuPkg/CpuDxe/CpuDxe.inf diff --git a/UefiPayloadPkg/UefiPayloadPkg.fdf b/UefiPayloadPkg/UefiPayloadPkg.fdf index 80af29408e..fcf1eadb3d 100644 --- a/UefiPayloadPkg/UefiPayloadPkg.fdf +++ b/UefiPayloadPkg/UefiPayloadPkg.fdf @@ -93,6 +93,7 @@ APRIORI DXE { !if $(SECURE_BOOT_ENABLE) == TRUE INF PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcatRealTimeClockRuntimeDxe.inf INF SecurityPkg/VariableAuthenticated/SecureBootDefaultKeysDxe/SecureBootDefaultKeysDxe.inf # After SMBusConfigLoader and PcatRealTimeClockRuntimeDxe, before Tcg2Dxe + INF UefiPayloadPkg/SecureBootEnrollDefaultKeys/SecureBootSetup.inf !endif } @@ -243,6 +244,7 @@ INF SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.inf !if $(SECURE_BOOT_ENABLE) == TRUE INF SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf INF SecurityPkg/VariableAuthenticated/SecureBootDefaultKeysDxe/SecureBootDefaultKeysDxe.inf + INF UefiPayloadPkg/SecureBootEnrollDefaultKeys/SecureBootSetup.inf FILE FREEFORM = 85254ea7-4759-4fc4-82d4-5eed5fb0a4a0 { SECTION RAW = UefiPayloadPkg/SecureBootEnrollDefaultKeys/keys/pk.crt