From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=134.134.136.24; helo=mga09.intel.com; envelope-from=chen.a.chen@intel.com; receiver=edk2-devel@lists.01.org Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) (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 4991621F3882E for ; Thu, 12 Oct 2017 23:46:55 -0700 (PDT) Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by orsmga102.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 12 Oct 2017 23:50:25 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.43,369,1503385200"; d="scan'208";a="322784427" Received: from fmsmsx106.amr.corp.intel.com ([10.18.124.204]) by fmsmga004.fm.intel.com with ESMTP; 12 Oct 2017 23:50:25 -0700 Received: from shsmsx151.ccr.corp.intel.com (10.239.6.50) by FMSMSX106.amr.corp.intel.com (10.18.124.204) with Microsoft SMTP Server (TLS) id 14.3.319.2; Thu, 12 Oct 2017 23:50:25 -0700 Received: from shsmsx152.ccr.corp.intel.com ([169.254.6.93]) by SHSMSX151.ccr.corp.intel.com ([169.254.3.98]) with mapi id 14.03.0319.002; Fri, 13 Oct 2017 14:50:24 +0800 From: "Chen, Chen A" To: "Zhang, Chao B" , "edk2-devel@lists.01.org" CC: "Long, Qin" Thread-Topic: [PATCH] SecurityPkg:AuthVariableLib:Implement ECR1707 for Private Auth Variable Thread-Index: AQHTQzqG7TffDGei2kaX5enUsXn9WqLhWIgQ Date: Fri, 13 Oct 2017 06:50:24 +0000 Message-ID: <8CCE94730AD68946B3D28B43192D9D2D0263D3AF@SHSMSX152.ccr.corp.intel.com> References: <20171012091425.29420-1-chao.b.zhang@intel.com> In-Reply-To: <20171012091425.29420-1-chao.b.zhang@intel.com> Accept-Language: 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:AuthVariableLib:Implement ECR1707 for Private Auth Variable X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 13 Oct 2017 06:46:55 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Reviewed-by: Chen A Chen -----Original Message----- From: Zhang, Chao B=20 Sent: Thursday, October 12, 2017 5:14 PM To: edk2-devel@lists.01.org Cc: Long, Qin ; Chen, Chen A ; Z= hang, Chao B Subject: [PATCH] SecurityPkg:AuthVariableLib:Implement ECR1707 for Private = Auth Variable ECR1707 for UEFI2.7 clarified certificate management rule for private time-= based AuthVariable.Trusted cert rule changed from whole signer's certificat= e stack to top-level issuer cert tbscertificate + SignerCert CN for better = management compatibility. Hash is used to reduce storage overhead. Cc: Long Qin Cc: Chen Chen Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Chao Zhang --- SecurityPkg/Library/AuthVariableLib/AuthService.c | 208 ++++++++++++++++++= ---- 1 file changed, 171 insertions(+), 37 deletions(-) diff --git a/SecurityPkg/Library/AuthVariableLib/AuthService.c b/SecurityPk= g/Library/AuthVariableLib/AuthService.c index a37ec0b..7188ff6 100644 --- a/SecurityPkg/Library/AuthVariableLib/AuthService.c +++ b/SecurityPkg/Library/AuthVariableLib/AuthService.c @@ -1530,6 +1530,85 @@ AuthServiceInternalCompareTimeStamp ( } =20 /** + Calculate SHA256 digest of SignerCert CommonName + ToplevelCert=20 + tbsCertificate SignerCert and ToplevelCert are inside the signer certifi= cate chain. + + @param[in] SignerCert A pointer to SignerCert data. + @param[in] SignerCertSize Length of SignerCert data. + @param[in] TopLevelCert A pointer to TopLevelCert data. + @param[in] TopLevelCertSize Length of TopLevelCert data. + @param[out] Sha256Digest Sha256 digest calculated. + + @return EFI_ABORTED Digest process failed. + @return EFI_SUCCESS SHA256 Digest is succesfully calculated. + +**/ +EFI_STATUS +CalculatePrivAuthVarSignChainSHA256Digest( + IN UINT8 *SignerCert, + IN UINTN SignerCertSize, + IN UINT8 *TopLevelCert, + IN UINTN TopLevelCertSize, + OUT UINT8 *Sha256Digest + ) +{ + UINT8 *TbsCert; + UINTN TbsCertSize; + UINT8 CertCommonName[128]; + UINTN CertCommonNameSize; + BOOLEAN CryptoStatus; + EFI_STATUS Status; + + CertCommonNameSize =3D sizeof(CertCommonName); + + // + // Get SignerCert CommonName + // + Status =3D X509GetCommonName(SignerCert, SignerCertSize,=20 + CertCommonName, &CertCommonNameSize); if (EFI_ERROR(Status)) { + DEBUG((DEBUG_INFO, "%a Get SignerCert CommonName failed with status %x= \n", __FUNCTION__, Status)); + return EFI_ABORTED; + } + + // + // Get TopLevelCert tbsCertificate + // + if (!X509GetTBSCert(TopLevelCert, TopLevelCertSize, &TbsCert, &TbsCertSi= ze)) { + DEBUG((DEBUG_INFO, "%a Get Top-level Cert tbsCertificate failed!\n", _= _FUNCTION__)); + return EFI_ABORTED; + } + + // + // Digest SignerCert CN + TopLevelCert tbsCertificate // ZeroMem=20 + (Sha256Digest, SHA256_DIGEST_SIZE); CryptoStatus =3D Sha256Init=20 + (mHashCtx); if (!CryptoStatus) { + return EFI_ABORTED; + } + + // + // '\0' is forced in CertCommonName. No overflow issue // =20 + CryptoStatus =3D Sha256Update (mHashCtx, CertCommonName,=20 + AsciiStrLen((CHAR8 *)CertCommonName)); if (!CryptoStatus) { + return EFI_ABORTED; + } + + CryptoStatus =3D Sha256Update (mHashCtx, TbsCert, TbsCertSize); if=20 + (!CryptoStatus) { + return EFI_ABORTED; + } + + CryptoStatus =3D Sha256Final (mHashCtx, Sha256Digest); if=20 + (!CryptoStatus) { + return EFI_ABORTED; + } + + return EFI_SUCCESS; +} + +/** Find matching signer's certificates for common authenticated variable by corresponding VariableName and VendorGuid from "certdb" or "certdbv". =20 @@ -1872,13 +1951,16 @@ DeleteCertsFromDb ( /** Insert signer's certificates for common authenticated variable with Vari= ableName and VendorGuid in AUTH_CERT_DB_DATA to "certdb" or "certdbv" according t= o - time based authenticated variable attributes. + time based authenticated variable attributes. CertData is the SHA256=20 + digest of SignerCert CommonName + TopLevelCert tbsCertificate. =20 - @param[in] VariableName Name of authenticated Variable. - @param[in] VendorGuid Vendor GUID of authenticated Variable. - @param[in] Attributes Attributes of authenticated variable. - @param[in] CertData Pointer to signer's certificates. - @param[in] CertDataSize Length of CertData in bytes. + @param[in] VariableName Name of authenticated Variable. + @param[in] VendorGuid Vendor GUID of authenticated Variable. + @param[in] Attributes Attributes of authenticated variable. + @param[in] SignerCert Signer certificate data. + @param[in] SignerCertSize Length of signer certificate. + @param[in] TopLevelCert Top-level certificate data. + @param[in] TopLevelCertSize Length of top-level certificate. =20 @retval EFI_INVALID_PARAMETER Any input parameter is invalid. @retval EFI_ACCESS_DENIED An AUTH_CERT_DB_DATA entry with same Vari= ableName @@ -1892,8 +1974,10 @@ InsertCertsToDb ( IN CHAR16 *VariableName, IN EFI_GUID *VendorGuid, IN UINT32 Attributes, - IN UINT8 *CertData, - IN UINTN CertDataSize + IN UINT8 *SignerCert, + IN UINTN SignerCertSize, + IN UINT8 *TopLevelCert, + IN UINTN TopLevelCertSize ) { EFI_STATUS Status; @@ -1904,10 +1988,12 @@ InsertCertsToDb ( UINT32 NewCertDbSize; UINT32 CertNodeSize; UINT32 NameSize; + UINT32 CertDataSize; AUTH_CERT_DB_DATA *Ptr; CHAR16 *DbName; + UINT8 Sha256Digest[SHA256_DIGEST_SIZE]; =20 - if ((VariableName =3D=3D NULL) || (VendorGuid =3D=3D NULL) || (CertData = =3D=3D NULL)) { + if ((VariableName =3D=3D NULL) || (VendorGuid =3D=3D NULL) || (SignerCer= t =3D=3D=20 + NULL) ||(TopLevelCert =3D=3D NULL)) { return EFI_INVALID_PARAMETER; } =20 @@ -1967,11 +2053,24 @@ InsertCertsToDb ( // Construct new data content of variable "certdb" or "certdbv". // NameSize =3D (UINT32) StrLen (VariableName); + CertDataSize =3D sizeof(Sha256Digest); CertNodeSize =3D sizeof (AUTH_CERT_DB_DATA) + (UINT32) CertDataSize + N= ameSize * sizeof (CHAR16); NewCertDbSize =3D (UINT32) DataSize + CertNodeSize; if (NewCertDbSize > mMaxCertDbSize) { return EFI_OUT_OF_RESOURCES; } + + Status =3D CalculatePrivAuthVarSignChainSHA256Digest( + SignerCert, + SignerCertSize, + TopLevelCert, + TopLevelCertSize, + Sha256Digest + ); + if (EFI_ERROR (Status)) { + return Status; + } + NewCertDb =3D (UINT8*) mCertDbStore; =20 // @@ -1999,7 +2098,7 @@ InsertCertsToDb ( =20 CopyMem ( (UINT8 *) Ptr + sizeof (AUTH_CERT_DB_DATA) + NameSize * sizeof (CHAR1= 6), - CertData, + Sha256Digest, CertDataSize ); =20 @@ -2181,19 +2280,27 @@ VerifyTimeBasedPayload ( UINTN NewDataSize; UINT8 *Buffer; UINTN Length; - UINT8 *RootCert; - UINTN RootCertSize; + UINT8 *TopLevelCert; + UINTN TopLevelCertSize; + UINT8 *TrustedCert; + UINTN TrustedCertSize; UINT8 *SignerCerts; UINTN CertStackSize; UINT8 *CertsInCertDb; UINT32 CertsSizeinDb; + UINT8 Sha256Digest[SHA256_DIGEST_SIZE]; =20 + // + // 1. TopLevelCert is the top-level issuer certificate in signature=20 + Signer Cert Chain // 2. TrustedCert is the certificate which firmware tr= usts. It could be saved in protected + // storage or PK payload on PK init + // VerifyStatus =3D FALSE; CertData =3D NULL; NewData =3D NULL; Attr =3D Attributes; SignerCerts =3D NULL; - RootCert =3D NULL; + TopLevelCert =3D NULL; CertsInCertDb =3D NULL; =20 // @@ -2325,8 +2432,8 @@ VerifyTimeBasedPayload ( SigDataSize, &SignerCerts, &CertStackSize, - &RootCert, - &RootCertSize + &TopLevelCert, + &TopLevelCertSize ); if (!VerifyStatus) { goto Exit; @@ -2348,8 +2455,8 @@ VerifyTimeBasedPayload ( } CertList =3D (EFI_SIGNATURE_LIST *) Data; Cert =3D (EFI_SIGNATURE_DATA *) ((UINT8 *) CertList + sizeof (EFI_= SIGNATURE_LIST) + CertList->SignatureHeaderSize); - if ((RootCertSize !=3D (CertList->SignatureSize - (sizeof (EFI_SIGNATU= RE_DATA) - 1))) || - (CompareMem (Cert->SignatureData, RootCert, RootCertSize) !=3D 0))= { + if ((TopLevelCertSize !=3D (CertList->SignatureSize - (sizeof (EFI_SIG= NATURE_DATA) - 1))) || + (CompareMem (Cert->SignatureData, TopLevelCert,=20 + TopLevelCertSize) !=3D 0)) { VerifyStatus =3D FALSE; goto Exit; } @@ -2360,8 +2467,8 @@ VerifyTimeBasedPayload ( VerifyStatus =3D Pkcs7Verify ( SigData, SigDataSize, - RootCert, - RootCertSize, + TopLevelCert, + TopLevelCertSize, NewData, NewDataSize ); @@ -2394,8 +2501,8 @@ VerifyTimeBasedPayload ( // // Iterate each Signature Data Node within this CertList for a v= erify // - RootCert =3D Cert->SignatureData; - RootCertSize =3D CertList->SignatureSize - (sizeof (EFI_SIGNATU= RE_DATA) - 1); + TrustedCert =3D Cert->SignatureData; + TrustedCertSize =3D CertList->SignatureSize - (sizeof=20 + (EFI_SIGNATURE_DATA) - 1); =20 // // Verify Pkcs7 SignedData via Pkcs7Verify library. @@ -2403,8 +2510,8 @@ VerifyTimeBasedPayload ( VerifyStatus =3D Pkcs7Verify ( SigData, SigDataSize, - RootCert, - RootCertSize, + TrustedCert, + TrustedCertSize, NewData, NewDataSize ); @@ -2428,8 +2535,8 @@ VerifyTimeBasedPayload ( SigDataSize, &SignerCerts, &CertStackSize, - &RootCert, - &RootCertSize + &TopLevelCert, + &TopLevelCertSize ); if (!VerifyStatus) { goto Exit; @@ -2448,17 +2555,36 @@ VerifyTimeBasedPayload ( goto Exit; } =20 - if ((CertStackSize !=3D CertsSizeinDb) || - (CompareMem (SignerCerts, CertsInCertDb, CertsSizeinDb) !=3D 0))= { - goto Exit; + if (CertsSizeinDb =3D=3D SHA256_DIGEST_SIZE) { + // + // Check hash of signer cert CommonName + Top-level issuer tbsCert= ificate against data in CertDb + // + Status =3D CalculatePrivAuthVarSignChainSHA256Digest( + SignerCerts + sizeof(UINT8) + sizeof(UINT32), + ReadUnaligned32 ((UINT32 *)(SignerCerts + sizeof(UINT8)= )), + TopLevelCert, + TopLevelCertSize, + Sha256Digest + ); + if (EFI_ERROR(Status) || CompareMem (Sha256Digest, CertsInCertDb, = CertsSizeinDb) !=3D 0){ + goto Exit; + } + } else { + // + // Keep backward compatible with previous solution which saves wh= ole signer certs stack in CertDb + // + if ((CertStackSize !=3D CertsSizeinDb) || + (CompareMem (SignerCerts, CertsInCertDb, CertsSizeinDb) !=3D = 0)) { + goto Exit; + } } } =20 VerifyStatus =3D Pkcs7Verify ( SigData, SigDataSize, - RootCert, - RootCertSize, + TopLevelCert, + TopLevelCertSize, NewData, NewDataSize ); @@ -2468,9 +2594,17 @@ VerifyTimeBasedPayload ( =20 if ((OrgTimeStamp =3D=3D NULL) && (PayloadSize !=3D 0)) { // - // Insert signer's certificates when adding a new common authenticat= ed variable. + // When adding a new common authenticated variable, always save=20 + Hash of cn of signer cert + tbsCertificate of Top-level issuer // - Status =3D InsertCertsToDb (VariableName, VendorGuid, Attributes, Si= gnerCerts, CertStackSize); + Status =3D InsertCertsToDb ( + VariableName, + VendorGuid, + Attributes, + SignerCerts + sizeof(UINT8) + sizeof(UINT32), + ReadUnaligned32 ((UINT32 *)(SignerCerts + sizeof(UINT8)))= , + TopLevelCert, + TopLevelCertSize + ); if (EFI_ERROR (Status)) { VerifyStatus =3D FALSE; goto Exit; @@ -2479,16 +2613,16 @@ VerifyTimeBasedPayload ( } else if (AuthVarType =3D=3D AuthVarTypePayload) { CertList =3D (EFI_SIGNATURE_LIST *) PayloadPtr; Cert =3D (EFI_SIGNATURE_DATA *) ((UINT8 *) CertList + sizeof (EFI_= SIGNATURE_LIST) + CertList->SignatureHeaderSize); - RootCert =3D Cert->SignatureData; - RootCertSize =3D CertList->SignatureSize - (sizeof (EFI_SIGNATURE_DAT= A) - 1); + TrustedCert =3D Cert->SignatureData; + TrustedCertSize =3D CertList->SignatureSize - (sizeof=20 + (EFI_SIGNATURE_DATA) - 1); // // Verify Pkcs7 SignedData via Pkcs7Verify library. // VerifyStatus =3D Pkcs7Verify ( SigData, SigDataSize, - RootCert, - RootCertSize, + TrustedCert, + TrustedCertSize, NewData, NewDataSize ); @@ -2499,7 +2633,7 @@ VerifyTimeBasedPayload ( Exit: =20 if (AuthVarType =3D=3D AuthVarTypePk || AuthVarType =3D=3D AuthVarTypePr= iv) { - Pkcs7FreeSigners (RootCert); + Pkcs7FreeSigners (TopLevelCert); Pkcs7FreeSigners (SignerCerts); } =20 -- 1.9.5.msysgit.1