[edk2] [PATCH] SecurityPkg : Add DEBUG messages in image verification

2015-12-21 Thread Samer El-Haj-Mahmoud
Add DEBUG messages in DxeImageerificationLib to help debug
Secure Boot image verification failures

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Samer El-Haj-Mahmoud 
---
 .../Library/DxeImageVerificationLib/DxeImageVerificationLib.c| 9 +
 1 file changed, 9 insertions(+)

diff --git 
a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c 
b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
index 4b4d3bf..888cd04 100644
--- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
+++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.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.
+(C) Copyright 2015 Hewlett Packard Enterprise Development LP
 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
@@ -2259,6 +2260,7 @@ DxeImageVerificationHandler (
 //
 // The information can't be got from the invalid PeImage
 //
+DEBUG ((DEBUG_ERROR, "PeImage invald. Cannot retrieve image 
information.\n"));
 goto Done;
   }
 
@@ -2282,6 +2284,7 @@ DxeImageVerificationHandler (
 //
 // It is not a valid Pe/Coff file.
 //
+DEBUG ((DEBUG_ERROR, "Not a valid PE/COFF image.\n"));
 goto Done;
   }
 
@@ -2327,6 +2330,7 @@ DxeImageVerificationHandler (
 // and not be reflected in the security data base "dbx".
 //
 if (!HashPeImage (HASHALG_SHA256)) {
+  DEBUG ((DEBUG_ERROR, "Failed to hash this image.\n"));
   goto Done;
 }
 
@@ -2334,6 +2338,7 @@ DxeImageVerificationHandler (
   //
   // Image Hash is in forbidden database (DBX).
   //
+  DEBUG ((DEBUG_ERROR, "Image is not signed and hash is in DBX.\n"));
   goto Done;
 }
 
@@ -2347,6 +2352,7 @@ DxeImageVerificationHandler (
 //
 // Image Hash is not found in both forbidden and allowed database.
 //
+DEBUG ((DEBUG_ERROR, "Image is not signed and hash is not found in 
DB/DBX.\n"));
 goto Done;
   }
 
@@ -2409,6 +2415,7 @@ DxeImageVerificationHandler (
 if (IsForbiddenByDbx (AuthData, AuthDataSize, FALSE, NULL, NULL)) {
   Action = EFI_IMAGE_EXECUTION_AUTH_SIG_FAILED;
   VerifyStatus = EFI_ACCESS_DENIED;
+  DEBUG ((DEBUG_ERROR, "Image is signed but digital signature is in 
DBX.\n"));
   break;
 }
 
@@ -2426,11 +2433,13 @@ DxeImageVerificationHandler (
 //
 if (IsSignatureFoundInDatabase (EFI_IMAGE_SECURITY_DATABASE1, 
mImageDigest, , mImageDigestSize)) {
   Action = EFI_IMAGE_EXECUTION_AUTH_SIG_FOUND;
+  DEBUG ((DEBUG_ERROR, "Image is signed but digital signature failed 
validation and hash is in DBX.\n"));
   VerifyStatus = EFI_ACCESS_DENIED;
   break;
 } else if (EFI_ERROR (VerifyStatus)) {
   if (IsSignatureFoundInDatabase (EFI_IMAGE_SECURITY_DATABASE, 
mImageDigest, , mImageDigestSize)) {
 VerifyStatus = EFI_SUCCESS;
+DEBUG ((DEBUG_ERROR, "Image is signed but digital signature failed 
validation and hash is not found in DB/DBX.\n"));
   }
 }
   }
-- 
2.6.3.windows.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH] SecurityPkg : Add DEBUG messages in image verification

2015-12-21 Thread Zhang, Chao B
Samer:
  1. Please add DxeImageVerificationLib as Prefix in debug message
  2. @@ -2409,6 +2415,7 @@ DxeImageVerificationHandler (
 if (IsForbiddenByDbx (AuthData, AuthDataSize, FALSE, NULL, NULL)) {
   Action = EFI_IMAGE_EXECUTION_AUTH_SIG_FAILED;
   VerifyStatus = EFI_ACCESS_DENIED;
+  DEBUG ((DEBUG_ERROR, "Image is signed but digital signature is in 
+ DBX.\n"));
   break;
 }

 Is better changed to "Image is signed but signature is rejected by DBX"

  3. You'd better add HASH type in debug log. 
  4. } else if (EFI_ERROR (VerifyStatus)) {
   if (IsSignatureFoundInDatabase (EFI_IMAGE_SECURITY_DATABASE, 
mImageDigest, , mImageDigestSize)) {
 VerifyStatus = EFI_SUCCESS;
+DEBUG ((DEBUG_ERROR, "Image is signed but digital signature 
+ failed validation and hash is not found in DB/DBX.\n"));
 The log is wrong & suggest not to include this debug print


Thanks & Best regards
Chao Zhang

-Original Message-
From: Samer El-Haj-Mahmoud [mailto:samer.el-haj-mahm...@hpe.com] 
Sent: Tuesday, December 22, 2015 8:02 AM
To: edk2-devel@lists.01.org
Cc: Zhang, Chao B; Samer El-Haj-Mahmoud; Samer El-Haj-Mahmoud
Subject: [PATCH] SecurityPkg : Add DEBUG messages in image verification

Add DEBUG messages in DxeImageerificationLib to help debug Secure Boot image 
verification failures

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Samer El-Haj-Mahmoud 
---
 .../Library/DxeImageVerificationLib/DxeImageVerificationLib.c| 9 +
 1 file changed, 9 insertions(+)

diff --git 
a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c 
b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
index 4b4d3bf..888cd04 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.
+(C) Copyright 2015 Hewlett Packard Enterprise Development LP
 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 @@ -2259,6 +2260,7 
@@ DxeImageVerificationHandler (
 //
 // The information can't be got from the invalid PeImage
 //
+DEBUG ((DEBUG_ERROR, "PeImage invald. Cannot retrieve image 
+ information.\n"));
 goto Done;
   }
 
@@ -2282,6 +2284,7 @@ DxeImageVerificationHandler (
 //
 // It is not a valid Pe/Coff file.
 //
+DEBUG ((DEBUG_ERROR, "Not a valid PE/COFF image.\n"));
 goto Done;
   }
 
@@ -2327,6 +2330,7 @@ DxeImageVerificationHandler (
 // and not be reflected in the security data base "dbx".
 //
 if (!HashPeImage (HASHALG_SHA256)) {
+  DEBUG ((DEBUG_ERROR, "Failed to hash this image.\n"));
   goto Done;
 }
 
@@ -2334,6 +2338,7 @@ DxeImageVerificationHandler (
   //
   // Image Hash is in forbidden database (DBX).
   //
+  DEBUG ((DEBUG_ERROR, "Image is not signed and hash is in 
+ DBX.\n"));
   goto Done;
 }
 
@@ -2347,6 +2352,7 @@ DxeImageVerificationHandler (
 //
 // Image Hash is not found in both forbidden and allowed database.
 //
+DEBUG ((DEBUG_ERROR, "Image is not signed and hash is not found in 
+ DB/DBX.\n"));
 goto Done;
   }
 
@@ -2409,6 +2415,7 @@ DxeImageVerificationHandler (
 if (IsForbiddenByDbx (AuthData, AuthDataSize, FALSE, NULL, NULL)) {
   Action = EFI_IMAGE_EXECUTION_AUTH_SIG_FAILED;
   VerifyStatus = EFI_ACCESS_DENIED;
+  DEBUG ((DEBUG_ERROR, "Image is signed but digital signature is in 
+ DBX.\n"));
   break;
 }
 
@@ -2426,11 +2433,13 @@ DxeImageVerificationHandler (
 //
 if (IsSignatureFoundInDatabase (EFI_IMAGE_SECURITY_DATABASE1, 
mImageDigest, , mImageDigestSize)) {
   Action = EFI_IMAGE_EXECUTION_AUTH_SIG_FOUND;
+  DEBUG ((DEBUG_ERROR, "Image is signed but digital signature 
+ failed validation and hash is in DBX.\n"));
   VerifyStatus = EFI_ACCESS_DENIED;
   break;
 } else if (EFI_ERROR (VerifyStatus)) {
   if (IsSignatureFoundInDatabase (EFI_IMAGE_SECURITY_DATABASE, 
mImageDigest, , mImageDigestSize)) {
 VerifyStatus = EFI_SUCCESS;
+DEBUG ((DEBUG_ERROR, "Image is signed but digital signature 
+ failed validation and hash is not found in DB/DBX.\n"));
   }
 }
   }
--
2.6.3.windows.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH] SecurityPkg : Add DEBUG messages in image verification

2015-12-21 Thread Paulo Alcantara
On Dec 21, 2015 9:02 PM, "Samer El-Haj-Mahmoud" <
samer.el-haj-mahm...@hpe.com> wrote:
>
> Add DEBUG messages in DxeImageerificationLib to help debug
> Secure Boot image verification failures
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Samer El-Haj-Mahmoud 
> ---
>  .../Library/DxeImageVerificationLib/DxeImageVerificationLib.c| 9
+
>  1 file changed, 9 insertions(+)
>
> diff --git
a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
> index 4b4d3bf..888cd04 100644
> ---
a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
> +++
b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.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.
> +(C) Copyright 2015 Hewlett Packard Enterprise Development LP
>  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
> @@ -2259,6 +2260,7 @@ DxeImageVerificationHandler (
>  //
>  // The information can't be got from the invalid PeImage
>  //
> +DEBUG ((DEBUG_ERROR, "PeImage invald. Cannot retrieve image
information.\n"));

s/invald/invalid/

Paulo

--
Paulo Alcantara, HP Inc.
Speaking for myself only.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel