Always require TLS connection to S3 bucket (#139)

Nuru · web-flow · commit 99453ccfc0d0 · 2023-05-31T13:02:49.000+03:00
* Always deny unencrypted HTTP connections

* tflint fixes
diff --git a/README.md b/README.md
@@ -286,7 +286,7 @@ Available targets:
 | [aws_s3_bucket_server_side_encryption_configuration.default](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/s3_bucket_server_side_encryption_configuration) | resource |
 | [aws_s3_bucket_versioning.default](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/s3_bucket_versioning) | resource |
 | [local_file.terraform_backend_config](https://registry.terraform.io/providers/hashicorp/local/latest/docs/resources/file) | resource |
-| [aws_iam_policy_document.prevent_unencrypted_uploads](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/iam_policy_document) | data source |
+| [aws_iam_policy_document.bucket_policy](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/iam_policy_document) | data source |
 | [aws_iam_policy_document.replication](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/iam_policy_document) | data source |
 | [aws_iam_policy_document.replication_sts](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/iam_policy_document) | data source |
 | [aws_region.current](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/region) | data source |
diff --git a/docs/terraform.md b/docs/terraform.md
@@ -41,7 +41,7 @@
 | [aws_s3_bucket_server_side_encryption_configuration.default](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/s3_bucket_server_side_encryption_configuration) | resource |
 | [aws_s3_bucket_versioning.default](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/s3_bucket_versioning) | resource |
 | [local_file.terraform_backend_config](https://registry.terraform.io/providers/hashicorp/local/latest/docs/resources/file) | resource |
-| [aws_iam_policy_document.prevent_unencrypted_uploads](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/iam_policy_document) | data source |
+| [aws_iam_policy_document.bucket_policy](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/iam_policy_document) | data source |
 | [aws_iam_policy_document.replication](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/iam_policy_document) | data source |
 | [aws_iam_policy_document.replication_sts](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/iam_policy_document) | data source |
 | [aws_region.current](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/region) | data source |
diff --git a/examples/complete/versions.tf b/examples/complete/versions.tf
@@ -4,7 +4,7 @@ terraform {
   required_providers {
     aws = {
       source  = "hashicorp/aws"
-      version = ">= 2.0"
+      version = ">= 4.9.0"
     }
     local = {
       source  = "hashicorp/local"
diff --git a/main.tf b/main.tf
@@ -8,10 +8,7 @@ locals {
 
   prevent_unencrypted_uploads = local.enabled && var.prevent_unencrypted_uploads
 
-  policy = local.prevent_unencrypted_uploads ? join(
-    "",
-    data.aws_iam_policy_document.prevent_unencrypted_uploads.*.json
-  ) : ""
+  policy = one(data.aws_iam_policy_document.bucket_policy[*].json)
 
   terraform_backend_config_file = format(
     "%s/%s",
@@ -56,63 +53,71 @@ module "bucket_label" {
 
 data "aws_region" "current" {}
 
-data "aws_iam_policy_document" "prevent_unencrypted_uploads" {
-  count = local.prevent_unencrypted_uploads ? 1 : 0
+data "aws_iam_policy_document" "bucket_policy" {
+  count = local.enabled ? 1 : 0
 
-  statement {
-    sid = "DenyIncorrectEncryptionHeader"
+  dynamic "statement" {
+    for_each = local.prevent_unencrypted_uploads ? ["true"] : []
 
-    effect = "Deny"
+    content {
+      sid = "DenyIncorrectEncryptionHeader"
 
-    principals {
-      identifiers = ["*"]
-      type        = "AWS"
-    }
+      effect = "Deny"
 
-    actions = [
-      "s3:PutObject"
-    ]
+      principals {
+        identifiers = ["*"]
+        type        = "AWS"
+      }
 
-    resources = [
-      "${var.arn_format}:s3:::${local.bucket_name}/*",
-    ]
-
-    condition {
-      test     = "StringNotEquals"
-      variable = "s3:x-amz-server-side-encryption"
+      actions = [
+        "s3:PutObject"
+      ]
 
-      values = [
-        "AES256",
-        "aws:kms"
+      resources = [
+        "${var.arn_format}:s3:::${local.bucket_name}/*",
       ]
+
+      condition {
+        test     = "StringNotEquals"
+        variable = "s3:x-amz-server-side-encryption"
+
+        values = [
+          "AES256",
+          "aws:kms"
+        ]
+      }
     }
   }
 
-  statement {
-    sid = "DenyUnEncryptedObjectUploads"
+  dynamic "statement" {
+    for_each = local.prevent_unencrypted_uploads ? ["true"] : []
 
-    effect = "Deny"
+    content {
+      sid = "DenyUnEncryptedObjectUploads"
 
-    principals {
-      identifiers = ["*"]
-      type        = "AWS"
-    }
+      effect = "Deny"
 
-    actions = [
-      "s3:PutObject"
-    ]
+      principals {
+        identifiers = ["*"]
+        type        = "AWS"
+      }
 
-    resources = [
-      "${var.arn_format}:s3:::${local.bucket_name}/*",
-    ]
-
-    condition {
-      test     = "Null"
-      variable = "s3:x-amz-server-side-encryption"
+      actions = [
+        "s3:PutObject"
+      ]
 
-      values = [
-        "true"
+      resources = [
+        "${var.arn_format}:s3:::${local.bucket_name}/*",
       ]
+
+      condition {
+        test     = "Null"
+        variable = "s3:x-amz-server-side-encryption"
+
+        values = [
+          "true"
+        ]
+      }
     }
   }
 
@@ -157,14 +162,14 @@ resource "aws_s3_bucket" "default" {
 resource "aws_s3_bucket_policy" "default" {
   count = local.bucket_enabled ? 1 : 0
 
-  bucket = one(aws_s3_bucket.default.*.id)
+  bucket = one(aws_s3_bucket.default[*].id)
   policy = local.policy
 }
 
 resource "aws_s3_bucket_acl" "default" {
   count = local.bucket_enabled && !var.bucket_ownership_enforced_enabled ? 1 : 0
 
-  bucket = one(aws_s3_bucket.default.*.id)
+  bucket = one(aws_s3_bucket.default[*].id)
   acl    = var.acl
 
   # Default "bucket ownership controls" for new S3 buckets is "BucketOwnerEnforced", which disables ACLs.
@@ -175,7 +180,7 @@ resource "aws_s3_bucket_acl" "default" {
 resource "aws_s3_bucket_versioning" "default" {
   count = local.bucket_enabled ? 1 : 0
 
-  bucket = one(aws_s3_bucket.default.*.id)
+  bucket = one(aws_s3_bucket.default[*].id)
 
   versioning_configuration {
     status     = "Enabled"
@@ -186,7 +191,7 @@ resource "aws_s3_bucket_versioning" "default" {
 resource "aws_s3_bucket_server_side_encryption_configuration" "default" {
   count = local.bucket_enabled ? 1 : 0
 
-  bucket = one(aws_s3_bucket.default.*.id)
+  bucket = one(aws_s3_bucket.default[*].id)
 
   rule {
     apply_server_side_encryption_by_default {
@@ -198,7 +203,7 @@ resource "aws_s3_bucket_server_side_encryption_configuration" "default" {
 resource "aws_s3_bucket_logging" "default" {
   count = local.bucket_enabled && length(var.logging) > 0 ? 1 : 0
 
-  bucket = one(aws_s3_bucket.default.*.id)
+  bucket = one(aws_s3_bucket.default[*].id)
 
   target_bucket = var.logging[0].target_bucket
   target_prefix = var.logging[0].target_prefix
@@ -207,7 +212,7 @@ resource "aws_s3_bucket_logging" "default" {
 resource "aws_s3_bucket_public_access_block" "default" {
   count = local.bucket_enabled && var.enable_public_access_block ? 1 : 0
 
-  bucket                  = one(aws_s3_bucket.default.*.id)
+  bucket                  = one(aws_s3_bucket.default[*].id)
   block_public_acls       = var.block_public_acls
   ignore_public_acls      = var.ignore_public_acls
   block_public_policy     = var.block_public_policy
@@ -218,7 +223,7 @@ resource "aws_s3_bucket_public_access_block" "default" {
 # See https://docs.aws.amazon.com/AmazonS3/latest/userguide/about-object-ownership.html
 resource "aws_s3_bucket_ownership_controls" "default" {
   count  = local.bucket_enabled ? 1 : 0
-  bucket = one(aws_s3_bucket.default.*.id)
+  bucket = one(aws_s3_bucket.default[*].id)
 
   rule {
     object_ownership = var.bucket_ownership_enforced_enabled ? "BucketOwnerEnforced" : "BucketOwnerPreferred"
diff --git a/outputs.tf b/outputs.tf
@@ -19,17 +19,17 @@ output "s3_replication_role_arn" {
 }
 
 output "dynamodb_table_name" {
-  value       = one(aws_dynamodb_table.with_server_side_encryption.*.name)
+  value       = one(aws_dynamodb_table.with_server_side_encryption[*].name)
   description = "DynamoDB table name"
 }
 
 output "dynamodb_table_id" {
-  value       = one(aws_dynamodb_table.with_server_side_encryption.*.id)
+  value       = one(aws_dynamodb_table.with_server_side_encryption[*].id)
   description = "DynamoDB table ID"
 }
 
 output "dynamodb_table_arn" {
-  value       = one(aws_dynamodb_table.with_server_side_encryption.*.arn)
+  value       = one(aws_dynamodb_table.with_server_side_encryption[*].arn)
   description = "DynamoDB table ARN"
 }
 
diff --git a/replication.tf b/replication.tf
@@ -71,8 +71,8 @@ data "aws_iam_policy_document" "replication" {
       "s3:ListBucket"
     ]
     resources = [
-      join("", aws_s3_bucket.default.*.arn),
-      "${join("", aws_s3_bucket.default.*.arn)}/*"
+      join("", aws_s3_bucket.default[*].arn),
+      "${join("", aws_s3_bucket.default[*].arn)}/*"
     ]
   }
 
diff --git a/variables.tf b/variables.tf
@@ -71,6 +71,7 @@ variable "ignore_public_acls" {
 }
 
 variable "block_public_policy" {
+  type        = bool
   description = "Whether Amazon S3 should block public bucket policies for this bucket"
   default     = true
 }

Original file line number	Diff line number	Diff line change
`@@ -4,7 +4,7 @@ terraform {`
`4`	`4`	`required_providers {`
`5`	`5`	`aws = {`
`6`	`6`	`source = "hashicorp/aws"`
`7`		`- version = ">= 2.0"`
	`7`	`+ version = ">= 4.9.0"`
`8`	`8`	`}`
`9`	`9`	`local = {`
`10`	`10`	`source = "hashicorp/local"`
Original file line number	Diff line number	Diff line change
`@@ -19,17 +19,17 @@ output "s3_replication_role_arn" {`
`19`	`19`	`}`
`20`	`20`
`21`	`21`	`output "dynamodb_table_name" {`
`22`		`- value = one(aws_dynamodb_table.with_server_side_encryption.*.name)`
	`22`	`+ value = one(aws_dynamodb_table.with_server_side_encryption[*].name)`
`23`	`23`	`description = "DynamoDB table name"`
`24`	`24`	`}`
`25`	`25`
`26`	`26`	`output "dynamodb_table_id" {`
`27`		`- value = one(aws_dynamodb_table.with_server_side_encryption.*.id)`
	`27`	`+ value = one(aws_dynamodb_table.with_server_side_encryption[*].id)`
`28`	`28`	`description = "DynamoDB table ID"`
`29`	`29`	`}`
`30`	`30`
`31`	`31`	`output "dynamodb_table_arn" {`
`32`		`- value = one(aws_dynamodb_table.with_server_side_encryption.*.arn)`
	`32`	`+ value = one(aws_dynamodb_table.with_server_side_encryption[*].arn)`
`33`	`33`	`description = "DynamoDB table ARN"`
`34`	`34`	`}`
`35`	`35`
Original file line number	Diff line number	Diff line change
`@@ -71,8 +71,8 @@ data "aws_iam_policy_document" "replication" {`
`71`	`71`	`"s3:ListBucket"`
`72`	`72`	`]`
`73`	`73`	`resources = [`
`74`		`- join("", aws_s3_bucket.default.*.arn),`
`75`		`- "${join("", aws_s3_bucket.default..arn)}/"`
	`74`	`+ join("", aws_s3_bucket.default[*].arn),`
	`75`	`+ "${join("", aws_s3_bucket.default[].arn)}/"`
`76`	`76`	`]`
`77`	`77`	`}`
`78`	`78`
Original file line number	Diff line number	Diff line change
`@@ -71,6 +71,7 @@ variable "ignore_public_acls" {`
`71`	`71`	`}`
`72`	`72`
`73`	`73`	`variable "block_public_policy" {`
	`74`	`+ type = bool`
`74`	`75`	`description = "Whether Amazon S3 should block public bucket policies for this bucket"`
`75`	`76`	`default = true`
`76`	`77`	`}`