public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [patch] SecurityPkg: enhance secure boot Config Dxe & Time Based AuthVariable.
@ 2017-02-17  7:04 Zhang Lubo
  2017-02-17  7:42 ` Long, Qin
  2017-02-17  8:51 ` Zhang, Chao B
  0 siblings, 2 replies; 3+ messages in thread
From: Zhang Lubo @ 2017-02-17  7:04 UTC (permalink / raw)
  To: edk2-devel; +Cc: Chao Zhang, Long Qin, Yao Jiewen

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 <lubo.zhang@intel.com>
Cc: Chao Zhang <chao.b.zhang@intel.com>
Cc: Long Qin <qin.long@intel.com>
Cc: Yao Jiewen <jiewen.yao@intel.com>
---
 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.<BR>
+Copyright (c) 2009 - 2017, Intel Corporation. All rights reserved.<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
 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, &mHash256OidValue, 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.<BR>
+Copyright (c) 2009 - 2017, Intel Corporation. All rights reserved.<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
 http://opensource.org/licenses/bsd-license.php
 
@@ -35,10 +35,12 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 #include <Library/PlatformSecureLib.h>
 
 #include <Guid/AuthenticatedVariableFormat.h>
 #include <Guid/ImageAuthentication.h>
 
+#define TWO_BYTE_ENCODE       0x82
+
 ///
 /// 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 b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c
index 0d96185..6f58729 100644
--- a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c
+++ b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c
@@ -1,9 +1,9 @@
 /** @file
   HII Config Access protocol implementation of SecureBoot configuration module.
 
-Copyright (c) 2011 - 2016, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2011 - 2017, Intel Corporation. All rights reserved.<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
 http://opensource.org/licenses/bsd-license.php
 
@@ -61,11 +61,10 @@ UINT8 mHashOidValue[] = {
   0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x02,   // OBJ_sha384
   0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x03,   // OBJ_sha512
   };
 
 HASH_TABLE mHash[] = {
-  { L"SHA1",   20, &mHashOidValue[8],  5, Sha1GetContextSize,   Sha1Init,   Sha1Update,   Sha1Final  },
   { L"SHA224", 28, &mHashOidValue[13], 9, NULL,                 NULL,       NULL,         NULL       },
   { L"SHA256", 32, &mHashOidValue[22], 9, Sha256GetContextSize, Sha256Init, Sha256Update, Sha256Final},
   { L"SHA384", 48, &mHashOidValue[31], 9, Sha384GetContextSize, Sha384Init, Sha384Update, Sha384Final},
   { L"SHA512", 64, &mHashOidValue[40], 9, Sha512GetContextSize, Sha512Init, Sha512Update, Sha512Final}
 };
@@ -1784,26 +1783,21 @@ HashPeImage (
 
   HashCtx       = NULL;
   SectionHeader = NULL;
   Status        = FALSE;
 
-  if ((HashAlg != HASHALG_SHA1) && (HashAlg != HASHALG_SHA256)) {
+  if (HashAlg != HASHALG_SHA256) {
     return FALSE;
   }
 
   //
   // Initialize context of hash.
   //
   ZeroMem (mImageDigest, MAX_DIGEST_SIZE);
 
-  if (HashAlg == HASHALG_SHA1) {
-    mImageDigestSize  = SHA1_DIGEST_SIZE;
-    mCertType         = gEfiCertSha1Guid;
-  } else if (HashAlg == HASHALG_SHA256) {
-    mImageDigestSize  = SHA256_DIGEST_SIZE;
-    mCertType         = gEfiCertSha256Guid;
-  }
+  mImageDigestSize  = SHA256_DIGEST_SIZE;
+  mCertType         = gEfiCertSha256Guid;
 
   CtxSize   = mHash[HashAlg].GetContextSize();
 
   HashCtx = AllocatePool (CtxSize);
   ASSERT (HashCtx != NULL);
diff --git a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.h b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.h
index 5055a9e..bea9470 100644
--- a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.h
+++ b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.h
@@ -1,10 +1,10 @@
 /** @file
   The header file of HII Config Access protocol implementation of SecureBoot
   configuration module.
 
-Copyright (c) 2011 - 2016, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2011 - 2017, Intel Corporation. All rights reserved.<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
 http://opensource.org/licenses/bsd-license.php
 
@@ -65,14 +65,11 @@ extern  EFI_IFR_GUID_LABEL         *mStartLabel;
 extern  EFI_IFR_GUID_LABEL         *mEndLabel;
 
 #define MAX_CHAR              480
 #define TWO_BYTE_ENCODE       0x82
 
-//
-// SHA-1 digest size in bytes.
-//
-#define SHA1_DIGEST_SIZE    20
+
 //
 // SHA-256 digest size in bytes
 //
 #define SHA256_DIGEST_SIZE  32
 //
@@ -92,11 +89,10 @@ extern  EFI_IFR_GUID_LABEL         *mEndLabel;
 #define WIN_CERT_UEFI_RSA2048_SIZE               256
 
 //
 // Support hash types
 //
-#define HASHALG_SHA1                           0x00000000
 #define HASHALG_SHA224                         0x00000001
 #define HASHALG_SHA256                         0x00000002
 #define HASHALG_SHA384                         0x00000003
 #define HASHALG_SHA512                         0x00000004
 #define HASHALG_RAW                            0x00000005
-- 
1.9.5.msysgit.1



^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [patch] SecurityPkg: enhance secure boot Config Dxe & Time Based AuthVariable.
  2017-02-17  7:04 [patch] SecurityPkg: enhance secure boot Config Dxe & Time Based AuthVariable Zhang Lubo
@ 2017-02-17  7:42 ` Long, Qin
  2017-02-17  8:51 ` Zhang, Chao B
  1 sibling, 0 replies; 3+ messages in thread
From: Long, Qin @ 2017-02-17  7:42 UTC (permalink / raw)
  To: Zhang, Lubo, edk2-devel@lists.01.org; +Cc: Zhang, Chao B, Yao, Jiewen

Hi, Lubo,

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

Reviewed-by: Long Qin <qin.long@intel.com>


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 <chao.b.zhang@intel.com>; Long, Qin
> <qin.long@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>
> 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 <lubo.zhang@intel.com>
> Cc: Chao Zhang <chao.b.zhang@intel.com>
> Cc: Long Qin <qin.long@intel.com>
> Cc: Yao Jiewen <jiewen.yao@intel.com>
> ---
>  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.<BR>
> +Copyright (c) 2009 - 2017, Intel Corporation. All rights reserved.<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
> 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, &mHash256OidValue, 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.<BR>
> +Copyright (c) 2009 - 2017, Intel Corporation. All rights reserved.<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
> http://opensource.org/licenses/bsd-license.php
> 
> @@ -35,10 +35,12 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF
> ANY KIND, EITHER EXPRESS OR IMPLIED.
>  #include <Library/PlatformSecureLib.h>
> 
>  #include <Guid/AuthenticatedVariableFormat.h>
>  #include <Guid/ImageAuthentication.h>
> 
> +#define TWO_BYTE_ENCODE       0x82
> +
>  ///
>  /// 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/SecureBootCon
> figImpl.c
> b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootCon
> figImpl.c
> index 0d96185..6f58729 100644
> ---
> a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootCon
> figImpl.c
> +++
> b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootCo
> +++ nfigImpl.c
> @@ -1,9 +1,9 @@
>  /** @file
>    HII Config Access protocol implementation of SecureBoot configuration
> module.
> 
> -Copyright (c) 2011 - 2016, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2011 - 2017, Intel Corporation. All rights reserved.<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
> http://opensource.org/licenses/bsd-license.php
> 
> @@ -61,11 +61,10 @@ UINT8 mHashOidValue[] = {
>    0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x02,   // OBJ_sha384
>    0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x03,   // OBJ_sha512
>    };
> 
>  HASH_TABLE mHash[] = {
> -  { L"SHA1",   20, &mHashOidValue[8],  5, Sha1GetContextSize,   Sha1Init,
> Sha1Update,   Sha1Final  },
>    { L"SHA224", 28, &mHashOidValue[13], 9, NULL,                 NULL,       NULL,
> NULL       },
>    { L"SHA256", 32, &mHashOidValue[22], 9, Sha256GetContextSize,
> Sha256Init, Sha256Update, Sha256Final},
>    { L"SHA384", 48, &mHashOidValue[31], 9, Sha384GetContextSize,
> Sha384Init, Sha384Update, Sha384Final},
>    { L"SHA512", 64, &mHashOidValue[40], 9, Sha512GetContextSize,
> Sha512Init, Sha512Update, Sha512Final}  }; @@ -1784,26 +1783,21 @@
> HashPeImage (
> 
>    HashCtx       = NULL;
>    SectionHeader = NULL;
>    Status        = FALSE;
> 
> -  if ((HashAlg != HASHALG_SHA1) && (HashAlg != HASHALG_SHA256)) {
> +  if (HashAlg != HASHALG_SHA256) {
>      return FALSE;
>    }
> 
>    //
>    // Initialize context of hash.
>    //
>    ZeroMem (mImageDigest, MAX_DIGEST_SIZE);
> 
> -  if (HashAlg == HASHALG_SHA1) {
> -    mImageDigestSize  = SHA1_DIGEST_SIZE;
> -    mCertType         = gEfiCertSha1Guid;
> -  } else if (HashAlg == HASHALG_SHA256) {
> -    mImageDigestSize  = SHA256_DIGEST_SIZE;
> -    mCertType         = gEfiCertSha256Guid;
> -  }
> +  mImageDigestSize  = SHA256_DIGEST_SIZE;
> +  mCertType         = gEfiCertSha256Guid;
> 
>    CtxSize   = mHash[HashAlg].GetContextSize();
> 
>    HashCtx = AllocatePool (CtxSize);
>    ASSERT (HashCtx != NULL);
> diff --git
> a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootCon
> figImpl.h
> b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootCon
> figImpl.h
> index 5055a9e..bea9470 100644
> ---
> a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootCon
> figImpl.h
> +++
> b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootCo
> +++ nfigImpl.h
> @@ -1,10 +1,10 @@
>  /** @file
>    The header file of HII Config Access protocol implementation of SecureBoot
>    configuration module.
> 
> -Copyright (c) 2011 - 2016, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2011 - 2017, Intel Corporation. All rights reserved.<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
> http://opensource.org/licenses/bsd-license.php
> 
> @@ -65,14 +65,11 @@ extern  EFI_IFR_GUID_LABEL         *mStartLabel;
>  extern  EFI_IFR_GUID_LABEL         *mEndLabel;
> 
>  #define MAX_CHAR              480
>  #define TWO_BYTE_ENCODE       0x82
> 
> -//
> -// SHA-1 digest size in bytes.
> -//
> -#define SHA1_DIGEST_SIZE    20
> +
>  //
>  // SHA-256 digest size in bytes
>  //
>  #define SHA256_DIGEST_SIZE  32
>  //
> @@ -92,11 +89,10 @@ extern  EFI_IFR_GUID_LABEL         *mEndLabel;
>  #define WIN_CERT_UEFI_RSA2048_SIZE               256
> 
>  //
>  // Support hash types
>  //
> -#define HASHALG_SHA1                           0x00000000
>  #define HASHALG_SHA224                         0x00000001
>  #define HASHALG_SHA256                         0x00000002
>  #define HASHALG_SHA384                         0x00000003
>  #define HASHALG_SHA512                         0x00000004
>  #define HASHALG_RAW                            0x00000005
> --
> 1.9.5.msysgit.1



^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [patch] SecurityPkg: enhance secure boot Config Dxe & Time Based AuthVariable.
  2017-02-17  7:04 [patch] SecurityPkg: enhance secure boot Config Dxe & Time Based AuthVariable Zhang Lubo
  2017-02-17  7:42 ` Long, Qin
@ 2017-02-17  8:51 ` Zhang, Chao B
  1 sibling, 0 replies; 3+ messages in thread
From: Zhang, Chao B @ 2017-02-17  8:51 UTC (permalink / raw)
  To: Zhang, Lubo, edk2-devel@lists.01.org; +Cc: Long, Qin, Yao, Jiewen

Reviewed-by: Chao Zhang <chao.b.zhang@intel.com>

-----Original Message-----
From: Zhang, Lubo 
Sent: Friday, February 17, 2017 3:05 PM
To: edk2-devel@lists.01.org
Cc: Zhang, Chao B <chao.b.zhang@intel.com>; Long, Qin <qin.long@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>
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 <lubo.zhang@intel.com>
Cc: Chao Zhang <chao.b.zhang@intel.com>
Cc: Long Qin <qin.long@intel.com>
Cc: Yao Jiewen <jiewen.yao@intel.com>
---
 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.<BR>
+Copyright (c) 2009 - 2017, Intel Corporation. All rights reserved.<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  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, &mHash256OidValue, 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.<BR>
+Copyright (c) 2009 - 2017, Intel Corporation. All rights reserved.<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  http://opensource.org/licenses/bsd-license.php
 
@@ -35,10 +35,12 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 #include <Library/PlatformSecureLib.h>
 
 #include <Guid/AuthenticatedVariableFormat.h>
 #include <Guid/ImageAuthentication.h>
 
+#define TWO_BYTE_ENCODE       0x82
+
 ///
 /// 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 b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c
index 0d96185..6f58729 100644
--- a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c
+++ b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootCo
+++ nfigImpl.c
@@ -1,9 +1,9 @@
 /** @file
   HII Config Access protocol implementation of SecureBoot configuration module.
 
-Copyright (c) 2011 - 2016, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2011 - 2017, Intel Corporation. All rights reserved.<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  http://opensource.org/licenses/bsd-license.php
 
@@ -61,11 +61,10 @@ UINT8 mHashOidValue[] = {
   0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x02,   // OBJ_sha384
   0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x03,   // OBJ_sha512
   };
 
 HASH_TABLE mHash[] = {
-  { L"SHA1",   20, &mHashOidValue[8],  5, Sha1GetContextSize,   Sha1Init,   Sha1Update,   Sha1Final  },
   { L"SHA224", 28, &mHashOidValue[13], 9, NULL,                 NULL,       NULL,         NULL       },
   { L"SHA256", 32, &mHashOidValue[22], 9, Sha256GetContextSize, Sha256Init, Sha256Update, Sha256Final},
   { L"SHA384", 48, &mHashOidValue[31], 9, Sha384GetContextSize, Sha384Init, Sha384Update, Sha384Final},
   { L"SHA512", 64, &mHashOidValue[40], 9, Sha512GetContextSize, Sha512Init, Sha512Update, Sha512Final}  }; @@ -1784,26 +1783,21 @@ HashPeImage (
 
   HashCtx       = NULL;
   SectionHeader = NULL;
   Status        = FALSE;
 
-  if ((HashAlg != HASHALG_SHA1) && (HashAlg != HASHALG_SHA256)) {
+  if (HashAlg != HASHALG_SHA256) {
     return FALSE;
   }
 
   //
   // Initialize context of hash.
   //
   ZeroMem (mImageDigest, MAX_DIGEST_SIZE);
 
-  if (HashAlg == HASHALG_SHA1) {
-    mImageDigestSize  = SHA1_DIGEST_SIZE;
-    mCertType         = gEfiCertSha1Guid;
-  } else if (HashAlg == HASHALG_SHA256) {
-    mImageDigestSize  = SHA256_DIGEST_SIZE;
-    mCertType         = gEfiCertSha256Guid;
-  }
+  mImageDigestSize  = SHA256_DIGEST_SIZE;
+  mCertType         = gEfiCertSha256Guid;
 
   CtxSize   = mHash[HashAlg].GetContextSize();
 
   HashCtx = AllocatePool (CtxSize);
   ASSERT (HashCtx != NULL);
diff --git a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.h b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.h
index 5055a9e..bea9470 100644
--- a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.h
+++ b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootCo
+++ nfigImpl.h
@@ -1,10 +1,10 @@
 /** @file
   The header file of HII Config Access protocol implementation of SecureBoot
   configuration module.
 
-Copyright (c) 2011 - 2016, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2011 - 2017, Intel Corporation. All rights reserved.<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  http://opensource.org/licenses/bsd-license.php
 
@@ -65,14 +65,11 @@ extern  EFI_IFR_GUID_LABEL         *mStartLabel;
 extern  EFI_IFR_GUID_LABEL         *mEndLabel;
 
 #define MAX_CHAR              480
 #define TWO_BYTE_ENCODE       0x82
 
-//
-// SHA-1 digest size in bytes.
-//
-#define SHA1_DIGEST_SIZE    20
+
 //
 // SHA-256 digest size in bytes
 //
 #define SHA256_DIGEST_SIZE  32
 //
@@ -92,11 +89,10 @@ extern  EFI_IFR_GUID_LABEL         *mEndLabel;
 #define WIN_CERT_UEFI_RSA2048_SIZE               256
 
 //
 // Support hash types
 //
-#define HASHALG_SHA1                           0x00000000
 #define HASHALG_SHA224                         0x00000001
 #define HASHALG_SHA256                         0x00000002
 #define HASHALG_SHA384                         0x00000003
 #define HASHALG_SHA512                         0x00000004
 #define HASHALG_RAW                            0x00000005
--
1.9.5.msysgit.1



^ permalink raw reply related	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2017-02-17  8:51 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-02-17  7:04 [patch] SecurityPkg: enhance secure boot Config Dxe & Time Based AuthVariable Zhang Lubo
2017-02-17  7:42 ` Long, Qin
2017-02-17  8:51 ` Zhang, Chao B

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox