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.

Sign up for GitHub

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

Modify rule S6249: fix Terraform code examples #4502

Merged
merged 2 commits into from
Nov 14, 2024
Merged

Modify rule S6249: fix Terraform code examples #4502

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6f73b3f
Improve rule S6249: fix Terraform code examples
GabinL21 Nov 14, 2024
70203ee
Improve rule S6249: fix Terraform code examples
GabinL21 Nov 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 10 additions & 6 deletions rules/S6249/terraform/rule.adoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ include::../recommended.adoc[]

No secure policy is attached to this bucket:

[source,terraform]
----
resource "aws_s3_bucket" "mynoncompliantbucket" { # Sensitive
bucket = "mynoncompliantbucketname"
Expand All @@ -16,6 +17,7 @@ resource "aws_s3_bucket" "mynoncompliantbucket" { # Sensitive

A policy is defined but forces only HTTPs communication for some users:

[source,terraform]
----
resource "aws_s3_bucket" "mynoncompliantbucket" { # Sensitive
bucket = "mynoncompliantbucketname"
Expand All @@ -31,13 +33,13 @@ resource "aws_s3_bucket_policy" "mynoncompliantbucketpolicy" {
{
Sid = "HTTPSOnly"
Effect = "Deny"
Principal = [
"arn:aws:iam::123456789123:root"
] # secondary location: only one principal is forced to use https
Principal = {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The previous code example was "right" too, but we're not currently detecting it with our rule implementation.

This change ensures that we're documenting a case that is working with our current implementation, while the implementation itself will be updated in other tickets.

Copy link
Contributor

Choose a reason for hiding this comment

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

Pasting the noncompliant example does not raise an issue in the current implementation (if I didn't do any mistake)

Copy link
Contributor

Choose a reason for hiding this comment

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

Nevermind, was a mistake on my setup.

"AWS": "arn:aws:iam::123456789123:root"
} # secondary location: only one principal is forced to use https
Action = "s3:*"
Resource = [
aws_s3_bucket.mynoncompliantbucketpolicy.arn,
"${aws_s3_bucket.mynoncompliantbucketpolicy.arn}/*",
aws_s3_bucket.mynoncompliantbucket.arn,
"${aws_s3_bucket.mynoncompliantbucket.arn}/*",
]
Condition = {
Bool = {
Expand Down Expand Up @@ -70,7 +72,9 @@ resource "aws_s3_bucket_policy" "mycompliantpolicy" {
{
Sid = "HTTPSOnly"
Effect = "Deny"
Principal = "*"
Principal = {
"AWS": "*"
}
Action = "s3:*"
Resource = [
aws_s3_bucket.mycompliantbucket.arn,
Expand Down