Re: [edk2] [patch] SecurityPkg: enhance secure boot Config Dxe & Time Based AuthVariable.

2017-02-17 Thread Zhang, Chao B 
Reviewed-by: Chao Zhang 

-Original Message-
From: Zhang, Lubo 
Sent: Friday, February 17, 2017 3:05 PM
To: edk2-devel@lists.01.org
Cc: Zhang, Chao B ; Long, Qin ; 
Yao, Jiewen 
Subject: [patch] SecurityPkg: enhance secure boot Config Dxe & Time Based 
AuthVariable.

V3: code clean up

prohibit Image SHA-1 hash option in SecureBootConfigDxe.
Timebased Auth Variable driver should ensure AuthAlgorithm is SHA256 before 
further verification

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Zhang Lubo 
Cc: Chao Zhang 
Cc: Long Qin 
Cc: Yao Jiewen 
---
 SecurityPkg/Library/AuthVariableLib/AuthService.c  | 27 +- 
 .../Library/AuthVariableLib/AuthServiceInternal.h  |  4 +++-
 .../SecureBootConfigDxe/SecureBootConfigImpl.c | 14 ---
 .../SecureBootConfigDxe/SecureBootConfigImpl.h |  8 ++-
 4 files changed, 35 insertions(+), 18 deletions(-)

diff --git a/SecurityPkg/Library/AuthVariableLib/AuthService.c 
b/SecurityPkg/Library/AuthVariableLib/AuthService.c
index b013d42..1cdfadc 100644
--- a/SecurityPkg/Library/AuthVariableLib/AuthService.c
+++ b/SecurityPkg/Library/AuthVariableLib/AuthService.c
@@ -16,11 +16,11 @@
 
   VerifyTimeBasedPayloadAndUpdate() and VerifyCounterBasedPayload() are sub 
function to do verification.
   They will do basic validation for authentication data structure, then call 
crypto library
   to verify the signature.
 
-Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.
+Copyright (c) 2009 - 2017, Intel Corporation. All rights reserved.
 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  
http://opensource.org/licenses/bsd-license.php
 
@@ -34,10 +34,12 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 //
 // Public Exponent of RSA Key.
 //
 CONST UINT8 mRsaE[] = { 0x01, 0x00, 0x01 };
 
+CONST UINT8 mHash256OidValue[] = { 0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 
+0x04, 0x02, 0x01 };
+
 //
 // Requirement for different signature type which have been defined in UEFI 
spec.
 // These data are used to perform SignatureList format check while setting 
PK/KEK variable.
 //
 EFI_SIGNATURE_ITEM mSupportSigItem[] = { @@ -2243,10 +2245,33 @@ 
VerifyTimeBasedPayload (
   //
   SigData = CertData->AuthInfo.CertData;
   SigDataSize = CertData->AuthInfo.Hdr.dwLength - (UINT32) (OFFSET_OF 
(WIN_CERTIFICATE_UEFI_GUID, CertData));
 
   //
+  // SignedData.digestAlgorithms shall contain the digest algorithm 
+ used when preparing the  // signature. Only a digest algorithm of SHA-256 is 
accepted.
+  //
+  //According to PKCS#7 Definition:
+  //SignedData ::= SEQUENCE {
+  //version Version,
+  //digestAlgorithms DigestAlgorithmIdentifiers,
+  //contentInfo ContentInfo,
+  // }
+  //The DigestAlgorithmIdentifiers can be used to determine the hash 
algorithm 
+  //in VARIABLE_AUTHENTICATION_2 descriptor.
+  //This field has the fixed offset (+13) and be calculated based on two 
bytes of length encoding.
+  //
+  if ((Attributes & EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS) != 0) {
+if (SigDataSize >= (13 + sizeof (mHash256OidValue))) {
+  if (((*(SigData + 1) & TWO_BYTE_ENCODE) != TWO_BYTE_ENCODE) || 
+   (CompareMem (SigData + 13, , sizeof 
(mHash256OidValue)) != 0)) {
+  return EFI_SECURITY_VIOLATION;
+}
+}
+  }
+
+  //
   // Find out the new data payload which follows Pkcs7 SignedData directly.
   //
   PayloadPtr = SigData + SigDataSize;
   PayloadSize = DataSize - OFFSET_OF_AUTHINFO2_CERT_DATA - (UINTN) SigDataSize;
 
diff --git a/SecurityPkg/Library/AuthVariableLib/AuthServiceInternal.h 
b/SecurityPkg/Library/AuthVariableLib/AuthServiceInternal.h
index e7c4bf0..e9b7cf3 100644
--- a/SecurityPkg/Library/AuthVariableLib/AuthServiceInternal.h
+++ b/SecurityPkg/Library/AuthVariableLib/AuthServiceInternal.h
@@ -10,11 +10,11 @@
  The whole SMM authentication variable design relies on the integrity of 
flash part and SMM.
   which is assumed to be protected by platform.  All variable code and 
metadata in flash/SMM Memory
   may not be modified without authorization. If platform fails to protect 
these resources,
   the authentication service provided in this driver will be broken, and the 
behavior is undefined.
 
-Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.
+Copyright (c) 2009 - 2017, Intel Corporation. All rights reserved.
 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

Re: [edk2] [patch] SecurityPkg: enhance secure boot Config Dxe & Time Based AuthVariable.

2017-02-16 Thread Long, Qin 
Hi, Lubo,

Please use "mSha256OidValue", instead of "mHash256OidValue", to identify the 
SHA-256 cipher name.

Reviewed-by: Long Qin 


Best Regards & Thanks,
LONG, Qin

> -Original Message-
> From: Zhang, Lubo
> Sent: Thursday, February 16, 2017 11:05 PM
> To: edk2-devel@lists.01.org
> Cc: Zhang, Chao B ; Long, Qin
> ; Yao, Jiewen 
> Subject: [patch] SecurityPkg: enhance secure boot Config Dxe & Time Based
> AuthVariable.
> 
> V3: code clean up
> 
> prohibit Image SHA-1 hash option in SecureBootConfigDxe.
> Timebased Auth Variable driver should ensure AuthAlgorithm is SHA256
> before further verification
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Zhang Lubo 
> Cc: Chao Zhang 
> Cc: Long Qin 
> Cc: Yao Jiewen 
> ---
>  SecurityPkg/Library/AuthVariableLib/AuthService.c  | 27
> +-
>   .../Library/AuthVariableLib/AuthServiceInternal.h  |  4 +++-
>  .../SecureBootConfigDxe/SecureBootConfigImpl.c | 14 ---
>  .../SecureBootConfigDxe/SecureBootConfigImpl.h |  8 ++-
>  4 files changed, 35 insertions(+), 18 deletions(-)
> 
> diff --git a/SecurityPkg/Library/AuthVariableLib/AuthService.c
> b/SecurityPkg/Library/AuthVariableLib/AuthService.c
> index b013d42..1cdfadc 100644
> --- a/SecurityPkg/Library/AuthVariableLib/AuthService.c
> +++ b/SecurityPkg/Library/AuthVariableLib/AuthService.c
> @@ -16,11 +16,11 @@
> 
>VerifyTimeBasedPayloadAndUpdate() and VerifyCounterBasedPayload()
> are sub function to do verification.
>They will do basic validation for authentication data structure, then call
> crypto library
>to verify the signature.
> 
> -Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.
> +Copyright (c) 2009 - 2017, Intel Corporation. All rights reserved.
>  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
> http://opensource.org/licenses/bsd-license.php
> 
> @@ -34,10 +34,12 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF
> ANY KIND, EITHER EXPRESS OR IMPLIED.
>  //
>  // Public Exponent of RSA Key.
>  //
>  CONST UINT8 mRsaE[] = { 0x01, 0x00, 0x01 };
> 
> +CONST UINT8 mHash256OidValue[] = { 0x60, 0x86, 0x48, 0x01, 0x65, 0x03,
> +0x04, 0x02, 0x01 };
> +
>  //
>  // Requirement for different signature type which have been defined in
> UEFI spec.
>  // These data are used to perform SignatureList format check while setting
> PK/KEK variable.
>  //
>  EFI_SIGNATURE_ITEM mSupportSigItem[] = { @@ -2243,10 +2245,33 @@
> VerifyTimeBasedPayload (
>//
>SigData = CertData->AuthInfo.CertData;
>SigDataSize = CertData->AuthInfo.Hdr.dwLength - (UINT32) (OFFSET_OF
> (WIN_CERTIFICATE_UEFI_GUID, CertData));
> 
>//
> +  // SignedData.digestAlgorithms shall contain the digest algorithm
> + used when preparing the  // signature. Only a digest algorithm of SHA-256
> is accepted.
> +  //
> +  //According to PKCS#7 Definition:
> +  //SignedData ::= SEQUENCE {
> +  //version Version,
> +  //digestAlgorithms DigestAlgorithmIdentifiers,
> +  //contentInfo ContentInfo,
> +  // }
> +  //The DigestAlgorithmIdentifiers can be used to determine the hash
> algorithm
> +  //in VARIABLE_AUTHENTICATION_2 descriptor.
> +  //This field has the fixed offset (+13) and be calculated based on two
> bytes of length encoding.
> +  //
> +  if ((Attributes &
> EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS) != 0) {
> +if (SigDataSize >= (13 + sizeof (mHash256OidValue))) {
> +  if (((*(SigData + 1) & TWO_BYTE_ENCODE) != TWO_BYTE_ENCODE) ||
> +   (CompareMem (SigData + 13, , sizeof
> (mHash256OidValue)) != 0)) {
> +  return EFI_SECURITY_VIOLATION;
> +}
> +}
> +  }
> +
> +  //
>// Find out the new data payload which follows Pkcs7 SignedData directly.
>//
>PayloadPtr = SigData + SigDataSize;
>PayloadSize = DataSize - OFFSET_OF_AUTHINFO2_CERT_DATA - (UINTN)
> SigDataSize;
> 
> diff --git a/SecurityPkg/Library/AuthVariableLib/AuthServiceInternal.h
> b/SecurityPkg/Library/AuthVariableLib/AuthServiceInternal.h
> index e7c4bf0..e9b7cf3 100644
> --- a/SecurityPkg/Library/AuthVariableLib/AuthServiceInternal.h
> +++ b/SecurityPkg/Library/AuthVariableLib/AuthServiceInternal.h
> @@ -10,11 +10,11 @@
>   The whole SMM authentication variable design relies on the integrity of
> flash part and SMM.
>which is assumed to be protected by platform.  All variable code and
> metadata in flash/SMM Memory
>may not be modified without authorization. If platform fails to protect
> these resources,
>the authentication service provided in this driver will be broken, and the
> behavior is

[edk2] [patch] SecurityPkg: enhance secure boot Config Dxe & Time Based AuthVariable.

2017-02-16 Thread Zhang Lubo 
V3: code clean up

prohibit Image SHA-1 hash option in SecureBootConfigDxe.
Timebased Auth Variable driver should ensure AuthAlgorithm
is SHA256 before further verification

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Zhang Lubo 
Cc: Chao Zhang 
Cc: Long Qin 
Cc: Yao Jiewen 
---
 SecurityPkg/Library/AuthVariableLib/AuthService.c  | 27 +-
 .../Library/AuthVariableLib/AuthServiceInternal.h  |  4 +++-
 .../SecureBootConfigDxe/SecureBootConfigImpl.c | 14 ---
 .../SecureBootConfigDxe/SecureBootConfigImpl.h |  8 ++-
 4 files changed, 35 insertions(+), 18 deletions(-)

diff --git a/SecurityPkg/Library/AuthVariableLib/AuthService.c 
b/SecurityPkg/Library/AuthVariableLib/AuthService.c
index b013d42..1cdfadc 100644
--- a/SecurityPkg/Library/AuthVariableLib/AuthService.c
+++ b/SecurityPkg/Library/AuthVariableLib/AuthService.c
@@ -16,11 +16,11 @@
 
   VerifyTimeBasedPayloadAndUpdate() and VerifyCounterBasedPayload() are sub 
function to do verification.
   They will do basic validation for authentication data structure, then call 
crypto library
   to verify the signature.
 
-Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.
+Copyright (c) 2009 - 2017, Intel Corporation. All rights reserved.
 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
 http://opensource.org/licenses/bsd-license.php
 
@@ -34,10 +34,12 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 //
 // Public Exponent of RSA Key.
 //
 CONST UINT8 mRsaE[] = { 0x01, 0x00, 0x01 };
 
+CONST UINT8 mHash256OidValue[] = { 0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 
0x02, 0x01 };
+
 //
 // Requirement for different signature type which have been defined in UEFI 
spec.
 // These data are used to perform SignatureList format check while setting 
PK/KEK variable.
 //
 EFI_SIGNATURE_ITEM mSupportSigItem[] = {
@@ -2243,10 +2245,33 @@ VerifyTimeBasedPayload (
   //
   SigData = CertData->AuthInfo.CertData;
   SigDataSize = CertData->AuthInfo.Hdr.dwLength - (UINT32) (OFFSET_OF 
(WIN_CERTIFICATE_UEFI_GUID, CertData));
 
   //
+  // SignedData.digestAlgorithms shall contain the digest algorithm used when 
preparing the
+  // signature. Only a digest algorithm of SHA-256 is accepted.
+  //
+  //According to PKCS#7 Definition:
+  //SignedData ::= SEQUENCE {
+  //version Version,
+  //digestAlgorithms DigestAlgorithmIdentifiers,
+  //contentInfo ContentInfo,
+  // }
+  //The DigestAlgorithmIdentifiers can be used to determine the hash 
algorithm 
+  //in VARIABLE_AUTHENTICATION_2 descriptor.
+  //This field has the fixed offset (+13) and be calculated based on two 
bytes of length encoding.
+  //
+  if ((Attributes & EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS) != 0) {
+if (SigDataSize >= (13 + sizeof (mHash256OidValue))) {
+  if (((*(SigData + 1) & TWO_BYTE_ENCODE) != TWO_BYTE_ENCODE) || 
+   (CompareMem (SigData + 13, , sizeof 
(mHash256OidValue)) != 0)) {
+  return EFI_SECURITY_VIOLATION;
+}
+}
+  }
+
+  //
   // Find out the new data payload which follows Pkcs7 SignedData directly.
   //
   PayloadPtr = SigData + SigDataSize;
   PayloadSize = DataSize - OFFSET_OF_AUTHINFO2_CERT_DATA - (UINTN) SigDataSize;
 
diff --git a/SecurityPkg/Library/AuthVariableLib/AuthServiceInternal.h 
b/SecurityPkg/Library/AuthVariableLib/AuthServiceInternal.h
index e7c4bf0..e9b7cf3 100644
--- a/SecurityPkg/Library/AuthVariableLib/AuthServiceInternal.h
+++ b/SecurityPkg/Library/AuthVariableLib/AuthServiceInternal.h
@@ -10,11 +10,11 @@
  The whole SMM authentication variable design relies on the integrity of 
flash part and SMM.
   which is assumed to be protected by platform.  All variable code and 
metadata in flash/SMM Memory
   may not be modified without authorization. If platform fails to protect 
these resources,
   the authentication service provided in this driver will be broken, and the 
behavior is undefined.
 
-Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.
+Copyright (c) 2009 - 2017, Intel Corporation. All rights reserved.
 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
 http://opensource.org/licenses/bsd-license.php
 
@@ -35,10 +35,12 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 #include 
 
 #include 
 #include 
 
+#define TWO_BYTE_ENCODE   0x82
+
 ///
 /// Struct to record signature requirement defined by UEFI spec.
 /// For SigHeaderSize and SigDataSize, ((UINT32) ~0) means NO

Re: [edk2] [patch] SecurityPkg: enhance secure boot Config Dxe & Time Based AuthVariable. 1. prohibit Image SHA-1 hash option in SecureBootConfigDxe. 2. Timebased Auth Variable driver should ensure Au

2017-02-15 Thread Zhang, Lubo 
Thanks for your comments, I will update the patch.

Lubo

-Original Message-
From: Zhang, Chao B 
Sent: Thursday, February 16, 2017 1:12 PM
To: Zhang, Lubo ; edk2-devel@lists.01.org
Cc: Long, Qin ; Yao, Jiewen 
Subject: RE: [patch] SecurityPkg: enhance secure boot Config Dxe & Time Based 
AuthVariable. 1. prohibit Image SHA-1 hash option in SecureBootConfigDxe. 2. 
Timebased Auth Variable driver should ensure AuthAlgorithm is SHA256 before 
further verification

Lubo:
   1. Please capitalize Hash256OidLength macro definition or simply use 
sizeof(mHash256OidValue). Others are good to me.
   2. Please run PatchCheck first. The title is too long to meet EDKII check-in 
log requirement
   

-Original Message-
From: Zhang, Lubo
Sent: Tuesday, February 14, 2017 10:56 AM
To: edk2-devel@lists.01.org
Cc: Zhang, Chao B ; Long, Qin ; 
Yao, Jiewen 
Subject: [patch] SecurityPkg: enhance secure boot Config Dxe & Time Based 
AuthVariable. 1. prohibit Image SHA-1 hash option in SecureBootConfigDxe. 2. 
Timebased Auth Variable driver should ensure AuthAlgorithm is SHA256 before 
further verification

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Zhang Lubo 
Cc: Chao Zhang 
Cc: Long Qin 
Cc: Yao Jiewen 
---
 SecurityPkg/Library/AuthVariableLib/AuthService.c  | 27 +- 
 .../Library/AuthVariableLib/AuthServiceInternal.h  |  5 +++-
 .../SecureBootConfigDxe/SecureBootConfigImpl.c | 14 ---
 .../SecureBootConfigDxe/SecureBootConfigImpl.h |  8 ++-
 4 files changed, 36 insertions(+), 18 deletions(-)

diff --git a/SecurityPkg/Library/AuthVariableLib/AuthService.c 
b/SecurityPkg/Library/AuthVariableLib/AuthService.c
index b013d42..75d2290 100644
--- a/SecurityPkg/Library/AuthVariableLib/AuthService.c
+++ b/SecurityPkg/Library/AuthVariableLib/AuthService.c
@@ -16,11 +16,11 @@
 
   VerifyTimeBasedPayloadAndUpdate() and VerifyCounterBasedPayload() are sub 
function to do verification.
   They will do basic validation for authentication data structure, then call 
crypto library
   to verify the signature.
 
-Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.
+Copyright (c) 2009 - 2017, Intel Corporation. All rights reserved.
 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  
http://opensource.org/licenses/bsd-license.php
 
@@ -34,10 +34,12 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 //
 // Public Exponent of RSA Key.
 //
 CONST UINT8 mRsaE[] = { 0x01, 0x00, 0x01 };
 
+CONST UINT8 mHash256OidValue[] = { 0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 
+0x04, 0x02, 0x01 };
+
 //
 // Requirement for different signature type which have been defined in UEFI 
spec.
 // These data are used to perform SignatureList format check while setting 
PK/KEK variable.
 //
 EFI_SIGNATURE_ITEM mSupportSigItem[] = { @@ -2243,10 +2245,33 @@ 
VerifyTimeBasedPayload (
   //
   SigData = CertData->AuthInfo.CertData;
   SigDataSize = CertData->AuthInfo.Hdr.dwLength - (UINT32) (OFFSET_OF 
(WIN_CERTIFICATE_UEFI_GUID, CertData));
 
   //
+  // SignedData.digestAlgorithms shall contain the digest algorithm 
+ used when preparing the  // signature. Only a digest algorithm of SHA-256 is 
accepted.
+  //
+  //According to PKCS#7 Definition:
+  //SignedData ::= SEQUENCE {
+  //version Version,
+  //digestAlgorithms DigestAlgorithmIdentifiers,
+  //contentInfo ContentInfo,
+  // }
+  //The DigestAlgorithmIdentifiers can be used to determine the hash 
algorithm 
+  //in VARIABLE_AUTHENTICATION_2 descriptor.
+  //This field has the fixed offset (+13) and be calculated based on two 
bytes of length encoding.
+  //
+  if ((Attributes & EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS) != 0) {
+if (SigDataSize >= (13 + Hash256OidLength)) {
+  if (((*(SigData + 1) & TWO_BYTE_ENCODE) != TWO_BYTE_ENCODE) || 
+   (CompareMem (SigData + 13, , Hash256OidLength) != 
0)) {
+  return EFI_SECURITY_VIOLATION;
+}
+}
+  }
+
+  //
   // Find out the new data payload which follows Pkcs7 SignedData directly.
   //
   PayloadPtr = SigData + SigDataSize;
   PayloadSize = DataSize - OFFSET_OF_AUTHINFO2_CERT_DATA - (UINTN) SigDataSize;
 
diff --git a/SecurityPkg/Library/AuthVariableLib/AuthServiceInternal.h 
b/SecurityPkg/Library/AuthVariableLib/AuthServiceInternal.h
index e7c4bf0..f1cf591 100644
--- a/SecurityPkg/Library/AuthVariableLib/AuthServiceInternal.h
+++ b/SecurityPkg/Library/AuthVariableLib/AuthServiceInternal.h
@@ -10,11 +10,11 @@
  The whole SMM authentication variable

Re: [edk2] [patch] SecurityPkg: enhance secure boot Config Dxe & Time Based AuthVariable. 1. prohibit Image SHA-1 hash option in SecureBootConfigDxe. 2. Timebased Auth Variable driver should ensure Au

2017-02-15 Thread Zhang, Chao B 
Lubo:
   1. Please capitalize Hash256OidLength macro definition or simply use 
sizeof(mHash256OidValue). Others are good to me.
   2. Please run PatchCheck first. The title is too long to meet EDKII check-in 
log requirement
   

-Original Message-
From: Zhang, Lubo 
Sent: Tuesday, February 14, 2017 10:56 AM
To: edk2-devel@lists.01.org
Cc: Zhang, Chao B ; Long, Qin ; 
Yao, Jiewen 
Subject: [patch] SecurityPkg: enhance secure boot Config Dxe & Time Based 
AuthVariable. 1. prohibit Image SHA-1 hash option in SecureBootConfigDxe. 2. 
Timebased Auth Variable driver should ensure AuthAlgorithm is SHA256 before 
further verification

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Zhang Lubo 
Cc: Chao Zhang 
Cc: Long Qin 
Cc: Yao Jiewen 
---
 SecurityPkg/Library/AuthVariableLib/AuthService.c  | 27 +- 
 .../Library/AuthVariableLib/AuthServiceInternal.h  |  5 +++-
 .../SecureBootConfigDxe/SecureBootConfigImpl.c | 14 ---
 .../SecureBootConfigDxe/SecureBootConfigImpl.h |  8 ++-
 4 files changed, 36 insertions(+), 18 deletions(-)

diff --git a/SecurityPkg/Library/AuthVariableLib/AuthService.c 
b/SecurityPkg/Library/AuthVariableLib/AuthService.c
index b013d42..75d2290 100644
--- a/SecurityPkg/Library/AuthVariableLib/AuthService.c
+++ b/SecurityPkg/Library/AuthVariableLib/AuthService.c
@@ -16,11 +16,11 @@
 
   VerifyTimeBasedPayloadAndUpdate() and VerifyCounterBasedPayload() are sub 
function to do verification.
   They will do basic validation for authentication data structure, then call 
crypto library
   to verify the signature.
 
-Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.
+Copyright (c) 2009 - 2017, Intel Corporation. All rights reserved.
 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  
http://opensource.org/licenses/bsd-license.php
 
@@ -34,10 +34,12 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 //
 // Public Exponent of RSA Key.
 //
 CONST UINT8 mRsaE[] = { 0x01, 0x00, 0x01 };
 
+CONST UINT8 mHash256OidValue[] = { 0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 
+0x04, 0x02, 0x01 };
+
 //
 // Requirement for different signature type which have been defined in UEFI 
spec.
 // These data are used to perform SignatureList format check while setting 
PK/KEK variable.
 //
 EFI_SIGNATURE_ITEM mSupportSigItem[] = { @@ -2243,10 +2245,33 @@ 
VerifyTimeBasedPayload (
   //
   SigData = CertData->AuthInfo.CertData;
   SigDataSize = CertData->AuthInfo.Hdr.dwLength - (UINT32) (OFFSET_OF 
(WIN_CERTIFICATE_UEFI_GUID, CertData));
 
   //
+  // SignedData.digestAlgorithms shall contain the digest algorithm 
+ used when preparing the  // signature. Only a digest algorithm of SHA-256 is 
accepted.
+  //
+  //According to PKCS#7 Definition:
+  //SignedData ::= SEQUENCE {
+  //version Version,
+  //digestAlgorithms DigestAlgorithmIdentifiers,
+  //contentInfo ContentInfo,
+  // }
+  //The DigestAlgorithmIdentifiers can be used to determine the hash 
algorithm 
+  //in VARIABLE_AUTHENTICATION_2 descriptor.
+  //This field has the fixed offset (+13) and be calculated based on two 
bytes of length encoding.
+  //
+  if ((Attributes & EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS) != 0) {
+if (SigDataSize >= (13 + Hash256OidLength)) {
+  if (((*(SigData + 1) & TWO_BYTE_ENCODE) != TWO_BYTE_ENCODE) || 
+   (CompareMem (SigData + 13, , Hash256OidLength) != 
0)) {
+  return EFI_SECURITY_VIOLATION;
+}
+}
+  }
+
+  //
   // Find out the new data payload which follows Pkcs7 SignedData directly.
   //
   PayloadPtr = SigData + SigDataSize;
   PayloadSize = DataSize - OFFSET_OF_AUTHINFO2_CERT_DATA - (UINTN) SigDataSize;
 
diff --git a/SecurityPkg/Library/AuthVariableLib/AuthServiceInternal.h 
b/SecurityPkg/Library/AuthVariableLib/AuthServiceInternal.h
index e7c4bf0..f1cf591 100644
--- a/SecurityPkg/Library/AuthVariableLib/AuthServiceInternal.h
+++ b/SecurityPkg/Library/AuthVariableLib/AuthServiceInternal.h
@@ -10,11 +10,11 @@
  The whole SMM authentication variable design relies on the integrity of 
flash part and SMM.
   which is assumed to be protected by platform.  All variable code and 
metadata in flash/SMM Memory
   may not be modified without authorization. If platform fails to protect 
these resources,
   the authentication service provided in this driver will be broken, and the 
behavior is undefined.
 
-Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.
+Copyright (c) 2009 - 2017, Intel Corporation. All rights reserved.
 This program and the accompanying materials

[edk2] [patch] SecurityPkg: enhance secure boot Config Dxe & Time Based AuthVariable. 1. prohibit Image SHA-1 hash option in SecureBootConfigDxe. 2. Timebased Auth Variable driver should ensure AuthAl

2017-02-13 Thread Zhang Lubo 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Zhang Lubo 
Cc: Chao Zhang 
Cc: Long Qin 
Cc: Yao Jiewen 
---
 SecurityPkg/Library/AuthVariableLib/AuthService.c  | 27 +-
 .../Library/AuthVariableLib/AuthServiceInternal.h  |  5 +++-
 .../SecureBootConfigDxe/SecureBootConfigImpl.c | 14 ---
 .../SecureBootConfigDxe/SecureBootConfigImpl.h |  8 ++-
 4 files changed, 36 insertions(+), 18 deletions(-)

diff --git a/SecurityPkg/Library/AuthVariableLib/AuthService.c 
b/SecurityPkg/Library/AuthVariableLib/AuthService.c
index b013d42..75d2290 100644
--- a/SecurityPkg/Library/AuthVariableLib/AuthService.c
+++ b/SecurityPkg/Library/AuthVariableLib/AuthService.c
@@ -16,11 +16,11 @@
 
   VerifyTimeBasedPayloadAndUpdate() and VerifyCounterBasedPayload() are sub 
function to do verification.
   They will do basic validation for authentication data structure, then call 
crypto library
   to verify the signature.
 
-Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.
+Copyright (c) 2009 - 2017, Intel Corporation. All rights reserved.
 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
 http://opensource.org/licenses/bsd-license.php
 
@@ -34,10 +34,12 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 //
 // Public Exponent of RSA Key.
 //
 CONST UINT8 mRsaE[] = { 0x01, 0x00, 0x01 };
 
+CONST UINT8 mHash256OidValue[] = { 0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 
0x02, 0x01 };
+
 //
 // Requirement for different signature type which have been defined in UEFI 
spec.
 // These data are used to perform SignatureList format check while setting 
PK/KEK variable.
 //
 EFI_SIGNATURE_ITEM mSupportSigItem[] = {
@@ -2243,10 +2245,33 @@ VerifyTimeBasedPayload (
   //
   SigData = CertData->AuthInfo.CertData;
   SigDataSize = CertData->AuthInfo.Hdr.dwLength - (UINT32) (OFFSET_OF 
(WIN_CERTIFICATE_UEFI_GUID, CertData));
 
   //
+  // SignedData.digestAlgorithms shall contain the digest algorithm used when 
preparing the
+  // signature. Only a digest algorithm of SHA-256 is accepted.
+  //
+  //According to PKCS#7 Definition:
+  //SignedData ::= SEQUENCE {
+  //version Version,
+  //digestAlgorithms DigestAlgorithmIdentifiers,
+  //contentInfo ContentInfo,
+  // }
+  //The DigestAlgorithmIdentifiers can be used to determine the hash 
algorithm 
+  //in VARIABLE_AUTHENTICATION_2 descriptor.
+  //This field has the fixed offset (+13) and be calculated based on two 
bytes of length encoding.
+  //
+  if ((Attributes & EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS) != 0) {
+if (SigDataSize >= (13 + Hash256OidLength)) {
+  if (((*(SigData + 1) & TWO_BYTE_ENCODE) != TWO_BYTE_ENCODE) || 
+   (CompareMem (SigData + 13, , Hash256OidLength) != 
0)) {
+  return EFI_SECURITY_VIOLATION;
+}
+}
+  }
+
+  //
   // Find out the new data payload which follows Pkcs7 SignedData directly.
   //
   PayloadPtr = SigData + SigDataSize;
   PayloadSize = DataSize - OFFSET_OF_AUTHINFO2_CERT_DATA - (UINTN) SigDataSize;
 
diff --git a/SecurityPkg/Library/AuthVariableLib/AuthServiceInternal.h 
b/SecurityPkg/Library/AuthVariableLib/AuthServiceInternal.h
index e7c4bf0..f1cf591 100644
--- a/SecurityPkg/Library/AuthVariableLib/AuthServiceInternal.h
+++ b/SecurityPkg/Library/AuthVariableLib/AuthServiceInternal.h
@@ -10,11 +10,11 @@
  The whole SMM authentication variable design relies on the integrity of 
flash part and SMM.
   which is assumed to be protected by platform.  All variable code and 
metadata in flash/SMM Memory
   may not be modified without authorization. If platform fails to protect 
these resources,
   the authentication service provided in this driver will be broken, and the 
behavior is undefined.
 
-Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.
+Copyright (c) 2009 - 2017, Intel Corporation. All rights reserved.
 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
 http://opensource.org/licenses/bsd-license.php
 
@@ -35,10 +35,13 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 #include 
 
 #include 
 #include 
 
+#define TWO_BYTE_ENCODE   0x82
+#define Hash256OidLength  9
+
 ///
 /// Struct to record signature requirement defined by UEFI spec.
 /// For SigHeaderSize and SigDataSize, ((UINT32) ~0) means NO exact length 
requirement for this field.
 ///
 typedef struct {
diff --git 
a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c