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

encrypt *LB access logs if KMS key is specified #737

Merged
merged 1 commit into from
Feb 13, 2024

Conversation

ab77
Copy link
Contributor

@ab77 ab77 commented Feb 9, 2024

  • only S3-SSE is available for *LB log delivery

(Override all values in parentheses)

(Run yamllint folder/template.yaml, cfn-lint -i E1019 E3002 E2520 -t folder/template.yaml, and aws cloudformation validate-template --template-body file://folder/template.yaml before you open a PR)

(Do not include multiple changes in one PR. Open additional PRs instead.)


Assume encryption of *LB access logs using S3-SSE if KMS key is provided.

@michaelwittig
Copy link
Contributor

I'm a bit confused that setting a KMS key results in a policy that only allows writes using S3-SSE.

I wonder if a new Access type (e.g. ElbAccessLogWriteEncrypted) would be more explicit?

@ab77
Copy link
Contributor Author

ab77 commented Feb 9, 2024

I'm a bit confused that setting a KMS key results in a policy that only allows writes using S3-SSE.

I wonder if a new Access type (e.g. ElbAccessLogWriteEncrypted) would be more explicit?

That's a much better idea, I'll refactor.

@ab77
Copy link
Contributor Author

ab77 commented Feb 12, 2024

I'm a bit confused that setting a KMS key results in a policy that only allows writes using S3-SSE.
I wonder if a new Access type (e.g. ElbAccessLogWriteEncrypted) would be more explicit?

That's a much better idea, I'll refactor.

@michaelwittig I've tested the new approach locally, works.

@ab77 ab77 force-pushed the ab77/encrypt-access-logs branch 3 times, most recently from 92f87c7 to 69b4be5 Compare February 12, 2024 22:09
* only S3-SSE is available for *LB log delivery
@michaelwittig michaelwittig merged commit f8d787f into widdix:master Feb 13, 2024
1 check passed
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