Re: [Qemu-devel] [PATCH] crypto: ensure XTS is only used with ciphers with 16 byte blocks

[Daniel P. Berrange]

On Fri, Aug 26, 2016 at 01:21:50PM -0500, Eric Blake wrote:
> On 08/26/2016 07:47 AM, Daniel P. Berrange wrote:
> > The XTS cipher mode needs to be used with a cipher which has
> > a block size of 16 bytes. If a mis-matching block size is used,
> > the code will either corrupt memory beyond the IV array, or
> > not fully encrypt/decrypt the IV.
> > 
> > This fixes a memory curruption crash when attempting to use
> 
> s/curruption/corruption/
> 
> > cast5-128 with xts, since the former has an 8 byte block size.
> > 
> > A test case is added to ensure the cipher creation fails with
> > such an invalid combination.
> > 
> > Signed-off-by: Daniel P. Berrange <address@hidden>
> > ---
> >  crypto/cipher-gcrypt.c     |  6 ++++++
> >  crypto/cipher-nettle.c     | 12 +++++++-----
> >  tests/test-crypto-cipher.c | 44 
> > ++++++++++++++++++++++++++++++++++++--------
> >  3 files changed, 49 insertions(+), 13 deletions(-)
> 
> Are you aiming for a last-minute 2.7 fix, or should this just be 2.8
> material and cc qemu-stable?

This isn't critical for 2.7, as this is already invalid usage. IOW anyone
who hits the crash, simply shouldn't use this combination.

> Reviewed-by: Eric Blake <address@hidden>
> 
> 
> > +++ b/tests/test-crypto-cipher.c
> > @@ -370,6 +370,17 @@ static QCryptoCipherTestData test_data[] = {
> 
> > @@ -449,8 +468,16 @@ static void test_cipher(const void *opaque)
> >      cipher = qcrypto_cipher_new(
> >          data->alg, data->mode,
> >          key, nkey,
> > -        &error_abort);
> > -    g_assert(cipher != NULL);
> > +        &err);
> > +    if (data->plaintext) {
> > +        g_assert(err == NULL);
> > +        g_assert(cipher != NULL);
> > +    } else {
> > +        g_assert(err != NULL);
> > +        error_free(err);
> 
> Could shorten these two lines as error_free_or_abort(&err), but that's
> cosmetic.

Will do.


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