refactor(sdk): make the AEAD tag a property of the AEAD output

[dmihalcik-virtru]

Stacked on #401. Base is DSPX-4703-reject-gmac-root; review that one first.

Why

#401 closes the GMAC-root hole at runtime — an allowlist over the manifest's declared root algorithm, plus a split of one signature routine into segmentIntegrity and rootIntegrity. The split is the load-bearing part, but both halves still take a byte[], so nothing stops a future caller from handing the aggregate hash back to the segment routine and reintroducing the bug.

TDF.aeadTag(byte[]) was the concrete shape of that hazard: it recovered an AES-GCM tag by copying the trailing 16 bytes off an array, and its correctness rested on a precondition the signature could not express — that the argument was the complete output of one AES-GCM encryption. Applied to anything else it is not a MAC, it is a keyless copy of the input's own trailing bytes, forgeable by whoever supplied them.

What

segmentIntegrity now takes AesGcm.Encrypted, the type the cipher actually produces. rootIntegrity keeps byte[] aggregateHash. Passing an aggregate hash to the GMAC path no longer compiles.

aeadTag and its duplicate of GCM_TAG_LENGTH are gone. Tag extraction lives on AesGcm.Encrypted, which now holds one contiguous iv || ciphertext || tag buffer behind a >= 28 byte construction-time invariant. That invariant makes authTag() total: no instance can be too short to have a tag, so the "payload too small" runtime branch has nowhere left to live.

encryptInto is now the only code in the SDK that writes the layout, with explicit getOutputSize and bytes-written assertions so a provider that sizes output differently fails loudly rather than writing a TDF this SDK cannot read back. BCFIPS is a supported provider, so that isn't hypothetical.

Scope of the guarantee

Deliberately not overclaimed, in the javadoc as well as here. Encrypted has a public constructor taking arbitrary bytes, so it cannot prove its contents came out of a cipher — on the read path the bytes are attacker-supplied by definition, and the tag check is what catches that. What the type rules out is the API misuse of treating a value that never passed through the AEAD as though it had. The runtime allowlists from #401 stay; they cover the untrusted-manifest axis. Both are needed.

Performance

Per segment: the write path loses a segment-sized memcpy and a segment-sized allocation; the read path loses a memcpy and an allocation. decrypt reads the buffer with offsets instead of reassembling it. At the default segment size the removed allocation is probably the bigger win.

Wire format

Unchanged. HS256 hashes an identical byte range and GMAC slices an identical byte range.

Verified across builds rather than by self-comparison, since the payload key is random per TDF (so output is not byte-for-byte reproducible — I persisted the fake-KAS keypair instead and tested both directions):

• this build reads TDFs written before the change ✅
• the pre-change build reads TDFs written after it ✅
• byte sizes identical (GMAC 59461, HS256 59522) ✅

All three for both GMAC and HS256 segment algorithms.

Verification

• Full reactor compiles with zero edits to sdk-pqc-bc or sdk-fips-bc — the real test that the API change is additive, since both consume AesGcm.Encrypted.
• 274 sdk tests green (262 baseline + 12 new), 8 skipped — unchanged skip count.
• mvn verify -P coverage (CI parity) BUILD SUCCESS.

Reviewer notes

• No test was added for "segmentIntegrity rejects an aggregate hash." It no longer compiles, which is the entire point.
• getIv()/getCiphertext() now return defensive copies. Zero callers in the repo; signatures unchanged; the only observable difference is that mutating the result no longer corrupts the instance.
• Encrypted is now final, the two-arg constructor requires a 12-byte IV, and the single-arg minimum rose 12 → 28 bytes. These are the only theoretically-breaking items for out-of-tree code. Encrypted(byte[]) has always split at offset 12 unconditionally, so a 16-byte-IV instance could never have round-tripped through asBytes() — the type was implicitly 12-byte-IV-only and this makes it explicit. Happy to drop final if you'd prefer maximal caution.
• The raw-byte[] encrypt/decrypt overloads are @Deprecated but fully working. A test pins that the new and deprecated encrypt overloads emit identical bytes — that's the migration guard.
• authTag() and bytesNoCopy() are package-private on purpose. authTag() must not become public API that invites constructing an Encrypted from junk just to read 16 bytes off the end.
• Tightening to >= 28 changes the failure for malformed KAS metadata from SDKException at decrypt to IllegalArgumentException at construction. Both unchecked, and TDF.java already threw IAE for < 12, so this widens existing behavior. No real wrapped key is under 28 bytes.

Follow-up, out of scope here

AesGcm.encrypt(byte[],int,int) calls SecureRandom.getInstanceStrong() on every invocation, which can block on Linux. Worth its own issue.

[coderabbitai]

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

• Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}