-
Notifications
You must be signed in to change notification settings - Fork 5
fix(sdk): allow duplicate KAOs on same KAS within a split (DSPX-3379) #937
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
Closed
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
2b86f4c
spec: scaffold for DSPX-3379
dmihalcik-virtru fe7e42f
fix(sdk): allow duplicate KAOs on same KAS within a split
dmihalcik-virtru 66f7953
fix(sdk): dedup identical KAOs and normalize sid (DSPX-3379)
dmihalcik-virtru 62dbef3
🤖 🎨 Autoformat
dmihalcik-virtru File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,51 @@ | ||
| --- | ||
| ticket: DSPX-3379 | ||
| title: Web SDK fails if the same KAS gets more than one copy of the same split | ||
| status: draft | ||
| authors: | ||
| - dmihalcik@virtru.com | ||
| branches: | ||
| - opentdf/web-sdk:DSPX-3379-splitfix | ||
| prs: [] | ||
| created: 2026-05-28T00:00:00Z | ||
| updated: 2026-05-28T00:00:00Z | ||
| jira_priority: Medium | ||
| --- | ||
|
|
||
|
|
||
| # Web SDK fails if the same KAS gets more than one copy of the same split | ||
|
|
||
| ## Summary | ||
| We should allow the same KAS to wrap the same split. This could indicate a mistake during creation (the same key used multiple times) or an intended behavior (different keys on the same KAS indicating different security profiles of life cycles). | ||
| Behavior: | ||
| When a KAO array has two elements with the same split id and the same kas uri, it fails with the error: | ||
| TODO: Fallback to no split ids. Repetition found for [https://virtru.gbr.dev.internal] on split [71ce30e4-cd90-4269-adf7-c06db931338f]TdfError: TODO: Fallback to no split ids. Repetition found for [https://virtru.gbr.dev.internal] on split [71ce30e4-cd90-4269-adf7-c06db931338f] at splitLookupTableFactory (https://virtru.gbr.dev.internal/secureviewer/assets/index-BySOd6k_.js:29440:13) at unwrapKey (https://virtru.gbr.dev.internal/secureviewer/assets/index-BySOd6k_.js:29453:27) at decryptStreamFrom (https://virtru.gbr.dev.internal/secureviewer/assets/index-BySOd6k_.js:29691:75) at ZTDFReader.decrypt (https://virtru.gbr.dev.internal/secureviewer/assets/index-BySOd6k_.js:30874:29)Expected behavior: | ||
| The file should decrypt (or fail with permission denied) as it would if the keys were stored on distinct URIs | ||
|
|
||
| ## Problem / Motivation | ||
| Producers can legitimately emit a KAO array that has more than one entry for the same `(splitId, kasUrl)` — either as an encryption-time mistake (same key reused) or intentionally (multiple `kid`s on one KAS representing different lifecycle/profile keys). The current Web SDK refuses to decrypt these files with a `Repetition found…` error, blocking users whose files are otherwise perfectly recoverable. | ||
|
|
||
| ## Proposed Solution | ||
| Treat duplicate `(splitId, kasUrl)` entries as independent rewrap candidates rather than rejecting them. Internally, the split lookup table holds a list of `KeyAccessObject` per `(splitId, kasUrl)` instead of a single value, and `unwrapKey` tries each entry in turn; the first successful rewrap wins. Fully identical KAOs (same `kid` + `wrappedKey`) are deduplicated to avoid redundant network requests. | ||
|
|
||
| ## Inputs / Outputs / Contracts | ||
| - `splitLookupTableFactory(keyAccess, allowedKases)` return type changes from `Record<string, Record<string, KeyAccessObject>>` to `Record<string, Record<string, KeyAccessObject[]>>`. Internal API; the symbol is exported only for tests. | ||
| - `unwrapKey` behavior is observably unchanged for callers: it still returns one rewrapped key per split, or throws `UnsafeUrlError` / `PermissionDeniedError` as before. | ||
| - The `InvalidFileError("…Repetition found…")` path is removed. | ||
|
|
||
| ## Edge Cases & Constraints | ||
| - `sid` is normalized to `''` consistently in both `splitIds` and `accessibleSplits`, so a TDF mixing `undefined` and `''` sids cannot bypass the disallowed-KAS check. | ||
| - Duplicate detection keys on `(kid, wrappedKey)` — the fields that determine the KAS response. Different `kid`s on the same KAS remain distinct candidates. | ||
| - Permission-denied across all duplicates surfaces the normal aggregate error from `anyPool`, not the old `Repetition found` `InvalidFileError`. | ||
|
|
||
| ## Out of Scope | ||
| - Encryption-side dedup of KAOs. | ||
| - Changing `anyPool` concurrency semantics. | ||
| - Broader refactor of `unwrapKey`. | ||
|
|
||
| ## Acceptance Criteria | ||
| - [x] A TDF with two KAOs sharing the same `sid` and `url` decrypts successfully (or fails with the existing permission error) instead of throwing `Repetition found`. | ||
| - [x] Two KAOs with the same `(sid, url)` but different `kid`s are both attempted. | ||
| - [x] Fully identical KAOs (`kid` + `wrappedKey`) are deduplicated to one rewrap request per KAS. | ||
| - [x] Mixed `undefined` / `''` `sid` values with a disallowed required split still throw `UnsafeUrlError`. | ||
| - [x] All existing `lib` mocha + WTR tests continue to pass. |
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.