From c7f2339d83fa45b3aadf795739c98ac8d0051356 Mon Sep 17 00:00:00 2001 From: jordan Date: Tue, 16 Jun 2026 10:46:15 -0500 Subject: [PATCH 1/5] esp: defer esp_replay_commit to after aead decrypt, and comments. --- src/wolfesp.c | 31 ++++++++++++++++++++++++------- 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/src/wolfesp.c b/src/wolfesp.c index 61226504..e69e1512 100644 --- a/src/wolfesp.c +++ b/src/wolfesp.c @@ -679,6 +679,19 @@ esp_const_memcmp(const uint8_t * vec_a, const uint8_t * vec_b, uint32_t len) #define esp_enc_payload(data, iv_len) \ ((data) + ESP_SPI_LEN + ESP_SEQ_LEN + (iv_len)) +/** + * note: encryption is done with these 8 functions: + * - esp_des3_rfc2451_enc (_dec) + * - esp_aes_rfc3602_enc (_dec) + * - esp_aes_rfc4106_enc (_dec) + * - esp_aes_rfc4543_enc (_dec) + * + * They use local Aes, Des3, Gmac structs, and do Init/SetKey on every crypt + * operation. It would be much more performant to move these structs into the + * esp_sa, and do Init/SetKey only once at SA creation. However that would use + * much more memory. + * */ + static int esp_aes_rfc3602_dec(const wolfIP_esp_sa * esp_sa, uint8_t * esp_data, uint32_t esp_len) @@ -1428,16 +1441,17 @@ esp_transport_unwrap(struct wolfIP_ip_packet *ip, uint32_t * frame_len) if (esp_sa->icv_len) { switch (esp_sa->auth) { + /* for hmac auths, icv calculated immediately */ case ESP_AUTH_MD5_RFC2403: case ESP_AUTH_SHA1_RFC2404: case ESP_AUTH_SHA256_RFC4868: err = esp_check_icv_hmac(esp_sa, ip->data, esp_len); break; + /* for aeads, icv calculated later during decrypt */ #if defined(WOLFSSL_AESGCM_STREAM) case ESP_AUTH_GCM_RFC4106: #endif /* WOLFSSL_AESGCM_STREAM */ case ESP_AUTH_GCM_RFC4543: - /* icv calculated during decrypt */ err = 0; break; case ESP_AUTH_NONE: @@ -1453,11 +1467,8 @@ 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. */ + /* hmac icv check good, now finish unwrapping esp packet. + * for aeads, the icv check happens in decrypt now. */ if (iv_len != 0) { /* Decrypt the payload in place. */ switch(esp_sa->enc) { @@ -1495,7 +1506,11 @@ esp_transport_unwrap(struct wolfIP_ip_packet *ip, uint32_t * frame_len) } } - /* Payload is now decrypted. We can now parse + /* icv verified for hmacs and aeads at this point. now safe to commit the + * sequence to the replay window (RFC 4303 s3.4.3). */ + esp_replay_commit(&esp_sa->replay, seq); + + /* Payload is now verified and decrypted. We can now parse * the ESP trailer for next header and pad_len. */ pad_len = *(ip->data + esp_len - esp_sa->icv_len - ESP_NEXT_HEADER_LEN - ESP_PADDING_LEN); @@ -1513,6 +1528,7 @@ esp_transport_unwrap(struct wolfIP_ip_packet *ip, uint32_t * frame_len) return -1; } } + /* verify padding is correct */ if (pad_len > 0) { const uint8_t *padding = ip->data + esp_len - esp_sa->icv_len - ESP_NEXT_HEADER_LEN - ESP_PADDING_LEN @@ -1529,6 +1545,7 @@ esp_transport_unwrap(struct wolfIP_ip_packet *ip, uint32_t * frame_len) } #ifdef DEBUG_ESP + /* optionally debug print the ESP packet, now that it's readable. */ wolfIP_print_esp(esp_sa, ip->data, esp_len, pad_len, nxt_hdr); #endif /* DEBUG_ESP */ From 15ffeba0a5ff127ed61e3575b6066f1ff8e57bbe Mon Sep 17 00:00:00 2001 From: jordan Date: Wed, 17 Jun 2026 14:33:50 -0500 Subject: [PATCH 2/5] esp: cleanup, rename esp_check_replay to esp_replay_check. --- src/test/unit/unit_esp.c | 48 ++++++++++++++++++++-------------------- src/wolfesp.c | 4 ++-- 2 files changed, 26 insertions(+), 26 deletions(-) diff --git a/src/test/unit/unit_esp.c b/src/test/unit/unit_esp.c index 1e09c7d8..109738be 100644 --- a/src/test/unit/unit_esp.c +++ b/src/test/unit/unit_esp.c @@ -464,7 +464,7 @@ START_TEST(test_sa_del_all) END_TEST /* - * replay window (esp_check_replay) + * replay window (esp_replay_check) * valid initial window: [1 .. 32] (seq=0 is always invalid per RFC 4303) * */ @@ -473,7 +473,7 @@ START_TEST(test_replay_seq_zero_rejected) { replay_t r; esp_replay_init(r); - ck_assert_int_ne(esp_check_replay(&r, 0U), 0); + ck_assert_int_ne(esp_replay_check(&r, 0U), 0); } END_TEST @@ -482,7 +482,7 @@ START_TEST(test_replay_first_packet_accepted) { replay_t r; esp_replay_init(r); /* hi_seq=32, seq_low=1 */ - ck_assert_int_eq(esp_check_replay(&r, 1U), 0); + ck_assert_int_eq(esp_replay_check(&r, 1U), 0); } END_TEST @@ -492,9 +492,9 @@ START_TEST(test_replay_duplicate_rejected) { replay_t r; esp_replay_init(r); - ck_assert_int_eq(esp_check_replay(&r, 5U), 0); + ck_assert_int_eq(esp_replay_check(&r, 5U), 0); esp_replay_commit(&r, 5U); /* ICV passed */ - ck_assert_int_ne(esp_check_replay(&r, 5U), 0); /* second time: replayed */ + ck_assert_int_ne(esp_replay_check(&r, 5U), 0); /* second time: replayed */ } END_TEST @@ -505,7 +505,7 @@ START_TEST(test_replay_multiple_in_window) uint32_t i; esp_replay_init(r); /* window [1..32] */ for (i = 1U; i <= 31U; i++) { - ck_assert_int_eq(esp_check_replay(&r, i), 0); + ck_assert_int_eq(esp_replay_check(&r, i), 0); esp_replay_commit(&r, i); } } @@ -517,10 +517,10 @@ 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); + ck_assert_int_eq(esp_replay_check(&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); + ck_assert_int_ne(esp_replay_check(&r, 1U), 0); } END_TEST @@ -529,7 +529,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); + ck_assert_int_eq(esp_replay_check(&r, 33U), 0); esp_replay_commit(&r, 33U); ck_assert_uint_eq(r.hi_seq, 33U); } @@ -540,9 +540,9 @@ 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); + ck_assert_int_eq(esp_replay_check(&r, 33U), 0); esp_replay_commit(&r, 33U); - ck_assert_int_ne(esp_check_replay(&r, 33U), 0); + ck_assert_int_ne(esp_replay_check(&r, 33U), 0); } END_TEST @@ -553,7 +553,7 @@ START_TEST(test_replay_low_hi_seq_accepts_seq_one) esp_replay_init(r); r.hi_seq = 1U; r.bitmap = 0U; - ck_assert_int_eq(esp_check_replay(&r, 1U), 0); + ck_assert_int_eq(esp_replay_check(&r, 1U), 0); } END_TEST @@ -563,16 +563,16 @@ START_TEST(test_replay_jump_resets_bitmap) replay_t r; esp_replay_init(r); /* Accept some sequences so the bitmap has bits set. */ - ck_assert_int_eq(esp_check_replay(&r, 1U), 0); + ck_assert_int_eq(esp_replay_check(&r, 1U), 0); esp_replay_commit(&r, 1U); - ck_assert_int_eq(esp_check_replay(&r, 2U), 0); + ck_assert_int_eq(esp_replay_check(&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); + ck_assert_int_eq(esp_replay_check(&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); + ck_assert_int_ne(esp_replay_check(&r, 1U), 0); } END_TEST @@ -582,17 +582,17 @@ 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_replay_check(&r, 10U), 0); esp_replay_commit(&r, 10U); - ck_assert_int_eq(esp_check_replay(&r, 500U), 0); + ck_assert_int_eq(esp_replay_check(&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); + ck_assert_int_ne(esp_replay_check(&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; + * ICV verification succeeds. esp_replay_check must be read-only; * esp_replay_commit updates the window after ICV passes. */ START_TEST(test_regression_replay_window_not_updated_before_icv) { @@ -602,9 +602,9 @@ START_TEST(test_regression_replay_window_not_updated_before_icv) esp_replay_init(r); /* Accept a few packets to establish window state */ - ck_assert_int_eq(esp_check_replay(&r, 1U), 0); + ck_assert_int_eq(esp_replay_check(&r, 1U), 0); esp_replay_commit(&r, 1U); - ck_assert_int_eq(esp_check_replay(&r, 2U), 0); + ck_assert_int_eq(esp_replay_check(&r, 2U), 0); esp_replay_commit(&r, 2U); /* Save the replay state before the "unverified" packet arrives */ @@ -612,9 +612,9 @@ START_TEST(test_regression_replay_window_not_updated_before_icv) /* 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); + ck_assert_int_eq(esp_replay_check(&r, 10U), 0); - /* esp_check_replay is now read-only (correct behavior), so the + /* esp_replay_check 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); diff --git a/src/wolfesp.c b/src/wolfesp.c index e69e1512..7b660851 100644 --- a/src/wolfesp.c +++ b/src/wolfesp.c @@ -1240,7 +1240,7 @@ esp_check_icv_hmac(const wolfIP_esp_sa * esp_sa, uint8_t * esp_data, * return 0 on success. * */ static int -esp_check_replay(const struct replay_t * replay, uint32_t seq) +esp_replay_check(const struct replay_t * replay, uint32_t seq) { #if !defined(ESP_REPLAY_WIN) /* anti-replay service not enabled */ @@ -1419,7 +1419,7 @@ esp_transport_unwrap(struct wolfIP_ip_packet *ip, uint32_t * frame_len) return -1; } - err = esp_check_replay(&esp_sa->replay, seq); + err = esp_replay_check(&esp_sa->replay, seq); if (err) { return -1; } From 63effb2cf671fcc53d4086bb6605e83ffe883970 Mon Sep 17 00:00:00 2001 From: jordan Date: Wed, 17 Jun 2026 17:19:50 -0500 Subject: [PATCH 3/5] esp: add test_replay_aead_verify unit test, and cleanup log msg. --- src/test/unit/unit_esp.c | 47 ++++++++++++++++++++++++++++++++++++++++ src/wolfesp.c | 5 ++--- 2 files changed, 49 insertions(+), 3 deletions(-) diff --git a/src/test/unit/unit_esp.c b/src/test/unit/unit_esp.c index 109738be..64b28746 100644 --- a/src/test/unit/unit_esp.c +++ b/src/test/unit/unit_esp.c @@ -670,6 +670,52 @@ START_TEST(test_replay_overflow) } END_TEST +/* The sequence number must only be committed after aead verify. + * Send plausible junk ESP packets for a real RFC4543 SA, and check the + * inbound replay state is not mutated after verify failure. */ +START_TEST(test_replay_aead_verify) +{ + static uint8_t buf[LINK_MTU + 256]; + uint8_t ref[64]; + uint32_t frame_len, i; + int ret; + wolfIP_esp_sa * esp_sa = NULL; + struct wolfIP_ip_packet *ip = (struct wolfIP_ip_packet *)buf; + + for (i = 0U; i < sizeof(ref); i++) ref[i] = (uint8_t)(i & 0xaaU); + + /* fake ESP packet with enough data to get through early processing, but + * eventually fail during GMAC verify. */ + memcpy(ref, spi_rt, sizeof(spi_rt)); + /* max sequence number */ + ref[4] = 0xff; ref[5] = 0xff; ref[6] = 0xff; ref[7] = 0xff; + + esp_setup(); + + ret = wolfIP_esp_sa_new_gcm(1, (uint8_t *)spi_rt, + atoip4(T_SRC), atoip4(T_DST), + ESP_ENC_GCM_RFC4543, + (uint8_t *)k_aes256_gcm, + sizeof(k_aes256_gcm)); + ck_assert_int_eq(ret, 0); + esp_sa = esp_sa_get(1, (uint8_t *)spi_rt); + ck_assert_ptr_nonnull(esp_sa); + /* sanity check esp_replay_init */ + ck_assert_int_eq(esp_sa->replay.hi_seq, ESP_REPLAY_WIN); + ck_assert_int_eq(esp_sa->replay.bitmap, 0U); + + /* should fail to unwrap. */ + frame_len = build_ip_packet(buf, sizeof(buf), WI_IPPROTO_UDP, + ref, sizeof(ref)); + ret = esp_transport_unwrap(ip, &frame_len); + ck_assert_int_eq(ret, -1); + + /* the hi seq and bitmap should be unchanged. */ + ck_assert_int_eq(esp_sa->replay.hi_seq, ESP_REPLAY_WIN); + ck_assert_int_eq(esp_sa->replay.bitmap, 0U); +} +END_TEST + /* * esp_transport_unwrap error paths */ @@ -2053,6 +2099,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_replay_aead_verify); tcase_add_test(tc, test_regression_replay_window_not_updated_before_icv); suite_add_tcase(s, tc); diff --git a/src/wolfesp.c b/src/wolfesp.c index 7b660851..c95e70e8 100644 --- a/src/wolfesp.c +++ b/src/wolfesp.c @@ -1155,7 +1155,7 @@ esp_aes_rfc4543_enc(const wolfIP_esp_sa * esp_sa, uint8_t * esp_data, uint8_t salt_len = ESP_GCM_RFC4106_SALT_LEN; uint8_t nonce[ESP_GCM_RFC4106_NONCE_LEN]; /* 4 salt + 8 iv */ - ESP_DEBUG("info: aes gcm enc: %d\n", esp_len); + ESP_DEBUG("info: aes gcm rfc4543 enc: %d\n", esp_len); /* get enc payload, iv, and icv pointers. */ iv = esp_enc_iv(esp_data); @@ -1500,8 +1500,7 @@ esp_transport_unwrap(struct wolfIP_ip_packet *ip, uint32_t * frame_len) } if (err) { - ESP_LOG("error: esp_decrypt(%02x): %d\n", esp_sa->enc, - err); + ESP_LOG("error: esp_decrypt(%02x): %d\n", esp_sa->enc, err); return -1; } } From 70f4ebfa2ed6c23bbf70afd5f9b79a3999346f40 Mon Sep 17 00:00:00 2001 From: jordan Date: Wed, 17 Jun 2026 22:10:13 -0500 Subject: [PATCH 4/5] esp: small unit test cleanup. --- src/test/unit/unit_esp.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/test/unit/unit_esp.c b/src/test/unit/unit_esp.c index 64b28746..10922338 100644 --- a/src/test/unit/unit_esp.c +++ b/src/test/unit/unit_esp.c @@ -701,18 +701,17 @@ START_TEST(test_replay_aead_verify) esp_sa = esp_sa_get(1, (uint8_t *)spi_rt); ck_assert_ptr_nonnull(esp_sa); /* sanity check esp_replay_init */ - ck_assert_int_eq(esp_sa->replay.hi_seq, ESP_REPLAY_WIN); - ck_assert_int_eq(esp_sa->replay.bitmap, 0U); + ck_assert_uint_eq(esp_sa->replay.hi_seq, ESP_REPLAY_WIN); + ck_assert_uint_eq(esp_sa->replay.bitmap, 0U); /* should fail to unwrap. */ - frame_len = build_ip_packet(buf, sizeof(buf), WI_IPPROTO_UDP, - ref, sizeof(ref)); + frame_len = build_ip_packet(buf, sizeof(buf), 0x32, ref, sizeof(ref)); ret = esp_transport_unwrap(ip, &frame_len); ck_assert_int_eq(ret, -1); /* the hi seq and bitmap should be unchanged. */ - ck_assert_int_eq(esp_sa->replay.hi_seq, ESP_REPLAY_WIN); - ck_assert_int_eq(esp_sa->replay.bitmap, 0U); + ck_assert_uint_eq(esp_sa->replay.hi_seq, ESP_REPLAY_WIN); + ck_assert_uint_eq(esp_sa->replay.bitmap, 0U); } END_TEST From 60cb6bcc7cd3ecb643017e582a5c1935b39fab51 Mon Sep 17 00:00:00 2001 From: jordan Date: Thu, 18 Jun 2026 10:51:14 -0500 Subject: [PATCH 5/5] esp: tiny unit_esp review cleanup. --- src/test/unit/unit_esp.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/test/unit/unit_esp.c b/src/test/unit/unit_esp.c index 10922338..9b00868a 100644 --- a/src/test/unit/unit_esp.c +++ b/src/test/unit/unit_esp.c @@ -671,7 +671,7 @@ START_TEST(test_replay_overflow) END_TEST /* The sequence number must only be committed after aead verify. - * Send plausible junk ESP packets for a real RFC4543 SA, and check the + * Send a plausible junk ESP packet for a real RFC4543 SA, and check the * inbound replay state is not mutated after verify failure. */ START_TEST(test_replay_aead_verify) { @@ -682,7 +682,7 @@ START_TEST(test_replay_aead_verify) wolfIP_esp_sa * esp_sa = NULL; struct wolfIP_ip_packet *ip = (struct wolfIP_ip_packet *)buf; - for (i = 0U; i < sizeof(ref); i++) ref[i] = (uint8_t)(i & 0xaaU); + for (i = 0U; i < sizeof(ref); i++) ref[i] = (uint8_t)(i & 0xffU); /* fake ESP packet with enough data to get through early processing, but * eventually fail during GMAC verify. */ @@ -705,7 +705,7 @@ START_TEST(test_replay_aead_verify) ck_assert_uint_eq(esp_sa->replay.bitmap, 0U); /* should fail to unwrap. */ - frame_len = build_ip_packet(buf, sizeof(buf), 0x32, ref, sizeof(ref)); + frame_len = build_ip_packet(buf, sizeof(buf), 0x32U, ref, sizeof(ref)); ret = esp_transport_unwrap(ip, &frame_len); ck_assert_int_eq(ret, -1);