Repository navigation
feat(sdk): stateful per-key AES-GCM IV counter (alternative to #4118) - #4121
Draft
dmihalcik-virtru wants to merge 14 commits into
Draft
dmihalcik-virtru wants to merge 14 commits into
dmihalcik-virtru wants to merge 14 commits into
Conversation
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
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. Comment |
github-actions
Bot
added
comp:sdk
Contributor
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
Contributor
2 of 3 tasks
dmihalcik-virtru
force-pushed
the
DSPX-4493-stateful-iv
branch
from
October 14, 2026 20:20
02a527f to
10d5844
Compare
Contributor
X-Test Failure Report |
Contributor
Benchmark results, click to expandBulk Benchmark Results
TDF3 Benchmark Results
|
Contributor
Benchmark results, click to expandBulk Benchmark Results
TDF3 Benchmark Results
|
Contributor
dmihalcik-virtru
added a commit
that referenced
this pull request
Oct 15, 2026
…r and 2b Rewrite the spec and plan around what #4121 does: a per-key counter in ocrypto.AesGcmSealer, a throwaway sealer for metadata, and one 2^32-1 cap in every FIPS mode. The #4118 design survives only as history and as a rejected option. Tighten the ADR without changing its decisions. Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
Contributor
Benchmark results, click to expandBulk Benchmark Results
TDF3 Benchmark Results
|
Contributor
Contributor
Benchmark results, click to expandBulk Benchmark Results
TDF3 Benchmark Results
|
Contributor
Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
Adds an opaque, keyed sealer that derives each part's AES-GCM IV from a per-message random field and a big-endian part number (NIST SP 800-38D 8.2.1) rather than drawing it from an RBG (8.2.2). Fusing the key and the message ID at construction makes cross-message IV reuse unconstructible. Where Go refuses caller-supplied AES-GCM IVs, or where FIPS 140-3 is enabled and a caller-supplied IV would make encryption a non-approved service, the sealer falls back to random IVs internally. The fallback is total -- same API, same wire bytes -- and surfaces only as a smaller MaxMessageParts (2^26, for a 2^-45 collision bound) which doubles as an encryption budget on that path. Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
… number Payload segment i is now sealed at message part i+1 and key access object metadata at part 0, all under one per-TDF message id, replacing the per-segment random nonce: NIST SP 800-38D 8.2.1 in place of 8.2.2. The wire format is unchanged. The IV is still the segment's 12-byte prefix and readers take it off the wire as they always have, so this is writer-side only, with no manifest or spec version change. - part = index + 1 is a pure function, so sparse, out-of-order, concurrent and retried writes all stay correct where a stateful counter would not. - One sealer registry per TDF, keyed by a copy of the key bytes, so the single-split case -- where the sole share is the DEK verbatim -- reaches one sealer for both the metadata and the payload, and part 0 stays reserved. - GetManifest and Finalize re-resolve key access on every call, so the encrypted metadata is pinned on the first build and a second value is refused with ErrChunkedMetadataChanged rather than sealed at a spent part. - maxPayloadSegments and defaultSegmentSize are derived from ocrypto.MaxMessageParts. They are the whole application-visible surface of the RBG fallback ocrypto takes where FIPS 140-3 is enabled. DSPX-4493 Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com> resolve
Fills in the DSPX-4493 spec, checks in the implementation plan beside it, and adds an ADR covering the two decisions this turns on: the IV construction itself (NIST SP 800-38D 8.2.1, fixed field || part number) and what to do where Go will not allow a caller-supplied IV. The FIPS section carries the cap arithmetic. On the RBG fallback MaxMessageParts drops to 2^26, six bits under NIST's approved 2^32 ceiling -- a 4096x margin on collision probability, P ~ 2^-45 -- which still reaches 256 TiB at the maximum segment size. Also records that the fallback and that cap are Go-specific and should not be mirrored by DSPX-4495 or DSPX-4496. Validation also records the checks that needed a build rather than a test: both modules under GOFIPS140=certified with no GODEBUG override (Go 1.27.1, certified module v1.0.0-c2097c7c), and cross-mode interop in both directions, which is the claim that the fallback changes how the header is chosen and never what it means. DSPX-4493 Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
Replace the stateless message-id || part-number construction with a sealer that owns its key, a random 64-bit fixed field, and an atomic invocation counter: IV = fixed || counter++. Callers no longer supply an ordinal, so retried, repeated and out-of-order encryptions can never reuse an IV. On the FIPS random-IV fallback the same counter is the encryption budget, so the two exhaustion errors merge into ErrSealerExhausted. MaxMessageParts becomes MaxSeals. Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
…umbers, and metadata pin With the IV counter living inside the key's sealer, the writer holds one sealer for the DEK and key access metadata is sealed by a throwaway sealer of its own. That removes the message ID, the key->sealer registry shared between CreateTDF and the writer, the metadataSealer closure threaded through key access resolution, the per-key metadata cache, the reserved metadata part and segmentPart, and ErrChunkedMetadataChanged: re-sealing metadata on every manifest build now just takes a fresh IV. Retried segments likewise take a fresh IV, so retry safety no longer depends on WriteSegment's error handling. The trade: in a single-split TDF the metadata and payload sealers share a key and count independently, kept apart only by their random 64-bit fixed fields, and an IV no longer identifies its segment index. Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com> resolve
Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
…nter Align the Go SDK with option 2b of the AES-GCM IV ADR (virtru-corp/adr#373): - ocrypto: MaxSeals is 2^32-1 on both the deterministic and the random-IV (FIPS) path, the cap #4203 already enforced on random IVs. At the cap the random path sits at P ~ 2^-33, inside SP 800-38D's 2^-32. - sdk: drop the derived default segment size; it is 2 MiB (max 4 MiB) again in every mode, and maxPayloadSegments is the archive's MaxInt32 again. - sdk: drop the writer's own encryption counter. The DEK sealer's counter is the only budget, and ocrypto.ErrSealerExhausted is reported wrapped as ErrAESGCMExhausted. Tests reach the cap through an injected sealer instead of a lowered writer cap. - adr: record the decision and the rejected 2^26 cap. Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
…r and 2b Rewrite the spec and plan around what #4121 does: a per-key counter in ocrypto.AesGcmSealer, a throwaway sealer for metadata, and one 2^32-1 cap in every FIPS mode. The #4118 design survives only as history and as a rejected option. Tighten the ADR without changing its decisions. Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
On the FIPS random-IV path Seal returned a header slice whose spare capacity ran into the ciphertext, so appending to the header overwrote it. Clip the header's capacity. Add tests the existing ones could not fail: - the production 2^32-1 ceiling on both paths, including counter 2^32, which must not truncate back to 0; - NewAESGcmSealer itself selecting random IVs under GODEBUG=fips140=on and =only, by re-running the test binary in each mode; - appending to a random-path header leaving the ciphertext intact. Cite SP 800-38D 8.2.1, not 8.3, for the counter path's bound. Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
The docs said an unmeasured stream in CreateTDF stops at the DEK sealer with ErrAESGCMExhausted. WriteSegment refuses index >= MaxInt32 first, with ErrChunkedSegmentIndexExhausted, and CreateTDF seals each index once, so only ChunkedWriter retries can exhaust the sealer. Say so on maxPayloadSegments and ErrAESGCMExhausted, and make the latter's message name the remedy rather than an IV collision limit. Re-export ErrChunkedSegmentIndexExhausted from the experimental writer as ErrSegmentIndexExhausted, list it in the writer's error conditions, and fix the stale collision-probability wording on its ErrAESGCMExhausted. Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
- spec: an unmeasured stream in CreateTDF stops at the MaxInt32 segment-index limit, not at the sealer; the acceptance item that claimed otherwise only held with an injected low-limit sealer. - spec/ADR: callers never supply an IV (Seal does return one). - spec: list ErrChunkedSegmentIndexExhausted as a public addition, scope metadata re-sealing to ChunkedWriter, and say what wraps what. - ADR: 2^-32 / 2^-45 is an 8192x margin, not 4096x; Go's counter-nonce GCM is internal, not unexported. - plan: segment sizes are the same in every FIPS mode, not fixed. Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
FIPS 140-3 mode sends every AES-GCM sealer down the random-IV path, which unit tests otherwise reach only through an injected sealer. Run the sdk and lib/ocrypto suites a second time with fips140=on so the whole writer and reader round-trip is checked on that path. fips140=only is left out: it also refuses the RSA-OAEP SHA-1 key wrap the TDF format uses, which fails unrelated tests. Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
dmihalcik-virtru
force-pushed
the
DSPX-4493-stateful-iv
branch
from
October 15, 2026 20:08
3ba61fd to
368cf5f
Compare
Contributor
|
Contributor
Benchmark results, click to expandBulk Benchmark Results
TDF3 Benchmark Results
|
Contributor
This branch has not been deployed
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
1 participant
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.
What this is
This is an experiment stacked on #4118, not a replacement. It keeps the NIST SP 800-38D §8.2.1 deterministic IV construction, but makes it stateful per sealer instead of stateless per message:
Each
ocrypto.AesGcmSealerowns its key, its fixed field and anatomic.Uint64counter.Seal(plaintext)takes the next counter value (oneAddper seal) and has no ordinal parameter.Does it simplify things?
Yes. In production code, compared with main:
Overall diff against #4118: 11 files, +586 / -1207.
Removed compared with #4118:
MessageID,NewMessageID,MessageIDFromBytes,ErrInvalidMessageIDCreateTDFand the writermetadataSealerclosure threaded throughresolve,resolvePolicyAndKeyAccessandbuildKeyAccessObjects(key_splitter.gogoes back to main)ErrChunkedMetadataChangedsegmentPart/metadataPart, and the reserved part 0WriteSegment. A retry now takes a fresh counter value by construction.ErrMessagePartsExhaustedandErrMessageInvocationsExhausted, merged intoErrSealerExhausted. The deterministic and fallback paths now count the same way.errSealerKeySizeKept from #4118:
header/zipHeadernamingencodeEncryptedMetadataWriteSegmentRebased onto #4203, and FIPS mode keeps the 2^32-1 cap (option 2b)
Rebased onto main, which now includes #4203 (
ErrAESGCMExhausted, a 2^32-1 cap on segment encryptions per DEK). The last commit brings this branch in line with option 2b of the Go supplement to the AES-GCM IV ADR (virtru-corp/adr#373, stacked on virtru-corp/adr#368):ocrypto.MaxSeals()is 2^32-1 on both paths. On the random-IV (FIPS) fallback that sits at P ≈ 2^-33 when a writer reaches the cap, inside SP 800-38D's 2^-32. This is the limit feat(sdk): return ErrAESGCMExhausted when IV collision risk exceeds 2^-32 (DSPX-4492) #4203 already enforced on every build.maxPayloadSegmentsisMaxInt32again. FIPS mode makes no difference an application can see.ocrypto.ErrSealerExhaustedreaches callers wrapped assdk.ErrAESGCMExhausted. Tests reach the cap by injecting a sealer instead of lowering the writer's cap.Trade-offs
ADR
adr/decisions/2026-09-28-deterministic-gcm-iv-construction.mdhas been rewritten on this branch. It now chooses the stateful option and lists #4118's stateless design as the rejected alternative. If we keep #4118 instead, drop the ADR commit.Testing
After the rebase and the 2b commit:
go test -racepasses forlib/ocryptoandsdk, and both also pass underGODEBUG=fips140=on.TestREADMECodeBlocksand theotdfctltests pass.golangci-lintreports no new issues.nsqzqtlqdoes not buildsdkon its own (theocryptoAPI changes one commit before the SDK catches up), as was already the case before the rebase. Thespec/DSPX-4493*.mdfiles and the ADR now describe the stateful design and option 2b.Before the rebase:
make fmtis clean.golangci-lintreports no issues in changed files; the pre-existing issues are in untouched files.go test -racepasses forlib/ocrypto,lib/flattening,lib/identifier,sdk,examplesandotdfctl.serviceunit tests pass.service/integrationandservice/entityresolution/integrationneed Postgres/Keycloak and were not run locally, and neither waslib/fixtures, which needs Keycloak.cd sdk && go test -run TestREADMECodeBlockspasses.GODEBUG=fips140=on go test ./...passes in bothsdkandlib/ocrypto, which covers the fallback path.