Skip to content

feat: change KMS discovery keyring configuration path #228

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.

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
Mar 25, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
34 changes: 24 additions & 10 deletions src/aws_encryption_sdk/keyrings/aws_kms/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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),
Expand All @@ -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("is_discovery cannot be False if no key IDs are 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:
Expand All @@ -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
Expand Down
13 changes: 8 additions & 5 deletions test/unit/keyrings/test_aws_kms.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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(
Expand Down