Additional example code for Keyrings #155

WesleyRosenblum · 2020-02-05T21:08:41Z

Description of changes:

Adding example code for Keyrings, as well as updating and adding the data key caching example code from the Documentation.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Check any applicable:

Were any files moved? Moving files changes their URL, which breaks all hyperlinks to the files.

juneb

Not finished. I'll finish tomorrow.

src/examples/java/com/amazonaws/crypto/examples/RawAesKeyringExample.java

juneb · 2020-02-05T23:28:46Z

src/examples/java/com/amazonaws/crypto/examples/RawAesKeyringExample.java

+     * In practice, this key would be saved in a secure location.
+     * For this demo, we generate a new random key for each operation.
+     */
+    private static SecretKey retrieveEncryptionKey() {


Since you're not retrieving anything from anywhere, I would call this generateEncryptKey. Do we need a warning that this is just a proof of concept or is this really a good, secure way to generate a key? If it is, I would say so.

according to the Java documentation it is a "cryptographically strong random number generator." so it should be fine but I'm not sure about what wording would make sense here. We could also generate a key in this manner:

KeyGenerator keyGenerator = KeyGenerator.getInstance("AES"); keyGenerator.init(128); return keyGenerator.generateKey();

I'm not sure if that is preferable, @SalusaSecondus do you have thoughts?

Both SecureRandom and KeyGenerator are generally equally good for AES keys. I think that I see SecureRandom slightly more commonly.

src/examples/java/com/amazonaws/crypto/examples/RawAesKeyringExample.java

juneb · 2020-02-05T23:58:26Z

src/examples/java/com/amazonaws/crypto/examples/RawAesKeyringExample.java

+                .keyring(keyring)
+                .ciphertext(ciphertext).build());
+
+        // 7. Before verifying the plaintext, verify that the key that was used in the encryption


Suggested change

// 7. Before verifying the plaintext, verify that the key that was used in the encryption

// 7. Before returning the plaintext, verify that the same encryption key was used in the encryption and decryption operations.

If the encryption key isn't the same, the decrypt fails. I think what we're testing here is that the AES key that we're using is the one that we expect. But the namespace and name are arbitrary, so you can spoof the namespace/name.

We need to explain this more clearly.

This language was adapted from the original BasicEncryptionExample that was using a KmsMasterKeyProvider. I think this verification doesn't make sense for the RawAES key, so I've removed this step. For the AwsKmsKeyring, the Keyring will only use the configured CMKs for decryption (which is different than the KmsMasterKeyProvider) so I'll update the current BasicEncryptionExample with new language related to that

src/examples/java/com/amazonaws/crypto/examples/RawRsaKeyringDecryptExample.java

SalusaSecondus

LGTM

src/examples/java/com/amazonaws/crypto/examples/BasicEncryptionExample.java

juneb · 2020-02-10T20:30:33Z

src/examples/java/com/amazonaws/crypto/examples/BasicEncryptionExample.java

+        // 7. Verify that the encryption context in the result contains the
+        //    data that we expect. The SDK can add values to the encryption context,
+        //    so there may be additional keys in the result context.
+        assert decryptResult.getEncryptionContext().get("ExampleContextKey").equals("ExampleContextValue");


Who is "we?"

To verify that the encryption context used to decrypt the data was the encryption context you expected, use the keyring trace. This helps to ensure that you decrypted the ciphertext that you intended.

When verifying, test that your expected encryption context is a subset of the actual encryption context, not an exact match. The Encryption SDK adds the signing key to the encryption context when appropriate.

I don't think you intended to mention the keyring trace here, I went with:

To verify that the encryption context used to decrypt the data was the encryption context you expected, examine the encryption context in the result. This helps to ensure that you decrypted the ciphertext that you intended.

src/examples/java/com/amazonaws/crypto/examples/FileStreamingExample.java

src/examples/java/com/amazonaws/crypto/examples/RawAesKeyringExample.java

src/examples/java/com/amazonaws/crypto/examples/RawRsaKeyringDecryptExample.java

...examples/java/com/amazonaws/crypto/examples/datakeycaching/LambdaDecryptAndWriteExample.java

...amples/java/com/amazonaws/crypto/examples/datakeycaching/MultiRegionRecordPusherExample.java

juneb · 2020-02-10T20:43:00Z

...amples/java/com/amazonaws/crypto/examples/datakeycaching/MultiRegionRecordPusherExample.java

+                    .generatorKeyId(AwsKmsCmkId.fromString(kmsAliasName)).build());
+        }
+
+        // Collect keyrings into single multi keyring and add cache. The keyring for the


s/multi keyring/multi-keyring

We've instantiated only one keyring (line 75). Where do the keyrings in the childrenKeyrings keyring list come from? (Sorry if that's a dumb q.)

It's instantiating a new Aws Kms Keyring for each iteration of the loop and adding it the list of keyrings.

juneb · 2020-02-10T20:44:19Z

...amples/java/com/amazonaws/crypto/examples/datakeycaching/MultiRegionRecordPusherExample.java

+        }
+
+        // Collect keyrings into single multi keyring and add cache. The keyring for the
+        // first region will be used to generate a data key.


We need to fix this sentence. It sounds like a rule; not an artifact of this particular code. The generatorKeyring is defined as a multiKeyring property.

juneb · 2020-02-10T22:35:04Z

...amples/java/com/amazonaws/crypto/examples/datakeycaching/MultiRegionRecordPusherExample.java

+                    .withRegion(region.getName())
+                    .build());
+
+            keyrings.add(StandardKeyrings.awsKms()


Maybe a comment:
Create a KMS keyring with one CMK that is the generator key.

…#151) * Incorporate Keyrings into AwsCrypto and deprecate MasterKeyProviders. * Update example code to use keyrings * Using try-with-resources for AwsCrypto streams * Splitting MKP and keyring unit tests * Making decryptData with ParsedCiphertext public * Mark KeyStoreProvider as deprecated * Reword some comments on the Basic Encryption example * Add test for compability of Keyrings with MasterKeyProviders * Create individual request types for each AwsCrypto method * Make EncryptionMaterials, DecryptionMaterials and KeyringTrace immutable * Rename KmsKeying and related classes to AwsKmsKeyring * Create builders for the standard keyrings * Create AwsKmsCmkId type to represent AWS KMS Key Ids * Add factory methods to Keyring builders * Add comment on not making a defensive copy of plaintext/ciphertext * Limit ability to create discovery AWS KMS Keyrings to explicit creation * Add withKeyring to CachingCMM builder * Fix DecryptRequestTest * Fix Junit 4 assertions in JUnit5 tests * Renaming StaticKeyring to TestKeyring * Adding convenience methods the create builders internally * Updating wording and adding more Deprecated annotations * Enable AwsKms Client Caching by default to match KmsMasterKeyProvider * Making tests opt-out instead of opt-in and update TestVectorRunner (#154) * Making tests opt-out instead of opt-in and update TestVectorRunner JUnit5 doesn't support test suites yet (see junit-team/junit5#744) and the existing test suites do not support the new JUnit5 tests that are being used for keyrings. This change removes the test suites, and configures Maven to include all tests except those marked with certain JUnit tags. Additionally, this change updates the TestVectorRunner to also test Keyrings and removes the redundant XCompat tests. * Client caching is now enabled by default in AwsKmsClientSupplier * Rename slow tag to ad_hoc and fix TestVectorRunner * Renaming StandardKeyring builder methods and other minors changes * Fixing test * Updating tests to use assertThrows * Additional example code for Keyrings (#155) * Additional example code for Keyrings * Updating wording * Remove AWS from AWS KMS keyring and make keyring lowercase

WesleyRosenblum requested review from juneb and SalusaSecondus February 5, 2020 21:09

mattsb42-aws mentioned this pull request Feb 5, 2020

add keyrings #102

Open

15 tasks

juneb reviewed Feb 6, 2020

View reviewed changes

WesleyRosenblum force-pushed the examplecode branch 2 times, most recently from c798b6e to 0bd2587 Compare February 7, 2020 19:06

SalusaSecondus approved these changes Feb 7, 2020

View reviewed changes

juneb reviewed Feb 10, 2020

View reviewed changes

WesleyRosenblum force-pushed the examplecode branch from 0bd2587 to afa2651 Compare February 11, 2020 00:46

WesleyRosenblum added 4 commits February 11, 2020 14:32

Additional example code for Keyrings

63f362a

Updating wording

18165ff

Updating wording of some examples

0f613c8

Updating wording

24edd13

WesleyRosenblum force-pushed the examplecode branch from afa2651 to 24edd13 Compare February 11, 2020 22:43

ttjsu-aws mentioned this pull request Feb 11, 2020

Revise Examples #108

Closed

15 tasks

juneb approved these changes Feb 11, 2020

View reviewed changes

WesleyRosenblum added 2 commits February 11, 2020 16:33

Remove AWS from AWS KMS keyring and make keyring lowercase

d83b630

Updating wording to match readme

ba3a35e

WesleyRosenblum merged commit 12f0c42 into materialsmanager Feb 12, 2020

WesleyRosenblum deleted the examplecode branch February 12, 2020 01:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Additional example code for Keyrings #155

Additional example code for Keyrings #155

WesleyRosenblum commented Feb 5, 2020

juneb left a comment

juneb Feb 5, 2020

WesleyRosenblum Feb 6, 2020 •

edited

Loading

SalusaSecondus Feb 7, 2020

juneb Feb 5, 2020

juneb Feb 6, 2020

WesleyRosenblum Feb 6, 2020

SalusaSecondus left a comment

juneb Feb 10, 2020

WesleyRosenblum Feb 11, 2020

juneb Feb 10, 2020

juneb Feb 10, 2020

WesleyRosenblum Feb 11, 2020

juneb Feb 10, 2020

WesleyRosenblum Feb 11, 2020

juneb Feb 10, 2020

	// 7. Before verifying the plaintext, verify that the key that was used in the encryption
	// 7. Before returning the plaintext, verify that the same encryption key was used in the encryption and decryption operations.

Additional example code for Keyrings #155

Additional example code for Keyrings #155

Conversation

WesleyRosenblum commented Feb 5, 2020

Check any applicable:

juneb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

WesleyRosenblum Feb 6, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SalusaSecondus left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

WesleyRosenblum Feb 6, 2020 •

edited

Loading