Skip to content

Remove lines mandating region be specified by KMS keyring generator #50

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.

Sign up for GitHub

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

Merged
merged 15 commits into from
Feb 18, 2020
Merged

Remove lines mandating region be specified by KMS keyring generator #50

Changes from 5 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
db5a6a5
Remove lines mandating a region be specified by the generator
MatthewBennington Oct 29, 2019
cc6c879
Changing previously missed paragraph, and fixing typos
MatthewBennington Oct 30, 2019
8d6cec2
Clarifications and fixes to KMS Keyring
MatthewBennington Nov 4, 2019
8977030
Pull request iteration
MatthewBennington Nov 20, 2019
a2455bd
Remove unreachable Decrypt behavior
MatthewBennington Nov 21, 2019
0c5c19a
Remove unreachable lines
MatthewBennington Nov 21, 2019
3dc5751
Update framework/kms-keyring.md
MatthewBennington Feb 17, 2020
7fffe53
"Input CMK -> "Response KeyID"
MatthewBennington Feb 17, 2020
e0748d5
Update framework/kms-keyring.md
MatthewBennington Feb 18, 2020
128c87b
Update framework/kms-keyring.md
MatthewBennington Feb 18, 2020
c4da98a
Update framework/kms-keyring.md
MatthewBennington Feb 18, 2020
6cd77d5
"contains" ->
MatthewBennington Feb 18, 2020
fe8e61d
Update framework/kms-keyring.md
MatthewBennington Feb 18, 2020
2143ee4
Update framework/kms-keyring.md
MatthewBennington Feb 18, 2020
ecdabbf
Update framework/kms-keyring.md
MatthewBennington Feb 18, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 26 additions & 25 deletions framework/kms-keyring.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,10 @@ On keyring initialization, a keyring MAY define the following:

### Client Supplier

A function that returns a KMS client that can make the following API calls in a particular AWS region:
A function which MUST take as input either a region string (e.g. `us-east-1`), or some value denoting an unknown region,
and MAY return a KMS Client which can make the following API calls in the given AWS region:

- [GenerateDataKey](#kms-generate-data-key)
- [GenerateDataKey](#kms-generatedatakey)
- [Encrypt](#kms-encrypt)
- [Decrypt](#kms-decrypt)

Expand All @@ -81,7 +82,7 @@ Each Key ID MUST be one of the following:

- A CMK [alias](https://docs.aws.amazon.com/kms/latest/developerguide/programming-aliases.html) (e.g. "alias/MyCryptoKey")
- A well-formed key ARN (e.g. arn:aws:kms:us-east-1:999999999999:key/01234567-89ab-cdef-fedc-ba9876543210)
- A well-formned alias ARN (e.g. arn:aws:kms:us-east-1:999999999999:alias/MyCryptoKey)
- A well-formed alias ARN (e.g. arn:aws:kms:us-east-1:999999999999:alias/MyCryptoKey)

See [AWS Documentation](https://docs.aws.amazon.com/general/latest/gr/aws-arns-and-namespaces.html#arn-syntax-kms).

Expand All @@ -102,7 +103,7 @@ The string MUST be one of the following:

- A CMK [alias](https://docs.aws.amazon.com/kms/latest/developerguide/programming-aliases.html) (e.g. "alias/MyCryptoKey")
- A well-formed key ARN (e.g. arn:aws:kms:us-east-1:999999999999:key/01234567-89ab-cdef-fedc-ba9876543210)
- A well-formned alias ARN (e.g. arn:aws:kms:us-east-1:999999999999:alias/MyCryptoKey)
- A well-formed alias ARN (e.g. arn:aws:kms:us-east-1:999999999999:alias/MyCryptoKey)

See [AWS Documentation](https://docs.aws.amazon.com/general/latest/gr/aws-arns-and-namespaces.html#arn-syntax-kms).

Expand Down Expand Up @@ -160,11 +161,12 @@ If the input [encryption materials](#structures.md#encryption-materials) do not
and this keyring has a [generator](#generator) defined, and onEncrypt MUST attempt to generate a new plaintext data key
and encrypt that data key by calling [KMS GenerateDataKey](#kms-generatedatakey).

The [KMS client](#kms-client) that calls [KMS GenerateDataKey](#kms-generatedatakey) MUST be the
client returned by the [client supplier](#client-supplier).
The client MUST be a client that calls the AWS region specified in the [generator](#generator) ARN.
If the [client supplier](#client-supplier) does not provide any client for the given region for this GenerateDataKey call,
OnEncrypt MUST fail.
If an AWS region can be extracted from the [generator](#generator), then the [KMS client](#kms-client) that calls
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is also true of the KMS Encrypt call below.

[KMS GenerateDataKey](#kms-generatedatakey) MUST be the client returned by the [client supplier](#client-supplier)
when given that region as input.
Comment on lines +164 to +166
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels like it is drawing an odd relationship that I don't think exists. The way this is worded make it sound to me like we are saying that there is something special about the region name extracted from this ARN that makes it different from the same region name extracted from some other ARN. This is not the case (though, side note, this is why I wanted to the client provider to accept a key name rather than a region name, but that's a different conversation).

I think we can simplify this to something along the lines of the following.

Client Provider

  • The client provider MUST require a region name.
  • The client provider MUST either provide a client for the requested region or fail.

Keyring

  • If the key ID is an ARN then the keyring MUST call the client provider with the region in the ARN.
  • If the key ID is not an ARN then the keyring MAY select a region with which to call the client provider.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The way this is worded make it sound to me like we are saying that there is something special about the region name extracted from this ARN that makes it different from the same region name extracted from some other ARN.

As I understand the spec, there is something special about this region. No other region derived from an ARN determines the region used by the client (or after this PR, the region given to the client supplier).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is something special about the generator key. That generator key happens to exist in a single region. The region is a side effect.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is this?

If the generator is formatted as either a well-formed key ARN, or a well-formed alias ARN, then an AWS region MUST be derived from the generator and used to call the client supplier. The client produced as output of the client supplier MUST be used to call KMS Encrypt/Decrypt/GenerateDataKey.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The client provider MUST require a region name.

I disagree.
The AWS SDK can pick up a default region in the same way it gets credentials.
The client provider is the interface to the AWS SDK.
Let the SDK handle it.
If there is any additional complication regarding what a default region should be,
putting this logic in a function is much easier for customers.

I have an example of what I'm thinking about here: aws/aws-encryption-sdk-javascript#231

If an AWS region cannot be extracted from the [generator](#generator) then the KMS Keyring MUST input a value denoting an unknown region.
If the [client supplier](#client-supplier) does not provide any client for the given region for this [KMS GenerateDataKey](#kms-generatedatakey) call, OnEncrypt MUST not modify the [encryption materials](#structures.md#encryption-materials.md)
and MUST fail.

When calling [KMS GenerateDataKey](#kms-generatedatakey), the keyring MUST call with a request constructed as follows:

Expand All @@ -179,7 +181,7 @@ If the call to [KMS GenerateDataKey](#kms-generatedatakey) does not succeed, OnE
modify the [encryption materials](#structures.md#encryption-materials) and MUST fail.

If the call succeeds, OnEncrypt MUST verify that the response `Plaintext` length matches the specification
of the [algorithm suite](#algorithm-suites.md).
of the [algorithm suite](algorithm-suites.md)'s Key Derivation Input Length field.
If it does not, OnEncrypt MUST fail.
If verified, OnEncrypt MUST do the following with the response from [KMS GenerateDataKey](#kms-generatedatakey):

Expand All @@ -193,12 +195,12 @@ If verified, OnEncrypt MUST do the following with the response from [KMS Generat
- append a new [record](#structures.md#record) to the [keyring trace](#structures.md#keyring-trace)
in the input [encryption materials](#structures.md#encryption-materials), constructed as follows:
- The string field KeyNamespace contains "aws-kms".
- The string field KeyName contains [generator](#generator).
- The string field KeyName contains the value of the KMS Keyring's [generator](#generator) field.
- The [flags](#structures.md$flags) field of this record includes exactly the following flags:
- [GENERATED DATA KEY](#structures.md#supported-flags)
- [ENCRYPTED DATA KEY](#structures.md#supported-flags)
- [SIGNED ENCRYPTION CONTEXT](#structures.md#supported-flags)

Given a plaintext data key in the [encryption materials](#structures.md#encryption-materials),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without this qualifier is it clear what plaintext data key is referring to on the next line? Maybe there is a way to restate this which is less confusing, or do you think it's not necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point about clarifying plaintext data key. I removed this line because I think it creates the appearance of an else to the if on line 154. I will try to find wording that addresses this.

OnEncrypt MUST attempt to encrypt the plaintext data key using each CMK specified in it's [key IDs](#key-ids) list.

Expand All @@ -207,11 +209,11 @@ as described above, OnEncrypt MUST also attempt to encrypt the plaintext data ke

To attempt to encrypt the plaintext data key with a particular CMK, OnEncrypt MUST call [KMS Encrypt](#kms-encrypt).

The [KMS client](#kms-client) that calls [KMS Encrypt](#kms-encrypt) MUST be the
client returned by the [client supplier](#client-supplier).
The client MUST be a client that calls the AWS region specified in the [generator](#generator) ARN.
If the [client supplier](#client-supplier) does not provide any client for the given region for this Encrypt call,
OnEncrypt MUST skip that particular CMK.
For each [KMS Encrypt](#kms-encrypt) call, if an AWS region can be extracted from the CMK ARN, then the
[KMS client](#kms-client) that calls [KMS Encrypt](#kms-encrypt) MUST be the client returned by the
[client supplier](#client-supplier) when given that region as input.
If an AWS region cannot be extracted from the CMK ARN then the KMS Keyring MUST input a value denoting an unknown region.
If the [client supplier](#client-supplier) does not provide any client for the given region for this Encrypt call, OnEncrypt MUST skip that particular CMK.

To encrypt the plaintext data key with a CMK, OnEncrypt MUST call [KMS Encrypt](#encrypt) with a request
constructed as follows:
Expand All @@ -235,7 +237,7 @@ If the call succeeds, OnEncrypt MUST do the following with the response from [KM
- append a new [record](#structures.md#record) to the [keyring trace](#structures.md#keyring-trace)
in the input [encryption materials](#structures.md#encryption-materials), constructed as follows:
- The string field KeyNamespace contains "aws-kms".
- The string field KeyName contains [generator](#generator).
- The string field KeyName contains the CMK ARN used for [KMS Encrypt](#kms-encrypt).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both the C and JS ESDKs do not do this. They both use the value returned from KMS.

- The [flags](#structures.md$flags) field of this record includes exactly the following flags:
- [ENCRYPTED DATA KEY](#structures.md#supported-flags)
- [SIGNED ENCRYPTION CONTEXT](#structures.md#supported-flags)
Expand All @@ -252,7 +254,7 @@ to decrypt depends on if this keyring is a [discovery keyring](#discovery) or no

If this keyring is a [discovery keyring](#discovery), OnDecrypt MUST attempt to decrypt every
[encrypted data key](#structures.md#encrypted-data-key) in the input encrypted data key list
with the following conditions, until it successfully decrypts one:
with the following condition, until it successfully decrypts one:

- the [key provider ID](#structures.md#key-provider-id) field has the value "aws-kms"

Expand All @@ -266,11 +268,10 @@ in the input encrypted data key list with the following conditions, until it suc
To attempt to decrypt a particular [encrypted data key](#structures.md#encrypted-data-key),
OnDecrypt MUST call [KMS Decrypt](#kms-decrypt).

The [KMS client](#kms-client) that calls [KMS Decrypt](#kms-decrypt) MUST be the
client returned by the [client supplier](#client-supplier).
The client MUST be a client that calls the AWS region specified in the [generator](#generator) ARN.
If the [client supplier](#client-supplier) does not provide any client for the given region for the Decrypt call,
OnDecrypt MUST skip that particular [encrypted data key](#encrypted-data-key).
For each [KMS Decrypt](#kms-decrypt) call, if an AWS region can be extracted from the [encrypted data key](structures.md#encrypted-data-key)'s [key provider info](structures.md#key-provider-information).
The KMS Keyring MUST call [KMS Decrypt](#kms-decrypt) using the client supplied by the [client supplier](#client-supplier), given the region as input.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoops, this wording needs to also be updated to remove the "if an AWS region can be extracted".


If the client supplier does not provide any client for the given region for this Decrypt call, OnDecrypt MUST skip that particular [encrypted data key](#encrypted-data-key).

When calling [KMS Decrypt](#kms-decrypt), the keyring MUST call with a request constructed as follows:

Expand Down Expand Up @@ -300,7 +301,7 @@ If the response is successfully verified, OnDecrypt MUST do the following with t
- append a new [record](#structures.md#record) to the [keyring trace](#structures.md#keyring-trace)
in the input [encryption materials](#structures.md#encryption-materials), constructed as follows:
- The string field KeyNamespace contains "aws-kms".
- The string field KeyName contains [generator](#generator).
- The string field KeyName contains the [encrypted data key's key provider info](structures.md#key-provider-information).
- The [flags](#structures.md$flags) field of this record includes exactly the following flags:
- [DECRYPTED DATA KEY](#structures.md#supported-flags)
- [VERIFIED ENCRYPTION CONTEXT](#structures.md#supported-flags)
Expand Down