From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42339) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZCPjK-0003pO-MB for qemu-devel@nongnu.org; Tue, 07 Jul 2015 06:04:00 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZCPjG-000383-4A for qemu-devel@nongnu.org; Tue, 07 Jul 2015 06:03:58 -0400 Received: from mx1.redhat.com ([209.132.183.28]:43418) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZCPjF-00037U-QW for qemu-devel@nongnu.org; Tue, 07 Jul 2015 06:03:54 -0400 References: <1435770638-25715-1-git-send-email-berrange@redhat.com> From: Paolo Bonzini Message-ID: <559BA404.9090104@redhat.com> Date: Tue, 7 Jul 2015 12:03:48 +0200 MIME-Version: 1.0 In-Reply-To: <1435770638-25715-1-git-send-email-berrange@redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v4 00/10] Consolidate crypto APIs & implementations List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Daniel P. Berrange" , qemu-devel@nongnu.org Cc: Kevin Wolf , Gonglei , Gerd Hoffmann , Richard Henderson On 01/07/2015 19:10, Daniel P. Berrange wrote: > This small series covers the crypto consolidation patches > I previously posted: > > RFC: https://lists.nongnu.org/archive/html/qemu-devel/2015-04/msg02038.html > v1: https://lists.nongnu.org/archive/html/qemu-devel/2015-05/msg04267.html > v2: https://lists.nongnu.org/archive/html/qemu-devel/2015-06/msg00601.html > v3: https://lists.nongnu.org/archive/html/qemu-devel/2015-06/msg05059.html > > Currently there are 5 main places in QEMU which use some > form of cryptographic hash or cipher algorithm. These are > the quorum block driver (hash), qcow{1,2} block driver (cipher), > VNC password auth (cipher), VNC websockets (hash) and some > of the CPU instruction emulation (cipher). > > For ciphers the code is using the in-tree implementations > of AES and/or the RFB cripple-DES. While there is nothing > broken about these implementations, it is none the less > desirable to be able to use the GNUTLS provided impls in > cases where we are already linking to GNUTLS. This will > allow QEMU to use FIPS certified implementations, which > have been well audited, have some protection against > side-channel leakage and are generally actively maintained > by people knowledgable about encryption. > > For hash digests the code is already using GNUTLS APIs. > > With the TLS work, and possible future improved block device > encryption, there will be more general purpose crypto APIs > needed in QEMU. > > It is undesirable to continue to litter the code with > countless #ifdef WITH_GNUTLS conditionals, as it makes > it increasingly hard to understand the code. > > The goal of this series is to thus consolidate all the > crypto code into a single logical place in QEMU - the > source in $GIT/crypto and heads in $GIT/include/crypto > The code in this location will provide QEMU internal > APIs for hash digests, ciphers, and later TLS and block > encryption primitives. The implementations will be > backed by GNUTLS, and either libgcrypt or nettle depending > on which of these GNUTLS is linking to. In the case where > GNUTLS is disabled at build time, we'll still keep the > built-in AES & RFB-cripple-DES implementations available > so we have no regression vs today's level of support. > > The callers of the crypto code can now be unconditionally > compiled and, if needed, they can check the availability > of algorithms they want at runtime and report clear errors > to the CLI or QMP if not available. This is a minor > difference in behaviour for the quorum block driver which > would previously be disabled at compile time if gnutls > was not available. > > A future posting will include the TLS crypto APIs. > > I have not attempted to convert the CPU emulation code to > use the new crypto APIs, since that code appears to have > quite specific need for access to the low level internal > stages of the AES algorithm. So I've left it using the > QEMU built-in AES code. > > I've added myself in the MAINTAINERS file for the new > directories, since it was't clear if anyone else on the > existing QEMU maintainer list had any interest / knowledge > in maintaining the crypto related pieces. > > Changes since v3: > > - Removed need for crypto-internal.h file which was > missing from v3 patches sent. > - Resolve conflicts with error reporting & main loop > API changes / cleanup on master > > Changes since v2: > > - Remove _(..) gettext markers from error messages > - Fix array bounds check in hash module (Richard Henderson) > - Fix null dereference in freeing of gcrypt cipher impl > (Gonglei) > > Changes since v1: > > - Add explicit algorithm constants for each AES key size, > instead of inferring it from array length > - Share code for munging des rfb key bit order > - Share code for validating key array size vs algorithm > - Refactor built-in cipher impl to reduce number of big > switch statements > - Fix uninitialized 'Error *err' var > - Add comments in places where error reporting should be > > Daniel P. Berrange (10): > crypto: introduce new module for computing hash digests > crypto: move built-in AES implementation into crypto/ > crypto: move built-in D3DES implementation into crypto/ > crypto: introduce generic cipher API & built-in implementation > crypto: add a gcrypt cipher implementation > crypto: add a nettle cipher implementation > block: convert quorum blockdrv to use crypto APIs > ui: convert VNC websockets to use crypto APIs > block: convert qcow/qcow2 to use generic cipher API > ui: convert VNC to use generic cipher API > > MAINTAINERS | 7 + > Makefile.objs | 1 + > block/Makefile.objs | 2 +- > block/qcow.c | 102 ++++++--- > block/qcow2-cluster.c | 46 +++- > block/qcow2.c | 96 ++++---- > block/qcow2.h | 13 +- > block/quorum.c | 41 ++-- > configure | 160 +++++++++----- > crypto/Makefile.objs | 5 + > {util => crypto}/aes.c | 2 +- > crypto/cipher-builtin.c | 398 ++++++++++++++++++++++++++++++++++ > crypto/cipher-gcrypt.c | 195 +++++++++++++++++ > crypto/cipher-nettle.c | 206 ++++++++++++++++++ > crypto/cipher.c | 75 +++++++ > ui/d3des.c => crypto/desrfb.c | 2 +- > crypto/hash.c | 200 +++++++++++++++++ > crypto/init.c | 150 +++++++++++++ > include/{qemu => crypto}/aes.h | 0 > include/crypto/cipher.h | 210 ++++++++++++++++++ > ui/d3des.h => include/crypto/desrfb.h | 0 > include/crypto/hash.h | 189 ++++++++++++++++ > include/crypto/init.h | 29 +++ > target-arm/crypto_helper.c | 2 +- > target-i386/fpu_helper.c | 1 - > target-i386/ops_sse.h | 2 +- > target-ppc/int_helper.c | 2 +- > tests/.gitignore | 2 + > tests/Makefile | 4 + > tests/test-crypto-cipher.c | 290 +++++++++++++++++++++++++ > tests/test-crypto-hash.c | 209 ++++++++++++++++++ > ui/Makefile.objs | 4 +- > ui/vnc-ws.c | 22 +- > ui/vnc-ws.h | 2 - > ui/vnc.c | 119 +++++----- > ui/vnc.h | 8 - > util/Makefile.objs | 2 +- > vl.c | 7 + > 38 files changed, 2541 insertions(+), 264 deletions(-) > create mode 100644 crypto/Makefile.objs > rename {util => crypto}/aes.c (99%) > create mode 100644 crypto/cipher-builtin.c > create mode 100644 crypto/cipher-gcrypt.c > create mode 100644 crypto/cipher-nettle.c > create mode 100644 crypto/cipher.c > rename ui/d3des.c => crypto/desrfb.c (99%) > create mode 100644 crypto/hash.c > create mode 100644 crypto/init.c > rename include/{qemu => crypto}/aes.h (100%) > create mode 100644 include/crypto/cipher.h > rename ui/d3des.h => include/crypto/desrfb.h (100%) > create mode 100644 include/crypto/hash.h > create mode 100644 include/crypto/init.h > create mode 100644 tests/test-crypto-cipher.c > create mode 100644 tests/test-crypto-hash.c > Since there were no more objections, I'm sending a pull request for this and assuming that further changes will be handled by you. Paolo