Skip to content

Commit

Permalink
SecurityPkg/DxeImageVerificationLib: Differentiate error/search resul…
Browse files Browse the repository at this point in the history 
…t (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 <[email protected]>
Cc: Chao Zhang <[email protected]>
Signed-off-by: Jian J Wang <[email protected]>
Signed-off-by: Laszlo Ersek <[email protected]>
Reviewed-by: Jiewen Yao <[email protected]>
  • Loading branch information
@mergify
Jian J Wang authored and mergify[bot] committed Feb 19, 2020
1 parent adc6898 commit a83dbf0
Showing 1 changed file with 42 additions and 22 deletions.
64 changes: 42 additions & 22 deletions SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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)) {
Expand Down Expand Up @@ -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;
}

Expand All @@ -923,7 +922,8 @@ IsCertHashFoundInDatabase (
//
// Hash of Certificate is found in forbidden database.
//
IsFound = TRUE;
Status = EFI_SUCCESS;
*IsFound = TRUE;

//
// Return the revocation time.
Expand All @@ -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;
}

/**
Expand Down Expand Up @@ -1216,6 +1218,7 @@ IsForbiddenByDbx (
{
EFI_STATUS Status;
BOOLEAN IsForbidden;
BOOLEAN IsFound;
UINT8 *Data;
UINTN DataSize;
EFI_SIGNATURE_LIST *CertList;
Expand Down Expand Up @@ -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;
}

}
Expand Down Expand Up @@ -1392,6 +1404,7 @@ IsAllowedByDb (
{
EFI_STATUS Status;
BOOLEAN VerifyStatus;
BOOLEAN IsFound;
EFI_SIGNATURE_LIST *CertList;
EFI_SIGNATURE_DATA *CertData;
UINTN DataSize;
Expand Down Expand Up @@ -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.
//
Expand Down

0 comments on commit a83dbf0

Please sign in to comment.