Master Key to Keyring API Evolution

[WesleyRosenblum]

Introduction

The Java version of the AWS Encryption SDK, having been developed prior to all other language variants of the AWS Encryption SDK, contains configuration and API constructs that do not exist in the more recently developed versions. Namely, the Master Key/Master Key Provider (MKP) construct has been replaced by Keyrings in the C, Javascript, and all future versions of the ESDK. To bring the Java ESDK in line with the other ESDKs, Keyring support is currently being added (see #102 ). However, since Master Key Providers are present in both the input and output of the Java ESDK, the process of introducing Keyrings while minimizing disruption to existing customers is complicated. This issue summarizes the current state of Master Key Providers in the Java ESDK and raises the following questions regarding the evolution of the Java ESDK API:

• Should we provide a Master Key Provider Keyring to ease transitioning to Keyrings?
• How should the result object of encryption and decryption operations be updated to support Keyrings?
• Should a new major version be introduced?

Current State

The AwsCrypto class serves as the primary entry-point to the Java ESDK. This class contains 30 public methods interacting with Master Key Providers, which are overloaded variations of the following 4 methods:

• estimateCiphertextSize, for estimating the output length of encrypting a plaintext
• encryptData/decryptData, the primary encryption and decryption methods
• createEncryptingStream/createDecryptingStream, for encrypting/decrypting input streams
• encryptString/decryptString, deprecated methods for encrypting/decrypting strings

Each of these methods use either a MasterKeyProvider or a CryptoMaterialsManager to perform its encryption/decryption objective. When a MasterKeyProvider is specified, the given MKP is wrapped in the DefaultCryptoMaterialsManager and handed off to the corresponding CryptoMaterialsManager based method. All of these methods (save for estimateCiphertextSize which returns a long) return a CryptoResult or a CryptoInputStream/CryptoOutputStream that wraps a CryptoResult. CryptoResult is a generic type that is parameterized with the MasterKey type that was specified to the method. CryptoResult contains a getMasterKeys method that in the case of encryption returns all the MasterKeys used to encrypt the ciphertext, and in the case of decryption returns the single MasterKey used to decrypt the ciphertext.

Should we provide a Master Key Provider Keyring?

The Master Key Provider Keyring is a Keyring that wraps a MasterKeyProvider that customers have developed so it can be used with the new methods in AwsCrypto that support Keyrings. Supplying this Keyring to customers has benefits, but also some drawbacks, and in particular has implications regarding the long term support plan for Master Key Providers. These benefits and drawbacks are delineated below:

Benefits:

• Customers with custom Master Key Providers can onboard to the new Keyring methods with minimal effort by constructing a Master Key Provider Keyring with their existing MKP and calling the new methods.
• Some internal classes (such as DefaultCryptoMaterialsManager) can be updated to work with Keyrings as the Master Key Provider they currently work with can be wrapped with the Master Key Provider Keyring.

Drawbacks

• Wrapping a custom Master Key Provider in the Master Key Provider Keyring defers any required investment by our customers towards converting their MKPs into Keyrings. This will prolong the usage of Master Keys. Once Master Keys are fully deprecated, customers will need to make a second, more extensive, code change to perform the conversion of their MKPs to Keyrings.
• If CryptoResult continues to provide access to Master Keys, Master Keys will need to be added to the EncryptionMaterials and DecryptionMaterials that Keyrings work with in order for the Master Key to be populated in the result. This means that deprecated concepts are being introduced into the new Keyring code.

How should the result object be updated to support Keyrings?

Given that the current CryptoResult provides access to Master Keys, this class will need to be updated or augmented to support Keyrings. Possible solutions involve removing Master Keys from the CryptoResult in favor of the Keyring trace, introducing a new result type used exclusively for when a Keyring is being used, or only populating MasterKeys in CryptoResult when a MKP is in use. These options are discussed below:

Option 1: Remove Master Keys from CryptoResult

The getMasterKeys and getMasterKeyIds methods are removed from CryptoResult and the generic MasterKey parameter is removed. A method for accessing the Keyring trace is added.

Benefits:

• Single result object, reducing complexity and confusion
• Public methods in AwsCrypto that accept a CryptoMaterialsManager instead of a MasterKeyProvider can be updated to support Keyrings
• Makes it easier to fully remove Master Keys later on, since customers will have already removed dependencies on getMasterKeys and getMasterKeyIds

Drawbacks:

• Non-backward compatible, breaking change for all customers that call getMasterKeys on CryptoResult. Removing the generic parameter for Master Key in CryptoResult will require a code change to compile.
• Customers that use the Master Key from a decryption CryptoResult will have to parse the Keyring trace to determine which Key was used for decryption.

Option 2: New result type for Keyrings

A new result type (KeyringCryptoResult for example) is created that is returned by all the AwsCrypto methods that work with Keyrings. The new type would include a Keyring Trace instead of MasterKeys. Existing methods in AwsCrypto would continue to return CryptoResult.

Benefits:

• Backwards compatible. Customers using existing MKP based methods would not need to make any code changes.
• Keyring and MKP code remains segregated for the most part, making it clear what data is expected in the result and easing the removal of MKPs later on.

Drawbacks:

• Public methods in AwsCrypto that accept a CryptoMaterialsManager instead of a MasterKeyProvider will need to be duplicated with an additional parameter or alternate name indicating they are for Keyrings, since we cannot have an identical method with different output. Overall, 24 new public methods will be required, versus 12 if a new result type is not introduced.
• CryptoInputStream and CryptoOutputStream will need to have a getKeyringCryptoResult method added. This would return null when Master Key Providers are used, and similarly getCryptoResult would return null when Keyrings are used, which could be confusing.

Option 3: Only populate Master Keys in CryptoResult when a MKP is used

The Master Key methods in CryptoResult remain the same, but only return a result if a Master Key Provider is being used. A method for accessing the Keyring trace is added to CryptoResult, which would only return a result if a Keyring is used.

Benefits:

• Single result object, reducing complexity and confusion
• Backwards compatible. Customers using existing MKP based methods would not need to make any code changes.
• Public methods in AwsCrypto that accept a CryptoMaterialsManager instead of a MasterKeyProvider can be updated to support Keyrings

Drawbacks:

• Potentially confusing, since the output would contain null or empty data depending on which methods in AwsCrypto were used
• Support for handling Master Keys must remain in DefaultCryptoMaterialsManager, EncryptionHandler, and DecryptionHandler while additional code for handling Keyring Traces is added, adding complexity.

Should a new major version be introduced?

Another alternative to both the MasterKeyProviderKeyring and updates to the CryptoResult is introducing a new, non-backward compatible major version of the Java ESDK that removes all references to Master Keys and Master Key Providers in favor of Keyrings while marking the old version of the Java ESDK as deprecated. While the introduction of Keyrings is likely not sufficiently beneficial to customers to warrant such an action, the benefits and drawbacks are nonetheless presented below for completeness:

Benefits:

• Java ESDK will match the design and functionality of the more recent ESDKs
• The amount of public methods in AwsCrypto does not increase
• A simpler and more elegant code base

Drawbacks:

• Non-backward compatible, breaking change for all customers requiring all customers to update their code, sometimes in significant ways, in order to use the new version.
• Multiple versions of the ESDK must be maintained until the MKP version reaches end of life.

[mattsb42-aws]

I think that any way we go about this, we are going to have to introduce breaking changes. The question in my mind is how can we make it as simple as possible for our customers to adapt to those changes so that we don't end up with customers sticking with old, unsupported, versions of the ESDK.

I think that the things that we need to optimize for, in order of importance, are:

1. API simplicity: It must be easy to use correctly and difficult to misuse.
2. Customer value: Why should a customer care about upgrading?
   • If we can provide direct customer value with each breaking change in addition to the underlying "do this so that you won't be stuck on an old version", customers will have incentive to make those changes.
   • Conversely, we should never ask customers to make a change to their code that does not add value for them.
3. Customer upgrade path: Minimize the friction for our customers to start using each new release.
   • Each breaking change we introduce should require as little change to customer code as possible.
   • No changes that we introduce should require any customer to change code that we will then ask them to change again because of another planned change.
   • Our more advanced users will probably be more likely to be ok with changing their code. I'm less concerned with minimizing changes that will have to be made by customers who have implemented custom master key providers than I am with minimizing changes that will have to be made by the [likely] majority of our customers who just use our clients out of the box.
4. Maintainability: How much will our decision increase our maintenance burden?

In light of those priorities, IMO the new major version that effectively forks the codebase is a non-starter. It creates a large amount of friction for all of our customers customers AND creates a large amount of ongoing maintenance burden for us, for what seems to me to be a marginal value to our customers.

What I would like to propose is that we approach this from a few different angles:

We have four basic objectives: we want to remove the dependence on master key providers (MKPs) in the return values, we want to enable use of keyrings for configuration, we want to set the groundwork for the eventual deprecation of MKPs, and we want to reduce the proliferation of entry methods.

remove MKPs from CryptoResult[/etc]

I think this is just something we need to do. It's a breaking change, but we can do what we can to make it simple.

Any code change that a customer needs to make should be as minimal as possible, and preferably deterministic. For example, the primary reason we expect customers to use getMasterKeys and getMasterKeyIds is to verify whether a given MKP was used. If we remove any reference to MKP in CryptoResult but now have everything inject a keyring trace, we could add a method to MasterKey for something along the lines of myActionsInKeyringTrace(KeyringTrace) or performedAction(KeyringTrace, ExpectedAction).

By not changing the return type, we avoid the requirement for people to change type signatures simply because they start using keyrings.

The question remains, though, how can we make this worth the time for a user to update? If the MKP helper method lets them look for specific actions, that might be worth it.

prepare for MKP deprecation

This is, in my mind, less a "what should we do" and more a "what should we not do".

We should not introduce anything that will need to again later be changed when we deprecate MKPs.
We should not be asking our customers to change any parts of their code that they will need to change again when we deprecate MKPs.

reduce API method proliferation

This looks initially like purely a benefit for us, since its primary goal is to reduce the code we need to maintain. However, it also adds benefit for the user, both in understanding documentation and in attempting to navigate our code.

Something that seems to me like it might help in this regard is if we change what we expect as an input. If we were to change AwsCrypto's methods to the type of pattern we use everywhere else in the framework, to accept a config object rather than a combination of parameters, then it would be fairly straightforward for us to create a builder that would let the user easily construct a request with any configuration they want for the key provider.

One problem I have with this idea is that while I can see the long-term value to customers in the form of simpler, easier to navigate, documentation and code, I'm having a hard time coming up with a compelling short-term value that would convince me (as a customer) to go and change my existing codebase to use it.

[WesleyRosenblum]

I think that any way we go about this, we are going to have to introduce breaking changes

I agree that eventually we will need to introduce breaking changes, but I wouldn't say that it is necessary that we introduce breaking changes immediately. While I like the idea of removing MKPs from CryptoResult, just the fact that it is breaking at all (however small in scope) leads me to believe if we do that we will effectively have to support two versions of the ESDK and back port any future bug fixes to the MKP version.

What do you think of the backward-compatible approach of adding keyring traces to CryptoResult, and just marking getMasterKeys and getMasterKeyIds as deprecated (as well as marking MKP methods in AwsCrypto as deprecated)? This way customers will not have to make any change immediately, but can begin to shift away from MKPs towards Keyrings at their leisure.

Also, I'd like your thoughts on the Master Key Provider Keyring, as my current thinking is that this does not make sense to provide to our customers, as it violates the tenet of "We should not be asking our customers to change any parts of their code that they will need to change again when we deprecate MKPs." Wrapping an MKP into a MKP-Keyring doesn't really give much benefit and will be thrown away once MKPs are deleted.

[mattsb42-aws]

I've been thinking about this, and I think you're right; the backwards-compatible CryptoResult provides the smoothest transition that I can think of.

I do think we should add something, and this might be something that we should add to the spec: Aside from constructing the MKPs/keyrings, the crux of this migration issue is how a user can check the results to see what a given MKP/keyring did. I don't remember if we've looked at this yet for keyrings, but if we can make whatever that is (I'm thinking a helper function that takes a keyring trace and a keyring) work with master keys as well, that might help ease the migration.

[...] Master Key Provider Keyring, as my current thinking is that this does not make sense to provide to our customers, as it violates the tenet of "We should not be asking our customers to change any parts of their code that they will need to change again when we deprecate MKPs."

The way that I've been thinking the MKP-keyring would manifest is that the user's code calling AwsCrypto would not change at all. They would continue on providing MKPs and we would wrap them in the same way that we currently wrap MKPs in a DefaultCryptoMaterialsManager. In this sense it is more of a convenience and organizational factor for us, allowing us to pull the MKP-specific logic out of DefaultCryptoMaterialsManager and ultimately making the MKP deprecation simpler. It would also allow us to inject a "keyring" trace for known master keys.

[WesleyRosenblum]

I've started implementing a version of AwsCrypto with new methods that look like:
CryptoResult<byte[], ?> encryptData(final AwsCryptoConfig config, final byte[] plaintext)
where AwsCryptoConfig contains the CryptoMaterialsManager or Keyring, and the EncryptionContext. All other methods that don't use AwsCryptoConfig will remain for now, but they will be marked as deprecated.

One issue I'm foreseeing is that even if a customer fully commits to use Keyrings now, and doesn't call getMasterKeys or getMasterKeyIds on CryptoResult, they will still have to make a code change when we fully remove MasterKeys. The reason is because CryptoResult is defined as:
public class CryptoResult<T, K extends MasterKey<K>>

Removing the second generic parameter will require all customers to update their code, even those customers that are already using Keyrings. This again violates the "We should not be asking our customers to change any parts of their code that they will need to change again when we deprecate MKPs" tenet, and even worse we would effectively be punishing the customers that proactively migrate to Keyrings.

So what am thinking again is defining a new return type (maybe AwsCryptoResult) that is returned by all the new AwsCryptoConfig based methods. This will also require new versions of CryptoInputStream and CryptoOutputStream, as those are similarly defined as:
public class CryptoInputStream<K extends MasterKey<K>>
public class CryptoOutputStream<K extends MasterKey<K>>

[seebees]

Reading over this,
it seems clear that we will eventually push a new major version.
Having done that,
we will have to backport fixes to the old version.
Before we push this new major version,
we should be clear what our support plan is.
I.e. How long will we support backporting fixes?
The number should be public and finite :)