Re: [Qemu-devel] [PATCH] quorum: Only compile when supported

[Daniel P. Berrange]

On Tue, Jul 05, 2016 at 10:26:56AM +0100, Daniel P. Berrange wrote:
> On Tue, Jul 05, 2016 at 11:18:29AM +0200, Alberto Garcia wrote:
> > On Tue 05 Jul 2016 10:45:21 AM CEST, Daniel P. Berrange wrote:
> > 
> > > The point of using qcrypto_hash_supports() is that it isolates the
> > > block code Makefile rules from the details of the current specific
> > > impl of the hash APIs in QEMU. As a prime example of why this is
> > > important, try rebasing to GIT master, and you'll find we no longer
> > > use gnutls for the hash APIs. We choose between libgcrypt, nettle or a
> > > empty stub for hash impls now. I think it is a backwards step to add
> > > back these makefile conditionals
> > 
> > Now that you mention this I wonder why we are not using glib for the
> > hashing functions. GChecksum is available since glib 2.16 (QEMU requires
> > 2.22) and it supports MD5, SHA1, SHA256 and SHA512. I see that in git
> > master there's now a few algorithms more, but for the Quorum case those
> > ones are enough.
> 
> The GChecksum API is inadequate for QEMU's needs, due to its limited
> range of algorithms. We absolutely do not want different areas of
> the code using different APIs either. The goal of the crypto APIs is
> to provide a standard internal API for all cryptographic related
> operations for use across the whole codebase. This has clarified much
> of our code by removing countless #ifdef conditionals from the code
> and similar from the build system. It also facilitates people auditing
> QEMU use & implementation of crypto as there is only one place to look
> at to review. It also ensures that QEMU is only using certified secure
> crypto libraries, not some custom re-implementation of the crypto
> algorithms that have never been through a security review. Finally is
> ensures that QEMU correctly responds to runtime configurable changes,
> such as FIPS mode which restricts use of certain crypto algorithms
> at runtime, even if they're technically available at compile time.

Acutally, having said that, what we could do is to replace the no-op
stub hash impl, with a GCheckusum based impl. That way if neither
gcrypt or nettle are available, we can fallback to GChecksum for a
sub-set of the hash algorithms.

That would allow us to move the gcrypto_hash_supports() check out
of the quorum register method, and into its open() method, and
avoid any Makefile.objs conditionals.


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