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
References: <CAOtvUMcwLtwgigFE2mx7LVjhhEgcZsSS4WyR_SQ2gixTZxyBfg@mail.gmail.com>
 <20200128023455.GC960@sol.localdomain> <20200128033824.p3z3jhc7mp7wlikp@gondor.apana.org.au>
In-Reply-To: <20200128033824.p3z3jhc7mp7wlikp@gondor.apana.org.au>
From:   Gilad Ben-Yossef <gilad@benyossef.com>
Date:   Tue, 28 Jan 2020 09:24:25 +0200
Message-ID: <CAOtvUMeJmhXL2V74e+LGxDEUJcDy5=f+x0MH86eyHq0u=HvKXw@mail.gmail.com>
Subject: Re: Possible issue with new inauthentic AEAD in extended crypto tests
To:     Herbert Xu <herbert@gondor.apana.org.au>
Cc:     Eric Biggers <ebiggers@kernel.org>,
        Stephan Mueller <smueller@chronox.de>,
        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>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Sender: linux-crypto-owner@vger.kernel.org
Precedence: bulk

On Tue, Jan 28, 2020 at 5:39 AM Herbert Xu <herbert@gondor.apana.org.au> wr=
ote:
>
> On Mon, Jan 27, 2020 at 06:34:55PM -0800, Eric Biggers wrote:
> >
> > My understanding is that all crypto API functions that take scatterlist=
s 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 shoul=
dn't look
> > beyond 'length', so in the case of 'length =3D=3D 0', they shouldn't lo=
ok 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'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 wh=
en 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.
> >
> > Herbert and Stephan, any thoughts on what was intended?
>
> I agree.  I think this is a bug in the driver.
>

Yes, I agree. After debugging it yesterday along with a similar but
not identical issue with the help of Geert it's a bug in the driver
and will send a fix to the root cause shortly.

<rant>
However while working on debugging this it became obvious to me how
convoluted are the requirements for what to expect from the source
scatterlist of an AEAD request from the transformation provider driver
point of view:

- The source is presumed to have enough room for both the associated
data and the plaintext.
- Unless it's in-place encryption, in which case, you also presume to
have room for the authentication tag
- The only way to tell if this is in-place encryption or not is to
compare the pointers to the source and destination - there is no flag.
- Also, if we happen to be dealing with RFC 4106, you also need to
presume to have room for the IV.
- You can count on the scattergather list not having  a first NULL
buffer, *unless* the plaintext and associated data length are both
zero AND it's not in place encryption.
- You can count on not getting NULL as a scatterlist point, *unless*
the plaintext and associated data length are both zero AND it's not in
place encryption. (I'm actually unsure of this one?)
- The behavior of mapping scattergather lists is dependent on the
architecture, platform and configuration - e.g. even turning on
scatterlist DMA mapping debug option did not detect the issue that
Geert is seeing on his arm64 board that do not appear in mine...

So it's no wonder in a sense we got it wrong and judging from some of
the commits for the other driver maintainer I'm not the only one.

I'm not sure there is something actionable here, maybe just clearer
documentation' but it is feel a somewhat brittle API to implement from
a security hardware driver perspective.

Oh well...
</rant>

Thank you all for your help!
Gilad


 --
> Email: Herbert Xu <herbert@gondor.apana.org.au>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt


--=20
Gilad Ben-Yossef
Chief Coffee Drinker

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