update anti-replay window in the essp code after icv verification of a

packet

Author: gasbytes

src/test/unit/unit_esp.c

@@ -424,8 +424,9 @@ START_TEST(test_replay_duplicate_rejected)
 {
     replay_t r;
     esp_replay_init(r);
-    ck_assert_int_eq(esp_check_replay(&r, 5U), 0);  /* first time: ok  */
-    ck_assert_int_ne(esp_check_replay(&r, 5U), 0);  /* second time: replayed */
+    ck_assert_int_eq(esp_check_replay(&r, 5U), 0);
+    esp_replay_commit(&r, 5U);                       /* ICV passed */
+    ck_assert_int_ne(esp_check_replay(&r, 5U), 0);   /* second time: replayed */
 }
 END_TEST
 
@@ -437,6 +438,7 @@ START_TEST(test_replay_multiple_in_window)
     esp_replay_init(r); /* window [1..32] */
     for (i = 1U; i <= 31U; i++) {
         ck_assert_int_eq(esp_check_replay(&r, i), 0);
+        esp_replay_commit(&r, i);
     }
 }
 END_TEST
@@ -447,7 +449,8 @@ START_TEST(test_replay_below_window_rejected)
     replay_t r;
     esp_replay_init(r);
     /* Advance the window by receiving a high sequence number. */
-    ck_assert_int_eq(esp_check_replay(&r, 64U), 0); /* hi_seq=64, seq_low=34 */
+    ck_assert_int_eq(esp_check_replay(&r, 64U), 0);
+    esp_replay_commit(&r, 64U);                      /* hi_seq=64, seq_low=34 */
     /* seq=1 is now below the window floor. */
     ck_assert_int_ne(esp_check_replay(&r, 1U), 0);
 }
@@ -459,6 +462,7 @@ START_TEST(test_replay_advance_hi_seq)
     replay_t r;
     esp_replay_init(r); /* hi_seq=32 */
     ck_assert_int_eq(esp_check_replay(&r, 33U), 0);
+    esp_replay_commit(&r, 33U);
     ck_assert_uint_eq(r.hi_seq, 33U);
 }
 END_TEST
@@ -469,6 +473,7 @@ START_TEST(test_replay_advanced_hi_seq_duplicate_rejected)
     replay_t r;
     esp_replay_init(r); /* hi_seq=32 */
     ck_assert_int_eq(esp_check_replay(&r, 33U), 0);
+    esp_replay_commit(&r, 33U);
     ck_assert_int_ne(esp_check_replay(&r, 33U), 0);
 }
 END_TEST
@@ -491,9 +496,12 @@ START_TEST(test_replay_jump_resets_bitmap)
     esp_replay_init(r);
     /* Accept some sequences so the bitmap has bits set. */
     ck_assert_int_eq(esp_check_replay(&r, 1U), 0);
+    esp_replay_commit(&r, 1U);
     ck_assert_int_eq(esp_check_replay(&r, 2U), 0);
+    esp_replay_commit(&r, 2U);
     /* Jump more than ESP_REPLAY_WIN (32) ahead. */
     ck_assert_int_eq(esp_check_replay(&r, 1000U), 0);
+    esp_replay_commit(&r, 1000U);
     ck_assert_uint_eq(r.hi_seq, 1000U);
     /* seq=1 is now far outside the window. */
     ck_assert_int_ne(esp_check_replay(&r, 1U), 0);
@@ -507,12 +515,44 @@ START_TEST(test_replay_old_seqs_after_jump)
     replay_t r;
     esp_replay_init(r);
     ck_assert_int_eq(esp_check_replay(&r, 10U), 0);
-    ck_assert_int_eq(esp_check_replay(&r, 500U), 0); /* jump > 32 */
+    esp_replay_commit(&r, 10U);
+    ck_assert_int_eq(esp_check_replay(&r, 500U), 0);
+    esp_replay_commit(&r, 500U); /* jump > 32 */
     /* 10 is now well below the new window floor (500-31=469). */
     ck_assert_int_ne(esp_check_replay(&r, 10U), 0);
 }
 END_TEST
 
+/* RFC 4303 s3.4.3: the replay window must not be updated until after
+ * ICV verification succeeds.  esp_check_replay must be read-only;
+ * esp_replay_commit updates the window after ICV passes. */
+START_TEST(test_regression_replay_window_not_updated_before_icv)
+{
+    replay_t r;
+    replay_t saved;
+
+    esp_replay_init(r);
+
+    /* Accept a few packets to establish window state */
+    ck_assert_int_eq(esp_check_replay(&r, 1U), 0);
+    esp_replay_commit(&r, 1U);
+    ck_assert_int_eq(esp_check_replay(&r, 2U), 0);
+    esp_replay_commit(&r, 2U);
+
+    /* Save the replay state before the "unverified" packet arrives */
+    memcpy(&saved, &r, sizeof(r));
+
+    /* Simulate receiving seq=10. This should only CHECK, not UPDATE.
+     * In the real flow, ICV verification would follow and might fail. */
+    ck_assert_int_eq(esp_check_replay(&r, 10U), 0);
+
+    /* esp_check_replay is now read-only (correct behavior), so the
+     * replay state must be unchanged. */
+    ck_assert_uint_eq(r.bitmap, saved.bitmap);
+    ck_assert_uint_eq(r.hi_seq, saved.hi_seq);
+}
+END_TEST
+
 /* The transmitted sequence number must never be allowed to overflow. */
 START_TEST(test_replay_overflow)
 {
@@ -1273,6 +1313,7 @@ static Suite *esp_suite(void)
     tcase_add_test(tc, test_replay_jump_resets_bitmap);
     tcase_add_test(tc, test_replay_old_seqs_after_jump);
     tcase_add_test(tc, test_replay_overflow);
+    tcase_add_test(tc, test_regression_replay_window_not_updated_before_icv);
     suite_add_tcase(s, tc);
 
     /* Unwrap error paths */

src/wolfesp.c

@@ -1159,19 +1159,20 @@ esp_check_icv_hmac(const wolfIP_esp_sa * esp_sa, uint8_t * esp_data,
 }
 
 /**
- * Check sequence number against replay_t state.
+ * Check sequence number against replay_t state (read-only).
+ * RFC 4303 s3.4.3: the window must not be updated until after ICV
+ * verification succeeds.  Call esp_replay_commit() after ICV passes.
  *
  * return 0 on success.
  * */
 static int
-esp_check_replay(struct replay_t * replay, uint32_t seq)
+esp_check_replay(const struct replay_t * replay, uint32_t seq)
 {
     #if !defined(ESP_REPLAY_WIN)
     /* anti-replay service not enabled */
     (void)replay;
     (void)seq;
     #else
-    uint32_t diff = 0;
     uint32_t bitn = 0;
     uint32_t seq_low = 1U;
 
@@ -1201,29 +1202,41 @@ esp_check_replay(struct replay_t * replay, uint32_t seq)
             ESP_LOG("error: seq replayed: %u, %d\n", bitn, seq);
             return -1;
         }
-        else {
-            ESP_DEBUG("info: new seq : %d\n", seq);
-            replay->bitmap |= bitn;
-        }
+    }
+    #endif /* ESP_REPLAY_WIN */
+
+    return 0;
+}
+
+/**
+ * Commit a verified sequence number to the replay window.
+ * Called only after ICV verification succeeds (RFC 4303 s3.4.3).
+ * */
+static void
+esp_replay_commit(struct replay_t * replay, uint32_t seq)
+{
+    #if !defined(ESP_REPLAY_WIN)
+    (void)replay;
+    (void)seq;
+    #else
+    uint32_t diff;
+
+    if (seq <= replay->hi_seq) {
+        /* Within window: mark the bit. */
+        replay->bitmap |= 1U << (replay->hi_seq - seq);
     }
     else {
-        /* seq number above window. */
-        ESP_DEBUG("info: new hi_seq : %d, %d\n", replay->hi_seq, seq);
+        /* Above window: slide up. */
         diff = seq - replay->hi_seq;
         if (diff < ESP_REPLAY_WIN) {
-            /* within a window width, slide up. */
             replay->bitmap = (replay->bitmap << diff) | 1U;
         }
         else {
-            /* reset window. */
             replay->bitmap = 1;
         }
-
         replay->hi_seq = seq;
     }
     #endif /* ESP_REPLAY_WIN */
-
-    return 0;
 }
 
 /**
@@ -1349,6 +1362,10 @@ esp_transport_unwrap(struct wolfIP_ip_packet *ip, uint32_t * frame_len)
         }
     }
 
+    /* ICV verified; now safe to commit the sequence to the replay window
+     * (RFC 4303 s3.4.3). */
+    esp_replay_commit(&esp_sa->replay, seq);
+
     /* icv check good, now finish unwrapping esp packet. */
     if (iv_len != 0) {
         /* Decrypt the payload in place. */