| From eb455dbd02cb1074b37872ffca30a81cb2a18eaa Mon Sep 17 00:00:00 2001 |
| From: Eric Biggers <ebiggers@google.com> |
| Date: Sun, 1 Dec 2019 13:53:26 -0800 |
| Subject: crypto: testmgr - don't try to decrypt uninitialized buffers |
| |
| From: Eric Biggers <ebiggers@google.com> |
| |
| commit eb455dbd02cb1074b37872ffca30a81cb2a18eaa upstream. |
| |
| Currently if the comparison fuzz tests encounter an encryption error |
| when generating an skcipher or AEAD test vector, they will still test |
| the decryption side (passing it the uninitialized ciphertext buffer) |
| and expect it to fail with the same error. |
| |
| This is sort of broken because it's not well-defined usage of the API to |
| pass an uninitialized buffer, and furthermore in the AEAD case it's |
| acceptable for the decryption error to be EBADMSG (meaning "inauthentic |
| input") even if the encryption error was something else like EINVAL. |
| |
| Fix this for skcipher by explicitly initializing the ciphertext buffer |
| on error, and for AEAD by skipping the decryption test on error. |
| |
| Reported-by: Pascal Van Leeuwen <pvanleeuwen@verimatrix.com> |
| Fixes: d435e10e67be ("crypto: testmgr - fuzz skciphers against their generic implementation") |
| Fixes: 40153b10d91c ("crypto: testmgr - fuzz AEADs against their generic implementation") |
| Signed-off-by: Eric Biggers <ebiggers@google.com> |
| Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| |
| --- |
| crypto/testmgr.c | 20 ++++++++++++++++---- |
| 1 file changed, 16 insertions(+), 4 deletions(-) |
| |
| --- a/crypto/testmgr.c |
| +++ b/crypto/testmgr.c |
| @@ -2102,6 +2102,7 @@ static void generate_random_aead_testvec |
| * If the key or authentication tag size couldn't be set, no need to |
| * continue to encrypt. |
| */ |
| + vec->crypt_error = 0; |
| if (vec->setkey_error || vec->setauthsize_error) |
| goto done; |
| |
| @@ -2245,10 +2246,12 @@ static int test_aead_vs_generic_impl(con |
| req, tsgls); |
| if (err) |
| goto out; |
| - err = test_aead_vec_cfg(driver, DECRYPT, &vec, vec_name, cfg, |
| - req, tsgls); |
| - if (err) |
| - goto out; |
| + if (vec.crypt_error == 0) { |
| + err = test_aead_vec_cfg(driver, DECRYPT, &vec, vec_name, |
| + cfg, req, tsgls); |
| + if (err) |
| + goto out; |
| + } |
| cond_resched(); |
| } |
| err = 0; |
| @@ -2678,6 +2681,15 @@ static void generate_random_cipher_testv |
| skcipher_request_set_callback(req, 0, crypto_req_done, &wait); |
| skcipher_request_set_crypt(req, &src, &dst, vec->len, iv); |
| vec->crypt_error = crypto_wait_req(crypto_skcipher_encrypt(req), &wait); |
| + if (vec->crypt_error != 0) { |
| + /* |
| + * The only acceptable error here is for an invalid length, so |
| + * skcipher decryption should fail with the same error too. |
| + * We'll test for this. But to keep the API usage well-defined, |
| + * explicitly initialize the ciphertext buffer too. |
| + */ |
| + memset((u8 *)vec->ctext, 0, vec->len); |
| + } |
| done: |
| snprintf(name, max_namelen, "\"random: len=%u klen=%u\"", |
| vec->len, vec->klen); |