On Tue, Jan 28, 2020 at 5:39 AM Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx> wrote: > > On Mon, Jan 27, 2020 at 06:34:55PM -0800, Eric Biggers wrote: > > > > 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'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. > > > > 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@xxxxxxxxxxxxxxxxxxx> > Home Page: http://gondor.apana.org.au/~herbert/ > PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt -- Gilad Ben-Yossef Chief Coffee Drinker values of β will give rise to dom!
