On Wed, Mar 02, 2016 at 05:31:13PM -0700, Eric Blake wrote:
> On 02/29/2016 05:00 AM, Daniel P. Berrange wrote:
> > There are a number of different algorithms that can be used
> > to generate initialization vectors for disk encryption. This
> > introduces a simple internal QCryptoBlockIV object to provide
> > a consistent internal API to the different algorithms. The
> > initially implemented algorithms are 'plain', 'plain64' and
> > 'essiv', each matching the same named algorithm provided
> > by the Linux kernel dm-crypt driver.
> >
> > Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
> > ---
>
> > +++ b/crypto/ivgen-essiv.c
>
> > +static int qcrypto_ivgen_essiv_init(QCryptoIVGen *ivgen,
> > + const uint8_t *key, size_t nkey,
> > + Error **errp)
> > +{
> > + uint8_t *salt;
> > + size_t nhash;
> > + size_t nsalt;
> > + QCryptoIVGenESSIV *essiv = g_new0(QCryptoIVGenESSIV, 1);
> > +
> > + /* Not neccessarily the same as nkey */
>
> s/neccessarily/necessarily/
>
> > +++ b/include/crypto/ivgen.h
>
> > + *
> > + * while (ndata) {
> > + * if (qcrypto_ivgen_calculate(ivgen, sector, iv, niv, errp) < 0) {
> > + * goto error;
> > + * }
> > + * if (qcrypto_cipher_setiv(cipher, iv, niv, errp) < 0) {
> > + * goto error;
> > + * }
> > + * if (qcrypto_cipher_encrypt(cipher,
> > + * data + (sector * 512),
> > + * data + (sector * 512),
> > + * 512, errp) < 0) {
>
> Don't you reuse a single in/out buffer later in the series? If so, don't
> forget to update the comment at that time (the compiler will only catch
> code changes).
In the higher level QCryptoBlock API we encrypt in-place, but this code
example is illustrating the lower layer QCryptoCipher API usage.
> > +##
> > +# QCryptoIVGenAlgorithm:
> > +#
> > +# The supported algorithms for generating initialization
> > +# vectors for full disk encryption. The 'plain' generator
> > +# should not be used for disks with sector numbers larger
> > +# than 2^32, except where compatibility with pre-existing
> > +# Linux dm-crypt volumes is required.
> > +#
> > +# @plain: 64-bit sector number truncated to 32-bits
> > +# @plain64: 64-bit sector number
> > +# @essiv: 64-bit sector number encrypted with a hash of the encryption key
> > +# Since: 2.6
>
> Worth warning that 'plain' and 'plain64' expose the encrypted disk to
> some weaknesses when compared to 'essiv'?
It is a bit more complicated than that. You have to consider the
combination of ivgen + encryption mode to determine if it is
secure or not. eg plain64 + xts is secure but plain64 + cbc is not
secure. I think explaining the security of the various combinations
is too much detail for the QAPI spec here.
> Fixes are minor, so I'm okay if you add:
> Reviewed-by: Eric Blake <ebl...@redhat.com>
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 :|
