Fix a DoS attack whereby an attacker could cause the replay counter to

advance with unauthenticated packets, thereby causing valid packets to
be discarded as replays. This has been sitting in my tree for a while,
and I've forgotten who it was that pointed out the problem.

2 files changed, 103 insertions, 27 deletions

diff --git a/sys/netinet/ip_ah.c b/sys/netinet/ip_ah.c
index 9dccd3e0373..dd5da97c1e6 100644
--- a/sys/netinet/ip_ah.c
+++ b/sys/netinet/ip_ah.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ip_ah.c,v 1.63 2001/06/26 06:18:58 angelos Exp $ */
+/*	$OpenBSD: ip_ah.c,v 1.64 2002/05/31 02:22:21 angelos Exp $ */
 /*
  * The authors of this code are John Ioannidis (ji@tla.org),
  * Angelos D. Keromytis (kermit@csd.uch.gr) and
@@ -516,7 +516,7 @@ ah_input(struct mbuf *m, struct tdb *tdb, int skip, int protoff)
 		btsx = ntohl(btsx);
 
 		switch (checkreplaywindow32(btsx, 0, &(tdb->tdb_rpl),
-		    tdb->tdb_wnd, &(tdb->tdb_bitmap))) {
+		    tdb->tdb_wnd, &(tdb->tdb_bitmap), 0)) {
 		case 0: /* All's well. */
 			break;
 
@@ -535,7 +535,6 @@ ah_input(struct mbuf *m, struct tdb *tdb, int skip, int protoff)
 			    "SA %s/%08x\n", ipsp_address(tdb->tdb_dst),
 			    ntohl(tdb->tdb_spi)));
 
-			ahstat.ahs_replay++;
 			m_freem(m);
 			return ENOBUFS;
 
@@ -693,6 +692,7 @@ ah_input_cb(void *op)
 	struct cryptop *crp;
 	struct m_tag *mtag;
 	struct tdb *tdb;
+	u_int32_t btsx;
 	u_int8_t prot;
 	caddr_t ptr;
 	int s, err;
@@ -709,8 +709,8 @@ ah_input_cb(void *op)
 	s = spltdb();
 
 	tdb = gettdb(tc->tc_spi, &tc->tc_dst, tc->tc_proto);
-	FREE(tc, M_XDATA);
 	if (tdb == NULL) {
+		FREE(tc, M_XDATA);
 		ahstat.ahs_notdb++;
 		DPRINTF(("ah_input_cb(): TDB is expired while in crypto"));
 		goto baddone;
@@ -725,9 +725,11 @@ ah_input_cb(void *op)
 
 		if (crp->crp_etype == EAGAIN) {
 			splx(s);
+			FREE(tc, M_XDATA);
 			return crypto_dispatch(crp);
 		}
 
+		FREE(tc, M_XDATA);
 		ahstat.ahs_noxform++;
 		DPRINTF(("ah_input_cb(): crypto error %d\n", crp->crp_etype));
 		error = crp->crp_etype;
@@ -739,6 +741,7 @@ ah_input_cb(void *op)
 
 	/* Shouldn't happen... */
 	if (m == NULL) {
+		FREE(tc, M_XDATA);
 		ahstat.ahs_crypto++;
 		DPRINTF(("ah_input_cb(): bogus returned buffer from "
 		    "crypto\n"));
@@ -763,6 +766,8 @@ ah_input_cb(void *op)
 
 		/* Verify authenticator. */
 		if (bcmp(ptr + skip + rplen, calc, ahx->authsize)) {
+			FREE(tc, M_XDATA);
+
 			DPRINTF(("ah_input(): authentication failed for "
 			    "packet in SA %s/%08x\n",
 			    ipsp_address(tdb->tdb_dst), ntohl(tdb->tdb_spi)));
@@ -783,6 +788,48 @@ ah_input_cb(void *op)
 		m_copyback(m, protoff, sizeof(u_int8_t), &prot);
 	}
 
+	FREE(tc, M_XDATA);
+
+	/* Replay window checking, if applicable. */
+	if ((tdb->tdb_wnd > 0) && (!(tdb->tdb_flags & TDBF_NOREPLAY))) {
+		m_copydata(m, skip + offsetof(struct ah, ah_rpl),
+		    sizeof(u_int32_t), (caddr_t) &btsx);
+		btsx = ntohl(btsx);
+
+		switch (checkreplaywindow32(btsx, 0, &(tdb->tdb_rpl),
+		    tdb->tdb_wnd, &(tdb->tdb_bitmap), 1)) {
+		case 0: /* All's well. */
+			break;
+
+		case 1:
+			DPRINTF(("ah_input(): replay counter wrapped for "
+			    "SA %s/%08x\n", ipsp_address(tdb->tdb_dst),
+			    ntohl(tdb->tdb_spi)));
+
+			ahstat.ahs_wrap++;
+			error = ENOBUFS;
+			goto baddone;
+
+		case 2:
+		case 3:
+			DPRINTF(("ah_input_cb(): duplicate packet received in "
+			    "SA %s/%08x\n", ipsp_address(tdb->tdb_dst),
+			    ntohl(tdb->tdb_spi)));
+
+			error = ENOBUFS;
+			goto baddone;
+
+		default:
+			DPRINTF(("ah_input_cb(): bogus value from "
+			    "checkreplaywindow32() in SA %s/%08x\n",
+			    ipsp_address(tdb->tdb_dst), ntohl(tdb->tdb_spi)));
+
+			ahstat.ahs_replay++;
+			error = ENOBUFS;
+			goto baddone;
+		}
+	}
+
 	/* Record the beginning of the AH header. */
 	m1 = m_getptr(m, skip, &roff);
 	if (m1 == NULL) {
diff --git a/sys/netinet/ip_esp.c b/sys/netinet/ip_esp.c
index 87e8b516062..e760cefa7db 100644
--- a/sys/netinet/ip_esp.c
+++ b/sys/netinet/ip_esp.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ip_esp.c,v 1.69 2001/06/26 06:18:59 angelos Exp $ */
+/*	$OpenBSD: ip_esp.c,v 1.70 2002/05/31 02:22:21 angelos Exp $ */
 /*
  * The authors of this code are John Ioannidis (ji@tla.org),
  * Angelos D. Keromytis (kermit@csd.uch.gr) and
@@ -301,36 +301,35 @@ esp_input(struct mbuf *m, struct tdb *tdb, int skip, int protoff)
 		}
 	}
 
-	/* Replay window checking, if appropriate */
+	/* Replay window checking, if appropriate -- no value commitment. */
 	if ((tdb->tdb_wnd > 0) && (!(tdb->tdb_flags & TDBF_NOREPLAY)))
 	{
 		m_copydata(m, skip + sizeof(u_int32_t), sizeof(u_int32_t),
 		    (unsigned char *) &btsx);
 		btsx = ntohl(btsx);
 
-		switch (checkreplaywindow32(btsx, 0, &(tdb->tdb_rpl), tdb->tdb_wnd,
-		    &(tdb->tdb_bitmap)))
+		switch (checkreplaywindow32(btsx, 0, &(tdb->tdb_rpl),
+		    tdb->tdb_wnd, &(tdb->tdb_bitmap), 0))
 		{
 		case 0: /* All's well */
 			break;
 
 		case 1:
+			m_freem(m);
 			DPRINTF(("esp_input(): replay counter wrapped for SA %s/%08x\n", ipsp_address(tdb->tdb_dst), ntohl(tdb->tdb_spi)));
 			espstat.esps_wrap++;
-			m_freem(m);
 			return EACCES;
 
 		case 2:
 		case 3:
 			DPRINTF(("esp_input(): duplicate packet received in SA %s/%08x\n", ipsp_address(tdb->tdb_dst), ntohl(tdb->tdb_spi)));
-			espstat.esps_replay++;
 			m_freem(m);
 			return EACCES;
 
 		default:
+			m_freem(m);
 			DPRINTF(("esp_input(): bogus value from checkreplaywindow32() in SA %s/%08x\n", ipsp_address(tdb->tdb_dst), ntohl(tdb->tdb_spi)));
 			espstat.esps_replay++;
-			m_freem(m);
 			return EACCES;
 		}
 	}
@@ -480,6 +479,7 @@ esp_input_cb(void *op)
 	struct cryptop *crp;
 	struct m_tag *mtag;
 	struct tdb *tdb;
+	u_int32_t btsx;
 	int s, err = 0;
 	caddr_t ptr;
 
@@ -563,6 +563,39 @@ esp_input_cb(void *op)
 		m_adj(m, -(esph->authsize));
 	}
 
+	/* Replay window checking, if appropriate */
+	if ((tdb->tdb_wnd > 0) && (!(tdb->tdb_flags & TDBF_NOREPLAY)))
+	{
+		m_copydata(m, skip + sizeof(u_int32_t), sizeof(u_int32_t),
+		    (unsigned char *) &btsx);
+		btsx = ntohl(btsx);
+
+		switch (checkreplaywindow32(btsx, 0, &(tdb->tdb_rpl),
+		    tdb->tdb_wnd, &(tdb->tdb_bitmap), 1))
+		{
+		case 0: /* All's well */
+			break;
+
+		case 1:
+			DPRINTF(("esp_input_cb(): replay counter wrapped for SA %s/%08x\n", ipsp_address(tdb->tdb_dst), ntohl(tdb->tdb_spi)));
+			espstat.esps_wrap++;
+			error = EACCES;
+			goto baddone;
+
+		case 2:
+		case 3:
+			DPRINTF(("esp_input_cb(): duplicate packet received in SA %s/%08x\n", ipsp_address(tdb->tdb_dst), ntohl(tdb->tdb_spi)));
+			error = EACCES;
+			goto baddone;
+
+		default:
+			DPRINTF(("esp_input_cb(): bogus value from checkreplaywindow32() in SA %s/%08x\n", ipsp_address(tdb->tdb_dst), ntohl(tdb->tdb_spi)));
+			espstat.esps_replay++;
+			error = EACCES;
+			goto baddone;
+		}
+	}
+
 	/* Release the crypto descriptors */
 	crypto_freereq(crp);
 
@@ -747,19 +780,6 @@ esp_output(struct mbuf *m, struct tdb *tdb, struct mbuf **mp, int skip,
 
 	espstat.esps_output++;
 
-	/*
-	 * Check for replay counter wrap-around in automatic (not
-	 * manual) keying.
-	 */
-	if ((!(tdb->tdb_flags & TDBF_NOREPLAY)) &&
-	    (tdb->tdb_rpl == 0) && (tdb->tdb_wnd > 0)) {
-		DPRINTF(("esp_output(): SA %s/%08x should have expired\n",
-		    ipsp_address(tdb->tdb_dst), ntohl(tdb->tdb_spi)));
-		m_freem(m);
-		espstat.esps_wrap++;
-		return EACCES;
-	}
-
 	switch (tdb->tdb_dst.sa.sa_family) {
 #ifdef INET
 	case AF_INET:
@@ -856,7 +876,7 @@ esp_output(struct mbuf *m, struct tdb *tdb, struct mbuf **mp, int skip,
 		    "SA %s/%08x\n", ipsp_address(tdb->tdb_dst),
 		    ntohl(tdb->tdb_spi)));
 		m_freem(m);
-		espstat.esps_wrap++;
+		espstat.esps_hdrops++;
 		return ENOBUFS;
 	}
 
@@ -1068,9 +1088,18 @@ esp_output_cb(void *op)
  */
 int
 checkreplaywindow32(u_int32_t seq, u_int32_t initial, u_int32_t *lastseq,
-    u_int32_t window, u_int32_t *bitmap)
+    u_int32_t window, u_int32_t *bitmap, int commit)
 {
-	u_int32_t diff;
+	u_int32_t diff, llseq, lbitmap;
+
+	/* Just do the checking, without "committing" any changes. */
+	if (commit == 0) {
+		llseq = *lastseq;
+		lbitmap = *bitmap;
+
+		lastseq = &llseq;
+		bitmap = &lbitmap;
+	}
 
 	seq -= initial;