Date: Mon, 13 Jul 2015 15:39:02 -0700 From: John-Mark Gurney <jmg@funkthat.com> To: George Neville-Neil <gnn@freebsd.org> Cc: "Matthew D. Fuller" <fullermd@over-yonder.net>, src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r285336 - in head/sys: netipsec opencrypto Message-ID: <20150713223902.GP8523@funkthat.com> In-Reply-To: <20150713223647.GO8523@funkthat.com> References: <201507091816.t69IGawf097288@repo.freebsd.org> <20150711044843.GG96394@over-yonder.net> <20150711075705.GC8523@funkthat.com> <815B402A-A14F-40F7-91CA-899C7A9597B3@freebsd.org> <20150713223647.GO8523@funkthat.com>
next in thread | previous in thread | raw e-mail | index | archive | help
John-Mark Gurney wrote this message on Mon, Jul 13, 2015 at 15:36 -0700: > George V. Neville-Neil wrote this message on Mon, Jul 13, 2015 at 11:25 -0400: > > On 11 Jul 2015, at 3:57, John-Mark Gurney wrote: > > > > > Matthew D. Fuller wrote this message on Fri, Jul 10, 2015 at 23:48 > > > -0500: > > >> On Thu, Jul 09, 2015 at 06:16:36PM +0000 I heard the voice of > > >> George V. Neville-Neil, and lo! it spake thus: > > >>> New Revision: 285336 > > >>> URL: https://svnweb.freebsd.org/changeset/base/285336 > > >>> > > >>> Log: > > >>> Add support for AES modes to IPSec. These modes work both in > > >>> software only > > >>> mode and with hardware support on systems that have AESNI > > >>> instructions. > > >> > > >> With (apparently) this change, I can trigger a panic at will by > > >> running > > >> > > >> % geli onetime -e AES-XTS -d /dev/ada0s1 > > >> > > >> My best guess is that it comes from > > >> > > >>> -#define RIJNDAEL128_BLOCK_LEN 16 > > >>> +#define AES_MIN_BLOCK_LEN 1 > > >> > > >>> - RIJNDAEL128_BLOCK_LEN, 8, 32, 64, > > >>> + AES_MIN_BLOCK_LEN, AES_XTS_IV_LEN, AES_XTS_MIN_KEY, > > >>> AES_XTS_MAX_KEY, > > >> > > >> changing that first arg from 16 to 1. It seems to be avoided with > > >> the > > >> following patch: > > >> > > >> ------8K-------- > > >> > > >> Index: sys/opencrypto/xform.c > > >> =================================================================== > > >> --- sys/opencrypto/xform.c (revision 285365) > > >> +++ sys/opencrypto/xform.c (working copy) > > >> @@ -257,7 +257,7 @@ > > >> > > >> struct enc_xform enc_xform_aes_xts = { > > >> CRYPTO_AES_XTS, "AES-XTS", > > >> - AES_MIN_BLOCK_LEN, AES_XTS_IV_LEN, AES_XTS_MIN_KEY, > > >> AES_XTS_MAX_KEY, > > >> + AES_BLOCK_LEN, AES_XTS_IV_LEN, AES_XTS_MIN_KEY, AES_XTS_MAX_KEY, > > >> aes_xts_encrypt, > > >> aes_xts_decrypt, > > >> aes_xts_setkey, > > >> > > >> ------8K-------- > > >> > > >> at least in a little testing here. If that's the actual fix, some of > > >> the other MIN_BLOCK_LEN changes in GCM and GMAC are probably suspect > > >> too. > > >> > > >> > > >> (I also wonder why AES-ICM is still using the RIJNDAEL128 #defines; > > >> shouldn't it be using the AES's too? But that's cosmtic...) > > > > > > Our XTS though should be a block size of 1, doesn't implement > > > cipher text stealing, so still must be 16... I assumed that the values > > > of all the defines did not change... That is clearly not the case... > > > > > > gnn, can you please make sure that the tables in xform.c match before > > > your change? If you think there needs to be a value changed, please > > > run it by me.. > > > > > > > Correct, I changed it from the RIJNDAEL value to the "correct" minimum > > value > > of 1. I can do a followup commit to bump it back to 16 if that's what > > you think > > it ought to be. > > The function swcr_encdec requires that the blocksize (of their xform > entry) be the cipher block size and nothing else... If it is anything > else, then the results will be incorrect, and likely buffer overflows > and other bad things may happen... > > Please ensure that ALL constants of the xform tables match exactly what > they were before... If there are any deviations that you believe are > required, let me review them to make sure they won't break anything... > > The name AES_MIN_BLOCK_LEN is bad and needs to be removed... There is > only one AES block size and that is 16... It cannot be longer or > shorter... Any shorter or longer block size is an additional > construction on top of AES and that would need to be added to the > name... P.S. There are tests in /usr/tests that you can run to verify that a large number of the ciphers are working properly. Easiest way to run them: pkg install python nist-kat cd /usr/tests/sys/opencrypto sh runtests You should skip two tests and the others should all pass. -- John-Mark Gurney Voice: +1 415 225 5579 "All that I will do, has been done, All that I have, has not."
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20150713223902.GP8523>