Repository navigation
fix(sdk): use a distinct IV for each key access object's metadata (DSPX-4492) - #1085
Open
dmihalcik-virtru wants to merge 6 commits into
Open
dmihalcik-virtru wants to merge 6 commits into
dmihalcik-virtru wants to merge 6 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 44.44% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 7 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly and concisely describes the primary change: using a distinct IV for each key access object's metadata. It matches the pull request objectives and changeset. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches: 💡 2
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
⚔️ Resolve merge conflicts: 💡
-
- Resolve merge conflict in branch
DSPX-4492-kao-metadata-iv
- Resolve merge conflict in branch
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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.
A rabbit checks each IV twice,
Then counts each segment, neat and precise.
Fresh keys of access get fresh starts,
The stream stops short when budget parts.
The burrow hums; no nonce repeats.
Comment @coderabbitai help to get the list of available commands.
dmihalcik-virtru
force-pushed
the
DSPX-4492-kao-metadata-iv
branch
from
October 14, 2026 14:18
cec178b to
cdd1c4c
Compare
dmihalcik-virtru
changed the title
X-Test Failure Reportcoverage-lib |
dmihalcik-virtru
force-pushed
the
DSPX-4492-kao-metadata-iv
branch
from
October 14, 2026 15:18
9f74378 to
b8e1e1c
Compare
X-Test Failure Reportcoverage-web-app-browser |
dmihalcik-virtru
force-pushed
the
DSPX-4492-exhausted
branch
from
October 16, 2026 16:03
74a5078 to
628c931
Compare
…32 (DSPX-4492) The TDF writer encrypts each payload segment with AES-GCM under one DEK using a random 96-bit IV. By the birthday bound, k invocations collide with probability about k^2 / 2^97; NIST SP 800-38D section 8 caps this at 2^-32. Add MAX_PAYLOAD_SEGMENTS_PER_KEY = 2^32 - 1 (invocation 0 is reserved for key access metadata) and have writeStream count every segment encryption attempt, throwing the new public IvExhaustionError before encrypting the segment that would exceed the cap. This applies to known- and unknown-length streams alike, since both share the same segment path. Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
…Stream wiring (DSPX-4492) - Rewrite the MAX_PAYLOAD_SEGMENTS_PER_KEY and IvExhaustionError docs around the §8.3 2^32-invocation cap, which is the binding limit, rather than a derived ~2^32.5 birthday bound; note the budget is per writeStream call. - Replace the log2 test with MAX + 1 <= 2^32. - Add an @internal maxPayloadSegments test seam, checked up front in writeStream so it can lower the cap but never raise or disable it, and writeStream tests that refuse the segment past the budget (full and short final segments) without encrypting it. Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
… docs (DSPX-4492) Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
dmihalcik-virtru
force-pushed
the
DSPX-4492-exhausted
branch
from
October 23, 2026 00:19
70db8f0 to
89ae6bf
Compare
…PX-4492) Hardening. SplitKey.getKeyAccessObjects encrypted every KAO's metadata with the one fixed IV from generateKey(). KAOs that share a split share its key (with a single split, the payload key itself). Client.encrypt gives every KAO the same metadata, so this only ever produced identical ciphertexts and leaked nothing; but per-KAO metadata, possible when building SplitKey.keyAccess directly, would reuse an AES-GCM nonce. Draw a fresh random IV per metadata encryption. Readers are unaffected: the IV was already written both as the ciphertext's 12-byte prefix and in the encryptedMetadata iv field, and both now carry the new IV. Also correct the segment-cap docs: metadata takes one invocation per KAO sharing the data key, not exactly one. Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
…et (DSPX-4492) With a single key split, every KAO's metadata is encrypted under the payload key itself, one AES-GCM invocation each. The writer assumed exactly one, so N KAOs let a write reach (2^32 - 1) + N invocations, over the SP 800-38D §8.3 cap of 2^32. Count invocations rather than payload segments: add MAX_INVOCATIONS_PER_KEY = 2^32 (MAX_PAYLOAD_SEGMENTS_PER_KEY stays as the single-KAO case, 2^32 - 1) and start writeStream's counter at SplitKey.payloadKeyMetadataInvocations(): one per KAO with a single split, none with several, since split shares are independent keys. This is the pre-increment a future §8.2.1 counter would need too. Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
dmihalcik-virtru
force-pushed
the
DSPX-4492-kao-metadata-iv
branch
from
October 23, 2026 00:23
b8e1e1c to
1a7fdd5
Compare
|
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
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.
Part of DSPX-4492. Stacked on #1077.
What
Hardening, not a fix for an exploitable bug.
SplitKey.getKeyAccessObjects(lib/tdf3/src/models/encryption-information.ts) used to encrypt every key access object's (KAO's) metadata with one fixed IV,keyInfo.unwrappedKeyIvBinary. That IV comes fromgenerateKey(). It now draws a fresh random IV for each metadata encryption.Why
KAOs that share a split ID share that split's key, for example one split wrapped for several KASes. With a single split, that key is the payload key itself. KAOs on different splits use different keys, so the shared IV never mattered for them.
Today's risk is low.
Client.encryptgives every KAO the samemetadata. KAOs that share a split therefore encrypt the same plaintext under the same key and IV, which produces identical ciphertext. That reveals only that the metadata matches, which is already evident. Nothing the SDK writes is affected.The latent risk: code that builds
SplitKey.keyAccessdirectly with different metadata per KAO would reuse an AES-GCM nonce for different plaintexts. That leaks the XOR of the plaintexts and the GHASH key. A distinct IV per KAO removes that trap. It also makes the DSPX-4492 invocation accounting accurate: each metadata encryption is its own random-IV invocation.Invocation accounting
With a single split, every KAO's metadata is one AES-GCM invocation under the payload key. The writer from #1077 assumed exactly one. With N KAOs, a write could therefore reach (2^32 - 1) + N invocations, which is over the SP 800-38D §8.3 cap of 2^32.
The second commit counts invocations instead of payload segments:
MAX_INVOCATIONS_PER_KEY = 2^32.MAX_PAYLOAD_SEGMENTS_PER_KEYstays as2^32 - 1, the single-KAO case, to keep the names the draft stack expects.SplitKey.payloadKeyMetadataInvocations(). It returns one per KAO when there is a single split, and zero when there are several, because each split's share is then a different, uniformly random key rather than the payload key.writeStreamstarts its counter at that value. This is the same pre-increment a future §8.2.1 counter would need, with metadata taking invocations 0 through N-1.maxPayloadSegments→maxInvocationsPerKey.Compatibility
Readers need no change. The IV was already written in two places: as the ciphertext's 12-byte prefix, and in the
ivfield ofencryptedMetadata. Both now carry the new per-KAO IV, so readers that use either one keep working.KeyInfo.unwrappedKeyIvBinaryand the manifest'smethod.ivare left as they are.How to test
cd lib && npm testtests/mocha/unit/encryption-information.spec.tsivfield.payloadKeyMetadataInvocationsreturns the right count for single and multiple splits.tests/mocha/unit/tdf.spec.tsThese tests fail if the fixed IV is put back, or if the counter always starts at 1.
Risk
Low. This touches crypto: KAO metadata is now encrypted with different IVs, but the format is unchanged and readers already take the IV from the ciphertext or from the
ivfield.Also fixes DSPX-5238: a custom encrypt
keyMiddlewarecould reuse a DEK and metadata IV across objects (AES-GCM nonce reuse). It was found independently in the DSPX-5124 analysis (virtru-corp/adr#377).Summary by CodeRabbit