On Fri, Feb 05, 2016 at 10:23:18AM +0000, Daniel P. Berrange wrote:
> On Thu, Feb 04, 2016 at 03:57:33PM -0700, Eric Blake wrote:
> > On 01/20/2016 10:38 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
> > > +++ b/crypto/ivgen-plain.c
> > > +static int qcrypto_ivgen_plain_calculate(QCryptoIVGen *ivgen,
> > > + uint64_t sector,
> > > + uint8_t *iv, size_t niv,
> > > + Error **errp)
> > > +{
> > > + size_t ivprefix;
> > > + uint32_t shortsector = cpu_to_le32((uint32_t)(sector & 0xffffffff));
> >
> > Why do you need both the cast and the mask to 32 bits?
>
> I'll remove the cast. I'll also add in
>
> if (shortsector != sector) {
> error_setg(errp, "Sector '%llu' is too large for 'plain' "
> "initialization vector generator",
> (long long unsigned)sector);
> return -1;
> }
[snip]
> > > + *
> > > + * - QCRYPTO_IVGEN_ALG_PLAIN
> > > + *
> > > + * The IVs are generated by the 32-bit truncated sector
> > > + * number. This should never be used for block devices
> > > + * that are larger than 2^32 sectors in size
> >
> > Should the code assert/set errp if sector is too large, rather than
> > blindly truncating it? I guess it is user-triggerable so assert is
> > probably bad, but it may still be nice to tell the user up front that
> > they can't use this mode based on the size of their block device, if we
> > can figure that out.
>
> I put an error check in as mentioned above.
Actually we must not treat this is an error - we must silently truncate
to 32-bits, in order to retain compatibility with Linux dm-crypt formatted
volumes.
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 :|
