chore(java): demo hv-2 does not send prefixed EC in examples #1529
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The key store now allows for both a default DynamoDB table, or any custom storage system. The important aspect about the key store is the fact that branch keys can be versioned easily, and are cryptographically safe to use. The actual storage medium is not important. See: https://github.com/awslabs/aws-encryption-sdk-specification/blob/master/changes/2024-6-17_key-store-persistance/background.md#background
Explicitly: - Change InitializeMutationFlag from a union to an enum for ToString reasons - Model `DoNotVersion` flag for Initialize Mutation - Refactor Describe Mutation output to detail Input so resume can be done - Refactor System Key to be optional, detailing that TrustStorage is the default - More errors - Smithy changes from PR feedback on #854 - Correction of spelling mistakes - Mutation Token's UUID is required Why change the flag to an enum? Dafny/Smithy-Dafny's support for Union's results in structures that do not print well. The intention of the `InitializeMutationFlag` is to inform customers about the result of their request. Such information may, possibly even should, be logged. Initialize Mutation and Apply Mutation MUST ensure that the UUID of the Index and Commitment agree. Apply MUST ensure that the UUID of the Commitment and Token agree. The Mutation Token's UUID is REQUIRED. It is how we track a mutation, much like how CFN tracks a change set. Fixed bug where UUID is a reserved word in DDB. Refactored some of the error messages. Utilize Java Example to demonstrate resume and restart. Finally, addressed some of the feedback on PR #854. feat(Mutations): Idempotent Resume (#854) Refactor Storage: - Rename Mutation Lock to Mutation Commitment - Introduce Mutation Index to describe what items of a Branch Key have been mutated - Add Input field to Mutation Commitment - Add Ciphertext field to Mutation Commitment - When Mutating an item, always write with an optimistic lock - Allow Initialize Mutation to over write a Version, instead of only creating a version - When Overwriting a Mutation Index, ensure it has not changed - Whenever writing for Mutation, ensure the Mutation Commitment's ENC is as expected (along with original and terminal) Refactor Storage to contain operations that: 1. Allow for Atomic Mutations (maybe cut later) 2. Allow for Deleting a Mutation 3. Allow for Creating a Mutation Index Refactor KeyStoreAdmin: - Support a System Key for Mutations - Stub out the System Key - Logic for handling Mutation Index Refactor Initialize Mutation: - If Commitment & Index already exist and match Input, write nothing and return token - If Commitment already exists and matches input, write index, and return token - If Commitment already exists and does not match input, fail - If no commitment, Initialize Mutation Refactor Apply Mutation: - Write an update Page Index. fix: Dafny intendation formatting chore: fix Java test examples chore: more fomratting for CI feat(Mutations): Native test for Loose access in-flight feat(Mutations): Example In Flight Mutation Scanner feat(Mutations-TODO): Some Terminal KMS Exceptions (#795) If the KMS Call, for mutating the Beacon, fails, it MAY indicate the MPL Consumer does not have access to the terminal KMS Key. If the KMS call for verifying a terminal version fails, it MAY indicate the MPL Consumer does not have access to the terminal KMS Key. Also fix some Dafny tests failing verification. feat(KS+): More modeled errors (#754) * refactor(Storage): Use TransactGet instead of BatchGet for GetInitMut (#753) This resolves feedback from both @seebees and @josecorella on Mutations branch. * chore(Mutations): address #754\#discussion_r1775676091 #754 (comment) * chore(Mutations): Address feedback from #750 See #750 * chore(Mutations): address feedback on #742 See #742 refactor(Storage): Use TransactGet instead of BatchGet for GetInitMut (#753) This resolves feedback from both @seebees and @josecorella on Mutations branch. Verification failure is due to (just like in #751 ): 1. Key Store Admin's Index.dfy not proving Fresh, which has been a long standing issue with the Admin 2. In `AwsCryptographyKeyStore/test/Storage/TestGetItemsForInitializeMutation.dfy`, the test assumes `type` is in the Encryption Context. It is, but Structure.dfy does not prove that, so it must be proven else where. A .NET MPL job failed due to .NET 48 Sig V4 Auth bug. test(Mutations): additional testing for Mutations feat(Mutations): more clean up feat(Mutations): log statements for ApplyMutation fix(Storage): typo fix(Storage): ensure KmsArn is valid chore(Mutations): respond to comments on PR #720 test(GHW): Run Java Examples for PR CI (#749) chore(Java): Examples for Mutations (#742) chore(Mutations): comment out non-GA Mutations (#750) Crypto Tools, at this time, intends to release Branch Key Mutations without some operations useful for recovering a dropped Mutation Token or dealing with the disagreement of a Mutation Token and a Mutation Lock. Additionally, we intend to release the Key Store Admin with only support for one Key Management Strategy. All checks are green except for Verification. Verification is failing for the Key Store Admin's Index.dfy. Verification for the Key Store Admin's Index.dfy has been failing for a long time. feat: Mutations BETA (#720) Beta build of Mutations with several substantial gaps
There are still more refactoring we could do in MutationsErrorRefinement, but this should be sufficient for now.
Co-authored-by: Jose Corella <[email protected]>
Co-authored-by: José Corella <[email protected]>
* fix(KSA): Describe Mutation bugs * fix(Java): Always clean up test resources
Fixes: - Wrong assertion for Java Example Test of In-Flight Mutation scanner - Initialize Mutation, when using Decrypt/Encrypt, must use Decrypt/Encrypt to mutate the Beacon Tests: - Explicitly Test Decrypt/Encrypt with different credentials that only can use the relevant KMS Keys - Refactor Examples to be easier to test across inputs Other: - Add Documentation to the Java Examples Co-authored-by: José Corella <[email protected]>
#1081) * test(KSA-Java): explicitly check for deletion of Index and Commitment at end of Mutation * chore: polish examples
This ensures that our implementation is correct, as if it was not, the incorrect KMS Client would be used, and the request would fail.
- Polish Documentation for Mutations and the Key Store Admin - Make the System Key parameter required for Initialize and Apply - Improve Java Example Tests to assert that all items of a Branch Key are use-able post Mutation and require the correct KMS Key.
Co-authored-by: Tony Knapp <[email protected]>
Co-authored-by: Tony Knapp <[email protected]>
Co-authored-by: Darwin Chowdary <[email protected]>
I am By passing a CI failure in Java . It is with Mutations, one of the three Java tests failed: ``` TestMutateHierarchyVersion.TestHV1toHV2HappyCaseDecryptEncrypt: FAILED dafny/AwsCryptographyKeyStoreAdmin/test/Mutations/TestMutationHappyPath.dfy(313,6): Actual customer send EC and expected customer send EC did not match. ``` We should look into this...
rishav-karanjit
commented
May 20, 2025
...ples/java/src/main/java/software/amazon/cryptography/example/hierarchy/CreateKeyExample.java
Outdated
Show resolved
Hide resolved
texastony
requested changes
May 20, 2025
Comment on lines
104
to
134
| // HV2 sends the encryption context without any transformation. | ||
| // We have a kms key `Fixtures.KMS_KEY_FOR_HV2_ONLY`, that requires EC to be exactly {"Robbie": "Is a Dog."} in its key policy. | ||
| // For demostration, we will create a key with a different EC then the one that is expected and see it fail. | ||
| final Map<String, String> encryptionContextFailingCase = | ||
| Collections.singletonMap("I", "am not a Dog."); | ||
| boolean exceptionThrown = false; | ||
| try { | ||
| _admin | ||
| .CreateKey( | ||
| CreateKeyInput | ||
| .builder() | ||
| // This is the KMS ARN that will be used to protect the Branch Key. | ||
| // It is a required argument. | ||
| .KmsArn(KmsSymmetricKeyArn.builder().KmsKeyArn(kmsKeyArn).build()) | ||
| // If you need to specify the Identifier for a Branch Key, you may. | ||
| // This is an optional argument. | ||
| .Identifier(branchKeyId) | ||
| // If a branch key Identifier is provided, | ||
| // custom encryption context MUST be provided as well. | ||
| .EncryptionContext(encryptionContextFailingCase) | ||
| // The Branch Key Store Admin can create HV-1 or HV-2 Branch Keys | ||
| .HierarchyVersion(_hierarchyVersion) | ||
| // But the Strategy MUST support the Hierarchy Version | ||
| .Strategy(strategy) | ||
| .build() | ||
| ) | ||
| .Identifier(); | ||
| } catch (KmsException e) { | ||
| exceptionThrown = true; | ||
| } | ||
| assert exceptionThrown; |
There was a problem hiding this comment.
Blocking: Just move all of this to a Test.
Do not keep it in line with the examples, that confuses customers, and complicates this CreateKey method that we use through out the Example Testing.
|
Closing as Tony did a similar test. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue #, if available:
Description of changes:
Squash/merge commit message, if applicable:
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.