From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37376) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZCQiZ-0001fY-D5 for qemu-devel@nongnu.org; Tue, 07 Jul 2015 07:07:16 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZCQiS-0004kw-Cm for qemu-devel@nongnu.org; Tue, 07 Jul 2015 07:07:15 -0400 Received: from szxga03-in.huawei.com ([119.145.14.66]:40736) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZCQiQ-0004i4-U1 for qemu-devel@nongnu.org; Tue, 07 Jul 2015 07:07:08 -0400 Message-ID: <559BB2BD.2040700@huawei.com> Date: Tue, 7 Jul 2015 19:06:37 +0800 From: Gonglei MIME-Version: 1.0 References: <1435770638-25715-1-git-send-email-berrange@redhat.com> <559BA404.9090104@redhat.com> In-Reply-To: <559BA404.9090104@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: Paolo Bonzini , "Daniel P. Berrange" , qemu-devel@nongnu.org Cc: Kevin Wolf , Gerd Hoffmann , Richard Henderson On 2015/7/7 18:03, Paolo Bonzini wrote: > > > 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 > Good :) For series: Reviewed-by: Gonglei Regards, -Gonglei