From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) (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 5DC041A1E73 for ; Wed, 26 Oct 2016 16:36:22 -0700 (PDT) Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by orsmga104.jf.intel.com with ESMTP; 26 Oct 2016 16:36:22 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.31,551,1473145200"; d="scan'208";a="778237303" Received: from orsmsx104.amr.corp.intel.com ([10.22.225.131]) by FMSMGA003.fm.intel.com with ESMTP; 26 Oct 2016 16:36:21 -0700 Received: from orsmsx160.amr.corp.intel.com (10.22.226.43) by ORSMSX104.amr.corp.intel.com (10.22.225.131) with Microsoft SMTP Server (TLS) id 14.3.248.2; Wed, 26 Oct 2016 16:36:21 -0700 Received: from orsmsx113.amr.corp.intel.com ([169.254.9.50]) by ORSMSX160.amr.corp.intel.com ([10.22.226.43]) with mapi id 14.03.0248.002; Wed, 26 Oct 2016 16:36:21 -0700 From: "Kinney, Michael D" To: "Yao, Jiewen" , "edk2-devel@lists.01.org" , "Kinney, Michael D" CC: "Tian, Feng" , "Gao, Liming" , "Zeng, Star" , "Zhang, Chao B" Thread-Topic: [edk2] [PATCH V4 6/8] QuarkPlatformPkg/PlatformBootManager: Add capsule/recovery handling. Thread-Index: AQHSLNWpVPAu85xSo0qnRztP2Ve9XaC7Z2QQ Date: Wed, 26 Oct 2016 23:36:20 +0000 Message-ID: References: <1477189908-8336-1-git-send-email-jiewen.yao@intel.com> <1477189908-8336-7-git-send-email-jiewen.yao@intel.com> In-Reply-To: <1477189908-8336-7-git-send-email-jiewen.yao@intel.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ctpclassification: CTP_IC x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiMjQyOGU5ZGYtMDA4NS00ZWUyLWFhNGQtMTI3MTFiZTg5MzVjIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE1LjkuNi42IiwiVHJ1c3RlZExhYmVsSGFzaCI6IlowWEthenJ3bm5xclgxakVjYzR1SlhUbWM4S1NtaXV6eUV4a0ROcjJkOFU9In0= x-originating-ip: [10.22.254.140] MIME-Version: 1.0 Subject: Re: [PATCH V4 6/8] QuarkPlatformPkg/PlatformBootManager: Add capsule/recovery handling. 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: Wed, 26 Oct 2016 23:36:22 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Jiewen, Can the code that checks for the use of a test key be moved into a common B= DS lib or module? Maybe in MdeModulePkg\Universal\BdsDxe\BdsEntry.c right before the call to= =20 PlatformBootManagerAfterConsole()? The logic in BdsEntry.c can do the chec= k and set the PcdTestKeyUsed PCD and can go a DEBUG() message for the use of a test key. With the current design, you depend on a platform specific BDS library to i= nclude the test=20 key check, and we want to make sure the check for the use of a test key is = always performed. Also, the test key check against PcdRsa2048Sha256PublicKeyBuffer is incompl= ete. The DEC file description of this PCD is as follows: ## Provides one or more SHA 256 Hashes of the RSA 2048 public keys used t= o verify Recovery and Capsule Update images # WARNING: The default value is treated as test key. Please do not use d= efault value in the production. # @Prompt One or more SHA 256 Hashes of RSA 2048 bit public keys used to = verify Recovery and Capsule Update images # gEfiSecurityPkgTokenSpaceGuid.PcdRsa2048Sha256PublicKeyBuffer|{0x91, 0x29= , 0xc4, 0xbd, 0xea, 0x6d, 0xda, 0xb3, 0xaa, 0x6f, 0x50, 0x16, 0xfc, 0xdb, 0= x4b, 0x7e, 0x3c, 0xd6, 0xdc, 0xa4, 0x7a, 0x0e, 0xdd, 0xe6, 0x15, 0x8c, 0x73= , 0x96, 0xa2, 0xd4, 0xa6, 0x4d}|VOID*|0x00010013 Since this PCD provides one or more SHA 256 Hashes, the check for the use o= f a test key needs to get the=20 Size, determine how many hashes are in this PCD, and compare the test key v= alue against each entry in this array. Thanks, Mike > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ji= ewen Yao > Sent: Saturday, October 22, 2016 7:32 PM > To: edk2-devel@lists.01.org > Cc: Tian, Feng ; Gao, Liming ;= Zeng, Star > ; Kinney, Michael D ; Zh= ang, Chao B > > Subject: [edk2] [PATCH V4 6/8] QuarkPlatformPkg/PlatformBootManager: Add > capsule/recovery handling. >=20 > 1) Add capsule and recovery boot path handling in platform BDS. > 2) Add check if the platform is using default test key for recovery or up= date. > Produce PcdTestKeyUsed to indicate if there is any > test key used in current BIOS, such as recovery key, > or capsule update key. > Then the generic UI may consume this PCD to show warning information. >=20 > Cc: Michael D Kinney > Cc: Kelly Steele > Cc: Feng Tian > Cc: Star Zeng > Cc: Liming Gao > Cc: Chao Zhang > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Jiewen Yao > --- > QuarkPlatformPkg/Library/PlatformBootManagerLib/PlatformBootManager.c = | 131 > +++++++++++++++++++- > QuarkPlatformPkg/Library/PlatformBootManagerLib/PlatformBootManager.h = | 9 +- > QuarkPlatformPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.i= nf | 16 ++- > 3 files changed, 151 insertions(+), 5 deletions(-) >=20 > diff --git a/QuarkPlatformPkg/Library/PlatformBootManagerLib/PlatformBoot= Manager.c > b/QuarkPlatformPkg/Library/PlatformBootManagerLib/PlatformBootManager.c > index 19ff3d0..f327c89 100644 > --- a/QuarkPlatformPkg/Library/PlatformBootManagerLib/PlatformBootManager= .c > +++ b/QuarkPlatformPkg/Library/PlatformBootManagerLib/PlatformBootManager= .c > @@ -2,7 +2,7 @@ > This file include all platform action which can be customized > by IBV/OEM. >=20 > -Copyright (c) 2015, Intel Corporation. All rights reserved.
> +Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.
> This program and the accompanying materials > are licensed and made available under the terms and conditions of the BS= D License > which accompanies this distribution. The full text of the license may b= e found at > @@ -205,6 +205,8 @@ PlatformBootManagerBeforeConsole ( > EFI_INPUT_KEY Enter; > EFI_INPUT_KEY F2; > EFI_BOOT_MANAGER_LOAD_OPTION BootOption; > + ESRT_MANAGEMENT_PROTOCOL *EsrtManagement; > + EFI_BOOT_MODE BootMode; > EFI_ACPI_S3_SAVE_PROTOCOL *AcpiS3Save; > EFI_HANDLE Handle; > EFI_EVENT EndOfDxeEvent; > @@ -246,6 +248,40 @@ PlatformBootManagerBeforeConsole ( > // > PlatformRegisterFvBootOption (&mUefiShellFileGuid, L"UEFI Shell", > LOAD_OPTION_ACTIVE); >=20 > + Status =3D gBS->LocateProtocol(&gEsrtManagementProtocolGuid, NULL, (VO= ID > **)&EsrtManagement); > + if (EFI_ERROR(Status)) { > + EsrtManagement =3D NULL; > + } > + > + BootMode =3D GetBootModeHob(); > + switch (BootMode) { > + case BOOT_ON_FLASH_UPDATE: > + DEBUG((EFI_D_INFO, "ProcessCapsules Before EndOfDxe ......\n")); > + Status =3D ProcessCapsules (); > + DEBUG((EFI_D_INFO, "ProcessCapsules %r\n", Status)); > + break; > + case BOOT_IN_RECOVERY_MODE: > + break; > + case BOOT_ASSUMING_NO_CONFIGURATION_CHANGES: > + case BOOT_WITH_MINIMAL_CONFIGURATION: > + case BOOT_ON_S4_RESUME: > + if (EsrtManagement !=3D NULL) { > + // > + // Lock ESRT cache repository before EndofDxe if ESRT sync is not = needed > + // > + EsrtManagement->LockEsrtRepository(); > + } > + break; > + default: > + // > + // Require to sync ESRT from FMP in a new boot > + // > + if (EsrtManagement !=3D NULL) { > + EsrtManagement->SyncEsrtFmp(); > + } > + break; > + } > + > // > // Prepare for S3 > // > @@ -303,7 +339,64 @@ PlatformBootManagerAfterConsole ( > VOID > ) > { > - EFI_STATUS Status; > + EFI_STATUS Status; > + EFI_BOOT_MODE BootMode; > + ESRT_MANAGEMENT_PROTOCOL *EsrtManagement; > + VOID *Buffer; > + UINTN Size; > + > + Status =3D gBS->LocateProtocol(&gEsrtManagementProtocolGuid, NULL, (VO= ID > **)&EsrtManagement); > + if (EFI_ERROR(Status)) { > + EsrtManagement =3D NULL; > + } > + > + BootMode =3D GetBootModeHob(); > + switch (BootMode) { > + case BOOT_ON_FLASH_UPDATE: > + DEBUG((EFI_D_INFO, "Capsule Mode detected\n")); > + if (FeaturePcdGet(PcdSupportUpdateCapsuleReset)) { > + EfiBootManagerConnectAll (); > + EfiBootManagerRefreshAllBootOption (); > + > + // > + // Always sync ESRT Cache from FMP Instances after connect all and= before > capsule process > + // > + if (EsrtManagement !=3D NULL) { > + EsrtManagement->SyncEsrtFmp(); > + } > + > + DEBUG((EFI_D_INFO, "ProcessCapsules After ConnectAll ......\n")); > + Status =3D ProcessCapsules(); > + DEBUG((EFI_D_INFO, "ProcessCapsules %r\n", Status)); > + } > + break; > + > + case BOOT_IN_RECOVERY_MODE: > + DEBUG((EFI_D_INFO, "Recovery Mode detected\n")); > + // Passthrough > + > + case BOOT_ASSUMING_NO_CONFIGURATION_CHANGES: > + case BOOT_WITH_MINIMAL_CONFIGURATION: > + case BOOT_WITH_FULL_CONFIGURATION: > + case BOOT_WITH_FULL_CONFIGURATION_PLUS_DIAGNOSTICS: > + case BOOT_WITH_DEFAULT_SETTINGS: > + default: > + EfiBootManagerConnectAll (); > + EfiBootManagerRefreshAllBootOption (); > + > + // > + // Sync ESRT Cache from FMP Instance on demand after Connect All > + // > + if ((BootMode !=3D BOOT_ASSUMING_NO_CONFIGURATION_CHANGES) && > + (BootMode !=3D BOOT_WITH_MINIMAL_CONFIGURATION) && > + (BootMode !=3D BOOT_ON_S4_RESUME)) { > + if (EsrtManagement !=3D NULL) { > + EsrtManagement->SyncEsrtFmp(); > + } > + } > + > + break; > + } >=20 > Print ( > L"\n" > @@ -313,6 +406,40 @@ PlatformBootManagerAfterConsole ( > ); >=20 > // > + // Check if the platform is using test key. > + // > + Status =3D GetSectionFromAnyFv( > + PcdGetPtr(PcdEdkiiRsa2048Sha256TestPublicKeyFileGuid), > + EFI_SECTION_RAW, > + 0, > + &Buffer, > + &Size > + ); > + if (!EFI_ERROR(Status)) { > + if ((Size =3D=3D PcdGetSize(PcdRsa2048Sha256PublicKeyBuffer)) && > + (CompareMem(Buffer, PcdGetPtr(PcdRsa2048Sha256PublicKeyBuffer), = Size) =3D=3D 0)) { > + Print(L"WARNING: Recovery Test Key is used.\n"); > + PcdSetBoolS(PcdTestKeyUsed, TRUE); > + } > + FreePool(Buffer); > + } > + Status =3D GetSectionFromAnyFv( > + PcdGetPtr(PcdEdkiiPkcs7TestPublicKeyFileGuid), > + EFI_SECTION_RAW, > + 0, > + &Buffer, > + &Size > + ); > + if (!EFI_ERROR(Status)) { > + if ((Size =3D=3D PcdGetSize(PcdPkcs7CertBuffer)) && > + (CompareMem(Buffer, PcdGetPtr(PcdPkcs7CertBuffer), Size) =3D=3D = 0)) { > + Print(L"WARNING: Capsule Test Key is used.\n"); > + PcdSetBoolS(PcdTestKeyUsed, TRUE); > + } > + FreePool(Buffer); > + } > + > + // > // Use a DynamicHii type pcd to save the boot status, which is used to > // control configuration mode, such as FULL/MINIMAL/NO_CHANGES configu= ration. > // > diff --git a/QuarkPlatformPkg/Library/PlatformBootManagerLib/PlatformBoot= Manager.h > b/QuarkPlatformPkg/Library/PlatformBootManagerLib/PlatformBootManager.h > index 7413883..395f78b 100644 > --- a/QuarkPlatformPkg/Library/PlatformBootManagerLib/PlatformBootManager= .h > +++ b/QuarkPlatformPkg/Library/PlatformBootManagerLib/PlatformBootManager= .h > @@ -1,7 +1,7 @@ > /** @file > Head file for BDS Platform specific code >=20 > -Copyright (c) 2015, Intel Corporation. All rights reserved.
> +Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.
> This program and the accompanying materials > are licensed and made available under the terms and conditions of the BS= D License > which accompanies this distribution. The full text of the license may b= e found at > @@ -21,6 +21,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITH= ER EXPRESS OR > IMPLIED. > #include > #include > #include > +#include > #include > #include > #include > @@ -32,9 +33,13 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EIT= HER EXPRESS OR > IMPLIED. > #include > #include > #include > +#include > #include > #include > - > +#include > +#include > +#include > +#include >=20 > typedef struct { > EFI_DEVICE_PATH_PROTOCOL *DevicePath; > diff --git a/QuarkPlatformPkg/Library/PlatformBootManagerLib/PlatformBoot= ManagerLib.inf > b/QuarkPlatformPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.= inf > index d59f14a..eadf1fe 100644 > --- a/QuarkPlatformPkg/Library/PlatformBootManagerLib/PlatformBootManager= Lib.inf > +++ b/QuarkPlatformPkg/Library/PlatformBootManagerLib/PlatformBootManager= Lib.inf > @@ -1,7 +1,7 @@ > ## @file > # Include all platform action which can be customized by IBV/OEM. > # > -# Copyright (c) 2012 - 2015, Intel Corporation. All rights reserved. > +# Copyright (c) 2012 - 2016, 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 ma= y be found at > @@ -38,6 +38,8 @@ > IntelFrameworkPkg/IntelFrameworkPkg.dec > IntelFrameworkModulePkg/IntelFrameworkModulePkg.dec > SourceLevelDebugPkg/SourceLevelDebugPkg.dec > + QuarkPlatformPkg/QuarkPlatformPkg.dec > + SecurityPkg/SecurityPkg.dec >=20 > [LibraryClasses] > BaseLib > @@ -49,11 +51,16 @@ > UefiBootServicesTableLib > UefiLib > UefiBootManagerLib > + PrintLib > + HobLib > + CapsuleLib > + DxeServicesLib >=20 > [Protocols] > gEfiFirmwareVolume2ProtocolGuid > gEfiAcpiS3SaveProtocolGuid > gEfiDxeSmmReadyToLockProtocolGuid > + gEsrtManagementProtocolGuid >=20 > [Guids] > gEfiPcAnsiGuid > @@ -70,3 +77,10 @@ > gEfiMdePkgTokenSpaceGuid.PcdUartDefaultStopBits > gEfiMdePkgTokenSpaceGuid.PcdDefaultTerminalType > gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdBootState > + gEfiMdeModulePkgTokenSpaceGuid.PcdSupportUpdateCapsuleReset > + gQuarkPlatformTokenSpaceGuid.PcdEdkiiRsa2048Sha256TestPublicKeyFileGui= d > + gQuarkPlatformTokenSpaceGuid.PcdEdkiiPkcs7TestPublicKeyFileGuid > + gEfiSecurityPkgTokenSpaceGuid.PcdRsa2048Sha256PublicKeyBuffer > + gEfiSecurityPkgTokenSpaceGuid.PcdPkcs7CertBuffer > + gEfiMdeModulePkgTokenSpaceGuid.PcdTestKeyUsed > + > -- > 2.7.4.windows.1 >=20 > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel