From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51562) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bdLr2-0000CJ-RX for qemu-devel@nongnu.org; Fri, 26 Aug 2016 14:27:50 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bdLqy-0000jn-K8 for qemu-devel@nongnu.org; Fri, 26 Aug 2016 14:27:47 -0400 Received: from mx1.redhat.com ([209.132.183.28]:54376) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bdLqy-0000jd-Da for qemu-devel@nongnu.org; Fri, 26 Aug 2016 14:27:44 -0400 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 49B4785A06 for ; Fri, 26 Aug 2016 18:27:43 +0000 (UTC) Date: Fri, 26 Aug 2016 14:27:41 -0400 From: "Daniel P. Berrange" Message-ID: <20160826182741.GA18684@redhat.com> Reply-To: "Daniel P. Berrange" References: <1472215666-13263-1-git-send-email-berrange@redhat.com> <57C088BE.3090106@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <57C088BE.3090106@redhat.com> Subject: Re: [Qemu-devel] [PATCH] crypto: ensure XTS is only used with ciphers with 16 byte blocks List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: qemu-devel@nongnu.org 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 > > --- > > 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 > > > > +++ 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 :|