Please see my comments below in [Chao]
The If{} else{} logic for debug log makes ImageVerificationLib logic more
complex and less readable.
My suggestion is to direct report image signature against DBX/DBT, image
signature against DB/DBT, image hash against DB/DBX result in debug log.
User can get clear log from each verification steps and find where the problem
is happening.
Thanks & Best regards
Chao Zhang
-----Original Message-----
From: Cinnamon Shia [mailto:cinnamon.s...@hpe.com]
Sent: Wednesday, April 27, 2016 12:52 AM
To: edk2-devel@lists.01.org
Cc: ler...@redhat.com; samer.el-haj-mahm...@hpe.com; Zhang, Chao B; Cinnamon
Shia
Subject: [PATCH v3] SecurityPkg/DxeImageVerificationLib: Add DEBUG messages for
image 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.s...@hpe.com<mailto:cinnamon.s...@hpe.com>>
Reviewed-by: Samer EL-Haj-Mahmoud <el...@hpe.com<mailto:el...@hpe.com>>
---
.../DxeImageVerificationLib.c | 49 ++++++++++++++++++----
1 file changed, 41 insertions(+), 8 deletions(-)
diff --git
a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
index 4b4d3bf..ee94dce 100644
--- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
+++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLi
+++ b.c
@@ -13,6 +13,7 @@
untrusted PE/COFF image and validate its data structure within this image
buffer before use.
Copyright (c) 2009 - 2015, Intel Corporation. All rights reserved.<BR>
+(C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
This program and the accompanying materials are licensed and made available
under the terms and conditions of the BSD License which accompanies this
distribution. The full text of the license may be found at @@ -71,6 +72,8 @@
HASH_TABLE mHash[] = {
{ L"SHA512", 64, &mHashOidValue[32], 9, Sha512GetContextSize, Sha512Init,
Sha512Update, Sha512Final} };
+EFI_STRING mHashTypeStr;
+
/**
SecureBoot Hook for processing image verification.
@@ -340,6 +343,7 @@ HashPeImage (
return FALSE;
}
+ mHashTypeStr = mHash[HashAlg].Name;
CtxSize = mHash[HashAlg].GetContextSize();
HashCtx = AllocatePool (CtxSize);
@@ -1782,14 +1786,16 @@ ImageVerificationInAuditMode (
UINT32 OffSet;
CHAR16 *FilePathStr;
UINTN SignatureListSize;
+ BOOLEAN SigAcceptedByCertsInDb;
- SignatureList = NULL;
- WinCertificate = NULL;
- SecDataDir = NULL;
- PkcsCertData = NULL;
- FilePathStr = NULL;
- Action = EFI_IMAGE_EXECUTION_AUTH_SIG_FAILED |
EFI_IMAGE_EXECUTION_INITIALIZED;
- Status = EFI_ACCESS_DENIED;
+ SignatureList = NULL;
+ WinCertificate = NULL;
+ SecDataDir = NULL;
+ PkcsCertData = NULL;
+ FilePathStr = NULL;
+ Action = EFI_IMAGE_EXECUTION_AUTH_SIG_FAILED |
EFI_IMAGE_EXECUTION_INITIALIZED;
+ Status = EFI_ACCESS_DENIED;
+ SigAcceptedByCertsInDb = FALSE;
//
@@ -1873,6 +1879,7 @@ ImageVerificationInAuditMode (
//
// The information can't be got from the invalid PeImage
//
+ DEBUG ((DEBUG_ERROR, "DxeImageVerificationLib: PeImage invalid.
+ Cannot retrieve image information.\n"));
goto END;
}
@@ -1896,6 +1903,7 @@ ImageVerificationInAuditMode (
//
// It is not a valid Pe/Coff file.
//
+ DEBUG ((DEBUG_ERROR, "DxeImageVerificationLib: Not a valid PE/COFF
+ image.\n"));
Status = EFI_ACCESS_DENIED;
goto END;
}
@@ -1942,6 +1950,7 @@ ImageVerificationInAuditMode (
// and not be reflected in the security data base "dbx".
//
if (!HashPeImage (HASHALG_SHA256)) {
+ DEBUG ((DEBUG_ERROR, "DxeImageVerificationLib: Failed to hash
+ this image using %s.\n", mHashTypeStr));
Status = EFI_ACCESS_DENIED;
goto END;
}
@@ -1955,7 +1964,14 @@ ImageVerificationInAuditMode (
//
if (IsSignatureFoundInDatabase (EFI_IMAGE_SECURITY_DATABASE,
mImageDigest, &mCertType, mImageDigestSize)) {
Action = EFI_IMAGE_EXECUTION_AUTH_SIG_PASSED |
EFI_IMAGE_EXECUTION_INITIALIZED;
+ } else {
+ //
+ // Image Hash is not in DB/DBX
+ //
+ DEBUG ((DEBUG_ERROR, "DxeImageVerificationLib: Image is not
+ signed and %s hash of image is not found in DB/DBX.\n",
+ mHashTypeStr));
}
+ } else {
+ DEBUG ((DEBUG_ERROR, "DxeImageVerificationLib: Image is not
+ signed and %s hash of image is rejected by DBX.\n", mHashTypeStr));
}
//
@@ -2029,7 +2045,9 @@ ImageVerificationInAuditMode (
// Check the digital signature against the valid certificate in allowed
database (db).
//
if (!IsForbiddenByDbx (AuthData, AuthDataSize, TRUE, FilePathStr, File)) {
- IsAllowedByDb (AuthData, AuthDataSize, TRUE, FilePathStr, File);
+ SigAcceptedByCertsInDb = IsAllowedByDb (AuthData, AuthDataSize, TRUE,
FilePathStr, File);
+ } else {
+ DEBUG ((DEBUG_ERROR, "DxeImageVerificationLib: Image is signed
+ but signature is rejected by DBX.\n"));
}
//
@@ -2038,7 +2056,13 @@ ImageVerificationInAuditMode (
if (!IsSignatureFoundInDatabase (EFI_IMAGE_SECURITY_DATABASE1,
mImageDigest, &mCertType, mImageDigestSize)) {
if (IsSignatureFoundInDatabase (EFI_IMAGE_SECURITY_DATABASE,
mImageDigest, &mCertType, mImageDigestSize)) {
Action = EFI_IMAGE_EXECUTION_AUTH_SIG_PASSED |
EFI_IMAGE_EXECUTION_INITIALIZED;
+ } else {
+ if (!SigAcceptedByCertsInDb) {
+ DEBUG ((DEBUG_ERROR, "DxeImageVerificationLib: Image is signed but
signature and %s hash of image are not found in DB/DBX.\n", mHashTypeStr));
[Chao] Image verification passes in DBX/DBT check but fails in DB/DBT can be
caused by different reasons
1. Image signature can't be verified by any certs from DB
2. Image signature can be verified by cert from DB, but signature passes
cert revocation time.
So here the debug log isn't complete.
+ }
}
+ } else {
+ DEBUG ((DEBUG_ERROR, "DxeImageVerificationLib: Image is signed
+ but %s hash of image is rejected by DBX.\n", mHashTypeStr));
}
//
@@ -2259,6 +2283,7 @@ DxeImageVerificationHandler (
//
// The information can't be got from the invalid PeImage
//
+ DEBUG ((DEBUG_ERROR, "DxeImageVerificationLib: PeImage invalid.
+ Cannot retrieve image information.\n"));
goto Done;
}
@@ -2282,6 +2307,7 @@ DxeImageVerificationHandler (
//
// It is not a valid Pe/Coff file.
//
+ DEBUG ((DEBUG_ERROR, "DxeImageVerificationLib: Not a valid PE/COFF
+ image.\n"));
goto Done;
}
@@ -2327,6 +2353,7 @@ DxeImageVerificationHandler (
// and not be reflected in the security data base "dbx".
//
if (!HashPeImage (HASHALG_SHA256)) {
+ DEBUG ((DEBUG_ERROR, "DxeImageVerificationLib: Failed to hash
+ this image using %s.\n", mHashTypeStr));
goto Done;
}
@@ -2334,6 +2361,7 @@ DxeImageVerificationHandler (
//
// Image Hash is in forbidden database (DBX).
//
+ DEBUG ((DEBUG_ERROR, "DxeImageVerificationLib: Image is not
+ signed and %s hash of image is rejected by DBX.\n", mHashTypeStr));
goto Done;
}
@@ -2347,6 +2375,7 @@ DxeImageVerificationHandler (
//
// Image Hash is not found in both forbidden and allowed database.
//
+ DEBUG ((DEBUG_ERROR, "DxeImageVerificationLib: Image is not signed
+ and %s hash of image is not found in DB/DBX.\n", mHashTypeStr));
goto Done;
}
@@ -2409,6 +2438,7 @@ DxeImageVerificationHandler (
if (IsForbiddenByDbx (AuthData, AuthDataSize, FALSE, NULL, NULL)) {
Action = EFI_IMAGE_EXECUTION_AUTH_SIG_FAILED;
VerifyStatus = EFI_ACCESS_DENIED;
+ DEBUG ((DEBUG_ERROR, "DxeImageVerificationLib: Image is signed
+ but signature is rejected by DBX.\n"));
break;
}
@@ -2426,11 +2456,14 @@ DxeImageVerificationHandler (
//
if (IsSignatureFoundInDatabase (EFI_IMAGE_SECURITY_DATABASE1,
mImageDigest, &mCertType, mImageDigestSize)) {
Action = EFI_IMAGE_EXECUTION_AUTH_SIG_FOUND;
+ DEBUG ((DEBUG_ERROR, "DxeImageVerificationLib: Image is signed
+ but %s hash of image is rejected by DBX.\n", mHashTypeStr));
VerifyStatus = EFI_ACCESS_DENIED;
break;
} else if (EFI_ERROR (VerifyStatus)) {
if (IsSignatureFoundInDatabase (EFI_IMAGE_SECURITY_DATABASE,
mImageDigest, &mCertType, mImageDigestSize)) {
VerifyStatus = EFI_SUCCESS;
+ } else {
+ DEBUG ((DEBUG_ERROR, "DxeImageVerificationLib: Image is signed
+ but signature and %s hash of image are not found in DB/DBX.\n",
+ mHashTypeStr));
[Chao] same as prior comment
}
}
}
--
2.8.1.windows.1
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
