[libvirt] [PATCH 1/3] firmware: include arch and features in firmware file list

[Daniel P. Berrange]

On Wed, Oct 19, 2016 at 01:07:25PM +0100, Richard W.M. Jones wrote:
> On Mon, Oct 03, 2016 at 04:49:47PM +0100, Daniel P. Berrange wrote:
> > --- a/src/qemu/qemu.conf
> > +++ b/src/qemu/qemu.conf
> > @@ -595,16 +595,22 @@
> >  # using this master file as image. Each UEFI firmware can,
> >  # however, have different variables store. Therefore the nvram is
> >  # a list of strings when a single item is in form of:
> > -#   ${PATH_TO_UEFI_FW}:${PATH_TO_UEFI_VARS}.
> > +#
> > +#   ${PATH_TO_UEFI_FW}:${PATH_TO_UEFI_VARS}:${ARCH}[:${FEATURE}:...].
> > +#
> > +# Current valid features include:
> > +#
> > +#   'secboot' - the firmware has secure boot enabled
> > +#
> >  # Later, when libvirt creates per domain variable store, this list is
> >  # searched for the master image. The UEFI firmware can be called
> >  # differently for different guest architectures. For instance, it's OVMF
> >  # for x86_64 and i686, but it's AAVMF for aarch64. The libvirt default
> >  # follows this scheme.
> >  #nvram = [
> > -#   "/usr/share/OVMF/OVMF_CODE.fd:/usr/share/OVMF/OVMF_VARS.fd",
> > -#   "/usr/share/OVMF/OVMF_CODE.secboot.fd:/usr/share/OVMF/OVMF_VARS.fd",
> > -#   "/usr/share/AAVMF/AAVMF_CODE.fd:/usr/share/AAVMF/AAVMF_VARS.fd"
> > +#   "/usr/share/OVMF/OVMF_CODE.fd:/usr/share/OVMF/OVMF_VARS.fd:x86_64",
> > +#   "/usr/share/OVMF/OVMF_CODE.secboot.fd:/usr/share/OVMF/OVMF_VARS.fd:x86_64:secboot",
> > +#   "/usr/share/AAVMF/AAVMF_CODE.fd:/usr/share/AAVMF/AAVMF_VARS.fd:aarch64"
> 
> This is all good, and could remove a duplicate copy of this database
> which is currently stored in libguestfs & virt-v2v:
> 
> https://github.com/libguestfs/libguestfs/blob/master/generator/uefi.ml#L30
> 
> The flags (arch, secboot) even precisely match the ones we currently
> need to store.
> 
> Unfortunately it's a case of so near and yet so far.  You're proposing
> this essentially static and non-secret data be stored in
> /etc/libvirt/qemu.conf, which is not readable as non-root.  virt-v2v
> (which can run as non-root) would still need to store a duplicate copy
> of the data.

What does v2v need the mapping data for ?  Any use case needs to be
addressed via the APIs, not having apps poke at libvirt private
config files.

> I don't see any need for config files to default to unreadable, it's
> just security through obscurity (and not even obscure), but assuming
> that isn't going to change, please put this into a different file
> which can be read as non-root.  There is literally nothing possibly
> secret about it, it's just the location of some files.

Even if the config files were readable, this data is not going
to be present in the config file by default - it'll only be
there if the admin has needed to override libvirts builtin
default value.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://entangle-photo.org       -o-    http://search.cpan.org/~danberr/ :|