diff --git a/lib/tdf3/src/tdf.ts b/lib/tdf3/src/tdf.ts index 4decf7a9b..fc44e02b9 100644 --- a/lib/tdf3/src/tdf.ts +++ b/lib/tdf3/src/tdf.ts @@ -706,11 +706,11 @@ export async function loadTDFStream(chunker: Chunker): Promise> { +): Record> { const allowed = (k: KeyAccessObject) => allowedKases.allows(k.url); const splitIds = new Set(keyAccess.map(({ sid }) => sid ?? '')); - const accessibleSplits = new Set(keyAccess.filter(allowed).map(({ sid }) => sid)); + const accessibleSplits = new Set(keyAccess.filter(allowed).map(({ sid }) => sid ?? '')); if (splitIds.size > accessibleSplits.size) { const disallowedKases = new Set(keyAccess.filter((k) => !allowed(k)).map(({ url }) => url)); throw new UnsafeUrlError( @@ -720,18 +720,25 @@ export function splitLookupTableFactory( ...disallowedKases ); } - const splitPotentials: Record> = Object.fromEntries( + const splitPotentials: Record> = Object.fromEntries( [...splitIds].map((s) => [s, {}]) ); for (const kao of keyAccess) { + if (!allowed(kao)) { + continue; + } const disjunction = splitPotentials[kao.sid ?? '']; - if (kao.url in disjunction) { - throw new InvalidFileError( - `TODO: Fallback to no split ids. Repetition found for [${kao.url}] on split [${kao.sid}]` + const existing = disjunction[kao.url]; + if (existing) { + const isDuplicate = existing.some( + (e) => e.kid === kao.kid && e.wrappedKey === kao.wrappedKey ); - } - if (allowed(kao)) { - disjunction[kao.url] = kao; + if (isDuplicate) { + continue; + } + existing.push(kao); + } else { + disjunction[kao.url] = [kao]; } } return splitPotentials; @@ -933,14 +940,17 @@ async function unwrapKey({ ); } const anyPromises: Record Promise> = {}; - for (const [kas, keySplitInfo] of Object.entries(potentials)) { - anyPromises[kas] = async () => { - try { - return await tryKasRewrap(keySplitInfo); - } catch (e) { - throw handleRewrapError(e as Error); - } - }; + for (const [kas, kaoList] of Object.entries(potentials)) { + kaoList.forEach((keySplitInfo, idx) => { + const key = kaoList.length === 1 ? kas : `${kas}#${idx}`; + anyPromises[key] = async () => { + try { + return await tryKasRewrap(keySplitInfo); + } catch (e) { + throw handleRewrapError(e as Error); + } + }; + }); } splitPromises[splitId] = () => anyPool(poolSize, anyPromises); } diff --git a/lib/tests/mocha/unit/tdf.spec.ts b/lib/tests/mocha/unit/tdf.spec.ts index 2e476ac3b..d5223ee0a 100644 --- a/lib/tests/mocha/unit/tdf.spec.ts +++ b/lib/tests/mocha/unit/tdf.spec.ts @@ -260,8 +260,8 @@ describe('splitLookupTableFactory', () => { const result = TDF.splitLookupTableFactory(keyAccess, allowedKases); expect(result).to.deep.equal({ - split1: { 'https://kas1': keyAccess[0] }, - split2: { 'https://kas2': keyAccess[1] }, + split1: { 'https://kas1': [keyAccess[0]] }, + split2: { 'https://kas2': [keyAccess[1]] }, }); }); @@ -275,8 +275,8 @@ describe('splitLookupTableFactory', () => { const result = TDF.splitLookupTableFactory(keyAccess, allowedKases); expect(result).to.deep.equal({ - split1: { 'https://kas1': keyAccess[0] }, - split2: { 'https://kas2': keyAccess[1] }, + split1: { 'https://kas1': [keyAccess[0]] }, + split2: { 'https://kas2': [keyAccess[1]] }, }); }); @@ -293,17 +293,45 @@ describe('splitLookupTableFactory', () => { ); }); - it('should throw for duplicate URLs in the same splitId', () => { + it('preserves duplicate URLs in the same splitId as a list', () => { const keyAccess: KeyAccessObject[] = [ - { sid: 'split1', type: 'remote', url: 'https://kas1', protocol: 'kas' }, - { sid: 'split1', type: 'remote', url: 'https://kas1', protocol: 'kas' }, // duplicate URL in same splitId + { + sid: 'split1', + type: 'wrapped', + url: 'https://kas1', + kid: 'k1', + wrappedKey: 'a', + protocol: 'kas', + }, + { + sid: 'split1', + type: 'wrapped', + url: 'https://kas1', + kid: 'k1', + wrappedKey: 'b', + protocol: 'kas', + }, ]; const allowedKases = new OriginAllowList(['https://kas1']); - expect(() => TDF.splitLookupTableFactory(keyAccess, allowedKases)).to.throw( - InvalidFileError, - 'TODO: Fallback to no split ids. Repetition found for [https://kas1] on split [split1]' - ); + const result = TDF.splitLookupTableFactory(keyAccess, allowedKases); + + expect(result).to.deep.equal({ + split1: { 'https://kas1': [keyAccess[0], keyAccess[1]] }, + }); + }); + + it('preserves multiple keys with distinct kids on the same KAS and split', () => { + const keyAccess: KeyAccessObject[] = [ + { sid: 'split1', type: 'wrapped', url: 'https://kas1', kid: 'k1', protocol: 'kas' }, + { sid: 'split1', type: 'wrapped', url: 'https://kas1', kid: 'k2', protocol: 'kas' }, + ]; + const allowedKases = new OriginAllowList(['https://kas1']); + + const result = TDF.splitLookupTableFactory(keyAccess, allowedKases); + + expect(result['split1']['https://kas1']).to.have.length(2); + expect(result['split1']['https://kas1'].map((k) => k.kid)).to.deep.equal(['k1', 'k2']); }); it('should handle empty keyAccess array', () => { @@ -336,7 +364,34 @@ describe('splitLookupTableFactory', () => { const result = TDF.splitLookupTableFactory(keyAccess, new OriginAllowList(allowedKases)); expect(result).to.deep.equal({ - '': { 'https://kas1': keyAccess[0] }, + '': { 'https://kas1': [keyAccess[0]] }, }); }); + + it('detects disallowed splits when sids mix undefined and empty string', () => { + const keyAccess: KeyAccessObject[] = [ + { sid: undefined, type: 'remote', url: 'https://kas1', protocol: 'kas' }, + { sid: '', type: 'remote', url: 'https://kas1', protocol: 'kas' }, + { sid: 'split1', type: 'remote', url: 'https://kas3', protocol: 'kas' }, + ]; + const allowedKases = new OriginAllowList(['https://kas1']); + + expect(() => TDF.splitLookupTableFactory(keyAccess, allowedKases)).to.throw(UnsafeUrlError); + }); + + it('deduplicates fully identical KAOs on the same KAS and split', () => { + const kao: KeyAccessObject = { + sid: 'split1', + type: 'wrapped', + url: 'https://kas1', + kid: 'k1', + wrappedKey: 'abc', + protocol: 'kas', + }; + const allowedKases = new OriginAllowList(['https://kas1']); + + const result = TDF.splitLookupTableFactory([kao, { ...kao }], allowedKases); + + expect(result['split1']['https://kas1']).to.have.length(1); + }); }); diff --git a/spec/DSPX-3379.md b/spec/DSPX-3379.md new file mode 100644 index 000000000..7a5d7bb79 --- /dev/null +++ b/spec/DSPX-3379.md @@ -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>` to `Record>`. 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.