Skip to content

esp: defer esp_replay_commit to after aead decrypt, and comments. #135

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
philljj wants to merge 5 commits into
base: master
Choose a base branch
from
Open

esp: defer esp_replay_commit to after aead decrypt, and comments. #135

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c7f2339
esp: defer esp_replay_commit to after aead decrypt, and comments.
philljj Jun 16, 2026
15ffeba
esp: cleanup, rename esp_check_replay to esp_replay_check.
philljj Jun 17, 2026
63effb2
esp: add test_replay_aead_verify unit test, and cleanup log msg.
philljj Jun 17, 2026
70f4ebf
esp: small unit test cleanup.
philljj Jun 18, 2026
60cb6bc
esp: tiny unit_esp review cleanup.
philljj Jun 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
94 changes: 70 additions & 24 deletions src/test/unit/unit_esp.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
* */

Expand All @@ -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

Expand All @@ -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

Expand All @@ -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

Expand All @@ -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);
}
}
Expand All @@ -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

Expand All @@ -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);
}
Expand All @@ -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

Expand All @@ -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

Expand All @@ -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

Expand All @@ -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)
{
Expand All @@ -602,19 +602,19 @@ 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 */
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);
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);
Expand Down Expand Up @@ -670,6 +670,51 @@ START_TEST(test_replay_overflow)
}
END_TEST

/* The sequence number must only be committed after aead verify.
* 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)
{
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 & 0xffU);

/* 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_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), 0x32U, 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_uint_eq(esp_sa->replay.hi_seq, ESP_REPLAY_WIN);
ck_assert_uint_eq(esp_sa->replay.bitmap, 0U);
}
END_TEST

/*
* esp_transport_unwrap error paths
*/
Expand Down Expand Up @@ -2053,6 +2098,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);

Expand Down
40 changes: 28 additions & 12 deletions src/wolfesp.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -1142,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);
Expand Down Expand Up @@ -1227,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 */
Expand Down Expand Up @@ -1406,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;
}
Expand All @@ -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:
Expand All @@ -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) {
Expand Down Expand Up @@ -1489,13 +1500,16 @@ 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;
}
}

/* 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);
Comment thread
philljj marked this conversation as resolved.

/* 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);
Expand All @@ -1513,6 +1527,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
Expand All @@ -1529,6 +1544,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 */

Expand Down
Loading