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

Adds flag and policy to require all buckets requests to be over SSL #67

Merged
merged 4 commits into from
Jul 22, 2021

Conversation

bradj
Copy link
Contributor

@bradj bradj commented Jul 22, 2021

what

Adds flag that requires all S3 requests to be over SSL

why

Compliance and security

references

n/a

@bradj bradj requested review from a team as code owners July 22, 2021 18:58
@bradj bradj requested a review from a team as a code owner July 22, 2021 18:59
@bradj bradj requested review from adamcrews and florian0410 and removed request for a team July 22, 2021 18:59
Benbentwo
Benbentwo previously approved these changes Jul 22, 2021
@Benbentwo
Copy link
Member

/test all

main.tf Outdated
for_each = var.allow_ssl_requests_only ? [1] : []

content {
sid = "ForceSSLOnlyAccess"
Copy link
Member

Choose a reason for hiding this comment

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

I feel like the language of the Sid should match the language of the feature flag

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@osterman updated

@Benbentwo
Copy link
Member

/test all

@bradj bradj merged commit 2c731bd into master Jul 22, 2021
@bradj bradj deleted the only-allow-ssl-requests branch July 22, 2021 19:18
sid = "AllowSSLRequestsOnly"
effect = "Deny"
actions = ["s3:*"]
resources = [local.bucket_arn, "${local.bucket_arn}/*"]
Copy link
Member

Choose a reason for hiding this comment

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

@bradj we should have updated the bucket ARNs in the other places as well

resources = ["${aws_s3_bucket.default[0].arn}/*"]

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.

5 participants