Skip to content

Fix GCM multi-call Encode/Decode streaming (GHASH + CTR) and multi-chunk tests#99

Open
omonien wants to merge 3 commits into
MHumm:developmentfrom
omonien:pr-fix-gcm-multichunk
Open

Fix GCM multi-call Encode/Decode streaming (GHASH + CTR) and multi-chunk tests#99
omonien wants to merge 3 commits into
MHumm:developmentfrom
omonien:pr-fix-gcm-multichunk

Conversation

@omonien

@omonien omonien commented Jul 23, 2026

Copy link
Copy Markdown

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 / Decode recomputed GHASH over only the current buffer and overwrote the tag each call. CTR state (FY) did advance across calls, so:

  • Multi-call ciphertext could look correct
  • The authentication tag was wrong for multi-chunk / streamed use

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 TGCM only — no new public AEAD base API:

  1. Keep running GHASH state (FX), partial block buffer, total ciphertext length
  2. Absorb AAD once; pad AAD to 16-byte boundary
  3. Each Encode/Decode updates CTR + GHASH for that chunk’s ciphertext
  4. Finalize lengths + tag in TGCM.Done, called from TDECCipherModes.Done before expected-tag verify
  5. Done is idempotent; after finalization, further Encode/Decode raise until Init

This 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

File Role
Source/DECCipherModesGCM.pas Streaming GHASH/CTR + Done lifecycle
Source/DECCipherModes.pas Call FGCM.Done before tag check
Unit Tests/Tests/TestDECCipherModesGCM.pas Multi-chunk even/uneven + Done lifecycle tests
Unit Tests/Data/gcmEncryptExtIV256_large.rsp Tag corrected (CT already matched AES-GCM; old tag did not — verified with PyCryptodome)

API / behaviour note for existing callers

Callers that read the auth tag without going through Done after multi-part Encode/Decode would previously get a wrong tag; they must finalize via Done (as the mode API already intends). Single-shot usage that already called Done stays 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

  • Independent of DUnitX migration (uses shared test units)
  • Independent of Keccak test PR and SHA-3 large-buffer PR
  • Can be reviewed/merged on its own

Test plan

  • Build DEC library + unit test suite
  • Run GCM tests (multi-chunk 16+16, uneven 7+25, double Done, reject after Done)
  • Full suite if possible — after this + Keccak test fix, former shared reds from those clusters should be gone

Suggested review order for this PR

  1. TGCM state machine (Init → Encode/Decode* → Done)
  2. Where tag is finalized vs where CTR advances
  3. Test vectors (NIST set 105 constants) and the large .rsp tag correction

omonien added 3 commits July 23, 2026 13:18
…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).
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.

1 participant