From 6b73e1348e8a633e5f04662792b9cb5808aab983 Mon Sep 17 00:00:00 2001 From: mattsb42-aws Date: Tue, 24 Mar 2020 21:35:34 -0700 Subject: [PATCH 1/2] feat: change KMS discovery keyring configuration path --- .../keyrings/aws_kms/__init__.py | 34 +++++++++++++------ test/unit/keyrings/test_aws_kms.py | 13 ++++--- 2 files changed, 32 insertions(+), 15 deletions(-) diff --git a/src/aws_encryption_sdk/keyrings/aws_kms/__init__.py b/src/aws_encryption_sdk/keyrings/aws_kms/__init__.py index b8cb94b81..1472b2964 100644 --- a/src/aws_encryption_sdk/keyrings/aws_kms/__init__.py +++ b/src/aws_encryption_sdk/keyrings/aws_kms/__init__.py @@ -55,10 +55,13 @@ class KmsKeyring(Keyring): but for the keyring to attempt to use them on decrypt you MUST specify the CMK ARN. - If you specify neither ``generator_key_id`` nor ``child_key_ids`` - then the keyring will operate in `discovery mode`_, + If you specify ``is_discovery=True`` the keyring will be a KMS discovery keyring, doing nothing on encrypt and attempting to decrypt any AWS KMS-encrypted data key on decrypt. + .. notice:: + + You must either set ``is_discovery=True`` or provide key IDs. + You can use the :class:`ClientSupplier` to customize behavior further, such as to provide different credentials for different regions or to restrict which regions are allowed. @@ -75,12 +78,14 @@ class KmsKeyring(Keyring): .. versionadded:: 1.5.0 :param ClientSupplier client_supplier: Client supplier that provides AWS KMS clients (optional) + :param bool is_discovery: Should this be a discovery keyring (optional) :param str generator_key_id: Key ID of AWS KMS CMK to use when generating data keys (optional) :param List[str] child_key_ids: Key IDs that will be used to encrypt and decrypt data keys (optional) :param List[str] grant_tokens: AWS KMS grant tokens to include in requests (optional) """ _client_supplier = attr.ib(default=attr.Factory(DefaultClientSupplier), validator=is_callable()) + _is_discovery = attr.ib(default=False, validator=instance_of(bool)) _generator_key_id = attr.ib(default=None, validator=optional(instance_of(six.string_types))) _child_key_ids = attr.ib( default=attr.Factory(tuple), @@ -93,6 +98,22 @@ class KmsKeyring(Keyring): def __attrs_post_init__(self): """Configure internal keyring.""" + key_ids_provided = self._generator_key_id is not None or self._child_key_ids + both = key_ids_provided and self._is_discovery + neither = not key_ids_provided and not self._is_discovery + + if both: + raise TypeError("is_discovery cannot be True if key IDs are provided") + + if neither: + raise TypeError("No key IDs provided") + + if self._is_discovery: + self._inner_keyring = _AwsKmsDiscoveryKeyring( + client_supplier=self._client_supplier, grant_tokens=self._grant_tokens + ) + return + if self._generator_key_id is None: generator_keyring = None else: @@ -107,14 +128,7 @@ def __attrs_post_init__(self): for key_id in self._child_key_ids ] - self._is_discovery = generator_keyring is None and not child_keyrings - - if self._is_discovery: - self._inner_keyring = _AwsKmsDiscoveryKeyring( - client_supplier=self._client_supplier, grant_tokens=self._grant_tokens - ) - else: - self._inner_keyring = MultiKeyring(generator=generator_keyring, children=child_keyrings) + self._inner_keyring = MultiKeyring(generator=generator_keyring, children=child_keyrings) def on_encrypt(self, encryption_materials): # type: (EncryptionMaterials) -> EncryptionMaterials diff --git a/test/unit/keyrings/test_aws_kms.py b/test/unit/keyrings/test_aws_kms.py index ad2e4b526..113ffc98f 100644 --- a/test/unit/keyrings/test_aws_kms.py +++ b/test/unit/keyrings/test_aws_kms.py @@ -24,6 +24,9 @@ pytest.param(dict(child_key_ids="some stuff"), id="child_key_ids is a string"), pytest.param(dict(grant_tokens=("foo", 5)), id="grant_tokens contains invalid values"), pytest.param(dict(grant_tokens="some stuff"), id="grant_tokens is a string"), + pytest.param(dict(generator_key_id="foo", is_discovery=True), id="generator and discovery"), + pytest.param(dict(child_key_ids=("foo",), is_discovery=True), id="child_key_ids and discovery"), + pytest.param(dict(), id="nothing"), ), ) def test_kms_keyring_invalid_parameters(kwargs): @@ -100,7 +103,7 @@ def test_kms_keyring_builds_correct_inner_keyring_discovery(): grants = ("asdf", "fdas") supplier = DefaultClientSupplier() - test = KmsKeyring(grant_tokens=grants, client_supplier=supplier) + test = KmsKeyring(is_discovery=True, grant_tokens=grants, client_supplier=supplier) # We specified neither a generator nor children, so the inner keyring MUST be a discovery keyring assert isinstance(test._inner_keyring, _AwsKmsDiscoveryKeyring) @@ -110,10 +113,10 @@ def test_kms_keyring_builds_correct_inner_keyring_discovery(): assert test._inner_keyring._client_supplier is supplier -def test_kms_keyring_on_encrypt(mocker): +def test_kms_keyring_inner_keyring_on_encrypt(mocker): mock_keyring = mocker.Mock() - keyring = KmsKeyring() + keyring = KmsKeyring(is_discovery=True) keyring._inner_keyring = mock_keyring test = keyring.on_encrypt(encryption_materials=mocker.sentinel.encryption_materials) @@ -123,10 +126,10 @@ def test_kms_keyring_on_encrypt(mocker): assert test is mock_keyring.on_encrypt.return_value -def test_kms_keyring_on_decrypt(mocker): +def test_kms_keyring_inner_keyring_on_decrypt(mocker): mock_keyring = mocker.Mock() - keyring = KmsKeyring() + keyring = KmsKeyring(is_discovery=True) keyring._inner_keyring = mock_keyring test = keyring.on_decrypt( From fb2ada95948ba0243423200479126d81209bb899 Mon Sep 17 00:00:00 2001 From: mattsb42-aws Date: Wed, 25 Mar 2020 13:54:32 -0700 Subject: [PATCH 2/2] fix: clarify error message when insufficient configuration is provided to KmsKeyring --- src/aws_encryption_sdk/keyrings/aws_kms/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/aws_encryption_sdk/keyrings/aws_kms/__init__.py b/src/aws_encryption_sdk/keyrings/aws_kms/__init__.py index 1472b2964..3630644c6 100644 --- a/src/aws_encryption_sdk/keyrings/aws_kms/__init__.py +++ b/src/aws_encryption_sdk/keyrings/aws_kms/__init__.py @@ -106,7 +106,7 @@ def __attrs_post_init__(self): raise TypeError("is_discovery cannot be True if key IDs are provided") if neither: - raise TypeError("No key IDs provided") + raise TypeError("is_discovery cannot be False if no key IDs are provided") if self._is_discovery: self._inner_keyring = _AwsKmsDiscoveryKeyring(