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 AWS KMS Encryption Context #76

Merged
merged 6 commits into from
Aug 22, 2016

Conversation

dictcp
Copy link
Contributor

@dictcp dictcp commented Aug 3, 2016

  • user can specify "Encryption Context" to each data key encrypted by KMS
  • the Encryption Context can be used to restrict access with KMS Policy

some reference on encryption context added:
http://docs.aws.amazon.com/kms/latest/developerguide/policy-conditions.html
http://docs.aws.amazon.com/kms/latest/developerguide/encryption-context.html

@@ -178,6 +178,9 @@ def main():
help="path to config file, disable recursive search"
" (default: {default})"
.format(default=DEFAULT_CONFIG_FILE))
argparser.add_argument('--encryption-context', dest='context',
help="KMS encryption context: "
"key-value pair dict encoded in JSON")
Copy link
Contributor

Choose a reason for hiding this comment

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

I haven't used KMS encryption contexts, but would there be a way to expand this to not require JSON on the command line but take individual parameters instead? We already have a custom csv format for specifying multiple kms and pgp on the command line, and I'd prefer to avoid introducing another format.

@jvehent
Copy link
Contributor

jvehent commented Aug 3, 2016

This is a interesting feature, thanks for sending the patch!

Could you add some documentation to the README to explain how this is meant to be used? I'd also suggest adding a couple unit tests.

@dictcp
Copy link
Contributor Author

dictcp commented Aug 8, 2016

@jvehent thanks for the comment.

the code is refactored to use context string like "Key1:Value1,Key2:Value2" to better align with existing convention. also I have update the README.md and unit test.

regrading to encryption context, you may be interested in related doc from credstash and sneaker (both of them support encryption context as well)
https://github.com/fugue/credstash#controlling-and-auditing-secrets
https://github.com/codahale/sneaker#encryption-contexts


SOPS has the ability to use AWS KMS key policy and encryption context
<http://docs.aws.amazon.com/kms/latest/developerguide/encryption-context.html>
to further fine control access under the same master key.
Copy link
Contributor

Choose a reason for hiding this comment

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

to refine the access control of a given KMS master key.

@jvehent
Copy link
Contributor

jvehent commented Aug 8, 2016

This is getting close, I just have a couple comments on the documentation. Once you fix them, I'll merge the patch and we can test it for a couple weeks before releasing 1.14.

@jvehent
Copy link
Contributor

jvehent commented Aug 22, 2016

Thanks for the patches! The build is breaking before of unrelated issues, so I'm going to merge this and fix the rest in master.

This looks neat, much appreciated! + 💯 👍

@jvehent jvehent merged commit 06b1c13 into getsops:master Aug 22, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants