From f3acec14352b034d79ca854258cb3c0bc06df3a9a8375d766c148c7bfb770835 Mon Sep 17 00:00:00 2001 From: Gary Ching-Pang Lin Date: Fri, 6 Mar 2020 06:45:54 +0000 Subject: [PATCH] Accepting request 782008 from home:gary_lin:branches:Virtualization - Update to edk2-stable202002 OBS-URL: https://build.opensuse.org/request/show/782008 OBS-URL: https://build.opensuse.org/package/show/Virtualization/ovmf?expand=0&rev=155 --- edk2-stable201911.tar.gz | 3 - edk2-stable202002.tar.gz | 3 + ovmf-bsc1163927-fix-ip4dxe-and-arpdxe.patch | 168 -- ...BootScriptLib-fix-numeric-truncation.patch | 166 -- ...3969-fix-DxeImageVerificationHandler.patch | 1870 ----------------- ovmf-gdb-symbols.patch | 20 +- ovmf.changes | 221 ++ ovmf.spec | 8 +- 8 files changed, 235 insertions(+), 2224 deletions(-) delete mode 100644 edk2-stable201911.tar.gz create mode 100644 edk2-stable202002.tar.gz delete mode 100644 ovmf-bsc1163927-fix-ip4dxe-and-arpdxe.patch delete mode 100644 ovmf-bsc1163959-PiDxeS3BootScriptLib-fix-numeric-truncation.patch delete mode 100644 ovmf-bsc1163969-fix-DxeImageVerificationHandler.patch diff --git a/edk2-stable201911.tar.gz b/edk2-stable201911.tar.gz deleted file mode 100644 index 833ecea..0000000 --- a/edk2-stable201911.tar.gz +++ /dev/null @@ -1,3 +0,0 @@ -version https://git-lfs.github.com/spec/v1 -oid sha256:23affd4ca2ba526747e72cbb350a4c95d192ac14eeb616778b1976577ed06001 -size 13821169 diff --git a/edk2-stable202002.tar.gz b/edk2-stable202002.tar.gz new file mode 100644 index 0000000..33a9172 --- /dev/null +++ b/edk2-stable202002.tar.gz @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:284eb25bdd78e40a969f3fd35e96c7c2a2a5903106ddf08282c194cb561400f4 +size 13988974 diff --git a/ovmf-bsc1163927-fix-ip4dxe-and-arpdxe.patch b/ovmf-bsc1163927-fix-ip4dxe-and-arpdxe.patch deleted file mode 100644 index 716c33f..0000000 --- a/ovmf-bsc1163927-fix-ip4dxe-and-arpdxe.patch +++ /dev/null @@ -1,168 +0,0 @@ -From 7f9f7fccf58af2db5ac8c88801f56f4efe664fcb Mon Sep 17 00:00:00 2001 -From: Jiaxin Wu -Date: Mon, 29 Apr 2019 09:51:53 +0800 -Subject: [PATCH 1/2] NetworkPkg/Ip4Dxe: Check the received package length - (CVE-2019-14559). - -v3: correct the coding style. -v2: correct the commit message & add BZ number. - -REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1610 - -This patch is to check the received package length to make sure the package -has a valid length field. - -Cc: Fu Siyuan -Cc: Maciej Rabeda -Signed-off-by: Wu Jiaxin -Reviewed-by: Siyuan Fu -(cherry picked from commit 578bcdc2605e3438b9cbdac4e68339f90f5bf8af) ---- - NetworkPkg/Ip4Dxe/Ip4Input.c | 46 +++++++++++++++++++++++++++++------- - 1 file changed, 37 insertions(+), 9 deletions(-) - -diff --git a/NetworkPkg/Ip4Dxe/Ip4Input.c b/NetworkPkg/Ip4Dxe/Ip4Input.c -index 24c584658803..fc1a892f14eb 100644 ---- a/NetworkPkg/Ip4Dxe/Ip4Input.c -+++ b/NetworkPkg/Ip4Dxe/Ip4Input.c -@@ -1,7 +1,7 @@ - /** @file - IP4 input process. - --Copyright (c) 2005 - 2018, Intel Corporation. All rights reserved.
-+Copyright (c) 2005 - 2020, Intel Corporation. All rights reserved.
- (C) Copyright 2015 Hewlett-Packard Development Company, L.P.
- - SPDX-License-Identifier: BSD-2-Clause-Patent -@@ -711,10 +711,6 @@ Ip4PreProcessPacket ( - // - // Check if the IP4 header is correctly formatted. - // -- if ((*Packet)->TotalSize < IP4_MIN_HEADLEN) { -- return EFI_INVALID_PARAMETER; -- } -- - HeadLen = (Head->HeadLen << 2); - TotalLen = NTOHS (Head->TotalLen); - -@@ -808,6 +804,30 @@ Ip4PreProcessPacket ( - return EFI_SUCCESS; - } - -+/** -+ This function checks the IPv4 packet length. -+ -+ @param[in] Packet Pointer to the IPv4 Packet to be checked. -+ -+ @retval TRUE The input IPv4 packet length is valid. -+ @retval FALSE The input IPv4 packet length is invalid. -+ -+**/ -+BOOLEAN -+Ip4IsValidPacketLength ( -+ IN NET_BUF *Packet -+ ) -+{ -+ // -+ // Check the IP4 packet length. -+ // -+ if (Packet->TotalSize < IP4_MIN_HEADLEN) { -+ return FALSE; -+ } -+ -+ return TRUE; -+} -+ - /** - The IP4 input routine. It is called by the IP4_INTERFACE when a - IP4 fragment is received from MNP. -@@ -844,6 +864,10 @@ Ip4AccpetFrame ( - goto DROP; - } - -+ if (!Ip4IsValidPacketLength (Packet)) { -+ goto RESTART; -+ } -+ - Head = (IP4_HEAD *) NetbufGetByte (Packet, 0, NULL); - ASSERT (Head != NULL); - OptionLen = (Head->HeadLen << 2) - IP4_MIN_HEADLEN; -@@ -890,10 +914,14 @@ Ip4AccpetFrame ( - // - ZeroMem (&ZeroHead, sizeof (IP4_HEAD)); - if (0 == CompareMem (Head, &ZeroHead, sizeof (IP4_HEAD))) { -- // Packet may have been changed. Head, HeadLen, TotalLen, and -- // info must be reloaded bofore use. The ownership of the packet -- // is transfered to the packet process logic. -- // -+ // Packet may have been changed. Head, HeadLen, TotalLen, and -+ // info must be reloaded before use. The ownership of the packet -+ // is transferred to the packet process logic. -+ // -+ if (!Ip4IsValidPacketLength (Packet)) { -+ goto RESTART; -+ } -+ - Head = (IP4_HEAD *) NetbufGetByte (Packet, 0, NULL); - ASSERT (Head != NULL); - Status = Ip4PreProcessPacket ( --- -2.25.0 - - -From 03225826203c978146e4067e1d14fe66fcb75e22 Mon Sep 17 00:00:00 2001 -From: Siyuan Fu -Date: Fri, 21 Feb 2020 10:14:18 +0800 -Subject: [PATCH 2/2] NetworkPkg/ArpDxe: Recycle invalid ARP packets - (CVE-2019-14559) - -REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2031 - -This patch triggers the RecycleEvent for invalid ARP packets. -Prior to this, we would just ignore invalid ARP packets, -and never free them. - -Cc: Jiaxin Wu -Cc: Maciej Rabeda -Cc: Siyuan Fu -Signed-off-by: Nicholas Armour -Reviewed-by: Siyuan Fu -(cherry picked from commit 1d3215fd24f47eaa4877542a59b4bbf5afc0cfe8) ---- - NetworkPkg/ArpDxe/ArpImpl.c | 6 +++--- - 1 file changed, 3 insertions(+), 3 deletions(-) - -diff --git a/NetworkPkg/ArpDxe/ArpImpl.c b/NetworkPkg/ArpDxe/ArpImpl.c -index 0e9ef103eff9..c7f770db0734 100644 ---- a/NetworkPkg/ArpDxe/ArpImpl.c -+++ b/NetworkPkg/ArpDxe/ArpImpl.c -@@ -1,7 +1,7 @@ - /** @file - The implementation of the ARP protocol. - --Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
-+Copyright (c) 2006 - 2020, Intel Corporation. All rights reserved.
- SPDX-License-Identifier: BSD-2-Clause-Patent - - **/ -@@ -113,7 +113,7 @@ ArpOnFrameRcvdDpc ( - // - // Restart the receiving if packet size is not correct. - // -- goto RESTART_RECEIVE; -+ goto RECYCLE_RXDATA; - } - - // -@@ -125,7 +125,7 @@ ArpOnFrameRcvdDpc ( - Head->OpCode = NTOHS (Head->OpCode); - - if (RxData->DataLength < (sizeof (ARP_HEAD) + 2 * Head->HwAddrLen + 2 * Head->ProtoAddrLen)) { -- goto RESTART_RECEIVE; -+ goto RECYCLE_RXDATA; - } - - if ((Head->HwType != ArpService->SnpMode.IfType) || --- -2.25.0 - diff --git a/ovmf-bsc1163959-PiDxeS3BootScriptLib-fix-numeric-truncation.patch b/ovmf-bsc1163959-PiDxeS3BootScriptLib-fix-numeric-truncation.patch deleted file mode 100644 index 98080d8..0000000 --- a/ovmf-bsc1163959-PiDxeS3BootScriptLib-fix-numeric-truncation.patch +++ /dev/null @@ -1,166 +0,0 @@ -From 322ac05f8bbc1bce066af1dabd1b70ccdbe28891 Mon Sep 17 00:00:00 2001 -From: Hao A Wu -Date: Fri, 28 Jun 2019 14:15:55 +0800 -Subject: [PATCH 1/1] MdeModulePkg/PiDxeS3BootScriptLib: Fix potential numeric - truncation (CVE-2019-14563) - -REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2001 - -For S3BootScriptLib APIs: - -S3BootScriptSaveIoWrite -S3BootScriptSaveMemWrite -S3BootScriptSavePciCfgWrite -S3BootScriptSavePciCfg2Write -S3BootScriptSaveSmbusExecute -S3BootScriptSaveInformation -S3BootScriptSaveInformationAsciiString -S3BootScriptLabel (happen in S3BootScriptLabelInternal()) - -possible numeric truncations will happen that may lead to S3 boot script -entry with improper size being returned to store the boot script data. -This commit will add checks to prevent this kind of issue. - -Please note that the remaining S3BootScriptLib APIs: - -S3BootScriptSaveIoReadWrite -S3BootScriptSaveMemReadWrite -S3BootScriptSavePciCfgReadWrite -S3BootScriptSavePciCfg2ReadWrite -S3BootScriptSaveStall -S3BootScriptSaveDispatch2 -S3BootScriptSaveDispatch -S3BootScriptSaveMemPoll -S3BootScriptSaveIoPoll -S3BootScriptSavePciPoll -S3BootScriptSavePci2Poll -S3BootScriptCloseTable -S3BootScriptExecute -S3BootScriptMoveLastOpcode -S3BootScriptCompare - -are not affected by such numeric truncation. - -Signed-off-by: Hao A Wu -Reviewed-by: Laszlo Ersek -Reviewed-by: Eric Dong -Acked-by: Jian J Wang ---- - .../PiDxeS3BootScriptLib/BootScriptSave.c | 52 ++++++++++++++++++- - 1 file changed, 51 insertions(+), 1 deletion(-) - -diff --git a/MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptSave.c b/MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptSave.c -index 9106e7d0f9f5..9315fc9f0188 100644 ---- a/MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptSave.c -+++ b/MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptSave.c -@@ -1,7 +1,7 @@ - /** @file - Save the S3 data to S3 boot script. - -- Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.
-+ Copyright (c) 2006 - 2020, Intel Corporation. All rights reserved.
- - SPDX-License-Identifier: BSD-2-Clause-Patent - -@@ -1006,6 +1006,14 @@ S3BootScriptSaveIoWrite ( - EFI_BOOT_SCRIPT_IO_WRITE ScriptIoWrite; - - WidthInByte = (UINT8) (0x01 << (Width & 0x03)); -+ -+ // -+ // Truncation check -+ // -+ if ((Count > MAX_UINT8) || -+ (WidthInByte * Count > MAX_UINT8 - sizeof (EFI_BOOT_SCRIPT_IO_WRITE))) { -+ return RETURN_OUT_OF_RESOURCES; -+ } - Length = (UINT8)(sizeof (EFI_BOOT_SCRIPT_IO_WRITE) + (WidthInByte * Count)); - - Script = S3BootScriptGetEntryAddAddress (Length); -@@ -1102,6 +1110,14 @@ S3BootScriptSaveMemWrite ( - EFI_BOOT_SCRIPT_MEM_WRITE ScriptMemWrite; - - WidthInByte = (UINT8) (0x01 << (Width & 0x03)); -+ -+ // -+ // Truncation check -+ // -+ if ((Count > MAX_UINT8) || -+ (WidthInByte * Count > MAX_UINT8 - sizeof (EFI_BOOT_SCRIPT_MEM_WRITE))) { -+ return RETURN_OUT_OF_RESOURCES; -+ } - Length = (UINT8)(sizeof (EFI_BOOT_SCRIPT_MEM_WRITE) + (WidthInByte * Count)); - - Script = S3BootScriptGetEntryAddAddress (Length); -@@ -1206,6 +1222,14 @@ S3BootScriptSavePciCfgWrite ( - } - - WidthInByte = (UINT8) (0x01 << (Width & 0x03)); -+ -+ // -+ // Truncation check -+ // -+ if ((Count > MAX_UINT8) || -+ (WidthInByte * Count > MAX_UINT8 - sizeof (EFI_BOOT_SCRIPT_PCI_CONFIG_WRITE))) { -+ return RETURN_OUT_OF_RESOURCES; -+ } - Length = (UINT8)(sizeof (EFI_BOOT_SCRIPT_PCI_CONFIG_WRITE) + (WidthInByte * Count)); - - Script = S3BootScriptGetEntryAddAddress (Length); -@@ -1324,6 +1348,14 @@ S3BootScriptSavePciCfg2Write ( - } - - WidthInByte = (UINT8) (0x01 << (Width & 0x03)); -+ -+ // -+ // Truncation check -+ // -+ if ((Count > MAX_UINT8) || -+ (WidthInByte * Count > MAX_UINT8 - sizeof (EFI_BOOT_SCRIPT_PCI_CONFIG2_WRITE))) { -+ return RETURN_OUT_OF_RESOURCES; -+ } - Length = (UINT8)(sizeof (EFI_BOOT_SCRIPT_PCI_CONFIG2_WRITE) + (WidthInByte * Count)); - - Script = S3BootScriptGetEntryAddAddress (Length); -@@ -1549,6 +1581,12 @@ S3BootScriptSaveSmbusExecute ( - return Status; - } - -+ // -+ // Truncation check -+ // -+ if (BufferLength > MAX_UINT8 - sizeof (EFI_BOOT_SCRIPT_SMBUS_EXECUTE)) { -+ return RETURN_OUT_OF_RESOURCES; -+ } - DataSize = (UINT8)(sizeof (EFI_BOOT_SCRIPT_SMBUS_EXECUTE) + BufferLength); - - Script = S3BootScriptGetEntryAddAddress (DataSize); -@@ -1736,6 +1774,12 @@ S3BootScriptSaveInformation ( - UINT8 *Script; - EFI_BOOT_SCRIPT_INFORMATION ScriptInformation; - -+ // -+ // Truncation check -+ // -+ if (InformationLength > MAX_UINT8 - sizeof (EFI_BOOT_SCRIPT_INFORMATION)) { -+ return RETURN_OUT_OF_RESOURCES; -+ } - Length = (UINT8)(sizeof (EFI_BOOT_SCRIPT_INFORMATION) + InformationLength); - - Script = S3BootScriptGetEntryAddAddress (Length); -@@ -2195,6 +2239,12 @@ S3BootScriptLabelInternal ( - UINT8 *Script; - EFI_BOOT_SCRIPT_INFORMATION ScriptInformation; - -+ // -+ // Truncation check -+ // -+ if (InformationLength > MAX_UINT8 - sizeof (EFI_BOOT_SCRIPT_INFORMATION)) { -+ return RETURN_OUT_OF_RESOURCES; -+ } - Length = (UINT8)(sizeof (EFI_BOOT_SCRIPT_INFORMATION) + InformationLength); - - Script = S3BootScriptGetEntryAddAddress (Length); --- -2.25.0 - diff --git a/ovmf-bsc1163969-fix-DxeImageVerificationHandler.patch b/ovmf-bsc1163969-fix-DxeImageVerificationHandler.patch deleted file mode 100644 index 1332a47..0000000 --- a/ovmf-bsc1163969-fix-DxeImageVerificationHandler.patch +++ /dev/null @@ -1,1870 +0,0 @@ -From 3d3fb711addaa2a740c3014641707fe525d7d0e5 Mon Sep 17 00:00:00 2001 -From: Laszlo Ersek -Date: Thu, 16 Jan 2020 11:37:04 +0100 -Subject: [PATCH 01/21] SecurityPkg/DxeImageVerificationHandler: simplify - "VerifyStatus" - -In the DxeImageVerificationHandler() function, the "VerifyStatus" variable -can only contain one of two values: EFI_SUCCESS and EFI_ACCESS_DENIED. -Furthermore, the variable is only consumed with EFI_ERROR(). - -Therefore, using the EFI_STATUS type for the variable is unnecessary. -Worse, given the complex meanings of the function's return values, using -EFI_STATUS for "VerifyStatus" is actively confusing. - -Rename the variable to "IsVerified", and make it a simple BOOLEAN. - -This patch is a no-op, regarding behavior. - -Cc: Chao Zhang -Cc: Jian J Wang -Cc: Jiewen Yao -Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2129 -Signed-off-by: Laszlo Ersek -Message-Id: <20200116190705.18816-2-lersek@redhat.com> -Reviewed-by: Michael D Kinney -[lersek@redhat.com: push with Mike's R-b due to Chinese New Year - Holiday: ; msgid - ] -(cherry picked from commit 1e0f973b65c34841288c25fd441a37eec8a30ac7) ---- - .../DxeImageVerificationLib.c | 20 +++++++++---------- - 1 file changed, 10 insertions(+), 10 deletions(-) - -diff --git a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c -index a0a12b50ddd1..5afd723adae8 100644 ---- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c -+++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c -@@ -1563,7 +1563,7 @@ DxeImageVerificationHandler ( - { - EFI_STATUS Status; - EFI_IMAGE_DOS_HEADER *DosHdr; -- EFI_STATUS VerifyStatus; -+ BOOLEAN IsVerified; - EFI_SIGNATURE_LIST *SignatureList; - UINTN SignatureListSize; - EFI_SIGNATURE_DATA *Signature; -@@ -1588,7 +1588,7 @@ DxeImageVerificationHandler ( - PkcsCertData = NULL; - Action = EFI_IMAGE_EXECUTION_AUTH_UNTESTED; - Status = EFI_ACCESS_DENIED; -- VerifyStatus = EFI_ACCESS_DENIED; -+ IsVerified = FALSE; - - - // -@@ -1812,16 +1812,16 @@ DxeImageVerificationHandler ( - // - if (IsForbiddenByDbx (AuthData, AuthDataSize)) { - Action = EFI_IMAGE_EXECUTION_AUTH_SIG_FAILED; -- VerifyStatus = EFI_ACCESS_DENIED; -+ IsVerified = FALSE; - break; - } - - // - // Check the digital signature against the valid certificate in allowed database (db). - // -- if (EFI_ERROR (VerifyStatus)) { -+ if (!IsVerified) { - if (IsAllowedByDb (AuthData, AuthDataSize)) { -- VerifyStatus = EFI_SUCCESS; -+ IsVerified = TRUE; - } - } - -@@ -1831,11 +1831,11 @@ DxeImageVerificationHandler ( - if (IsSignatureFoundInDatabase (EFI_IMAGE_SECURITY_DATABASE1, mImageDigest, &mCertType, mImageDigestSize)) { - Action = EFI_IMAGE_EXECUTION_AUTH_SIG_FOUND; - DEBUG ((DEBUG_INFO, "DxeImageVerificationLib: Image is signed but %s hash of image is found in DBX.\n", mHashTypeStr)); -- VerifyStatus = EFI_ACCESS_DENIED; -+ IsVerified = FALSE; - break; -- } else if (EFI_ERROR (VerifyStatus)) { -+ } else if (!IsVerified) { - if (IsSignatureFoundInDatabase (EFI_IMAGE_SECURITY_DATABASE, mImageDigest, &mCertType, mImageDigestSize)) { -- VerifyStatus = EFI_SUCCESS; -+ IsVerified = TRUE; - } else { - DEBUG ((DEBUG_INFO, "DxeImageVerificationLib: Image is signed but signature is not allowed by DB and %s hash of image is not found in DB/DBX.\n", mHashTypeStr)); - } -@@ -1846,10 +1846,10 @@ DxeImageVerificationHandler ( - // - // The Size in Certificate Table or the attribute certificate table is corrupted. - // -- VerifyStatus = EFI_ACCESS_DENIED; -+ IsVerified = FALSE; - } - -- if (!EFI_ERROR (VerifyStatus)) { -+ if (IsVerified) { - return EFI_SUCCESS; - } else { - Status = EFI_ACCESS_DENIED; --- -2.25.0 - - -From 24062a7eae07957c37e09d5344504b8802eafe40 Mon Sep 17 00:00:00 2001 -From: Laszlo Ersek -Date: Thu, 16 Jan 2020 11:44:09 +0100 -Subject: [PATCH 02/21] SecurityPkg/DxeImageVerificationHandler: remove "else" - after return/break - -In the code structure - - if (condition) { - // - // block1 - // - return; - } else { - // - // block2 - // - } - -nesting "block2" in an "else" branch is superfluous, and harms -readability. It can be transformed to: - - if (condition) { - // - // block1 - // - return; - } - // - // block2 - // - -with identical behavior, and improved readability (less nesting). - -The same applies to "break" (instead of "return") in a loop body. - -Perform these transformations on DxeImageVerificationHandler(). - -This patch is a no-op for behavior. Use - - git show -b -W - -for reviewing it more easily. - -Cc: Chao Zhang -Cc: Jian J Wang -Cc: Jiewen Yao -Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2129 -Signed-off-by: Laszlo Ersek -Message-Id: <20200116190705.18816-3-lersek@redhat.com> -Reviewed-by: Michael D Kinney -[lersek@redhat.com: push with Mike's R-b due to Chinese New Year - Holiday: ; msgid - ] -(cherry picked from commit eccb856f013aec700234211e7371f03454ef9d52) ---- - .../DxeImageVerificationLib.c | 41 ++++++++++--------- - 1 file changed, 21 insertions(+), 20 deletions(-) - -diff --git a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c -index 5afd723adae8..8204c9c0f105 100644 ---- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c -+++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c -@@ -1621,7 +1621,8 @@ DxeImageVerificationHandler ( - // - if (Policy == ALWAYS_EXECUTE) { - return EFI_SUCCESS; -- } else if (Policy == NEVER_EXECUTE) { -+ } -+ if (Policy == NEVER_EXECUTE) { - return EFI_ACCESS_DENIED; - } - -@@ -1833,7 +1834,8 @@ DxeImageVerificationHandler ( - DEBUG ((DEBUG_INFO, "DxeImageVerificationLib: Image is signed but %s hash of image is found in DBX.\n", mHashTypeStr)); - IsVerified = FALSE; - break; -- } else if (!IsVerified) { -+ } -+ if (!IsVerified) { - if (IsSignatureFoundInDatabase (EFI_IMAGE_SECURITY_DATABASE, mImageDigest, &mCertType, mImageDigestSize)) { - IsVerified = TRUE; - } else { -@@ -1851,25 +1853,24 @@ DxeImageVerificationHandler ( - - if (IsVerified) { - return EFI_SUCCESS; -- } else { -- Status = EFI_ACCESS_DENIED; -- if (Action == EFI_IMAGE_EXECUTION_AUTH_SIG_FAILED || Action == EFI_IMAGE_EXECUTION_AUTH_SIG_FOUND) { -- // -- // Get image hash value as signature of executable. -- // -- SignatureListSize = sizeof (EFI_SIGNATURE_LIST) + sizeof (EFI_SIGNATURE_DATA) - 1 + mImageDigestSize; -- SignatureList = (EFI_SIGNATURE_LIST *) AllocateZeroPool (SignatureListSize); -- if (SignatureList == NULL) { -- Status = EFI_OUT_OF_RESOURCES; -- goto Done; -- } -- SignatureList->SignatureHeaderSize = 0; -- SignatureList->SignatureListSize = (UINT32) SignatureListSize; -- SignatureList->SignatureSize = (UINT32) (sizeof (EFI_SIGNATURE_DATA) - 1 + mImageDigestSize); -- CopyMem (&SignatureList->SignatureType, &mCertType, sizeof (EFI_GUID)); -- Signature = (EFI_SIGNATURE_DATA *) ((UINT8 *) SignatureList + sizeof (EFI_SIGNATURE_LIST)); -- CopyMem (Signature->SignatureData, mImageDigest, mImageDigestSize); -+ } -+ Status = EFI_ACCESS_DENIED; -+ if (Action == EFI_IMAGE_EXECUTION_AUTH_SIG_FAILED || Action == EFI_IMAGE_EXECUTION_AUTH_SIG_FOUND) { -+ // -+ // Get image hash value as signature of executable. -+ // -+ SignatureListSize = sizeof (EFI_SIGNATURE_LIST) + sizeof (EFI_SIGNATURE_DATA) - 1 + mImageDigestSize; -+ SignatureList = (EFI_SIGNATURE_LIST *) AllocateZeroPool (SignatureListSize); -+ if (SignatureList == NULL) { -+ Status = EFI_OUT_OF_RESOURCES; -+ goto Done; - } -+ SignatureList->SignatureHeaderSize = 0; -+ SignatureList->SignatureListSize = (UINT32) SignatureListSize; -+ SignatureList->SignatureSize = (UINT32) (sizeof (EFI_SIGNATURE_DATA) - 1 + mImageDigestSize); -+ CopyMem (&SignatureList->SignatureType, &mCertType, sizeof (EFI_GUID)); -+ Signature = (EFI_SIGNATURE_DATA *) ((UINT8 *) SignatureList + sizeof (EFI_SIGNATURE_LIST)); -+ CopyMem (Signature->SignatureData, mImageDigest, mImageDigestSize); - } - - Done: --- -2.25.0 - - -From 76d93e6a0d966fdbefbe3e1df304dc22aae47ac1 Mon Sep 17 00:00:00 2001 -From: Laszlo Ersek -Date: Thu, 16 Jan 2020 12:14:14 +0100 -Subject: [PATCH 03/21] SecurityPkg/DxeImageVerificationHandler: keep PE/COFF - info status internal - -The PeCoffLoaderGetImageInfo() function may return various error codes, -such as RETURN_INVALID_PARAMETER and RETURN_UNSUPPORTED. - -Such error values should not be assigned to our "Status" variable in the -DxeImageVerificationHandler() function, because "Status" generally stands -for the main exit value of the function. And -SECURITY2_FILE_AUTHENTICATION_HANDLER functions are expected to return one -of EFI_SUCCESS, EFI_SECURITY_VIOLATION, and EFI_ACCESS_DENIED only. - -Introduce the "PeCoffStatus" helper variable for keeping the return value -of PeCoffLoaderGetImageInfo() internal to the function. If -PeCoffLoaderGetImageInfo() fails, we'll jump to the "Done" label with -"Status" being EFI_ACCESS_DENIED, inherited from the top of the function. - -Note that this is consistent with the subsequent PE/COFF Signature check, -where we jump to the "Done" label with "Status" having been re-set to -EFI_ACCESS_DENIED. - -As a consequence, we can at once remove the - - Status = EFI_ACCESS_DENIED; - -assignment right after the "PeCoffStatus" check. - -This patch does not change the control flow in the function, it only -changes the "Status" outcome from API-incompatible error codes to -EFI_ACCESS_DENIED, under some circumstances. - -Cc: Chao Zhang -Cc: Jian J Wang -Cc: Jiewen Yao -Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2129 -Signed-off-by: Laszlo Ersek -Message-Id: <20200116190705.18816-4-lersek@redhat.com> -Reviewed-by: Michael D Kinney -[lersek@redhat.com: push with Mike's R-b due to Chinese New Year - Holiday: ; msgid - ] -(cherry picked from commit 61a9fa589a15e9005bec293f9766c78b60fbc9fc) ---- - .../DxeImageVerificationLib/DxeImageVerificationLib.c | 7 +++---- - 1 file changed, 3 insertions(+), 4 deletions(-) - -diff --git a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c -index 8204c9c0f105..e6c8a5408752 100644 ---- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c -+++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c -@@ -1580,6 +1580,7 @@ DxeImageVerificationHandler ( - EFI_IMAGE_DATA_DIRECTORY *SecDataDir; - UINT32 OffSet; - CHAR16 *NameStr; -+ RETURN_STATUS PeCoffStatus; - - SignatureList = NULL; - SignatureListSize = 0; -@@ -1669,8 +1670,8 @@ DxeImageVerificationHandler ( - // - // Get information about the image being loaded - // -- Status = PeCoffLoaderGetImageInfo (&ImageContext); -- if (EFI_ERROR (Status)) { -+ PeCoffStatus = PeCoffLoaderGetImageInfo (&ImageContext); -+ if (RETURN_ERROR (PeCoffStatus)) { - // - // The information can't be got from the invalid PeImage - // -@@ -1678,8 +1679,6 @@ DxeImageVerificationHandler ( - goto Done; - } - -- Status = EFI_ACCESS_DENIED; -- - DosHdr = (EFI_IMAGE_DOS_HEADER *) mImageBase; - if (DosHdr->e_magic == EFI_IMAGE_DOS_SIGNATURE) { - // --- -2.25.0 - - -From aab72f2a7dc9583963461d1cd75073c52a83af5c Mon Sep 17 00:00:00 2001 -From: Laszlo Ersek -Date: Thu, 16 Jan 2020 12:56:59 +0100 -Subject: [PATCH 04/21] SecurityPkg/DxeImageVerificationHandler: narrow down - PE/COFF hash status - -Inside the "for" loop that scans the signatures of the image, we call -HashPeImageByType(), and assign its return value to "Status". - -Beyond the immediate retval check, this assignment is useless (never -consumed). That's because a subsequent access to "Status" may only be one -of the following: - -- the "Status" assignment when we call HashPeImageByType() in the next - iteration of the loop, - -- the "Status = EFI_ACCESS_DENIED" assignment right after the final - "IsVerified" check. - -To make it clear that the assignment is only useful for the immediate -HashPeImageByType() retval check, introduce a specific helper variable, -called "HashStatus". - -This patch is a no-op, functionally. - -Cc: Chao Zhang -Cc: Jian J Wang -Cc: Jiewen Yao -Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2129 -Signed-off-by: Laszlo Ersek -Message-Id: <20200116190705.18816-5-lersek@redhat.com> -Reviewed-by: Michael D Kinney -[lersek@redhat.com: push with Mike's R-b due to Chinese New Year - Holiday: ; msgid - ] -(cherry picked from commit 47650a5cab608e07c31d66bdb9b4cc6e58bdf22f) ---- - .../DxeImageVerificationLib/DxeImageVerificationLib.c | 5 +++-- - 1 file changed, 3 insertions(+), 2 deletions(-) - -diff --git a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c -index e6c8a5408752..5cc82c1b3b22 100644 ---- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c -+++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c -@@ -1581,6 +1581,7 @@ DxeImageVerificationHandler ( - UINT32 OffSet; - CHAR16 *NameStr; - RETURN_STATUS PeCoffStatus; -+ EFI_STATUS HashStatus; - - SignatureList = NULL; - SignatureListSize = 0; -@@ -1802,8 +1803,8 @@ DxeImageVerificationHandler ( - continue; - } - -- Status = HashPeImageByType (AuthData, AuthDataSize); -- if (EFI_ERROR (Status)) { -+ HashStatus = HashPeImageByType (AuthData, AuthDataSize); -+ if (EFI_ERROR (HashStatus)) { - continue; - } - --- -2.25.0 - - -From 2cc08bd79fcea93c9495f17c55bf6dbb7076b657 Mon Sep 17 00:00:00 2001 -From: Laszlo Ersek -Date: Thu, 16 Jan 2020 13:07:46 +0100 -Subject: [PATCH 05/21] SecurityPkg/DxeImageVerificationHandler: fix retval on - memalloc failure - -A SECURITY2_FILE_AUTHENTICATION_HANDLER function is not expected to return -EFI_OUT_OF_RESOURCES. We should only return EFI_SUCCESS, -EFI_SECURITY_VIOLATION, or EFI_ACCESS_DENIED. - -In case we run out of memory while preparing "SignatureList" for -AddImageExeInfo(), we should simply stick with the EFI_ACCESS_DENIED value -that is already in "Status" -- from just before the "Action" condition --, -and not suppress it with EFI_OUT_OF_RESOURCES. - -This patch does not change the control flow in the function, it only -changes the "Status" outcome from API-incompatible error codes to -EFI_ACCESS_DENIED, under some circumstances. - -Cc: Chao Zhang -Cc: Jian J Wang -Cc: Jiewen Yao -Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2129 -Fixes: 570b3d1a7278df29878da87990e8366bd42d0ec5 -Signed-off-by: Laszlo Ersek -Message-Id: <20200116190705.18816-6-lersek@redhat.com> -Reviewed-by: Michael D Kinney -[lersek@redhat.com: push with Mike's R-b due to Chinese New Year - Holiday: ; msgid - ] -(cherry picked from commit f891b052c5ec13c1032fb9d340d5262ac1a7e7e1) ---- - .../Library/DxeImageVerificationLib/DxeImageVerificationLib.c | 2 -- - 1 file changed, 2 deletions(-) - -diff --git a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c -index 5cc82c1b3b22..5f09a66bc9ce 100644 ---- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c -+++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c -@@ -1541,7 +1541,6 @@ Done: - and non-NULL FileBuffer did authenticate, and the platform - policy dictates that the DXE Foundation may execute the image in - FileBuffer. -- @retval EFI_OUT_RESOURCE Fail to allocate memory. - @retval EFI_SECURITY_VIOLATION The file specified by File did not authenticate, and - the platform policy dictates that File should be placed - in the untrusted state. The image has been added to the file -@@ -1862,7 +1861,6 @@ DxeImageVerificationHandler ( - SignatureListSize = sizeof (EFI_SIGNATURE_LIST) + sizeof (EFI_SIGNATURE_DATA) - 1 + mImageDigestSize; - SignatureList = (EFI_SIGNATURE_LIST *) AllocateZeroPool (SignatureListSize); - if (SignatureList == NULL) { -- Status = EFI_OUT_OF_RESOURCES; - goto Done; - } - SignatureList->SignatureHeaderSize = 0; --- -2.25.0 - - -From a3080e1d7d73d14dbe0509ad4f3e595300af6691 Mon Sep 17 00:00:00 2001 -From: Laszlo Ersek -Date: Thu, 16 Jan 2020 13:19:26 +0100 -Subject: [PATCH 06/21] SecurityPkg/DxeImageVerificationHandler: remove - superfluous Status setting - -After the final "IsVerified" check, we set "Status" to EFI_ACCESS_DENIED. -This is superfluous, as "Status" already carries EFI_ACCESS_DENIED value -there, from the top of the function. Remove the assignment. - -Functionally, this change is a no-op. - -Cc: Chao Zhang -Cc: Jian J Wang -Cc: Jiewen Yao -Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2129 -Signed-off-by: Laszlo Ersek -Message-Id: <20200116190705.18816-7-lersek@redhat.com> -Reviewed-by: Michael D Kinney -[lersek@redhat.com: push with Mike's R-b due to Chinese New Year - Holiday: ; msgid - ] -(cherry picked from commit 12a4ef58a8b1f8610f6f7cd3ffb973f924f175fb) ---- - .../Library/DxeImageVerificationLib/DxeImageVerificationLib.c | 1 - - 1 file changed, 1 deletion(-) - -diff --git a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c -index 5f09a66bc9ce..6ccce1f35843 100644 ---- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c -+++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c -@@ -1853,7 +1853,6 @@ DxeImageVerificationHandler ( - if (IsVerified) { - return EFI_SUCCESS; - } -- Status = EFI_ACCESS_DENIED; - if (Action == EFI_IMAGE_EXECUTION_AUTH_SIG_FAILED || Action == EFI_IMAGE_EXECUTION_AUTH_SIG_FOUND) { - // - // Get image hash value as signature of executable. --- -2.25.0 - - -From 117e5b029dd7a8952168cc528a734ed7a7dc6b62 Mon Sep 17 00:00:00 2001 -From: Laszlo Ersek -Date: Thu, 16 Jan 2020 13:23:10 +0100 -Subject: [PATCH 07/21] SecurityPkg/DxeImageVerificationHandler: unnest - AddImageExeInfo() call - -Before the "Done" label at the end of DxeImageVerificationHandler(), we -now have a single access to "Status": we set "Status" to EFI_ACCESS_DENIED -at the top of the function. Therefore, the (Status != EFI_SUCCESS) -condition is always true under the "Done" label. - -Accordingly, unnest the AddImageExeInfo() call dependent on that -condition, remove the condition, and also rename the "Done" label to -"Failed". - -Functionally, this patch is a no-op. It's easier to review with: - - git show -b -W - -Cc: Chao Zhang -Cc: Jian J Wang -Cc: Jiewen Yao -Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2129 -Signed-off-by: Laszlo Ersek -Message-Id: <20200116190705.18816-8-lersek@redhat.com> -Reviewed-by: Michael D Kinney -[lersek@redhat.com: replace EFI_D_INFO w/ DEBUG_INFO for PatchCheck.py] -[lersek@redhat.com: push with Mike's R-b due to Chinese New Year - Holiday: ; msgid - ] -(cherry picked from commit c602e97446a8e818bf09182f5dc9f3fa409ece95) ---- - .../DxeImageVerificationLib.c | 34 +++++++++---------- - 1 file changed, 16 insertions(+), 18 deletions(-) - -diff --git a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c -index 6ccce1f35843..51968bd9c855 100644 ---- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c -+++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c -@@ -1676,7 +1676,7 @@ DxeImageVerificationHandler ( - // The information can't be got from the invalid PeImage - // - DEBUG ((DEBUG_INFO, "DxeImageVerificationLib: PeImage invalid. Cannot retrieve image information.\n")); -- goto Done; -+ goto Failed; - } - - DosHdr = (EFI_IMAGE_DOS_HEADER *) mImageBase; -@@ -1698,7 +1698,7 @@ DxeImageVerificationHandler ( - // It is not a valid Pe/Coff file. - // - DEBUG ((DEBUG_INFO, "DxeImageVerificationLib: Not a valid PE/COFF image.\n")); -- goto Done; -+ goto Failed; - } - - if (mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) { -@@ -1729,7 +1729,7 @@ DxeImageVerificationHandler ( - // - if (!HashPeImage (HASHALG_SHA256)) { - DEBUG ((DEBUG_INFO, "DxeImageVerificationLib: Failed to hash this image using %s.\n", mHashTypeStr)); -- goto Done; -+ goto Failed; - } - - if (IsSignatureFoundInDatabase (EFI_IMAGE_SECURITY_DATABASE1, mImageDigest, &mCertType, mImageDigestSize)) { -@@ -1737,7 +1737,7 @@ DxeImageVerificationHandler ( - // Image Hash is in forbidden database (DBX). - // - DEBUG ((DEBUG_INFO, "DxeImageVerificationLib: Image is not signed and %s hash of image is forbidden by DBX.\n", mHashTypeStr)); -- goto Done; -+ goto Failed; - } - - if (IsSignatureFoundInDatabase (EFI_IMAGE_SECURITY_DATABASE, mImageDigest, &mCertType, mImageDigestSize)) { -@@ -1751,7 +1751,7 @@ DxeImageVerificationHandler ( - // Image Hash is not found in both forbidden and allowed database. - // - DEBUG ((DEBUG_INFO, "DxeImageVerificationLib: Image is not signed and %s hash of image is not found in DB/DBX.\n", mHashTypeStr)); -- goto Done; -+ goto Failed; - } - - // -@@ -1860,7 +1860,7 @@ DxeImageVerificationHandler ( - SignatureListSize = sizeof (EFI_SIGNATURE_LIST) + sizeof (EFI_SIGNATURE_DATA) - 1 + mImageDigestSize; - SignatureList = (EFI_SIGNATURE_LIST *) AllocateZeroPool (SignatureListSize); - if (SignatureList == NULL) { -- goto Done; -+ goto Failed; - } - SignatureList->SignatureHeaderSize = 0; - SignatureList->SignatureListSize = (UINT32) SignatureListSize; -@@ -1870,19 +1870,17 @@ DxeImageVerificationHandler ( - CopyMem (Signature->SignatureData, mImageDigest, mImageDigestSize); - } - --Done: -- if (Status != EFI_SUCCESS) { -- // -- // Policy decides to defer or reject the image; add its information in image executable information table. -- // -- NameStr = ConvertDevicePathToText (File, FALSE, TRUE); -- AddImageExeInfo (Action, NameStr, File, SignatureList, SignatureListSize); -- if (NameStr != NULL) { -- DEBUG((EFI_D_INFO, "The image doesn't pass verification: %s\n", NameStr)); -- FreePool(NameStr); -- } -- Status = EFI_SECURITY_VIOLATION; -+Failed: -+ // -+ // Policy decides to defer or reject the image; add its information in image executable information table. -+ // -+ NameStr = ConvertDevicePathToText (File, FALSE, TRUE); -+ AddImageExeInfo (Action, NameStr, File, SignatureList, SignatureListSize); -+ if (NameStr != NULL) { -+ DEBUG ((DEBUG_INFO, "The image doesn't pass verification: %s\n", NameStr)); -+ FreePool(NameStr); - } -+ Status = EFI_SECURITY_VIOLATION; - - if (SignatureList != NULL) { - FreePool (SignatureList); --- -2.25.0 - - -From 86eda5226eec2c867e0777997e7dde2985fd00c0 Mon Sep 17 00:00:00 2001 -From: Laszlo Ersek -Date: Thu, 16 Jan 2020 13:34:21 +0100 -Subject: [PATCH 08/21] SecurityPkg/DxeImageVerificationHandler: eliminate - "Status" variable - -The "Status" variable is set to EFI_ACCESS_DENIED at the top of the -function. Then it is overwritten with EFI_SECURITY_VIOLATION under the -"Failed" (earlier: "Done") label. We finally return "Status". - -The above covers the complete usage of "Status" in -DxeImageVerificationHandler(). Remove the variable, and simply return -EFI_SECURITY_VIOLATION in the end. - -This patch is a no-op, regarding behavior. - -Cc: Chao Zhang -Cc: Jian J Wang -Cc: Jiewen Yao -Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2129 -Signed-off-by: Laszlo Ersek -Message-Id: <20200116190705.18816-9-lersek@redhat.com> -Reviewed-by: Michael D Kinney -[lersek@redhat.com: push with Mike's R-b due to Chinese New Year - Holiday: ; msgid - ] -(cherry picked from commit fb02f5b2cd0b2a2d413a4f4fc41e085be2ede089) ---- - .../DxeImageVerificationLib/DxeImageVerificationLib.c | 5 +---- - 1 file changed, 1 insertion(+), 4 deletions(-) - -diff --git a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c -index 51968bd9c855..b49fe87a10dd 100644 ---- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c -+++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c -@@ -1560,7 +1560,6 @@ DxeImageVerificationHandler ( - IN BOOLEAN BootPolicy - ) - { -- EFI_STATUS Status; - EFI_IMAGE_DOS_HEADER *DosHdr; - BOOLEAN IsVerified; - EFI_SIGNATURE_LIST *SignatureList; -@@ -1588,7 +1587,6 @@ DxeImageVerificationHandler ( - SecDataDir = NULL; - PkcsCertData = NULL; - Action = EFI_IMAGE_EXECUTION_AUTH_UNTESTED; -- Status = EFI_ACCESS_DENIED; - IsVerified = FALSE; - - -@@ -1880,13 +1878,12 @@ Failed: - DEBUG ((DEBUG_INFO, "The image doesn't pass verification: %s\n", NameStr)); - FreePool(NameStr); - } -- Status = EFI_SECURITY_VIOLATION; - - if (SignatureList != NULL) { - FreePool (SignatureList); - } - -- return Status; -+ return EFI_SECURITY_VIOLATION; - } - - /** --- -2.25.0 - - -From 0091dec81c57ef448e87b2bb6c5640b41699eba9 Mon Sep 17 00:00:00 2001 -From: Laszlo Ersek -Date: Thu, 16 Jan 2020 13:39:19 +0100 -Subject: [PATCH 09/21] SecurityPkg/DxeImageVerificationHandler: fix retval for - (FileBuffer==NULL) - -"FileBuffer" is a non-optional input (pointer) parameter to -DxeImageVerificationHandler(). Normally, when an edk2 function receives a -NULL argument for such a parameter, we return EFI_INVALID_PARAMETER or -RETURN_INVALID_PARAMETER. However, those don't conform to the -SECURITY2_FILE_AUTHENTICATION_HANDLER prototype. - -Return EFI_ACCESS_DENIED when "FileBuffer" is NULL; it means that no image -has been loaded. - -This patch does not change the control flow in the function, it only -changes the "Status" outcome from API-incompatible error codes to -EFI_ACCESS_DENIED, under some circumstances. - -Cc: Chao Zhang -Cc: Jian J Wang -Cc: Jiewen Yao -Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2129 -Fixes: 570b3d1a7278df29878da87990e8366bd42d0ec5 -Signed-off-by: Laszlo Ersek -Message-Id: <20200116190705.18816-10-lersek@redhat.com> -Reviewed-by: Michael D Kinney -[lersek@redhat.com: push with Mike's R-b due to Chinese New Year - Holiday: ; msgid - ] -(cherry picked from commit 6d57592740cdd0b6868baeef7929d6e6fef7a8e3) ---- - .../Library/DxeImageVerificationLib/DxeImageVerificationLib.c | 2 +- - 1 file changed, 1 insertion(+), 1 deletion(-) - -diff --git a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c -index b49fe87a10dd..c98b9e45923f 100644 ---- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c -+++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c -@@ -1655,7 +1655,7 @@ DxeImageVerificationHandler ( - // Read the Dos header. - // - if (FileBuffer == NULL) { -- return EFI_INVALID_PARAMETER; -+ return EFI_ACCESS_DENIED; - } - - mImageBase = (UINT8 *) FileBuffer; --- -2.25.0 - - -From 1fccc0ca4febaae8eb0b8abc14a42e58417d0780 Mon Sep 17 00:00:00 2001 -From: Laszlo Ersek -Date: Thu, 16 Jan 2020 14:19:58 +0100 -Subject: [PATCH 10/21] SecurityPkg/DxeImageVerificationHandler: fix imgexec - info on memalloc fail - -It makes no sense to call AddImageExeInfo() with (Signature == NULL) and -(SignatureSize > 0). AddImageExeInfo() does not crash in such a case -- it -avoids the CopyMem() call --, but it creates an invalid -EFI_IMAGE_EXECUTION_INFO record. Namely, the -"EFI_IMAGE_EXECUTION_INFO.InfoSize" field includes "SignatureSize", but -the actual signature bytes are not filled in. - -Document and ASSERT() this condition in AddImageExeInfo(). - -In DxeImageVerificationHandler(), zero out "SignatureListSize" if we set -"SignatureList" to NULL due to AllocateZeroPool() failure. - -(Another approach could be to avoid calling AddImageExeInfo() completely, -in case AllocateZeroPool() fails. Unfortunately, the UEFI v2.8 spec does -not seem to state clearly whether a signature is mandatory in -EFI_IMAGE_EXECUTION_INFO, if the "Action" field is -EFI_IMAGE_EXECUTION_AUTH_SIG_FAILED or EFI_IMAGE_EXECUTION_AUTH_SIG_FOUND. - -For now, the EFI_IMAGE_EXECUTION_INFO addition logic is not changed; we -only make sure that the record we add is not malformed.) - -Cc: Chao Zhang -Cc: Jian J Wang -Cc: Jiewen Yao -Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2129 -Signed-off-by: Laszlo Ersek -Message-Id: <20200116190705.18816-11-lersek@redhat.com> -Reviewed-by: Michael D Kinney -[lersek@redhat.com: push with Mike's R-b due to Chinese New Year - Holiday: ; msgid - ] -(cherry picked from commit 6aa31db5ebebe18b55aa5359142223a03592416f) ---- - .../Library/DxeImageVerificationLib/DxeImageVerificationLib.c | 4 +++- - 1 file changed, 3 insertions(+), 1 deletion(-) - -diff --git a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c -index c98b9e45923f..015a5b61a301 100644 ---- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c -+++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c -@@ -704,7 +704,7 @@ GetImageExeInfoTableSize ( - @param[in] Name Input a null-terminated, user-friendly name. - @param[in] DevicePath Input device path pointer. - @param[in] Signature Input signature info in EFI_SIGNATURE_LIST data structure. -- @param[in] SignatureSize Size of signature. -+ @param[in] SignatureSize Size of signature. Must be zero if Signature is NULL. - - **/ - VOID -@@ -761,6 +761,7 @@ AddImageExeInfo ( - // - // Signature size can be odd. Pad after signature to ensure next EXECUTION_INFO entry align - // -+ ASSERT (Signature != NULL || SignatureSize == 0); - NewImageExeInfoEntrySize = sizeof (EFI_IMAGE_EXECUTION_INFO) + NameStringLen + DevicePathSize + SignatureSize; - - NewImageExeInfoTable = (EFI_IMAGE_EXECUTION_INFO_TABLE *) AllocateRuntimePool (ImageExeInfoTableSize + NewImageExeInfoEntrySize); -@@ -1858,6 +1859,7 @@ DxeImageVerificationHandler ( - SignatureListSize = sizeof (EFI_SIGNATURE_LIST) + sizeof (EFI_SIGNATURE_DATA) - 1 + mImageDigestSize; - SignatureList = (EFI_SIGNATURE_LIST *) AllocateZeroPool (SignatureListSize); - if (SignatureList == NULL) { -+ SignatureListSize = 0; - goto Failed; - } - SignatureList->SignatureHeaderSize = 0; --- -2.25.0 - - -From 59e0f2bef458d4d580fb3edfe083b19697ea7a2f Mon Sep 17 00:00:00 2001 -From: Laszlo Ersek -Date: Thu, 16 Jan 2020 14:45:38 +0100 -Subject: [PATCH 11/21] SecurityPkg/DxeImageVerificationHandler: fix "defer" - vs. "deny" policies - -In DxeImageVerificationHandler(), we should return EFI_SECURITY_VIOLATION -for a rejected image only if the platform sets -DEFER_EXECUTE_ON_SECURITY_VIOLATION as the policy for the image's source. -Otherwise, EFI_ACCESS_DENIED must be returned. - -Right now, EFI_SECURITY_VIOLATION is returned for all rejected images, -which is wrong -- it causes LoadImage() to hold on to rejected images (in -untrusted state), for further platform actions. However, if a platform -already set DENY_EXECUTE_ON_SECURITY_VIOLATION, the platform will not -expect the rejected image to stick around in memory (regardless of its -untrusted state). - -Therefore, adhere to the platform policy in the return value of the -DxeImageVerificationHandler() function. - -Furthermore, according to "32.4.2 Image Execution Information Table" in -the UEFI v2.8 spec, and considering that edk2 only supports (AuditMode==0) -at the moment: - -> When AuditMode==0, if the image's signature is not found in the -> authorized database, or is found in the forbidden database, the image -> will not be started and instead, information about it will be placed in -> this table. - -we have to store an EFI_IMAGE_EXECUTION_INFO record in both the "defer" -case and the "deny" case. Thus, the AddImageExeInfo() call is not being -made conditional on (Policy == DEFER_EXECUTE_ON_SECURITY_VIOLATION); the -documentation is updated instead. - -Cc: Chao Zhang -Cc: Jian J Wang -Cc: Jiewen Yao -Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2129 -Fixes: 5db28a6753d307cdfb1cfdeb2f63739a9f959837 -Signed-off-by: Laszlo Ersek -Message-Id: <20200116190705.18816-12-lersek@redhat.com> -Reviewed-by: Michael D Kinney -[lersek@redhat.com: push with Mike's R-b due to Chinese New Year - Holiday: ; msgid - ] -(cherry picked from commit 8b0932c19f31cbf9da26d3b8d4e8d954bdbb5269) ---- - .../DxeImageVerificationLib/DxeImageVerificationLib.c | 11 ++++++++--- - 1 file changed, 8 insertions(+), 3 deletions(-) - -diff --git a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c -index 015a5b61a301..dbfbfcb4fb3a 100644 ---- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c -+++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c -@@ -1548,7 +1548,8 @@ Done: - execution table. - @retval EFI_ACCESS_DENIED The file specified by File and FileBuffer did not - authenticate, and the platform policy dictates that the DXE -- Foundation many not use File. -+ Foundation may not use File. The image has -+ been added to the file execution table. - - **/ - EFI_STATUS -@@ -1872,7 +1873,8 @@ DxeImageVerificationHandler ( - - Failed: - // -- // Policy decides to defer or reject the image; add its information in image executable information table. -+ // Policy decides to defer or reject the image; add its information in image -+ // executable information table in either case. - // - NameStr = ConvertDevicePathToText (File, FALSE, TRUE); - AddImageExeInfo (Action, NameStr, File, SignatureList, SignatureListSize); -@@ -1885,7 +1887,10 @@ Failed: - FreePool (SignatureList); - } - -- return EFI_SECURITY_VIOLATION; -+ if (Policy == DEFER_EXECUTE_ON_SECURITY_VIOLATION) { -+ return EFI_SECURITY_VIOLATION; -+ } -+ return EFI_ACCESS_DENIED; - } - - /** --- -2.25.0 - - -From 46f09201a83791336668789add280b42a7b2eafc Mon Sep 17 00:00:00 2001 -From: Jian J Wang -Date: Thu, 10 Oct 2019 11:06:53 +0800 -Subject: [PATCH 12/21] SecurityPkg/DxeImageVerificationLib: Fix memory leaks - (CVE-2019-14575) - -REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1608 - -Pointer HashCtx used in IsCertHashFoundInDatabase() is not freed inside -the while-loop, if it will run more than once. - -Cc: Jiewen Yao -Cc: Chao Zhang -Signed-off-by: Jian J Wang -Reviewed-by: Jiewen Yao -(cherry picked from commit fbb96072233b5eaecf4d229cbee47b13dcab39e1) ---- - .../Library/DxeImageVerificationLib/DxeImageVerificationLib.c | 3 +++ - 1 file changed, 3 insertions(+) - -diff --git a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c -index dbfbfcb4fb3a..74dbffa1227f 100644 ---- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c -+++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c -@@ -908,6 +908,9 @@ IsCertHashFoundInDatabase ( - goto Done; - } - -+ FreePool (HashCtx); -+ HashCtx = NULL; -+ - SiglistHeaderSize = sizeof (EFI_SIGNATURE_LIST) + DbxList->SignatureHeaderSize; - CertHash = (EFI_SIGNATURE_DATA *) ((UINT8 *) DbxList + SiglistHeaderSize); - CertHashCount = (DbxList->SignatureListSize - SiglistHeaderSize) / DbxList->SignatureSize; --- -2.25.0 - - -From ffc961111fffebffd2aa0158321c4c7e25ac4e5c Mon Sep 17 00:00:00 2001 -From: Jian J Wang -Date: Thu, 10 Oct 2019 11:14:47 +0800 -Subject: [PATCH 13/21] SecurityPkg/DxeImageVerificationLib: reject - CertStack.CertNumber==0 per DBX (CVE-2019-14575) - -In case the signers' certificate stack, retrieved from the PE/COFF image's -Authenticode blob, has zero elements (=there are zero signer certificates), -then we should consider the image forbidden by DBX, not accepted by DBX. - -Cc: Jiewen Yao -Cc: Chao Zhang -Signed-off-by: Jian J Wang -Reviewed-by: Laszlo Ersek -Reviewed-by: Jiewen Yao -(cherry picked from commit c13742b180095e5181e41dffda954581ecbd9b9c) ---- - .../Library/DxeImageVerificationLib/DxeImageVerificationLib.c | 2 +- - 1 file changed, 1 insertion(+), 1 deletion(-) - -diff --git a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c -index 74dbffa1227f..5dcd6efed534 100644 ---- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c -+++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c -@@ -1326,7 +1326,7 @@ IsForbiddenByDbx ( - // UINT8 Certn[]; - // - Pkcs7GetSigners (AuthData, AuthDataSize, &CertBuffer, &BufferLength, &TrustedCert, &TrustedCertLength); -- if ((BufferLength == 0) || (CertBuffer == NULL)) { -+ if ((BufferLength == 0) || (CertBuffer == NULL) || (*CertBuffer) == 0) { - IsForbidden = TRUE; - goto Done; - } --- -2.25.0 - - -From 43809b0deb50918a9a1184b3642b8d55fafc3157 Mon Sep 17 00:00:00 2001 -From: Jian J Wang -Date: Thu, 10 Oct 2019 11:46:16 +0800 -Subject: [PATCH 14/21] SecurityPkg/DxeImageVerificationLib: fix wrong fetch - dbx in IsAllowedByDb (CVE-2019-14575) - -REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1608 - -Normally two times of calling gRT->GetVariable() are needed to get -the data of a variable: get the variable size by passing zero variable -size, and then allocate enough memory and pass the correct variable size -and buffer. - -But in the inner loop in IsAllowedByDb(), the DbxDataSize was not -initialized to zero before calling gRT->GetVariable(). It won't cause -problem if dbx does not exist. But it will give wrong result if dbx -exists and the DbxDataSize happens to be a small enough value. In this -situation, EFI_BUFFER_TOO_SMALL will be returned. Then the result check -code followed will jump to 'Done', which is not correct because it's -actually the value expected. - - if (Status == EFI_BUFFER_TOO_SMALL) { - goto Done; - } - -Cc: Jiewen Yao -Cc: Chao Zhang -Signed-off-by: Jian J Wang -Reviewed-by: Jiewen Yao -(cherry picked from commit 9e569700901857d0ba418ebdd30b8086b908688c) ---- - .../Library/DxeImageVerificationLib/DxeImageVerificationLib.c | 3 ++- - 1 file changed, 2 insertions(+), 1 deletion(-) - -diff --git a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c -index 5dcd6efed534..1efb2f96cdcc 100644 ---- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c -+++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c -@@ -1456,8 +1456,9 @@ IsAllowedByDb ( - // - // Here We still need to check if this RootCert's Hash is revoked - // -+ DbxDataSize = 0; - Status = gRT->GetVariable (EFI_IMAGE_SECURITY_DATABASE1, &gEfiImageSecurityDatabaseGuid, NULL, &DbxDataSize, NULL); -- if (Status == EFI_BUFFER_TOO_SMALL) { -+ if (Status != EFI_BUFFER_TOO_SMALL) { - goto Done; - } - DbxData = (UINT8 *) AllocateZeroPool (DbxDataSize); --- -2.25.0 - - -From d9c5a03a6d08dcc48aa0c6a1408820a4ba68d570 Mon Sep 17 00:00:00 2001 -From: Jian J Wang -Date: Thu, 10 Oct 2019 14:28:36 +0800 -Subject: [PATCH 15/21] SecurityPkg/DxeImageVerificationLib: avoid bypass in - fetching dbx (CVE-2019-14575) - -REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1608 - -In timestamp check after the cert is found in db, the original code jumps -to 'Done' if any error happens in fetching dbx variable. At any of the -jump, VerifyStatus equals to TRUE, which means allowed-by-db. This should -not be allowed except to EFI_NOT_FOUND case (meaning dbx doesn't exist), -because it could be used to bypass timestamp check. - -This patch add code to change VerifyStatus to FALSE in the case of memory -allocation failure and dbx fetching failure to avoid potential bypass -issue. - -Cc: Jiewen Yao -Cc: Chao Zhang -Signed-off-by: Jian J Wang -Reviewed-by: Jiewen Yao -(cherry picked from commit 929d1a24d12822942fd4f9fa83582e27f92de243) ---- - .../DxeImageVerificationLib/DxeImageVerificationLib.c | 11 +++++++++++ - 1 file changed, 11 insertions(+) - -diff --git a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c -index 1efb2f96cdcc..ed5dbf26b041 100644 ---- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c -+++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c -@@ -1459,15 +1459,26 @@ IsAllowedByDb ( - DbxDataSize = 0; - Status = gRT->GetVariable (EFI_IMAGE_SECURITY_DATABASE1, &gEfiImageSecurityDatabaseGuid, NULL, &DbxDataSize, NULL); - if (Status != EFI_BUFFER_TOO_SMALL) { -+ if (Status != EFI_NOT_FOUND) { -+ VerifyStatus = FALSE; -+ } - goto Done; - } - DbxData = (UINT8 *) AllocateZeroPool (DbxDataSize); - if (DbxData == NULL) { -+ // -+ // Force not-allowed-by-db to avoid bypass -+ // -+ VerifyStatus = FALSE; - goto Done; - } - - Status = gRT->GetVariable (EFI_IMAGE_SECURITY_DATABASE1, &gEfiImageSecurityDatabaseGuid, NULL, &DbxDataSize, (VOID *) DbxData); - if (EFI_ERROR (Status)) { -+ // -+ // Force not-allowed-by-db to avoid bypass -+ // -+ VerifyStatus = FALSE; - goto Done; - } - --- -2.25.0 - - -From 0fb79f0611cf37deb0eef3ec0b555c87f8d6c2ba Mon Sep 17 00:00:00 2001 -From: Jian J Wang -Date: Thu, 10 Oct 2019 15:49:55 +0800 -Subject: [PATCH 16/21] SecurityPkg/DxeImageVerificationLib: refactor db/dbx - fetching code (CVE-2019-14575) - -REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1608 - -The dbx fetching code inside the while/for-loop causes code hard to -understand. Since there's no need to get dbx more than once, this patch -simplify the code logic by moving related code to be outside the while- -loop. db fetching code is also refined accordingly to reduce the indent -level of code. - -More comments are also added or refined to explain more details. - -Cc: Jiewen Yao -Cc: Chao Zhang -Signed-off-by: Jian J Wang -Reviewed-by: Jiewen Yao -(cherry picked from commit adc6898366298d1f64b91785e50095527f682758) ---- - .../DxeImageVerificationLib.c | 144 ++++++++++-------- - 1 file changed, 83 insertions(+), 61 deletions(-) - -diff --git a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c -index ed5dbf26b041..8739d1fa2946 100644 ---- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c -+++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c -@@ -1412,76 +1412,92 @@ IsAllowedByDb ( - RootCertSize = 0; - VerifyStatus = FALSE; - -+ // -+ // Fetch 'db' content. If 'db' doesn't exist or encounters problem to get the -+ // data, return not-allowed-by-db (FALSE). -+ // - DataSize = 0; - Status = gRT->GetVariable (EFI_IMAGE_SECURITY_DATABASE, &gEfiImageSecurityDatabaseGuid, NULL, &DataSize, NULL); -- if (Status == EFI_BUFFER_TOO_SMALL) { -- Data = (UINT8 *) AllocateZeroPool (DataSize); -- if (Data == NULL) { -- return VerifyStatus; -+ ASSERT (EFI_ERROR (Status)); -+ if (Status != EFI_BUFFER_TOO_SMALL) { -+ return VerifyStatus; -+ } -+ -+ Data = (UINT8 *) AllocateZeroPool (DataSize); -+ if (Data == NULL) { -+ return VerifyStatus; -+ } -+ -+ Status = gRT->GetVariable (EFI_IMAGE_SECURITY_DATABASE, &gEfiImageSecurityDatabaseGuid, NULL, &DataSize, (VOID *) Data); -+ if (EFI_ERROR (Status)) { -+ goto Done; -+ } -+ -+ // -+ // Fetch 'dbx' content. If 'dbx' doesn't exist, continue to check 'db'. -+ // If any other errors occured, no need to check 'db' but just return -+ // not-allowed-by-db (FALSE) to avoid bypass. -+ // -+ DbxDataSize = 0; -+ Status = gRT->GetVariable (EFI_IMAGE_SECURITY_DATABASE1, &gEfiImageSecurityDatabaseGuid, NULL, &DbxDataSize, NULL); -+ ASSERT (EFI_ERROR (Status)); -+ if (Status != EFI_BUFFER_TOO_SMALL) { -+ if (Status != EFI_NOT_FOUND) { -+ goto Done; -+ } -+ // -+ // 'dbx' does not exist. Continue to check 'db'. -+ // -+ } else { -+ // -+ // 'dbx' exists. Get its content. -+ // -+ DbxData = (UINT8 *) AllocateZeroPool (DbxDataSize); -+ if (DbxData == NULL) { -+ goto Done; - } - -- Status = gRT->GetVariable (EFI_IMAGE_SECURITY_DATABASE, &gEfiImageSecurityDatabaseGuid, NULL, &DataSize, (VOID *) Data); -+ Status = gRT->GetVariable (EFI_IMAGE_SECURITY_DATABASE1, &gEfiImageSecurityDatabaseGuid, NULL, &DbxDataSize, (VOID *) DbxData); - if (EFI_ERROR (Status)) { - goto Done; - } -+ } - -- // -- // Find X509 certificate in Signature List to verify the signature in pkcs7 signed data. -- // -- CertList = (EFI_SIGNATURE_LIST *) Data; -- while ((DataSize > 0) && (DataSize >= CertList->SignatureListSize)) { -- if (CompareGuid (&CertList->SignatureType, &gEfiCertX509Guid)) { -- CertData = (EFI_SIGNATURE_DATA *) ((UINT8 *) CertList + sizeof (EFI_SIGNATURE_LIST) + CertList->SignatureHeaderSize); -- CertCount = (CertList->SignatureListSize - sizeof (EFI_SIGNATURE_LIST) - CertList->SignatureHeaderSize) / CertList->SignatureSize; -+ // -+ // Find X509 certificate in Signature List to verify the signature in pkcs7 signed data. -+ // -+ CertList = (EFI_SIGNATURE_LIST *) Data; -+ while ((DataSize > 0) && (DataSize >= CertList->SignatureListSize)) { -+ if (CompareGuid (&CertList->SignatureType, &gEfiCertX509Guid)) { -+ CertData = (EFI_SIGNATURE_DATA *) ((UINT8 *) CertList + sizeof (EFI_SIGNATURE_LIST) + CertList->SignatureHeaderSize); -+ CertCount = (CertList->SignatureListSize - sizeof (EFI_SIGNATURE_LIST) - CertList->SignatureHeaderSize) / CertList->SignatureSize; - -- for (Index = 0; Index < CertCount; Index++) { -- // -- // Iterate each Signature Data Node within this CertList for verify. -- // -- RootCert = CertData->SignatureData; -- RootCertSize = CertList->SignatureSize - sizeof (EFI_GUID); -+ for (Index = 0; Index < CertCount; Index++) { -+ // -+ // Iterate each Signature Data Node within this CertList for verify. -+ // -+ RootCert = CertData->SignatureData; -+ RootCertSize = CertList->SignatureSize - sizeof (EFI_GUID); - -+ // -+ // Call AuthenticodeVerify library to Verify Authenticode struct. -+ // -+ VerifyStatus = AuthenticodeVerify ( -+ AuthData, -+ AuthDataSize, -+ RootCert, -+ RootCertSize, -+ mImageDigest, -+ mImageDigestSize -+ ); -+ if (VerifyStatus) { - // -- // Call AuthenticodeVerify library to Verify Authenticode struct. -+ // The image is signed and its signature is found in 'db'. - // -- VerifyStatus = AuthenticodeVerify ( -- AuthData, -- AuthDataSize, -- RootCert, -- RootCertSize, -- mImageDigest, -- mImageDigestSize -- ); -- if (VerifyStatus) { -+ if (DbxData != NULL) { - // - // Here We still need to check if this RootCert's Hash is revoked - // -- DbxDataSize = 0; -- Status = gRT->GetVariable (EFI_IMAGE_SECURITY_DATABASE1, &gEfiImageSecurityDatabaseGuid, NULL, &DbxDataSize, NULL); -- if (Status != EFI_BUFFER_TOO_SMALL) { -- if (Status != EFI_NOT_FOUND) { -- VerifyStatus = FALSE; -- } -- goto Done; -- } -- DbxData = (UINT8 *) AllocateZeroPool (DbxDataSize); -- if (DbxData == NULL) { -- // -- // Force not-allowed-by-db to avoid bypass -- // -- VerifyStatus = FALSE; -- goto Done; -- } -- -- Status = gRT->GetVariable (EFI_IMAGE_SECURITY_DATABASE1, &gEfiImageSecurityDatabaseGuid, NULL, &DbxDataSize, (VOID *) DbxData); -- if (EFI_ERROR (Status)) { -- // -- // Force not-allowed-by-db to avoid bypass -- // -- VerifyStatus = FALSE; -- goto Done; -- } -- - if (IsCertHashFoundInDatabase (RootCert, RootCertSize, (EFI_SIGNATURE_LIST *)DbxData, DbxDataSize, &RevocationTime)) { - // - // Check the timestamp signature and signing time to determine if the RootCert can be trusted. -@@ -1491,17 +1507,23 @@ IsAllowedByDb ( - DEBUG ((DEBUG_INFO, "DxeImageVerificationLib: Image is signed and signature is accepted by DB, but its root cert failed the timestamp check.\n")); - } - } -- -- goto Done; - } - -- CertData = (EFI_SIGNATURE_DATA *) ((UINT8 *) CertData + CertList->SignatureSize); -+ // -+ // There's no 'dbx' to check revocation time against (must-be pass), -+ // or, there's revocation time found in 'dbx' and checked againt 'dbt' -+ // (maybe pass or fail, depending on timestamp compare result). Either -+ // way the verification job has been completed at this point. -+ // -+ goto Done; - } -+ -+ CertData = (EFI_SIGNATURE_DATA *) ((UINT8 *) CertData + CertList->SignatureSize); - } -- -- DataSize -= CertList->SignatureListSize; -- CertList = (EFI_SIGNATURE_LIST *) ((UINT8 *) CertList + CertList->SignatureListSize); - } -+ -+ DataSize -= CertList->SignatureListSize; -+ CertList = (EFI_SIGNATURE_LIST *) ((UINT8 *) CertList + CertList->SignatureListSize); - } - - Done: --- -2.25.0 - - -From fad31a3ab1bb12812f00d599d768b3055935a84d Mon Sep 17 00:00:00 2001 -From: Jian J Wang -Date: Mon, 16 Sep 2019 16:52:58 +0800 -Subject: [PATCH 17/21] SecurityPkg/DxeImageVerificationLib: Differentiate - error/search result (1) (CVE-2019-14575) - -REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1608 - -To avoid false-negative issue in check hash against dbx, both error -condition (as return value) and check result (as out parameter) of -IsCertHashFoundInDatabase() are added. So the caller of this function -will know exactly if a failure is caused by a black list hit or -other error happening, and enforce a more secure operation to prevent -secure boot from being bypassed. For a white list check (db), there's -no such necessity. - -Cc: Jiewen Yao -Cc: Chao Zhang -Signed-off-by: Jian J Wang -Signed-off-by: Laszlo Ersek -Reviewed-by: Jiewen Yao -(cherry picked from commit a83dbf008cc73406cbdc0d5ac3164cc19fff6683) ---- - .../DxeImageVerificationLib.c | 64 ++++++++++++------- - 1 file changed, 42 insertions(+), 22 deletions(-) - -diff --git a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c -index 8739d1fa2946..85261ba7f2ae 100644 ---- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c -+++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c -@@ -822,22 +822,23 @@ AddImageExeInfo ( - @param[in] SignatureList Pointer to the Signature List in forbidden database. - @param[in] SignatureListSize Size of Signature List. - @param[out] RevocationTime Return the time that the certificate was revoked. -+ @param[out] IsFound Search result. Only valid if EFI_SUCCESS returned. - -- @return TRUE The certificate hash is found in the forbidden database. -- @return FALSE The certificate hash is not found in the forbidden database. -+ @retval EFI_SUCCESS Finished the search without any error. -+ @retval Others Error occurred in the search of database. - - **/ --BOOLEAN -+EFI_STATUS - IsCertHashFoundInDatabase ( - IN UINT8 *Certificate, - IN UINTN CertSize, - IN EFI_SIGNATURE_LIST *SignatureList, - IN UINTN SignatureListSize, -- OUT EFI_TIME *RevocationTime -+ OUT EFI_TIME *RevocationTime, -+ OUT BOOLEAN *IsFound - ) - { -- BOOLEAN IsFound; -- BOOLEAN Status; -+ EFI_STATUS Status; - EFI_SIGNATURE_LIST *DbxList; - UINTN DbxSize; - EFI_SIGNATURE_DATA *CertHash; -@@ -851,21 +852,22 @@ IsCertHashFoundInDatabase ( - UINT8 *TBSCert; - UINTN TBSCertSize; - -- IsFound = FALSE; -+ Status = EFI_ABORTED; -+ *IsFound = FALSE; - DbxList = SignatureList; - DbxSize = SignatureListSize; - HashCtx = NULL; - HashAlg = HASHALG_MAX; - - if ((RevocationTime == NULL) || (DbxList == NULL)) { -- return FALSE; -+ return EFI_INVALID_PARAMETER; - } - - // - // Retrieve the TBSCertificate from the X.509 Certificate. - // - if (!X509GetTBSCert (Certificate, CertSize, &TBSCert, &TBSCertSize)) { -- return FALSE; -+ return Status; - } - - while ((DbxSize > 0) && (SignatureListSize >= DbxList->SignatureListSize)) { -@@ -895,16 +897,13 @@ IsCertHashFoundInDatabase ( - if (HashCtx == NULL) { - goto Done; - } -- Status = mHash[HashAlg].HashInit (HashCtx); -- if (!Status) { -+ if (!mHash[HashAlg].HashInit (HashCtx)) { - goto Done; - } -- Status = mHash[HashAlg].HashUpdate (HashCtx, TBSCert, TBSCertSize); -- if (!Status) { -+ if (!mHash[HashAlg].HashUpdate (HashCtx, TBSCert, TBSCertSize)) { - goto Done; - } -- Status = mHash[HashAlg].HashFinal (HashCtx, CertDigest); -- if (!Status) { -+ if (!mHash[HashAlg].HashFinal (HashCtx, CertDigest)) { - goto Done; - } - -@@ -923,7 +922,8 @@ IsCertHashFoundInDatabase ( - // - // Hash of Certificate is found in forbidden database. - // -- IsFound = TRUE; -+ Status = EFI_SUCCESS; -+ *IsFound = TRUE; - - // - // Return the revocation time. -@@ -938,12 +938,14 @@ IsCertHashFoundInDatabase ( - DbxList = (EFI_SIGNATURE_LIST *) ((UINT8 *) DbxList + DbxList->SignatureListSize); - } - -+ Status = EFI_SUCCESS; -+ - Done: - if (HashCtx != NULL) { - FreePool (HashCtx); - } - -- return IsFound; -+ return Status; - } - - /** -@@ -1216,6 +1218,7 @@ IsForbiddenByDbx ( - { - EFI_STATUS Status; - BOOLEAN IsForbidden; -+ BOOLEAN IsFound; - UINT8 *Data; - UINTN DataSize; - EFI_SIGNATURE_LIST *CertList; -@@ -1344,20 +1347,29 @@ IsForbiddenByDbx ( - // - CertPtr = CertPtr + sizeof (UINT32) + CertSize; - -- if (IsCertHashFoundInDatabase (Cert, CertSize, (EFI_SIGNATURE_LIST *)Data, DataSize, &RevocationTime)) { -+ Status = IsCertHashFoundInDatabase (Cert, CertSize, (EFI_SIGNATURE_LIST *)Data, DataSize, &RevocationTime, &IsFound); -+ if (EFI_ERROR (Status)) { - // -- // Check the timestamp signature and signing time to determine if the image can be trusted. -+ // Error in searching dbx. Consider it as 'found'. RevocationTime might -+ // not be valid in such situation. - // - IsForbidden = TRUE; -+ } else if (IsFound) { -+ // -+ // Found Cert in dbx successfully. Check the timestamp signature and -+ // signing time to determine if the image can be trusted. -+ // - if (PassTimestampCheck (AuthData, AuthDataSize, &RevocationTime)) { - IsForbidden = FALSE; - // - // Pass DBT check. Continue to check other certs in image signer's cert list against DBX, DBT - // - continue; -+ } else { -+ IsForbidden = TRUE; -+ DEBUG ((DEBUG_INFO, "DxeImageVerificationLib: Image is signed but signature failed the timestamp check.\n")); -+ goto Done; - } -- DEBUG ((DEBUG_INFO, "DxeImageVerificationLib: Image is signed but signature failed the timestamp check.\n")); -- goto Done; - } - - } -@@ -1392,6 +1404,7 @@ IsAllowedByDb ( - { - EFI_STATUS Status; - BOOLEAN VerifyStatus; -+ BOOLEAN IsFound; - EFI_SIGNATURE_LIST *CertList; - EFI_SIGNATURE_DATA *CertData; - UINTN DataSize; -@@ -1498,7 +1511,14 @@ IsAllowedByDb ( - // - // Here We still need to check if this RootCert's Hash is revoked - // -- if (IsCertHashFoundInDatabase (RootCert, RootCertSize, (EFI_SIGNATURE_LIST *)DbxData, DbxDataSize, &RevocationTime)) { -+ Status = IsCertHashFoundInDatabase (RootCert, RootCertSize, (EFI_SIGNATURE_LIST *)DbxData, DbxDataSize, &RevocationTime, &IsFound); -+ if (EFI_ERROR (Status)) { -+ // -+ // Error in searching dbx. Consider it as 'found'. RevocationTime might -+ // not be valid in such situation. -+ // -+ VerifyStatus = FALSE; -+ } else if (IsFound) { - // - // Check the timestamp signature and signing time to determine if the RootCert can be trusted. - // --- -2.25.0 - - -From afc84bb8b23aec482bed92d3e1b4eb8706eb74f6 Mon Sep 17 00:00:00 2001 -From: Jian J Wang -Date: Tue, 17 Sep 2019 11:04:33 +0800 -Subject: [PATCH 18/21] SecurityPkg/DxeImageVerificationLib: tighten default - result (CVE-2019-14575) - -REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1608 - -All intermediate results inside this function will be checked and -returned immediately upon any failure or error, like out-of-resource, -hash calculation error or certificate retrieval failure. - -Cc: Jiewen Yao -Cc: Chao Zhang -Signed-off-by: Jian J Wang -Signed-off-by: Laszlo Ersek -Reviewed-by: Jiewen Yao -(cherry picked from commit 5cd8be6079ea7e5638903b2f3da0f4c10ec7f1da) ---- - .../DxeImageVerificationLib/DxeImageVerificationLib.c | 11 ++++++++++- - 1 file changed, 10 insertions(+), 1 deletion(-) - -diff --git a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c -index 85261ba7f2ae..470a0d20efca 100644 ---- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c -+++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c -@@ -1240,7 +1240,7 @@ IsForbiddenByDbx ( - // - // Variable Initialization - // -- IsForbidden = FALSE; -+ IsForbidden = TRUE; - Data = NULL; - CertList = NULL; - CertData = NULL; -@@ -1257,7 +1257,14 @@ IsForbiddenByDbx ( - // - DataSize = 0; - Status = gRT->GetVariable (EFI_IMAGE_SECURITY_DATABASE1, &gEfiImageSecurityDatabaseGuid, NULL, &DataSize, NULL); -+ ASSERT (EFI_ERROR (Status)); - if (Status != EFI_BUFFER_TOO_SMALL) { -+ if (Status == EFI_NOT_FOUND) { -+ // -+ // Evidently not in dbx if the database doesn't exist. -+ // -+ IsForbidden = FALSE; -+ } - return IsForbidden; - } - Data = (UINT8 *) AllocateZeroPool (DataSize); -@@ -1374,6 +1381,8 @@ IsForbiddenByDbx ( - - } - -+ IsForbidden = FALSE; -+ - Done: - if (Data != NULL) { - FreePool (Data); --- -2.25.0 - - -From 6f0cb956c5763479fc6d553da37033d1c89edf09 Mon Sep 17 00:00:00 2001 -From: Laszlo Ersek -Date: Wed, 25 Sep 2019 13:41:57 +0200 -Subject: [PATCH 19/21] SecurityPkg/DxeImageVerificationLib: plug Data leak in - IsForbiddenByDbx() (CVE-2019-14575) - -REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1608 - -If the second GetVariable() call for "dbx" fails, in IsForbiddenByDbx(), -we have to free Data. Jump to "Done" for that. - -Cc: Jiewen Yao -Cc: Chao Zhang -Signed-off-by: Laszlo Ersek -Reviewed-by: Jiewen Yao -(cherry picked from commit cb30c8f25162e6d8142c6b098f14c1e4e7f125ce) ---- - .../Library/DxeImageVerificationLib/DxeImageVerificationLib.c | 2 +- - 1 file changed, 1 insertion(+), 1 deletion(-) - -diff --git a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c -index 470a0d20efca..f20640af68b6 100644 ---- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c -+++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c -@@ -1274,7 +1274,7 @@ IsForbiddenByDbx ( - - Status = gRT->GetVariable (EFI_IMAGE_SECURITY_DATABASE1, &gEfiImageSecurityDatabaseGuid, NULL, &DataSize, (VOID *) Data); - if (EFI_ERROR (Status)) { -- return IsForbidden; -+ goto Done; - } - - // --- -2.25.0 - - -From 3ed61eef265893a59cf0d3d6dc31c92f106ecd1a Mon Sep 17 00:00:00 2001 -From: Jian J Wang -Date: Thu, 10 Oct 2019 15:02:17 +0800 -Subject: [PATCH 20/21] SecurityPkg/DxeImageVerificationLib: Differentiate - error/search result (2) (CVE-2019-14575) - -REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1608 - -To avoid false-negative issue in check hash against dbx, both error -condition (as return value) and check result (as out parameter) of -IsSignatureFoundInDatabase() are added. So the caller of this function -will know exactly if a failure is caused by a black list hit or -other error happening, and enforce a more secure operation to prevent -secure boot from being bypassed. For a white list check (db), there's -no such necessity. - -All intermediate results inside this function will be checked and -returned immediately upon any failure or error, like out-of-resource, -hash calculation error or certificate retrieval failure. - -Cc: Jiewen Yao -Cc: Chao Zhang -Signed-off-by: Jian J Wang -Reviewed-by: Laszlo Ersek -Reviewed-by: Jiewen Yao -(cherry picked from commit b1c11470598416c89c67b75c991fd0773bcbab9d) ---- - .../DxeImageVerificationLib.c | 77 ++++++++++++++----- - 1 file changed, 58 insertions(+), 19 deletions(-) - -diff --git a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c -index f20640af68b6..0e1587bc3c3c 100644 ---- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c -+++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c -@@ -955,17 +955,19 @@ Done: - @param[in] Signature Pointer to signature that is searched for. - @param[in] CertType Pointer to hash algorithm. - @param[in] SignatureSize Size of Signature. -+ @param[out] IsFound Search result. Only valid if EFI_SUCCESS returned - -- @return TRUE Found the signature in the variable database. -- @return FALSE Not found the signature in the variable database. -+ @retval EFI_SUCCESS Finished the search without any error. -+ @retval Others Error occurred in the search of database. - - **/ --BOOLEAN -+EFI_STATUS - IsSignatureFoundInDatabase ( -- IN CHAR16 *VariableName, -- IN UINT8 *Signature, -- IN EFI_GUID *CertType, -- IN UINTN SignatureSize -+ IN CHAR16 *VariableName, -+ IN UINT8 *Signature, -+ IN EFI_GUID *CertType, -+ IN UINTN SignatureSize, -+ OUT BOOLEAN *IsFound - ) - { - EFI_STATUS Status; -@@ -975,22 +977,28 @@ IsSignatureFoundInDatabase ( - UINT8 *Data; - UINTN Index; - UINTN CertCount; -- BOOLEAN IsFound; - - // - // Read signature database variable. - // -- IsFound = FALSE; -+ *IsFound = FALSE; - Data = NULL; - DataSize = 0; - Status = gRT->GetVariable (VariableName, &gEfiImageSecurityDatabaseGuid, NULL, &DataSize, NULL); - if (Status != EFI_BUFFER_TOO_SMALL) { -- return FALSE; -+ if (Status == EFI_NOT_FOUND) { -+ // -+ // No database, no need to search. -+ // -+ Status = EFI_SUCCESS; -+ } -+ -+ return Status; - } - - Data = (UINT8 *) AllocateZeroPool (DataSize); - if (Data == NULL) { -- return FALSE; -+ return EFI_OUT_OF_RESOURCES; - } - - Status = gRT->GetVariable (VariableName, &gEfiImageSecurityDatabaseGuid, NULL, &DataSize, Data); -@@ -1010,7 +1018,7 @@ IsSignatureFoundInDatabase ( - // - // Find the signature in database. - // -- IsFound = TRUE; -+ *IsFound = TRUE; - // - // Entries in UEFI_IMAGE_SECURITY_DATABASE that are used to validate image should be measured - // -@@ -1023,7 +1031,7 @@ IsSignatureFoundInDatabase ( - Cert = (EFI_SIGNATURE_DATA *) ((UINT8 *) Cert + CertList->SignatureSize); - } - -- if (IsFound) { -+ if (*IsFound) { - break; - } - } -@@ -1037,7 +1045,7 @@ Done: - FreePool (Data); - } - -- return IsFound; -+ return Status; - } - - /** -@@ -1648,6 +1656,8 @@ DxeImageVerificationHandler ( - CHAR16 *NameStr; - RETURN_STATUS PeCoffStatus; - EFI_STATUS HashStatus; -+ EFI_STATUS DbStatus; -+ BOOLEAN IsFound; - - SignatureList = NULL; - SignatureListSize = 0; -@@ -1656,7 +1666,7 @@ DxeImageVerificationHandler ( - PkcsCertData = NULL; - Action = EFI_IMAGE_EXECUTION_AUTH_UNTESTED; - IsVerified = FALSE; -- -+ IsFound = FALSE; - - // - // Check the image type and get policy setting. -@@ -1798,7 +1808,14 @@ DxeImageVerificationHandler ( - goto Failed; - } - -- if (IsSignatureFoundInDatabase (EFI_IMAGE_SECURITY_DATABASE1, mImageDigest, &mCertType, mImageDigestSize)) { -+ DbStatus = IsSignatureFoundInDatabase ( -+ EFI_IMAGE_SECURITY_DATABASE1, -+ mImageDigest, -+ &mCertType, -+ mImageDigestSize, -+ &IsFound -+ ); -+ if (EFI_ERROR (DbStatus) || IsFound) { - // - // Image Hash is in forbidden database (DBX). - // -@@ -1806,7 +1823,14 @@ DxeImageVerificationHandler ( - goto Failed; - } - -- if (IsSignatureFoundInDatabase (EFI_IMAGE_SECURITY_DATABASE, mImageDigest, &mCertType, mImageDigestSize)) { -+ DbStatus = IsSignatureFoundInDatabase ( -+ EFI_IMAGE_SECURITY_DATABASE, -+ mImageDigest, -+ &mCertType, -+ mImageDigestSize, -+ &IsFound -+ ); -+ if (!EFI_ERROR (DbStatus) && IsFound) { - // - // Image Hash is in allowed database (DB). - // -@@ -1894,14 +1918,29 @@ DxeImageVerificationHandler ( - // - // Check the image's hash value. - // -- if (IsSignatureFoundInDatabase (EFI_IMAGE_SECURITY_DATABASE1, mImageDigest, &mCertType, mImageDigestSize)) { -+ DbStatus = IsSignatureFoundInDatabase ( -+ EFI_IMAGE_SECURITY_DATABASE1, -+ mImageDigest, -+ &mCertType, -+ mImageDigestSize, -+ &IsFound -+ ); -+ if (EFI_ERROR (DbStatus) || IsFound) { - Action = EFI_IMAGE_EXECUTION_AUTH_SIG_FOUND; - DEBUG ((DEBUG_INFO, "DxeImageVerificationLib: Image is signed but %s hash of image is found in DBX.\n", mHashTypeStr)); - IsVerified = FALSE; - break; - } -+ - if (!IsVerified) { -- if (IsSignatureFoundInDatabase (EFI_IMAGE_SECURITY_DATABASE, mImageDigest, &mCertType, mImageDigestSize)) { -+ DbStatus = IsSignatureFoundInDatabase ( -+ EFI_IMAGE_SECURITY_DATABASE, -+ mImageDigest, -+ &mCertType, -+ mImageDigestSize, -+ &IsFound -+ ); -+ if (!EFI_ERROR (DbStatus) && IsFound) { - IsVerified = TRUE; - } else { - DEBUG ((DEBUG_INFO, "DxeImageVerificationLib: Image is signed but signature is not allowed by DB and %s hash of image is not found in DB/DBX.\n", mHashTypeStr)); --- -2.25.0 - - -From 3391fa5e0db3b438bfb9587912faed75ac7c3a94 Mon Sep 17 00:00:00 2001 -From: Jian J Wang -Date: Fri, 14 Feb 2020 13:50:32 +0800 -Subject: [PATCH 21/21] SecurityPkg/DxeImageVerificationLib: change - IsCertHashFoundInDatabase name (CVE-2019-14575) - -IsCertHashFoundInDatabase() is actually used only for searching dbx, -according to the function logic, its comments and its use cases. Changing -it to IsCertHashFoundInDbx to avoid confusion. - -REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1608 -Cc: Jiewen Yao -Cc: Chao Zhang -Signed-off-by: Jian J Wang -Reviewed-by: Jiewen Yao -(cherry picked from commit c230c002accc4281ccc57bba7153a9b2d9b9ccd3) ---- - .../DxeImageVerificationLib/DxeImageVerificationLib.c | 6 +++--- - 1 file changed, 3 insertions(+), 3 deletions(-) - -diff --git a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c -index 0e1587bc3c3c..b7fa8ea8c55c 100644 ---- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c -+++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c -@@ -829,7 +829,7 @@ AddImageExeInfo ( - - **/ - EFI_STATUS --IsCertHashFoundInDatabase ( -+IsCertHashFoundInDbx ( - IN UINT8 *Certificate, - IN UINTN CertSize, - IN EFI_SIGNATURE_LIST *SignatureList, -@@ -1362,7 +1362,7 @@ IsForbiddenByDbx ( - // - CertPtr = CertPtr + sizeof (UINT32) + CertSize; - -- Status = IsCertHashFoundInDatabase (Cert, CertSize, (EFI_SIGNATURE_LIST *)Data, DataSize, &RevocationTime, &IsFound); -+ Status = IsCertHashFoundInDbx (Cert, CertSize, (EFI_SIGNATURE_LIST *)Data, DataSize, &RevocationTime, &IsFound); - if (EFI_ERROR (Status)) { - // - // Error in searching dbx. Consider it as 'found'. RevocationTime might -@@ -1528,7 +1528,7 @@ IsAllowedByDb ( - // - // Here We still need to check if this RootCert's Hash is revoked - // -- Status = IsCertHashFoundInDatabase (RootCert, RootCertSize, (EFI_SIGNATURE_LIST *)DbxData, DbxDataSize, &RevocationTime, &IsFound); -+ Status = IsCertHashFoundInDbx (RootCert, RootCertSize, (EFI_SIGNATURE_LIST *)DbxData, DbxDataSize, &RevocationTime, &IsFound); - if (EFI_ERROR (Status)) { - // - // Error in searching dbx. Consider it as 'found'. RevocationTime might --- -2.25.0 - diff --git a/ovmf-gdb-symbols.patch b/ovmf-gdb-symbols.patch index b491cba..abc6bd4 100644 --- a/ovmf-gdb-symbols.patch +++ b/ovmf-gdb-symbols.patch @@ -1,4 +1,4 @@ -From 129c582687484ac3f6aa2d5eeb6e517c053337eb Mon Sep 17 00:00:00 2001 +From 7eef4b6160dc5503acdf3b6a5b932085fe67abe6 Mon Sep 17 00:00:00 2001 From: Gary Ching-Pang Lin Date: Tue, 24 Jun 2014 11:57:32 +0800 Subject: [PATCH 1/3] Add DebugPkg @@ -548,10 +548,10 @@ index 000000000000..dac215c53893 + + -- -2.23.0 +2.25.1 -From 84c13bdbdc050c12c55de76ff62ed3f1b89c8f63 Mon Sep 17 00:00:00 2001 +From 91908717fae43b7fa1d4b2e353258b2c93677216 Mon Sep 17 00:00:00 2001 From: Gary Ching-Pang Lin Date: Tue, 24 Jun 2014 11:59:02 +0800 Subject: [PATCH 2/3] Compile DebugPkg and load .debug files @@ -582,20 +582,20 @@ index dac215c53893..3db87a4de430 100644 (sym_name, long (base))) diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc -index ba7a75884490..c35371447a75 100644 +index f6c1d8d228c6..e886e2468f28 100644 --- a/OvmfPkg/OvmfPkgX64.dsc +++ b/OvmfPkg/OvmfPkgX64.dsc -@@ -923,3 +923,5 @@ [Components] - NULL|SecurityPkg/Library/HashInstanceLibSm3/HashInstanceLibSm3.inf - } +@@ -940,3 +940,5 @@ [Components] + SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf + !endif !endif + + DebugPkg/GdbSyms/GdbSyms.inf -- -2.23.0 +2.25.1 -From dcbdaa6cccce44133f8cb6a1a78a0cebd10ac172 Mon Sep 17 00:00:00 2001 +From 768ec2226be984a63afa9329f183ab74a6671a4b Mon Sep 17 00:00:00 2001 From: Gary Lin Date: Mon, 16 Oct 2017 16:32:27 +0800 Subject: [PATCH 3/3] DebugPkg: Print the local variables @@ -628,5 +628,5 @@ index 2551dfab90b6..0d591e5a8f22 100644 } -- -2.23.0 +2.25.1 diff --git a/ovmf.changes b/ovmf.changes index 8e852ca..d67db42 100644 --- a/ovmf.changes +++ b/ovmf.changes @@ -1,3 +1,224 @@ +------------------------------------------------------------------- +Fri Mar 6 03:11:48 UTC 2020 - Gary Ching-Pang Lin + +- Update to edk2-stable202002 + + UefiCpuPkg/MpInitLib: Skip reading PlatformId on AMD processors. + + BaseTools: Remove invalid leading space before !INCLUDE in Makefile + + OvmfPkg/QemuVideoDxe: unbreak "secondary-vga" and "bochs-display" support + + NetworkPkg/ArpDxe: Recycle invalid ARP packets (CVE-2019-14559) + + ShellPkg: acpiview: Prevent infinite loop if structure length is 0 + + CryptoPkg/BaseHashApiLib: Change PcdHashApiLibPolicy type to FixedAtBuild + + CryptoPkg/BaseHashApiLib: Align BaseHashApiLib with TPM 2.0 Implementation + + MdeModulePkg: Make retval in UninstallMultipleProtocol follow Spec + + SecurityPkg/DxeImageVerificationLib: change IsCertHashFoundInDatabase + name (CVE-2019-14575) + + SecurityPkg/DxeImageVerificationLib: Differentiate error/search + result (2) (CVE-2019-14575) + + SecurityPkg/DxeImageVerificationLib: plug Data leak in + IsForbiddenByDbx() (CVE-2019-14575) + + SecurityPkg/DxeImageVerificationLib: tighten default result + (CVE-2019-14575) + + SecurityPkg/DxeImageVerificationLib: Differentiate error/search + result (1) (CVE-2019-14575) + + SecurityPkg/DxeImageVerificationLib: refactor db/dbx fetching + code (CVE-2019-14575) + + SecurityPkg/DxeImageVerificationLib: avoid bypass in fetching + dbx (CVE-2019-14575) + + SecurityPkg/DxeImageVerificationLib: fix wrong fetch dbx in + IsAllowedByDb (CVE-2019-14575) + + SecurityPkg/DxeImageVerificationLib: reject + CertStack.CertNumber==0 per DBX (CVE-2019-14575) + + SecurityPkg/DxeImageVerificationLib: Fix memory leaks + (CVE-2019-14575) + + NetworkPkg/Ip4Dxe: Check the received package length + (CVE-2019-14559). + + ShellPkg: acpiview: Validate ACPI table 'Length' field + + ShellPkg: acpiview: Remove duplicate ACPI structure size definitions + + UefiCpuPkg RegisterCpuFeaturesLib: Match data type and format specifier + + MdeModulePkg/SdMmcPciHcDxe: Fix double PciIo Unmap in TRB creation + (CVE-2019-14587) + + MdeModulePkg/DisplayEngine: Zero memory before free (CVE-2019-14558) + + MdeModulePkg/String.c: Zero memory before free (CVE-2019-14558) + + MdeModulePkg/HiiDB: Remove configuration table when it's freed + (CVE-2019-14586) + + MdePkg: Remove FIT table industry standard header file. + + UefiCpuPkg: Remove FIT based microcode shadow logic from MpInitLib. + + UefiCpuPkg/CpuFeature: Introduce First to indicate 1st unit. + + UefiCpuPkg/RegisterCpuFeaturesLib: Rename [Before|After]FeatureBitMask + + UefiCpuPkg/RegisterCpuFeaturesLib: Delete CPU_FEATURE_[BEFORE|AFTER] + + MdePkg: Add PCCT table signature definition + + BaseTools: Fixed build failure when using python38 + + BaseTools:fix Ecc tool issue for check StructPcd + + BaseTools: Remove caret in NASM_INC macro + + BaseTools: Rationalise makefile generation + + MdePkg: Add PCI Express 5.0 Header File + + MdePkg: Disable EBC for unit tests in MdePkg.dsc + + MdePkg/SmBios.h: Add two additional DWORD for smbios 3.3.0 type17 + + UefiCpuPkg/MpInitLib: Not pass microcode info between archs in CPU_MP_DATA + + Revert UefiCpuPkg/MpInitLib: Relocate microcode patch fields in CPU_MP_DATA + + ShellPkg: acpiview: Validate global pointers before use + + ShellPkg: acpiview: Validate System Locality count + + ShellPkg: acpiview: Set ItemPtr to NULL for unprocessed table fields + + ShellPkg: Document UpdateArgcArgv returns EFI_INVALID_PARAMETER + + ShellPkg: Document ParseCommandLineToArgs returns EFI_INVALID_PARAMETER + + ShellPkg/UefiShellAcpiViewCommandLib: Fix FADT Parser + + SecurityPkg: Fix incorrect return value when File is NULL + + BaseTools: Fixed a Incremental build issue + + CryptoPkg/CryptoPkg.dsc: Add build of Crypto libraries/modules + + CryptoPkg/Library: Add BaseCryptLibOnProtocolPpi instances + + CryptoPkg/Driver: Add Crypto PEIM, DXE, and SMM modules + + CryptoPkg: Add EDK II Crypto Protocols/PPIs/PCDs + + CryptoPkg/BaseCryptLib: Add X509ConstructCertificateStackV(). + + MdeModulePkg/PiDxeS3BootScriptLib: Fix potential numeric truncation + (CVE-2019-14563) + + MdeModulePkg/Capsule: Remove RT restriction in UpdateCapsule service. + + SecurityPkg/TcgPhysicalPresenceLib: Replace the ASSERT with error code + + BaseTools/PcdValueCommon: Fix 64-bit host compiler error + + BaseTools/Build: Do not use Common.lib in Structured PCD app + + MdeModulePkg/BaseSerialPortLib16550: Fix Serial Port Ready + + BaseTools: Script for converting .aml to .hex + + MdeModulePkg: Perform test only if not ignore memory test + + UefiCpuPkg/MpInitLib: Always get CPUID & PlatformID in MicrocodeDetect() + + OvmfPkg/PlatformPei: detect SMRAM at default SMBASE (for real) + + OvmfPkg: introduce PcdCsmEnable feature flag + + OvmfPkg/SmmAccess: close and lock SMRAM at default SMBASE + + OvmfPkg/SEV: don't manage the lifecycle of the SMRAM at the default SMBASE + + OvmfPkg/PlatformPei: reserve the SMRAM at the default SMBASE, if it exists + + OvmfPkg/PlatformPei: assert there's no permanent PEI RAM at default SMBASE + + OvmfPkg/PlatformPei: detect SMRAM at default SMBASE (skeleton) + + OvmfPkg/PlatformPei: factor out Q35BoardVerification() + + OvmfPkg/IndustryStandard: add MCH_DEFAULT_SMBASE* register macros + + OvmfPkg/IndustryStandard: increase vertical whitespace in Q35 macro defs + + OvmfPkg: introduce PcdQ35SmramAtDefaultSmbase + + CryptoPkg/BaseCryptLibNull: Add missing HkdfSha256ExtractAndExpand() + + BaseTools/DscBuildData: Fix PCD autogen include file conflict + + CryptoPkg/BaseHashApiLib: Implement Unified Hash Calculation API + + CryptoPkg: Add CryptoPkg Token Space GUID + + BaseTools/Conf/gitattributes: fix "--function-context" for C source code + + SecurityPkg/DxeImageVerificationHandler: fix "defer" vs. "deny" policies + + SecurityPkg/DxeImageVerificationHandler: fix imgexec info on memalloc fail + + SecurityPkg/DxeImageVerificationHandler: fix retval for (FileBuffer==NULL) + + SecurityPkg/DxeImageVerificationHandler: eliminate "Status" variable + + SecurityPkg/DxeImageVerificationHandler: unnest AddImageExeInfo() call + + SecurityPkg/DxeImageVerificationHandler: remove superfluous Status setting + + SecurityPkg/DxeImageVerificationHandler: fix retval on memalloc failure + + SecurityPkg/DxeImageVerificationHandler: narrow down PE/COFF hash status + + SecurityPkg/DxeImageVerificationHandler: keep PE/COFF info status internal + + SecurityPkg/DxeImageVerificationHandler: remove "else" after return/break + + SecurityPkg/DxeImageVerificationHandler: simplify "VerifyStatus" + + OvmfPkg/PlatformPei: rewrite MaxCpuCountInitialization() for CPU hotplug + + OvmfPkg/IndustryStandard: define macros for QEMU's CPU hotplug registers + + OvmfPkg/OvmfXen.dsc: remove PcdCpu* dynamic defaults + + CryptoPkg/BaseCryptLib: remove HmacXxxGetContextSize interface + + CryptoPkg/BaseCryptLib: replace HmacXxxInit API with HmacXxxSetKey + + BaseTools: Fixed a incremental build bug + + UefiCpuPkg/MpInitLib: Fix possible uninitialized 'InitFlag' field + + FmdDevicePkg/FmpDxe: Support Fmp Capsule Dependency. + + MdeModulePkg/CapsuleApp: Enhance CapsuleApp for Fmp Capsule Dependency + + MdePkg: Add definition for Fmp Capsule Dependency. + + MdeModulePkg/SdMmcPciHcDxe: Add retries for async commands + + MdeModulePkg/SdMmcPciHcDxe: Add retries for sync commands + + MdeModulePkg/SdMmcPciHcDxe: Refactor command error detection + + MdeModulePkg/SdMmcPciHcDxe: Fix DAT lane SW reset + + UefiCpuPkg/PiSmmCpuDxeSmm: fix 2M->4K page splitting regression for PDEs + + MdeModulePkg/Variable: Fix VarErrorFlag RT cache offset calculation + + MdePkg Base.h: Use correct style to check the defined macro + + ShellPkg: acpiview: Update SRAT parser to ACPI 6.3 + + BaseTools/Capsule: Add capsule dependency support + + MdeModulePkg/Setup: Update opcode number variable type to UINTN + + ArmPlatformPkg/PrePeiCore: enable VFP at startup + + ArmPkg/ArmSmcPsciResetSystemLib: remove EnterS3WithImmediateWake () + + NetworkPkg/HttpDxe: fix 32-bit truncation in HTTPS download + + MdeModulePkg/UefiBootManagerLib: log reserved mem allocation failure + + BaseTools/Scripts/PatchCheck: Address false error conditions + + BaseTools:Fix GenFds issue for BuildOption replace GenFdsOption + + BaseTools:Change the case rules for ECC check pointer names + + MdeModulePkg/SdMmcPciHcDxe: Fix unknown doxygen tag error + + ArmVirtPkg: remove EnterS3WithImmediateWake () from ResetSystemLib + + OvmfPkg: remove EnterS3WithImmediateWake () from ResetSystemLib + + UefiPayloadPkg: remove EnterS3WithImmediateWake () from ResetSystemLib + + PcAtChipsetPkg: remove EnterS3WithImmediateWake () from ResetSystemLib + + MdeModulePkg: remove EnterS3WithImmediateWake () from ResetSystemLib + + UefiCpuPkg: Shadow microcode patch according to FIT microcode entry. + + MdePkg: Add header file for Firmware Interface Table specification. + + UefiCpuPkg/CpuCommonFeaturesLib: SMXE bit of CR4 should set + + MdePkg BaseLib.h: Update IA32_CR4 strut to include all public fields + + MdePkg: Do not use CreateEventEx unless required + + UefiCpuPkg/PiSmmCpuDxeSmm: Add missed comments for parameter. + + OvmfPkg: use HII type PCDs for TPM2 config related variables + + OvmfPkg: reorganize TPM2 support in DSC/FDF files + + BaseTools/PatchCheck.py: Ignore CR and LF characters in subject length + + MdeModulePkg: Add EDK2 Platform Boot Manager Protocol + + CryptoPkg: Support for SHA384 & SHA512 RSA signing schemes + + UefiCpuPkg: Always load microcode patch on AP processor. + + UefiCpuPkg: Remove alignment check when calculate microcode size. + + Revert "UefiCpuPkg/PiSmmCpuDxeSmm: Fix buffer overflow issue." + + MdeModulePkg/UsbMouseAbsolutePointer: Fix endpoint selection + + MdeModulePkg/Usb/UsbMouse: Fix endpoint selection + + MdeModulePkg/Usb/EfiKey: Fix endpoint selection + + SecurityPkg/Tcg2Pei: Add TCG PFP 105 support. + + MdeModulePkg/Smbios: Add TCG PFP rev 105 support. + + MdeModulePkg/dec: add PcdTcgPfpMeasurementRevision PCD + + MdeModulePkg/Smbios: Done measure Smbios multiple times. + + SecurityPkg/Tcg2Dxe: Add Tcg2Dxe to support 800-155 event. + + SecurityPkg/Guid: Add TCG 800-155 event GUID definition. + + MdeModulePkg/SdMmcPciHcDxe: Add function to start SD clock + + MdeModulePkg/SdMmcPciHcDxe: Hook SwitchClockFreq after SD clock start + + UefiCpuPkg/PiSmmCpuDxeSmm: Pre-allocate PROCEDURE_TOKEN buffer + + UefiPayloadPkg/BootManager: Add PS2 keyboard support + + UefiCpuPkg/MpInitLib: Remove redundant microcode fields in CPU_MP_DATA + + UefiCpuPkg/MpInitLib: Relocate microcode patch fields in CPU_MP_DATA + + UefiCpuPkg/MpInitLib: Produce EDKII microcode patch HOB + + UefiCpuPkg: Add definitions for EDKII microcode patch HOB + + UefiCpuPkg/MpInitLib: Reduce the size when loading microcode patches + + UefiCpuPkg/MpInitLib: Collect processors' CPUID & Platform ID info + + BaseTools/Scripts: Add sendemail.transferEncoding to SetupGit.py + + UefiCpuPkg/PiSmmCpuDxeSmm: Fix buffer overflow issue. + + UefiCpuPkg/PiSmmCpuDxeSmm: Remove dependence between APs + + edksetup.bat stuck on unicode locale Windows + + MdePkg/Tcg: Add new definition in TCG PFP spec. + + MdePkg: Use __builtin_offset with CLANGPDB toolchain + + MdePkg PciExpress21: PCI_REG_PCIE_DEVICE_CONTROL2 struct has 17 bits + + ShellPkg/ShellProtocol: Return error code while fail parsing cmd-line + + MdePkg/Spdm: fix Nonce structure error. + + BaseTools: Resolve a issue of Incremental build + + Maintainers.txt: Update email address and role + + BaseTools:replaces the two offending quotes by ascii quotes + + BaseTools: Fix build failure when multiple build targets given + + MdePkg/Include: Add DCC and BCM2835 SPCR UART types + + ArmPkg/MmCommunicationDxe: relay architected PI events to MM context + + SecurityPkg/Tcg2Smm: Measure the table before patch. + + BaseTools: Remove redundant binary cache file + + BaseTools: Leverage compiler output to optimize binary cache + + BaseTools: enhance the CacheCopyFile method arg names + + BaseTools: store more complete output files in binary cache + + BaseTools: Enhance Basetool for incremental build + + BaseTools: Update build_rule.txt to generate dependent files. + + BaseTools: Generate dependent files for ASL and ASM files + + BaseTools: Add build option for dependency file generation + + UefiCpuPkg/PiSmmCpuDxeSmm: Avoid allocate Token every time + + BaseTools: Avoid "is" with a literal Python 3.8 warnings + + ArmPkg: Dispatch deferred images after EndOfDxe + + ShellPkg/UefiHandleParsingLib: Fix error allocate pool + + ShellPkg/edit: typo "%d Lines Wrote" + + ShellPkg: acpiview: IORT Spec Rev D updates + + ShellPkg: acpiview: Add support for parsing FACS + + MdeModulePkg: Add ARM/Aarch64 support which were missing + + MdeModulePkg: LzmaCustomDecompressLib.inf don't support EBC anymore + + BaseTools:Enhance the way to handling included dsc file + + UefiCpuPkg/UefiCpuPkg.uni: Add missing strings for PCD + + NetworkPkg/NetworkPkg.uni: Add missing strings for PCD + + MdeModulePkg/MdeModulePkg.uni: Add missing strings for PCD + + NetworkPkg: Fixes to static code analysis hits + + CryptoPkg/OpensslLib.inf: list OpenSSL local header "ms/uplink.h" + + CryptoPkg/OpensslLib: improve INF file consistency + + MdeModulePkg/VariableSmmRuntimeDxe.inf: list local header "Variable.h" +- Drop upstreamed fixes + + ovmf-bsc1163927-fix-ip4dxe-and-arpdxe.patch + + ovmf-bsc1163959-PiDxeS3BootScriptLib-fix-numeric-truncation.patch + + ovmf-bsc1163969-fix-DxeImageVerificationHandler.patch +- Refresh ovmf-gdb-symbols.patch + ------------------------------------------------------------------- Mon Feb 24 04:00:24 UTC 2020 - Gary Ching-Pang Lin diff --git a/ovmf.spec b/ovmf.spec index 0eaef6c..c66de6f 100644 --- a/ovmf.spec +++ b/ovmf.spec @@ -28,7 +28,7 @@ URL: http://sourceforge.net/apps/mediawiki/tianocore/index.php?title= Summary: Open Virtual Machine Firmware License: BSD-2-Clause-Patent Group: System/Emulators/PC -Version: 201911 +Version: 202002 Release: 0 Source0: https://github.com/tianocore/edk2/archive/edk2-stable%{version}.tar.gz Source1: https://www.openssl.org/source/openssl-%{openssl_version}.tar.gz @@ -49,9 +49,6 @@ Patch2: %{name}-gdb-symbols.patch Patch3: %{name}-pie.patch Patch4: %{name}-disable-ia32-firmware-piepic.patch Patch5: %{name}-set-fixed-enroll-time.patch -Patch6: %{name}-bsc1163959-PiDxeS3BootScriptLib-fix-numeric-truncation.patch -Patch7: %{name}-bsc1163969-fix-DxeImageVerificationHandler.patch -Patch8: %{name}-bsc1163927-fix-ip4dxe-and-arpdxe.patch Patch100: openssl-fix-syntax-error.patch BuildRoot: %{_tmppath}/%{name}-%{version}-build BuildRequires: bc @@ -175,9 +172,6 @@ rm -rf $PKG_TO_REMOVE %patch3 -p1 %patch4 -p1 %patch5 -p1 -%patch6 -p1 -%patch7 -p1 -%patch8 -p1 # add openssl pushd CryptoPkg/Library/OpensslLib/openssl