Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
23 changes: 23 additions & 0 deletions catalog/region-restriction-templates/DenyRegions.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@
- "iam:*"
- "importexport:*"
- "kms:*"
%{ if cloudfront_lambda_edge_enabled == "true" }
- "lambda:*"
%{ endif }
Comment on lines +31 to +33
Copy link
Contributor

Choose a reason for hiding this comment

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

This is good: if cloudfront_lambda_edge_enabled != true (the default, right?), then the rule is not changed, and there will be no Terraform drift.

However, let's clean up the whitespace and remove the redundant condition

Suggested change
%{ if cloudfront_lambda_edge_enabled == "true" }
- "lambda:*"
%{ endif }
%{~ if cloudfront_lambda_edge_enabled && strcontains(denied_regions, "us-east-1") ~}
- "lambda:*"
%{~ endif ~}

- "mobileanalytics:*"
- "networkmanager:*"
- "organizations:*"
Expand All @@ -37,6 +40,9 @@
- "s3:GetAccountPublic*"
- "s3:ListAllMyBuckets"
- "s3:PutAccountPublic*"
%{ if servicequotas_enabled == "true" }
- "servicequotas:*"
%{ endif }
Comment on lines +43 to +45
Copy link
Contributor

Choose a reason for hiding this comment

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

You need to handle servicequotas_enabled just like cloudfront_lambda_edge_enabled

- "shield:*"
- "sts:*"
- "support:*"
Expand All @@ -56,3 +62,20 @@
%{ endfor }
resources:
- "*"

- sid: "DenyLambdaInRegions"
effect: "Deny"
actions:
- "lambda:*"
condition:
- test: "StringEqualsIgnoreCase"
variable: "aws:RequestedRegion"
# List of denied regions
values:
%{ for r in split(",", denied_regions) }
%{ if cloudfront_lambda_edge_enabled != "true" || trimspace(r) != "us-east-1" }
- ${trimspace(r)}
%{ endif }
%{ endfor }
resources:
- "*"
Comment on lines +66 to +81
Copy link
Contributor

Choose a reason for hiding this comment

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

This entire statement is not needed unless cloudfront_lambda_edge_enabled == true and us-east-1 is one of several denied regions, and will cause drift and be useless otherwise.

Suggested change
- sid: "DenyLambdaInRegions"
effect: "Deny"
actions:
- "lambda:*"
condition:
- test: "StringEqualsIgnoreCase"
variable: "aws:RequestedRegion"
# List of denied regions
values:
%{ for r in split(",", denied_regions) }
%{ if cloudfront_lambda_edge_enabled != "true" || trimspace(r) != "us-east-1" }
- ${trimspace(r)}
%{ endif }
%{ endfor }
resources:
- "*"
%{ if cloudfront_lambda_edge_enabled && strcontains(denied_regions, "us-east-1") && trimspace(denied_regions) != "us-east-1" }
- sid: "DenyLambdaInRegions"
effect: "Deny"
actions:
- "lambda:*"
condition:
- test: "StringEqualsIgnoreCase"
variable: "aws:RequestedRegion"
# List of denied regions
values:
%{~ for r in split(",", denied_regions) ~}
%{~ if trimspace(r) != "us-east-1" ~}
- ${trimspace(r)}
%{~ endif ~}
%{~ endfor ~}
resources:
- "*"
%{ endif }

Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@
- "iam:*"
- "importexport:*"
- "kms:*"
%{ if cloudfront_lambda_edge_enabled == "true" }
- "lambda:*"
%{ endif }
Comment on lines +31 to +33
Copy link
Contributor

Choose a reason for hiding this comment

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

Same idea as with DenyLambdaInRegions

Suggested change
%{ if cloudfront_lambda_edge_enabled == "true" }
- "lambda:*"
%{ endif }
%{~ if cloudfront_lambda_edge_enabled && !strcontains(allowed_regions, "us-east-1") ~}
- "lambda:*"
%{~ endif ~}

- "mobileanalytics:*"
- "networkmanager:*"
- "organizations:*"
Expand All @@ -37,6 +40,9 @@
- "s3:GetAccountPublic*"
- "s3:ListAllMyBuckets"
- "s3:PutAccountPublic*"
%{ if servicequotas_enabled == "true" }
- "servicequotas:*"
%{ endif }
Comment on lines +43 to +45
Copy link
Contributor

Choose a reason for hiding this comment

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

Again, needs to be handled like cloudfront_lambda_edge_enabled

- "shield:*"
- "sts:*"
- "support:*"
Expand All @@ -56,3 +62,22 @@
%{ endfor }
resources:
- "*"

- sid: "RestrictLambdaToSpecifiedRegions"
effect: "Deny"
actions:
- "lambda:*"
condition:
- test: "StringNotEqualsIgnoreCase"
variable: "aws:RequestedRegion"
# List of allowed regions
values:
# Us east-1 required for cloudfront lambda on edge
%{ if cloudfront_lambda_edge_enabled == "true" && !contains(split(",", allowed_regions), "us-east-1") }
- us-east-1
%{ endif }
%{ for r in split(",", allowed_regions) }
- ${trimspace(r)}
%{ endfor }
resources:
- "*"
Comment on lines +66 to +83
Copy link
Contributor

Choose a reason for hiding this comment

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

Same idea as with DenyLambdaInRegions

Suggested change
- sid: "RestrictLambdaToSpecifiedRegions"
effect: "Deny"
actions:
- "lambda:*"
condition:
- test: "StringNotEqualsIgnoreCase"
variable: "aws:RequestedRegion"
# List of allowed regions
values:
# Us east-1 required for cloudfront lambda on edge
%{ if cloudfront_lambda_edge_enabled == "true" && !contains(split(",", allowed_regions), "us-east-1") }
- us-east-1
%{ endif }
%{ for r in split(",", allowed_regions) }
- ${trimspace(r)}
%{ endfor }
resources:
- "*"
%{ if cloudfront_lambda_edge_enabled && !strcontains(allowed_regions, "us-east-1") }
- sid: "RestrictLambdaToSpecifiedRegions"
effect: "Deny"
actions:
- "lambda:*"
condition:
- test: "StringNotEqualsIgnoreCase"
variable: "aws:RequestedRegion"
# List of allowed regions
values:
# Us east-1 required for cloudfront lambda on edge
- us-east-1
%{~ for r in split(",", allowed_regions) ~}
- ${trimspace(r)}
%{~ endfor ~}
resources:
- "*"
%{ endif }

14 changes: 8 additions & 6 deletions examples/complete/fixtures.us-east-2.tfvars
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,12 @@ name = "scp"
service_control_policy_description = "Test Service Control Policy"

parameters = {
ami_creator_account = "account_creator"
ami_tag_key = "ami_tag_key"
ami_tag_value = "ami_tag_value"
allowed_regions = "eu-central-1, eu-west-1"
denied_regions = "sa-east-1"
s3_regions_lockdown = "eu-central-1, eu-west-1"
ami_creator_account = "account_creator"
ami_tag_key = "ami_tag_key"
ami_tag_value = "ami_tag_value"
allowed_regions = "eu-central-1, eu-west-1"
denied_regions = "sa-east-1"
s3_regions_lockdown = "eu-central-1, eu-west-1"
cloudfront_lambda_edge_enabled = "false"
servicequotas_enabled = "false"
}