From: bluhm <bluhm@openbsd.org>
Date: Fri, 22 Oct 2021 15:44:20 +0000 (+0000)
Subject: Make error handling in IPsec consistent.  Pass errors to the callers.
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=bc489a1c753e4a67196061c7e8b1776a88c1fd8c;p=openbsd

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 59c3f2e930e..a10c2a7b7ba 100644
--- 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 d88aaf6da5a..1b96a2b942d 100644
--- 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 653d48368a8..902955c9d6e 100644
--- 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 9760d920c81..6a4ef5929a6 100644
--- 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 94c5a5ffabb..2a9a378bde7 100644
--- 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 dcde98fc5d8..ed11171b543 100644
--- 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);