sravan/system76-edk2
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

SecurityPkg/DxeImageVerificationLib: Differentiate error/search result (2) (CVE-2019-14575)

Browse Source 
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>
This commit is contained in:
Jian J Wang
2019-10-10 15:02:17 +08:00
committed by mergify[bot]
parent cb30c8f251
commit b1c1147059
1 changed files with 58 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files

71
SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
View File

@ -955,17 +955,19 @@ Done:
@param[in] Signature Pointer to signature that is searched for. @param[in] Signature Pointer to signature that is searched for.
@param[in] CertType Pointer to hash algorithm. @param[in] CertType Pointer to hash algorithm.
@param[in] SignatureSize Size of Signature. @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. @retval EFI_SUCCESS Finished the search without any error.
@return FALSE Not found the signature in the variable database. @retval Others Error occurred in the search of database.
**/ **/
BOOLEAN EFI_STATUS
IsSignatureFoundInDatabase ( IsSignatureFoundInDatabase (
IN CHAR16 *VariableName, IN CHAR16 *VariableName,
IN UINT8 *Signature, IN UINT8 *Signature,
IN EFI_GUID *CertType, IN EFI_GUID *CertType,
IN UINTN SignatureSize IN UINTN SignatureSize,
OUT BOOLEAN *IsFound
) )
{ {
EFI_STATUS Status; EFI_STATUS Status;
@ -975,22 +977,28 @@ IsSignatureFoundInDatabase (
UINT8 *Data; UINT8 *Data;
UINTN Index; UINTN Index;
UINTN CertCount; UINTN CertCount;
BOOLEAN IsFound;
// //
// Read signature database variable. // Read signature database variable.
// //
IsFound = FALSE; *IsFound = FALSE;
Data = NULL; Data = NULL;
DataSize = 0; DataSize = 0;
Status = gRT->GetVariable (VariableName, &gEfiImageSecurityDatabaseGuid, NULL, &DataSize, NULL); Status = gRT->GetVariable (VariableName, &gEfiImageSecurityDatabaseGuid, NULL, &DataSize, NULL);
if (Status != EFI_BUFFER_TOO_SMALL) { 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); Data = (UINT8 *) AllocateZeroPool (DataSize);
if (Data == NULL) { if (Data == NULL) {
return FALSE; return EFI_OUT_OF_RESOURCES;
} }
Status = gRT->GetVariable (VariableName, &gEfiImageSecurityDatabaseGuid, NULL, &DataSize, Data); Status = gRT->GetVariable (VariableName, &gEfiImageSecurityDatabaseGuid, NULL, &DataSize, Data);
@ -1010,7 +1018,7 @@ IsSignatureFoundInDatabase (
// //
// Find the signature in database. // Find the signature in database.
// //
IsFound = TRUE; *IsFound = TRUE;
// //
// Entries in UEFI_IMAGE_SECURITY_DATABASE that are used to validate image should be measured // 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); Cert = (EFI_SIGNATURE_DATA *) ((UINT8 *) Cert + CertList->SignatureSize);
} }
if (IsFound) { if (*IsFound) {
break; break;
} }
} }
@ -1037,7 +1045,7 @@ Done:
FreePool (Data); FreePool (Data);
} }
return IsFound; return Status;
} }
/** /**
@ -1648,6 +1656,8 @@ DxeImageVerificationHandler (
CHAR16 *NameStr; CHAR16 *NameStr;
RETURN_STATUS PeCoffStatus; RETURN_STATUS PeCoffStatus;
EFI_STATUS HashStatus; EFI_STATUS HashStatus;
EFI_STATUS DbStatus;
BOOLEAN IsFound;
SignatureList = NULL; SignatureList = NULL;
SignatureListSize = 0; SignatureListSize = 0;
@ -1656,7 +1666,7 @@ DxeImageVerificationHandler (
PkcsCertData = NULL; PkcsCertData = NULL;
Action = EFI_IMAGE_EXECUTION_AUTH_UNTESTED; Action = EFI_IMAGE_EXECUTION_AUTH_UNTESTED;
IsVerified = FALSE; IsVerified = FALSE;
IsFound = FALSE;
// //
// Check the image type and get policy setting. // Check the image type and get policy setting.
@ -1798,7 +1808,14 @@ DxeImageVerificationHandler (
goto Failed; 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). // Image Hash is in forbidden database (DBX).
// //
@ -1806,7 +1823,14 @@ DxeImageVerificationHandler (
goto Failed; 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). // Image Hash is in allowed database (DB).
// //
@ -1894,14 +1918,29 @@ DxeImageVerificationHandler (
// //
// Check the image's hash value. // 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; 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)); DEBUG ((DEBUG_INFO, "DxeImageVerificationLib: Image is signed but %s hash of image is found in DBX.\n", mHashTypeStr));
IsVerified = FALSE; IsVerified = FALSE;
break; break;
} }
if (!IsVerified) { 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; IsVerified = TRUE;
} else { } 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)); 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));