From ede2f7731def0fbe8529ab825b19fddb75adfee5 Mon Sep 17 00:00:00 2001 From: Conrad Meyer Date: Mon, 4 Jun 2018 18:51:06 +0000 Subject: [PATCH] Correctly handle the padding for IPv6-AH, as specified by RFC4302 The RFC specifies that under IPv6 the complete AH header must be 64 bit aligned, and under IPv4, 32 bit aligned. Prior to this change, we (along with other BSDs and MacOS) had violated this requirement. This makes it possible to set up IPv6-AH between Linux and BSD, and also probably between Windows and BSD. PR: 222684 Reported and tested by: Jason Mader Obtained from: NetBSD xform_ah.c 1.105 (b939fe2483972eb43d71bf990cfb7f26dece7839 NetBSD/src on GH) by Maxime Villard MFC after: 35.2731 hours Relnotes: probably (breaks ipv6 compat with older FreeBSD/NetBSD/MacOS) Sponsored by: Dell EMC Isilon --- sys/netipsec/xform_ah.c | 56 ++++++++++++++++++++++++++--------------- 1 file changed, 36 insertions(+), 20 deletions(-) diff --git a/sys/netipsec/xform_ah.c b/sys/netipsec/xform_ah.c index 3adc8dc38351..1be268fc5c16 100644 --- a/sys/netipsec/xform_ah.c +++ b/sys/netipsec/xform_ah.c @@ -147,11 +147,21 @@ ah_hdrsiz(struct secasvar *sav) size_t size; if (sav != NULL) { - int authsize; + int authsize, rplen, align; + IPSEC_ASSERT(sav->tdb_authalgxform != NULL, ("null xform")); /*XXX not right for null algorithm--does it matter??*/ + + /* RFC4302: use the correct alignment. */ + align = sizeof(uint32_t); +#ifdef INET6 + if (sav->sah->saidx.dst.sa.sa_family == AF_INET6) { + align = sizeof(uint64_t); + } +#endif + rplen = HDRSIZE(sav); authsize = AUTHSIZE(sav); - size = roundup(authsize, sizeof (u_int32_t)) + HDRSIZE(sav); + size = roundup(rplen + authsize, align); } else { /* default guess */ size = sizeof (struct ah) + sizeof (u_int32_t) + 16; @@ -535,7 +545,7 @@ ah_input(struct mbuf *m, struct secasvar *sav, int skip, int protoff) struct xform_data *xd; struct newah *ah; uint64_t cryptoid; - int hl, rplen, authsize, error; + int hl, rplen, authsize, ahsize, error; IPSEC_ASSERT(sav != NULL, ("null SA")); IPSEC_ASSERT(sav->key_auth != NULL, ("null authentication key")); @@ -569,23 +579,24 @@ ah_input(struct mbuf *m, struct secasvar *sav, int skip, int protoff) SECASVAR_UNLOCK(sav); /* Verify AH header length. */ - hl = ah->ah_len * sizeof (u_int32_t); + hl = sizeof(struct ah) + (ah->ah_len * sizeof (u_int32_t)); ahx = sav->tdb_authalgxform; authsize = AUTHSIZE(sav); - if (hl != authsize + rplen - sizeof (struct ah)) { + ahsize = ah_hdrsiz(sav); + if (hl != ahsize) { DPRINTF(("%s: bad authenticator length %u (expecting %lu)" " for packet in SA %s/%08lx\n", __func__, hl, - (u_long) (authsize + rplen - sizeof (struct ah)), + (u_long)ahsize, ipsec_address(&sav->sah->saidx.dst, buf, sizeof(buf)), (u_long) ntohl(sav->spi))); AHSTAT_INC(ahs_badauthl); error = EACCES; goto bad; } - if (skip + authsize + rplen > m->m_pkthdr.len) { + if (skip + ahsize > m->m_pkthdr.len) { DPRINTF(("%s: bad mbuf length %u (expecting %lu)" " for packet in SA %s/%08lx\n", __func__, - m->m_pkthdr.len, (u_long) (skip + authsize + rplen), + m->m_pkthdr.len, (u_long)(skip + ahsize), ipsec_address(&sav->sah->saidx.dst, buf, sizeof(buf)), (u_long) ntohl(sav->spi))); AHSTAT_INC(ahs_badauthl); @@ -689,7 +700,7 @@ ah_input_cb(struct cryptop *crp) struct secasindex *saidx; caddr_t ptr; uint64_t cryptoid; - int authsize, rplen, error, skip, protoff; + int authsize, rplen, ahsize, error, skip, protoff; uint8_t nxt; m = (struct mbuf *) crp->crp_buf; @@ -736,6 +747,7 @@ ah_input_cb(struct cryptop *crp) /* Figure out header size. */ rplen = HDRSIZE(sav); authsize = AUTHSIZE(sav); + ahsize = ah_hdrsiz(sav); /* Copy authenticator off the packet. */ m_copydata(m, skip + rplen, authsize, calc); @@ -784,7 +796,7 @@ ah_input_cb(struct cryptop *crp) /* * Remove the AH header and authenticator from the mbuf. */ - error = m_striphdr(m, skip, rplen + authsize); + error = m_striphdr(m, skip, ahsize); if (error) { DPRINTF(("%s: mangled mbuf chain for SA %s/%08lx\n", __func__, ipsec_address(&saidx->dst, buf, sizeof(buf)), @@ -839,7 +851,7 @@ ah_output(struct mbuf *m, struct secpolicy *sp, struct secasvar *sav, struct newah *ah; uint64_t cryptoid; uint16_t iplen; - int error, rplen, authsize, maxpacketsize, roff; + int error, rplen, authsize, ahsize, maxpacketsize, roff; uint8_t prot; IPSEC_ASSERT(sav != NULL, ("null SA")); @@ -850,6 +862,8 @@ ah_output(struct mbuf *m, struct secpolicy *sp, struct secasvar *sav, /* Figure out header size. */ rplen = HDRSIZE(sav); + authsize = AUTHSIZE(sav); + ahsize = ah_hdrsiz(sav); /* Check for maximum packet size violations. */ switch (sav->sah->saidx.dst.sa.sa_family) { @@ -873,13 +887,12 @@ ah_output(struct mbuf *m, struct secpolicy *sp, struct secasvar *sav, error = EPFNOSUPPORT; goto bad; } - authsize = AUTHSIZE(sav); - if (rplen + authsize + m->m_pkthdr.len > maxpacketsize) { + if (ahsize + m->m_pkthdr.len > maxpacketsize) { DPRINTF(("%s: packet in SA %s/%08lx got too big " "(len %u, max len %u)\n", __func__, ipsec_address(&sav->sah->saidx.dst, buf, sizeof(buf)), (u_long) ntohl(sav->spi), - rplen + authsize + m->m_pkthdr.len, maxpacketsize)); + ahsize + m->m_pkthdr.len, maxpacketsize)); AHSTAT_INC(ahs_toobig); error = EMSGSIZE; goto bad; @@ -899,11 +912,10 @@ ah_output(struct mbuf *m, struct secpolicy *sp, struct secasvar *sav, } /* Inject AH header. */ - mi = m_makespace(m, skip, rplen + authsize, &roff); + mi = m_makespace(m, skip, ahsize, &roff); if (mi == NULL) { DPRINTF(("%s: failed to inject %u byte AH header for SA " - "%s/%08lx\n", __func__, - rplen + authsize, + "%s/%08lx\n", __func__, ahsize, ipsec_address(&sav->sah->saidx.dst, buf, sizeof(buf)), (u_long) ntohl(sav->spi))); AHSTAT_INC(ahs_hdrops); /*XXX differs from openbsd */ @@ -919,13 +931,17 @@ ah_output(struct mbuf *m, struct secpolicy *sp, struct secasvar *sav, /* Initialize the AH header. */ m_copydata(m, protoff, sizeof(u_int8_t), (caddr_t) &ah->ah_nxt); - ah->ah_len = (rplen + authsize - sizeof(struct ah)) / sizeof(u_int32_t); + ah->ah_len = (ahsize - sizeof(struct ah)) / sizeof(u_int32_t); ah->ah_reserve = 0; ah->ah_spi = sav->spi; /* Zeroize authenticator. */ m_copyback(m, skip + rplen, authsize, ipseczeroes); + /* Zeroize padding */ + m_copyback(m, skip + rplen + authsize, ahsize - (rplen + authsize), + ipseczeroes); + /* Insert packet replay counter, as requested. */ SECASVAR_LOCK(sav); if (sav->replay) { @@ -994,7 +1010,7 @@ ah_output(struct mbuf *m, struct secpolicy *sp, struct secasvar *sav, bcopy(((caddr_t)(xd + 1)) + offsetof(struct ip, ip_len), (caddr_t) &iplen, sizeof(u_int16_t)); - iplen = htons(ntohs(iplen) + rplen + authsize); + iplen = htons(ntohs(iplen) + ahsize); m_copyback(m, offsetof(struct ip, ip_len), sizeof(u_int16_t), (caddr_t) &iplen); break; @@ -1005,7 +1021,7 @@ ah_output(struct mbuf *m, struct secpolicy *sp, struct secasvar *sav, bcopy(((caddr_t)(xd + 1)) + offsetof(struct ip6_hdr, ip6_plen), (caddr_t) &iplen, sizeof(uint16_t)); - iplen = htons(ntohs(iplen) + rplen + authsize); + iplen = htons(ntohs(iplen) + ahsize); m_copyback(m, offsetof(struct ip6_hdr, ip6_plen), sizeof(uint16_t), (caddr_t) &iplen); break;