-
Notifications
You must be signed in to change notification settings - Fork 334
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
S3 Server-Side Encryption #784
Conversation
@@ -1,3 +1,87 @@ | |||
// KMS key for encrypting CloudTrail logs |
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.
Is there a reference in AWS docs for the policies below? If so could you add it? 🙇
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.
Sadly not (that I'm aware of). I got the policy by manually creating a CloudTrail, encrypting it from the console, and seeing what policy AWS automatically attached to the key. I'll add a comment to that effect
|
||
data "aws_iam_policy_document" "cloudtrail_encryption" { | ||
statement { | ||
sid = "Enable IAM User Permissions" |
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.
Isn't this policy to allow root to manage the key?
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.
Sort of - it's a required policy that enables any IAM user in the account (with the appropriate permissions) to manage the key - https://docs.aws.amazon.com/kms/latest/developerguide/key-policies.html#key-policy-default-allow-root-enable-iam
So if this permission is granted to "root", I as an IAM user in the account am allowed to modify the key, provided that I have kms:*
permissions on my IAM user
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.
Two comments, LGTM otherwise!
to: @jacknagz
cc: @airbnb/streamalert-maintainers
size: medium
part of #758
Background
S3 Server-Side Encryption adds a layer of protection to data in S3 buckets. If encrypted with KMS, additional permissions to use the KMS key are required for access (in addition to access to S3).
Changes
streamalert.data
,streamalert.secrets
,streamalert.terraform.state
,streamalerts
streamalert.secrets
is actually encrypted twice (client-side and server-side). We will eventually move these into Secrets Manager and get rid of this S3 bucket completelymoto
, some of the Lambda mocking has taken longer or started throwing exceptions. For these, I just used basic mocks in place ofmoto
.Testing
master
, then apply these changes:Athena queries worked the same (with no observable performance impact) with none, some, or all objects KMS encrypted.