Am Freitag, 7. Februar 2020, 15:07:49 CET schrieb Van Leeuwen, Pascal: Hi Pascal, > Hi Stephan, > > > > -----Original Message----- > > From: linux-crypto-owner@xxxxxxxxxxxxxxx > > <linux-crypto-owner@xxxxxxxxxxxxxxx> On Behalf Of Stephan Mueller Sent: > > Friday, February 7, 2020 8:56 AM > > To: Eric Biggers <ebiggers@xxxxxxxxxx> > > Cc: Gilad Ben-Yossef <gilad@xxxxxxxxxxxxx>; Herbert Xu > > <herbert@xxxxxxxxxxxxxxxxxxx>; Linux Crypto Mailing List <linux- > > crypto@xxxxxxxxxxxxxxx>; Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>; David > > Miller <davem@xxxxxxxxxxxxx>; Ofir Drang <Ofir.Drang@xxxxxxx> > > 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. > > > 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. > 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. The use of the IV as part of the AAD is just a use case for rfc4106. 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. > > > 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. Thus, I would reiterate my suggestion from above to always use req->iv as your IV. > 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