Skip to content

Commit b36eba4

Browse files
committed
fix: Missing errors in raw keyrings
On decrypt, error messages can be passed over. However, if the keyring attempts to decrypt, and fails how does anyone know what happend? This is a similar solution to aws#212
1 parent c82226a commit b36eba4

File tree

2 files changed

+30
-7
lines changed

2 files changed

+30
-7
lines changed

modules/raw-keyring/src/raw_keyring_decorators.ts

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import {
66
DecryptionMaterial,
77
SupportedAlgorithmSuites,
88
EncryptedDataKey,
9+
needs,
910
} from '@aws-crypto/material-management'
1011

1112
export interface RawKeyRing<S extends SupportedAlgorithmSuites> {
@@ -47,17 +48,37 @@ export function _onDecrypt<
4748
/* Check for early return (Postcondition): If there are not EncryptedDataKeys for this keyring, do nothing. */
4849
if (!edks.length) return material
4950

51+
const cmkErrors: Error[] = []
52+
5053
for (const edk of edks) {
5154
try {
5255
return await this._unwrapKey(material, edk)
5356
} catch (e) {
54-
// there should be some debug here? or wrap?
55-
// Failures decrypt should not short-circuit the process
56-
// If the caller does not have access they may have access
57-
// through another Keyring.
57+
/* Failures onDecrypt should not short-circuit the process
58+
* If the caller does not have access they may have access
59+
* through another Keyring.
60+
*/
61+
cmkErrors.push(e)
5862
}
5963
}
6064

65+
/* Postcondition: An EDK must provide a valid data key or _unwrapKey must not have raised any errors.
66+
* If I have a data key,
67+
* decrypt errors can be ignored.
68+
* However, if I was unable to decrypt a data key AND I have errors,
69+
* these errors should bubble up.
70+
* Otherwise, the only error customers will see is that
71+
* the material does not have an unencrypted data key.
72+
* So I return a concatenated Error message
73+
*/
74+
needs(
75+
material.hasValidKey() || (!material.hasValidKey() && !cmkErrors.length),
76+
cmkErrors.reduce(
77+
(m, e, i) => `${m} Error #${i + 1} \n ${e.stack} \n`,
78+
`Unable to decrypt data key ${this.keyName} ${this.keyNamespace}.\n `
79+
)
80+
)
81+
6182
return material
6283
}
6384
}

modules/raw-keyring/test/raw_keyring_decorators.test.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,7 @@ describe('_onDecrypt', () => {
202202
expect(filterCalled).to.equal(1)
203203
})
204204

205-
it('errors in _unwrapKey should not cause _onDecrypt to throw.', async () => {
205+
it('Postcondition: An EDK must provide a valid data key or _unwrapKey must not have raised any errors.', async () => {
206206
const suite = new NodeAlgorithmSuite(
207207
AlgorithmSuiteIdentifier.ALG_AES128_GCM_IV12_TAG16
208208
)
@@ -235,8 +235,10 @@ describe('_onDecrypt', () => {
235235
_filter,
236236
} as any
237237

238-
const test = await testKeyring._onDecrypt(material, [edk])
239-
expect(test.hasValidKey()).to.equal(false)
238+
await expect(testKeyring._onDecrypt(material, [edk])).to.rejectedWith(
239+
Error,
240+
'Unable to decrypt data key'
241+
)
240242
expect(unwrapCalled).to.equal(1)
241243
expect(filterCalled).to.equal(1)
242244
})

0 commit comments

Comments
 (0)