Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support MultiProvider Encrypt when not all providers are available #25

Open
rmmeans opened this issue Aug 29, 2017 · 8 comments
Open

Support MultiProvider Encrypt when not all providers are available #25

rmmeans opened this issue Aug 29, 2017 · 8 comments
Assignees

Comments

@rmmeans
Copy link

rmmeans commented Aug 29, 2017

First off - what a great library. This solves many problems that a developer would have otherwise needed to handle on their own. Thank you!

We are looking to use this library to provide Multi-Region KMS encrypt and decrypt operations, where if any KMS service in any single region is down, we are not down. This works today for Decrypt operations which is awesome, but not Encrypt operations - all providers must be available to encrypt data. In a sense, what one was attempting to achieve in reducing the risk of an outage by leveraging two different KMS’s (for instance), you actually increase your outage risk on encrypt operations because if a single KMS provider is down in any region, you are down for encrypts.

I would like to see the MultiProviderFactory have some construction options where I can specify the minimum number of providers (or a quorum) that must be available for encrypt to succeed - or something of the sort. Leaving it configurable allows the product to decide how much risk to take on (for encrypting data that can not be decrypted later by every single provider in the event providers are not available)

@mattsb42-aws
Copy link
Member

Awesome, I'm glad you have found the library useful!

We have had some requests for functionality like this and it is something we are looking into. We don't have any hard commitments or timelines around release of such functionality at this time, but I'll leave this issue open and keep it updated with any information as we have it. We'll need to discuss it among the team and nail down an API that we want to support before committing to adding something like this.

If you have an urgent need for this functionality and are interested in taking a shot at it for yourself in the meantime, the best level to insert this logic would be in a cryptographic materials manager. The simplest implementation would be almost identical to the DefaultCryptoMaterialsManager, with the exception that it would not require all master keys to successfully encrypt the data key (and it would need some configuration to determine how many/which master keys to require to succeed).

@DarrellMozingo
Copy link

+1 on Ryan's comment - great SDK that we're starting to use!

We have the same concerns regarding multi-region and were thinking a simpler approach to start might work well. Instead of a quorum of available providers, just a simple loop over them and collecting exceptions like the decrypt functionality does now? We could do the loop both to obtain the data key (rather than the current index 0 try) and the data key encryption of the other regions. Does that sound easier or less error prone? I don't see it requiring a change to the API. Failures could be dealth with lazily at decrypt time if a client wanted to (ie "only 3 keys available, missing region xyz, re-encrypt it all to get the missing region").

If we did that on our fork would you be interested in pulling the functionality up while you work on a more robust quorum based solution?

@mattsb42-aws
Copy link
Member

great SDK that we're starting to use!

Thanks! We're always happy to hear feedback from customers.

We would definitely not want to merge the described behavior into any existing CMMs. This is a significant departure from current behavior and will need to be very clearly differentiated. However, we are definitely open to contributions and suggestions.

We're still working on the best way to handle new feature proposals because in order for us to add any additional fundamental features (which this would be) we need to define them as generalized specifications as well as implementing them in both current clients.

I'll bring this up with the team and see if we can decide on a good starting point for feature proposals.

@DarrellMozingo
Copy link

That makes complete sense @mattsb42-aws, thanks. Do let us know if you put a proposal out there for feedback, I'd love to take a look!

@willwhitaker
Copy link

How would something like this be @mattsb42-aws ?

This is largely a copy of the DefaultCryptoMaterials manager apart from the last part of the getMaterialsForEncrypt method which returns as many key blobs as it can upto the required "quorum". If the quorum isn't reach then it fails and it follows the ordering of the input master keys so prioritizing ones earlier in the list.

This obviously needs more work but interested to know if this is something we could try to get in.

https://gist.github.com/willwhitaker/371f9c83bfff71edf488a8d182bed215

@mattsb42-aws
Copy link
Member

Sorry for the delayed response. We're working on finalizing our RFC process right now, I so should have a better answer soon (or, at least, a better place to discuss this in detail). I'm hesitant to start too in depth a conversation around this before we have the process in place just to minimize the cognitive shift when we move it over.

That said, some of the points that we have identified so far that we will need to decide what approach makes the most sense include:

  • How should we determine which master key generates the data key?
  • Do we want to allow users to identify required master keys vs optional master keys?
  • Do we want to allow just a minimum threshold of encrypted data keys or also allow all master keys to be attempted?
  • Should high latency identify a master key as failed, or just strictly failed calls? What about number of retries?
  • On decrypt, should we change the order in which encrypted data keys are processed to reduce latency, or is that out of scope for this change? If we do, what behavior do we want to show?
  • Given that this is starting to blur the conceptual lines between cryptographic materials manager and master key provider because it is making decisions about how master keys are used, is the CMM really the best conceptual tier to place this logic? If not, what would make the most sense for an interface and structure to own this logic (ex: bulk_encrypt_data_key method on master key providers?)

@DarrellMozingo
Copy link

All very good points @mattsb42-aws, and coincidentally the exact same issues we've been discussing internally for our own implementation of this feature (largely based off @willwhitaker's gist). We've settled on what works for the above points for us based on trade-offs so will be keen to discuss them in a better RFC process :)

@texastony
Copy link
Contributor

@rmmeans , @DarrellMozingo , @willwhitaker

While we have not updated our Spec to incorporate this feature, AWS' KeyManagementService has released Multi-Region Keys, or MRKs.

The Encryption SDK (ESDK) supports MRKs. Using a MRK may replace the need for a failure tolerant CMM or MasterKeyProvider.

If the objective is to encrypt data such that it can be decrypted in multiple regions, instead of encrypting the data with multiple KMS keys, one from each region, you could encrypt the data with just a Multi Region Key that is replicated in multiple regions.

You can read more about using MRKs with the ESDK here.

We have some MRK examples as well: BasicMultiRegionKeyEncryptionExample and DiscoveryMultiRegionDecryptionExample.

Much Obliged,
AWS Crypto Tools

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants