Hi Pascal, On Thu, Jul 04, 2019 at 08:37:11AM +0000, Pascal Van Leeuwen wrote: > Hi, > > I was attempting to get some fuzzing going for the RFC3686 AEAD ciphers I'm adding to the > inside-secure driver, and I noticed some more things besides what I mentioned below: > > 1) If there is no test suite, but the entry does point to something other then alg_test_null, > then fuzzing is still not performed if there is no test suite, as all of the alg_test_xxx routines > first check for suite->count being > 0 and exit due to count being 0 in this case. > I would think that if there are no reference vectors, then fuzzing against the generic > implementation (if enabled) is the very least you can do? > > 2) The AEAD fuzzing routine attempts to determine the maximum key size by actually > scanning the test suite. So if there is no test suite, this will remain at zero and the AEAD > fuzzing routine will still exit without performing any tests because of this. > Isn't there a better way to determine the maximum key size for AEAD ciphers? > > 3) The AEAD fuzzing vector generation generates fully random keydata that is <= maxlen. > However, for AEAD ciphers, the key blob is actually some RTA struct containing length > fields and types. Which means that most of the time, it will simply be generating illegal > key blobs and you are merely testing whether both implementations correctly flag the > key as illegal. (for which they likely use the same crypto_authenc_extractkeys > subroutine, so that check probably/likely always passes - and therefore is not very useful) > Yes, these are real issues; we need to make the testing code smarter and perhaps add some more test vectors too. But just to clarify (since you keep using the more general phrase "AEAD ciphers"), these issues actually only apply to RFC3686 ciphers, a.k.a. algorithms with "authenc" in their name, not to other AEADs in the crypto API such as GCM, ChaCha20-Poly1305, and AEGIS128. There's no way to easily determine the max key size of an arbitrary AEAD currently, since it's not stored in struct aead_alg. That's why the current code is scanning the test vectors. Instead, we probably should store information about the supported key sizes and formats directly in struct alg_test_desc, independent of the test vectors themselves. That would make it possible to solve all three issues you've identified. - Eric
