svn commit: r285336 - in head/sys: netipsec opencrypto

[John-Mark Gurney]

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."