Re: [Qemu-devel] [PATCH v3 2/4] hw/firmware: Add Edk2Crypto and edk2_add_host_crypto_policy()

[Daniel P . Berrangé]

On Wed, Mar 13, 2019 at 10:43:29AM +0100, Laszlo Ersek wrote:
> On 03/10/19 01:47, Philippe Mathieu-Daudé wrote:
> > The Edk2Crypto object is used to hold configuration values specific
> > to EDK2.
> > 
> > The edk2_add_host_crypto_policy() function loads crypto policies
> > from the host, and register them as fw_cfg named file items.
> > So far only the 'https' policy is supported.
> > 
> > A usercase example is the 'HTTPS Boof' feature of OVMF [*].
> > 
> > Usage example:
> > 
> >   $ qemu-system-x86_64 \
> >       --object edk2_crypto,id=https,\
> >               ciphers=/etc/crypto-policies/back-ends/openssl.config,\
> >               cacerts=/etc/pki/ca-trust/extracted/edk2/cacerts.bin
> > 
> > (On Fedora these files are provided by the ca-certificates and
> > crypto-policies packages).
> > 
> > [*]: https://github.com/tianocore/edk2/blob/master/OvmfPkg/README
> > 
> > Signed-off-by: Philippe Mathieu-Daudé <address@hidden>
> > ---
> > v3:
> > - '-object' -> '--object' in commit description (Eric)
> > - reworded the 'TODO: g_free' comment
> > ---
> >  MAINTAINERS                             |   8 ++
> >  hw/Makefile.objs                        |   1 +
> >  hw/firmware/Makefile.objs               |   1 +
> >  hw/firmware/uefi_edk2_crypto_policies.c | 177 ++++++++++++++++++++++++
> >  include/hw/firmware/uefi_edk2.h         |  28 ++++
> >  5 files changed, 215 insertions(+)
> >  create mode 100644 hw/firmware/Makefile.objs
> >  create mode 100644 hw/firmware/uefi_edk2_crypto_policies.c
> >  create mode 100644 include/hw/firmware/uefi_edk2.h
> > 
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index cf09a4c127..70122b3d0d 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -2206,6 +2206,14 @@ F: include/hw/i2c/smbus_master.h
> >  F: include/hw/i2c/smbus_slave.h
> >  F: include/hw/i2c/smbus_eeprom.h
> >  
> > +EDK2 Firmware
> > +M: Laszlo Ersek <address@hidden>
> > +M: Philippe Mathieu-Daudé <address@hidden>
> > +S: Maintained
> > +F: docs/interop/firmware.json
> > +F: hw/firmware/uefi_edk2_crypto_policies.c
> > +F: include/hw/firmware/uefi_edk2.h
> > +
> 
> I'm not happy with this.
> 
> First, "docs/interop/firmware.json" is meant for more than just EDK2. We
> shouldn't list it in a section called "EDK2 Firmware". I can't suggest
> an alternative (MAINTAINERS is *huge* -- 2500+ lines), but this one
> would be misleading.

We can add arbitrary entries, so I'd would split the above into 2 sections

  Firmware specs
  M: Laszlo Ersek <address@hidden>
  M: Philippe Mathieu-Daudé <address@hidden>
  S: Maintained
  F: docs/interop/firmware.json

  EDK2 Firmware
  M: Laszlo Ersek <address@hidden>
  M: Philippe Mathieu-Daudé <address@hidden>
  S: Maintained
  F: hw/firmware/uefi_edk2_crypto_policies.c
  F: include/hw/firmware/uefi_edk2.h


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|