On Fri, Apr 12, 2019 at 02:04:20PM -0700, Ard Biesheuvel wrote: > On Thu, 11 Apr 2019 at 22:00, Eric Biggers <ebiggers@xxxxxxxxxx> wrote: > > > > Hello, > > > > In the crypto API, all implementations of each algorithm are supposed to > > produce the same results. However, testing of this is currently limited > > to the list of test vectors hardcoded for each algorithm. Although > > after recent improvements the self-tests do much more with each test > > vector, hardcoded test vectors can never cover all cases. > > > > This series improves the situation by making the self-tests > > automatically generate random test vectors using the corresponding > > generic implementation, then run them against the algorithm under test. > > This detects bugs where the implementations don't match. > > > > This has already found many bugs and inconsistencies, including an > > integer overflow bug in the x86_64 implementation of Poly1305. > > > > These new fuzz tests are behind CONFIG_CRYPTO_MANAGER_EXTRA_TESTS. > > > > Patch 1-6 are the testmgr changes themselves. Patch 7 makes the generic > > implementations be registered earlier so that they're available when > > optimized implementations are being tested, when both are built-in. > > Note that even after this, for many algorithms it's still possible to > > make the generic implementation unset or modular. Thus a missing > > generic implementation just causes the comparison tests to be skipped > > with a warning; they aren't failed. > > > > So far I've tested all generic, x86, arm, and arm64 algorithms, plus > > some PowerPC algorithms. I have not tested hardware drivers. I > > encourage people to run the tests on drivers and other architectures, as > > they will find more bugs. > > > > This can also be found in git at: > > > > URL: https://git.kernel.org/pub/scm/linux/kernel/git/ebiggers/linux.git > > Branch: cryptofuzz-vs-generic > > > > Changed since v1: > > > > - Make cryptomgr use arch_initcall(), so we don't rely on the order > > in which the object files are linked. > > > > - Show the expected error code when a test fails due to the wrong > > error code being returned. > > > > - Generate zero-length associated data more often for AEADs > > (about 1/4 of the time rather than about 1/256 of the time). > > > > - A few other minor cleanups. > > > > Eric Biggers (7): > > crypto: testmgr - expand ability to test for errors > > crypto: testmgr - identify test vectors by name rather than number > > crypto: testmgr - add helpers for fuzzing against generic > > implementation > > crypto: testmgr - fuzz hashes against their generic implementation > > crypto: testmgr - fuzz skciphers against their generic implementation > > crypto: testmgr - fuzz AEADs against their generic implementation > > crypto: run initcalls for generic implementations earlier > > > > This looks alright to me, but I have to admit I did not look at every > patch in great detail. I did put it through kernelci, though, and it > built and booted fine on all systems. > > Acked-by: Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx> > Tested-by: Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx> > Thanks Ard. Note that the logs from the kernelci run do show new test failures in two drivers: alg: skcipher: ecb-aes-s5p encryption failed on test vector \"random: len=0 klen=32\"; expected_error=0, actual_error=-22, cfg=\"random: inplace use_final src_divs=[<flush>73.70%@alignmask+4049, 11.97%@+3977, <reimport,nosimd>3.65%@+4013, <reimport>10.68%@alignmask+12] iv_offset=4\" alg: skcipher: cbc-aes-s5p encryption failed on test vector \"random: len=0 klen=32\"; expected_error=0, actual_error=-22, cfg=\"random: use_finup src_divs=[<reimport>100.0%@+22] dst_divs=[100.0%@+258]\" alg: skcipher: blocksize for ctr-aes-s5p (16) doesn't match generic impl (1) alg: skcipher: ecb-aes-rk encryption failed on test vector \"random: len=0 klen=32\"; expected_error=0, actual_error=-22, cfg=\"random: inplace use_digest src_divs=[100.0%@alignmask+2107] iv_offset=38\" So, unlike the generic implementations, the s5p-sss driver doesn't allow empty messages for AES-ECB and AES-CBC, and it sets cra_blocksize for AES-CTR to 16 bytes rather than 1. (It's supposed to be set to 1 for all stream ciphers.) And the Rockchip crypto driver doesn't allow empty messages for AES-ECB. None of these appear super important, but the tests seem to be working as intended to find them, and they'll need to be fixed. - Eric
