FYI, I've thought more about ESN and also about what I would need to make TLS work well for TLS 1.1 (and to help with KTLS receive), and my plan is to add a flag to permit the entire AAD to be provided in a separate buffer via a crp_aad pointer as an optional feature (via a new feature flag in csp_flag similar to what I've done for separate in/out buffers in D24545). This is similar to what we do with IVs where an IV can either be inline via crp_iv_start or it can be stored in crp_iv, and it is not specific to ESN but is instead a more generic feature that can be used by different consumers. I was thinking of just having a pointer to the AAD though rather than an inline array as unlike IV's there isn't a clear upper bound on the size of AAD (T6 hardware supports AAD up to 512 bytes for example and 512 bytes is too large of an array to stick inline in crp).

Hi @jhb,

Thank you for your feedback.

Providing separate crp_aad buffer will not work for ESN. It would, if we would take into consideration only combined modes (aes-gcm, aes-ccm), where ESN could be treated as part of AAD (SPI + ESN (32 high order bits) + Seq nr (32 low order bits)).
[ RFC4106, Chapter 5 AAD Construction]

Nevertheless for not-combined modes ESN (high order 32-bits) is not part of AAD. Quote from rfc4303 RFC4303: "the ICV computation encompasses the SPI, Sequence Number, Payload Data, Padding (if present), Pad Length, and  Next Header. ... If the ESN option is enabled for the SA, the high-order 32 bits of the sequence number are appended after the Next Header field for purposes of this computation, but are not transmitted."

Therefore, I am afraid, that trying to provide a more generic feature that could be used by different consumers and not only ESN may not be possible/useful in this case.
Please let me know if I missed something. I am looking forward for your further feedback.

Thank you,
Grzegorz

I think for the AEAD case a separate crp_aad will still be the simplest path and will minimize changes to other drivers to support both ESN and TLS for CSP_MODE_AEAD. I haven't looked, but I'm curious what Chacha20 + Poly1035 for IPsec uses. Hopefully it follows the AEAD route. Just adding a new request flag alone is probably not ideal as other drivers might claim the session and fail to support the new flag. I think at a minimum you will have to add a new flag for ESN support in the non-AEAD case in csp_flags so that drivers that don't support the mode won't claim it. It looks like a flag though might be better than a new CSP_MODE_ESN that implies CSP_MODE_ETA since you can use ESN's with auth-only sessions.

Hi @jhb

Thank you for your feedback.

1. Fortunately for Chacha20 + Poly1305: the AAD construction is the same as for AES-GCM, AES-CCM modes (RFC7634 - AAD).

2. I think I get your point but just to make sure that I've understand correctly:

• For AEAD case we should use separate buffer via crp_aad pointer, which will be used instead crp_aad_start in case of Ipsec ESN. This will work only for drivers which "advertise" (via *_probesession e.g. aesni_probesession) support for an optional feature via csp_flag e.g. something like CSP_F_AAD_SEPARATE. For drivers that do not advertise CSP_F_AAD_SEPARATE the *_probesession will simply fail with EINVAL.

• For ETA case we should use both: crp_aad_start as it is, and additionally use separate buffer via crp_aad pointer. This will work only for drivers which "advertise" support for an optional feature via csp_flag e.g. something like CSP_F_ESN. In this case the crp_aad will hold only high-order 32 bits of the seq number and not entire AAD.

Could you please confirm above? Looking forward to your feedback.

Best regards,
Grzegorz

In D24838#550268, @jaz_semihalf.com wrote:

Hi @jhb

Thank you for your feedback.

1. Fortunately for Chacha20 + Poly1305: the AAD construction is the same as for AES-GCM, AES-CCM modes (RFC7634 - AAD).

2. I think I get your point but just to make sure that I've understand correctly:

• For AEAD case we should use separate buffer via crp_aad pointer, which will be used instead crp_aad_start in case of Ipsec ESN. This will work only for drivers which "advertise" (via *_probesession e.g. aesni_probesession) support for an optional feature via csp_flag e.g. something like CSP_F_AAD_SEPARATE. For drivers that do not advertise CSP_F_AAD_SEPARATE the *_probesession will simply fail with EINVAL.

• For ETA case we should use both: crp_aad_start as it is, and additionally use separate buffer via crp_aad pointer. This will work only for drivers which "advertise" support for an optional feature via csp_flag e.g. something like CSP_F_ESN. In this case the crp_aad will hold only high-order 32 bits of the seq number and not entire AAD.

Could you please confirm above? Looking forward to your feedback.

Yes, I think that all sounds good. I might even work on the generic crp_aad support this week for some KTLS related work. The use of crp_aad to hold the ETA case for ESN is kind of interesting, but I think I probably prefer to just use the crp_esn field you have in the existing patch so it is a bit clearer. In theory a driver might even support the initial AAD via crp_aad while still supporting ESN for IPsec. More likely though, I think it will be simpler to implement the right thing in drivers if they are able to assume that crp_aad is the AAD region when it is present. In the case of separate AAD, it might be simpler to use crp_aad != NULL instead of a new CRYPTO_F flag since it will be a pointer one can check against NULL (the way the per-op keys work) unlike the IV case where the array is inline. Also, using crp_esn means the two patches will compose a bit easier, so you can start work on the ESN thing now if I do work on crp_aad this week and we won't have any major conflicts (the only one would probably be the CSP_F_* values which is trivial to resolve). Does that make sense?

Yes it make sense and thank you for feedback. Just please let me know when you will have some patches around KTLS and crp_aad ready.

@jhb thank you for your review and approval. I just noticed that by accident you weren't added as reviewer for other patches related to ESN. I've just fixed that and will appreciate your review.

One thing I forgot is that the new session flag needs to be documented in crypto_session.9, and crp_esn should be documented in crypto_request.9.