Incorporate Keyrings into AwsCrypto and deprecate MasterKeyProviders. #151

WesleyRosenblum · 2020-01-17T01:54:14Z

Description of changes:

This change incorporates Keyrings into AwsCrypto and deprecates MasterKeys and MasterKeyProviders.

See #150 for a discussion on the evolution of the API as we transition to Keyrings
See #153 for a discussion on the ergonomics of Keyrings

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.

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

src/main/java/com/amazonaws/encryptionsdk/AwsCrypto.java

src/test/java/com/amazonaws/encryptionsdk/DefaultCryptoMaterialsManagerTest.java

src/test/java/com/amazonaws/encryptionsdk/internal/StaticKeyring.java

src/main/java/com/amazonaws/encryptionsdk/AwsCrypto.java

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

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

src/main/java/com/amazonaws/encryptionsdk/AwsCrypto.java

src/main/java/com/amazonaws/encryptionsdk/EncryptRequest.java

mattsb42-aws · 2020-01-30T00:25:49Z

Not entirely relevant to this PR, but since we don't have a better place and this is where you said you'll be doing the change:

As mentioned here[1], can we make sure that we call the AWS KMS keyring the "AWS KMS" keyring, not just the "KMS" keyring?

[1] awslabs/aws-encryption-sdk-specification#67

mattsb42-aws

Other than the minor naming issue, I like it.

Obviously this needs review from one of our resident Java folks, but as far as the overall design of the keyrings and how they're being integrated into the existing system, I like it.

src/test/java/com/amazonaws/encryptionsdk/internal/StaticKeyring.java

mattsb42-aws

Dependent on the review of one of our Java experts, LGTM.

robin-aws

Just some initial thoughts on the API changes based on reviewing the examples. More to come! :)

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

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

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

robin-aws

Another batch of comments - I've looked at about half of the non-test file changes now.

src/main/java/com/amazonaws/encryptionsdk/AwsCrypto.java

src/main/java/com/amazonaws/encryptionsdk/AwsCryptoResult.java

src/main/java/com/amazonaws/encryptionsdk/internal/MessageCryptoHandler.java

src/main/java/com/amazonaws/encryptionsdk/internal/DecryptionHandler.java

) * 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

robin-aws

Finished reviewing all source file, will do one more quick batch that looks at the test files.

src/main/java/com/amazonaws/encryptionsdk/internal/EncryptionHandler.java

src/main/java/com/amazonaws/encryptionsdk/CreateDecryptingInputStreamRequest.java

src/main/java/com/amazonaws/encryptionsdk/DefaultCryptoMaterialsManager.java

src/main/java/com/amazonaws/encryptionsdk/caching/CachingCryptoMaterialsManager.java

src/main/java/com/amazonaws/encryptionsdk/kms/AwsKmsCmkId.java

src/main/java/com/amazonaws/encryptionsdk/model/EncryptionMaterials.java

src/main/java/com/amazonaws/encryptionsdk/CryptoInputStream.java

src/test/java/com/amazonaws/crypto/examples/FileStreamingExampleTest.java

src/test/java/com/amazonaws/encryptionsdk/AwsCryptoTest.java

src/main/java/com/amazonaws/encryptionsdk/kms/AwsKmsCmkId.java

robin-aws

Just minor stuff in the tests.

src/test/java/com/amazonaws/encryptionsdk/DecryptRequestTest.java

src/test/java/com/amazonaws/encryptionsdk/DefaultCryptoMaterialsManagerTest.java

src/test/java/com/amazonaws/encryptionsdk/kms/AwsKmsCmkIdTest.java

robin-aws

LGTM, thanks for addressing all the feedback!

* Additional example code for Keyrings * Updating wording * Remove AWS from AWS KMS keyring and make keyring lowercase

WesleyRosenblum requested review from mattsb42-aws, robin-aws and SalusaSecondus January 17, 2020 01:54

mattsb42-aws mentioned this pull request Jan 20, 2020

add keyrings #102

Open

15 tasks

WesleyRosenblum added 2 commits January 22, 2020 16:31

Incorporate Keyrings into AwsCrypto and deprecate MasterKeyProviders.

6bb85a0

Update example code to use keyrings

63ff149

WesleyRosenblum force-pushed the materialsmanager branch from bd5f9e4 to 63ff149 Compare January 23, 2020 00:40

robin-aws reviewed Jan 23, 2020

View reviewed changes

src/examples/java/com/amazonaws/crypto/examples/FileStreamingExample.java Outdated Show resolved Hide resolved

Using try-with-resources for AwsCrypto streams

d444f52

SalusaSecondus suggested changes Jan 23, 2020

View reviewed changes

WesleyRosenblum added 2 commits January 23, 2020 12:42

Splitting MKP and keyring unit tests

0a3e6e3

Making decryptData with ParsedCiphertext public

30c51a9

SalusaSecondus previously approved these changes Jan 23, 2020

View reviewed changes

Mark KeyStoreProvider as deprecated

4d8614f

WesleyRosenblum dismissed SalusaSecondus’s stale review via 4d8614f January 24, 2020 00:15

SalusaSecondus previously approved these changes Jan 24, 2020

View reviewed changes

juneb reviewed Jan 25, 2020

View reviewed changes

Reword some comments on the Basic Encryption example

400f521

WesleyRosenblum dismissed SalusaSecondus’s stale review via 400f521 January 27, 2020 20:44

Add test for compability of Keyrings with MasterKeyProviders

59e3045

mattsb42-aws requested changes Jan 28, 2020

View reviewed changes

src/examples/java/com/amazonaws/crypto/examples/EscrowedEncryptExample.java Outdated Show resolved Hide resolved

src/main/java/com/amazonaws/encryptionsdk/AwsCrypto.java Outdated Show resolved Hide resolved

Create individual request types for each AwsCrypto method

6e473a1

praus reviewed Jan 29, 2020

View reviewed changes

src/main/java/com/amazonaws/encryptionsdk/EncryptRequest.java Show resolved Hide resolved

WesleyRosenblum added 6 commits January 29, 2020 17:51

Make EncryptionMaterials, DecryptionMaterials and KeyringTrace immutable

623f6f5

Rename KmsKeying and related classes to AwsKmsKeyring

3af73f0

Create builders for the standard keyrings

5258475

Create AwsKmsCmkId type to represent AWS KMS Key Ids

e04c285

Add factory methods to Keyring builders

6396c6c

Add comment on not making a defensive copy of plaintext/ciphertext

15c1def

WesleyRosenblum added 4 commits January 31, 2020 14:25

Limit ability to create discovery AWS KMS Keyrings to explicit creation

6af9b02

Add withKeyring to CachingCMM builder

c182cd5

Fix DecryptRequestTest

37d42bb

Fix Junit 4 assertions in JUnit5 tests

63057f5

mattsb42-aws requested changes Feb 4, 2020

View reviewed changes

src/test/java/com/amazonaws/encryptionsdk/internal/StaticKeyring.java Outdated Show resolved Hide resolved

Renaming StaticKeyring to TestKeyring

f415b7f

mattsb42-aws previously approved these changes Feb 4, 2020

View reviewed changes

robin-aws reviewed Feb 5, 2020

View reviewed changes

Adding convenience methods the create builders internally

7ec91d6

WesleyRosenblum dismissed mattsb42-aws’s stale review via 7ec91d6 February 6, 2020 00:45

robin-aws reviewed Feb 6, 2020

View reviewed changes

WesleyRosenblum and others added 3 commits February 6, 2020 11:28

Updating wording and adding more Deprecated annotations

876dd5e

Enable AwsKms Client Caching by default to match KmsMasterKeyProvider

214fabd

robin-aws reviewed Feb 10, 2020

View reviewed changes

WesleyRosenblum added 2 commits February 10, 2020 15:48

Renaming StandardKeyring builder methods and other minors changes

8ed8619

Fixing test

28aeee2

robin-aws reviewed Feb 11, 2020

View reviewed changes

Updating tests to use assertThrows

ff9dab6

robin-aws previously approved these changes Feb 11, 2020

View reviewed changes

ttjsu-aws mentioned this pull request Feb 11, 2020

Revise Examples #108

Closed

15 tasks

Additional example code for Keyrings (#155)

12f0c42

* Additional example code for Keyrings * Updating wording * Remove AWS from AWS KMS keyring and make keyring lowercase

WesleyRosenblum dismissed robin-aws’s stale review via 12f0c42 February 12, 2020 01:15

robin-aws approved these changes Feb 12, 2020

View reviewed changes

WesleyRosenblum merged commit 8bdb1d4 into keyring Feb 12, 2020

WesleyRosenblum deleted the materialsmanager branch February 12, 2020 02:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Incorporate Keyrings into AwsCrypto and deprecate MasterKeyProviders. #151

Incorporate Keyrings into AwsCrypto and deprecate MasterKeyProviders. #151

WesleyRosenblum commented Jan 17, 2020 •

edited

Loading

mattsb42-aws commented Jan 30, 2020

mattsb42-aws left a comment

mattsb42-aws left a comment

robin-aws left a comment

robin-aws left a comment

robin-aws left a comment

robin-aws left a comment

robin-aws left a comment

Incorporate Keyrings into AwsCrypto and deprecate MasterKeyProviders. #151

Incorporate Keyrings into AwsCrypto and deprecate MasterKeyProviders. #151

Conversation

WesleyRosenblum commented Jan 17, 2020 • edited Loading

Check any applicable:

mattsb42-aws commented Jan 30, 2020

mattsb42-aws left a comment

Choose a reason for hiding this comment

mattsb42-aws left a comment

Choose a reason for hiding this comment

robin-aws left a comment

Choose a reason for hiding this comment

robin-aws left a comment

Choose a reason for hiding this comment

robin-aws left a comment

Choose a reason for hiding this comment

robin-aws left a comment

Choose a reason for hiding this comment

robin-aws left a comment

Choose a reason for hiding this comment

WesleyRosenblum commented Jan 17, 2020 •

edited

Loading