Re: [Qemu-block] [PATCH v4 09/26] crypto: import an implementation of the XTS cipher mode

[Daniel P. Berrange]

On Fri, Mar 11, 2016 at 12:51:29PM -0700, Eric Blake wrote:
> On 02/29/2016 05:00 AM, Daniel P. Berrange wrote:
> > The XTS (XEX with tweaked-codebook and ciphertext stealing)
> > cipher mode is commonly used in full disk encryption. There
> > is unfortunately no implementation of it in either libgcrypt
> > or nettle, so we need to provide our own.
> > 
> > The libtomcrypt project provides a repository of crypto
> > algorithms under a choice of either "public domain" or
> > the "what the fuck public license".
> > 
> > So this impl is taken from the libtomcrypt GIT repo and
> > adapted to be compatible with the way we need to call
> > ciphers provided by nettle/gcrypt.
> > 
> > Signed-off-by: Daniel P. Berrange <address@hidden>
> > ---
> 
> > +++ b/crypto/xts.c
> > @@ -0,0 +1,256 @@
> > +/*
> > + * QEMU Crypto XTS cipher mode
> > + *
> > + * Copyright (c) 2015 Red Hat, Inc.
> 
> Want to add 2016?
> 
> > +
> > +#include "qemu/osdep.h"
> > +#include "crypto/xts.h"
> > +
> > +static void xts_mult_x(uint8_t *I)
> > +{
> > +    int x;
> > +    uint8_t t, tt;
> > +
> > +    for (x = t = 0; x < 16; x++) {
> > +        tt = I[x] >> 7;
> > +        I[x] = ((I[x] << 1) | t) & 0xFF;
> 
> Why '& 0xf'f? I[x] is already an 8-bit field.  But since it is a direct
> copy from
> https://github.com/libtom/libtomcrypt/blob/develop/src/modes/xts/xts_mult_x.c,
> I won't reject it.  (I could understand the mask if the original code
> were using uint_fast8_t for speed at the expense of worrying about
> potential padding bits, but no one does that in crypto...)

Yeah as you saw, this is just copied as-is from tomcrypt

> > +static void xts_tweak_decrypt(const void *ctx,
> > +                              xts_cipher_func *func,
> > +                              const uint8_t *src,
> > +                              uint8_t *dst,
> > +                              uint8_t *iv)
> > +{
> > +    unsigned long x;
> > +
> > +    /* tweak encrypt block i */
> > +#ifdef LTC_FAST
> > +    for (x = 0; x < XTS_BLOCK_SIZE; x += sizeof(LTC_FAST_TYPE)) {
> > +        *((LTC_FAST_TYPE *)&dst[x]) =
> > +            *((LTC_FAST_TYPE *)&src[x]) ^ *((LTC_FAST_TYPE *)&iv[x]);
> > +    }
> 
> Nothing in our configure sets LTC_FAST and friends; should we just nuke
> these expressions as dead code?  I see the point of what it is trying to
> do: if the data is aligned (or if the processor doesn't care about
> alignment), then vectorize it...

I'll just nuke th blocks inside LTC_FAST - I in fact meant todo that
already before sending. Once all the LUKS stuff is merged we'll look
at getting some complete stack performance tests done and look at
where we want to optimize based on real data.

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 :|