From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50124) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aMCxu-0001K9-TG for qemu-devel@nongnu.org; Thu, 21 Jan 2016 05:59:48 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aMCxt-000286-9f for qemu-devel@nongnu.org; Thu, 21 Jan 2016 05:59:46 -0500 Date: Thu, 21 Jan 2016 10:59:35 +0000 From: "Daniel P. Berrange" Message-ID: <20160121105935.GF19835@redhat.com> References: <1453311539-1193-1-git-send-email-berrange@redhat.com> <1453311539-1193-4-git-send-email-berrange@redhat.com> <20160121065924.GE31960@ad.usersys.redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20160121065924.GE31960@ad.usersys.redhat.com> Subject: Re: [Qemu-devel] [PATCH v2 03/17] crypto: add support for PBKDF2 algorithm Reply-To: "Daniel P. Berrange" List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Fam Zheng Cc: Kevin Wolf , qemu-devel@nongnu.org, qemu-block@nongnu.org On Thu, Jan 21, 2016 at 02:59:24PM +0800, Fam Zheng wrote: > On Wed, 01/20 17:38, Daniel P. Berrange wrote: > > The LUKS data format includes use of PBKDF2 (Password-Based > > Key Derivation Function). The Nettle library can provide > > an implementation of this, but we don't want code directly > > depending on a specific crypto library backend. Introduce > > a include/crypto/pbkdf.h header which defines a QEMU > > API for invoking PBKDK2. The initial implementations are > > backed by nettle & gcrypt, which are commonly available > > with distros shipping GNUTLS. > > > > The test suite data is taken from the cryptsetup codebase > > under the LGPLv2.1+ license. This merely aims to verify > > that whatever backend we provide for this function in QEMU > > will comply with the spec. > > > > Signed-off-by: Daniel P. Berrange > > --- > > crypto/Makefile.objs | 6 +- > > crypto/pbkdf-gcrypt.c | 65 ++++++++ > > crypto/pbkdf-nettle.c | 64 ++++++++ > > crypto/pbkdf-stub.c | 41 +++++ > > crypto/pbkdf.c | 68 +++++++++ > > include/crypto/pbkdf.h | 152 +++++++++++++++++++ > > tests/.gitignore | 1 + > > tests/Makefile | 2 + > > tests/test-crypto-pbkdf.c | 378 ++++++++++++++++++++++++++++++++++++++++++++++ > > 9 files changed, 776 insertions(+), 1 deletion(-) > > create mode 100644 crypto/pbkdf-gcrypt.c > > create mode 100644 crypto/pbkdf-nettle.c > > create mode 100644 crypto/pbkdf-stub.c > > create mode 100644 crypto/pbkdf.c > > create mode 100644 include/crypto/pbkdf.h > > create mode 100644 tests/test-crypto-pbkdf.c > > > > +int qcrypto_pbkdf2(QCryptoHashAlgorithm hash, > > + const uint8_t *key, size_t nkey, > > + const uint8_t *salt, size_t nsalt, > > + unsigned int iterations, > > + uint8_t *out, size_t nout, > > + Error **errp) > > +{ > > + static const int hash_map[QCRYPTO_HASH_ALG__MAX] = { > > + [QCRYPTO_HASH_ALG_MD5] = GCRY_MD_MD5, > > + [QCRYPTO_HASH_ALG_SHA1] = GCRY_MD_SHA1, > > + [QCRYPTO_HASH_ALG_SHA256] = GCRY_MD_SHA256, > > + }; > > + int ret; > > + > > + if (hash > G_N_ELEMENTS(hash_map)) { > > Do you want ">="? Yes. > > > + error_setg(errp, "Unexpected hash algorithm %d", hash); > > + return -1; > > + } > > + > > + ret = gcry_kdf_derive(key, nkey, GCRY_KDF_PBKDF2, > > + hash_map[hash], > > + salt, nsalt, iterations, > > + nout, out); > > We go ahead with QCRYPTO_HASH_ALG_MD5 here, but we didn't accept it in > qcrypto_pbkdf2_supports, is that a mistake? Yes, I should have reported MD5 in the earlier function, since gcrypt allows that. > > diff --git a/crypto/pbkdf.c b/crypto/pbkdf.c > > new file mode 100644 > > index 0000000..71f96cd > > --- /dev/null > > +++ b/crypto/pbkdf.c > > @@ -0,0 +1,68 @@ > > +/* > > + * QEMU Crypto PBKDF support (Password-Based Key Derivation Function) > > + * > > + * Copyright (c) 2015-2016 Red Hat, Inc. > > + * > > + * This library is free software; you can redistribute it and/or > > + * modify it under the terms of the GNU Lesser General Public > > + * License as published by the Free Software Foundation; either > > + * version 2 of the License, or (at your option) any later version. > > + * > > + * This library is distributed in the hope that it will be useful, > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > > + * Lesser General Public License for more details. > > + * > > + * You should have received a copy of the GNU Lesser General Public > > + * License along with this library; if not, see . > > + * > > + */ > > + > > +#include "crypto/pbkdf.h" > > +#include > > + > > + > > +int qcrypto_pbkdf2_count_iters(QCryptoHashAlgorithm hash, > > + const uint8_t *key, size_t nkey, > > + const uint8_t *salt, size_t nsalt, > > + Error **errp) > > +{ > > + uint8_t out[32]; > > + int iterations = (1 << 15); > > To be safe from overflow, I'd use at least 64 bits to do the math, just in case > that some machine is too good at computing this stuff. :) Hmm, probably need to change the return type to be 64 bit too then > > > + struct rusage start, end; > > + unsigned long long delta; > > + > > + while (1) { > > + if (getrusage(RUSAGE_THREAD, &start) < 0) { > > + error_setg_errno(errp, errno, "Unable to get thread CPU usage"); > > + return -1; > > + } > > + if (qcrypto_pbkdf2(hash, > > + key, nkey, > > + salt, nsalt, > > + iterations, > > + out, sizeof(out), > > + errp) < 0) { > > + return -1; > > + } > > + if (getrusage(RUSAGE_THREAD, &end) < 0) { > > + error_setg_errno(errp, errno, "Unable to get thread CPU usage"); > > + return -1; > > + } > > + > > + delta = (((end.ru_utime.tv_sec * 1000ll) + > > + (end.ru_utime.tv_usec / 1000)) - > > + ((start.ru_utime.tv_sec * 1000ll) + > > + (start.ru_utime.tv_usec / 1000))); > > + > > + if (delta > 500) { > > + break; > > + } else if (delta < 100) { > > + iterations = iterations * 10; > > + } else { > > + iterations = (iterations * 1000 / delta); > > + } > > + } > > + > > + return iterations * 1000 / delta; > > +} > > +/** > > + * This module provides an interface to the PBKDF2 algorithm > > + * > > + * https://en.wikipedia.org/wiki/PBKDF2 > > + * > > + * > > + * Generating a AES encryption key from a user password > > s/a AES/an AES/ ? Yes 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 :|