summaryrefslogtreecommitdiffstats
path: root/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
Commit message (Collapse)AuthorAgeFilesLines
* SecurityPkg/SecureBoot: Support RSA4096 and RSA3072Sheng Wei2023-09-071-30/+44
| | | | | | | | | | | | REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3413 Cc: Jiewen Yao <jiewen.yao@intel.com> Cc: Jian J Wang <jian.j.wang@intel.com> Cc: Min Xu <min.m.xu@intel.com> Cc: Zeyi Chen <zeyi.chen@intel.com> Cc: Fiona Wang <fiona.wang@intel.com> Signed-off-by: Sheng Wei <w.sheng@intel.com> Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
* SecurityPkg/DxeImageVerificationLib: Add AUTH_SIG_NOT_FOUND ActionNhi Pham2023-04-281-0/+1
| | | | | | | | | | | | | | | | | | | Add the AUTH_SIG_NOT_FOUND Action to the Image Execution Info Table when the Image is signed but signature is not allowed by DB and the hash of image is not found in DB/DBX. This is documented in the UEFI spec 2.10, table 32.5. This issue is found by the SIE SCT with the error message as follows: SecureBoot - TestImage1.bin in Image Execution Info Table with SIG_NOT_FOUND. --FAILURE B3A670AA-0FBA-48CA-9D01-0EE9700965A9 SctPkg/TestCase/UEFI/EFI/RuntimeServices/SecureBoot/BlackBoxTest/ ImageLoadingBBTest.c:1079:Status Success Signed-off-by: Nhi Pham <nhi@os.amperecomputing.com> Reviewed-by: Min Xu <min.m.xu@intel.com> Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
* SecurityPkg/DxeImageVerificationLib: Check result of GetEfiGlobalVariable2Gerd Hoffmann2023-03-211-7/+12
| | | | | | | | | | | | | | | | | | | | | | | | | Call gRT->GetVariable() directly to read the SecureBoot variable. It is one byte in size so we can easily place it on the stack instead of having GetEfiGlobalVariable2() allocate it for us, which avoids a few possible error cases. Skip secure boot checks if (and only if): (a) the SecureBoot variable is not present (EFI_NOT_FOUND) according to the return value, or (b) the SecureBoot variable was read successfully and is set to SECURE_BOOT_MODE_DISABLE. Previously the code skipped the secure boot checks on *any* gRT->GetVariable() error (GetEfiGlobalVariable2 sets the variable value to NULL in that case) and also on memory allocation failures. Fixes: CVE-2019-14560 Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=2167 Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> Suggested-by: Marvin Häuser <mhaeuser@posteo.de> Reviewed-by: Min Xu <min.m.xu@intel.com> Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
* SecurityPkg: Apply uncrustify changesMichael Kubacki2021-12-071-387/+420
| | | | | | | | | | | | REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3737 Apply uncrustify changes to .c/.h files in the SecurityPkg package Cc: Andrew Fish <afish@apple.com> Cc: Leif Lindholm <leif@nuviainc.com> Cc: Michael D Kinney <michael.d.kinney@intel.com> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com> Reviewed-by: Jian J Wang <jian.j.wang@intel.com>
* SecurityPkg: Change OPTIONAL keyword usage styleMichael D Kinney2021-12-071-1/+1
| | | | | | | | | | | | REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3760 Update all use of ', OPTIONAL' to ' OPTIONAL,' for function params. Cc: Andrew Fish <afish@apple.com> Cc: Leif Lindholm <leif@nuviainc.com> Cc: Michael Kubacki <michael.kubacki@microsoft.com> Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com> Reviewed-by: Jian J Wang <jian.j.wang@intel.com>
* SecurityPkg/DxeImageVerificationLib: Disable SHA1 base on MACROZhichao Gao2020-09-071-0/+6
| | | | | | | | | | | | | | | | | | REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2943 Disable SHA1 base on the MACRO DISABLE_SHA1_DEPRECATED_INTERFACES. SHA1 is deprecated function and the MACRO is used to remove the whole implementation of the SHA1. For the platforms that do not need SHA1 for security, the MACRO should works for DxeImageVerificationLib as well. Signed-off-by: Zhichao Gao <zhichao.gao@intel.com> Cc: Jiewen Yao <jiewen.yao@intel.com> Cc: Jian J Wang <jian.j.wang@intel.com> Cc: Min Xu <min.m.xu@intel.com> Cc: Qi Zhang <qi1.zhang@intel.com> Reviewed-by: Jiewen Yao <Jiewen.yao@intel.com> Reviewed-by: Jian J Wang <jian.j.wang@intel.com>
* SecurityPkg/DxeImageVerificationLib: catch alignment overflow (CVE-2019-14562)Laszlo Ersek2020-09-021-1/+3
| | | | | | | | | | | | | | | | | | | | | | | The DxeImageVerificationHandler() function currently checks whether "SecDataDir" has enough room for "WinCertificate->dwLength". However, for advancing "OffSet", "WinCertificate->dwLength" is aligned to the next multiple of 8. If "WinCertificate->dwLength" is large enough, the alignment will return 0, and "OffSet" will be stuck at the same value. Check whether "SecDataDir" has room left for both "WinCertificate->dwLength" and the alignment. Cc: Jian J Wang <jian.j.wang@intel.com> Cc: Jiewen Yao <jiewen.yao@intel.com> Cc: Min Xu <min.m.xu@intel.com> Cc: Wenyi Xie <xiewenyi2@huawei.com> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2215 Signed-off-by: Laszlo Ersek <lersek@redhat.com> Message-Id: <20200901091221.20948-4-lersek@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Tested-by: Wenyi Xie <xiewenyi2@huawei.com> Reviewed-by: Min M Xu <min.m.xu@intel.com> Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
* SecurityPkg/DxeImageVerificationLib: assign WinCertificate after size checkLaszlo Ersek2020-09-021-3/+5
| | | | | | | | | | | | | | | | | | | | | | | | | | Currently the (SecDataDirLeft <= sizeof (WIN_CERTIFICATE)) check only guards the de-referencing of the "WinCertificate" pointer. It does not guard the calculation of the pointer itself: WinCertificate = (WIN_CERTIFICATE *) (mImageBase + OffSet); This is wrong; if we don't know for sure that we have enough room for a WIN_CERTIFICATE, then even creating such a pointer, not just de-referencing it, may invoke undefined behavior. Move the pointer calculation after the size check. Cc: Jian J Wang <jian.j.wang@intel.com> Cc: Jiewen Yao <jiewen.yao@intel.com> Cc: Min Xu <min.m.xu@intel.com> Cc: Wenyi Xie <xiewenyi2@huawei.com> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2215 Signed-off-by: Laszlo Ersek <lersek@redhat.com> Message-Id: <20200901091221.20948-3-lersek@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Tested-by: Wenyi Xie <xiewenyi2@huawei.com> Reviewed-by: Min M Xu <min.m.xu@intel.com> Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
* SecurityPkg/DxeImageVerificationLib: extract SecDataDirEnd, SecDataDirLeftLaszlo Ersek2020-09-021-4/+8
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The following two quantities: SecDataDir->VirtualAddress + SecDataDir->Size SecDataDir->VirtualAddress + SecDataDir->Size - OffSet are used multiple times in DxeImageVerificationHandler(). Introduce helper variables for them: "SecDataDirEnd" and "SecDataDirLeft", respectively. This saves us multiple calculations and significantly simplifies the code. Note that all three summands above have type UINT32, therefore the new variables are also of type UINT32. This patch does not change behavior. (Note that the code already handles the case when the SecDataDir->VirtualAddress + SecDataDir->Size UINT32 addition overflows -- namely, in that case, the certificate loop is never entered, and the corruption check right after the loop fires.) Cc: Jian J Wang <jian.j.wang@intel.com> Cc: Jiewen Yao <jiewen.yao@intel.com> Cc: Min Xu <min.m.xu@intel.com> Cc: Wenyi Xie <xiewenyi2@huawei.com> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2215 Signed-off-by: Laszlo Ersek <lersek@redhat.com> Message-Id: <20200901091221.20948-2-lersek@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Tested-by: Wenyi Xie <xiewenyi2@huawei.com> Reviewed-by: Min M Xu <min.m.xu@intel.com> Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
* SecurityPkg: Fix spelling mistake for occurredMichael D Kinney2020-08-191-1/+1
| | | | | | | | | | | REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2361 Cc: Qi Zhang <qi1.zhang@intel.com> Cc: Rahul Kumar <rahul1.kumar@intel.com> Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com> Reviewed-by: Guomin Jiang <guomin.jiang@intel.com> Reviewed-by: Maciej Rabeda <maciej.rabeda@linux.intel.com> Reviewed-by: Jian J Wang <jian.j.wang@intel.com>
* SecurityPkg/DxeImageVerificationLib: Mark the File parameter as OPTIONALGuomin Jiang2020-04-221-1/+1
| | | | | | | | | | | | | REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2652 According to the File description, the File is optional and can be NULL. Signed-off-by: Guomin Jiang <guomin.jiang@intel.com> Cc: Jiewen Yao <jiewen.yao@intel.com> Cc: Jian J Wang <jian.j.wang@intel.com> Cc: Chao Zhang <chao.b.zhang@intel.com> Reviewed-by: Jian J Wang <jian.j.wang@intel.com>
* SecurityPkg/DxeImageVerificationLib: change IsCertHashFoundInDatabase name ↵Jian J Wang2020-02-191-3/+3
| | | | | | | | | | | | | | (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 <jiewen.yao@intel.com> Cc: Chao Zhang <chao.b.zhang@intel.com> Signed-off-by: Jian J Wang <jian.j.wang@intel.com> Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
* SecurityPkg/DxeImageVerificationLib: Differentiate error/search result (2) ↵Jian J Wang2020-02-191-19/+58
| | | | | | | | | | | | | | | | | | | | | | | | (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 <jiewen.yao@intel.com> Cc: Chao Zhang <chao.b.zhang@intel.com> Signed-off-by: Jian J Wang <jian.j.wang@intel.com> Reviewed-by: Laszlo Ersek <lersek@redhat.com> Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
* SecurityPkg/DxeImageVerificationLib: plug Data leak in IsForbiddenByDbx() ↵Laszlo Ersek2020-02-191-1/+1
| | | | | | | | | | | | | | (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 <jiewen.yao@intel.com> Cc: Chao Zhang <chao.b.zhang@intel.com> Signed-off-by: Laszlo Ersek <lersek@redhat.com> Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
* SecurityPkg/DxeImageVerificationLib: tighten default result (CVE-2019-14575)Jian J Wang2020-02-191-1/+10
| | | | | | | | | | | | | | 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 <jiewen.yao@intel.com> Cc: Chao Zhang <chao.b.zhang@intel.com> Signed-off-by: Jian J Wang <jian.j.wang@intel.com> Signed-off-by: Laszlo Ersek <lersek@redhat.com> Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
* SecurityPkg/DxeImageVerificationLib: Differentiate error/search result (1) ↵Jian J Wang2020-02-191-22/+42
| | | | | | | | | | | | | | | | | | | | (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 <jiewen.yao@intel.com> Cc: Chao Zhang <chao.b.zhang@intel.com> Signed-off-by: Jian J Wang <jian.j.wang@intel.com> Signed-off-by: Laszlo Ersek <lersek@redhat.com> Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
* SecurityPkg/DxeImageVerificationLib: refactor db/dbx fetching code ↵Jian J Wang2020-02-191-61/+83
| | | | | | | | | | | | | | | | | | | (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 <jiewen.yao@intel.com> Cc: Chao Zhang <chao.b.zhang@intel.com> Signed-off-by: Jian J Wang <jian.j.wang@intel.com> Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
* SecurityPkg/DxeImageVerificationLib: avoid bypass in fetching dbx ↵Jian J Wang2020-02-191-0/+11
| | | | | | | | | | | | | | | | | | | | | (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 <jiewen.yao@intel.com> Cc: Chao Zhang <chao.b.zhang@intel.com> Signed-off-by: Jian J Wang <jian.j.wang@intel.com> Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
* SecurityPkg/DxeImageVerificationLib: fix wrong fetch dbx in IsAllowedByDb ↵Jian J Wang2020-02-191-1/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | (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 <jiewen.yao@intel.com> Cc: Chao Zhang <chao.b.zhang@intel.com> Signed-off-by: Jian J Wang <jian.j.wang@intel.com> Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
* SecurityPkg/DxeImageVerificationLib: reject CertStack.CertNumber==0 per DBX ↵Jian J Wang2020-02-191-1/+1
| | | | | | | | | | | | | | (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 <jiewen.yao@intel.com> Cc: Chao Zhang <chao.b.zhang@intel.com> Signed-off-by: Jian J Wang <jian.j.wang@intel.com> Reviewed-by: Laszlo Ersek <lersek@redhat.com> Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
* SecurityPkg/DxeImageVerificationLib: Fix memory leaks (CVE-2019-14575)Jian J Wang2020-02-191-0/+3
| | | | | | | | | | | | 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 <jiewen.yao@intel.com> Cc: Chao Zhang <chao.b.zhang@intel.com> Signed-off-by: Jian J Wang <jian.j.wang@intel.com> Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
* SecurityPkg/DxeImageVerificationHandler: fix "defer" vs. "deny" policiesLaszlo Ersek2020-01-311-3/+8
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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 <chao.b.zhang@intel.com> Cc: Jian J Wang <jian.j.wang@intel.com> Cc: Jiewen Yao <jiewen.yao@intel.com> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2129 Fixes: 5db28a6753d307cdfb1cfdeb2f63739a9f959837 Signed-off-by: Laszlo Ersek <lersek@redhat.com> Message-Id: <20200116190705.18816-12-lersek@redhat.com> Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com> [lersek@redhat.com: push with Mike's R-b due to Chinese New Year Holiday: <https://edk2.groups.io/g/devel/message/53429>; msgid <d3fbb76dabed4e1987c512c328c82810@intel.com>]
* SecurityPkg/DxeImageVerificationHandler: fix imgexec info on memalloc failLaszlo Ersek2020-01-311-1/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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 <chao.b.zhang@intel.com> Cc: Jian J Wang <jian.j.wang@intel.com> Cc: Jiewen Yao <jiewen.yao@intel.com> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2129 Signed-off-by: Laszlo Ersek <lersek@redhat.com> Message-Id: <20200116190705.18816-11-lersek@redhat.com> Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com> [lersek@redhat.com: push with Mike's R-b due to Chinese New Year Holiday: <https://edk2.groups.io/g/devel/message/53429>; msgid <d3fbb76dabed4e1987c512c328c82810@intel.com>]
* SecurityPkg/DxeImageVerificationHandler: fix retval for (FileBuffer==NULL)Laszlo Ersek2020-01-311-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | "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 <chao.b.zhang@intel.com> Cc: Jian J Wang <jian.j.wang@intel.com> Cc: Jiewen Yao <jiewen.yao@intel.com> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2129 Fixes: 570b3d1a7278df29878da87990e8366bd42d0ec5 Signed-off-by: Laszlo Ersek <lersek@redhat.com> Message-Id: <20200116190705.18816-10-lersek@redhat.com> Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com> [lersek@redhat.com: push with Mike's R-b due to Chinese New Year Holiday: <https://edk2.groups.io/g/devel/message/53429>; msgid <d3fbb76dabed4e1987c512c328c82810@intel.com>]
* SecurityPkg/DxeImageVerificationHandler: eliminate "Status" variableLaszlo Ersek2020-01-311-4/+1
| | | | | | | | | | | | | | | | | | | | | | | 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 <chao.b.zhang@intel.com> Cc: Jian J Wang <jian.j.wang@intel.com> Cc: Jiewen Yao <jiewen.yao@intel.com> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2129 Signed-off-by: Laszlo Ersek <lersek@redhat.com> Message-Id: <20200116190705.18816-9-lersek@redhat.com> Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com> [lersek@redhat.com: push with Mike's R-b due to Chinese New Year Holiday: <https://edk2.groups.io/g/devel/message/53429>; msgid <d3fbb76dabed4e1987c512c328c82810@intel.com>]
* SecurityPkg/DxeImageVerificationHandler: unnest AddImageExeInfo() callLaszlo Ersek2020-01-311-18/+16
| | | | | | | | | | | | | | | | | | | | | | | | | | | 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 <chao.b.zhang@intel.com> Cc: Jian J Wang <jian.j.wang@intel.com> Cc: Jiewen Yao <jiewen.yao@intel.com> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2129 Signed-off-by: Laszlo Ersek <lersek@redhat.com> Message-Id: <20200116190705.18816-8-lersek@redhat.com> Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com> [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: <https://edk2.groups.io/g/devel/message/53429>; msgid <d3fbb76dabed4e1987c512c328c82810@intel.com>]
* SecurityPkg/DxeImageVerificationHandler: remove superfluous Status settingLaszlo Ersek2020-01-311-1/+0
| | | | | | | | | | | | | | | | | | | 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 <chao.b.zhang@intel.com> Cc: Jian J Wang <jian.j.wang@intel.com> Cc: Jiewen Yao <jiewen.yao@intel.com> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2129 Signed-off-by: Laszlo Ersek <lersek@redhat.com> Message-Id: <20200116190705.18816-7-lersek@redhat.com> Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com> [lersek@redhat.com: push with Mike's R-b due to Chinese New Year Holiday: <https://edk2.groups.io/g/devel/message/53429>; msgid <d3fbb76dabed4e1987c512c328c82810@intel.com>]
* SecurityPkg/DxeImageVerificationHandler: fix retval on memalloc failureLaszlo Ersek2020-01-311-2/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | 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 <chao.b.zhang@intel.com> Cc: Jian J Wang <jian.j.wang@intel.com> Cc: Jiewen Yao <jiewen.yao@intel.com> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2129 Fixes: 570b3d1a7278df29878da87990e8366bd42d0ec5 Signed-off-by: Laszlo Ersek <lersek@redhat.com> Message-Id: <20200116190705.18816-6-lersek@redhat.com> Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com> [lersek@redhat.com: push with Mike's R-b due to Chinese New Year Holiday: <https://edk2.groups.io/g/devel/message/53429>; msgid <d3fbb76dabed4e1987c512c328c82810@intel.com>]
* SecurityPkg/DxeImageVerificationHandler: narrow down PE/COFF hash statusLaszlo Ersek2020-01-311-2/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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 <chao.b.zhang@intel.com> Cc: Jian J Wang <jian.j.wang@intel.com> Cc: Jiewen Yao <jiewen.yao@intel.com> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2129 Signed-off-by: Laszlo Ersek <lersek@redhat.com> Message-Id: <20200116190705.18816-5-lersek@redhat.com> Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com> [lersek@redhat.com: push with Mike's R-b due to Chinese New Year Holiday: <https://edk2.groups.io/g/devel/message/53429>; msgid <d3fbb76dabed4e1987c512c328c82810@intel.com>]
* SecurityPkg/DxeImageVerificationHandler: keep PE/COFF info status internalLaszlo Ersek2020-01-311-4/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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 <chao.b.zhang@intel.com> Cc: Jian J Wang <jian.j.wang@intel.com> Cc: Jiewen Yao <jiewen.yao@intel.com> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2129 Signed-off-by: Laszlo Ersek <lersek@redhat.com> Message-Id: <20200116190705.18816-4-lersek@redhat.com> Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com> [lersek@redhat.com: push with Mike's R-b due to Chinese New Year Holiday: <https://edk2.groups.io/g/devel/message/53429>; msgid <d3fbb76dabed4e1987c512c328c82810@intel.com>]
* SecurityPkg/DxeImageVerificationHandler: remove "else" after return/breakLaszlo Ersek2020-01-311-20/+21
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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 <chao.b.zhang@intel.com> Cc: Jian J Wang <jian.j.wang@intel.com> Cc: Jiewen Yao <jiewen.yao@intel.com> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2129 Signed-off-by: Laszlo Ersek <lersek@redhat.com> Message-Id: <20200116190705.18816-3-lersek@redhat.com> Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com> [lersek@redhat.com: push with Mike's R-b due to Chinese New Year Holiday: <https://edk2.groups.io/g/devel/message/53429>; msgid <d3fbb76dabed4e1987c512c328c82810@intel.com>]
* SecurityPkg/DxeImageVerificationHandler: simplify "VerifyStatus"Laszlo Ersek2020-01-311-10/+10
| | | | | | | | | | | | | | | | | | | | | | | | | 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 <chao.b.zhang@intel.com> Cc: Jian J Wang <jian.j.wang@intel.com> Cc: Jiewen Yao <jiewen.yao@intel.com> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2129 Signed-off-by: Laszlo Ersek <lersek@redhat.com> Message-Id: <20200116190705.18816-2-lersek@redhat.com> Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com> [lersek@redhat.com: push with Mike's R-b due to Chinese New Year Holiday: <https://edk2.groups.io/g/devel/message/53429>; msgid <d3fbb76dabed4e1987c512c328c82810@intel.com>]
* SecurityPkg: Fix spelling errorsSean Brogan2019-10-231-5/+5
| | | | | | | | | | | https://bugzilla.tianocore.org/show_bug.cgi?id=2265 Cc: Jiewen Yao <jiewen.yao@intel.com> Cc: Jian J Wang <jian.j.wang@intel.com> Cc: Chao Zhang <chao.b.zhang@intel.com> Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com> Reviewed-by: Jiewen Yao <Jiewen.yao@intel.com> Reviewed-by: Jian J Wang <jian.j.wang@intel.com>
* SecurityPkg: Replace BSD License with BSD+Patent LicenseMichael D Kinney2019-04-091-7/+1
| | | | | | | | | | | | | | | | | | | | | https://bugzilla.tianocore.org/show_bug.cgi?id=1373 Replace BSD 2-Clause License with BSD+Patent License. This change is based on the following emails: https://lists.01.org/pipermail/edk2-devel/2019-February/036260.html https://lists.01.org/pipermail/edk2-devel/2018-October/030385.html RFCs with detailed process for the license change: V3: https://lists.01.org/pipermail/edk2-devel/2019-March/038116.html V2: https://lists.01.org/pipermail/edk2-devel/2019-March/037669.html V1: https://lists.01.org/pipermail/edk2-devel/2019-March/037500.html Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com> Reviewed-by: Jiewen Yao <Jiewen.yao@intel.com> Reviewed-by: Jian J Wang <jian.j.wang@intel.com>
* SecurityPkg: remove PE/COFF header workaround for ELILO on IPFArd Biesheuvel2018-09-201-39/+8
| | | | | | | | | | | Now that Itanium support has been dropped, we can remove the various occurrences of the ELILO on Itanium PE/COFF header workaround. Link: https://bugzilla.tianocore.org/show_bug.cgi?id=816 Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> Reviewed-by: Yao Jiewen <jiewen.yao@intel.com> Reviewed-by: Zhang Chao B <chao.b.zhang@intel.com>
* SecurityPkg: Clean up source filesLiming Gao2018-06-281-7/+7
| | | | | | | | | 1. Do not use tab characters 2. No trailing white space in one line 3. All files must end with CRLF Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Liming Gao <liming.gao@intel.com>
* SecurityPkg: Refine type cast for pointer subtractionHao Wu2017-03-061-8/+8
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | For pointer subtraction, the result is of type "ptrdiff_t". According to the C11 standard (Committee Draft - April 12, 2011): "When two pointers are subtracted, both shall point to elements of the same array object, or one past the last element of the array object; the result is the difference of the subscripts of the two array elements. The size of the result is implementation-defined, and its type (a signed integer type) is ptrdiff_t defined in the <stddef.h> header. If the result is not representable in an object of that type, the behavior is undefined." In our codes, there are cases that the pointer subtraction is not performed by pointers to elements of the same array object. This might lead to potential issues, since the behavior is undefined according to C11 standard. Also, since the size of type "ptrdiff_t" is implementation-defined. Some static code checkers may warn that the pointer subtraction might underflow first and then being cast to a bigger size. For example: UINT8 *Ptr1, *Ptr2; UINTN PtrDiff; ... PtrDiff = (UINTN) (Ptr1 - Ptr2); The commit will refine the pointer subtraction expressions by casting each pointer to UINTN first and then perform the subtraction: PtrDiff = (UINTN) Ptr1 - (UINTN) Ptr2; Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Hao Wu <hao.a.wu@intel.com> Acked-by: Laszlo Ersek <lersek@redhat.com> Reviewed-by: Chao Zhang <chao.b.zhang@intel.com>
* SecurityPkg: DxeImageVerificationLib: Update PCR[7] measure logicZhang, Chao B2017-01-221-3/+7
| | | | | | | | | | | | | | Update PCR[7] measure logic according to TCG PC Client PFP 00.37. Only entries in DB that is used for image authentication need to be measured. http://www.trustedcomputinggroup.org/wp-content/uploads/PC-ClientSpecific_Platform_Profile_for_TPM_2p0_Systems_v21.pdf Cc: Star Zeng <star.zeng@intel.com> Cc: Yao Jiewen <jiewen.yao@intel.com> Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Chao Zhang <chao.b.zhang@intel.com> Reviewed-by: Star Zeng <star.zeng@intel.com> Reviewed-by: Yao Jiewen <jiewen.yao@intel.com>
* SecurityPkg DxeImageVerificationLib: Add comments in HashPeImage()Liming Gao2016-07-141-1/+4
| | | | | | | | | The input PeImage in HashPeImage() has been checked. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Liming Gao <liming.gao@intel.com> Reviewed-by: Jiewen Yao <jiewen.yao@intel.com> Reviewed-by: Chao Zhang <chao.b.zhang@intel.com>
* SecurityPkg/DxeImageVerificationLib: Add DEBUG messages for image ↵Cinnamon Shia2016-05-191-1/+18
| | | | | | | | | | | verification failures Add DEBUG messages in DxeImageerificationLib to help debug Secure Boot image verification failures Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Cinnamon Shia <cinnamon.shia@hpe.com> Reviewed-by: Samer EL-Haj-Mahmoud <elhaj@hpe.com> Reviewed-by: Chao Zhang <chao.b.zhang@intel.com>
* SecurityPkg: AuthVariableLib: Remove Customized SecureBoot Mode transition.Zhang, Chao B2016-05-041-604/+17
| | | | | | | | | | | | | | Remove Customized SecureBoot Mode transition logic for Mantis 1263, including AuditMode/DeployedMode/PK update management. Also remove image verification logic in AuditMode. The feature has been moved to https://github.com/tianocore/edk2-staging/tree/Customized-Secure-Boot Previous check-in hash is SHA-1: 4fc08e8d683522f255727626197d919a40d4836c Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Chao Zhang <chao.b.zhang@intel.com> Reviewed-by: El-Haj-Mahmoud Samer <samer.el-haj-mahmoud@hpe.com> Reviewed-by: Fu Siyuan <siyuan.fu@intel.com>
* SecuritPkg: DxeImageVerificationLib: Fix wrong verification logic in DBX & DBTZhang, Chao B2016-04-291-2/+9
| | | | | | | | | In image verification, if image verified pass in DBT, still need to verify if it is blocked by any other cert/cert hash from DBX. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Chao Zhang <chao.b.zhang@intel.com> Reviewed-by: Long Qin <qin.long@intel.com> Reviewed-by: Dick Wilkins <dick_wilkins@phoenix.com>
* SecurityPkg: AuthVariableLib: Customized SecureBoot Mode transition.Chao Zhang2015-12-071-36/+634
| | | | | | | | | | | | | Implement Customized SecureBoot Mode transition logic according to Mantis 1263, including AuditMode/DeployedMode/PK update management. Also implement image verification logic in AuditMode. Image Certificate & Hash are recorded to EFI Image Execution Table. https://mantis.uefi.org/mantis/view.php?id=1263 Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Chao Zhang <chao.b.zhang@intel.com> Reviewed-by: Zeng Star <star.zeng@intel.com> Reviewed-by: Long Qin <qin.long@intel.com> git-svn-id: https://svn.code.sf.net/p/edk2/code/trunk/edk2@19133 6f19259b-4bc3-4df7-8a09-765794883524
* SecurityPkg: Update SignatureSize to comply UEFI specChao Zhang2015-08-171-1/+1
| | | | | | | | | | Update SignatureSize to include SignatureOwner GUID. This behavior is defined by UEFI spec Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Chao Zhang <chao.b.zhang@intel.com> Reviewed-by: Qin Long <qin.long@intel.com> git-svn-id: https://svn.code.sf.net/p/edk2/code/trunk/edk2@18226 6f19259b-4bc3-4df7-8a09-765794883524
* SecurityPkg: Fix DBX Variable Read Error in ImageVerificationLibChao Zhang2015-07-151-1/+1
| | | | | | | | | | ImageVerificationLib passes wrong data buffer size when reading DBX variable, causing heap crash. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Chao Zhang <chao.b.zhang@intel.com> Reviewed-by: Qin Long <qin.long@intel.com> git-svn-id: https://svn.code.sf.net/p/edk2/code/trunk/edk2@17981 6f19259b-4bc3-4df7-8a09-765794883524
* SecurityPkg: Fix wrong calculation of ImageExeInfoEntrySizeLiming Gao2015-06-231-2/+2
| | | | | | | | | | | | | Per UEFI spec, EFI_IMAGE_EXECUTION_INFO structure is updated to comment Signature field. So, its structure doesn't include Signature field. But, ImageExeInfoEntrySize uses its structure size minor Signature size. It will be corrected in this change. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Liming Gao <liming.gao@intel.com> Reviewed-by: Chao Zhang <chao.b.zhang@intel.com> git-svn-id: https://svn.code.sf.net/p/edk2/code/trunk/edk2@17687 6f19259b-4bc3-4df7-8a09-765794883524
* SecurityPkg: Fix wrong cert data measurement in DBX pathChao Zhang2015-06-091-1/+1
| | | | | | | | | | Fix wrong cert data measurement when image is rejected by DBX Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Chao Zhang <chao.b.zhang@intel.com> Reviewed-by: Long Qin <qin.long@intel.com> git-svn-id: https://svn.code.sf.net/p/edk2/code/trunk/edk2@17591 6f19259b-4bc3-4df7-8a09-765794883524
* SecurityPkg: Fix typo.Shumin Qiu2015-03-131-2/+2
| | | | | | | | | | Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Shumin Qiu <shumin.qiu@intel.com> Reviewed-by: Guo Dong <guo.dong@intel.com> git-svn-id: https://svn.code.sf.net/p/edk2/code/trunk/edk2@17047 6f19259b-4bc3-4df7-8a09-765794883524
* Add the missed local variable initialization to remove the possible warning.Long, Qin2015-02-051-0/+1
| | | | | | | | | | Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: "Long, Qin" <qin.long@intel.com> Reviewed-by: "Fu, Siyuan" <siyuan.fu@intel.com> Reviewed-by: "Dong, Guo" <guo.dong@initel.com> git-svn-id: https://svn.code.sf.net/p/edk2/code/trunk/edk2@16763 6f19259b-4bc3-4df7-8a09-765794883524
* Correct the DBX and Certificate Hash Checking.Long, Qin2015-02-041-10/+92
| | | | | | | | | | | | | Add the missed image signature verification against DBX; and add the missed logic to enhance the certificate hash checking when handling DB database. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: "Long, Qin" <qin.long@intel.com> Reviewed-by: "Fu, Siyuan" <siyuan.fu@intel.com> Reviewed-by: "Dong, Guo" <guo.dong@initel.com> git-svn-id: https://svn.code.sf.net/p/edk2/code/trunk/edk2@16744 6f19259b-4bc3-4df7-8a09-765794883524
generated by cgit v1.2.3 (git 2.25.1) at 2024-08-03 22:06:17 +0000