Skip to content

feat: immutable cryptographic materials (for keyrings) #231

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 12 commits into from
Mar 31, 2020
Merged

feat: immutable cryptographic materials (for keyrings) #231

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
8e7a636
feat: change keyring materials modifiers to immutable constructors
mattsb42-aws Mar 27, 2020
b88b8aa
feat: add key_name parameter and key_namespace accessor to MasterKeyInfo
mattsb42-aws Mar 27, 2020
4a18fbd
feat: adopt immutable materials for KMS keyring
mattsb42-aws Mar 27, 2020
c6fb4e7
feat: adopt immutable materials for multi-keyring
mattsb42-aws Mar 27, 2020
7f9d74a
fix: revise test helpers to use key IDs that are not used in tests
mattsb42-aws Mar 27, 2020
030c736
feat: add key_name parameter and key_namespace accessor to MasterKeyI…
mattsb42-aws Mar 27, 2020
54bcdb3
feat: fix raw keyrings and move to immutable materials
mattsb42-aws Mar 27, 2020
8de2b7a
fix: fix incorrectly placed test assertions
mattsb42-aws Mar 27, 2020
c3844dd
chore: clean up linting
mattsb42-aws Mar 27, 2020
d9e3a0d
docs: update src/aws_encryption_sdk/keyrings/raw.py
mattsb42-aws Mar 27, 2020
2d3f9a3
chore: remove unnecessary copies on encrypt and avoid overwriting poi…
mattsb42-aws Mar 30, 2020
8a31fa0
chore: finish copy cleanup
mattsb42-aws Mar 30, 2020
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
2 changes: 1 addition & 1 deletion src/aws_encryption_sdk/internal/formatting/serialize.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,6 @@ def serialize_wrapped_key(key_provider, wrapping_algorithm, wrapping_key_id, enc
)
key_ciphertext = encrypted_wrapped_key.ciphertext + encrypted_wrapped_key.tag
return EncryptedDataKey(
key_provider=MasterKeyInfo(provider_id=key_provider.provider_id, key_info=key_info),
key_provider=MasterKeyInfo(provider_id=key_provider.provider_id, key_info=key_info, key_name=wrapping_key_id),
encrypted_data_key=key_ciphertext,
)
28 changes: 13 additions & 15 deletions src/aws_encryption_sdk/keyrings/aws_kms/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
.. versionadded:: 1.5.0

"""
import copy
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you still need this?

Copy link
Member Author

Choose a reason for hiding this comment

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

Nope! That'll be why CI is failing. :)

import logging

import attr
Expand Down Expand Up @@ -180,25 +181,26 @@ class _AwsKmsSingleCmkKeyring(Keyring):
def on_encrypt(self, encryption_materials):
# type: (EncryptionMaterials) -> EncryptionMaterials
trace_info = MasterKeyInfo(provider_id=_PROVIDER_ID, key_info=self._key_id)
new_materials = copy.copy(encryption_materials)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is it necessary to copy the materials?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's not strictly necessary, but I think that having access to the original materials inside that scope could help with debugging if things aren't doing what people thing they should.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe this is a Python subtlety I'm missing, but aren't the original materials already available through the "encryption_materials" input?

Copy link
Member Author

Choose a reason for hiding this comment

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

The flow is:

if no data key:
    data key, EDK = generate()
    new materials = materials(with data key)
else:
    EDK = encrypt(data key)

new materials = materials with data key(with EDK)

I went with that flow so that we could single-source the "with EDK" bit, but because of that, if we want to retain an available reference to the materials in the request we have a choice: we can either copy the request materials first:

copied materials = copy(request materials)

if no data key:
    data key, EDK = generate()
    copied materials = copied materials(with data key)
else:
    EDK = encrypt(data key)

copied materials = copied materials(with EDK)

or we can balance a null value

new materials = None

if no data key:
    data key, EDK = generate()
    new materials = request materials(with data key)
else:
    EDK = encrypt(data key)

if new materials is None:
    new materials = request materials(with EDK)
else:
    new materials = new materials(with EDK)

The copy flow seems cleaner to me.

Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't work?

new materials = request materials

if no data key:
    data key, EDK = generate()
    new materials = new materials(with data key)
else:
    EDK = encrypt(data key)

new materials = new materials(with EDK)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I guess that would be fine. It feels less explicit, but since we're only ever using the immutable APIs it would effectively be doing the same thing.

Copy link
Contributor

Choose a reason for hiding this comment

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

That's how I did it in Java, but I'll defer to your judgement on what you prefer for Python

try:
if encryption_materials.data_encryption_key is None:
if new_materials.data_encryption_key is None:
plaintext_key, encrypted_key = _do_aws_kms_generate_data_key(
client_supplier=self._client_supplier,
key_name=self._key_id,
encryption_context=encryption_materials.encryption_context,
algorithm=encryption_materials.algorithm,
encryption_context=new_materials.encryption_context,
algorithm=new_materials.algorithm,
grant_tokens=self._grant_tokens,
)
encryption_materials.add_data_encryption_key(
new_materials = new_materials.with_data_encryption_key(
data_encryption_key=plaintext_key,
keyring_trace=KeyringTrace(wrapping_key=trace_info, flags=_GENERATE_FLAGS),
)
else:
encrypted_key = _do_aws_kms_encrypt(
client_supplier=self._client_supplier,
key_name=self._key_id,
plaintext_data_key=encryption_materials.data_encryption_key,
encryption_context=encryption_materials.encryption_context,
plaintext_data_key=new_materials.data_encryption_key,
encryption_context=new_materials.encryption_context,
grant_tokens=self._grant_tokens,
)
except Exception: # pylint: disable=broad-except
Expand All @@ -207,12 +209,10 @@ def on_encrypt(self, encryption_materials):
_LOGGER.exception(message)
raise EncryptKeyError(message)

encryption_materials.add_encrypted_data_key(
return new_materials.with_encrypted_data_key(
encrypted_data_key=encrypted_key, keyring_trace=KeyringTrace(wrapping_key=trace_info, flags=_ENCRYPT_FLAGS)
)

return encryption_materials

def on_decrypt(self, decryption_materials, encrypted_data_keys):
# type: (DecryptionMaterials, Iterable[EncryptedDataKey]) -> DecryptionMaterials
for edk in encrypted_data_keys:
Expand Down Expand Up @@ -293,14 +293,12 @@ def _try_aws_kms_decrypt(client_supplier, decryption_materials, grant_tokens, en
except Exception: # pylint: disable=broad-except
# We intentionally WANT to catch all exceptions here
_LOGGER.exception("Unable to decrypt encrypted data key from %s", encrypted_data_key.key_provider)
else:
decryption_materials.add_data_encryption_key(
data_encryption_key=plaintext_key,
keyring_trace=KeyringTrace(wrapping_key=encrypted_data_key.key_provider, flags=_DECRYPT_FLAGS),
)
return decryption_materials

return decryption_materials
return decryption_materials.with_data_encryption_key(
data_encryption_key=plaintext_key,
keyring_trace=KeyringTrace(wrapping_key=encrypted_data_key.key_provider, flags=_DECRYPT_FLAGS),
)


def _do_aws_kms_decrypt(client_supplier, key_name, encrypted_data_key, encryption_context, grant_tokens):
Expand Down
14 changes: 9 additions & 5 deletions src/aws_encryption_sdk/keyrings/multi.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# Copyright Amazon.com Inc. or its affiliates. All Rights Reserved.
# SPDX-License-Identifier: Apache-2.0
"""Resources required for Multi Keyrings."""
import copy
Copy link
Contributor

Choose a reason for hiding this comment

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

Needed?

import itertools

import attr
Expand Down Expand Up @@ -67,20 +68,21 @@ def on_encrypt(self, encryption_materials):
"and encryption materials do not already contain a plaintext data key."
)

new_materials = copy.copy(encryption_materials)

# Call on_encrypt on the generator keyring if it is provided
if self.generator is not None:

encryption_materials = self.generator.on_encrypt(encryption_materials=encryption_materials)
new_materials = self.generator.on_encrypt(encryption_materials=new_materials)

# Check if data key is generated
if encryption_materials.data_encryption_key is None:
if new_materials.data_encryption_key is None:
raise GenerateKeyError("Unable to generate data encryption key.")

# Call on_encrypt on all other keyrings
for keyring in self.children:
encryption_materials = keyring.on_encrypt(encryption_materials=encryption_materials)
new_materials = keyring.on_encrypt(encryption_materials=new_materials)

return encryption_materials
return new_materials

def on_decrypt(self, decryption_materials, encrypted_data_keys):
# type: (DecryptionMaterials, Iterable[EncryptedDataKey]) -> DecryptionMaterials
Expand All @@ -95,7 +97,9 @@ def on_decrypt(self, decryption_materials, encrypted_data_keys):
for keyring in self._decryption_keyrings:
if decryption_materials.data_encryption_key is not None:
return decryption_materials

decryption_materials = keyring.on_decrypt(
decryption_materials=decryption_materials, encrypted_data_keys=encrypted_data_keys
)

return decryption_materials
100 changes: 57 additions & 43 deletions src/aws_encryption_sdk/keyrings/raw.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# Copyright Amazon.com Inc. or its affiliates. All Rights Reserved.
# SPDX-License-Identifier: Apache-2.0
"""Resources required for Raw Keyrings."""
import copy
import logging
import os

Expand All @@ -11,7 +12,7 @@
from cryptography.hazmat.primitives import serialization
from cryptography.hazmat.primitives.asymmetric.rsa import RSAPrivateKey, RSAPublicKey

from aws_encryption_sdk.exceptions import GenerateKeyError
from aws_encryption_sdk.exceptions import EncryptKeyError, GenerateKeyError
from aws_encryption_sdk.identifiers import EncryptionKeyType, KeyringTraceFlag, WrappingAlgorithm
from aws_encryption_sdk.internal.crypto.wrapping_keys import EncryptedData, WrappingKey
from aws_encryption_sdk.internal.formatting.deserialize import deserialize_wrapped_key
Expand All @@ -35,12 +36,13 @@ def _generate_data_key(
encryption_materials, # type: EncryptionMaterials
key_provider, # type: MasterKeyInfo
):
# type: (...) -> bytes
# type: (...) -> EncryptionMaterials
"""Generates plaintext data key for the keyring.

:param EncryptionMaterials encryption_materials: Encryption materials for the keyring to modify.
:param MasterKeyInfo key_provider: Information about the key in the keyring.
:return bytes: Plaintext data key
:rtype: EncryptionMaterials
:returns: Encryption materials containins data encryption key
"""
# Check if encryption materials contain data encryption key
if encryption_materials.data_encryption_key is not None:
Expand All @@ -60,10 +62,9 @@ def _generate_data_key(
# plaintext_data_key to RawDataKey
data_encryption_key = RawDataKey(key_provider=key_provider, data_key=plaintext_data_key)

# Add generated data key to encryption_materials
encryption_materials.add_data_encryption_key(data_encryption_key, keyring_trace)

return plaintext_data_key
return encryption_materials.with_data_encryption_key(
data_encryption_key=data_encryption_key, keyring_trace=keyring_trace
)


@attr.s
Expand Down Expand Up @@ -123,17 +124,20 @@ def on_encrypt(self, encryption_materials):
"""Generate a data key if not present and encrypt it using any available wrapping key

:param EncryptionMaterials encryption_materials: Encryption materials for the keyring to modify
:returns: Optionally modified encryption materials
:returns: Encryption materials containing data key and encrypted data key
:rtype: EncryptionMaterials
"""
if encryption_materials.data_encryption_key is None:
_generate_data_key(encryption_materials=encryption_materials, key_provider=self._key_provider)
new_materials = copy.copy(encryption_materials)

if new_materials.data_encryption_key is None:
# Get encryption materials with a new data key.
new_materials = _generate_data_key(encryption_materials=new_materials, key_provider=self._key_provider)

try:
# Encrypt data key
encrypted_wrapped_key = self._wrapping_key_structure.encrypt(
plaintext_data_key=encryption_materials.data_encryption_key.data_key,
encryption_context=encryption_materials.encryption_context,
plaintext_data_key=new_materials.data_encryption_key.data_key,
encryption_context=new_materials.encryption_context,
)

# EncryptedData to EncryptedDataKey
Expand All @@ -144,26 +148,25 @@ def on_encrypt(self, encryption_materials):
encrypted_wrapped_key=encrypted_wrapped_key,
)
except Exception: # pylint: disable=broad-except
error_message = "Raw AES Keyring unable to encrypt data key"
error_message = "Raw AES keyring unable to encrypt data key"
_LOGGER.exception(error_message)
return encryption_materials
raise EncryptKeyError(error_message)

# Update Keyring Trace
keyring_trace = KeyringTrace(
wrapping_key=encrypted_data_key.key_provider, flags={KeyringTraceFlag.ENCRYPTED_DATA_KEY}
wrapping_key=self._key_provider,
flags={KeyringTraceFlag.ENCRYPTED_DATA_KEY, KeyringTraceFlag.SIGNED_ENCRYPTION_CONTEXT},
)

# Add encrypted data key to encryption_materials
encryption_materials.add_encrypted_data_key(encrypted_data_key=encrypted_data_key, keyring_trace=keyring_trace)
return encryption_materials
return new_materials.with_encrypted_data_key(encrypted_data_key=encrypted_data_key, keyring_trace=keyring_trace)

def on_decrypt(self, decryption_materials, encrypted_data_keys):
# type: (DecryptionMaterials, Iterable[EncryptedDataKey]) -> DecryptionMaterials
"""Attempt to decrypt the encrypted data keys.

:param DecryptionMaterials decryption_materials: Decryption materials for the keyring to modify
:param List[EncryptedDataKey] encrypted_data_keys: List of encrypted data keys
:returns: Optionally modified decryption materials
:returns: Decryption materials that MAY include a plaintext data key
:rtype: DecryptionMaterials
"""
if decryption_materials.data_encryption_key is not None:
Expand All @@ -173,9 +176,6 @@ def on_decrypt(self, decryption_materials, encrypted_data_keys):
expected_key_info_len = len(self._key_info_prefix) + self._wrapping_algorithm.algorithm.iv_len
for key in encrypted_data_keys:

if decryption_materials.data_encryption_key is not None:
return decryption_materials

if (
key.key_provider.provider_id != self._key_provider.provider_id
or len(key.key_provider.key_info) != expected_key_info_len
Expand All @@ -196,16 +196,25 @@ def on_decrypt(self, decryption_materials, encrypted_data_keys):
)

except Exception: # pylint: disable=broad-except
# We intentionally WANT to catch all exceptions here
error_message = "Raw AES Keyring unable to decrypt data key"
_LOGGER.exception(error_message)
return decryption_materials
# The Raw AES keyring MUST evaluate every encrypted data key
# until it either succeeds or runs out of encrypted data keys.
continue

# Create a keyring trace
keyring_trace = KeyringTrace(wrapping_key=self._key_provider, flags={KeyringTraceFlag.DECRYPTED_DATA_KEY})
keyring_trace = KeyringTrace(
wrapping_key=self._key_provider,
flags={KeyringTraceFlag.DECRYPTED_DATA_KEY, KeyringTraceFlag.VERIFIED_ENCRYPTION_CONTEXT},
)

# Update decryption materials
data_encryption_key = RawDataKey(key_provider=self._key_provider, data_key=plaintext_data_key)
decryption_materials.add_data_encryption_key(data_encryption_key, keyring_trace)

return decryption_materials.with_data_encryption_key(
data_encryption_key=data_encryption_key, keyring_trace=keyring_trace
)

return decryption_materials

Expand Down Expand Up @@ -331,22 +340,24 @@ def on_encrypt(self, encryption_materials):
and encrypt it using any available wrapping key in any child keyring.

:param EncryptionMaterials encryption_materials: Encryption materials for keyring to modify.
:returns: Optionally modified encryption materials.
:returns: Encryption materials containing data key and encrypted data key
:rtype: EncryptionMaterials
"""
if encryption_materials.data_encryption_key is None:
_generate_data_key(encryption_materials=encryption_materials, key_provider=self._key_provider)
new_materials = copy.copy(encryption_materials)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
new_materials = copy.copy(encryption_materials)
new_materials = encryption_materials


if new_materials.data_encryption_key is None:
new_materials = _generate_data_key(encryption_materials=new_materials, key_provider=self._key_provider)

if self._public_wrapping_key is None:
return encryption_materials
# This should be impossible, but just in case, give a useful error message.
raise EncryptKeyError("Raw RSA keyring unable to encrypt data key: no public key available")

try:
# Encrypt data key
encrypted_wrapped_key = EncryptedData(
iv=None,
ciphertext=self._public_wrapping_key.encrypt(
plaintext=encryption_materials.data_encryption_key.data_key,
padding=self._wrapping_algorithm.padding,
plaintext=new_materials.data_encryption_key.data_key, padding=self._wrapping_algorithm.padding,
),
tag=None,
)
Expand All @@ -359,19 +370,15 @@ def on_encrypt(self, encryption_materials):
encrypted_wrapped_key=encrypted_wrapped_key,
)
except Exception: # pylint: disable=broad-except
error_message = "Raw RSA Keyring unable to encrypt data key"
error_message = "Raw RSA keyring unable to encrypt data key"
_LOGGER.exception(error_message)
return encryption_materials
raise EncryptKeyError(error_message)

# Update Keyring Trace
keyring_trace = KeyringTrace(
wrapping_key=encrypted_data_key.key_provider, flags={KeyringTraceFlag.ENCRYPTED_DATA_KEY}
)
keyring_trace = KeyringTrace(wrapping_key=self._key_provider, flags={KeyringTraceFlag.ENCRYPTED_DATA_KEY})

# Add encrypted data key to encryption_materials
encryption_materials.add_encrypted_data_key(encrypted_data_key=encrypted_data_key, keyring_trace=keyring_trace)

return encryption_materials
return new_materials.with_encrypted_data_key(encrypted_data_key=encrypted_data_key, keyring_trace=keyring_trace)

def on_decrypt(self, decryption_materials, encrypted_data_keys):
# type: (DecryptionMaterials, Iterable[EncryptedDataKey]) -> DecryptionMaterials
Expand All @@ -380,18 +387,20 @@ def on_decrypt(self, decryption_materials, encrypted_data_keys):
:param DecryptionMaterials decryption_materials: Decryption materials for keyring to modify.
:param encrypted_data_keys: List of encrypted data keys.
:type: List[EncryptedDataKey]
:returns: Optionally modified decryption materials.
:returns: Decryption materials that MAY include a plaintext data key
:rtype: DecryptionMaterials
"""
if decryption_materials.data_encryption_key is not None:
return decryption_materials

if self._private_wrapping_key is None:
return decryption_materials

# Decrypt data key
for key in encrypted_data_keys:
if decryption_materials.data_encryption_key is not None:
return decryption_materials
if key.key_provider != self._key_provider:
continue

# Wrapped EncryptedDataKey to deserialized EncryptedData
encrypted_wrapped_key = deserialize_wrapped_key(
wrapping_algorithm=self._wrapping_algorithm, wrapping_key_id=self.key_name, wrapped_encrypted_key=key
Expand All @@ -403,13 +412,18 @@ def on_decrypt(self, decryption_materials, encrypted_data_keys):
except Exception: # pylint: disable=broad-except
error_message = "Raw RSA Keyring unable to decrypt data key"
_LOGGER.exception(error_message)
# The Raw RSA keyring MUST evaluate every encrypted data key
# until it either succeeds or runs out of encrypted data keys.
continue

# Create a keyring trace
keyring_trace = KeyringTrace(wrapping_key=self._key_provider, flags={KeyringTraceFlag.DECRYPTED_DATA_KEY})

# Update decryption materials
data_encryption_key = RawDataKey(key_provider=self._key_provider, data_key=plaintext_data_key)
decryption_materials.add_data_encryption_key(data_encryption_key, keyring_trace)

return decryption_materials.with_data_encryption_key(
data_encryption_key=data_encryption_key, keyring_trace=keyring_trace
)

return decryption_materials
Loading