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 eef2ec9e08557ebaa7a9825536dd05b460642652..23ee219d14b58cf5983da1c421c7377c85ee62b6 100644 (file)
--- 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 d38646cc92573f83bb5d3c1d220ab4f37821fb12..5f9a5df3ff8dce31fda1a46fc6e776fff66494b4 100644 (file)
--- 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;