From 99453ccfc0d01551458a29c35175b52fb0dfa906 Mon Sep 17 00:00:00 2001 From: Nuru Date: Wed, 31 May 2023 03:02:49 -0700 Subject: [PATCH] Always require TLS connection to S3 bucket (#139) * Always deny unencrypted HTTP connections * tflint fixes --- README.md | 2 +- docs/terraform.md | 2 +- examples/complete/versions.tf | 2 +- main.tf | 109 ++++++++++++++++++---------------- outputs.tf | 6 +- replication.tf | 4 +- variables.tf | 1 + 7 files changed, 66 insertions(+), 60 deletions(-) diff --git a/README.md b/README.md index dbe2248..c650dba 100644 --- 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 index 085c152..cd63531 100644 --- 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 index 006f5f4..876cde7 100644 --- 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 index 0ef48e5..5a50729 100644 --- 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 index c40e395..5137810 100644 --- 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 index a08270b..2bea084 100644 --- 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 index dac01c2..922efa3 100644 --- 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 }