Skip to content

fix: KeyringTraceFlag requirements and data key caching #210

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Sep 19, 2019
Merged

fix: KeyringTraceFlag requirements and data key caching #210

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
107566e
fix: KeyringTraceFlag requirements and data key caching
seebees Sep 13, 2019
51873c2
Merge branch 'master' into clone-decrypt-materials
seebees Sep 19, 2019
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
25 changes: 15 additions & 10 deletions modules/cache-material/test/caching_cryptographic_materials_decorators.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,22 +198,27 @@ describe('Cryptographic Material Functions', () => {

const nodeSuite = new NodeAlgorithmSuite(suiteId)
const udk128 = new Uint8Array([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16])
const trace = {
const encryptTrace = {
keyNamespace: 'keyNamespace',
keyName: 'keyName',
flags: KeyringTraceFlag.WRAPPING_KEY_GENERATED_DATA_KEY | KeyringTraceFlag.WRAPPING_KEY_DECRYPTED_DATA_KEY
flags: KeyringTraceFlag.WRAPPING_KEY_GENERATED_DATA_KEY
}
const decryptTrace = {
keyNamespace: 'keyNamespace',
keyName: 'keyName',
flags: KeyringTraceFlag.WRAPPING_KEY_DECRYPTED_DATA_KEY
}

const edk1 = new EncryptedDataKey({ providerId: 'keyNamespace', providerInfo: 'keyName', encryptedDataKey: new Uint8Array([1]) })
const edk2 = new EncryptedDataKey({ providerId: 'p2', providerInfo: 'pi2', encryptedDataKey: new Uint8Array([2]) })

const encryptionMaterial = new NodeEncryptionMaterial(nodeSuite, {})
.setUnencryptedDataKey(udk128, trace)
.setUnencryptedDataKey(udk128, encryptTrace)
.addEncryptedDataKey(edk1, KeyringTraceFlag.WRAPPING_KEY_ENCRYPTED_DATA_KEY)
.addEncryptedDataKey(edk2, KeyringTraceFlag.WRAPPING_KEY_ENCRYPTED_DATA_KEY)

const decryptionMaterial = new NodeDecryptionMaterial(nodeSuite, {})
.setUnencryptedDataKey(udk128, trace)
.setUnencryptedDataKey(udk128, decryptTrace)

const context = {}

Expand Down Expand Up @@ -343,17 +348,17 @@ describe('Cryptographic Material Functions', () => {

const nodeSuite = new NodeAlgorithmSuite(suiteId)
const udk128 = new Uint8Array([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16])
const trace = {
const encryptTrace = {
keyNamespace: 'keyNamespace',
keyName: 'keyName',
flags: KeyringTraceFlag.WRAPPING_KEY_GENERATED_DATA_KEY | KeyringTraceFlag.WRAPPING_KEY_DECRYPTED_DATA_KEY
flags: KeyringTraceFlag.WRAPPING_KEY_GENERATED_DATA_KEY
}

const edk1 = new EncryptedDataKey({ providerId: 'keyNamespace', providerInfo: 'keyName', encryptedDataKey: new Uint8Array([1]) })
const edk2 = new EncryptedDataKey({ providerId: 'p2', providerInfo: 'pi2', encryptedDataKey: new Uint8Array([2]) })

const encryptionMaterial = new NodeEncryptionMaterial(nodeSuite, {})
.setUnencryptedDataKey(udk128, trace)
.setUnencryptedDataKey(udk128, encryptTrace)
.addEncryptedDataKey(edk1, KeyringTraceFlag.WRAPPING_KEY_ENCRYPTED_DATA_KEY)
.addEncryptedDataKey(edk2, KeyringTraceFlag.WRAPPING_KEY_ENCRYPTED_DATA_KEY)

Expand Down Expand Up @@ -401,17 +406,17 @@ describe('Cryptographic Material Functions', () => {

const nodeSuite = new NodeAlgorithmSuite(suiteId)
const udk128 = new Uint8Array([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16])
const trace = {
const encryptTrace = {
keyNamespace: 'keyNamespace',
keyName: 'keyName',
flags: KeyringTraceFlag.WRAPPING_KEY_GENERATED_DATA_KEY | KeyringTraceFlag.WRAPPING_KEY_DECRYPTED_DATA_KEY
flags: KeyringTraceFlag.WRAPPING_KEY_GENERATED_DATA_KEY
}

const edk1 = new EncryptedDataKey({ providerId: 'keyNamespace', providerInfo: 'keyName', encryptedDataKey: new Uint8Array([1]) })
const edk2 = new EncryptedDataKey({ providerId: 'p2', providerInfo: 'pi2', encryptedDataKey: new Uint8Array([2]) })

const encryptionMaterial = new NodeEncryptionMaterial(nodeSuite, {})
.setUnencryptedDataKey(udk128, trace)
.setUnencryptedDataKey(udk128, encryptTrace)
.addEncryptedDataKey(edk1, KeyringTraceFlag.WRAPPING_KEY_ENCRYPTED_DATA_KEY)
.addEncryptedDataKey(edk2, KeyringTraceFlag.WRAPPING_KEY_ENCRYPTED_DATA_KEY)

Expand Down
3 changes: 2 additions & 1 deletion modules/kms-keyring/src/kms_keyring.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,8 @@ export function KmsKeyringClass<S extends SupportedAlgorithmSuites, Client exten
* See cryptographic_materials as setUnencryptedDataKey will throw in this case.
*/
.setUnencryptedDataKey(dataKey.Plaintext, trace)
.addEncryptedDataKey(kmsResponseToEncryptedDataKey(dataKey), KeyringTraceFlag.WRAPPING_KEY_ENCRYPTED_DATA_KEY)
.addEncryptedDataKey(kmsResponseToEncryptedDataKey(dataKey),
KeyringTraceFlag.WRAPPING_KEY_ENCRYPTED_DATA_KEY | KeyringTraceFlag.WRAPPING_KEY_SIGNED_ENC_CTX)
} else if (generatorKeyId) {
keyIds.unshift(generatorKeyId)
}
Expand Down
17 changes: 11 additions & 6 deletions modules/kms-keyring/test/kms_keyring.onencrypt.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,17 +81,22 @@ describe('KmsKeyring: _onEncrypt', () => {
expect(edkEncrypt.providerId).to.equal('aws-kms')
expect(edkEncrypt.providerInfo).to.equal(encryptKmsKey)

expect(material.keyringTrace).to.have.lengthOf(2)
const [traceGenerate, traceEncrypt] = material.keyringTrace
expect(material.keyringTrace).to.have.lengthOf(3)
const [traceGenerate, traceEncrypt1, traceEncrypt2] = material.keyringTrace
expect(traceGenerate.keyNamespace).to.equal('aws-kms')
expect(traceGenerate.keyName).to.equal(generatorKeyId)
expect(traceGenerate.flags & KeyringTraceFlag.WRAPPING_KEY_GENERATED_DATA_KEY).to.equal(KeyringTraceFlag.WRAPPING_KEY_GENERATED_DATA_KEY)
expect(traceGenerate.flags & KeyringTraceFlag.WRAPPING_KEY_ENCRYPTED_DATA_KEY).to.equal(KeyringTraceFlag.WRAPPING_KEY_ENCRYPTED_DATA_KEY)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do WRAPPING_KEY_ENCRYPTED_DATA_KEY and WRAPPING_KEY_SIGNED_ENC_CTX exist on the generate trace? Should they if the wrapping key hasn't been used to encrypt yet?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the KMS case. On generate, the encrypted data key is also returned....
It is just so that people can see the relationship between
the generate trace and the encrypt trace.

expect(traceGenerate.flags & KeyringTraceFlag.WRAPPING_KEY_SIGNED_ENC_CTX).to.equal(KeyringTraceFlag.WRAPPING_KEY_SIGNED_ENC_CTX)
expect(traceEncrypt.keyNamespace).to.equal('aws-kms')
expect(traceEncrypt.keyName).to.equal(encryptKmsKey)
expect(traceEncrypt.flags & KeyringTraceFlag.WRAPPING_KEY_ENCRYPTED_DATA_KEY).to.equal(KeyringTraceFlag.WRAPPING_KEY_ENCRYPTED_DATA_KEY)
expect(traceEncrypt.flags & KeyringTraceFlag.WRAPPING_KEY_SIGNED_ENC_CTX).to.equal(KeyringTraceFlag.WRAPPING_KEY_SIGNED_ENC_CTX)

expect(traceEncrypt1.keyNamespace).to.equal('aws-kms')
expect(traceEncrypt1.keyName).to.equal(generatorKeyId)
expect(traceEncrypt1.flags & KeyringTraceFlag.WRAPPING_KEY_ENCRYPTED_DATA_KEY).to.equal(KeyringTraceFlag.WRAPPING_KEY_ENCRYPTED_DATA_KEY)
expect(traceEncrypt1.flags & KeyringTraceFlag.WRAPPING_KEY_SIGNED_ENC_CTX).to.equal(KeyringTraceFlag.WRAPPING_KEY_SIGNED_ENC_CTX)
expect(traceEncrypt2.keyNamespace).to.equal('aws-kms')
expect(traceEncrypt2.keyName).to.equal(encryptKmsKey)
expect(traceEncrypt2.flags & KeyringTraceFlag.WRAPPING_KEY_ENCRYPTED_DATA_KEY).to.equal(KeyringTraceFlag.WRAPPING_KEY_ENCRYPTED_DATA_KEY)
expect(traceEncrypt2.flags & KeyringTraceFlag.WRAPPING_KEY_SIGNED_ENC_CTX).to.equal(KeyringTraceFlag.WRAPPING_KEY_SIGNED_ENC_CTX)
})

it('Check for early return (Postcondition): Discovery Keyrings do not encrypt.', async () => {
Expand Down
37 changes: 25 additions & 12 deletions modules/material-management/src/clone_cryptographic_material.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import {
import {
AwsEsdkKeyObject // eslint-disable-line no-unused-vars
} from './types'
import { KeyringTraceFlag } from './keyring_trace'
import { needs } from './needs'

type Material = NodeEncryptionMaterial|NodeDecryptionMaterial|WebCryptoEncryptionMaterial|WebCryptoDecryptionMaterial

Expand All @@ -42,36 +42,49 @@ export function cloneMaterial<M extends Material> (source: M): M {
? new WebCryptoEncryptionMaterial(suite, encryptionContext)
: new WebCryptoDecryptionMaterial(suite, encryptionContext))

/* The WRAPPING_KEY_GENERATED_DATA_KEY _should_ be the first trace,
* but it is better to look for it explicitly.
/* The setTrace _must_ be the first trace,
* If the material is an EncryptionMaterial
* then the data key *must* have been generated.
* If the material is DecryptionMaterial
* then the data key *must* have been decrypted.
* i.e. the required flags are:
* WRAPPING_KEY_GENERATED_DATA_KEY, WRAPPING_KEY_DECRYPTED_DATA_KEY
* These are controlled by the material itself.
* Furthermore, subsequent trace entries,
* *must* be in the same order as the added encrypted data keys.
* See cryptographic_materials.ts `decorateCryptographicMaterial`, `decorateWebCryptoMaterial`.
*/
const trace = source.keyringTrace.find(({ flags }) => flags & KeyringTraceFlag.WRAPPING_KEY_GENERATED_DATA_KEY)
const [setTrace, ...traces] = source.keyringTrace.slice()

if (source.hasUnencryptedDataKey) {
const udk = cloneUnencryptedDataKey(source.getUnencryptedDataKey())
if (!trace) throw new Error('Malformed source material.')
clone.setUnencryptedDataKey(udk, trace)
clone.setUnencryptedDataKey(udk, setTrace)
}

if ((<WebCryptoDecryptionMaterial>source).hasCryptoKey) {
const cryptoKey = (<WebCryptoDecryptionMaterial>source).getCryptoKey()
if (!trace) throw new Error('Malformed source material.')
;(<WebCryptoDecryptionMaterial>clone)
.setCryptoKey(cryptoKey, trace)
.setCryptoKey(cryptoKey, setTrace)
}

if (isEncryptionMaterial(source) && isEncryptionMaterial(clone)) {
source.encryptedDataKeys.forEach(edk => {
const encryptedDataKeys = source.encryptedDataKeys
/* Precondition: For each encrypted data key, there must be a trace. */
needs(encryptedDataKeys.length === traces.length, 'KeyringTrace length does not match encrypted data keys.')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is this true if there is a separate trace for generate?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

const [generate, ...others] = traces

If you see above :)

encryptedDataKeys.forEach((edk, i) => {
const { providerInfo, providerId } = edk
const trace = source.keyringTrace.find(({ keyNamespace, keyName }) => keyName === providerInfo && keyNamespace === providerId)
if (!trace) throw new Error('Malformed Encrypted Data Key')
clone.addEncryptedDataKey(edk, trace.flags)
const { keyNamespace, keyName, flags } = traces[i]
/* Precondition: The traces must be in the same order as the encrypted data keys. */
needs(keyName === providerInfo && keyNamespace === providerId, 'Keyring trace does not match encrypted data key.')
clone.addEncryptedDataKey(edk, flags)
})

if (source.suite.signatureCurve && source.signatureKey) {
clone.setSignatureKey(source.signatureKey)
}
} else if (isDecryptionMaterial(source) && isDecryptionMaterial(clone)) {
/* Precondition: On Decrypt there must not be any additional traces other than the setTrace. */
needs(!traces.length, 'Only 1 trace is valid on DecryptionMaterials.')
if (source.suite.signatureCurve && source.verificationKey) {
clone.setVerificationKey(source.verificationKey)
}
Expand Down
50 changes: 31 additions & 19 deletions modules/material-management/src/cryptographic_material.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,20 @@ export function isDecryptionMaterial (obj: any): obj is WebCryptoDecryptionMater
return (obj instanceof WebCryptoDecryptionMaterial) || (obj instanceof NodeDecryptionMaterial)
}

export function decorateCryptographicMaterial<T extends CryptographicMaterial<T>> (material: T, setFlags: KeyringTraceFlag) {
export function decorateCryptographicMaterial<T extends CryptographicMaterial<T>> (material: T, setFlag: KeyringTraceFlag) {
/* Precondition: setFlag must be in the set of KeyringTraceFlag.SET_FLAGS. */
needs(setFlag & KeyringTraceFlag.SET_FLAGS, 'Invalid setFlag')
/* When a KeyringTraceFlag is passed to setUnencryptedDataKey,
* it must be valid for the type of material.
* It is invalid to claim that EncryptionMaterial were decrypted.
*/
const deniedSetFlags = (KeyringTraceFlag.SET_FLAGS ^ setFlag) | (
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Going back to the SET_FLAG naming, it feels weird to organize the SET_FLAGS but then also have this logic here say "if you're encrypting actually we only care about the generate flag and if you're decrypting we only care about the decrypt flag". If this function requires this logic anyway, why try to reason in terms of SET_FLAGS at all and instead just think in terms of the generate and decrypt flags?

setFlag === KeyringTraceFlag.WRAPPING_KEY_GENERATED_DATA_KEY
? KeyringTraceFlag.DECRYPT_FLAGS
: setFlag === KeyringTraceFlag.WRAPPING_KEY_DECRYPTED_DATA_KEY
? KeyringTraceFlag.ENCRYPT_FLAGS
: 0)

let unencryptedDataKeyZeroed = false
let unencryptedDataKey: AwsEsdkKeyObject | Uint8Array
// This copy of the unencryptedDataKey is stored to insure that the
Expand Down Expand Up @@ -390,11 +403,16 @@ export function decorateCryptographicMaterial<T extends CryptographicMaterial<T>
/* Precondition: Trace must be set, and the flag must indicate that the data key was generated. */
needs(trace && trace.keyName && trace.keyNamespace, 'Malformed KeyringTrace')
/* Precondition: On set the required KeyringTraceFlag must be set. */
needs(trace.flags & setFlags, 'Required KeyringTraceFlag not set')
needs(trace.flags & setFlag, 'Required KeyringTraceFlag not set')
/* Precondition: Only valid flags are allowed.
* An unencrypted data key can not be both generated and decrypted.
*/
needs(!(trace.flags & deniedSetFlags), 'Invalid KeyringTraceFlags set.')
}
}

export function decorateEncryptionMaterial<T extends EncryptionMaterial<T>> (material: T) {
const deniedEncryptFlags = KeyringTraceFlag.SET_FLAGS | KeyringTraceFlag.DECRYPT_FLAGS
const encryptedDataKeys: EncryptedDataKey[] = []
let signatureKey: Readonly<SignatureKey>|undefined

Expand All @@ -412,16 +430,18 @@ export function decorateEncryptionMaterial<T extends EncryptionMaterial<T>> (mat

/* Precondition: flags must indicate that the key was encrypted. */
needs(flags & KeyringTraceFlag.WRAPPING_KEY_ENCRYPTED_DATA_KEY, 'Encrypted data key flag must be set.')
/* When the unencrypted data key is first set, a given wrapping key may or may not also encrypt that key.
* This means that the first EDK that is added may already have a trace.
* The flags for the EDK and the existing trace should be merged iif this is the first EDK
* and the only existing trace corresponds to this EDK.

/* Precondition: flags must not include a setFlag or a decrypt flag.
* The setFlag is reserved for setting the unencrypted data key
* and must only occur once in the set of KeyringTrace flags.
* The two setFlags in use are:
* KeyringTraceFlag.WRAPPING_KEY_DECRYPTED_DATA_KEY
* KeyringTraceFlag.WRAPPING_KEY_GENERATED_DATA_KEY
*
* KeyringTraceFlag.WRAPPING_KEY_VERIFIED_ENC_CTX is reserved for the decrypt path
*/
if (firstEdkAndTraceMatch(encryptedDataKeys, material.keyringTrace, edk)) {
material.keyringTrace[0].flags |= flags
} else {
material.keyringTrace.push({ keyName: edk.providerInfo, keyNamespace: edk.providerId, flags })
}
needs(!(flags & deniedEncryptFlags), 'Invalid flag for EncryptedDataKey.')
material.keyringTrace.push({ keyName: edk.providerInfo, keyNamespace: edk.providerId, flags })

encryptedDataKeys.push(edk)
return material
Expand Down Expand Up @@ -463,14 +483,6 @@ export function decorateEncryptionMaterial<T extends EncryptionMaterial<T>> (mat
return material
}

/* Verify that the this is the first EDK and that it matches the 1 and only 1 trace. */
function firstEdkAndTraceMatch (edks: EncryptedDataKey[], traces: KeyringTrace[], edk: EncryptedDataKey) {
return edks.length === 0 &&
traces.length === 1 &&
edk.providerId === traces[0].keyNamespace &&
edk.providerInfo === traces[0].keyName
}

export function decorateDecryptionMaterial<T extends DecryptionMaterial<T>> (material: T) {
// Verification Key
let verificationKey: Readonly<VerificationKey>|undefined
Expand Down
22 changes: 22 additions & 0 deletions modules/material-management/src/keyring_trace.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,4 +72,26 @@ export enum KeyringTraceFlag {
* Bit flag indicating this wrapping key verified the signature of the encryption context.
*/
WRAPPING_KEY_VERIFIED_ENC_CTX = (1 << 4), // eslint-disable-line no-unused-vars

/* KeyringTraceFlags are organized here.
* The three groupings are set, encrypt, and decrypt.
* An unencrypted data key is set and is required to have a SET_FLAG.
* For the encrypt path, the unencrypted data key must be generated.
* For the decrypt path, the unencrypted data key must be decrypted.
*
* A encrypted data key must be encrypted
* and the encryption context may be signed.
*
* When an encrypted data key is decrypted,
* the encryption context may be verified.
*
* This organization is to keep a KeyringTrace for an encrypted data key
* for listing the WRAPPING_KEY_VERIFIED_ENC_CTX flag.
*/

ENCRYPT_FLAGS = WRAPPING_KEY_ENCRYPTED_DATA_KEY | WRAPPING_KEY_SIGNED_ENC_CTX, // eslint-disable-line no-unused-vars

SET_FLAGS = WRAPPING_KEY_GENERATED_DATA_KEY | WRAPPING_KEY_DECRYPTED_DATA_KEY, // eslint-disable-line no-unused-vars
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After going over the PR a couple times the inclusion of WRAPPING_KEY_DECRYPTED_DATA_KEY here finally makes sense, but is initially very confusing. When I think of the decrypt flag I think of the decrypt operation. The fact that the output was of that set in the materials is something that seems less salient. I'm not sure if there is another way to name this/organize these fields that is more intuitive. 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also am thinking about it. That is why I called it SET because the function setUnencryptedDataKey


DECRYPT_FLAGS = WRAPPING_KEY_DECRYPTED_DATA_KEY | WRAPPING_KEY_VERIFIED_ENC_CTX, // eslint-disable-line no-unused-vars
}
Loading