-
Notifications
You must be signed in to change notification settings - Fork 26
docs: Address KMS keyring on-encrypt issues #74
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
Conversation
framework/kms-keyring.md
Outdated
|
||
However, if the keyring attempts to decrypt using CMK C and cannot, | ||
this failure still honors the configured intent and MUST NOT halt decryption. | ||
The configured intent is that they keyring MUST *attempt* with these CMKs, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would try to make it a bit more clear that these CMKs
refers to the CMKs configured with this keyring, not "CMK A, B, and C". Maybe make this more clear by explicitly giving the example of "If you configure the KMS keyring for decrypt with CMKs C, D, E" or something like that? Or is that more confusing?
they are stating their intent that they need each one of those CMKs to be able to | ||
independently decrypt the resulting encrypted message. | ||
|
||
For example, if a user configures a KMS keyring with CMKs A, B, and C |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO There should be a separate "Example Behaviors" section.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel that would be distracting in this particular case. What I'm trying to do here is demonstrate how this intent manifests as a "show by example" means of explaining the behavior.
For example, if a user configures a KMS keyring with CMK C | ||
and uses it to decrypt an encrypted message | ||
that contains encrypted data keys for CMKs A, B, and C, | ||
then the keyring will attempt to decrypt using CMK C. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is only true if the string identifying the CMK is a well-formed key ARN.
Co-Authored-By: Wesley Rosenblum <55108558+WesleyRosenblum@users.noreply.github.com>
…ent to communicate inability to supply client
… any CMKs fail on encrypt
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good.
Issue #, if available:
resolves: #73
Description of changes:
This resolves the discrepancies found between the intent of KMS keyring behavior and configuration and the spec and adds a description of that intent.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.