RE: Possible issue with new inauthentic AEAD in extended crypto tests

["Van Leeuwen, Pascal" <pvanleeuwen@rambus.com>]

> -----Original Message-----
> From: linux-crypto-owner@vger.kernel.org <linux-crypto-owner@vger.kernel.org> On Behalf Of RBRpvanleeuwen
> Sent: Friday, February 7, 2020 4:37 PM
> To: Stephan Mueller <smueller@chronox.de>
> Cc: Eric Biggers <ebiggers@kernel.org>; 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
>
> <<< External Email >>>
> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the
> sender/sender address and know the content is safe.
>
>
> > -----Original Message-----
> > From: Stephan Mueller <smueller@chronox.de>
> > Sent: Friday, February 7, 2020 3:29 PM
> > To: Van Leeuwen, Pascal <pvanleeuwen@rambus.com>
> > Cc: Eric Biggers <ebiggers@kernel.org>; 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
> >
> > <<< External Email >>>
> > Am Freitag, 7. Februar 2020, 15:07:49 CET schrieb Van Leeuwen, Pascal:
> >
> > Hi Pascal,
> >
> > > Hi Stephan,
> > >
> > >
> > > > -----Original Message-----
> > > > From: linux-crypto-owner@vger.kernel.org
> > > > <linux-crypto-owner@vger.kernel.org> On Behalf Of Stephan Mueller
> >  Sent:
> > > > Friday, February 7, 2020 8:56 AM
> > > > 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
> > >
> > > >
> > > >
> > > > <<< External Email >>>
> > > > CAUTION: This email originated from outside of the organization. Do not
> > > > click links or open attachments unless you recognize the
> >  sender/sender
> > > > address and know the content is safe.
> > > >
> > > >
> > > >
> > > >
> > > > Am Freitag, 7. Februar 2020, 08:27:09 CET schrieb Eric Biggers:
> > > >
> > > >
> > > >
> > > > Hi Eric,
> > > >
> > > >
> > > >
> > > > > On Wed, Feb 05, 2020 at 04:48:16PM +0200, Gilad Ben-Yossef wrote:
> > > > >
> > > > > > Probably another issue with my driver, but just in case -
> > > > > >
> > > > > >
> > > > > >
> > > > > > include/crypot/aead.h says:
> > > > > >
> > > > > >  * The scatter list pointing to the input data must contain:
> > > > > >  *
> > > > > >  * * for RFC4106 ciphers, the concatenation of
> > > > > >  *   associated authentication data || IV || plaintext or ciphertext.
> > > > > >  Note, the *   same IV (buffer) is also set with the
> > > > > >  aead_request_set_crypt call. Note, *   the API call of
> > > > > >  aead_request_set_ad must provide the length of the AAD and *   the
> > > > > >  IV.
> > > > > >  The API call of aead_request_set_crypt only points to the size of *
> > > > > >  the input plaintext or ciphertext.
> > > > > >
> > > > > >
> > > > > >
> > > > > > I seem to be missing the place where this is handled in
> > > > > > generate_random_aead_testvec()
> > > > > > and generate_aead_message()
> > > > > >
> > > > > >
> > > > > >
> > > > > > We seem to be generating a random IV for providing as the parameter
> > > > > > to
> > > > > > aead_request_set_crypt()
> > > > > > but than have other random bytes set in aead_request_set_ad() - or am
> > > > > > I'm missing something again?
> > > > >
> > > > >
> > > > >
> > > > > Yes, for rfc4106 the tests don't pass the same IV in both places.  This
> > > > > is
> > > > > because I wrote the tests from the perspective of a generic AEAD that
> > > > > doesn't have this weird IV quirk, and then I added the minimum quirks
> > > > > to
> > > > > get the weird algorithms like rfc4106 passing.
> > > > >
> > > > >
> > > > >
> > > > > Since the actual behavior of the generic implementation of rfc4106 is
> > > > > that
> > > > > the last 8 bytes of the AAD are ignored, that means that currently the
> > > > > tests just avoid mutating these bytes when generating inauthentic input
> > > > > tests.  They don't know that they're (apparently) meant to be another
> > > > > copy
> > > > > of the IV.
> > > > >
> > > > >
> > > > >
> > > > > So it seems we need to clearly define the behavior when the two IV
> > > > > copies
> > > > > don't match.  Should one or the other be used, should an error be
> > > > > returned,
> >  or should the behavior be unspecified (in which case the
> > > > > tests would need to be updated)?
> > > > >
> > > > >
> > > > >
> > > > > Unspecified behavior is bad, but it would be easiest for software to
> > > > > use
> > > > > req->iv, while hardware might want to use the IV in the scatterlist...
> > > > >
> > > > >
> > > > >
> > > > > Herbert and Stephan, any idea what was intended here?
> > > > >
> > > > >
> > > > >
> > > > > - Eric
> > > >
> > > >
> > > >
> > > > The full structure of RFC4106 is the following:
> > > >
> > > >
> > > >
> > > > - the key to be set is always 4 bytes larger than required for the
> > > > respective
> >  AES operation (i.e. the key is 20, 28 or 36 bytes
> > > > respectively). The key value contains the following information: key ||
> > > > first 4 bytes of the IV (note, the first 4 bytes of the IV are the bytes
> > > > derived from the KDF invoked by IKE - i.e. they come from user space and
> > > > are fixed)
> > > >
> > > >
> > > >
> > > > - data block contains AAD || trailing 8 bytes of IV || plaintext or
> > > > ciphertext
> >  - the trailing 8 bytes of the IV are the SPI which is updated
> > > > for each new IPSec package
> > > >
> > > >
> > >
> > > By SPI you must mean sequence number?
> > > (The SPI is actually the SA index which certainly doesn't change per
> > > packet!)
> > > That would be one possible way of generating the explicit IV, but
> > > you certainly cannot count on that. Anything unique under the key would be
> > > fine for GCM.
> >
> > The IV actually is generated with an IV generator (I think it is the SEQIV
> > generator from crypto/seqiv.c - it is set in the XFRM framework). It is a
> > deterministic construction XORed with a random number from the SP800-90A DRBG.
> >
> That would be a good way of generating IV's for CBC mode (which requires
> unpredictability and sufficient Hamming distance precluding a counter), but I would not
> recommend that for CTR based modes like GCM, where all you need is a nonce because:
>
> a) randomness does not guarantee uniqueness perse
> b) it is far too heavy on the CPU for this purpose
>
> So I would certainly hope it doesn't do it like that? The name seqiv alone would imply
> something based on a sequence numberand not a DRBG  ... IIRC it was doing sequence
> number XOR some key material?
>
> > >
> > > > aead_request_set_ad points to the AAD plus the 8 bytes of IV in the use
> > > > case
> >  of rfc4106(gcm(aes)) as part of IPSec.
> > > >
> > > >
> > > >
> > > > Considering your question about the aead_request_set_ad vs
> > > > aead_request_set_crypt I think the RFC4106 gives the answer: the IV is
> > > > used in
> >  two locations considering that the IV is also the SPI in our
> > > > case. If you see RFC 4106 chapter 3 you see the trailing 8 bytes of the
> > > > IV as, well, the GCM IV (which is extended by the 4 byte salt as defined
> > > > in chapter 4 that we provide with the trailing 4 bytes of the key). The
> > > > kernel uses the SPI for this.>
> > > >
> > >
> > > Again, by  SPI you must mean sequence number. The SPI itself is entirely
> > > seperate.
> >
> > See above, it is actually not the SPI, or sequence number, it is what the IV
> > generator provides.
> >
> Yes. But what you were describing sounded like the sequence number.
> Which would be perfectly legal to use _directly_ for this (unlike the SPI).
> Thats what our hardware does in case of full protocol offload.
>
> > > So the IV is not "used in two places", it is only used as IV for
> > > the AEAD operation, with the explicit part (8 bytes) inserted into the
> > > packet.
> > > [For GCM the IV, despite being in the AAD buffer,  is _not_ authenticated]
> > > The sequence number _may_ be used in two places (AAD and explicit part of
> > > the IV),
> > > but that is not a given and out of the scope of the crypto API. I
> > > would not make any assumptions there.
> > >
> > > The "problem" Gilad was referring to is that the _explicit_ part of the  IV
> > > appears to be
> > > available  from both req->iv and from the AAD scatterbuffer.
> > > Which one should you use? API wise I would assume req->iv but from a (our)
> > > hardware perspective, it would be more efficient to extract it from the
> > > datastream. But is it allowed to assume there is a valid IV stored there?
> > > (which implies that it has to match req->iv, otherwise behaviour would
> > > deviate from implementations using that)
> >
> > req->iv is your IV.
> >
> But the IV is also in the last bytes of the AAD buffer. Which would be _way_ more
> convenient to use _directly_ compared to req->iv.
> Saves a lot of effort in both the driver and the HW to get the IV to where it
> is actually needed. For _our_ driver and hardware, anyway.
>
> So that's the point: if it's already where you want it to be, then why insisting
> on getting it from a different location (i.e. req->iv) just for the sake of entertaining
> some generic API? These rfcxxxx ciphersuites appear to be for a very specific use
> case  (IPsec) and are already deviating from the normal AEAD implementations.
>
> > The use of the IV as part of the AAD is just a use case for rfc4106.
> >
> No, it is most definitely not. The IV is _not_ part of the AAD for rfc4106.
> Just take another long look at chapter 5 and tell me where it says "IV".
> It's just SPI and (full extended) sequence number, no more.
>
> So actually, the implementation needs to be aware of this and stop
> authenticating IV size bytes before the end of the AAD buffer. Which is
> rather strange if you think about it ... but I guess it is what is is now.
>
Then again, this was probably done to provide a common AEAD API from the
kernel IPsec stack, such that that doesn't need to worry about these ciphersuite
specific details ... and that had to be shoe-horned into the existing kernel crypto
API also making sure it doesn't get inefficient on that side ...

> > Although
> > I doubt that the rfc4106 structure will change any time soon, I would not use
> > the IV from the AAD but only look at the req->iv.
> >
> That is what I did mostly because I didn't know if I could rely on the IV in the
> AAD buffer being correct.
> BUT if you're not allowed to use it from the AAD buffer, then why is it even there?
>
> Let's put it differently: if I _would_ take it from the AAD buffer instead of req->iv,
> the current test vectors from testmgr.h would pass just fine(!)
>
> And how about GMAC (rfc4543), where IV _does_ need to be authenticated. If
> it doesn't match req->iv there, it would certainly not result in output complying
> with rfc4543. (though you could argue this to be useful for _other_ purposes)
>
> I think, especially considering the only user being the kernel IPsec stack here,
> it would make some sense to _require_ req->iv to match the AAD buffer IV and
> allow taking the IV from there instead of from req->iv, if that is more convenient.
>
> > >
> > > > In chapter 5 RFC4106 you see that the SP is however used as part of the
> > > > AAD as
> >  well.
> > > >
> > > >
> > > >
> > > > Bottom line: if you do not set the same IV value for both, the AAD and the
> > > > GCM
> >  IV, you deviate from the use case of rfc4106(gcm(aes)) in IPSec.
> > > > Yet, from a pure mathematical point of view and also from a cipher
> > > > implementation point of view, it does not matter whether the AAD and the
> > > > IV point to the same value - the implementation must always process that
> > > > data. The result however will not be identical to the IPSec use case.
> > > >
> > > >
> > >
> > > For the IPsec use case, it's perfectly legal to have IV != sequence number
> > > as long
> > > as it is unique under the key.
> >
> > Right, it is a perfectly legal way of doing it, but it is currently not done
> > that way in the kernel.
> >
> I guess my main point was there is no "IV for AAD" (if not the sequence number)
> so it can't possibly mismatch the "IV for GCM", ergo it's not possible to deviate from
> any IPsec use case. (for GCM anyway, for GMAC you could)
>
> > Thus, I would reiterate my suggestion from above to always use req->iv as your IV.
> >
> Which is what I do, BUT is rather silly _if_ req->iv in practice will always point to
> the IV stored in the AAD scatter buffer.
>
> > > So you should not assume the sequence number part of the AAD buffer to
> > > match
> > > the IV part (or req->iv), but it _would_ make sense if the IV part
> > > of the AAD matches req->iv. (then again, if this is not _required_ by the
> > > API the application might not bother providing it, which is my reason not
> > > to use in in the inside_secure driver)
> >
> > Precisely.
> >
> > Ciao
> > Stephan
>
> Regards,
> Pascal van Leeuwen
> Silicon IP Architect Multi-Protocol Engines, Rambus Security
> Rambus ROTW Holding BV
> +31-73 6581953
>
> Note: The Inside Secure/Verimatrix Silicon IP team was recently acquired by Rambus.
> Please be so kind to update your e-mail address book with my new e-mail address.
>
> ** This message and any attachments are for the sole use of the intended recipient(s). It may contain information that is confidential
> and privileged. If you are not the intended recipient of this message, you are prohibited from printing, copying, forwarding or saving it.
> Please delete the message and attachments and notify the sender immediately. **
>
> Rambus
> Inc.<https://nam12.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.rambus.com&amp;data=01%7C01%7Cpvanleeuwe
> n%40verimatrix.com%7C5c74040eea5748c69aee08d7abe388cb%7Cdcb260f9022d44958602eae51035a0d0%7C0&amp;sdata=Wlq96le14
> BiueepIAtGY6MykFRcKKcR7JGnYNAYVqPM%3D&amp;reserved=0>

Regards,
Pascal van Leeuwen
Silicon IP Architect Multi-Protocol Engines, Rambus Security
Rambus ROTW Holding BV
+31-73 6581953

Note: The Inside Secure/Verimatrix Silicon IP team was recently acquired by Rambus.
Please be so kind to update your e-mail address book with my new e-mail address.


** This message and any attachments are for the sole use of the intended recipient(s). It may contain information that is confidential and privileged. If you are not the intended recipient of this message, you are prohibited from printing, copying, forwarding or saving it. Please delete the message and attachments and notify the sender immediately. **

Rambus Inc.<http://www.rambus.com>