Received-SPF: pass (google.com: best guess record for domain of linux-crypto-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
From:   Gilad Ben-Yossef <gilad@benyossef.com>
Date:   Mon, 27 Jan 2020 10:04:26 +0200
Message-ID: <CAOtvUMcwLtwgigFE2mx7LVjhhEgcZsSS4WyR_SQ2gixTZxyBfg@mail.gmail.com>
Subject: Possible issue with new inauthentic AEAD in extended crypto tests
To:     Linux Crypto Mailing List <linux-crypto@vger.kernel.org>,
        Eric Biggers <ebiggers@google.com>
Cc:     Geert Uytterhoeven <geert@linux-m68k.org>,
        Herbert Xu <herbert@gondor.apana.org.au>,
        David Miller <davem@davemloft.net>,
        Ofir Drang <Ofir.Drang@arm.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Sender: linux-crypto-owner@vger.kernel.org
Precedence: bulk

Hi,

I've ran into some problems when enabling the extended crypto tests
after commit 49763fc6b1af ("crypto: testmgr - generate inauthentic
AEAD test vectors").
After looking into the matter, I've found something that seems like a
possible problem with the tests, but I am not sure and would
appreciate your consideration:

include/crypto/aead.h has this piece of wisdom to offer:

"* It is important to note that if multiple scatter gather list entries for=
m
 * the input data mentioned above, the first entry must not point to a NULL
 * buffer. If there is any potential where the AAD buffer can be NULL, the
 * calling code must contain a precaution to ensure that this does not resu=
lt
 * in the first scatter gather list entry pointing to a NULL buffer."

However, in generate_random_aead_testvec() we have:

        /* AAD, plaintext, and ciphertext lengths */
        total_len =3D generate_random_length(maxdatasize);
        if (prandom_u32() % 4 =3D=3D 0)
                vec->alen =3D 0;
        else
                vec->alen =3D generate_random_length(total_len);
        vec->plen =3D total_len - vec->alen;
        vec->clen =3D vec->plen + authsize;

Which later calls into generate_aead_message() that has:

                int i =3D 0;
                struct scatterlist src[2], dst;
                u8 iv[MAX_IVLEN];
                DECLARE_CRYPTO_WAIT(wait);

                /* Generate a random plaintext and encrypt it. */
                sg_init_table(src, 2);
                if (vec->alen)
                        sg_set_buf(&src[i++], vec->assoc, vec->alen);
                if (vec->plen) {
                        generate_random_bytes((u8 *)vec->ptext, vec->plen);
                        sg_set_buf(&src[i++], vec->ptext, vec->plen);
                }
                sg_init_one(&dst, vec->ctext, vec->alen + vec->clen);
                memcpy(iv, vec->iv, ivsize);
                aead_request_set_callback(req, 0, crypto_req_done, &wait);
                aead_request_set_crypt(req, src, &dst, vec->plen, iv);
                aead_request_set_ad(req, vec->alen);
                vec->crypt_error =3D crypto_wait_req(crypto_aead_encrypt(re=
q),
                                                   &wait);


When both vec->alen and vec->plen are 0, which can happen as
generate_random_bytes will happily generate  zero length from time to
time,
we seem to be getting a scatterlist with the first entry (as well as
the 2nd) being a NULL.

This seems to violate the words of wisdom from aead.h and much more
important to me crashes the ccree driver :-)

Is there anything I am missing or is this a valid concern?

Thanks!
Gilad

--=20
Gilad Ben-Yossef
Chief Coffee Drinker

values of =CE=B2 will give rise to dom!