Skip to content

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

Open
philljj wants to merge 5 commits into
wolfSSL:masterfrom
philljj:fix_replay
Open

esp: defer esp_replay_commit to after aead decrypt, and comments.#135
philljj wants to merge 5 commits into
wolfSSL:masterfrom
philljj:fix_replay

Conversation

@philljj

@philljj philljj commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Description

Defer esp_replay_commit() until after decrypt, which is where aead icv check happens.

Add a unit test for this.

Also, add some comments.

Fixes ZD-21997.
Fixes F-4769.

@philljj philljj self-assigned this Jun 16, 2026
Copilot AI review requested due to automatic review settings June 16, 2026 15:52

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adjusts ESP transport-mode inbound processing so anti-replay state is only committed after AEAD authentication has been verified (which occurs during decrypt/verify for GCM/GMAC), aligning with RFC 4303 replay-window guidance and preventing replay-window advancement on failed AEAD verification.

Changes:

  • Move esp_replay_commit() to after the decrypt/verify step so AEAD ICV/tag verification happens before committing the sequence number.
  • Add/adjust comments describing when ICV verification occurs (HMAC vs AEAD) and add small clarifying comments around padding/debug printing.
  • Add a note documenting current per-packet cipher context initialization tradeoffs.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/wolfesp.c
Comment thread src/wolfesp.c
Comment thread src/wolfesp.c
Comment thread src/wolfesp.c

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

Comment thread src/test/unit/unit_esp.c Outdated
Comment thread src/test/unit/unit_esp.c Outdated
Comment thread src/test/unit/unit_esp.c Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Comment thread src/test/unit/unit_esp.c Outdated
@philljj philljj assigned danielinux and unassigned philljj Jun 18, 2026
@philljj philljj requested review from danielinux and gasbytes June 18, 2026 14:49
Comment thread src/test/unit/unit_esp.c Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants