Re: [PATCH 0/5] crypto: eliminate redundant decryption test vectors

2018-05-30 Thread Herbert Xu
On Sun, May 20, 2018 at 10:50:24PM -0700, Eric Biggers wrote:
> Hello,
> 
> When adding the Speck cipher support I was annoyed by having to add both
> encryption and decryption test vectors, since they are redundant: the
> decryption ones are just the encryption ones with the input and result
> flipped.
> 
> It turns out that's nearly always the case for all the other
> ciphers/skciphers too.  A few have slight differences, but they seem to
> be accidental except for "kw(aes)", and we can still handle "kw(aes)"
> nearly as easily with just one copy of the test vectors.
> 
> Therefore, this series removes all the decryption cipher_testvecs and
> updates testmgr to test both encryption and decryption using what used
> to be the encryption test vectors.  I did not change any of the AEAD
> test vectors, though a similar change could be made for them too.
> 
> Patches 1-4 add some encryption test vectors, just so no test coverage
> is lost.  Patch 5 is the real patch.  Due to the 1+ lines deleted
> from testmgr.h, the patch file is 615 KB so it may be too large for the
> mailing list.  You can also grab the series from git:
> https://github.com/ebiggers/linux, branch "test_vector_redundancy_v1"
> (HEAD is a09e48518f957bb61bb278227917eaad64cf13be).  Most of the patch
> is scripted, but there are also some manual changes, mostly to
> testmgr.c.  For review purposes, in case the full 615 KB patch doesn't
> reach the mailing list, I'm also pasting an abbreviated version of the
> patch below that excludes the scripted changes to testmgr.h, i.e. it
> only includes my manual changes on top of the scripted changes.
> 
> Eric Biggers (5):
>   crypto: testmgr - add extra ecb(des) encryption test vectors
>   crypto: testmgr - make an cbc(des) encryption test vector chunked
>   crypto: testmgr - add extra ecb(tnepres) encryption test vectors
>   crypto: testmgr - add extra kw(aes) encryption test vector
>   crypto: testmgr - eliminate redundant decryption test vectors
> 
>  crypto/testmgr.c |   409 +-
>  crypto/testmgr.h | 12227 -
>  2 files changed, 954 insertions(+), 11682 deletions(-)

All applied.  Thanks.
-- 
Email: Herbert Xu 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt


[PATCH 0/5] crypto: eliminate redundant decryption test vectors

2018-05-20 Thread Eric Biggers
Hello,

When adding the Speck cipher support I was annoyed by having to add both
encryption and decryption test vectors, since they are redundant: the
decryption ones are just the encryption ones with the input and result
flipped.

It turns out that's nearly always the case for all the other
ciphers/skciphers too.  A few have slight differences, but they seem to
be accidental except for "kw(aes)", and we can still handle "kw(aes)"
nearly as easily with just one copy of the test vectors.

Therefore, this series removes all the decryption cipher_testvecs and
updates testmgr to test both encryption and decryption using what used
to be the encryption test vectors.  I did not change any of the AEAD
test vectors, though a similar change could be made for them too.

Patches 1-4 add some encryption test vectors, just so no test coverage
is lost.  Patch 5 is the real patch.  Due to the 1+ lines deleted
from testmgr.h, the patch file is 615 KB so it may be too large for the
mailing list.  You can also grab the series from git:
https://github.com/ebiggers/linux, branch "test_vector_redundancy_v1"
(HEAD is a09e48518f957bb61bb278227917eaad64cf13be).  Most of the patch
is scripted, but there are also some manual changes, mostly to
testmgr.c.  For review purposes, in case the full 615 KB patch doesn't
reach the mailing list, I'm also pasting an abbreviated version of the
patch below that excludes the scripted changes to testmgr.h, i.e. it
only includes my manual changes on top of the scripted changes.

Eric Biggers (5):
  crypto: testmgr - add extra ecb(des) encryption test vectors
  crypto: testmgr - make an cbc(des) encryption test vector chunked
  crypto: testmgr - add extra ecb(tnepres) encryption test vectors
  crypto: testmgr - add extra kw(aes) encryption test vector
  crypto: testmgr - eliminate redundant decryption test vectors

 crypto/testmgr.c |   409 +-
 crypto/testmgr.h | 12227 -
 2 files changed, 954 insertions(+), 11682 deletions(-)

(Abbreviated patch for review purposes only begins here, in case full
 patch is too large for the list; also see git link above)

[PATCH 5/5] crypto: testmgr - eliminate redundant decryption test vectors

Currently testmgr has separate encryption and decryption test vectors
for symmetric ciphers.  That's massively redundant, since with few
exceptions (mostly mistakes, apparently), all decryption tests are
identical to the encryption tests, just with the input/result flipped.

Therefore, eliminate the redundancy by removing the decryption test
vectors and updating testmgr to test both encryption and decryption
using what used to be the encryption test vectors.  Naming is adjusted
accordingly: each cipher_testvec now has a 'ptext' (plaintext), 'ctext'
(ciphertext), and 'len' instead of an 'input', 'result', 'ilen', and
'rlen'.  Note that it was always the case that 'ilen == rlen'.

AES keywrap ("kw(aes)") is special because its IV is generated by the
encryption.  Previously this was handled by specifying 'iv_out' for
encryption and 'iv' for decryption.  To make it work cleanly with only
one set of test vectors, put the IV in 'iv', remove 'iv_out', and add a
boolean that indicates that the IV is generated by the encryption.

In total, this removes over 1 lines from testmgr.h, with no
reduction in test coverage since prior patches already copied the few
unique decryption test vectors into the encryption test vectors.

This covers all algorithms that used 'struct cipher_testvec', e.g. any
block cipher in the ECB, CBC, CTR, XTS, LRW, CTS-CBC, PCBC, OFB, or
keywrap modes, and Salsa20 and ChaCha20.  No change is made to AEAD
tests, though we probably can eliminate a similar redundancy there too.

The testmgr.h portion of this patch was automatically generated using
the following awk script, with some slight manual fixups on top (updated
'struct cipher_testvec' definition, updated a few comments, and fixed up
the AES keywrap test vectors):

BEGIN { OTHER = 0; ENCVEC = 1; DECVEC = 2; DECVEC_TAIL = 3; mode = OTHER }

/^static const struct cipher_testvec.*_enc_/ { sub("_enc", ""); mode = 
ENCVEC }
/^static const struct cipher_testvec.*_dec_/ { mode = DECVEC }
mode == ENCVEC && !/\.ilen[[:space:]]*=/ {
sub(/\.input[[:space:]]*=$/,".ptext =")
sub(/\.input[[:space:]]*=/, ".ptext\t=")
sub(/\.result[[:space:]]*=$/,   ".ctext =")
sub(/\.result[[:space:]]*=/,".ctext\t=")
sub(/\.rlen[[:space:]]*=/,  ".len\t=")
print
}
mode == DECVEC_TAIL && /[^[:space:]]/ { mode = OTHER }
mode == OTHER { print }
mode == ENCVEC && /^};/   { mode = OTHER }
mode == DECVEC && /^};/   { mode = DECVEC_TAIL }

Note that git's default diff algorithm gets confused by the testmgr.h
portion of this patch, and reports too many lines added and removed.
It's better viewed with 'git diff --minimal' (or 'git show --minimal'),