From 25a9c1bd139a3d65f7ca723f0d90e0ad33f44452 Mon Sep 17 00:00:00 2001 From: bluhm Date: Sun, 18 Jul 2021 14:38:20 +0000 Subject: [PATCH] The IPsec authentication before decryption used a different replay counter than after decryption. This could result in "esp_input_cb: authentication failed for packet in SA" errors. As we run crypto operations async, thousands of packets are stored in the crypto task. During the queueing the replay counter of the tdb can change. Then the higher 32 bits may increment although the lower 32 bits did not wrap. checkreplaywindow() must be called twice per packet with the same replay counter. Store the value in struct tdb_crypto while dangling in the task queue and doing crypto operations. tested by Hrvoje Popovski; joint work with tobhe@ --- sys/netinet/ip_ah.c | 9 ++++++--- sys/netinet/ip_esp.c | 16 ++++++++++------ sys/netinet/ip_ipsp.h | 9 +++++---- 3 files changed, 21 insertions(+), 13 deletions(-) diff --git a/sys/netinet/ip_ah.c b/sys/netinet/ip_ah.c index f6b5640239a..b9c4ffab16c 100644 --- a/sys/netinet/ip_ah.c +++ b/sys/netinet/ip_ah.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ip_ah.c,v 1.150 2021/07/08 21:07:19 bluhm Exp $ */ +/* $OpenBSD: ip_ah.c,v 1.151 2021/07/18 14:38:20 bluhm Exp $ */ /* * The authors of this code are John Ioannidis (ji@tla.org), * Angelos D. Keromytis (kermit@csd.uch.gr) and @@ -550,7 +550,7 @@ ah_input(struct mbuf *m, struct tdb *tdb, int skip, int protoff) sizeof(u_int32_t), &btsx); btsx = ntohl(btsx); - switch (checkreplaywindow(tdb, btsx, &esn, 0)) { + switch (checkreplaywindow(tdb, tdb->tdb_rpl, btsx, &esn, 0)) { case 0: /* All's well. */ break; case 1: @@ -697,6 +697,7 @@ ah_input(struct mbuf *m, struct tdb *tdb, int skip, int protoff) tc->tc_proto = tdb->tdb_sproto; tc->tc_rdomain = tdb->tdb_rdomain; memcpy(&tc->tc_dst, &tdb->tdb_dst, sizeof(union sockaddr_union)); + tc->tc_rpl = tdb->tdb_rpl; KERNEL_LOCK(); error = crypto_dispatch(crp); @@ -715,6 +716,7 @@ ah_input_cb(struct tdb *tdb, struct tdb_crypto *tc, struct mbuf *m, int clen) { const struct auth_hash *ahx = tdb->tdb_authalgxform; int roff, rplen, skip, protoff; + u_int64_t rpl; u_int32_t btsx, esn; caddr_t ptr; unsigned char calc[AH_ALEN_MAX]; @@ -727,6 +729,7 @@ ah_input_cb(struct tdb *tdb, struct tdb_crypto *tc, struct mbuf *m, int clen) skip = tc->tc_skip; protoff = tc->tc_protoff; + rpl = tc->tc_rpl; rplen = AH_FLENGTH + sizeof(u_int32_t); @@ -756,7 +759,7 @@ ah_input_cb(struct tdb *tdb, struct tdb_crypto *tc, struct mbuf *m, int clen) sizeof(u_int32_t), &btsx); btsx = ntohl(btsx); - switch (checkreplaywindow(tdb, btsx, &esn, 1)) { + switch (checkreplaywindow(tdb, rpl, btsx, &esn, 1)) { case 0: /* All's well. */ #if NPFSYNC > 0 pfsync_update_tdb(tdb,0); diff --git a/sys/netinet/ip_esp.c b/sys/netinet/ip_esp.c index 9d6b4f831f0..11a323f34ef 100644 --- a/sys/netinet/ip_esp.c +++ b/sys/netinet/ip_esp.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ip_esp.c,v 1.168 2021/07/16 15:08:39 bluhm Exp $ */ +/* $OpenBSD: ip_esp.c,v 1.169 2021/07/18 14:38:20 bluhm Exp $ */ /* * The authors of this code are John Ioannidis (ji@tla.org), * Angelos D. Keromytis (kermit@csd.uch.gr) and @@ -388,7 +388,7 @@ esp_input(struct mbuf *m, struct tdb *tdb, int skip, int protoff) &btsx); btsx = ntohl(btsx); - switch (checkreplaywindow(tdb, btsx, &esn, 0)) { + switch (checkreplaywindow(tdb, tdb->tdb_rpl, btsx, &esn, 0)) { case 0: /* All's well */ break; case 1: @@ -511,6 +511,7 @@ esp_input(struct mbuf *m, struct tdb *tdb, int skip, int protoff) tc->tc_proto = tdb->tdb_sproto; tc->tc_rdomain = tdb->tdb_rdomain; tc->tc_dst = tdb->tdb_dst; + tc->tc_rpl = tdb->tdb_rpl; /* Decryption descriptor */ if (espx) { @@ -549,6 +550,7 @@ esp_input_cb(struct tdb *tdb, struct tdb_crypto *tc, struct mbuf *m, int clen) int hlen, roff, skip, protoff; struct mbuf *m1, *mo; const struct auth_hash *esph; + u_int64_t rpl; u_int32_t btsx, esn; caddr_t ptr; #ifdef ENCDEBUG @@ -557,6 +559,7 @@ esp_input_cb(struct tdb *tdb, struct tdb_crypto *tc, struct mbuf *m, int clen) skip = tc->tc_skip; protoff = tc->tc_protoff; + rpl = tc->tc_rpl; NET_ASSERT_LOCKED(); @@ -590,7 +593,7 @@ esp_input_cb(struct tdb *tdb, struct tdb_crypto *tc, struct mbuf *m, int clen) &btsx); btsx = ntohl(btsx); - switch (checkreplaywindow(tdb, btsx, &esn, 1)) { + switch (checkreplaywindow(tdb, rpl, btsx, &esn, 1)) { case 0: /* All's well */ #if NPFSYNC > 0 pfsync_update_tdb(tdb,0); @@ -1049,14 +1052,15 @@ esp_output_cb(struct tdb *tdb, struct tdb_crypto *tc, struct mbuf *m, int ilen, * return 3 for packet within current window but already received */ int -checkreplaywindow(struct tdb *tdb, u_int32_t seq, u_int32_t *seqh, int commit) +checkreplaywindow(struct tdb *tdb, u_int64_t t, u_int32_t seq, u_int32_t *seqh, + int commit) { u_int32_t tl, th, wl; u_int32_t packet, window = TDB_REPLAYMAX - TDB_REPLAYWASTE; int idx, esn = tdb->tdb_flags & TDBF_ESN; - tl = (u_int32_t)tdb->tdb_rpl; - th = (u_int32_t)(tdb->tdb_rpl >> 32); + tl = (u_int32_t)t; + th = (u_int32_t)(t >> 32); /* Zero SN is not allowed */ if ((esn && seq == 0 && tl <= AH_HMAC_INITIAL_RPL && th == 0) || diff --git a/sys/netinet/ip_ipsp.h b/sys/netinet/ip_ipsp.h index 41801db1200..ba2c7c616fb 100644 --- a/sys/netinet/ip_ipsp.h +++ b/sys/netinet/ip_ipsp.h @@ -1,4 +1,4 @@ -/* $OpenBSD: ip_ipsp.h,v 1.201 2021/07/13 08:16:17 mvs Exp $ */ +/* $OpenBSD: ip_ipsp.h,v 1.202 2021/07/18 14:38:20 bluhm Exp $ */ /* * The authors of this code are John Ioannidis (ji@tla.org), * Angelos D. Keromytis (kermit@csd.uch.gr), @@ -432,12 +432,13 @@ struct tdb_ident { }; struct tdb_crypto { - u_int32_t tc_spi; union sockaddr_union tc_dst; - u_int8_t tc_proto; + u_int64_t tc_rpl; + u_int32_t tc_spi; int tc_protoff; int tc_skip; u_int tc_rdomain; + u_int8_t tc_proto; }; struct ipsecinit { @@ -622,7 +623,7 @@ int tcp_signature_tdb_output(struct mbuf *, struct tdb *, struct mbuf **, int, int); /* Replay window */ -int checkreplaywindow(struct tdb *, u_int32_t, u_int32_t *, int); +int checkreplaywindow(struct tdb *, u_int64_t, u_int32_t, u_int32_t *, int); /* Packet processing */ int ipsp_process_packet(struct mbuf *, struct tdb *, int, int); -- 2.20.1