From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 16A7581F45 for ; Thu, 16 Feb 2017 23:42:59 -0800 (PST) Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga101.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 16 Feb 2017 23:42:58 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.35,171,1484035200"; d="scan'208";a="47519286" Received: from fmsmsx103.amr.corp.intel.com ([10.18.124.201]) by orsmga002.jf.intel.com with ESMTP; 16 Feb 2017 23:42:58 -0800 Received: from fmsmsx119.amr.corp.intel.com (10.18.124.207) by FMSMSX103.amr.corp.intel.com (10.18.124.201) with Microsoft SMTP Server (TLS) id 14.3.248.2; Thu, 16 Feb 2017 23:42:58 -0800 Received: from shsmsx102.ccr.corp.intel.com (10.239.4.154) by FMSMSX119.amr.corp.intel.com (10.18.124.207) with Microsoft SMTP Server (TLS) id 14.3.248.2; Thu, 16 Feb 2017 23:42:58 -0800 Received: from shsmsx103.ccr.corp.intel.com ([169.254.4.20]) by shsmsx102.ccr.corp.intel.com ([169.254.2.88]) with mapi id 14.03.0248.002; Fri, 17 Feb 2017 15:42:54 +0800 From: "Long, Qin" To: "Zhang, Lubo" , "edk2-devel@lists.01.org" CC: "Zhang, Chao B" , "Yao, Jiewen" Thread-Topic: [patch] SecurityPkg: enhance secure boot Config Dxe & Time Based AuthVariable. Thread-Index: AQHSiOwziIieIPqxaECmr30TMTud9KFsz45A Date: Fri, 17 Feb 2017 07:42:53 +0000 Message-ID: References: <1487315080-39500-1-git-send-email-lubo.zhang@intel.com> In-Reply-To: <1487315080-39500-1-git-send-email-lubo.zhang@intel.com> Accept-Language: zh-CN, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [patch] SecurityPkg: enhance secure boot Config Dxe & Time Based AuthVariable. X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 17 Feb 2017 07:42:59 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Hi, Lubo, Please use "mSha256OidValue", instead of "mHash256OidValue", to identify th= e 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. >=20 > V3: code clean up >=20 > prohibit Image SHA-1 hash option in SecureBootConfigDxe. > Timebased Auth Variable driver should ensure AuthAlgorithm is SHA256 > before further verification >=20 > 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(-) >=20 > 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 @@ >=20 > 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. >=20 > -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 >=20 > @@ -34,10 +34,12 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF > ANY KIND, EITHER EXPRESS OR IMPLIED. > // > // Public Exponent of RSA Key. > // > CONST UINT8 mRsaE[] =3D { 0x01, 0x00, 0x01 }; >=20 > +CONST UINT8 mHash256OidValue[] =3D { 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 setti= ng > PK/KEK variable. > // > EFI_SIGNATURE_ITEM mSupportSigItem[] =3D { @@ -2243,10 +2245,33 @@ > VerifyTimeBasedPayload ( > // > SigData =3D CertData->AuthInfo.CertData; > SigDataSize =3D CertData->AuthInfo.Hdr.dwLength - (UINT32) (OFFSET_OF > (WIN_CERTIFICATE_UEFI_GUID, CertData)); >=20 > // > + // SignedData.digestAlgorithms shall contain the digest algorithm > + used when preparing the // signature. Only a digest algorithm of SHA-2= 56 > is accepted. > + // > + // According to PKCS#7 Definition: > + // SignedData ::=3D 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) !=3D 0) { > + if (SigDataSize >=3D (13 + sizeof (mHash256OidValue))) { > + if (((*(SigData + 1) & TWO_BYTE_ENCODE) !=3D TWO_BYTE_ENCODE) || > + (CompareMem (SigData + 13, &mHash256OidValue, sizeof > (mHash256OidValue)) !=3D 0)) { > + return EFI_SECURITY_VIOLATION; > + } > + } > + } > + > + // > // Find out the new data payload which follows Pkcs7 SignedData direct= ly. > // > PayloadPtr =3D SigData + SigDataSize; > PayloadSize =3D DataSize - OFFSET_OF_AUTHINFO2_CERT_DATA - (UINTN) > SigDataSize; >=20 > 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 integrit= y 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 protec= t > these resources, > the authentication service provided in this driver will be broken, and= the > behavior is undefined. >=20 > -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 >=20 > @@ -35,10 +35,12 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF > ANY KIND, EITHER EXPRESS OR IMPLIED. > #include >=20 > #include > #include >=20 > +#define TWO_BYTE_ENCODE 0x82 > + > /// > /// Struct to record signature requirement defined by UEFI spec. > /// For SigHeaderSize and SigDataSize, ((UINT32) ~0) means NO exact leng= th > 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. >=20 > -Copyright (c) 2011 - 2016, Intel Corporation. All rights reserved.
> +Copyright (c) 2011 - 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 >=20 > @@ -61,11 +61,10 @@ UINT8 mHashOidValue[] =3D { > 0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x02, // OBJ_sha384 > 0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x03, // OBJ_sha512 > }; >=20 > HASH_TABLE mHash[] =3D { > - { 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 ( >=20 > HashCtx =3D NULL; > SectionHeader =3D NULL; > Status =3D FALSE; >=20 > - if ((HashAlg !=3D HASHALG_SHA1) && (HashAlg !=3D HASHALG_SHA256)) { > + if (HashAlg !=3D HASHALG_SHA256) { > return FALSE; > } >=20 > // > // Initialize context of hash. > // > ZeroMem (mImageDigest, MAX_DIGEST_SIZE); >=20 > - if (HashAlg =3D=3D HASHALG_SHA1) { > - mImageDigestSize =3D SHA1_DIGEST_SIZE; > - mCertType =3D gEfiCertSha1Guid; > - } else if (HashAlg =3D=3D HASHALG_SHA256) { > - mImageDigestSize =3D SHA256_DIGEST_SIZE; > - mCertType =3D gEfiCertSha256Guid; > - } > + mImageDigestSize =3D SHA256_DIGEST_SIZE; > + mCertType =3D gEfiCertSha256Guid; >=20 > CtxSize =3D mHash[HashAlg].GetContextSize(); >=20 > HashCtx =3D AllocatePool (CtxSize); > ASSERT (HashCtx !=3D 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 Secure= Boot > configuration module. >=20 > -Copyright (c) 2011 - 2016, Intel Corporation. All rights reserved.
> +Copyright (c) 2011 - 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 >=20 > @@ -65,14 +65,11 @@ extern EFI_IFR_GUID_LABEL *mStartLabel; > extern EFI_IFR_GUID_LABEL *mEndLabel; >=20 > #define MAX_CHAR 480 > #define TWO_BYTE_ENCODE 0x82 >=20 > -// > -// 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 >=20 > // > // 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