Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 17 additions & 19 deletions lib/tdf3/src/tdf.ts
Original file line number Diff line number Diff line change
Expand Up @@ -706,7 +706,7 @@
export function splitLookupTableFactory(
keyAccess: KeyAccessObject[],
allowedKases: OriginAllowList
): Record<string, Record<string, KeyAccessObject>> {
): Record<string, KeyAccessObject[]> {
const allowed = (k: KeyAccessObject) => allowedKases.allows(k.url);
const splitIds = new Set(keyAccess.map(({ sid }) => sid ?? ''));

Expand All @@ -720,23 +720,17 @@
...disallowedKases
);
}
const splitPotentials: Record<string, Record<string, KeyAccessObject>> = Object.fromEntries(
[...splitIds].map((s) => [s, {}])
// Each split id maps to the list of KAOs that can unwrap it (a disjunction:
// any one succeeding unwraps the split). The same KAS may legitimately appear
// more than once for a split - an authoring mistake (same key used twice) or
// intentional multiple keys on one KAS - so we keep every allowed KAO as an
// alternative rather than rejecting duplicates. See DSPX-3379.
const splitPotentials: Record<string, KeyAccessObject[]> = Object.fromEntries(
[...splitIds].map((s) => [s, []])
);
for (const kao of keyAccess) {
const disjunction = splitPotentials[kao.sid ?? ''];
if (kao.url in disjunction) {
// TODO(DSPX-3454): Handle duplicate KAS URLs with different KIDs.
// Each KAO contains a KID - the function should be updated to use this
// information to differentiate between keys from the same KAS.
// Cross-SDK validation needed via xtest.
throw new InvalidFileError(
`Unable to decrypt: Multiple keys detected for Key Access Server [${kao.url}]. ` +
`Please contact your administrator.`
);
}
if (allowed(kao)) {
disjunction[kao.url] = kao;
splitPotentials[kao.sid ?? ''].push(kao);
}
}
return splitPotentials;
Expand Down Expand Up @@ -931,22 +925,26 @@
const splitPromises: Record<string, () => Promise<RewrapResponseData>> = {};
for (const splitId of Object.keys(splitPotentials)) {
const potentials = splitPotentials[splitId];
if (!potentials || !Object.keys(potentials).length) {
if (!potentials || !potentials.length) {

Check warning on line 928 in lib/tdf3/src/tdf.ts

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Prefer using an optional chain expression instead, as it's more concise and easier to read.

See more on https://sonarcloud.io/project/issues?id=opentdf_client-web&issues=AZ9igavFW63rbi1ojk6Y&open=AZ9igavFW63rbi1ojk6Y&pullRequest=969
throw new UnsafeUrlError(
`Unreconstructable key - no valid KAS found for split ${JSON.stringify(splitId)}`,
''
);
}
const anyPromises: Record<string, () => Promise<RewrapResponseData>> = {};
for (const [kas, keySplitInfo] of Object.entries(potentials)) {
anyPromises[kas] = async () => {
potentials.forEach((keySplitInfo, i) => {
// Key by url+kid+index so multiple KAOs on the same KAS stay distinct
// alternatives within the split's disjunction (anyPool tries each until
// one succeeds). See DSPX-3379.
const alternativeKey = `${keySplitInfo.url}#${keySplitInfo.kid ?? ''}#${i}`;
anyPromises[alternativeKey] = async () => {
try {
return await tryKasRewrap(keySplitInfo);
} catch (e) {
throw handleRewrapError(e as Error);
}
};
}
});
splitPromises[splitId] = () => anyPool(poolSize, anyPromises);
}
try {
Expand Down
6 changes: 6 additions & 0 deletions lib/tests/mocha/client.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ describe('client wrapper tests', function () {
});

it('encrypt error', async function () {
const fetchStub = sinon.stub(globalThis, 'fetch').rejects(new Error('Network error'));
const encryptParams = new TDF.EncryptParamsBuilder().withStringSource('hello world').build();
const config = {
kasEndpoint: 'https://kasUrl',
Expand All @@ -123,10 +124,13 @@ describe('client wrapper tests', function () {
assert.fail('did not throw');
} catch (expected) {
assert.ok(expected);
} finally {
fetchStub.restore();
}
});

it('decrypt error', async function () {
const fetchStub = sinon.stub(globalThis, 'fetch').rejects(new Error('Network error'));
const decryptParams = new TDF.DecryptParamsBuilder().withStringSource('not a tdf').build();
const config = {
kasEndpoint: 'https://kasUrl',
Expand All @@ -138,6 +142,8 @@ describe('client wrapper tests', function () {
assert.fail('did not throw');
} catch (expected) {
assert.ok(expected);
} finally {
fetchStub.restore();
}
});

Expand Down
52 changes: 52 additions & 0 deletions lib/tests/mocha/encrypt-decrypt.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -363,6 +363,58 @@ describe('encrypt decrypt test', async function () {
}
}

it('decrypts when the same KAS wraps the same split twice (DSPX-3379)', async function () {
const cipher = new AesGcmCipher(WebCryptoService);
const encryptionInformation = new SplitKey(cipher);
const key1 = await encryptionInformation.generateKey();
const keyMiddleware = async () => ({ keyForEncryption: key1, keyForManifest: key1 });

const client = new Client.Client({
kasEndpoint: kasUrl,
platformUrl: kasUrl,
allowedKases: [kasUrl],
dpopKeys: Mocks.entityKeyPair(),
clientId: 'id',
authProvider,
});

const scope: Scope = { dissem: ['user@domain.com'], attributes: [] };

// Two KAOs pointing at the same KAS for the same split id: the same KAS
// wraps the same split twice. Previously this threw; now the copies are
// disjunction alternatives and the file must still decrypt.
const encryptedStream = await client.encrypt({
metadata: Mocks.getMetadataObject(),
wrappingKeyAlgorithm: 'rsa:2048',
offline: true,
scope,
keyMiddleware,
splitPlan: [
{ kas: kasUrl, sid: '1' },
{ kas: kasUrl, sid: '1' },
],
source: new ReadableStream({
start(controller) {
controller.enqueue(new TextEncoder().encode(expectedVal));
controller.close();
},
}),
});

const kaos = encryptedStream.manifest.encryptionInformation.keyAccess;
assert.equal(kaos.length, 2, 'expected two KAOs for the duplicated split');
assert.equal(kaos[0].url, kaos[1].url);
assert.equal(kaos[0].sid, kaos[1].sid);

const decryptStream = await client.decrypt({
source: { type: 'stream', location: encryptedStream.stream },
wrappingKeyAlgorithm: 'rsa:2048',
});

const { value: decryptedText } = await decryptStream.stream.getReader().read();
assert.equal(new TextDecoder().decode(decryptedText), expectedVal);
});

it('encrypt-decrypt with system metadata assertion', async function () {
const cipher = new AesGcmCipher(WebCryptoService);
const encryptionInformation = new SplitKey(cipher);
Expand Down
40 changes: 28 additions & 12 deletions lib/tests/mocha/unit/tdf.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: [keyAccess[0]],
split2: [keyAccess[1]],
});
});

Expand All @@ -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: [keyAccess[0]],
split2: [keyAccess[1]],
});
});

Expand All @@ -293,17 +293,33 @@ describe('splitLookupTableFactory', () => {
);
});

it('should throw for duplicate URLs in the same splitId', () => {
it('should keep duplicate URLs in the same splitId as alternatives (DSPX-3379)', () => {
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: 'remote', url: 'https://kas1', protocol: 'kas' }, // same KAS + split
];
const allowedKases = new OriginAllowList(['https://kas1']);

expect(() => TDF.splitLookupTableFactory(keyAccess, allowedKases)).to.throw(
InvalidFileError,
'Unable to decrypt: Multiple keys detected for Key Access Server [https://kas1]. Please contact your administrator.'
);
const result = TDF.splitLookupTableFactory(keyAccess, allowedKases);

// Both copies are retained as disjunction alternatives; unwrap tries each.
expect(result).to.deep.equal({
split1: [keyAccess[0], keyAccess[1]],
});
});

it('should keep same-KAS different-kid entries in the same splitId (DSPX-3379)', () => {
const keyAccess: KeyAccessObject[] = [
{ sid: 'split1', type: 'remote', url: 'https://kas1', protocol: 'kas', kid: 'k1' },
{ sid: 'split1', type: 'remote', url: 'https://kas1', protocol: 'kas', kid: 'k2' },
];
const allowedKases = new OriginAllowList(['https://kas1']);

const result = TDF.splitLookupTableFactory(keyAccess, allowedKases);

expect(result).to.deep.equal({
split1: [keyAccess[0], keyAccess[1]],
});
});

it('should handle empty keyAccess array', () => {
Expand All @@ -323,7 +339,7 @@ describe('splitLookupTableFactory', () => {

expect(() => TDF.splitLookupTableFactory(keyAccess, allowedKases)).to.throw(
InvalidFileError,
'Unreconstructable key - disallowed KASes include: ["https://kas1"]'
'Unreconstructable key - disallowed KASes include: ["https://kas1"] from splitIds ["split1"]'
);
});

Expand All @@ -336,7 +352,7 @@ describe('splitLookupTableFactory', () => {
const result = TDF.splitLookupTableFactory(keyAccess, new OriginAllowList(allowedKases));

expect(result).to.deep.equal({
'': { 'https://kas1': keyAccess[0] },
'': [keyAccess[0]],
});
});
});
Loading