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;
From:   Stephan Mueller <smueller@chronox.de>
To:     Eric Biggers <ebiggers@kernel.org>
Cc:     Gilad Ben-Yossef <gilad@benyossef.com>,
        Herbert Xu <herbert@gondor.apana.org.au>,
        Linux Crypto Mailing List <linux-crypto@vger.kernel.org>,
        Geert Uytterhoeven <geert@linux-m68k.org>,
        David Miller <davem@davemloft.net>,
        Ofir Drang <Ofir.Drang@arm.com>
Subject: Re: Possible issue with new inauthentic AEAD in extended crypto tests
Date:   Tue, 28 Jan 2020 04:15:11 +0100
Message-ID: <3730881.07ufDO6WFW@tauon.chronox.de>
In-Reply-To: <20200128023455.GC960@sol.localdomain>
References: <CAOtvUMcwLtwgigFE2mx7LVjhhEgcZsSS4WyR_SQ2gixTZxyBfg@mail.gmail.com> <20200128023455.GC960@sol.localdomain>
MIME-Version: 1.0
Content-Transfer-Encoding: 7Bit
Content-Type: text/plain; charset="us-ascii"
Sender: linux-crypto-owner@vger.kernel.org
Precedence: bulk

Am Dienstag, 28. Januar 2020, 03:34:55 CET schrieb Eric Biggers:

Hi Eric,

> On Mon, Jan 27, 2020 at 10:04:26AM +0200, Gilad Ben-Yossef wrote:
> > 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?
> 
> My understanding is that all crypto API functions that take scatterlists
> only forbid zero-length scatterlist elements in the part of the scatterlist
> that's actually passed to the API call.  The input to these functions is
> never simply a scatterlist, but rather a (scatterlist, length) pair. 
> Algorithms shouldn't look beyond 'length', so in the case of 'length == 0',
> they shouldn't look at the scatterlist at all -- which may be just a NULL
> pointer.
> 
> If that's the case, there's no problem with this test code.

I agree with your assessment. Not only when looking at cipher or template 
implementations, but also when looking at the scatterwalk API the SGL length 
field is processed first. If the length field is insufficient then the SGL is 
not processed.
> 
> I'm not sure the comment in aead.h is relevant here.  It sounds like it's
> warning about not providing an empty scatterlist element for the AAD when
> it's followed by a nonempty scatterlist element for the plaintext.  I'm not
> sure it's meant to also cover the case where both are empty.

The statement here (and maybe it could be updated) refers to a valid SGL with 
a size > 0, but where the first SGL entry points to a NULL buffer. This is an 
invalid use of an SGL.

Specifically for AEAD, the SGL must have the form of (assoc data || 
plaintext). As the AAD is not required for a successful cipher operation, the 
caller of the crypto API must guarantee the AAD is either non-NULL or the SGL 
must start with the plaintext as the first entry.
> 
> Herbert and Stephan, any thoughts on what was intended?
> 
> - Eric


Ciao
Stephan