KMS: RotateKeyOnDemand api update #12806
Conversation
|
All contributors have signed the CLA ✍️ ✅ |
|
I have read the CLA Document and I hereby sign the CLA |
|
recheck |
There was a problem hiding this comment.
Hi @willdefig thank you for your contribution 🚀
As mentioned in my comment below, I believe it’s important to add the test case described in the article to check that this implementation aligns with AWS behavior. Without this, the behavior may not fully match AWS KMS behavior.
tests/aws/services/kms/test_kms.py
Outdated
| def test_rotate_key_on_demand_succeeds_for_key_with_imported_key_material( | ||
| self, kms_create_key, aws_client, snapshot | ||
| ): | ||
| key_id = kms_create_key(Origin="EXTERNAL")["KeyId"] | ||
|
|
||
| with pytest.raises(ClientError) as e: | ||
| aws_client.kms.rotate_key_on_demand(KeyId=key_id) | ||
| snapshot.match("error-response", e.value.response) | ||
| response = aws_client.kms.rotate_key_on_demand(KeyId=key_id) | ||
| snapshot.match("rotate-on-demand-response", response) |
There was a problem hiding this comment.
Could you add an integration test to confirm that a KMS key with imported key material can be created and used, and that after rotating the key material using the rotate_key_on_demand operation, the KMS key continues to work correctly for both encryption and decryption? The test should also verify that data encrypted with the previous key material can still be successfully decrypted.
This test case is described in this AWS blog post about using on-demand rotation for imported keys.
- Creates Key - Imports custom Key Material - Creates Encrypted data key - Tests it can use it to decrypt - Creates new key material - Imports new material - Rotates new material - Creates new encrypted key data - Tests it can decrypt that - Tests it can still decrypt old key data
- updated test to include checking rotated keys are still usable. - Tidied up import_key_material importing for EXTERNAL keys - rotate_key_on_demand stopped key being recreated if using EXTERNAL keys
- Fix for TestKMS::test_key_rotations_limits failing due to the last change
|
Currently, only patch changes are allowed on master. Your PR labels (aws:kms, semver: minor) indicate that it cannot be merged into the master at this time. |
| def test_rotate_key_on_demand_succeeds_for_key_with_imported_key_material( | ||
| self, kms_create_key, aws_client, snapshot | ||
| ): | ||
| key_id = kms_create_key(Origin="EXTERNAL")["KeyId"] | ||
| key_id = kms_create_key( | ||
| Origin="EXTERNAL", KeySpec="SYMMETRIC_DEFAULT", KeyUsage="ENCRYPT_DECRYPT" | ||
| )["KeyId"] | ||
| response = aws_client.kms.rotate_key_on_demand(KeyId=key_id) | ||
| snapshot.match("rotate-on-demand-response", response) |
There was a problem hiding this comment.
Can we merge this test with the test_key_before_and_after_rotations_on_imported_key test? The logic is the same, aside from the snapshot, which can be added to the test_key_before_and_after_rotations_on_imported_key
| response_data = {"KeyId": key_to_import_material_to.metadata["KeyId"]} | ||
| response_data["KeyMaterialId"] = key_material.hex() |
There was a problem hiding this comment.
You can merge these two lines into one or define values directly in the ImportKeyMaterialResponse
| assert retry( | ||
| function=_verify_key_meta, | ||
| retries=2, | ||
| sleep=1.0, | ||
| aws_client=aws_client, | ||
| key_id=key_id, | ||
| item_key="CurrentKeyMaterialId", | ||
| item_value=first_key_response["KeyMaterialId"], | ||
| ) |
There was a problem hiding this comment.
I think it would be useful to capture a snapshot of the aws_client.kms.describe_key response to record the state both before and after the key material is rotated.
Motivation
In response to the latest changes to KMS that AWS have made it is now possible to rotate keys on demand if they are and external key,
https://aws.amazon.com/blogs/security/how-to-use-on-demand-rotation-for-aws-kms-imported-keys/
Also there is a new item in the metadata for the key to show the CurrentKeyMaterialId this should update when the key is rotated.
Changes
RotateKeyOnDemandto acceptEXTERNALType keysTests
test_rotate_key_on_demand_fails_for_key_with_imported_key_material totest_rotate_key_on_demand_succeeds_for_key_with_imported_key_materialtest_key_before_and_after_rotations_on_imported_keyto fully test the import and rotation of anEXTERNALkeyresolves #12801