Re: [PATCH 0/4] MdeModulePkg, OvmfPkg: support large CA cert list for HTTPS boot

[Gary Lin <glin@suse.com>]

On Wed, Mar 28, 2018 at 10:26:47PM +0200, Laszlo Ersek wrote:
> Repo:   https://github.com/lersek/edk2.git
> Branch: https_cacert_rhbz_1536624
> 
This patch series is great and I like it :)
Configuring the CA list dynamically is really useful and flexible for
the administrator.

I read the code and tested it with my self-signed server, and it worked
as expected.

Reviewed-by: Gary Lin <glin@suse.com>
Tested-by: Gary Lin <glin@suse.com>

> The trusted CA certificates for HTTPS boot can be specified in
> EFI_TLS_CA_CERTIFICATE_VARIABLE. The platform may choose to create this
> variable as volatile and set it on every boot as appropriate. The OVMF
> feature is that the virtualization host passes down an fw_cfg blob that
> carries the CA certs trusted on the host side, and the OVMF HTTPS boot
> will verify web servers against that certificate bundle. (For (part of)
> the host side implementation, refer to
> <https://github.com/p11-glue/p11-kit/pull/137.)
> 
> The challenge for edk2 is that the CA cert list from the host side is
> huge; on my laptop it is 182KB when formatted to the EFI_SIGNATURE_LIST
> sequence expected by NetworkPkg/HttpDxe. Storing this in a non-volatile
> EFI_TLS_CA_CERTIFICATE_VARIABLE is out of the question, but even when
> making EFI_TLS_CA_CERTIFICATE_VARIABLE volatile, there are two limits
> that need raising:
> 
> (1) the individual limit on volatile variables,
> (2) the cumulative limit on volatile variables.
> 
> Regarding (1), the edk2 variable driver does not distinguish a limit for
> volatile non-auth vs. non-volatile non-auth variables. The first patch
> introduces "PcdMaxVolatileVariableSize" for this, in a backwards
> compatible way (i.e. platforms that don't care need not learn about it).
> The new PCD lets a platform raise the individual limit just for volatile
> non-auth variables.
> 
> Regarding (2), OvmfPkg/EmuVariableFvbRuntimeDxe has a bug where it
> abuses the cumulative limit on volatile variables for the live size of
> the emulated non-volatile variable store. The difference is that
> "volatile variables" are volatile on the UEFI service API level
> (gRT->SetVariable() etc), and the driver stack expects the FVB impls to
> use the non-volatile storage PCDs (regardless of the actual FVB backing
> store). Patch #2 fixes this (without change in behavior) in
> OvmfPkg/EmuVariableFvbRuntimeDxe.
> 
> Patch #3 adds a bit of documentation to the OVMF DSC files, as a
> continuation of patch #2.
> 
> Patch #4 implements the feature, raising both limits (liberated in
> earlier patches) and populating EFI_TLS_CA_CERTIFICATE_VARIABLE from
> fw_cfg.
> 
> I've done reasonable HTTPS boot testing and regression testing too
> (including "-bios" with OVMF and pflash with ArmVirtQemu). Indepdent
> testing would be highly appreciated (feature and regression alike).
> 
> This email is too long and so are the commit messages, but I'm too tired
> to trim them; apologies.
> 
> Cc: Anthony Perard <anthony.perard@citrix.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Gary Ching-Pang Lin <glin@suse.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Julien Grall <julien.grall@linaro.org>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> 
> Thanks,
> Laszlo
> 
> Laszlo Ersek (4):
>   MdeModulePkg/Variable/RuntimeDxe: introduce PcdMaxVolatileVariableSize
>   OvmfPkg/EmuVariableFvbRuntimeDxe: stop using PcdVariableStoreSize
>   OvmfPkg: annotate "PcdVariableStoreSize :=
>     PcdFlashNvStorageVariableSize"
>   OvmfPkg/TlsAuthConfigLib: configure trusted CA certs for HTTPS boot
> 
>  MdeModulePkg/MdeModulePkg.dec                                     |   8 ++
>  MdeModulePkg/MdeModulePkg.uni                                     |   8 ++
>  MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c             |  50 ++++++--
>  MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h             |  12 ++
>  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf |   1 +
>  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c          |   2 +-
>  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf        |   1 +
>  OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c                            |   6 +-
>  OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.inf                          |   3 +-
>  OvmfPkg/Library/TlsAuthConfigLib/TlsAuthConfigLib.c               | 133 ++++++++++++++++++++
>  OvmfPkg/Library/TlsAuthConfigLib/TlsAuthConfigLib.inf             |  55 ++++++++
>  OvmfPkg/OvmfPkgIa32.dsc                                           |  15 ++-
>  OvmfPkg/OvmfPkgIa32X64.dsc                                        |  15 ++-
>  OvmfPkg/OvmfPkgX64.dsc                                            |  15 ++-
>  14 files changed, 308 insertions(+), 16 deletions(-)
>  create mode 100644 OvmfPkg/Library/TlsAuthConfigLib/TlsAuthConfigLib.c
>  create mode 100644 OvmfPkg/Library/TlsAuthConfigLib/TlsAuthConfigLib.inf
> 
> -- 
> 2.14.1.3.gb7cf6e02401b
> 
>