public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Michael Kubacki" <mikuback@linux.microsoft.com>
To: devel@edk2.groups.io, judah.vang@intel.com
Subject: Re: [edk2-devel] [Patch v2 00/28] UEFI variable protection
Date: Mon, 16 May 2022 22:48:56 -0400	[thread overview]
Message-ID: <672bd35e-b376-7d7d-dc0c-9b0dc23a9b3c@linux.microsoft.com> (raw)
In-Reply-To: <20220429180430.3292-1-judah.vang@intel.com>

Hi Judah,

Do you have reference information for the following?

1. Overall boot time impact for a sample variable store?

-  In particular:
   - Initial HMAC calculation/verification time.
   - Non-volatile write impact time to caluclate new store HMAC value 
and update MetaDataHmacVar.
   - Variable reclaim before and after time.

2. Overall non-volatile store size overhead impact with AES-CBC 
encrypted variables?

I understand these will vary based on system properties like SPI flash 
parameters, cryptographic processor details, etc. I'm trying to get an 
idea of the impact from sample data or averages on a particular system 
configuration. Also to learn whether the native encryption instruction 
(AES-NI) was used and if that could provide any benefit given the 
potential number of encryption/decryption operations introduced.

For the code design, I feel the ProtectedVariableLib interface is a bit 
too coupled against internal implementation details of the variable 
driver. I generally understand why the code is split out to wrap 
operations around the new functionality and it follows the AuthVarLib 
pattern but changing the library or driver will continue to require 
large changes across both like this due to the coupling.

Small things I noticed:
1. VariableKeyLib.inf should not be "BASE", it directly depends on PEI 
services
2. Typo "varabile" in some files
3. Does ProtectedVariableLibNull actually need to depend on BaseMemoryLib?

Thanks,
Michael

On 4/29/2022 2:04 PM, Judah Vang wrote:
> For a more detail description of the UEFI variable protected feature you can
> view the Readme.md located at the following location:
> https://github.com/judahvang/edk2/tree/rpmc-update
> 
> 
> Judah Vang (28):
>    MdeModulePkg: Add new GUID for Variable Store Info
>    SecurityPkg: Add new GUIDs for
>    MdeModulePkg: Update AUTH_VARIABLE_INFO struct
>    MdeModulePkg: Add reference to new Ppi Guid
>    MdeModulePkg: Add new ProtectedVariable GUIDs
>    MdeModulePkg: Add new include files
>    MdeModulePkg: Add Null ProtectedVariable Library
>    MdeModulePkg: Add new Variable functionality
>    MdeModulePkg: Add support for Protected Variables
>    SecurityPkg: Add new KeyService types and defines
>    SecurityPkg: Update RPMC APIs with index
>    SecurityPkg: Add new variable types and functions
>    SecurityPkg: Fix GetVariableKey API
>    SecurityPkg: Add null encryption variable libs
>    SecurityPkg: Add VariableKey library function
>    SecurityPkg: Add EncryptionVariable lib with AES
>    SecurityPkg: Add Protected Variable Services
>    MdeModulePkg: Reference Null ProtectedVariableLib
>    SecurityPkg: Add references to new *.inf files
>    ArmVirtPkg: Add reference to ProtectedVariableNull
>    UefiPayloadPkg: Add ProtectedVariable reference
>    EmulatorPkg: Add ProtectedVariable reference
>    OvmfPkg: Add ProtectedVariable reference
>    OvmfPkg: Add ProtectedVariableLib reference
>    OvmfPkg: Add ProtectedVariableLib reference
>    OvmfPkg: Add ProtectedVariableLib reference
>    OvmfPkg: Add ProtectedVariable reference
>    CryptoPkg: Enable cypto HMAC KDF library
> 
>   MdeModulePkg/MdeModulePkg.dec                                               |   13 +-
>   SecurityPkg/SecurityPkg.dec                                                 |   43 +-
>   ArmVirtPkg/ArmVirtQemu.dsc                                                  |    3 +-
>   EmulatorPkg/EmulatorPkg.dsc                                                 |    3 +-
>   MdeModulePkg/MdeModulePkg.dsc                                               |    4 +-
>   OvmfPkg/AmdSev/AmdSevX64.dsc                                                |    3 +-
>   OvmfPkg/Bhyve/BhyveX64.dsc                                                  |    3 +-
>   OvmfPkg/CloudHv/CloudHvX64.dsc                                              |    1 +
>   OvmfPkg/Microvm/MicrovmX64.dsc                                              |    3 +-
>   OvmfPkg/OvmfPkgIa32.dsc                                                     |    1 +
>   OvmfPkg/OvmfPkgIa32X64.dsc                                                  |    1 +
>   OvmfPkg/OvmfPkgX64.dsc                                                      |    1 +
>   OvmfPkg/OvmfXen.dsc                                                         |    3 +-
>   SecurityPkg/SecurityPkg.dsc                                                 |   13 +-
>   UefiPayloadPkg/UefiPayloadPkg.dsc                                           |    2 +
>   CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf                              |    2 +-
>   MdeModulePkg/Library/ProtectedVariableLibNull/ProtectedVariableLibNull.inf  |   34 +
>   MdeModulePkg/Universal/Variable/Pei/VariablePei.inf                         |   10 +-
>   MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf           |    3 +-
>   MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf                  |    3 +-
>   MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf        |    4 +-
>   MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf         |    3 +-
>   SecurityPkg/Library/EncryptionVariableLib/EncryptionVariableLib.inf         |   43 +
>   SecurityPkg/Library/EncryptionVariableLibNull/EncryptionVariableLibNull.inf |   38 +
>   SecurityPkg/Library/ProtectedVariableLib/DxeProtectedVariableLib.inf        |   64 +
>   SecurityPkg/Library/ProtectedVariableLib/PeiProtectedVariableLib.inf        |   68 +
>   SecurityPkg/Library/ProtectedVariableLib/SmmProtectedVariableLib.inf        |   67 +
>   SecurityPkg/Library/ProtectedVariableLib/SmmRuntimeProtectedVariableLib.inf |   62 +
>   SecurityPkg/Library/VariableKeyLib/VariableKeyLib.inf                       |   36 +
>   MdeModulePkg/Include/Guid/ProtectedVariable.h                               |   22 +
>   MdeModulePkg/Include/Library/AuthVariableLib.h                              |    4 +-
>   MdeModulePkg/Include/Library/EncryptionVariableLib.h                        |  165 ++
>   MdeModulePkg/Include/Library/ProtectedVariableLib.h                         |  700 +++++++
>   MdeModulePkg/Universal/Variable/Pei/Variable.h                              |   80 +-
>   MdeModulePkg/Universal/Variable/Pei/VariableParsing.h                       |  309 +++
>   MdeModulePkg/Universal/Variable/Pei/VariableStore.h                         |  116 ++
>   MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h                       |  126 +-
>   MdeModulePkg/Universal/Variable/RuntimeDxe/VariableParsing.h                |   91 +-
>   MdePkg/Include/Ppi/ReadOnlyVariable2.h                                      |    4 +-
>   SecurityPkg/Include/Library/RpmcLib.h                                       |   15 +-
>   SecurityPkg/Include/Library/VariableKeyLib.h                                |   37 +-
>   SecurityPkg/Include/Ppi/KeyServicePpi.h                                     |   57 +
>   SecurityPkg/Library/EncryptionVariableLib/EncryptionVariable.h              |   49 +
>   SecurityPkg/Library/ProtectedVariableLib/ProtectedVariableInternal.h        |  611 ++++++
>   MdeModulePkg/Library/ProtectedVariableLibNull/ProtectedVariable.c           |  449 ++++
>   MdeModulePkg/Universal/Variable/Pei/Variable.c                              |  886 ++------
>   MdeModulePkg/Universal/Variable/Pei/VariableParsing.c                       |  941 +++++++++
>   MdeModulePkg/Universal/Variable/Pei/VariableStore.c                         |  305 +++
>   MdeModulePkg/Universal/Variable/RuntimeDxe/Reclaim.c                        |  349 +++-
>   MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c                       | 2139 +++++++++++---------
>   MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c                    |   26 +-
>   MdeModulePkg/Universal/Variable/RuntimeDxe/VariableExLib.c                  |  167 +-
>   MdeModulePkg/Universal/Variable/RuntimeDxe/VariableNonVolatile.c            |  194 +-
>   MdeModulePkg/Universal/Variable/RuntimeDxe/VariableParsing.c                |  320 ++-
>   MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeCache.c           |    2 +-
>   MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c                    |   39 +-
>   MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c          |   41 +-
>   SecurityPkg/Library/EncryptionVariableLib/EncryptionVariable.c              |  728 +++++++
>   SecurityPkg/Library/EncryptionVariableLibNull/EncryptionVariable.c          |  107 +
>   SecurityPkg/Library/ProtectedVariableLib/ProtectedVariableCommon.c          | 2095 +++++++++++++++++++
>   SecurityPkg/Library/ProtectedVariableLib/ProtectedVariableDxe.c             |  163 ++
>   SecurityPkg/Library/ProtectedVariableLib/ProtectedVariablePei.c             | 1331 ++++++++++++
>   SecurityPkg/Library/ProtectedVariableLib/ProtectedVariableSmm.c             |  209 ++
>   SecurityPkg/Library/ProtectedVariableLib/ProtectedVariableSmmDxeCommon.c    |  975 +++++++++
>   SecurityPkg/Library/ProtectedVariableLib/ProtectedVariableSmmRuntime.c      |  233 +++
>   SecurityPkg/Library/RpmcLibNull/RpmcLibNull.c                               |    8 +-
>   SecurityPkg/Library/VariableKeyLib/VariableKeyLib.c                         |   59 +
>   SecurityPkg/Library/VariableKeyLibNull/VariableKeyLibNull.c                 |    6 +-
>   SecurityPkg/Library/EncryptionVariableLibNull/EncryptionVariableLibNull.uni |   16 +
>   69 files changed, 12845 insertions(+), 1863 deletions(-)
>   create mode 100644 MdeModulePkg/Library/ProtectedVariableLibNull/ProtectedVariableLibNull.inf
>   create mode 100644 SecurityPkg/Library/EncryptionVariableLib/EncryptionVariableLib.inf
>   create mode 100644 SecurityPkg/Library/EncryptionVariableLibNull/EncryptionVariableLibNull.inf
>   create mode 100644 SecurityPkg/Library/ProtectedVariableLib/DxeProtectedVariableLib.inf
>   create mode 100644 SecurityPkg/Library/ProtectedVariableLib/PeiProtectedVariableLib.inf
>   create mode 100644 SecurityPkg/Library/ProtectedVariableLib/SmmProtectedVariableLib.inf
>   create mode 100644 SecurityPkg/Library/ProtectedVariableLib/SmmRuntimeProtectedVariableLib.inf
>   create mode 100644 SecurityPkg/Library/VariableKeyLib/VariableKeyLib.inf
>   create mode 100644 MdeModulePkg/Include/Guid/ProtectedVariable.h
>   create mode 100644 MdeModulePkg/Include/Library/EncryptionVariableLib.h
>   create mode 100644 MdeModulePkg/Include/Library/ProtectedVariableLib.h
>   create mode 100644 MdeModulePkg/Universal/Variable/Pei/VariableParsing.h
>   create mode 100644 MdeModulePkg/Universal/Variable/Pei/VariableStore.h
>   create mode 100644 SecurityPkg/Include/Ppi/KeyServicePpi.h
>   create mode 100644 SecurityPkg/Library/EncryptionVariableLib/EncryptionVariable.h
>   create mode 100644 SecurityPkg/Library/ProtectedVariableLib/ProtectedVariableInternal.h
>   create mode 100644 MdeModulePkg/Library/ProtectedVariableLibNull/ProtectedVariable.c
>   create mode 100644 MdeModulePkg/Universal/Variable/Pei/VariableParsing.c
>   create mode 100644 MdeModulePkg/Universal/Variable/Pei/VariableStore.c
>   create mode 100644 SecurityPkg/Library/EncryptionVariableLib/EncryptionVariable.c
>   create mode 100644 SecurityPkg/Library/EncryptionVariableLibNull/EncryptionVariable.c
>   create mode 100644 SecurityPkg/Library/ProtectedVariableLib/ProtectedVariableCommon.c
>   create mode 100644 SecurityPkg/Library/ProtectedVariableLib/ProtectedVariableDxe.c
>   create mode 100644 SecurityPkg/Library/ProtectedVariableLib/ProtectedVariablePei.c
>   create mode 100644 SecurityPkg/Library/ProtectedVariableLib/ProtectedVariableSmm.c
>   create mode 100644 SecurityPkg/Library/ProtectedVariableLib/ProtectedVariableSmmDxeCommon.c
>   create mode 100644 SecurityPkg/Library/ProtectedVariableLib/ProtectedVariableSmmRuntime.c
>   create mode 100644 SecurityPkg/Library/VariableKeyLib/VariableKeyLib.c
>   create mode 100644 SecurityPkg/Library/EncryptionVariableLibNull/EncryptionVariableLibNull.uni
> 

  parent reply	other threads:[~2022-05-17  2:48 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-29 18:04 [Patch v2 00/28] UEFI variable protection Judah Vang
2022-04-29 18:04 ` [Patch v2 01/28] MdeModulePkg: Add new GUID for Variable Store Info Judah Vang
2022-05-12  9:32   ` Wang, Jian J
2022-04-29 18:04 ` [Patch v2 02/28] SecurityPkg: Add new GUIDs for Judah Vang
2022-05-12  9:33   ` Wang, Jian J
2022-04-29 18:04 ` [Patch v2 03/28] MdeModulePkg: Update AUTH_VARIABLE_INFO struct Judah Vang
2022-05-12  9:33   ` Wang, Jian J
2022-04-29 18:04 ` [Patch v2 04/28] MdeModulePkg: Add reference to new Ppi Guid Judah Vang
2022-05-12  9:32   ` Wang, Jian J
2022-04-29 18:04 ` [Patch v2 05/28] MdeModulePkg: Add new ProtectedVariable GUIDs Judah Vang
2022-05-12  9:32   ` Wang, Jian J
2022-04-29 18:04 ` [Patch v2 06/28] MdeModulePkg: Add new include files Judah Vang
2022-05-12  9:31   ` Wang, Jian J
2022-04-29 18:04 ` [Patch v2 07/28] MdeModulePkg: Add Null ProtectedVariable Library Judah Vang
2022-05-22  8:38   ` Wang, Jian J
2022-04-29 18:04 ` [Patch v2 08/28] MdeModulePkg: Add new Variable functionality Judah Vang
2022-05-22 10:24   ` Wang, Jian J
2022-04-29 18:04 ` [Patch v2 09/28] MdeModulePkg: Add support for Protected Variables Judah Vang
2022-05-22 14:03   ` Wang, Jian J
2022-04-29 18:04 ` [Patch v2 10/28] SecurityPkg: Add new KeyService types and defines Judah Vang
2022-05-22 14:06   ` Wang, Jian J
2022-04-29 18:04 ` [Patch v2 11/28] SecurityPkg: Update RPMC APIs with index Judah Vang
2022-05-22 14:07   ` Wang, Jian J
2022-04-29 18:04 ` [Patch v2 12/28] SecurityPkg: Add new variable types and functions Judah Vang
2022-05-22 14:12   ` Wang, Jian J
2022-04-29 18:04 ` [Patch v2 13/28] SecurityPkg: Fix GetVariableKey API Judah Vang
2022-05-22 14:15   ` Wang, Jian J
2022-04-29 18:04 ` [Patch v2 14/28] SecurityPkg: Add null encryption variable libs Judah Vang
2022-05-22 14:20   ` Wang, Jian J
2022-04-29 18:04 ` [Patch v2 15/28] SecurityPkg: Add VariableKey library function Judah Vang
2022-04-29 18:04 ` [Patch v2 16/28] SecurityPkg: Add EncryptionVariable lib with AES Judah Vang
2022-04-29 18:04 ` [Patch v2 17/28] SecurityPkg: Add Protected Variable Services Judah Vang
2022-04-29 18:04 ` [Patch v2 18/28] MdeModulePkg: Reference Null ProtectedVariableLib Judah Vang
2022-04-29 18:04 ` [Patch v2 19/28] SecurityPkg: Add references to new *.inf files Judah Vang
2022-04-29 18:04 ` [Patch v2 20/28] ArmVirtPkg: Add reference to ProtectedVariableNull Judah Vang
2022-05-03  7:29   ` Ard Biesheuvel
2022-04-29 18:04 ` [Patch v2 21/28] UefiPayloadPkg: Add ProtectedVariable reference Judah Vang
2022-04-29 21:03   ` Guo Dong
2022-04-29 18:04 ` [Patch v2 22/28] EmulatorPkg: " Judah Vang
2022-04-29 18:04 ` [Patch v2 23/28] OvmfPkg: " Judah Vang
2022-04-29 18:04 ` [Patch v2 24/28] OvmfPkg: Add ProtectedVariableLib reference Judah Vang
2022-04-29 18:04 ` [Patch v2 25/28] " Judah Vang
2022-04-29 18:04 ` [Patch v2 26/28] " Judah Vang
2022-04-29 18:04 ` [Patch v2 27/28] OvmfPkg: Add ProtectedVariable reference Judah Vang
2022-05-03  7:30   ` [edk2-devel] " Ard Biesheuvel
2022-04-29 18:04 ` [Patch v2 28/28] CryptoPkg: Enable cypto HMAC KDF library Judah Vang
2022-05-17  2:48 ` Michael Kubacki [this message]
     [not found] ` <16EFC4965F71DFB8.20068@groups.io>
2022-06-16 19:13   ` [edk2-devel] [Patch v2 00/28] UEFI variable protection Michael Kubacki
2022-08-23  3:35     ` Michael Kubacki

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=672bd35e-b376-7d7d-dc0c-9b0dc23a9b3c@linux.microsoft.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox