Date: Thu, 03 Nov 2022 08:55:50 +0100 From: Kristof Provost <kp@FreeBSD.org> To: John Baldwin <jhb@FreeBSD.org> Cc: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: Re: git: 9f8f3a8e9ad4 - main - ipsec: add support for CHACHA20POLY1305 Message-ID: <3084EABF-AFC1-4DFF-ACD6-F0DC39E05764@FreeBSD.org> In-Reply-To: <580620b9-5a11-4437-75f9-8dc2dc839007@FreeBSD.org> References: <202211021421.2A2ELuBA032661@gitrepo.freebsd.org> <580620b9-5a11-4437-75f9-8dc2dc839007@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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=3D9f8f3a8e9ad4fbdcdfd14eb= 4d3977e587ab41341 >> >> 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 =3D=3D SADB_X_EALG_AESGCM12 || \ >> (_sav)->alg_enc =3D=3D SADB_X_EALG_AESGCM16) >> #define SAV_ISCTR(_sav) ((_sav)->alg_enc =3D=3D SADB_X_EALG_AESCTR) >> +#define SAV_ISCHACHA(_sav) \ >> + ((_sav)->alg_enc =3D=3D 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 =3D esph->hashsize / 2; /* RFC4868 2.3 */ >> break; >> + case CRYPTO_POLY1305: >> case CRYPTO_AES_NIST_GMAC: >> alen =3D 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 =3D _KEYLEN(sav->key_enc) - SAV_ISCTRORGCM(sav) * 4; >> + keylen =3D _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 =3D 8; /* RFC4106 3.1 and RFC3686 3.1 */ >> else >> sav->ivlen =3D txform->ivsize; >> @@ -226,6 +227,12 @@ esp_init(struct secasvar *sav, struct xformsw *xs= p) >> csp.csp_mode =3D CSP_MODE_AEAD; >> if (sav->flags & SADB_X_SAFLAGS_ESN) >> csp.csp_flags |=3D CSP_F_SEPARATE_AAD; >> + } else if (sav->alg_enc =3D=3D SADB_X_EALG_CHACHA20POLY1305) { >> + sav->alg_auth =3D SADB_X_AALG_CHACHA20POLY1305; >> + sav->tdb_authalgxform =3D &auth_hash_poly1305; >> + csp.csp_mode =3D CSP_MODE_AEAD; >> + if (sav->flags & SADB_X_SAFLAGS_ESN) >> + csp.csp_flags |=3D CSP_F_SEPARATE_AAD; >> } else if (sav->alg_auth !=3D 0) { >> csp.csp_mode =3D 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 !=3D CRYPTO_NULL_CBC) { >> csp.csp_cipher_key =3D sav->key_enc->key_data; >> csp.csp_cipher_klen =3D _KEYBITS(sav->key_enc) / 8 - >> - SAV_ISCTRORGCM(sav) * 4; >> + SAV_ISCTRORGCM(sav) * 4 - SAV_ISCHACHA(sav) * 4; >> }; >> csp.csp_ivlen =3D txform->ivsize; >> @@ -368,7 +375,7 @@ esp_input(struct mbuf *m, struct secasvar *sav, i= nt skip, int protoff) >> if (esph !=3D NULL) { >> crp->crp_op =3D CRYPTO_OP_VERIFY_DIGEST; >> - if (SAV_ISGCM(sav)) >> + if (SAV_ISGCM(sav) || SAV_ISCHACHA(sav)) >> crp->crp_aad_length =3D 8; /* RFC4106 5, SPI + SN */ >> else >> crp->crp_aad_length =3D hlen; >> @@ -428,7 +435,7 @@ esp_input(struct mbuf *m, struct secasvar *sav, in= t skip, int protoff) >> crp->crp_payload_length =3D m->m_pkthdr.len - (skip + hlen + alen);= >> /* Generate or read cipher IV. */ >> - if (SAV_ISCTRORGCM(sav)) { >> + if (SAV_ISCTRORGCM(sav) || SAV_ISCHACHA(sav)) { >> ivp =3D &crp->crp_iv[0]; >> /* >> @@ -811,7 +818,7 @@ esp_output(struct mbuf *m, struct secpolicy *sp, s= truct secasvar *sav, >> SECREPLAY_UNLOCK(sav->replay); >> } >> cryptoid =3D sav->tdb_cryptoid; >> - if (SAV_ISCTRORGCM(sav)) >> + if (SAV_ISCTRORGCM(sav) || SAV_ISCHACHA(sav)) >> cntr =3D 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 =3D &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, s= truct secasvar *sav, >> if (esph) { >> /* Authentication descriptor. */ >> crp->crp_op |=3D CRYPTO_OP_COMPUTE_DIGEST; >> - if (SAV_ISGCM(sav)) >> + if (SAV_ISGCM(sav) || SAV_ISCHACHA(sav)) >> crp->crp_aad_length =3D 8; /* RFC4106 5, SPI + SN */ >> else >> crp->crp_aad_length =3D hlen; > > For some of these conditionals, I wonder if we want a SAV_IS_AEAD() tha= t 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 di= d in a few places > above). That is, I suspect that many of these things aren't specific t= o the ciphers in the > RFCs but are generic to any AEAD ciphersuite for IPsec. > Possibly, but I don=E2=80=99t know enough about other IPSec ciphers to ma= ke 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 =E2=80= =98is this an authenticating cipher=E2=80=99. Best regards, Kristof
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?3084EABF-AFC1-4DFF-ACD6-F0DC39E05764>