From: David Woodhouse <David.Woodhouse@intel.com>
Date: Fri, 2 Sep 2016 10:32:41 +0000 (+0100)
Subject: Reorder ESP sequence checks
X-Git-Tag: v7.08~94
X-Git-Url: https://www.infradead.org/git/?a=commitdiff_plain;h=122dcc953054b909fec643a4824edd95511cc30c;p=users%2Fdwmw2%2Fopenconnect.git

Reorder ESP sequence checks

Make it slightly cleaner... maybe.

Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
---

diff --git a/esp-seqno.c b/esp-seqno.c
index eef2ec9e..23ee219d 100644
--- a/esp-seqno.c
+++ b/esp-seqno.c
@@ -61,34 +61,10 @@ int verify_packet_seqno(struct openconnect_info *vpninfo,
 			     _("Accepting expected ESP packet with seq %u\n"),
 			     seq);
 		return 0;
-	} else if ((uint64_t)seq + 65 < esp->seq) {
-		/* Too old. We can't know if it's a replay. */
-		vpn_progress(vpninfo, PRG_DEBUG,
-			     _("Discarding ancient ESP packet with seq %u (expected %" PRIu64 ")\n"),
-			     seq, esp->seq);
-		return -EINVAL;
-	} else if (seq == esp->seq - 1) {
-		/* This is a repeat of the latest packet we already received. */
-	replayed:
-		vpn_progress(vpninfo, PRG_DEBUG,
-			     _("Discarding replayed ESP packet with seq %u\n"),
-			     seq);
-		return -EINVAL;
-	} else if (seq < esp->seq) {
-		/* Within the backlog window, so we remember whether we've seen it or not. */
-		uint64_t mask = 1ULL << (esp->seq - seq - 2);
-
-		if (!(esp->seq_backlog & mask))
-			goto replayed;
-
-		esp->seq_backlog &= ~mask;
-		vpn_progress(vpninfo, PRG_TRACE,
-			     _("Accepting out-of-order ESP packet with seq %u (expected %" PRIu64 ")\n"),
-			     seq, esp->seq);
-		return 0;
-	} else {
-		/* The packet we were expecting has gone missing; this one is newer. */
-		uint64_t delta = seq - esp->seq;
+	} else if (seq > esp->seq) {
+		/* The packet we were expecting has gone missing; this one is newer.
+		 * We always advance the window to accommodate it. */
+		uint32_t delta = seq - esp->seq;
 
 		if (delta >= 64) {
 			/* We jumped a long way into the future. We have not seen
@@ -116,6 +92,37 @@ int verify_packet_seqno(struct openconnect_info *vpninfo,
 			     seq, esp->seq);
 		esp->seq = (uint64_t)seq + 1;
 		return 0;
+	} else {
+		/* This packet is older than the one we were expecting. By how much...? */
+		uint32_t delta = esp->seq - seq;
+
+		/* delta==0 is the overflow case where esp->seq is 0x100000000 and seq is 0 */
+		if (delta > 65 || delta == 0) {
+			/* Too old. We can't know if it's a replay. */
+			vpn_progress(vpninfo, PRG_DEBUG,
+				     _("Discarding ancient ESP packet with seq %u (expected %" PRIu64 ")\n"),
+				     seq, esp->seq);
+			return -EINVAL;
+		} else if (delta == 1) {
+			/* This is a repeat of the latest packet we already received. */
+		replayed:
+			vpn_progress(vpninfo, PRG_DEBUG,
+				     _("Discarding replayed ESP packet with seq %u\n"),
+				     seq);
+			return -EINVAL;
+		} else {
+			/* Within the backlog window, so we remember whether we've seen it or not. */
+			uint64_t mask = 1ULL << delta - 2;
+
+			if (!(esp->seq_backlog & mask))
+				goto replayed;
+
+			esp->seq_backlog &= ~mask;
+			vpn_progress(vpninfo, PRG_TRACE,
+				     _("Accepting out-of-order ESP packet with seq %u (expected %" PRIu64 ")\n"),
+				     seq, esp->seq);
+			return 0;
+		}
 	}
 }
 
diff --git a/tests/seqtest.c b/tests/seqtest.c
index d38646cc..5f9a5df3 100644
--- a/tests/seqtest.c
+++ b/tests/seqtest.c
@@ -72,13 +72,14 @@ int main(void)
 	    !verify_packet_seqno(NULL, &esptest, 270) ||
 	    verify_packet_seqno(NULL, &esptest, 0xfffffffd) ||
 	    !verify_packet_seqno(NULL, &esptest, 1) ||
-	    verify_packet_seqno(NULL, &esptest, 0xffffffc0) ||
+	    verify_packet_seqno(NULL, &esptest, 0xffffffc1) ||
 	    verify_packet_seqno(NULL, &esptest, 0xfffffffc) ||
 	    verify_packet_seqno(NULL, &esptest, 0xffffffff) ||
-	    !verify_packet_seqno(NULL, &esptest, 1) ||
+	    !verify_packet_seqno(NULL, &esptest, 0) ||
 	    !verify_packet_seqno(NULL, &esptest, 0xffffffbe) ||
 	    verify_packet_seqno(NULL, &esptest, 0xffffffbf) ||
-	    !verify_packet_seqno(NULL, &esptest, 0xffffffc0))
+	    !verify_packet_seqno(NULL, &esptest, 0xffffffc1) ||
+	    verify_packet_seqno(NULL, &esptest, 0xffffffc0))
 		return 1;
 
 	return 0;