On Tue, Nov 09, 2021 at 05:27:16PM +0100, Alexander Bluhm wrote:
> On Tue, Nov 09, 2021 at 02:00:01PM +0100, Denis Fondras wrote:
> > > @@ -352,19 +343,19 @@ ipsec_common_input(struct mbuf **mp, int
> > > * Call appropriate transform and return -- callback takes care of
> > > * everything else.
> > > */
> > > - error = (*(tdbp->tdb_xform->xf_input))(mp, tdbp, skip, protoff);
> > > - if (error) {
> > > + prot = (*(tdbp->tdb_xform->xf_input))(mp, tdbp, skip, protoff);
> > > + if (prot == IPPROTO_DONE) {
> > > ipsecstat_inc(ipsec_idrops);
> > > tdbp->tdb_idrops++;
> > > }
> > > - return error;
> > > + return prot;
> > >
> >
> > As I read & understand, prot is always IPPROTO_DONE and ipsec_idrops is
> > always
> > incremented. Can you explain in which case it is not ?
>
> For ESP it works this way.
>
> esp_input()
> /* Save the last three bytes of decrypted data */
> m_copydata(m, m->m_pkthdr.len - 3, 3, lastthree);
> ...
> /* Restore the Next Protocol field */
> m_copyback(m, protoff, sizeof(u_int8_t), lastthree + 2, M_NOWAIT);
> ipsec_common_input_cb()
> prot = ip->ip_p;
> ...
> /* Save protocol */
> m_copydata(m, protoff, 1, (caddr_t) &prot);
> ...
> /* Return to the appropriate protocol handler in deliver loop. */
> return prot;
> esp_input()
> /* Back to generic IPsec input processing */
> return ipsec_common_input_cb(mp, tdb, skip, protoff);
>
> The protocol is stored in the mbuf to print it correctly in tcpdump -i enc0 .
>
> I just realized that I forgot to include the bridge(4) part in my diff.
> As I have no IPsec bridge(4) setup, could someone test this?
>
> New diff. ok?
>
> bluhm
veb(4) has the identical to bridge(4) (*xf_input)() call in the disabled
ipsec(4) code. I have no idea should it be also modified, just pointing.
I can't test bridge(4), but the rest of diff is ok by me.
>
> Index: net/if_bridge.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/net/if_bridge.c,v
> retrieving revision 1.357
> diff -u -p -r1.357 if_bridge.c
> --- net/if_bridge.c 23 Oct 2021 22:19:37 -0000 1.357
> +++ net/if_bridge.c 1 Nov 2021 09:21:14 -0000
> @@ -1487,7 +1487,7 @@ bridge_ipsec(struct ifnet *ifp, struct e
> struct tdb *tdb;
> u_int32_t spi;
> u_int16_t cpi;
> - int error, off;
> + int error, off, prot;
> u_int8_t proto = 0;
> struct ip *ip;
> #ifdef INET6
> @@ -1575,7 +1575,10 @@ bridge_ipsec(struct ifnet *ifp, struct e
> tdb->tdb_soft_first_use);
> }
>
> - (*(tdb->tdb_xform->xf_input))(&m, tdb, hlen, off);
> + prot = (*(tdb->tdb_xform->xf_input))(&m, tdb, hlen,
> + off);
> + if (prot != IPPROTO_DONE)
> + ip_deliver(&m, &hlen, prot, af);
> return (1);
> } else {
> skiplookup:
> Index: netinet/ip_ah.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_ah.c,v
> retrieving revision 1.165
> diff -u -p -r1.165 ip_ah.c
> --- netinet/ip_ah.c 25 Oct 2021 09:47:02 -0000 1.165
> +++ netinet/ip_ah.c 1 Nov 2021 09:21:14 -0000
> @@ -563,21 +563,18 @@ ah_input(struct mbuf **mp, struct tdb *t
> ipsp_address(&tdb->tdb_dst, buf, sizeof(buf)),
> ntohl(tdb->tdb_spi));
> ahstat_inc(ahs_wrap);
> - error = ENOBUFS;
> goto drop;
> case 2:
> DPRINTF("old packet received in SA %s/%08x",
> ipsp_address(&tdb->tdb_dst, buf, sizeof(buf)),
> ntohl(tdb->tdb_spi));
> ahstat_inc(ahs_replay);
> - error = ENOBUFS;
> goto drop;
> case 3:
> DPRINTF("duplicate packet received in SA %s/%08x",
> ipsp_address(&tdb->tdb_dst, buf, sizeof(buf)),
> ntohl(tdb->tdb_spi));
> ahstat_inc(ahs_replay);
> - error = ENOBUFS;
> goto drop;
> default:
> DPRINTF("bogus value from checkreplaywindow() "
> @@ -585,7 +582,6 @@ ah_input(struct mbuf **mp, struct tdb *t
> ipsp_address(&tdb->tdb_dst, buf, sizeof(buf)),
> ntohl(tdb->tdb_spi));
> ahstat_inc(ahs_replay);
> - error = ENOBUFS;
> goto drop;
> }
> }
> @@ -597,7 +593,6 @@ ah_input(struct mbuf **mp, struct tdb *t
> ipsp_address(&tdb->tdb_dst, buf, sizeof(buf)),
> ntohl(tdb->tdb_spi));
> ahstat_inc(ahs_badauthl);
> - error = EACCES;
> goto drop;
> }
> if (skip + ahx->authsize + rplen > m->m_pkthdr.len) {
> @@ -607,7 +602,6 @@ ah_input(struct mbuf **mp, struct tdb *t
> ipsp_address(&tdb->tdb_dst, buf, sizeof(buf)),
> ntohl(tdb->tdb_spi));
> ahstat_inc(ahs_badauthl);
> - error = EACCES;
> goto drop;
> }
>
> @@ -622,7 +616,6 @@ ah_input(struct mbuf **mp, struct tdb *t
> tdb->tdb_cur_bytes >= tdb->tdb_exp_bytes) {
> pfkeyv2_expire(tdb, SADB_EXT_LIFETIME_HARD);
> tdb_delete(tdb);
> - error = ENXIO;
> goto drop;
> }
>
> @@ -638,7 +631,6 @@ ah_input(struct mbuf **mp, struct tdb *t
> if (crp == NULL) {
> DPRINTF("failed to acquire crypto descriptors");
> ahstat_inc(ahs_crypto);
> - error = ENOBUFS;
> goto drop;
> }
>
> @@ -664,7 +656,6 @@ ah_input(struct mbuf **mp, struct tdb *t
> if (ptr == NULL) {
> DPRINTF("failed to allocate buffer");
> ahstat_inc(ahs_crypto);
> - error = ENOBUFS;
> goto drop;
> }
>
> @@ -720,7 +711,6 @@ ah_input(struct mbuf **mp, struct tdb *t
> ipsp_address(&tdb->tdb_dst, buf, sizeof(buf)),
> ntohl(tdb->tdb_spi));
> ahstat_inc(ahs_badauth);
> - error = -1;
> goto drop;
> }
>
> @@ -750,21 +740,18 @@ ah_input(struct mbuf **mp, struct tdb *t
> ipsp_address(&tdb->tdb_dst, buf, sizeof(buf)),
> ntohl(tdb->tdb_spi));
> ahstat_inc(ahs_wrap);
> - error = -1;
> goto drop;
> case 2:
> DPRINTF("old packet received in SA %s/%08x",
> ipsp_address(&tdb->tdb_dst, buf, sizeof(buf)),
> ntohl(tdb->tdb_spi));
> ahstat_inc(ahs_replay);
> - error = -1;
> goto drop;
> case 3:
> DPRINTF("duplicate packet received in SA %s/%08x",
> ipsp_address(&tdb->tdb_dst, buf, sizeof(buf)),
> ntohl(tdb->tdb_spi));
> ahstat_inc(ahs_replay);
> - error = -1;
> goto drop;
> default:
> DPRINTF("bogus value from checkreplaywindow() "
> @@ -772,7 +759,6 @@ ah_input(struct mbuf **mp, struct tdb *t
> ipsp_address(&tdb->tdb_dst, buf, sizeof(buf)),
> ntohl(tdb->tdb_spi));
> ahstat_inc(ahs_replay);
> - error = -1;
> goto drop;
> }
> }
> @@ -784,7 +770,6 @@ ah_input(struct mbuf **mp, struct tdb *t
> ipsp_address(&tdb->tdb_dst, buf, sizeof(buf)),
> ntohl(tdb->tdb_spi));
> ahstat_inc(ahs_hdrops);
> - error = -1;
> goto drop;
> }
>
> @@ -863,7 +848,7 @@ ah_input(struct mbuf **mp, struct tdb *t
> free(ptr, M_XDATA, 0);
> m_freemp(mp);
> crypto_freereq(crp);
> - return error;
> + return IPPROTO_DONE;
> }
>
> /*
> Index: netinet/ip_esp.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_esp.c,v
> retrieving revision 1.184
> diff -u -p -r1.184 ip_esp.c
> --- netinet/ip_esp.c 24 Oct 2021 23:33:37 -0000 1.184
> +++ netinet/ip_esp.c 1 Nov 2021 09:21:14 -0000
> @@ -362,7 +362,6 @@ esp_input(struct mbuf **mp, struct tdb *
> if (plen <= 0) {
> DPRINTF("invalid payload length");
> espstat_inc(esps_badilen);
> - error = EINVAL;
> goto drop;
> }
>
> @@ -378,7 +377,6 @@ esp_input(struct mbuf **mp, struct tdb *
> ipsp_address(&tdb->tdb_dst, buf, sizeof(buf)),
> ntohl(tdb->tdb_spi));
> espstat_inc(esps_badilen);
> - error = EINVAL;
> goto drop;
> }
> }
> @@ -397,21 +395,18 @@ esp_input(struct mbuf **mp, struct tdb *
> ipsp_address(&tdb->tdb_dst, buf, sizeof(buf)),
> ntohl(tdb->tdb_spi));
> espstat_inc(esps_wrap);
> - error = EACCES;
> goto drop;
> case 2:
> DPRINTF("old packet received in SA %s/%08x",
> ipsp_address(&tdb->tdb_dst, buf, sizeof(buf)),
> ntohl(tdb->tdb_spi));
> espstat_inc(esps_replay);
> - error = EACCES;
> goto drop;
> case 3:
> DPRINTF("duplicate packet received in SA %s/%08x",
> ipsp_address(&tdb->tdb_dst, buf, sizeof(buf)),
> ntohl(tdb->tdb_spi));
> espstat_inc(esps_replay);
> - error = EACCES;
> goto drop;
> default:
> DPRINTF("bogus value from checkreplaywindow() "
> @@ -419,7 +414,6 @@ esp_input(struct mbuf **mp, struct tdb *
> ipsp_address(&tdb->tdb_dst, buf, sizeof(buf)),
> ntohl(tdb->tdb_spi));
> espstat_inc(esps_replay);
> - error = EACCES;
> goto drop;
> }
> }
> @@ -434,7 +428,6 @@ esp_input(struct mbuf **mp, struct tdb *
> (tdb->tdb_cur_bytes >= tdb->tdb_exp_bytes)) {
> pfkeyv2_expire(tdb, SADB_EXT_LIFETIME_HARD);
> tdb_delete(tdb);
> - error = ENXIO;
> goto drop;
> }
>
> @@ -450,7 +443,6 @@ esp_input(struct mbuf **mp, struct tdb *
> if (crp == NULL) {
> DPRINTF("failed to acquire crypto descriptors");
> espstat_inc(esps_crypto);
> - error = ENOBUFS;
> goto drop;
> }
>
> @@ -537,7 +529,6 @@ esp_input(struct mbuf **mp, struct tdb *
> ipsp_address(&tdb->tdb_dst, buf, sizeof(buf)),
> ntohl(tdb->tdb_spi));
> espstat_inc(esps_badauth);
> - error = -1;
> goto drop;
> }
>
> @@ -563,21 +554,18 @@ esp_input(struct mbuf **mp, struct tdb *
> ipsp_address(&tdb->tdb_dst, buf, sizeof(buf)),
> ntohl(tdb->tdb_spi));
> espstat_inc(esps_wrap);
> - error = -1;
> goto drop;
> case 2:
> DPRINTF("old packet received in SA %s/%08x",
> ipsp_address(&tdb->tdb_dst, buf, sizeof(buf)),
> ntohl(tdb->tdb_spi));
> espstat_inc(esps_replay);
> - error = -1;
> goto drop;
> case 3:
> DPRINTF("duplicate packet received in SA %s/%08x",
> ipsp_address(&tdb->tdb_dst, buf, sizeof(buf)),
> ntohl(tdb->tdb_spi));
> espstat_inc(esps_replay);
> - error = -1;
> goto drop;
> default:
> DPRINTF("bogus value from checkreplaywindow() "
> @@ -585,7 +573,6 @@ esp_input(struct mbuf **mp, struct tdb *
> ipsp_address(&tdb->tdb_dst, buf, sizeof(buf)),
> ntohl(tdb->tdb_spi));
> espstat_inc(esps_replay);
> - error = -1;
> goto drop;
> }
> }
> @@ -597,7 +584,6 @@ esp_input(struct mbuf **mp, struct tdb *
> ipsp_address(&tdb->tdb_dst, buf, sizeof(buf)),
> ntohl(tdb->tdb_spi));
> espstat_inc(esps_hdrops);
> - error = -1;
> goto drop;
> }
>
> @@ -668,7 +654,6 @@ esp_input(struct mbuf **mp, struct tdb *
> ipsp_address(&tdb->tdb_dst, buf, sizeof(buf)),
> ntohl(tdb->tdb_spi));
> espstat_inc(esps_badilen);
> - error = -1;
> goto drop;
> }
>
> @@ -678,7 +663,6 @@ esp_input(struct mbuf **mp, struct tdb *
> ipsp_address(&tdb->tdb_dst, buf, sizeof(buf)),
> ntohl(tdb->tdb_spi));
> espstat_inc(esps_badenc);
> - error = -1;
> goto drop;
> }
>
> @@ -694,7 +678,7 @@ esp_input(struct mbuf **mp, struct tdb *
> drop:
> m_freemp(mp);
> crypto_freereq(crp);
> - return error;
> + return IPPROTO_DONE;
> }
>
> /*
> Index: netinet/ip_ipcomp.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_ipcomp.c,v
> retrieving revision 1.86
> diff -u -p -r1.86 ip_ipcomp.c
> --- netinet/ip_ipcomp.c 24 Oct 2021 18:15:58 -0000 1.86
> +++ netinet/ip_ipcomp.c 1 Nov 2021 09:21:14 -0000
> @@ -154,7 +154,6 @@ ipcomp_input(struct mbuf **mp, struct td
> if (crp == NULL) {
> DPRINTF("failed to acquire crypto descriptors");
> ipcompstat_inc(ipcomps_crypto);
> - error = ENOBUFS;
> goto drop;
> }
> crdc = &crp->crp_desc[0];
> @@ -202,7 +201,6 @@ ipcomp_input(struct mbuf **mp, struct td
> (tdb->tdb_cur_bytes >= tdb->tdb_exp_bytes)) {
> pfkeyv2_expire(tdb, SADB_EXT_LIFETIME_HARD);
> tdb_delete(tdb);
> - error = -1;
> goto drop;
> }
> /* Notify on soft expiration */
> @@ -218,7 +216,6 @@ ipcomp_input(struct mbuf **mp, struct td
> if (m->m_len < skip + hlen &&
> (m = *mp = m_pullup(m, skip + hlen)) == NULL) {
> ipcompstat_inc(ipcomps_hdrops);
> - error = -1;
> goto drop;
> }
>
> @@ -229,7 +226,6 @@ ipcomp_input(struct mbuf **mp, struct td
> ipsp_address(&tdb->tdb_dst, buf, sizeof(buf)),
> ntohl(tdb->tdb_spi));
> ipcompstat_inc(ipcomps_hdrops);
> - error = -1;
> goto drop;
> }
> /* Keep the next protocol field */
> @@ -295,7 +291,7 @@ ipcomp_input(struct mbuf **mp, struct td
> drop:
> m_freemp(mp);
> crypto_freereq(crp);
> - return error;
> + return IPPROTO_DONE;
> }
>
> /*
> Index: netinet/ipsec_input.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ipsec_input.c,v
> retrieving revision 1.190
> diff -u -p -r1.190 ipsec_input.c
> --- netinet/ipsec_input.c 1 Nov 2021 09:19:10 -0000 1.190
> +++ netinet/ipsec_input.c 1 Nov 2021 09:21:14 -0000
> @@ -194,7 +194,7 @@ ipsec_common_input(struct mbuf **mp, int
> struct ifnet *encif;
> u_int32_t spi;
> u_int16_t cpi;
> - int error;
> + int prot;
> #ifdef ENCDEBUG
> char buf[INET6_ADDRSTRLEN];
> #endif
> @@ -207,14 +207,12 @@ ipsec_common_input(struct mbuf **mp, int
> if ((sproto == IPPROTO_IPCOMP) && (m->m_flags & M_COMP)) {
> DPRINTF("repeated decompression");
> ipcompstat_inc(ipcomps_pdrops);
> - error = EINVAL;
> goto drop;
> }
>
> if (m->m_pkthdr.len - skip < 2 * sizeof(u_int32_t)) {
> DPRINTF("packet too small");
> IPSEC_ISTAT(esps_hdrops, ahs_hdrops, ipcomps_hdrops);
> - error = EINVAL;
> goto drop;
> }
>
> @@ -268,7 +266,6 @@ ipsec_common_input(struct mbuf **mp, int
> default:
> DPRINTF("unsupported protocol family %d", af);
> IPSEC_ISTAT(esps_nopf, ahs_nopf, ipcomps_nopf);
> - error = EPFNOSUPPORT;
> goto drop;
> }
>
> @@ -278,7 +275,6 @@ ipsec_common_input(struct mbuf **mp, int
> DPRINTF("could not find SA for packet to %s, spi %08x",
> ipsp_address(&dst_address, buf, sizeof(buf)), ntohl(spi));
> IPSEC_ISTAT(esps_notdb, ahs_notdb, ipcomps_notdb);
> - error = ENOENT;
> goto drop;
> }
>
> @@ -287,7 +283,6 @@ ipsec_common_input(struct mbuf **mp, int
> ipsp_address(&dst_address, buf, sizeof(buf)),
> ntohl(spi), tdbp->tdb_sproto);
> IPSEC_ISTAT(esps_invalid, ahs_invalid, ipcomps_invalid);
> - error = EINVAL;
> goto drop;
> }
>
> @@ -296,7 +291,6 @@ ipsec_common_input(struct mbuf **mp, int
> ipsp_address(&dst_address, buf, sizeof(buf)),
> ntohl(spi), tdbp->tdb_sproto);
> espstat_inc(esps_udpinval);
> - error = EINVAL;
> goto drop;
> }
>
> @@ -305,7 +299,6 @@ ipsec_common_input(struct mbuf **mp, int
> ipsp_address(&dst_address, buf, sizeof(buf)),
> ntohl(spi), tdbp->tdb_sproto);
> espstat_inc(esps_udpneeded);
> - error = EINVAL;
> goto drop;
> }
>
> @@ -314,7 +307,6 @@ ipsec_common_input(struct mbuf **mp, int
> ipsp_address(&dst_address, buf, sizeof(buf)),
> ntohl(spi), tdbp->tdb_sproto);
> IPSEC_ISTAT(esps_noxform, ahs_noxform, ipcomps_noxform);
> - error = ENXIO;
> goto drop;
> }
>
> @@ -326,7 +318,6 @@ ipsec_common_input(struct mbuf **mp, int
> ipsp_address(&dst_address, buf, sizeof(buf)),
> ntohl(spi), tdbp->tdb_sproto);
> IPSEC_ISTAT(esps_pdrops, ahs_pdrops, ipcomps_pdrops);
> - error = EACCES;
> goto drop;
> }
>
> @@ -352,19 +343,19 @@ ipsec_common_input(struct mbuf **mp, int
> * Call appropriate transform and return -- callback takes care of
> * everything else.
> */
> - error = (*(tdbp->tdb_xform->xf_input))(mp, tdbp, skip, protoff);
> - if (error) {
> + prot = (*(tdbp->tdb_xform->xf_input))(mp, tdbp, skip, protoff);
> + if (prot == IPPROTO_DONE) {
> ipsecstat_inc(ipsec_idrops);
> tdbp->tdb_idrops++;
> }
> - return error;
> + return prot;
>
> drop:
> m_freemp(mp);
> ipsecstat_inc(ipsec_idrops);
> if (tdbp != NULL)
> tdbp->tdb_idrops++;
> - return error;
> + return IPPROTO_DONE;
> }
>
> /*
> @@ -630,16 +621,15 @@ ipsec_common_input_cb(struct mbuf **mp,
> m = *mp;
> if_put(ifp);
> if (m == NULL)
> - return 0;
> + return IPPROTO_DONE;
> }
> #endif
> - /* Call the appropriate IPsec transform callback. */
> - ip_deliver(mp, &skip, prot, af);
> - return 0;
> + /* Return to the appropriate protocol handler in deliver loop. */
> + return prot;
>
> baddone:
> m_freemp(mp);
> - return -1;
> + return IPPROTO_DONE;
> #undef IPSEC_ISTAT
> }
>
> @@ -829,8 +819,7 @@ ah46_input(struct mbuf **mp, int *offp,
> return IPPROTO_DONE;
> }
>
> - ipsec_common_input(mp, *offp, protoff, af, proto, 0);
> - return IPPROTO_DONE;
> + return ipsec_common_input(mp, *offp, protoff, af, proto, 0);
> }
>
> void
> @@ -863,8 +852,7 @@ esp46_input(struct mbuf **mp, int *offp,
> return IPPROTO_DONE;
> }
>
> - ipsec_common_input(mp, *offp, protoff, af, proto, 0);
> - return IPPROTO_DONE;
> + return ipsec_common_input(mp, *offp, protoff, af, proto, 0);
> }
>
> /* IPv4 IPCOMP wrapper */
> @@ -888,8 +876,7 @@ ipcomp46_input(struct mbuf **mp, int *of
> return IPPROTO_DONE;
> }
>
> - ipsec_common_input(mp, *offp, protoff, af, proto, 0);
> - return IPPROTO_DONE;
> + return ipsec_common_input(mp, *offp, protoff, af, proto, 0);
> }
>
> void
> Index: netinet/tcp_subr.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/tcp_subr.c,v
> retrieving revision 1.181
> diff -u -p -r1.181 tcp_subr.c
> --- netinet/tcp_subr.c 23 Oct 2021 22:19:37 -0000 1.181
> +++ netinet/tcp_subr.c 1 Nov 2021 09:26:48 -0000
> @@ -964,7 +964,7 @@ tcp_signature_tdb_input(struct mbuf **mp
> int protoff)
> {
> m_freemp(mp);
> - return (EINVAL);
> + return (IPPROTO_DONE);
> }
>
> int
> Index: netinet/udp_usrreq.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/udp_usrreq.c,v
> retrieving revision 1.263
> diff -u -p -r1.263 udp_usrreq.c
> --- netinet/udp_usrreq.c 23 Oct 2021 22:19:37 -0000 1.263
> +++ netinet/udp_usrreq.c 2 Nov 2021 09:49:29 -0000
> @@ -305,9 +305,8 @@ udp_input(struct mbuf **mp, int *offp, i
> espstat_inc(esps_udpencin);
> protoff = af == AF_INET ? offsetof(struct ip, ip_p) :
> offsetof(struct ip6_hdr, ip6_nxt);
> - ipsec_common_input(mp, skip, protoff,
> + return ipsec_common_input(mp, skip, protoff,
> af, IPPROTO_ESP, 1);
> - return IPPROTO_DONE;
> }
> }
> #endif
>
