Make error handling in IPsec consistent. Pass errors to the callers.

OK tobhe@

diff --git a/sys/netinet/ip_ah.c b/sys/netinet/ip_ah.c
index 59c3f2e..a10c2a7 100644 (file)
--- a/sys/netinet/ip_ah.c
+++ b/sys/netinet/ip_ah.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: ip_ah.c,v 1.157 2021/10/21 22:59:07 tobhe Exp $ */
+/*     $OpenBSD: ip_ah.c,v 1.158 2021/10/22 15:44:20 bluhm Exp $ */
 /*
  * The authors of this code are John Ioannidis (ji@tla.org),
  * Angelos D. Keromytis (kermit@csd.uch.gr) and
@@ -1164,6 +1164,7 @@ ah_output_cb(struct tdb *tdb, struct tdb_crypto *tc, struct mbuf *m, int ilen,
 {
        int skip = tc->tc_skip;
        caddr_t ptr = (caddr_t) (tc + 1);
+       int error;
 
        /*
         * Copy original headers (with the new protocol number) back
@@ -1175,10 +1176,8 @@ ah_output_cb(struct tdb *tdb, struct tdb_crypto *tc, struct mbuf *m, int ilen,
        free(tc, M_XDATA, 0);
 
        /* Call the IPsec input callback. */
-       if (ipsp_process_done(m, tdb)) {
+       error = ipsp_process_done(m, tdb);
+       if (error)
                ahstat_inc(ahs_outfail);
-               return -1;
-       }
-
-       return 0;
+       return error;
 }

diff --git a/sys/netinet/ip_esp.c b/sys/netinet/ip_esp.c
index d88aaf6..1b96a2b 100644 (file)
--- a/sys/netinet/ip_esp.c
+++ b/sys/netinet/ip_esp.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: ip_esp.c,v 1.176 2021/10/21 22:59:07 tobhe Exp $ */
+/*     $OpenBSD: ip_esp.c,v 1.177 2021/10/22 15:44:20 bluhm Exp $ */
 /*
  * The authors of this code are John Ioannidis (ji@tla.org),
  * Angelos D. Keromytis (kermit@csd.uch.gr) and
@@ -1024,16 +1024,16 @@ int
 esp_output_cb(struct tdb *tdb, struct tdb_crypto *tc, struct mbuf *m, int ilen,
     int olen)
 {
+       int error;
+
        /* Release crypto descriptors. */
        free(tc, M_XDATA, 0);
 
        /* Call the IPsec input callback. */
-       if (ipsp_process_done(m, tdb)) {
+       error = ipsp_process_done(m, tdb);
+       if (error)
                espstat_inc(esps_outfail);
-               return -1;
-       }
-
-       return 0;
+       return error;
 }
 
 #define SEEN_SIZE      howmany(TDB_REPLAYMAX, 32)

diff --git a/sys/netinet/ip_ipcomp.c b/sys/netinet/ip_ipcomp.c
index 653d483..902955c 100644 (file)
--- a/sys/netinet/ip_ipcomp.c
+++ b/sys/netinet/ip_ipcomp.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ip_ipcomp.c,v 1.78 2021/10/22 12:30:53 bluhm Exp $ */
+/* $OpenBSD: ip_ipcomp.c,v 1.79 2021/10/22 15:44:20 bluhm Exp $ */
 
 /*
  * Copyright (c) 2001 Jean-Jacques Bernard-Gundol (jj@wabbitt.org)
@@ -505,6 +505,7 @@ ipcomp_output_cb(struct tdb *tdb, struct tdb_crypto *tc, struct mbuf *m,
 #ifdef ENCDEBUG
        char buf[INET6_ADDRSTRLEN];
 #endif
+       int error;
 
        skip = tc->tc_skip;
        rlen = ilen - skip;
@@ -523,7 +524,8 @@ ipcomp_output_cb(struct tdb *tdb, struct tdb_crypto *tc, struct mbuf *m,
                    ipsp_address(&tdb->tdb_dst, buf, sizeof(buf)),
                    ntohl(tdb->tdb_spi));
                ipcompstat_inc(ipcomps_wrap);
-               goto baddone;
+               error = ENOBUFS;
+               goto drop;
        }
 
        /* Initialize the IPCOMP header */
@@ -552,21 +554,21 @@ ipcomp_output_cb(struct tdb *tdb, struct tdb_crypto *tc, struct mbuf *m,
                    ipsp_address(&tdb->tdb_dst, buf, sizeof(buf)),
                    ntohl(tdb->tdb_spi));
                ipcompstat_inc(ipcomps_nopf);
-               goto baddone;
+               error = EPFNOSUPPORT;
+               goto drop;
        }
 
  skiphdr:
        /* Release the crypto descriptor. */
        free(tc, M_XDATA, 0);
 
-       if (ipsp_process_done(m, tdb)) {
+       error = ipsp_process_done(m, tdb);
+       if (error)
                ipcompstat_inc(ipcomps_outfail);
-               return -1;
-       }
-       return 0;
+       return error;
 
- baddone:
+ drop:
        m_freem(m);
        free(tc, M_XDATA, 0);
-       return -1;
+       return error;
 }

diff --git a/sys/netinet/ip_ipip.c b/sys/netinet/ip_ipip.c
index 9760d92..6a4ef59 100644 (file)
--- a/sys/netinet/ip_ipip.c
+++ b/sys/netinet/ip_ipip.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: ip_ipip.c,v 1.95 2021/10/13 14:36:31 bluhm Exp $ */
+/*     $OpenBSD: ip_ipip.c,v 1.96 2021/10/22 15:44:20 bluhm Exp $ */
 /*
  * The authors of this code are John Ioannidis (ji@tla.org),
  * Angelos D. Keromytis (kermit@csd.uch.gr) and
@@ -524,7 +524,7 @@ ipip_output(struct mbuf **mp, struct tdb *tdb)
                DPRINTF("unsupported protocol family %d",
                    tdb->tdb_dst.sa.sa_family);
                ipipstat_inc(ipips_family);
-               error = EAFNOSUPPORT;
+               error = EPFNOSUPPORT;
                goto drop;
        }
 

diff --git a/sys/netinet/ipsec_input.c b/sys/netinet/ipsec_input.c
index 94c5a5f..2a9a378 100644 (file)
--- a/sys/netinet/ipsec_input.c
+++ b/sys/netinet/ipsec_input.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: ipsec_input.c,v 1.184 2021/10/13 22:49:11 bluhm Exp $ */
+/*     $OpenBSD: ipsec_input.c,v 1.185 2021/10/22 15:44:20 bluhm Exp $ */
 /*
  * The authors of this code are John Ioannidis (ji@tla.org),
  * Angelos D. Keromytis (kermit@csd.uch.gr) and
@@ -359,10 +359,10 @@ ipsec_common_input(struct mbuf *m, int skip, int protoff, int af, int sproto,
        return error;
 
  drop:
+       m_freem(m);
        ipsecstat_inc(ipsec_idrops);
        if (tdbp != NULL)
                tdbp->tdb_idrops++;
-       m_freem(m);
        return error;
 }
 
@@ -423,7 +423,6 @@ ipsec_input_cb(struct cryptop *crp)
                panic("%s: unknown/unsupported security protocol %d",
                    __func__, tdb->tdb_sproto);
        }
-
        if (error) {
                ipsecstat_inc(ipsec_idrops);
                tdb->tdb_idrops++;
@@ -431,12 +430,12 @@ ipsec_input_cb(struct cryptop *crp)
        return;
 
  drop:
+       m_freem(m);
+       free(tc, M_XDATA, 0);
+       crypto_freereq(crp);
        ipsecstat_inc(ipsec_idrops);
        if (tdb != NULL)
                tdb->tdb_idrops++;
-       free(tc, M_XDATA, 0);
-       m_freem(m);
-       crypto_freereq(crp);
 }
 
 /*
@@ -448,19 +447,15 @@ ipsec_common_input_cb(struct mbuf *m, struct tdb *tdbp, int skip, int protoff)
 {
        int af, sproto;
        u_int8_t prot;
-
 #if NBPFILTER > 0
        struct ifnet *encif;
 #endif
-
        struct ip *ip, ipn;
-
 #ifdef INET6
        struct ip6_hdr *ip6, ip6n;
 #endif /* INET6 */
        struct m_tag *mtag;
        struct tdb_ident *tdbi;
-
 #ifdef ENCDEBUG
        char buf[INET6_ADDRSTRLEN];
 #endif
@@ -477,7 +472,7 @@ ipsec_common_input_cb(struct mbuf *m, struct tdb *tdbp, int skip, int protoff)
                            ipsp_address(&tdbp->tdb_dst, buf, sizeof(buf)),
                            ntohl(tdbp->tdb_spi));
                        IPSEC_ISTAT(esps_hdrops, ahs_hdrops, ipcomps_hdrops);
-                       return -1;
+                       goto baddone;
                }
 
                ip = mtod(m, struct ip *);
@@ -489,10 +484,9 @@ ipsec_common_input_cb(struct mbuf *m, struct tdb *tdbp, int skip, int protoff)
                /* IP-in-IP encapsulation */
                if (prot == IPPROTO_IPIP) {
                        if (m->m_pkthdr.len - skip < sizeof(struct ip)) {
-                               m_freem(m);
                                IPSEC_ISTAT(esps_hdrops, ahs_hdrops,
                                    ipcomps_hdrops);
-                               return -1;
+                               goto baddone;
                        }
                        /* ipn will now contain the inner IPv4 header */
                        m_copydata(m, skip, sizeof(struct ip),
@@ -503,10 +497,9 @@ ipsec_common_input_cb(struct mbuf *m, struct tdb *tdbp, int skip, int protoff)
                /* IPv6-in-IP encapsulation. */
                if (prot == IPPROTO_IPV6) {
                        if (m->m_pkthdr.len - skip < sizeof(struct ip6_hdr)) {
-                               m_freem(m);
                                IPSEC_ISTAT(esps_hdrops, ahs_hdrops,
                                    ipcomps_hdrops);
-                               return -1;
+                               goto baddone;
                        }
                        /* ip6n will now contain the inner IPv6 header. */
                        m_copydata(m, skip, sizeof(struct ip6_hdr),
@@ -517,8 +510,7 @@ ipsec_common_input_cb(struct mbuf *m, struct tdb *tdbp, int skip, int protoff)
 
 #ifdef INET6
        /* Fix IPv6 header */
-       if (af == AF_INET6)
-       {
+       if (af == AF_INET6) {
                if (m->m_len < sizeof(struct ip6_hdr) &&
                    (m = m_pullup(m, sizeof(struct ip6_hdr))) == NULL) {
 
@@ -526,7 +518,7 @@ ipsec_common_input_cb(struct mbuf *m, struct tdb *tdbp, int skip, int protoff)
                            ipsp_address(&tdbp->tdb_dst, buf, sizeof(buf)),
                            ntohl(tdbp->tdb_spi));
                        IPSEC_ISTAT(esps_hdrops, ahs_hdrops, ipcomps_hdrops);
-                       return -1;
+                       goto baddone;
                }
 
                ip6 = mtod(m, struct ip6_hdr *);
@@ -538,10 +530,9 @@ ipsec_common_input_cb(struct mbuf *m, struct tdb *tdbp, int skip, int protoff)
                /* IP-in-IP encapsulation */
                if (prot == IPPROTO_IPIP) {
                        if (m->m_pkthdr.len - skip < sizeof(struct ip)) {
-                               m_freem(m);
                                IPSEC_ISTAT(esps_hdrops, ahs_hdrops,
                                    ipcomps_hdrops);
-                               return -1;
+                               goto baddone;
                        }
                        /* ipn will now contain the inner IPv4 header */
                        m_copydata(m, skip, sizeof(struct ip), (caddr_t) &ipn);
@@ -550,10 +541,9 @@ ipsec_common_input_cb(struct mbuf *m, struct tdb *tdbp, int skip, int protoff)
                /* IPv6-in-IP encapsulation */
                if (prot == IPPROTO_IPV6) {
                        if (m->m_pkthdr.len - skip < sizeof(struct ip6_hdr)) {
-                               m_freem(m);
                                IPSEC_ISTAT(esps_hdrops, ahs_hdrops,
                                    ipcomps_hdrops);
-                               return -1;
+                               goto baddone;
                        }
                        /* ip6n will now contain the inner IPv6 header. */
                        m_copydata(m, skip, sizeof(struct ip6_hdr),
@@ -574,10 +564,9 @@ ipsec_common_input_cb(struct mbuf *m, struct tdb *tdbp, int skip, int protoff)
                switch (prot) {
                case IPPROTO_UDP:
                        if (m->m_pkthdr.len < skip + sizeof(struct udphdr)) {
-                               m_freem(m);
                                IPSEC_ISTAT(esps_hdrops, ahs_hdrops,
                                    ipcomps_hdrops);
-                               return -1;
+                               goto baddone;
                        }
                        cksum = 0;
                        m_copyback(m, skip + offsetof(struct udphdr, uh_sum),
@@ -593,10 +582,9 @@ ipsec_common_input_cb(struct mbuf *m, struct tdb *tdbp, int skip, int protoff)
                        break;
                case IPPROTO_TCP:
                        if (m->m_pkthdr.len < skip + sizeof(struct tcphdr)) {
-                               m_freem(m);
                                IPSEC_ISTAT(esps_hdrops, ahs_hdrops,
                                    ipcomps_hdrops);
-                               return -1;
+                               goto baddone;
                        }
                        cksum = 0;
                        m_copyback(m, skip + offsetof(struct tcphdr, th_sum),
@@ -623,10 +611,9 @@ ipsec_common_input_cb(struct mbuf *m, struct tdb *tdbp, int skip, int protoff)
                mtag = m_tag_get(PACKET_TAG_IPSEC_IN_DONE,
                    sizeof(struct tdb_ident), M_NOWAIT);
                if (mtag == NULL) {
-                       m_freem(m);
                        DPRINTF("failed to get tag");
                        IPSEC_ISTAT(esps_hdrops, ahs_hdrops, ipcomps_hdrops);
-                       return -1;
+                       goto baddone;
                }
 
                tdbi = (struct tdb_ident *)(mtag + 1);
@@ -703,22 +690,24 @@ ipsec_common_input_cb(struct mbuf *m, struct tdb *tdbp, int skip, int protoff)
 
                /* This is the enc0 interface unless for ipcomp. */
                if ((ifp = if_get(m->m_pkthdr.ph_ifidx)) == NULL) {
-                       m_freem(m);
-                       return -1;
+                       goto baddone;
                }
                if (pf_test(af, PF_IN, ifp, &m) != PF_PASS) {
                        if_put(ifp);
-                       m_freem(m);
-                       return -1;
+                       goto baddone;
                }
                if_put(ifp);
                if (m == NULL)
-                       return -1;
+                       return 0;
        }
 #endif
        /* Call the appropriate IPsec transform callback. */
        ip_deliver(&m, &skip, prot, af);
        return 0;
+
+ baddone:
+       m_freem(m);
+       return -1;
 #undef IPSEC_ISTAT
 }
 

diff --git a/sys/netinet/ipsec_output.c b/sys/netinet/ipsec_output.c
index dcde98f..ed11171 100644 (file)
--- a/sys/netinet/ipsec_output.c
+++ b/sys/netinet/ipsec_output.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: ipsec_output.c,v 1.89 2021/10/13 22:43:44 bluhm Exp $ */
+/*     $OpenBSD: ipsec_output.c,v 1.90 2021/10/22 15:44:20 bluhm Exp $ */
 /*
  * The author of this code is Angelos D. Keromytis (angelos@cis.upenn.edu)
  *
@@ -130,7 +130,7 @@ ipsp_process_packet(struct mbuf *m, struct tdb *tdb, int af, int tunalready)
                    ipsp_address(&tdb->tdb_dst, buf, sizeof(buf)),
                    ntohl(tdb->tdb_spi), tdb->tdb_sproto,
                    tdb->tdb_dst.sa.sa_family);
-               error = ENXIO;
+               error = EPFNOSUPPORT;
                goto drop;
        }
 
@@ -348,7 +348,7 @@ ipsp_process_packet(struct mbuf *m, struct tdb *tdb, int af, int tunalready)
                break;
 #endif /* INET6 */
        default:
-               error = EINVAL;
+               error = EPFNOSUPPORT;
                goto drop;
        }
 
@@ -434,10 +434,9 @@ ipsec_output_cb(struct cryptop *crp)
                error = ipcomp_output_cb(tdb, tc, m, ilen, olen);
                break;
        default:
-               panic("%s: unknown/unsupported security protocol %d",
+               panic("%s: unhandled security protocol %d",
                    __func__, tdb->tdb_sproto);
        }
-
        if (error) {
                ipsecstat_inc(ipsec_odrops);
                tdb->tdb_odrops++;
@@ -445,12 +444,12 @@ ipsec_output_cb(struct cryptop *crp)
        return;
 
  drop:
-       if (tdb != NULL)
-               tdb->tdb_odrops++;
        m_freem(m);
        free(tc, M_XDATA, 0);
        crypto_freereq(crp);
        ipsecstat_inc(ipsec_odrops);
+       if (tdb != NULL)
+               tdb->tdb_odrops++;
 }
 
 /*
@@ -494,7 +493,7 @@ ipsp_process_done(struct mbuf *m, struct tdb *tdb)
                default:
                        DPRINTF("unknown protocol family (%d)",
                            tdb->tdb_dst.sa.sa_family);
-                       error = ENXIO;
+                       error = EPFNOSUPPORT;
                        goto drop;
                }
 
@@ -548,7 +547,7 @@ ipsp_process_done(struct mbuf *m, struct tdb *tdb)
        default:
                DPRINTF("unknown protocol family (%d)",
                    tdb->tdb_dst.sa.sa_family);
-               error = ENXIO;
+               error = EPFNOSUPPORT;
                goto drop;
        }
 
@@ -596,18 +595,22 @@ ipsp_process_done(struct mbuf *m, struct tdb *tdb)
         */
        switch (tdb->tdb_dst.sa.sa_family) {
        case AF_INET:
-               return (ip_output(m, NULL, NULL, IP_RAWOUTPUT, NULL, NULL, 0));
-
+               error = ip_output(m, NULL, NULL, IP_RAWOUTPUT, NULL, NULL, 0);
+               break;
 #ifdef INET6
        case AF_INET6:
                /*
                 * We don't need massage, IPv6 header fields are always in
                 * net endian.
                 */
-               return (ip6_output(m, NULL, NULL, 0, NULL, NULL));
+               error = ip6_output(m, NULL, NULL, 0, NULL, NULL);
+               break;
 #endif /* INET6 */
+       default:
+               error = EPFNOSUPPORT;
+               break;
        }
-       error = EINVAL; /* Not reached. */
+       return error;
 
  drop:
        m_freem(m);