From: mpi <mpi@openbsd.org>
Date: Thu, 12 Jul 2018 15:51:50 +0000 (+0000)
Subject: Introduce ipsec_output_cb() to merge duplicate code and account for
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=7ced204ee2fdf209a5f0c84ecd143e75b272f616;p=openbsd

Introduce ipsec_output_cb() to merge duplicate code and account for
dropped packets in the output path.

While here fix a memory leak when compression is not needed w/ IPcomp.

ok markus@
---

diff --git a/sys/netinet/ip_ah.c b/sys/netinet/ip_ah.c
index 72fab716f0d..1be8d723649 100644
--- a/sys/netinet/ip_ah.c
+++ b/sys/netinet/ip_ah.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ip_ah.c,v 1.141 2018/07/11 09:07:59 mpi Exp $ */
+/*	$OpenBSD: ip_ah.c,v 1.142 2018/07/12 15:51:50 mpi Exp $ */
 /*
  * The authors of this code are John Ioannidis (ji@tla.org),
  * Angelos D. Keromytis (kermit@csd.uch.gr) and
@@ -76,7 +76,6 @@
 #define DPRINTF(x)
 #endif
 
-void	ah_output_cb(struct cryptop *);
 int	ah_massage_headers(struct mbuf **, int, int, int, int);
 
 const unsigned char ipseczeroes[IPSEC_ZEROES_SIZE]; /* zeroes! */
@@ -1132,7 +1131,7 @@ ah_output(struct mbuf *m, struct tdb *tdb, struct mbuf **mp, int skip,
 	crp->crp_ilen = m->m_pkthdr.len; /* Total input length. */
 	crp->crp_flags = CRYPTO_F_IMBUF;
 	crp->crp_buf = (caddr_t)m;
-	crp->crp_callback = ah_output_cb;
+	crp->crp_callback = ipsec_output_cb;
 	crp->crp_sid = tdb->tdb_cryptoid;
 	crp->crp_opaque = (caddr_t)tc;
 
@@ -1154,52 +1153,14 @@ ah_output(struct mbuf *m, struct tdb *tdb, struct mbuf **mp, int skip,
 }
 
 /*
- * AH output callback, called directly from the crypto handler.
+ * AH output callback.
  */
-void
-ah_output_cb(struct cryptop *crp)
+int
+ah_output_cb(struct tdb *tdb, struct tdb_crypto *tc, struct mbuf *m, int ilen,
+    int olen)
 {
-	int skip;
-	struct tdb_crypto *tc = NULL;
-	struct tdb *tdb = NULL;
-	struct mbuf *m;
-	caddr_t ptr;
-
-	tc = (struct tdb_crypto *) crp->crp_opaque;
-	skip = tc->tc_skip;
-	ptr = (caddr_t) (tc + 1);
-
-	m = (struct mbuf *) crp->crp_buf;
-	if (m == NULL) {
-		/* Shouldn't happen... */
-		DPRINTF(("%s: bogus returned buffer from crypto\n", __func__));
-		ahstat_inc(ahs_crypto);
-		goto droponly;
-	}
-
-	NET_LOCK();
-
-	tdb = gettdb(tc->tc_rdomain, tc->tc_spi, &tc->tc_dst, tc->tc_proto);
-	if (tdb == NULL) {
-		DPRINTF(("%s: TDB is expired while in crypto\n", __func__));
-		ahstat_inc(ahs_notdb);
-		goto baddone;
-	}
-
-	/* Check for crypto errors. */
-	if (crp->crp_etype) {
-		if (crp->crp_etype == EAGAIN) {
-			/* Reset the session ID */
-			if (tdb->tdb_cryptoid != 0)
-				tdb->tdb_cryptoid = crp->crp_sid;
-			NET_UNLOCK();
-			crypto_dispatch(crp);
-			return;
-		}
-		DPRINTF(("%s: crypto error %d\n", __func__, crp->crp_etype));
-		ahstat_inc(ahs_noxform);
-		goto baddone;
-	}
+	int skip = tc->tc_skip;
+	caddr_t ptr = (caddr_t) (tc + 1);
 
 	/*
 	 * Copy original headers (with the new protocol number) back
@@ -1208,18 +1169,13 @@ ah_output_cb(struct cryptop *crp)
 	m_copyback(m, 0, skip, ptr, M_NOWAIT);
 
 	/* No longer needed. */
-	crypto_freereq(crp);
 	free(tc, M_XDATA, 0);
 
-	if (ipsp_process_done(m, tdb))
+	/* Call the IPsec input callback. */
+	if (ipsp_process_done(m, tdb)) {
 		ahstat_inc(ahs_outfail);
-	NET_UNLOCK();
-	return;
+		return -1;
+	}
 
- baddone:
-	NET_UNLOCK();
- droponly:
-	m_freem(m);
-	crypto_freereq(crp);
-	free(tc, M_XDATA, 0);
+	return 0;
 }
diff --git a/sys/netinet/ip_esp.c b/sys/netinet/ip_esp.c
index 657e39ab887..990ceb59a80 100644
--- a/sys/netinet/ip_esp.c
+++ b/sys/netinet/ip_esp.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ip_esp.c,v 1.156 2018/07/11 09:07:59 mpi Exp $ */
+/*	$OpenBSD: ip_esp.c,v 1.157 2018/07/12 15:51:50 mpi Exp $ */
 /*
  * The authors of this code are John Ioannidis (ji@tla.org),
  * Angelos D. Keromytis (kermit@csd.uch.gr) and
@@ -69,8 +69,6 @@
 
 #include "bpfilter.h"
 
-void esp_output_cb(struct cryptop *);
-
 #ifdef ENCDEBUG
 #define DPRINTF(x)	if (encdebug) printf x
 #else
@@ -966,7 +964,7 @@ esp_output(struct mbuf *m, struct tdb *tdb, struct mbuf **mp, int skip,
 	crp->crp_ilen = m->m_pkthdr.len; /* Total input length. */
 	crp->crp_flags = CRYPTO_F_IMBUF;
 	crp->crp_buf = (caddr_t)m;
-	crp->crp_callback = esp_output_cb;
+	crp->crp_callback = ipsec_output_cb;
 	crp->crp_opaque = (caddr_t)tc;
 	crp->crp_sid = tdb->tdb_cryptoid;
 
@@ -1005,66 +1003,20 @@ esp_output(struct mbuf *m, struct tdb *tdb, struct mbuf **mp, int skip,
 	return error;
 }
 
-/*
- * ESP output callback, called directly by the crypto driver.
- */
-void
-esp_output_cb(struct cryptop *crp)
+int
+esp_output_cb(struct tdb *tdb, struct tdb_crypto *tc, struct mbuf *m, int ilen,
+    int olen)
 {
-	struct tdb_crypto *tc;
-	struct tdb *tdb;
-	struct mbuf *m;
-
-	tc = (struct tdb_crypto *) crp->crp_opaque;
-
-	m = (struct mbuf *) crp->crp_buf;
-	if (m == NULL) {
-		/* Shouldn't happen... */
-		DPRINTF(("%s: bogus returned buffer from crypto\n", __func__));
-		espstat_inc(esps_crypto);
-		goto droponly;
-	}
-
-	NET_LOCK();
-
-	tdb = gettdb(tc->tc_rdomain, tc->tc_spi, &tc->tc_dst, tc->tc_proto);
-	if (tdb == NULL) {
-		DPRINTF(("%s: TDB is expired while in crypto\n", __func__));
-		espstat_inc(esps_notdb);
-		goto baddone;
-	}
-
-	/* Check for crypto errors. */
-	if (crp->crp_etype) {
-		if (crp->crp_etype == EAGAIN) {
-			/* Reset the session ID */
-			if (tdb->tdb_cryptoid != 0)
-				tdb->tdb_cryptoid = crp->crp_sid;
-			NET_UNLOCK();
-			crypto_dispatch(crp);
-			return;
-		}
-		DPRINTF(("%s: crypto error %d\n", __func__, crp->crp_etype));
-		espstat_inc(esps_noxform);
-		goto baddone;
-	}
-
 	/* Release crypto descriptors. */
-	crypto_freereq(crp);
 	free(tc, M_XDATA, 0);
 
 	/* Call the IPsec input callback. */
-	if (ipsp_process_done(m, tdb))
+	if (ipsp_process_done(m, tdb)) {
 		espstat_inc(esps_outfail);
-	NET_UNLOCK();
-	return;
+		return -1;
+	}
 
- baddone:
-	NET_UNLOCK();
- droponly:
-	m_freem(m);
-	crypto_freereq(crp);
-	free(tc, M_XDATA, 0);
+	return 0;
 }
 
 #define SEEN_SIZE	howmany(TDB_REPLAYMAX, 32)
diff --git a/sys/netinet/ip_ipcomp.c b/sys/netinet/ip_ipcomp.c
index f7f6a78c636..3e83110e320 100644
--- a/sys/netinet/ip_ipcomp.c
+++ b/sys/netinet/ip_ipcomp.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ip_ipcomp.c,v 1.62 2018/07/11 09:07:59 mpi Exp $ */
+/* $OpenBSD: ip_ipcomp.c,v 1.63 2018/07/12 15:51:50 mpi Exp $ */
 
 /*
  * Copyright (c) 2001 Jean-Jacques Bernard-Gundol (jj@wabbitt.org)
@@ -56,8 +56,6 @@
 
 #include "bpfilter.h"
 
-void ipcomp_output_cb(struct cryptop *);
-
 #ifdef ENCDEBUG
 #define DPRINTF(x)      if (encdebug) printf x
 #else
@@ -467,7 +465,7 @@ ipcomp_output(struct mbuf *m, struct tdb *tdb, struct mbuf **mp, int skip,
 	crp->crp_ilen = m->m_pkthdr.len;	/* Total input length */
 	crp->crp_flags = CRYPTO_F_IMBUF;
 	crp->crp_buf = (caddr_t)m;
-	crp->crp_callback = ipcomp_output_cb;
+	crp->crp_callback = ipsec_output_cb;
 	crp->crp_opaque = (caddr_t)tc;
 	crp->crp_sid = tdb->tdb_cryptoid;
 
@@ -480,14 +478,13 @@ ipcomp_output(struct mbuf *m, struct tdb *tdb, struct mbuf **mp, int skip,
 }
 
 /*
- * IPComp output callback, called directly from the crypto driver
+ * IPComp output callback.
  */
-void
-ipcomp_output_cb(struct cryptop *crp)
+int
+ipcomp_output_cb(struct tdb *tdb, struct tdb_crypto *tc, struct mbuf *m,
+    int ilen, int olen)
 {
-	struct tdb_crypto *tc;
-	struct tdb *tdb;
-	struct mbuf *m, *mo;
+	struct mbuf *mo;
 	int skip, rlen, roff;
 	u_int16_t cpi;
 	struct ip *ip;
@@ -499,51 +496,14 @@ ipcomp_output_cb(struct cryptop *crp)
 	char buf[INET6_ADDRSTRLEN];
 #endif
 
-	tc = (struct tdb_crypto *) crp->crp_opaque;
 	skip = tc->tc_skip;
-	rlen = crp->crp_ilen - skip;
-
-	m = (struct mbuf *) crp->crp_buf;
-	if (m == NULL) {
-		/* Shouldn't happen... */
-		DPRINTF(("%s: bogus returned buffer from crypto\n", __func__));
-		ipcompstat_inc(ipcomps_crypto);
-		goto droponly;
-	}
-
-	NET_LOCK();
-
-	tdb = gettdb(tc->tc_rdomain, tc->tc_spi, &tc->tc_dst, tc->tc_proto);
-	if (tdb == NULL) {
-		DPRINTF(("%s: TDB expired while in crypto\n", __func__));
-		ipcompstat_inc(ipcomps_notdb);
-		goto baddone;
-	}
-
-	/* Check for crypto errors. */
-	if (crp->crp_etype) {
-		if (crp->crp_etype == EAGAIN) {
-			/* Reset the session ID */
-			if (tdb->tdb_cryptoid != 0)
-				tdb->tdb_cryptoid = crp->crp_sid;
-			NET_UNLOCK();
-			crypto_dispatch(crp);
-			return;
-		}
-		DPRINTF(("%s: crypto error %d\n", __func__, crp->crp_etype));
-		ipcompstat_inc(ipcomps_noxform);
-		goto baddone;
-	}
+	rlen = ilen - skip;
 
 	/* Check sizes. */
-	if (rlen < crp->crp_olen) {
+	if (rlen < olen) {
 		/* Compression was useless, we have lost time. */
 		ipcompstat_inc(ipcomps_minlen); /* misnomer, but like to count */
-		crypto_freereq(crp);
-		if (ipsp_process_done(m, tdb))
-			ipcompstat_inc(ipcomps_outfail);
-		NET_UNLOCK();
-		return;
+		goto skiphdr;
 	}
 
 	/* Inject IPCOMP header */
@@ -583,22 +543,20 @@ ipcomp_output_cb(struct cryptop *crp)
 		    ntohl(tdb->tdb_spi)));
 		ipcompstat_inc(ipcomps_nopf);
 		goto baddone;
-		break;
 	}
 
+ skiphdr:
 	/* Release the crypto descriptor. */
-	crypto_freereq(crp);
 	free(tc, M_XDATA, 0);
 
-	if (ipsp_process_done(m, tdb))
+	if (ipsp_process_done(m, tdb)) {
 		ipcompstat_inc(ipcomps_outfail);
-	NET_UNLOCK();
-	return;
+		return -1;
+	}
+	return 0;
 
  baddone:
-	NET_UNLOCK();
- droponly:
 	m_freem(m);
-	crypto_freereq(crp);
 	free(tc, M_XDATA, 0);
+	return -1;
 }
diff --git a/sys/netinet/ip_ipsp.h b/sys/netinet/ip_ipsp.h
index f1685adfc34..ba8652e7773 100644
--- a/sys/netinet/ip_ipsp.h
+++ b/sys/netinet/ip_ipsp.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ip_ipsp.h,v 1.191 2018/07/11 09:07:59 mpi Exp $	*/
+/*	$OpenBSD: ip_ipsp.h,v 1.192 2018/07/12 15:51:50 mpi Exp $	*/
 /*
  * The authors of this code are John Ioannidis (ji@tla.org),
  * Angelos D. Keromytis (kermit@csd.uch.gr),
@@ -574,6 +574,8 @@ int 	ah_zeroize(struct tdb *);
 int	ah_input(struct mbuf *, struct tdb *, int, int);
 int	ah_input_cb(struct tdb *, struct tdb_crypto *, struct mbuf *, int);
 int	ah_output(struct mbuf *, struct tdb *, struct mbuf **, int, int);
+int	ah_output_cb(struct tdb *, struct tdb_crypto *, struct mbuf *, int,
+	    int);
 int	ah_sysctl(int *, u_int, void *, size_t *, void *, size_t);
 
 int	ah4_input(struct mbuf **, int *, int, int);
@@ -591,6 +593,8 @@ int	esp_zeroize(struct tdb *);
 int	esp_input(struct mbuf *, struct tdb *, int, int);
 int	esp_input_cb(struct tdb *, struct tdb_crypto *, struct mbuf *, int);
 int	esp_output(struct mbuf *, struct tdb *, struct mbuf **, int, int);
+int	esp_output_cb(struct tdb *, struct tdb_crypto *, struct mbuf *, int,
+	    int);
 int	esp_sysctl(int *, u_int, void *, size_t *, void *, size_t);
 
 int	esp4_input(struct mbuf **, int *, int, int);
@@ -607,6 +611,8 @@ int	ipcomp_zeroize(struct tdb *);
 int	ipcomp_input(struct mbuf *, struct tdb *, int, int);
 int	ipcomp_input_cb(struct tdb *, struct tdb_crypto *, struct mbuf *, int);
 int	ipcomp_output(struct mbuf *, struct tdb *, struct mbuf **, int, int);
+int	ipcomp_output_cb(struct tdb *, struct tdb_crypto *, struct mbuf *, int,
+	    int);
 int	ipcomp_sysctl(int *, u_int, void *, size_t *, void *, size_t);
 int	ipcomp4_input(struct mbuf **, int *, int, int);
 #ifdef INET6
@@ -644,6 +650,7 @@ void	ipsec_init(void);
 int	ipsec_sysctl(int *, u_int, void *, size_t *, void *, size_t);
 int	ipsec_common_input(struct mbuf *, int, int, int, int, int);
 void	ipsec_input_cb(struct cryptop *);
+void	ipsec_output_cb(struct cryptop *);
 int	ipsec_common_input_cb(struct mbuf *, struct tdb *, int, int);
 int	ipsec_delete_policy(struct ipsec_policy *);
 ssize_t	ipsec_hdrsz(struct tdb *);
diff --git a/sys/netinet/ip_output.c b/sys/netinet/ip_output.c
index a140d1ddf80..162803b7264 100644
--- a/sys/netinet/ip_output.c
+++ b/sys/netinet/ip_output.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ip_output.c,v 1.346 2018/03/21 14:42:41 bluhm Exp $	*/
+/*	$OpenBSD: ip_output.c,v 1.347 2018/07/12 15:51:50 mpi Exp $	*/
 /*	$NetBSD: ip_output.c,v 1.28 1996/02/13 23:43:07 christos Exp $	*/
 
 /*
@@ -564,6 +564,7 @@ ip_output_ipsec_send(struct tdb *tdb, struct mbuf *m, struct route *ro, int fwd)
 	struct ifnet *encif;
 #endif
 	struct ip *ip;
+	int error;
 
 #if NPF > 0
 	/*
@@ -633,7 +634,10 @@ ip_output_ipsec_send(struct tdb *tdb, struct mbuf *m, struct route *ro, int fwd)
 	m->m_flags &= ~(M_MCAST | M_BCAST);
 
 	/* Callee frees mbuf */
-	return ipsp_process_packet(m, tdb, AF_INET, 0);
+	error = ipsp_process_packet(m, tdb, AF_INET, 0);
+	if (error)
+		ipsecstat_inc(ipsec_odrops);
+	return error;
 }
 #endif /* IPSEC */
 
diff --git a/sys/netinet/ipsec_output.c b/sys/netinet/ipsec_output.c
index fa5efbc18ae..840903d16bd 100644
--- a/sys/netinet/ipsec_output.c
+++ b/sys/netinet/ipsec_output.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ipsec_output.c,v 1.72 2018/06/04 12:13:01 bluhm Exp $ */
+/*	$OpenBSD: ipsec_output.c,v 1.73 2018/07/12 15:51:50 mpi Exp $ */
 /*
  * The author of this code is Angelos D. Keromytis (angelos@cis.upenn.edu)
  *
@@ -46,6 +46,8 @@
 #include <netinet/ip_ah.h>
 #include <netinet/ip_esp.h>
 #include <netinet/ip_ipcomp.h>
+
+#include <crypto/cryptodev.h>
 #include <crypto/xform.h>
 
 #ifdef ENCDEBUG
@@ -358,6 +360,8 @@ ipsp_process_packet(struct mbuf *m, struct tdb *tdb, int af, int tunalready)
 		goto drop;
 	}
 
+        ipsecstat_add(ipsec_ouncompbytes, m->m_pkthdr.len);
+
 	/* Non expansion policy for IPCOMP */
 	if (tdb->tdb_sproto == IPPROTO_IPCOMP) {
 		if ((m->m_pkthdr.len - hlen) < tdb->tdb_compalgxform->minlen) {
@@ -375,6 +379,81 @@ ipsp_process_packet(struct mbuf *m, struct tdb *tdb, int af, int tunalready)
 	return error;
 }
 
+/*
+ * IPsec output callback, called directly by the crypto driver.
+ */
+void
+ipsec_output_cb(struct cryptop *crp)
+{
+	struct tdb_crypto *tc = (struct tdb_crypto *) crp->crp_opaque;
+	struct mbuf *m = (struct mbuf *) crp->crp_buf;
+	struct tdb *tdb;
+	int error, ilen, olen;
+
+	if (m == NULL) {
+		DPRINTF(("%s: bogus returned buffer from crypto\n", __func__));
+		ipsecstat_inc(ipsec_crypto);
+		goto droponly;
+	}
+
+	NET_LOCK();
+	tdb = gettdb(tc->tc_rdomain, tc->tc_spi, &tc->tc_dst, tc->tc_proto);
+	if (tdb == NULL) {
+		DPRINTF(("%s: TDB is expired while in crypto\n", __func__));
+		ipsecstat_inc(ipsec_notdb);
+		goto baddone;
+	}
+
+	/* Check for crypto errors. */
+	if (crp->crp_etype) {
+		if (crp->crp_etype == EAGAIN) {
+			/* Reset the session ID */
+			if (tdb->tdb_cryptoid != 0)
+				tdb->tdb_cryptoid = crp->crp_sid;
+			NET_UNLOCK();
+			crypto_dispatch(crp);
+			return;
+		}
+		DPRINTF(("%s: crypto error %d\n", __func__, crp->crp_etype));
+		ipsecstat_inc(ipsec_noxform);
+		goto baddone;
+	}
+
+	olen = crp->crp_olen;
+	ilen = crp->crp_ilen;
+
+	/* Release crypto descriptors. */
+	crypto_freereq(crp);
+
+	switch (tdb->tdb_sproto) {
+	case IPPROTO_ESP:
+		error = esp_output_cb(tdb, tc, m, ilen, olen);
+		break;
+	case IPPROTO_AH:
+		error = ah_output_cb(tdb, tc, m, ilen, olen);
+		break;
+	case IPPROTO_IPCOMP:
+		error = ipcomp_output_cb(tdb, tc, m, ilen, olen);
+		break;
+	default:
+		panic("%s: unknown/unsupported security protocol %d",
+		    __func__, tdb->tdb_sproto);
+	}
+
+	NET_UNLOCK();
+	if (error)
+		ipsecstat_inc(ipsec_odrops);
+	return;
+
+ baddone:
+	NET_UNLOCK();
+ droponly:
+	m_freem(m);
+	free(tc, M_XDATA, 0);
+	crypto_freereq(crp);
+	ipsecstat_inc(ipsec_odrops);
+}
+
 /*
  * Called by the IPsec output transform callbacks, to transmit the packet
  * or do further processing, as necessary.
@@ -498,6 +577,9 @@ ipsp_process_done(struct mbuf *m, struct tdb *tdb)
 		return ipsp_process_packet(m, tdb->tdb_onext,
 		    tdb->tdb_dst.sa.sa_family, 0);
 
+	ipsecstat_inc(ipsec_opackets);
+	ipsecstat_add(ipsec_obytes, m->m_pkthdr.len);
+
 #if NPF > 0
 	/* Add pf tag if requested. */
 	pf_tag_packet(m, tdb->tdb_tag, -1);
diff --git a/sys/netinet6/ip6_output.c b/sys/netinet6/ip6_output.c
index c84b5503851..4b9fffbbff2 100644
--- a/sys/netinet6/ip6_output.c
+++ b/sys/netinet6/ip6_output.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ip6_output.c,v 1.237 2018/03/27 15:03:52 dhill Exp $	*/
+/*	$OpenBSD: ip6_output.c,v 1.238 2018/07/12 15:51:50 mpi Exp $	*/
 /*	$KAME: ip6_output.c,v 1.172 2001/03/25 09:55:56 itojun Exp $	*/
 
 /*
@@ -2765,6 +2765,7 @@ ip6_output_ipsec_send(struct tdb *tdb, struct mbuf *m, int tunalready, int fwd)
 #if NPF > 0
 	struct ifnet *encif;
 #endif
+	int error;
 
 #if NPF > 0
 	if ((encif = enc_getif(tdb->tdb_rdomain, tdb->tdb_tap)) == NULL ||
@@ -2786,6 +2787,9 @@ ip6_output_ipsec_send(struct tdb *tdb, struct mbuf *m, int tunalready, int fwd)
 	m->m_flags &= ~(M_BCAST | M_MCAST);	/* just in case */
 
 	/* Callee frees mbuf */
-	return ipsp_process_packet(m, tdb, AF_INET6, tunalready);
+	error = ipsp_process_packet(m, tdb, AF_INET6, tunalready);
+	if (error)
+		ipsecstat_inc(ipsec_odrops);
+	return error;
 }
 #endif /* IPSEC */