Fix GCM multi-call Encode/Decode streaming (GHASH + CTR) and multi-chunk tests#99
Open
omonien wants to merge 3 commits into
Open
Fix GCM multi-call Encode/Decode streaming (GHASH + CTR) and multi-chunk tests#99omonien wants to merge 3 commits into
omonien wants to merge 3 commits into
Conversation
…Accumulate GHASH and partial CTR keystream across Encode/Decode calls; finalize the authentication tag in TGCM.Done (wired from TDECCipherModes). Add multi-chunk stream tests (2x16 and uneven 7+25). Correct the large CAVS fixture tag to match AES-GCM (verified with PyCryptodome). Document Option A vs later AEAD package in Cleanup-Roadmap.
…inalized so repeated Done leaves the tag unchanged and Encode/Decode after finalization raise until Init. Document the contract in Option A and add lifecycle regression tests (double Done, reject after Done, uneven multi-chunk decrypt).
Deduplicate the NIST set-105 key/IV/PT/CT/tag literals used by the six multi-chunk and Done-lifecycle tests (CodeRabbit DRY nitpick).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
For Markus — theme: product bugfix (GCM multi-chunk / Option A)
This is a real GCM correctness fix, not a test-only tweak.
Problem (short)
TGCM.Encode/Decoderecomputed GHASH over only the current buffer and overwrote the tag each call. CTR state (FY) did advance across calls, so:CAVS set 105 (first 2-block case) is the smallest case that exposes this; single-chunk vectors still passed.
Strategy: Option A (local, reviewable)
Fix inside
TGCMonly — no new public AEAD base API:FX), partial block buffer, total ciphertext lengthEncode/Decodeupdates CTR + GHASH for that chunk’s ciphertextTGCM.Done, called fromTDECCipherModes.Donebefore expected-tag verifyDoneis idempotent; after finalization, furtherEncode/Decoderaise untilInitThis is intentionally the GCM-specific core a later AEAD architecture package (split from PR #90) should absorb, not throw away. We are not inventing the final public InitAuth/Update/Finalize API here.
Files
Source/DECCipherModesGCM.pasSource/DECCipherModes.pasFGCM.Donebefore tag checkUnit Tests/Tests/TestDECCipherModesGCM.pasUnit Tests/Data/gcmEncryptExtIV256_large.rspAPI / behaviour note for existing callers
Callers that read the auth tag without going through
Doneafter multi-partEncode/Decodewould previously get a wrong tag; they must finalize viaDone(as the mode API already intends). Single-shot usage that already calledDonestays the normal path.Out of scope
Risk
Higher than the other cleanup PRs — touches core GCM auth. Mitigations: CAVS multi-chunk tests, Done lifecycle tests, large fixture tag fix verified externally.
Independence
Test plan
Suggested review order for this PR
TGCMstate machine (Init → Encode/Decode* → Done).rsptag correction