Skip to content

Commit ebf0687

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 0a1ef65 commit ebf0687

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
@@ -8,6 +8,7 @@ import {
88
KeyringTrace,
99
KeyringTraceFlag,
1010
EncryptedDataKey,
11+
needs,
1112
} from '@aws-crypto/material-management'
1213

1314
export interface RawKeyRing<S extends SupportedAlgorithmSuites> {
@@ -54,17 +55,37 @@ export function _onDecrypt<
5455
/* Check for early return (Postcondition): If there are not EncryptedDataKeys for this keyring, do nothing. */
5556
if (!edks.length) return material
5657

58+
const cmkErrors: Error[] = []
59+
5760
for (const edk of edks) {
5861
try {
5962
return await this._unwrapKey(material, edk)
6063
} catch (e) {
61-
// there should be some debug here? or wrap?
62-
// Failures decrypt should not short-circuit the process
63-
// If the caller does not have access they may have access
64-
// through another Keyring.
64+
/* Failures onDecrypt should not short-circuit the process
65+
* If the caller does not have access they may have access
66+
* through another Keyring.
67+
*/
68+
cmkErrors.push(e)
6569
}
6670
}
6771

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

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

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

228-
it('errors in _unwrapKey should not cause _onDecrypt to throw.', async () => {
228+
it('Postcondition: An EDK must provide a valid data key or _unwrapKey must not have raised any errors.', async () => {
229229
const suite = new NodeAlgorithmSuite(
230230
AlgorithmSuiteIdentifier.ALG_AES128_GCM_IV12_TAG16
231231
)
@@ -258,8 +258,10 @@ describe('_onDecrypt', () => {
258258
_filter,
259259
} as any
260260

261-
const test = await testKeyring._onDecrypt(material, [edk])
262-
expect(test.hasValidKey()).to.equal(false)
261+
await expect(testKeyring._onDecrypt(material, [edk])).to.rejectedWith(
262+
Error,
263+
'Unable to decrypt data key'
264+
)
263265
expect(unwrapCalled).to.equal(1)
264266
expect(filterCalled).to.equal(1)
265267
})

0 commit comments

Comments
 (0)