On Fri, Feb 7, 2020 at 9:56 AM Stephan Mueller <smueller@xxxxxxxxxx> wrote: > > 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 > > 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. 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. > It is correct, but is it smart? Either we require the same IV to be passed twice as we do today, in which case passing different IV should fail in a predictable manner OR we should define the operation is taking two IV like structures - one as the IV and one as bytes in the associated data and have the IPsec code use it in a specific way of happen to pass the same IV in both places. I don't care either way - but right now the tests basically relies on undefined behaviour which is always a bad thing, I think. Gilad -- Gilad Ben-Yossef Chief Coffee Drinker values of β will give rise to dom!
