On Thu, Feb 04, 2016 at 03:14:10PM -0700, Eric Blake wrote:
> On 01/20/2016 10:38 AM, 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
>
> 'an include/...', or maybe 'a new include/...'?
>
> > 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 <berra...@redhat.com>
> > ---
>
> In addition to Fam's review,
>
> > +++ b/crypto/pbkdf-gcrypt.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,
> > + };
>
> If QCRYPTO_HASH_ gains future enum values, those elements of the array
> will be 0-initialized.
>
> > + int ret;
> > +
> > + if (hash > G_N_ELEMENTS(hash_map)) {
> > + error_setg(errp, "Unexpected hash algorithm %d", hash);
> > + return -1;
> > + }
>
> This checks for beyond the bounds of the array, but not for an element
> that was 0-initialized. Is that a problem we need to worry about?
I'll add '|| hash_map[hash] == GCRY_MD_NONE' to this condition
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 :|
