Multi keyring generator

[seebees]

Should the multi keyring generator REQUIRE generation? The specification as written lets you compose multi keyrings that all have generators in ANY combination. It is easier to get the organization correct by construction if there is one and only one path to data key generation.

[mattsb42-aws]

My 2p on this:

The purpose of specifying the generator is to identify the wrapping key that you want to use to generate the data key. The current contract is that if the generator completes its on-encrypt call and there is no data key then the multi-keyring will fail. This does not imply that the generator MUST generate the data key. This is problematic because it means that the behavior of the multi-keyring changes depending on how it is used within a keyring construction: if it is the top-level keyring the generator will generate but if it is, say, itself a child keyring in another multi-keyring with its own generator then this keyring's generator will not generate. In this model, the caller creating the multi-keyring might not be able to know if it is correct on construction or not.

If we do not make a guarantee that the generator will generate, then its usefulness is limited. Say you have a compliance requirement that all data keys MUST be generated by a specific HSM, or you are using AWS KMS and you want to lock down true least-privilege permissions for your key policies. Under the current model, you have to be very careful to make sure that you know exactly where and how a given multi-keyring configuration is being used rather than being certain that it will always do the same thing no matter what.

My proposal is that we change the multi-keyring generator definition to:

The generator keyring MUST generate the data key.

This translates into two checks:

1. When the multi-keyring's on-encrypt call starts, if a generator keyring is defined then the encryption materials MUST NOT already have a data key set.
2. After the multi-keyring's on-encrypt flow passes the generator call (whether it is defined and called or not) the encryption materials MUST have a data key set.

[seebees]

I also prefer this. It make things correct by construction, and a given construction will be reproducible. It is less flexible, and I think that is its virtue.

[robin-aws]

I definitely like the proposal. We could actually carry it even further to say that ALL keyrings should be intentionally configured to EITHER generate a plaintext data key OR encrypt an existing one. That would raise the bar on correct-by-construction even further:

1. All keyrings would support an IsGenerator() predicate (a.k.a. "operation returning a boolean") to indicate whether they expect no existing data key and create one, or require one to be provided.
2. "Individual" keyrings (e.g. a raw RSA keyring) could be configured with an IsGenerator parameter to control this explicitly
   1. Note this would help define more explicitly exactly what KMS permissions you need to grant for a keyring to work.
3. The multi-keyring would NOT accept this switch, instead switching depending on whether a generator keyring is provided. It would also fail to initialize if any child keyrings returned true from IsGenerator(), or if a provided generator keyring returns false

This feels like the logical conclusion from @mattsb42-aws' idea that the multi-keyring's behaviour should be more predictable without having to consider context. I think that applies to all keyrings: it's unfortunate that a KMS keyring will generate when used by itself but not when used as a child keyring of a multi-keyring.

[seebees]

Should these be different keyrings? I have to say that optional booleans is a thing of horror.

[robin-aws]

This would apply to all keyrings, so it would mean close to doubling the "number of keyrings". But it comes down to whether you consider the results of two different keyring-producing operations with slightly different names that take the same input "different keyrings" :)

I didn't actually say IsGenerator parameter would be a boolean, I agree boolean parameters suck. It should probably be an enumeration instead where supported (and probably with a better name). I also didn't technically say it would be optional: as I wrote above it would actually be a breaking change where each keyring has to be explicitly configured to either "require generation" or "forbid generation". But then again @mattsb42-aws' suggestion is a breaking change for multi-keyrings already!

It might still be more reasonable to have a third configuration option of "generate if needed", which would be the default for backwards-compatibility. Perhaps multi-keyrings could still have that option transitively. Customers would be motivated to use the new configuration if they have the kinds of compliance or permissions requirements outlined above.

Another point I missed: the default CMM initialization should fail if given a Keyring for which IsGenerator() returns false. That's another case of failing earlier rather than waiting until OnEncrypt is actually called.

Note this would make it more feasible to define Keyrings that ONLY generate plaintext data keys without encrypting, since such keyrings could only ever be configured to generate and not encrypt, and you would have an initialization failure if you tried to use them as a child keyring in a multi-keyring. I don't know if there's any actual need for that though.

[seebees]

So why not split the Keyring call in two? onEncrypt onGenerate? If I am in a situation where I need to generate why am I responsible for checking?

If we are this deep, then I would suggest that we also discuss the decrypt path. I may have a configuration of keyrings that I ONLY want to decrypt with.

Is ALL this something that would be better handled by a wrapping keyring? Since all of this logic (expect in the case of the multi keyring) is the same and can be evaluated without regard to the specific underlying keyring. e.g. Both AWS KMS and Raw AES would do something like

needs(material.hasUnencryptedDataKey() === false , 'This keyring MUST generate')
return childKeyring.onEncrypt(material)

[robin-aws]

So why not split the Keyring call in two? onEncrypt onGenerate? If I am in a situation where I need to generate why am I responsible for checking?

I think that's an independent API choice and doesn't really change the story here: there's a direct mapping between onEncrypt <==> onEncrypt(<with plaintext data key>) and onGenerate <==> onEncrypt(<without plaintext data key>). Either way you still end up with a dynamic decision about whether to allow the call, and I don't think we want to go down the road of something other than a Keyring being responsible for generating (at least not without a REALLY good reason).

If we are this deep, then I would suggest that we also discuss the decrypt path. I may have a configuration of keyrings that I ONLY want to decrypt with.

Are you talking about locking down decrypt-only keyrings to reject both generating and encrypting?

Is ALL this something that would be better handled by a wrapping keyring? Since all of this logic (expect in the case of the multi keyring) is the same and can be evaluated without regard to the specific underlying keyring. e.g. Both AWS KMS and Raw AES would do something like

needs(material.hasUnencryptedDataKey() === false , 'This keyring MUST generate')
return childKeyring.onEncrypt(material)

Are you talking about the boilerplate code to fail if asked to generate when you don't support it, or given a plaintext data key when you require generating? Yes, I could see that being convenient to implement in a wrapper or an abstract base class of some kind.

[seebees]

I think that's an independent API choice and doesn't really change the story here

Sure, it does not need to be. I just wanted see if that might redirect your intention in a different way :)

Are you talking about locking down decrypt-only keyrings to reject both generating and encrypting?

Yes. And the second question is yes as well. All three of these "ideas" are asking Is the failure the thing or the runtime interrogation the thing?

I think that the failure is the thing, because the runtime interrogation, while AWSOME is more complicated than just Can Generate.

[robin-aws]

Yeah, any kind of interrogative feature is definitely going to be more complicated since you have both a CanDoTheThing and a DoTheThing operation, and you have to make sure they are consistent. I don't think it's a lot worse than having common requirements on operations that multiple implementations will have to ensure somehow though, and different instances of the ESDK will have different strategies for sharing code.

I end up proposing the interrogation because once you make the leap that all keyrings should fail if generation doesn't happen as expected, it's a very small step to make what they expect queryable at runtime. Given that, it's a great opportunity to make the multi-keyring, and any other higher-order keyring wrapper, more correct by construction.

[mattsb42-aws]

So why not split the Keyring call in two? onEncrypt onGenerate?

This feels like a regression to me.

For the record, these two paths were merged into one very intentionally when we designed keyrings. Master key providers had this (and a few other) APIs, and the problem is that if you have more than one API that can be called during a single top-level operation (encrypt/decrypt), you MUST have something outside of the thing with that API that coordinates the behavior of that thing. This makes creating things that interact with either side of that API much more complicated.

If I am in a situation where I need to generate why am I responsible for checking?

I like the idea of being able to interrogate keyrings on initialization about what they MUST do, but this feels like the wrong shape to me.

I think this comes back to one of the things that we've discussed in the "how to fail with keyrings" doc #131 , which is that any given keyring cannot know whether it is the only keyring and therefore every keyring MUST assume that it is the top keyring.

If we had a control such as you proposed, @robin-aws , then in order to build a multi-keyring I MUST configure keyrings that can never succeed if they are the top keyring. This conflicts with the statement that no keyring can know whether or not it is the top keyring.

Maybe something in the middle could be useful? Each keyring exposes a set of things that it CAN do, rather than that it MUST do? That way the DefaultCMM and the multi-keyring can still interrogate the things that they are given to determine if success is possible, but we do not place the entire responsibility for that understanding in the DefaultCMM (as would be necessary if keyrings could be configured as @robin-aws proposed).

[robin-aws]

Maybe something in the middle could be useful? Each keyring exposes a set of things that it CAN do, rather than that it MUST do? That way the DefaultCMM and the multi-keyring can still interrogate the things that they are given to determine if success is possible, but we do not place the entire responsibility for that understanding in the DefaultCMM (as would be necessary if keyrings could be configured as @robin-aws proposed).

I think we're actually leaning the same way, as I suggested the 'third configuration option of "generate if needed"' above as well. The best way to model things may be separate CanGenerate() and CanEncrypt() predicates. Most keyrings would support both, and presumably no keyrings would support neither.

[mattsb42-aws]

and presumably no keyrings would support neither.

nit: AWS KMS discovery keyring

[mattsb42-aws]

There's also CanDecrypt(); for example, a keyring that only generates and never encrypts or decrypts.

[seebees]

The nit is why I was talking about onDecrypt :)

For me, I think that having the MultiKeyring generator is a solid win. But I would like to address ALL the other things a keyring can do all together.

I would move the discussion of keyring contracts and predicates into a separate issue and try and talk about ALL of these issues togeter.

[robin-aws]

I would move the discussion of keyring contracts and predicates into a separate issue and try and talk about ALL of these issues togeter.

I like this - I'll fully admit that my ideas are relevant but possibly off-topic. :) We can agree on tightening up the multi-keyring behaviour dynamically, and then argue more about how far we push correct-by-construction and catch more things statically. I'll cut a separate issue.

[robin-aws]

Note for the record that this point is already covered in the current spec:

After the multi-keyring's on-encrypt flow passes the generator call (whether it is defined and called or not) the encryption materials MUST have a data key set.

It currently reads:

If the generator keyring returns encryption materials missing a plaintext data key, OnEncrypt MUST fail. If this keyring does not have a generator keyring, and the input encryption materials does not include a plaintext data key, OnEncrypt MUST fail.

Therefore the only actual change is failing if we have a generator but are given materials with a plaintext data key already set.