feat: Add ability to create deny insecure transport policy

terraform-aws-modules · Mar 29, 2021 · d7af646 · d7af646
1 parent 1f1689d
commit d7af646
Show file tree

Hide file tree

Showing 4 changed files with 58 additions and 7 deletions.
diff --git a/README.md b/README.md
@@ -112,6 +112,7 @@ No Modules.
 | acceleration\_status | (Optional) Sets the accelerate configuration of an existing bucket. Can be Enabled or Suspended. | `string` | `null` | no |
 | acl | (Optional) The canned ACL to apply. Defaults to 'private'. Conflicts with `grant` | `string` | `"private"` | no |
 | attach\_elb\_log\_delivery\_policy | Controls if S3 bucket should have ELB log delivery policy attached | `bool` | `false` | no |
+| attach\_deny\_insecure\_transport\_policy | Controls if S3 bucket should have [deny non-encrypted transport policy](https://aws.amazon.com/premiumsupport/knowledge-center/s3-bucket-policy-for-config-rule/) attached | `bool` | `false`| no |
 | attach\_policy | Controls if S3 bucket should have bucket policy attached (set to `true` to use value of `policy` as bucket policy) | `bool` | `false` | no |
 | attach\_public\_policy | Controls if a user defined public bucket policy will be attached (set to `false` to allow upstream to apply defaults to the bucket) | `bool` | `true` | no |
 | block\_public\_acls | Whether Amazon S3 should block public ACLs for this bucket. | `bool` | `false` | no |

diff --git a/examples/complete/main.tf b/examples/complete/main.tf
@@ -51,10 +51,11 @@ data "aws_iam_policy_document" "bucket_policy" {
 module "log_bucket" {
   source = "../../"
 
-  bucket                         = "logs-${random_pet.this.id}"
-  acl                            = "log-delivery-write"
-  force_destroy                  = true
-  attach_elb_log_delivery_policy = true
+  bucket                                = "logs-${random_pet.this.id}"
+  acl                                   = "log-delivery-write"
+  force_destroy                         = true
+  attach_elb_log_delivery_policy        = true
+  attach_deny_insecure_transport_policy = true
 }
 
 module "cloudfront_log_bucket" {

diff --git a/main.tf b/main.tf
@@ -231,10 +231,23 @@ resource "aws_s3_bucket" "this" {
 }
 
 resource "aws_s3_bucket_policy" "this" {
-  count = var.create_bucket && (var.attach_elb_log_delivery_policy || var.attach_policy) ? 1 : 0
+  count = var.create_bucket && (var.attach_elb_log_delivery_policy || var.attach_deny_insecure_transport_policy || var.attach_policy) ? 1 : 0
 
   bucket = aws_s3_bucket.this[0].id
-  policy = var.attach_elb_log_delivery_policy ? data.aws_iam_policy_document.elb_log_delivery[0].json : var.policy
+  policy = (
+    var.attach_elb_log_delivery_policy && var.attach_deny_insecure_transport_policy ? data.aws_iam_policy_document.combined[0].json
+    : var.attach_elb_log_delivery_policy ? data.aws_iam_policy_document.elb_log_delivery[0].json
+    : var.attach_deny_insecure_transport_policy ? data.aws_iam_policy_document.deny_insecure_transport[0].json
+    : var.policy
+  )
+}
+
+data "aws_iam_policy_document" "combined" {
+  count = var.create_bucket && var.attach_elb_log_delivery_policy && var.attach_deny_insecure_transport_policy ? 1 : 0
+  source_policy_documents = [
+    data.aws_iam_policy_document.elb_log_delivery[0].json,
+    data.aws_iam_policy_document.deny_insecure_transport[0].json
+  ]
 }
 
 # AWS Load Balancer access log delivery policy
@@ -265,12 +278,42 @@ data "aws_iam_policy_document" "elb_log_delivery" {
   }
 }
 
+data "aws_iam_policy_document" "deny_insecure_transport" {
+  count = var.create_bucket && var.attach_deny_insecure_transport_policy ? 1 : 0
+  statement {
+    sid    = "denyInsecureTransport"
+    effect = "Deny"
+
+    actions = [
+      "s3:*",
+    ]
+
+    resources = [
+      aws_s3_bucket.this[0].arn,
+      "${aws_s3_bucket.this[0].arn}/*",
+    ]
+
+    principals {
+      type        = "*"
+      identifiers = ["*"]
+    }
+
+    condition {
+      test     = "Bool"
+      variable = "aws:SecureTransport"
+      values = [
+        "false"
+      ]
+    }
+  }
+}
+
 resource "aws_s3_bucket_public_access_block" "this" {
   count = var.create_bucket && var.attach_public_policy ? 1 : 0
 
   # Chain resources (s3_bucket -> s3_bucket_policy -> s3_bucket_public_access_block)
   # to prevent "A conflicting conditional operation is currently in progress against this resource."
-  bucket = (var.attach_elb_log_delivery_policy || var.attach_policy) ? aws_s3_bucket_policy.this[0].id : aws_s3_bucket.this[0].id
+  bucket = (var.attach_elb_log_delivery_policy || var.attach_deny_insecure_transport_policy || var.attach_policy) ? aws_s3_bucket_policy.this[0].id : aws_s3_bucket.this[0].id
 
   block_public_acls       = var.block_public_acls
   block_public_policy     = var.block_public_policy

diff --git a/variables.tf b/variables.tf
@@ -10,6 +10,12 @@ variable "attach_elb_log_delivery_policy" {
   default     = false
 }
 
+variable "attach_deny_insecure_transport_policy" {
+  description = "Controls if S3 bucket should have deny non-encrypted transport policy attached"
+  type        = bool
+  default     = false
+}
+
 variable "attach_policy" {
   description = "Controls if S3 bucket should have bucket policy attached (set to `true` to use value of `policy` as bucket policy)"
   type        = bool