From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by mx.groups.io with SMTP id smtpd.web11.21872.1655406817096912505 for ; Thu, 16 Jun 2022 12:13:37 -0700 Authentication-Results: mx.groups.io; dkim=fail reason="body hash did not verify" header.i=@linux.microsoft.com header.s=default header.b=svZNd3K9; spf=pass (domain: linux.microsoft.com, ip: 13.77.154.182, mailfrom: mikuback@linux.microsoft.com) Received: from [192.168.4.22] (unknown [47.195.228.134]) by linux.microsoft.com (Postfix) with ESMTPSA id 63DC820C32D7; Thu, 16 Jun 2022 12:13:36 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 63DC820C32D7 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1655406816; bh=7laspcrpcxy3Y8g6JE2X0qyw9TlHK5mgTipcDzoBKN0=; h=Date:Subject:From:To:Reply-To:References:In-Reply-To:From; b=svZNd3K9g7Wi5OZgz9CoIRaYCpmrGCItZLBbQp3DK0XRqDCupQSgpMhXINoGZ6LHI dXc/ylIJFvE//S0sSmCEKcg4ZX6XUSzqxdrFAtN0hffdkvFfARFmKuPeyMDj4yS0PW 8tQ64mgjyX8rnPwlDRwr5mNKnsUZ/J1jfQMwtU2M= Message-ID: Date: Thu, 16 Jun 2022 15:13:35 -0400 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.10.0 Subject: Re: [edk2-devel] [Patch v2 00/28] UEFI variable protection From: "Michael Kubacki" To: devel@edk2.groups.io, judah.vang@intel.com Reply-To: devel@edk2.groups.io, mikuback@linux.microsoft.com References: <20220429180430.3292-1-judah.vang@intel.com> <16EFC4965F71DFB8.20068@groups.io> In-Reply-To: <16EFC4965F71DFB8.20068@groups.io> Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable Do you have the data below? This is important for platform adoption and would be useful to include=20 in the following section. https://github.com/judahvang/edk2/tree/rpmc-update#platform-integration-con= siderations Thanks, Michael On 5/16/2022 10:48 PM, Michael Kubacki wrote: > Hi Judah, >=20 > Do you have reference information for the following? >=20 > 1. Overall boot time impact for a sample variable store? >=20 > -=C2=A0 In particular: > =C2=A0 - Initial HMAC calculation/verification time. > =C2=A0 - Non-volatile write impact time to caluclate new store HMAC valu= e=20 > and update MetaDataHmacVar. > =C2=A0 - Variable reclaim before and after time. >=20 > 2. Overall non-volatile store size overhead impact with AES-CBC=20 > encrypted variables? >=20 > I understand these will vary based on system properties like SPI flash=20 > parameters, cryptographic processor details, etc. I'm trying to get an=20 > idea of the impact from sample data or averages on a particular system=20 > configuration. Also to learn whether the native encryption instruction=20 > (AES-NI) was used and if that could provide any benefit given the=20 > potential number of encryption/decryption operations introduced. >=20 > For the code design, I feel the ProtectedVariableLib interface is a bit= =20 > too coupled against internal implementation details of the variable=20 > driver. I generally understand why the code is split out to wrap=20 > operations around the new functionality and it follows the AuthVarLib=20 > pattern but changing the library or driver will continue to require=20 > large changes across both like this due to the coupling. >=20 > Small things I noticed: > 1. VariableKeyLib.inf should not be "BASE", it directly depends on PEI=20 > services > 2. Typo "varabile" in some files > 3. Does ProtectedVariableLibNull actually need to depend on BaseMemoryLib= ? >=20 > Thanks, > Michael >=20 > On 4/29/2022 2:04 PM, Judah Vang wrote: >> For a more detail description of the UEFI variable protected feature=20 >> you can >> view the Readme.md located at the following location: >> https://github.com/judahvang/edk2/tree/rpmc-update >> >> >> Judah Vang (28): >> =C2=A0=C2=A0 MdeModulePkg: Add new GUID for Variable Store Info >> =C2=A0=C2=A0 SecurityPkg: Add new GUIDs for >> =C2=A0=C2=A0 MdeModulePkg: Update AUTH_VARIABLE_INFO struct >> =C2=A0=C2=A0 MdeModulePkg: Add reference to new Ppi Guid >> =C2=A0=C2=A0 MdeModulePkg: Add new ProtectedVariable GUIDs >> =C2=A0=C2=A0 MdeModulePkg: Add new include files >> =C2=A0=C2=A0 MdeModulePkg: Add Null ProtectedVariable Library >> =C2=A0=C2=A0 MdeModulePkg: Add new Variable functionality >> =C2=A0=C2=A0 MdeModulePkg: Add support for Protected Variables >> =C2=A0=C2=A0 SecurityPkg: Add new KeyService types and defines >> =C2=A0=C2=A0 SecurityPkg: Update RPMC APIs with index >> =C2=A0=C2=A0 SecurityPkg: Add new variable types and functions >> =C2=A0=C2=A0 SecurityPkg: Fix GetVariableKey API >> =C2=A0=C2=A0 SecurityPkg: Add null encryption variable libs >> =C2=A0=C2=A0 SecurityPkg: Add VariableKey library function >> =C2=A0=C2=A0 SecurityPkg: Add EncryptionVariable lib with AES >> =C2=A0=C2=A0 SecurityPkg: Add Protected Variable Services >> =C2=A0=C2=A0 MdeModulePkg: Reference Null ProtectedVariableLib >> =C2=A0=C2=A0 SecurityPkg: Add references to new *.inf files >> =C2=A0=C2=A0 ArmVirtPkg: Add reference to ProtectedVariableNull >> =C2=A0=C2=A0 UefiPayloadPkg: Add ProtectedVariable reference >> =C2=A0=C2=A0 EmulatorPkg: Add ProtectedVariable reference >> =C2=A0=C2=A0 OvmfPkg: Add ProtectedVariable reference >> =C2=A0=C2=A0 OvmfPkg: Add ProtectedVariableLib reference >> =C2=A0=C2=A0 OvmfPkg: Add ProtectedVariableLib reference >> =C2=A0=C2=A0 OvmfPkg: Add ProtectedVariableLib reference >> =C2=A0=C2=A0 OvmfPkg: Add ProtectedVariable reference >> =C2=A0=C2=A0 CryptoPkg: Enable cypto HMAC KDF library >> >> =20 >> MdeModulePkg/MdeModulePkg.dec = =20 >> |=C2=A0=C2=A0 13 +- >> =20 >> SecurityPkg/SecurityPkg.dec = =20 >> |=C2=A0=C2=A0 43 +- >> =20 >> ArmVirtPkg/ArmVirtQemu.dsc = =20 >> |=C2=A0=C2=A0=C2=A0 3 +- >> =20 >> EmulatorPkg/EmulatorPkg.dsc = =20 >> |=C2=A0=C2=A0=C2=A0 3 +- >> =20 >> MdeModulePkg/MdeModulePkg.dsc = =20 >> |=C2=A0=C2=A0=C2=A0 4 +- >> =20 >> OvmfPkg/AmdSev/AmdSevX64.dsc = =20 >> |=C2=A0=C2=A0=C2=A0 3 +- >> =20 >> OvmfPkg/Bhyve/BhyveX64.dsc = =20 >> |=C2=A0=C2=A0=C2=A0 3 +- >> =20 >> OvmfPkg/CloudHv/CloudHvX64.dsc = =20 >> |=C2=A0=C2=A0=C2=A0 1 + >> =20 >> OvmfPkg/Microvm/MicrovmX64.dsc = =20 >> |=C2=A0=C2=A0=C2=A0 3 +- >> =20 >> OvmfPkg/OvmfPkgIa32.dsc = =20 >> |=C2=A0=C2=A0=C2=A0 1 + >> =20 >> OvmfPkg/OvmfPkgIa32X64.dsc = =20 >> |=C2=A0=C2=A0=C2=A0 1 + >> =20 >> OvmfPkg/OvmfPkgX64.dsc = =20 >> |=C2=A0=C2=A0=C2=A0 1 + >> =20 >> OvmfPkg/OvmfXen.dsc = =20 >> |=C2=A0=C2=A0=C2=A0 3 +- >> =20 >> SecurityPkg/SecurityPkg.dsc = =20 >> |=C2=A0=C2=A0 13 +- >> =20 >> UefiPayloadPkg/UefiPayloadPkg.dsc = =20 >> |=C2=A0=C2=A0=C2=A0 2 + >> =20 >> CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf = =20 >> |=C2=A0=C2=A0=C2=A0 2 +- >> =20 >> MdeModulePkg/Library/ProtectedVariableLibNull/ProtectedVariableLibNull.i= nf =20 >> |=C2=A0=C2=A0 34 + >> =20 >> MdeModulePkg/Universal/Variable/Pei/VariablePei.inf = =20 >> |=C2=A0=C2=A0 10 +- >> =20 >> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf = =20 >> |=C2=A0=C2=A0=C2=A0 3 +- >> =20 >> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf = =20 >> |=C2=A0=C2=A0=C2=A0 3 +- >> =20 >> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf = =20 >> |=C2=A0=C2=A0=C2=A0 4 +- >> =20 >> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf = =20 >> |=C2=A0=C2=A0=C2=A0 3 +- >> =20 >> SecurityPkg/Library/EncryptionVariableLib/EncryptionVariableLib.inf = =20 >> |=C2=A0=C2=A0 43 + >> =20 >> SecurityPkg/Library/EncryptionVariableLibNull/EncryptionVariableLibNull.= inf=20 >> |=C2=A0=C2=A0 38 + >> =20 >> SecurityPkg/Library/ProtectedVariableLib/DxeProtectedVariableLib.inf = =20 >> |=C2=A0=C2=A0 64 + >> =20 >> SecurityPkg/Library/ProtectedVariableLib/PeiProtectedVariableLib.inf = =20 >> |=C2=A0=C2=A0 68 + >> =20 >> SecurityPkg/Library/ProtectedVariableLib/SmmProtectedVariableLib.inf = =20 >> |=C2=A0=C2=A0 67 + >> =20 >> SecurityPkg/Library/ProtectedVariableLib/SmmRuntimeProtectedVariableLib.= inf=20 >> |=C2=A0=C2=A0 62 + >> =20 >> SecurityPkg/Library/VariableKeyLib/VariableKeyLib.inf = =20 >> |=C2=A0=C2=A0 36 + >> =20 >> MdeModulePkg/Include/Guid/ProtectedVariable.h = =20 >> |=C2=A0=C2=A0 22 + >> =20 >> MdeModulePkg/Include/Library/AuthVariableLib.h = =20 >> |=C2=A0=C2=A0=C2=A0 4 +- >> =20 >> MdeModulePkg/Include/Library/EncryptionVariableLib.h = =20 >> |=C2=A0 165 ++ >> =20 >> MdeModulePkg/Include/Library/ProtectedVariableLib.h = =20 >> |=C2=A0 700 +++++++ >> =20 >> MdeModulePkg/Universal/Variable/Pei/Variable.h = =20 >> |=C2=A0=C2=A0 80 +- >> =20 >> MdeModulePkg/Universal/Variable/Pei/VariableParsing.h = =20 >> |=C2=A0 309 +++ >> =20 >> MdeModulePkg/Universal/Variable/Pei/VariableStore.h = =20 >> |=C2=A0 116 ++ >> =20 >> MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h = =20 >> |=C2=A0 126 +- >> =20 >> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableParsing.h = =20 >> |=C2=A0=C2=A0 91 +- >> =20 >> MdePkg/Include/Ppi/ReadOnlyVariable2.h = =20 >> |=C2=A0=C2=A0=C2=A0 4 +- >> =20 >> SecurityPkg/Include/Library/RpmcLib.h = =20 >> |=C2=A0=C2=A0 15 +- >> =20 >> SecurityPkg/Include/Library/VariableKeyLib.h = =20 >> |=C2=A0=C2=A0 37 +- >> =20 >> SecurityPkg/Include/Ppi/KeyServicePpi.h = =20 >> |=C2=A0=C2=A0 57 + >> =20 >> SecurityPkg/Library/EncryptionVariableLib/EncryptionVariable.h = =20 >> |=C2=A0=C2=A0 49 + >> =20 >> SecurityPkg/Library/ProtectedVariableLib/ProtectedVariableInternal.h = =20 >> |=C2=A0 611 ++++++ >> =20 >> MdeModulePkg/Library/ProtectedVariableLibNull/ProtectedVariable.c = =20 >> |=C2=A0 449 ++++ >> =20 >> MdeModulePkg/Universal/Variable/Pei/Variable.c = =20 >> |=C2=A0 886 ++------ >> =20 >> MdeModulePkg/Universal/Variable/Pei/VariableParsing.c = =20 >> |=C2=A0 941 +++++++++ >> =20 >> MdeModulePkg/Universal/Variable/Pei/VariableStore.c = =20 >> |=C2=A0 305 +++ >> =20 >> MdeModulePkg/Universal/Variable/RuntimeDxe/Reclaim.c = =20 >> |=C2=A0 349 +++- >> =20 >> MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c = =20 >> | 2139 +++++++++++--------- >> =20 >> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c = =20 >> |=C2=A0=C2=A0 26 +- >> =20 >> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableExLib.c = =20 >> |=C2=A0 167 +- >> =20 >> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableNonVolatile.c = =20 >> |=C2=A0 194 +- >> =20 >> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableParsing.c = =20 >> |=C2=A0 320 ++- >> =20 >> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeCache.c = =20 >> |=C2=A0=C2=A0=C2=A0 2 +- >> =20 >> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c = =20 >> |=C2=A0=C2=A0 39 +- >> =20 >> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c = =20 >> |=C2=A0=C2=A0 41 +- >> =20 >> SecurityPkg/Library/EncryptionVariableLib/EncryptionVariable.c = =20 >> |=C2=A0 728 +++++++ >> =20 >> SecurityPkg/Library/EncryptionVariableLibNull/EncryptionVariable.c = =20 >> |=C2=A0 107 + >> =20 >> SecurityPkg/Library/ProtectedVariableLib/ProtectedVariableCommon.c = =20 >> | 2095 +++++++++++++++++++ >> =20 >> SecurityPkg/Library/ProtectedVariableLib/ProtectedVariableDxe.c = =20 >> |=C2=A0 163 ++ >> =20 >> SecurityPkg/Library/ProtectedVariableLib/ProtectedVariablePei.c = =20 >> | 1331 ++++++++++++ >> =20 >> SecurityPkg/Library/ProtectedVariableLib/ProtectedVariableSmm.c = =20 >> |=C2=A0 209 ++ >> =20 >> SecurityPkg/Library/ProtectedVariableLib/ProtectedVariableSmmDxeCommon.c= =20 >> |=C2=A0 975 +++++++++ >> =20 >> SecurityPkg/Library/ProtectedVariableLib/ProtectedVariableSmmRuntime.c = =20 >> |=C2=A0 233 +++ >> =20 >> SecurityPkg/Library/RpmcLibNull/RpmcLibNull.c = =20 >> |=C2=A0=C2=A0=C2=A0 8 +- >> =20 >> SecurityPkg/Library/VariableKeyLib/VariableKeyLib.c = =20 >> |=C2=A0=C2=A0 59 + >> =20 >> SecurityPkg/Library/VariableKeyLibNull/VariableKeyLibNull.c = =20 >> |=C2=A0=C2=A0=C2=A0 6 +- >> =20 >> SecurityPkg/Library/EncryptionVariableLibNull/EncryptionVariableLibNull.= uni=20 >> |=C2=A0=C2=A0 16 + >> =C2=A0 69 files changed, 12845 insertions(+), 1863 deletions(-) >> =C2=A0 create mode 100644=20 >> MdeModulePkg/Library/ProtectedVariableLibNull/ProtectedVariableLibNull.i= nf=20 >> >> =C2=A0 create mode 100644=20 >> SecurityPkg/Library/EncryptionVariableLib/EncryptionVariableLib.inf >> =C2=A0 create mode 100644=20 >> SecurityPkg/Library/EncryptionVariableLibNull/EncryptionVariableLibNull.= inf=20 >> >> =C2=A0 create mode 100644=20 >> SecurityPkg/Library/ProtectedVariableLib/DxeProtectedVariableLib.inf >> =C2=A0 create mode 100644=20 >> SecurityPkg/Library/ProtectedVariableLib/PeiProtectedVariableLib.inf >> =C2=A0 create mode 100644=20 >> SecurityPkg/Library/ProtectedVariableLib/SmmProtectedVariableLib.inf >> =C2=A0 create mode 100644=20 >> SecurityPkg/Library/ProtectedVariableLib/SmmRuntimeProtectedVariableLib.= inf=20 >> >> =C2=A0 create mode 100644=20 >> SecurityPkg/Library/VariableKeyLib/VariableKeyLib.inf >> =C2=A0 create mode 100644 MdeModulePkg/Include/Guid/ProtectedVariable.h >> =C2=A0 create mode 100644 MdeModulePkg/Include/Library/EncryptionVariabl= eLib.h >> =C2=A0 create mode 100644 MdeModulePkg/Include/Library/ProtectedVariable= Lib.h >> =C2=A0 create mode 100644=20 >> MdeModulePkg/Universal/Variable/Pei/VariableParsing.h >> =C2=A0 create mode 100644 MdeModulePkg/Universal/Variable/Pei/VariableSt= ore.h >> =C2=A0 create mode 100644 SecurityPkg/Include/Ppi/KeyServicePpi.h >> =C2=A0 create mode 100644=20 >> SecurityPkg/Library/EncryptionVariableLib/EncryptionVariable.h >> =C2=A0 create mode 100644=20 >> SecurityPkg/Library/ProtectedVariableLib/ProtectedVariableInternal.h >> =C2=A0 create mode 100644=20 >> MdeModulePkg/Library/ProtectedVariableLibNull/ProtectedVariable.c >> =C2=A0 create mode 100644=20 >> MdeModulePkg/Universal/Variable/Pei/VariableParsing.c >> =C2=A0 create mode 100644 MdeModulePkg/Universal/Variable/Pei/VariableSt= ore.c >> =C2=A0 create mode 100644=20 >> SecurityPkg/Library/EncryptionVariableLib/EncryptionVariable.c >> =C2=A0 create mode 100644=20 >> SecurityPkg/Library/EncryptionVariableLibNull/EncryptionVariable.c >> =C2=A0 create mode 100644=20 >> SecurityPkg/Library/ProtectedVariableLib/ProtectedVariableCommon.c >> =C2=A0 create mode 100644=20 >> SecurityPkg/Library/ProtectedVariableLib/ProtectedVariableDxe.c >> =C2=A0 create mode 100644=20 >> SecurityPkg/Library/ProtectedVariableLib/ProtectedVariablePei.c >> =C2=A0 create mode 100644=20 >> SecurityPkg/Library/ProtectedVariableLib/ProtectedVariableSmm.c >> =C2=A0 create mode 100644=20 >> SecurityPkg/Library/ProtectedVariableLib/ProtectedVariableSmmDxeCommon.c >> =C2=A0 create mode 100644=20 >> SecurityPkg/Library/ProtectedVariableLib/ProtectedVariableSmmRuntime.c >> =C2=A0 create mode 100644 SecurityPkg/Library/VariableKeyLib/VariableKey= Lib.c >> =C2=A0 create mode 100644=20 >> SecurityPkg/Library/EncryptionVariableLibNull/EncryptionVariableLibNull.= uni=20 >> >> >=20 >=20 >=20 >=20