From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33809) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bKNEQ-0008ST-1A for qemu-devel@nongnu.org; Tue, 05 Jul 2016 06:05:31 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bKNEN-0004l0-Iy for qemu-devel@nongnu.org; Tue, 05 Jul 2016 06:05:28 -0400 Date: Tue, 5 Jul 2016 11:05:17 +0100 From: "Daniel P. Berrange" Message-ID: <20160705100517.GI6553@redhat.com> Reply-To: "Daniel P. Berrange" References: <20160628014747.20971-1-famz@redhat.com> <20160705084521.GD6553@redhat.com> <20160705092656.GE6553@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20160705092656.GE6553@redhat.com> Subject: Re: [Qemu-devel] [PATCH] quorum: Only compile when supported List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alberto Garcia Cc: Kevin Wolf , Fam Zheng , qemu-devel@nongnu.org, qemu-block@nongnu.org, Max Reitz 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 :|