From 3ed526d1e841940b80b13fa0eec1f51ec669e5aa Mon Sep 17 00:00:00 2001
From: Achim Kraus <achim.kraus@bosch.io>
Date: Thu, 23 Dec 2021 19:32:08 +0100
Subject: [PATCH] dtls.c: cleanup record_sequence_filter.

Reorder check and reuse record_sequence difference and drop duplicates
earlier.

Fixes: Issue #71

Signed-off-by: Achim Kraus <achim.kraus@bosch.io>
---
 crypto.h |  8 +++++-
 dtls.c   | 79 ++++++++++++++++++++++++++++++--------------------------
 2 files changed, 50 insertions(+), 37 deletions(-)

diff --git a/crypto.h b/crypto.h
index 41148268..5df6047a 100644
--- a/crypto.h
+++ b/crypto.h
@@ -92,7 +92,13 @@ typedef struct {
 } dtls_handshake_parameters_psk_t;
 
 typedef struct {
-    uint64_t cseq;
+    uint64_t cseq;        /**< current read sequence number */
+    /**
+     * bitfield of already received sequence numbers.
+     * B0 := cseqn, B1 := cseqn -1, ..., B63 := cseqn - 63
+     * Initially 0, set to 1 (B0) with the first received message of the epoch,
+     * or -1 (B0..B63) with a verified ClientHello (server-side only)
+     */
     uint64_t bitfield;
 } seqnum_t;
 
diff --git a/dtls.c b/dtls.c
index 1ee8d131..19bb34e8 100644
--- a/dtls.c
+++ b/dtls.c
@@ -4249,48 +4249,55 @@ dtls_handle_message(dtls_context_t *ctx,
       dtls_warn("No security context for epoch: %i\n", epoch);
       data_length = -1;
     } else {
-      if(pkt_seq_nr == 0 && security->cseq.cseq == 0) {
+      dtls_debug("bitfield is %" PRIx64 " sequence base %" PRIx64 " rseqn %" PRIx64 "\n",
+                  security->cseq.bitfield, security->cseq.cseq, pkt_seq_nr);
+      if (security->cseq.bitfield == 0) { /* first message of epoch */
         data_length = decrypt_verify(peer, msg, rlen, &data);
-        if (data_length > 0) {
-          security->cseq.cseq = 0;
-          /* bitfield. B0 last seq seen.  B1 seq-1 seen, B2 seq-2 seen etc. */
-          security->cseq.bitfield = 1;
+        if(data_length > 0) {
+            security->cseq.cseq = pkt_seq_nr;
+            security->cseq.bitfield = 1;
+            dtls_debug("init bitfield is %" PRIx64 " sequence base %" PRIx64 "\n",
+                        security->cseq.bitfield, security->cseq.cseq);
         }
-      } else if (pkt_seq_nr == security->cseq.cseq) {
-        dtls_info("Duplicate packet arrived (cseq=%" PRIu64 ")\n", security->cseq.cseq);
-        return 0;
-      } else if ((int64_t)(security->cseq.cseq-pkt_seq_nr) > 0) { /* pkt_seq_nr < security->cseq.cseq */
-        if (((security->cseq.cseq-1)-pkt_seq_nr) < 64) {
-          if(security->cseq.bitfield & ((uint64_t)1<<((security->cseq.cseq-1)-pkt_seq_nr))) {
-            dtls_info("Duplicate packet arrived (bitfield)\n");
-            /* seen it */
+      } else {
+        int64_t seqn_diff = (int64_t)(pkt_seq_nr - security->cseq.cseq);
+        if(seqn_diff == 0) {
+          /* already seen */
+          dtls_debug("Drop: duplicate packet arrived (cseq=%" PRIu64 " bitfield's start)\n", pkt_seq_nr);
+          return 0;
+        } else if (seqn_diff < 0) { /* older pkt_seq_nr < security->cseq.cseq */
+          if (seqn_diff < -63) { /* too old */
+            dtls_debug("Drop: packet from before the bitfield arrived\n");
             return 0;
-          } else {
-            dtls_info("Packet arrived out of order\n");
-            data_length = decrypt_verify(peer, msg, rlen, &data);
-            if(data_length > 0) {
-              security->cseq.bitfield |= ((uint64_t)1<<((security->cseq.cseq-1)-pkt_seq_nr));
-            }
           }
-        } else {
-          dtls_info("Packet from before the bitfield arrived\n");
+          uint64_t seqn_bit = ((uint64_t)1 << -seqn_diff);
+          if (security->cseq.bitfield & seqn_bit) { /* seen it */
+            dtls_debug("Drop: duplicate packet arrived (bitfield)\n");
             return 0;
-        }
-      } else { /* pkt_seq_nr > security->cseq.cseq */
-        data_length = decrypt_verify(peer, msg, rlen, &data);
-        if(data_length > 0) {
-          if (pkt_seq_nr - security->cseq.cseq > 63) {
-            /* reset bitfield if new packet number is beyond its boundaries */
-            security->cseq.bitfield = 0;
-          } else {
-            /* shift bitfield */
-            security->cseq.bitfield <<= (pkt_seq_nr-security->cseq.cseq);
           }
-          security->cseq.bitfield |= 1;
-          /* bitfield. B0 last seq seen.  B1 seq-1 seen, B2 seq-2 seen etc. */
-          security->cseq.cseq = pkt_seq_nr;
-          dtls_debug("new bitfield is %" PRIx64 " sequence base %" PRIx64 "\n",
-                      security->cseq.bitfield, security->cseq.cseq);
+          dtls_debug("Packet arrived out of order\n");
+          data_length = decrypt_verify(peer, msg, rlen, &data);
+          if(data_length > 0) {
+            security->cseq.bitfield |= seqn_bit;
+            dtls_debug("update bitfield is %" PRIx64 " keep sequence base %" PRIx64 "\n",
+                        security->cseq.bitfield, security->cseq.cseq);
+          }
+        } else { /* newer pkt_seq_nr > security->cseq.cseq */
+          data_length = decrypt_verify(peer, msg, rlen, &data);
+          if(data_length > 0) {
+            security->cseq.cseq = pkt_seq_nr;
+            /* bitfield. B0 last seq seen.  B1 seq-1 seen, B2 seq-2 seen etc. */
+            if (seqn_diff > 63) {
+              /* reset bitfield if new packet number is beyond its boundaries */
+              security->cseq.bitfield = 1;
+            } else {
+              /* shift bitfield */
+              security->cseq.bitfield <<= seqn_diff;
+              security->cseq.bitfield |= 1;
+            }
+            dtls_debug("update bitfield is %" PRIx64 " new sequence base %" PRIx64 "\n",
+                        security->cseq.bitfield, security->cseq.cseq);
+          }
         }
       }
     }