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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

> 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.
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)

> 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.
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)

> Some code to illustrate it - this code is from my CAVS test harness used to
> perform the crypto testing for FIPS 140-2:
>
>
> Preparation of the key:
>
>         /*
>          * RFC4106 special handling: append the first 4 bytes of the IV to
>          * the key. If IV is NULL, append NULL string (i.e. the fixed field is
>          * zero in case of internal IV generation). The first 4 bytes of
>          * the IV must be removed from the IV string.
>          */
>         if (strcasestr(ciphername, "rfc4106")) {
>                 struct buffer rfc;
>
>                 memset(&rfc, 0, sizeof(struct buffer));
>                 if (alloc_buf(data->key.len + 4, &rfc))
>                         goto out;
>
>                 /* copy the key into buffer */
>                 memcpy(rfc.buf, data->key.buf, data->key.len);
>                 if (data->iv.len >= 4) {
>                         uint32_t i = 0;
>
>                         /* Copy first four bytes of the IV into key */
>                         memcpy(rfc.buf + data->key.len, data->iv.buf, 4);
>
>                         /* move remaining bytes to the front to be used as IV
> */
>                         for (i = 0; i < (data->iv.len - 4); i++)
>                                 data->iv.buf[i] = data->iv.buf[(i + 4)];
>                         data->iv.len -= 4;
>                 }
>
>
> Preparation of the SGL - the IV here is the trailing 8 bytes after the
> operation above:
>
>         if (aead_assoc->len) {
>                 if (rfc4106) {
>                         sg_init_table(sg, 3);
>                         sg_set_buf(&sg[0], aead_assoc->data, aead_assoc->len);
>                         sg_set_buf(&sg[1], iv->data, iv->len);
>                         sg_set_buf(&sg[2], data->data, data->len +
>                                 (kccavs_test->type & TYPE_ENC ? authsize :
> 0));
>                 } else {
>                         sg_init_table(sg, 2);
>                         sg_set_buf(&sg[0], aead_assoc->data, aead_assoc->len);
>                         sg_set_buf(&sg[1], data->data, data->len +
>                                 (kccavs_test->type & TYPE_ENC ? authsize :
> 0));
>                 }
>         } else {
>                 if (rfc4106) {
>                         sg_init_table(sg, 2);
>                         sg_set_buf(&sg[0], iv->data, iv->len);
>                         sg_set_buf(&sg[1], data->data, data->len +
>                                 (kccavs_test->type & TYPE_ENC ? authsize :
> 0));
>                 } else {
>                         sg_init_table(sg, 1);
>                         sg_set_buf(&sg[0], data->data, data->len +
>                                 (kccavs_test->type & TYPE_ENC ? authsize :
> 0));
>                 }
>         }
>
>
> Informing the kernel crypto API about the AAD size:
>
>         if (rfc4106)
>                 aead_request_set_ad(req, aead_assoc->len + iv->len);
>         else
>                 aead_request_set_ad(req, aead_assoc->len);
>
>
> Set the buffers:
>
>         aead_request_set_crypt(req, sg, sg, data->len, iv->data);
>
> 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.<http://www.rambus.com>




[Index of Archives]     [Kernel]     [Gnu Classpath]     [Gnu Crypto]     [DM Crypt]     [Netfilter]     [Bugtraq]

  Powered by Linux