Re: git: 9f8f3a8e9ad4 - main - ipsec: add support for CHACHA20POLY1305
- In reply to: John Baldwin : "Re: git: 9f8f3a8e9ad4 - main - ipsec: add support for CHACHA20POLY1305"
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Thu, 03 Nov 2022 07:55:50 UTC
On 2 Nov 2022, at 20:17, John Baldwin wrote:
> On 11/2/22 7:21 AM, Kristof Provost wrote:
>> The branch main has been updated by kp:
>>
>> URL: https://cgit.FreeBSD.org/src/commit/?id=9f8f3a8e9ad4fbdcdfd14eb4d3977e587ab41341
>>
>> commit 9f8f3a8e9ad4fbdcdfd14eb4d3977e587ab41341
>> Author: Kristof Provost <kp@FreeBSD.org>
>> AuthorDate: 2022-10-18 16:31:02 +0000
>> Commit: Kristof Provost <kp@FreeBSD.org>
>> CommitDate: 2022-11-02 13:19:04 +0000
>>
>> ipsec: add support for CHACHA20POLY1305
>> Based on a patch by ae@.
>> Reviewed by: gbe (man page), pauamma (man page)
>> Sponsored by: Rubicon Communications, LLC ("Netgate")
>> Differential Revision: https://reviews.freebsd.org/D37180
>> ---
>> lib/libipsec/pfkey_dump.c | 6 ++++++
>> sbin/setkey/setkey.8 | 4 +++-
>> sbin/setkey/token.l | 2 ++
>> sys/net/pfkeyv2.h | 2 ++
>> sys/netipsec/key.c | 2 ++
>> sys/netipsec/keydb.h | 2 ++
>> sys/netipsec/xform_ah.c | 1 +
>> sys/netipsec/xform_esp.c | 23 +++++++++++++++--------
>> 8 files changed, 33 insertions(+), 9 deletions(-)
>>
>> diff --git a/sys/netipsec/keydb.h b/sys/netipsec/keydb.h
>> index a2da0da613e2..4e55a9abc34b 100644
>> --- a/sys/netipsec/keydb.h
>> +++ b/sys/netipsec/keydb.h
>> @@ -200,6 +200,8 @@ struct secasvar {
>> (_sav)->alg_enc == SADB_X_EALG_AESGCM12 || \
>> (_sav)->alg_enc == SADB_X_EALG_AESGCM16)
>> #define SAV_ISCTR(_sav) ((_sav)->alg_enc == SADB_X_EALG_AESCTR)
>> +#define SAV_ISCHACHA(_sav) \
>> + ((_sav)->alg_enc == SADB_X_EALG_CHACHA20POLY1305)
>> #define SAV_ISCTRORGCM(_sav) (SAV_ISCTR((_sav)) || SAV_ISGCM((_sav)))
>> #define IPSEC_SEQH_SHIFT 32
>> diff --git a/sys/netipsec/xform_ah.c b/sys/netipsec/xform_ah.c
>> index 2600a49ebcdf..a504225ab929 100644
>> --- a/sys/netipsec/xform_ah.c
>> +++ b/sys/netipsec/xform_ah.c
>> @@ -131,6 +131,7 @@ xform_ah_authsize(const struct auth_hash *esph)
>> alen = esph->hashsize / 2; /* RFC4868 2.3 */
>> break;
>> + case CRYPTO_POLY1305:
>> case CRYPTO_AES_NIST_GMAC:
>> alen = esph->hashsize;
>> break;
>> diff --git a/sys/netipsec/xform_esp.c b/sys/netipsec/xform_esp.c
>> index 4a94960fd2e1..4ae081ae7f2a 100644
>> --- a/sys/netipsec/xform_esp.c
>> +++ b/sys/netipsec/xform_esp.c
>> @@ -169,7 +169,8 @@ esp_init(struct secasvar *sav, struct xformsw *xsp)
>> }
>> /* subtract off the salt, RFC4106, 8.1 and RFC3686, 5.1 */
>> - keylen = _KEYLEN(sav->key_enc) - SAV_ISCTRORGCM(sav) * 4;
>> + keylen = _KEYLEN(sav->key_enc) - SAV_ISCTRORGCM(sav) * 4 -
>> + SAV_ISCHACHA(sav) * 4;> if (txform->minkey > keylen || keylen > txform->maxkey) {
>> DPRINTF(("%s: invalid key length %u, must be in the range "
>> "[%u..%u] for algorithm %s\n", __func__,
>> @@ -178,7 +179,7 @@ esp_init(struct secasvar *sav, struct xformsw *xsp)
>> return EINVAL;
>> }
>> - if (SAV_ISCTRORGCM(sav))
>> + if (SAV_ISCTRORGCM(sav) || SAV_ISCHACHA(sav))
>> sav->ivlen = 8; /* RFC4106 3.1 and RFC3686 3.1 */
>> else
>> sav->ivlen = txform->ivsize;
>> @@ -226,6 +227,12 @@ esp_init(struct secasvar *sav, struct xformsw *xsp)
>> csp.csp_mode = CSP_MODE_AEAD;
>> if (sav->flags & SADB_X_SAFLAGS_ESN)
>> csp.csp_flags |= CSP_F_SEPARATE_AAD;
>> + } else if (sav->alg_enc == SADB_X_EALG_CHACHA20POLY1305) {
>> + sav->alg_auth = SADB_X_AALG_CHACHA20POLY1305;
>> + sav->tdb_authalgxform = &auth_hash_poly1305;
>> + csp.csp_mode = CSP_MODE_AEAD;
>> + if (sav->flags & SADB_X_SAFLAGS_ESN)
>> + csp.csp_flags |= CSP_F_SEPARATE_AAD;
>> } else if (sav->alg_auth != 0) {
>> csp.csp_mode = CSP_MODE_ETA;
>> if (sav->flags & SADB_X_SAFLAGS_ESN)
>> @@ -238,7 +245,7 @@ esp_init(struct secasvar *sav, struct xformsw *xsp)
>> if (csp.csp_cipher_alg != CRYPTO_NULL_CBC) {
>> csp.csp_cipher_key = sav->key_enc->key_data;
>> csp.csp_cipher_klen = _KEYBITS(sav->key_enc) / 8 -
>> - SAV_ISCTRORGCM(sav) * 4;
>> + SAV_ISCTRORGCM(sav) * 4 - SAV_ISCHACHA(sav) * 4;
>> };
>> csp.csp_ivlen = txform->ivsize;
>> @@ -368,7 +375,7 @@ esp_input(struct mbuf *m, struct secasvar *sav, int skip, int protoff)
>> if (esph != NULL) {
>> crp->crp_op = CRYPTO_OP_VERIFY_DIGEST;
>> - if (SAV_ISGCM(sav))
>> + if (SAV_ISGCM(sav) || SAV_ISCHACHA(sav))
>> crp->crp_aad_length = 8; /* RFC4106 5, SPI + SN */
>> else
>> crp->crp_aad_length = hlen;
>> @@ -428,7 +435,7 @@ esp_input(struct mbuf *m, struct secasvar *sav, int skip, int protoff)
>> crp->crp_payload_length = m->m_pkthdr.len - (skip + hlen + alen);
>> /* Generate or read cipher IV. */
>> - if (SAV_ISCTRORGCM(sav)) {
>> + if (SAV_ISCTRORGCM(sav) || SAV_ISCHACHA(sav)) {
>> ivp = &crp->crp_iv[0];
>> /*
>> @@ -811,7 +818,7 @@ esp_output(struct mbuf *m, struct secpolicy *sp, struct secasvar *sav,
>> SECREPLAY_UNLOCK(sav->replay);
>> }
>> cryptoid = sav->tdb_cryptoid;
>> - if (SAV_ISCTRORGCM(sav))
>> + if (SAV_ISCTRORGCM(sav) || SAV_ISCHACHA(sav))
>> cntr = sav->cntr++;
>> SECASVAR_RUNLOCK(sav);
>> @@ -878,7 +885,7 @@ esp_output(struct mbuf *m, struct secpolicy *sp, struct secasvar *sav,
>> /* Generate cipher and ESP IVs. */
>> ivp = &crp->crp_iv[0];
>> - if (SAV_ISCTRORGCM(sav)) {
>> + if (SAV_ISCTRORGCM(sav) || SAV_ISCHACHA(sav)) {
>> /*
>> * See comment in esp_input() for details on the
>> * cipher IV. A simple per-SA counter stored in
>> @@ -914,7 +921,7 @@ esp_output(struct mbuf *m, struct secpolicy *sp, struct secasvar *sav,
>> if (esph) {
>> /* Authentication descriptor. */
>> crp->crp_op |= CRYPTO_OP_COMPUTE_DIGEST;
>> - if (SAV_ISGCM(sav))
>> + if (SAV_ISGCM(sav) || SAV_ISCHACHA(sav))
>> crp->crp_aad_length = 8; /* RFC4106 5, SPI + SN */
>> else
>> crp->crp_aad_length = hlen;
>
> For some of these conditionals, I wonder if we want a SAV_IS_AEAD() that is true for both
> GCM and CHACHA20. We might then have a 'SAV_IS_AEAD_OR_CTR()' for the cases that are also
> true for AES-CTR (or just spell it out as separate conditions as you did in a few places
> above). That is, I suspect that many of these things aren't specific to the ciphers in the
> RFCs but are generic to any AEAD ciphersuite for IPsec.
>
Possibly, but I don’t know enough about other IPSec ciphers to make a reasonable judgement.
A lot of these checks look like differences in how the cipher is used (i.e. what data is authenticated, how do we build the IV, ..) more than ‘is this an authenticating cipher’.
Best regards,
Kristof