From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) (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 35D9481ECD for ; Sat, 21 Jan 2017 17:19:57 -0800 (PST) Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by fmsmga105.fm.intel.com with ESMTP; 21 Jan 2017 17:19:56 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.33,266,1477983600"; d="scan'208";a="56722105" Received: from fmsmsx106.amr.corp.intel.com ([10.18.124.204]) by fmsmga006.fm.intel.com with ESMTP; 21 Jan 2017 17:19:56 -0800 Received: from fmsmsx111.amr.corp.intel.com (10.18.116.5) by FMSMSX106.amr.corp.intel.com (10.18.124.204) with Microsoft SMTP Server (TLS) id 14.3.248.2; Sat, 21 Jan 2017 17:19:56 -0800 Received: from shsmsx104.ccr.corp.intel.com (10.239.4.70) by fmsmsx111.amr.corp.intel.com (10.18.116.5) with Microsoft SMTP Server (TLS) id 14.3.248.2; Sat, 21 Jan 2017 17:19:56 -0800 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.88]) by SHSMSX104.ccr.corp.intel.com ([10.239.4.70]) with mapi id 14.03.0248.002; Sun, 22 Jan 2017 09:19:54 +0800 From: "Zeng, Star" To: "Zhang, Chao B" , "edk2-devel@lists.01.org" CC: "Yao, Jiewen" , "Zeng, Star" Thread-Topic: [PATCH V2 3/3] MdeModulePkg: Variable: Update PCR[7] measure for new TCG spec Thread-Index: AQHSchLehb7Iy3G4pUCDqLk7V0iQ96FDtgdg Date: Sun, 22 Jan 2017 01:19:53 +0000 Message-ID: <0C09AFA07DD0434D9E2A0C6AEB0483103B818BB6@shsmsx102.ccr.corp.intel.com> References: <20170119051359.14044-1-chao.b.zhang@intel.com> <20170119051359.14044-3-chao.b.zhang@intel.com> In-Reply-To: <20170119051359.14044-3-chao.b.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 V2 3/3] MdeModulePkg: Variable: Update PCR[7] measure for new TCG spec 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: Sun, 22 Jan 2017 01:19:57 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Chao, I have some minor comments. 1. Suggest to add mSecureBootVarData and mSecureBootVarDataSize and remove = VARIABLE_FOLLOW_TYPE and SecureBootFollowUpdate. 2. Suggest to update function name RecordSecureBootPolicyVarFollow to Recor= dSecureBootPolicyVarData. 3. Please remember to add " ## SOMETIMES_CONSUMES ## Variable:L"DBT" " i= n VariableRuntimeDxe.inf and VariableSmmRuntimeDxe.inf. With those comments covered, Reviewed-by: Star Zeng Thanks, Star -----Original Message----- From: Zhang, Chao B=20 Sent: Thursday, January 19, 2017 1:14 PM To: edk2-devel@lists.01.org Cc: yao.jiewen@intel.com; Zeng, Star ; Yao, Jiewen ; Zhang, Chao B Subject: [PATCH V2 3/3] MdeModulePkg: Variable: Update PCR[7] measure for n= ew TCG spec Measure DBT into PCR[7] when it is updated between initial measure and Exit= BootService. Measure "SecureBoot" change after PK update. Spec version : TCG PC Client PFP 00.37. http://www.trustedcomputinggroup.or= g/wp-content/uploads/PC-ClientSpecific_Platform_Profile_for_TPM_2p0_Systems= _v21.pdf Cc: Star Zeng Cc: Yao Jiewen Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Chao Zhang --- .../Universal/Variable/RuntimeDxe/Measurement.c | 88 ++++++++++++++++++= +++- .../Universal/Variable/RuntimeDxe/VariableDxe.c | 17 +++++ .../Variable/RuntimeDxe/VariableSmmRuntimeDxe.c | 17 +++++ 3 files changed, 121 insertions(+), 1 deletion(-) diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Measurement.c b/Mde= ModulePkg/Universal/Variable/RuntimeDxe/Measurement.c index 2f92fae..707f988 100644 --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Measurement.c +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Measurement.c @@ -1,7 +1,7 @@ /** @file Measure TrEE required variable. =20 -Copyright (c) 2013 - 2014, Intel Corporation. All rights reserved.
+Copyright (c) 2013 - 2017, Intel Corporation. All rights reserved.
This program and the accompanying materials are licensed and made availab= le under the terms and conditions of the BSD License which accompanies thi= s distribution. The full text of the license may be found at @@ -36,6 +36,= 24 @@ VARIABLE_TYPE mVariableType[] =3D { {EFI_KEY_EXCHANGE_KEY_NAME, &gEfiGlobalVariableGuid}, {EFI_IMAGE_SECURITY_DATABASE, &gEfiImageSecurityDatabaseGuid}, {EFI_IMAGE_SECURITY_DATABASE1, &gEfiImageSecurityDatabaseGuid}, + {EFI_IMAGE_SECURITY_DATABASE2, &gEfiImageSecurityDatabaseGuid}, }; + +typedef struct { + CHAR16 *VariableName; + EFI_GUID *VendorGuid; + UINT8 *VarData; + UINTN VarDataSize; +} VARIABLE_FOLLOW_TYPE; + +// +// "SecureBoot" may update following PK Del/Add // static=20 +VARIABLE_FOLLOW_TYPE SecureBootFollowUpdate =3D { + EFI_SECURE_BOOT_MODE_NAME, + &gEfiGlobalVariableGuid, + NULL, + 0, }; =20 /** @@ -251,5 +269,73 @@ SecureBootHook ( FreePool (VariableData); } =20 + // + // "SecureBoot" is 8bit & read-only. It can only be changed according=20 + to PK update // if ((StrCmp (VariableName, EFI_PLATFORM_KEY_NAME) =3D= =3D=20 + 0) && + CompareGuid (VendorGuid, &gEfiGlobalVariableGuid)) { + Status =3D InternalGetVariable ( + SecureBootFollowUpdate.VariableName, + SecureBootFollowUpdate.VendorGuid, + &VariableData, + &VariableDataSize + ); + if (EFI_ERROR (Status)) { + return; + } + + if ((SecureBootFollowUpdate.VarData !=3D NULL) && + (CompareMem(SecureBootFollowUpdate.VarData, VariableData, Variabl= eDataSize) !=3D 0)) { + FreePool(SecureBootFollowUpdate.VarData); + SecureBootFollowUpdate.VarData =3D VariableData; + SecureBootFollowUpdate.VarDataSize =3D VariableDataSize; + + DEBUG((DEBUG_INFO, "%s variable updated according to PK change. Rem= easure the value!\n", SecureBootFollowUpdate.VariableName)); + Status =3D MeasureVariable ( + SecureBootFollowUpdate.VariableName, + SecureBootFollowUpdate.VendorGuid, + SecureBootFollowUpdate.VarData, + SecureBootFollowUpdate.VarDataSize + ); + DEBUG ((DEBUG_INFO, "MeasureBootPolicyVariable - %r\n", Status)); + } else { + // + // "SecureBoot" variable is not changed + // + FreePool(VariableData); + } + } + return ; } + +/** + Some Secure Boot Policy Variable may update following other variable cha= nges(SecureBoot follows PK change, etc). + Record their initial State when variable write service is ready. + +**/ +VOID +EFIAPI +RecordSecureBootPolicyVarFollow( + VOID + ) +{ + EFI_STATUS Status; + + // + // Record initial "SecureBoot" variable value. + // It is used to detect SecureBoot variable change in SecureBootHook. + // + Status =3D InternalGetVariable ( + SecureBootFollowUpdate.VariableName, + SecureBootFollowUpdate.VendorGuid, + (VOID **)&SecureBootFollowUpdate.VarData, + &SecureBootFollowUpdate.VarDataSize + ); + if (EFI_ERROR(Status)) { + // + // Read could fail when Auth Variable solution is not supported + // + DEBUG((DEBUG_INFO, "RecordSecureBootPolicyVarFollow GetVariable %s=20 +Status %x\n", SecureBootFollowUpdate.VariableName, Status)); + } +} diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c b/Mde= ModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c index 3d3cd24..5d81f87 100644 --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c @@ -32,6 +32,17 @@ EDKII_VAR_CHECK_PROTOCOL mVarCheck = =3D { VarCheckRegis VarChe= ckVariablePropertyGet }; =20 /** + Some Secure Boot Policy Variable may update following other variable cha= nges(SecureBoot follows PK change, etc). + Record their initial State when variable write service is ready. + +**/ +VOID +EFIAPI +RecordSecureBootPolicyVarFollow( + VOID + ); + +/** Return TRUE if ExitBootServices () has been called. =20 @retval TRUE If ExitBootServices () has been called. @@ -415,6 +426,12 @@ FtwNotificationEvent ( } =20 // + // Some Secure Boot Policy Var (SecureBoot, etc) updates following=20 + other // Secure Boot Policy Variable change. Record their initial value. + // + RecordSecureBootPolicyVarFollow(); + + // // Install the Variable Write Architectural protocol. // Status =3D gBS->InstallProtocolInterface ( diff --git a/MdeModulePkg/Uni= versal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c b/MdeModulePkg/Universal= /Variable/RuntimeDxe/VariableSmmRuntimeDxe.c index 0a076ae..3d0925d 100644 --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c @@ -71,6 +71,17 @@ SecureBootHook ( ); =20 /** + Some Secure Boot Policy Variable may update following other variable cha= nges(SecureBoot follows PK change, etc). + Record their initial State when variable write service is ready. + +**/ +VOID +EFIAPI +RecordSecureBootPolicyVarFollow( + VOID + ); + +/** Acquires lock only at boot time. Simply returns at runtime. =20 This is a temperary function that will be removed when @@ -1079,6 +1090,= 12 @@ SmmVariableWriteReady ( return; } =20 + // + // Some Secure Boot Policy Var (SecureBoot, etc) updates following=20 + other // Secure Boot Policy Variable change. Record their initial value= . + // + RecordSecureBootPolicyVarFollow(); + Status =3D gBS->InstallProtocolInterface ( &mHandle, &gEfiVariableWriteArchProtocolGuid, -- 1.9.5.msysgit.1