Skip to content

fix: correct KMS keyring parameter name from child_key_ids to key_ids #240

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
Apr 8, 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
8 changes: 4 additions & 4 deletions examples/src/keyring/aws_kms/multiple_regions.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,16 +46,16 @@ def run(aws_kms_generator_cmk, aws_kms_additional_cmks, source_plaintext):
}

# Create the keyring that will encrypt your data keys under all requested CMKs.
many_cmks_keyring = KmsKeyring(generator_key_id=aws_kms_generator_cmk, child_key_ids=aws_kms_additional_cmks)
many_cmks_keyring = KmsKeyring(generator_key_id=aws_kms_generator_cmk, key_ids=aws_kms_additional_cmks)

# Create keyrings that each only use one of the CMKs.
# We will use these later to demonstrate that any of the CMKs can be used to decrypt the message.
#
# We provide these in "child_key_ids" rather than "generator_key_id"
# We provide these in "key_ids" rather than "generator_key_id"
# so that these keyrings cannot be used to generate a new data key.
# We will only be using them on decrypt.
single_cmk_keyring_that_generated = KmsKeyring(child_key_ids=[aws_kms_generator_cmk])
single_cmk_keyring_that_encrypted = KmsKeyring(child_key_ids=[aws_kms_additional_cmks[0]])
single_cmk_keyring_that_generated = KmsKeyring(key_ids=[aws_kms_generator_cmk])
single_cmk_keyring_that_encrypted = KmsKeyring(key_ids=[aws_kms_additional_cmks[0]])

# Encrypt your plaintext data using the keyring that uses all requests CMKs.
ciphertext, encrypt_header = aws_encryption_sdk.encrypt(
Expand Down
12 changes: 6 additions & 6 deletions src/aws_encryption_sdk/keyrings/aws_kms/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,10 @@ class KmsKeyring(Keyring):
Set ``generator_key_id`` to require that the keyring use that CMK to generate the data key.
If you do not set ``generator_key_id``, the keyring will not generate a data key.

Set ``child_key_ids`` to specify additional CMKs that the keyring will use to encrypt the data key.
Set ``key_ids`` to specify additional CMKs that the keyring will use to encrypt the data key.

The keyring will attempt to use any CMKs
identified by CMK ARN in either ``generator_key_id`` or ``child_key_ids`` on decrypt.
identified by CMK ARN in either ``generator_key_id`` or ``key_ids`` on decrypt.

You can identify CMKs by any `valid key ID`_ for the keyring to use on encrypt,
but for the keyring to attempt to use them on decrypt
Expand Down Expand Up @@ -82,14 +82,14 @@ class KmsKeyring(Keyring):
: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] 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(
_key_ids = attr.ib(
default=attr.Factory(tuple),
validator=(deep_iterable(member_validator=instance_of(six.string_types)), value_is_not_a_string),
)
Expand All @@ -100,7 +100,7 @@ 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
key_ids_provided = self._generator_key_id is not None or self._key_ids
both = key_ids_provided and self._is_discovery
neither = not key_ids_provided and not self._is_discovery

Expand All @@ -127,7 +127,7 @@ def __attrs_post_init__(self):
_AwsKmsSingleCmkKeyring(
key_id=key_id, client_supplier=self._client_supplier, grant_tokens=self._grant_tokens
)
for key_id in self._child_key_ids
for key_id in self._key_ids
]

self._inner_keyring = MultiKeyring(generator=generator_keyring, children=child_keyrings)
Expand Down
2 changes: 1 addition & 1 deletion test/functional/keyrings/aws_kms/test_aws_kms.py
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ def test_aws_kms_single_cmk_keyring_on_decrypt_single_cmk(fake_generator):
def test_aws_kms_single_cmk_keyring_on_decrypt_multiple_cmk(fake_generator_and_child):
generator, child = fake_generator_and_child

encrypting_keyring = KmsKeyring(generator_key_id=generator, child_key_ids=(child,))
encrypting_keyring = KmsKeyring(generator_key_id=generator, key_ids=(child,))
decrypting_keyring = _AwsKmsSingleCmkKeyring(key_id=child, client_supplier=DefaultClientSupplier())

initial_encryption_materials = EncryptionMaterials(algorithm=ALGORITHM, encryption_context={})
Expand Down
2 changes: 1 addition & 1 deletion test/integration/integration_test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ def build_aws_kms_keyring(generate=True, cache=True):
if generate:
kwargs = dict(generator_key_id=cmk_arn)
else:
kwargs = dict(child_key_ids=[cmk_arn])
kwargs = dict(key_ids=[cmk_arn])

keyring = KmsKeyring(**kwargs)

Expand Down
10 changes: 5 additions & 5 deletions test/unit/keyrings/test_aws_kms.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,12 @@
(
pytest.param(dict(client_supplier=None), id="client_supplier is invalid"),
pytest.param(dict(generator_key_id=5), id="generator_id is invalid"),
pytest.param(dict(child_key_ids=("foo", 5)), id="child_key_ids contains invalid values"),
pytest.param(dict(child_key_ids="some stuff"), id="child_key_ids is a string"),
pytest.param(dict(key_ids=("foo", 5)), id="key_ids contains invalid values"),
pytest.param(dict(key_ids="some stuff"), id="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(key_ids=("foo",), is_discovery=True), id="key_ids and discovery"),
pytest.param(dict(), id="nothing"),
),
)
Expand All @@ -43,7 +43,7 @@ def test_kms_keyring_builds_correct_inner_keyring_multikeyring():

test = KmsKeyring(
generator_key_id=generator_id,
child_key_ids=(child_id_1, child_id_2),
key_ids=(child_id_1, child_id_2),
grant_tokens=grants,
client_supplier=supplier,
)
Expand Down Expand Up @@ -74,7 +74,7 @@ def test_kms_keyring_builds_correct_inner_keyring_multikeyring():


def test_kms_keyring_builds_correct_inner_keyring_multikeyring_no_generator():
test = KmsKeyring(child_key_ids=("bar", "baz"))
test = KmsKeyring(key_ids=("bar", "baz"))

# We specified child IDs, so the inner keyring MUST be a multikeyring
assert isinstance(test._inner_keyring, MultiKeyring)
Expand Down