Always require TLS connection to S3 bucket (#139)

* Always deny unencrypted HTTP connections * tflint fixes
cloudposse · May 31, 2023 · 99453cc · 99453cc
1 parent 1d48b08
commit 99453cc
Show file tree

Hide file tree

Showing 7 changed files with 66 additions and 60 deletions.
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
 }