From 19b011a2e9364bcf2095d922b4970e7ec5ba97b5 Mon Sep 17 00:00:00 2001 From: Andres Montalban Date: Thu, 25 Jul 2024 19:53:00 -0300 Subject: [PATCH] Enforce the usage of modern TLS versions (1.2 or higher) for S3 connections (#237) * fix: Update Terraform version to match the version in variables.tf * feat: Enforce the usage of modern TLS versions (1.2 or higher) * refactor: Change input to minimum_tls_version * fix: Correct for_each * fix: Correct tests and add var.minimum_tls_version --- Makefile | 6 +++--- README.md | 1 + docs/terraform.md | 1 + examples/complete/fixtures.us-east-2.tfvars | 2 ++ examples/complete/main.tf | 1 + examples/complete/variables.tf | 6 ++++++ main.tf | 22 +++++++++++++++++++++ variables.tf | 6 ++++++ 8 files changed, 42 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index 4401521e..2212b558 100644 --- a/Makefile +++ b/Makefile @@ -1,5 +1,5 @@ SHELL := /bin/bash -export TERRAFORM_VERSION = 1.1.6 +export TERRAFORM_VERSION = 1.3.0 # List of targets the `readme` target should call before generating the readme export README_DEPS ?= docs/targets.md docs/terraform.md @@ -14,5 +14,5 @@ lint: test/%: @cd examples/complete && \ terraform init && \ - terraform $* -var-file=fixtures.us-west-1.tfvars && \ - terraform $* -var-file=grants.us-west-1.tfvars + terraform $* -var-file=fixtures.us-east-2.tfvars && \ + terraform $* -var-file=grants.us-east-2.tfvars diff --git a/README.md b/README.md index 25c77b88..9766b74b 100644 --- a/README.md +++ b/README.md @@ -293,6 +293,7 @@ Available targets: | [lifecycle\_rule\_ids](#input\_lifecycle\_rule\_ids) | DEPRECATED (use `lifecycle_configuration_rules`): A list of IDs to assign to corresponding `lifecycle_rules` | `list(string)` | `[]` | no | | [lifecycle\_rules](#input\_lifecycle\_rules) | DEPRECATED (`use lifecycle_configuration_rules`): A list of lifecycle rules | 
list(object({
    prefix  = string
    enabled = bool
    tags    = map(string)

    enable_glacier_transition            = bool
    enable_deeparchive_transition        = bool
    enable_standard_ia_transition        = bool
    enable_current_object_expiration     = bool
    enable_noncurrent_version_expiration = bool

    abort_incomplete_multipart_upload_days         = number
    noncurrent_version_glacier_transition_days     = number
    noncurrent_version_deeparchive_transition_days = number
    noncurrent_version_expiration_days             = number

    standard_transition_days    = number
    glacier_transition_days     = number
    deeparchive_transition_days = number
    expiration_days             = number
  }))
| `null` | no | | [logging](#input\_logging) | Bucket access logging configuration. Empty list for no logging, list of 1 to enable logging. | 
list(object({
    bucket_name = string
    prefix      = string
  }))
| `[]` | no | +| [minimum\_tls\_version](#input\_minimum\_tls\_version) | Set the minimum TLS version for in-transit traffic | `string` | `null` | no | | [name](#input\_name) | ID element. Usually the component or solution name, e.g. 'app' or 'jenkins'.
This is the only ID element not also included as a `tag`.
The "name" tag is set to the full `id` string. There is no tag with the value of the `name` input. | `string` | `null` | no | | [namespace](#input\_namespace) | ID element. Usually an abbreviation of your organization name, e.g. 'eg' or 'cp', to help ensure generated IDs are globally unique | `string` | `null` | no | | [object\_lock\_configuration](#input\_object\_lock\_configuration) | A configuration for S3 object locking. With S3 Object Lock, you can store objects using a `write once, read many` (WORM) model. Object Lock can help prevent objects from being deleted or overwritten for a fixed amount of time or indefinitely. | 
object({
    mode  = string # Valid values are GOVERNANCE and COMPLIANCE.
    days  = number
    years = number
  })
| `null` | no | diff --git a/docs/terraform.md b/docs/terraform.md index 77260f0c..9e584898 100644 --- a/docs/terraform.md +++ b/docs/terraform.md @@ -85,6 +85,7 @@ | [lifecycle\_rule\_ids](#input\_lifecycle\_rule\_ids) | DEPRECATED (use `lifecycle_configuration_rules`): A list of IDs to assign to corresponding `lifecycle_rules` | `list(string)` | `[]` | no | | [lifecycle\_rules](#input\_lifecycle\_rules) | DEPRECATED (`use lifecycle_configuration_rules`): A list of lifecycle rules | 
list(object({
    prefix  = string
    enabled = bool
    tags    = map(string)

    enable_glacier_transition            = bool
    enable_deeparchive_transition        = bool
    enable_standard_ia_transition        = bool
    enable_current_object_expiration     = bool
    enable_noncurrent_version_expiration = bool

    abort_incomplete_multipart_upload_days         = number
    noncurrent_version_glacier_transition_days     = number
    noncurrent_version_deeparchive_transition_days = number
    noncurrent_version_expiration_days             = number

    standard_transition_days    = number
    glacier_transition_days     = number
    deeparchive_transition_days = number
    expiration_days             = number
  }))
| `null` | no | | [logging](#input\_logging) | Bucket access logging configuration. Empty list for no logging, list of 1 to enable logging. | 
list(object({
    bucket_name = string
    prefix      = string
  }))
| `[]` | no | +| [minimum\_tls\_version](#input\_minimum\_tls\_version) | Set the minimum TLS version for in-transit traffic | `string` | `null` | no | | [name](#input\_name) | ID element. Usually the component or solution name, e.g. 'app' or 'jenkins'.
This is the only ID element not also included as a `tag`.
The "name" tag is set to the full `id` string. There is no tag with the value of the `name` input. | `string` | `null` | no | | [namespace](#input\_namespace) | ID element. Usually an abbreviation of your organization name, e.g. 'eg' or 'cp', to help ensure generated IDs are globally unique | `string` | `null` | no | | [object\_lock\_configuration](#input\_object\_lock\_configuration) | A configuration for S3 object locking. With S3 Object Lock, you can store objects using a `write once, read many` (WORM) model. Object Lock can help prevent objects from being deleted or overwritten for a fixed amount of time or indefinitely. | 
object({
    mode  = string # Valid values are GOVERNANCE and COMPLIANCE.
    days  = number
    years = number
  })
| `null` | no | diff --git a/examples/complete/fixtures.us-east-2.tfvars b/examples/complete/fixtures.us-east-2.tfvars index 48e76942..bbecb070 100644 --- a/examples/complete/fixtures.us-east-2.tfvars +++ b/examples/complete/fixtures.us-east-2.tfvars @@ -28,3 +28,5 @@ allowed_bucket_actions = [ ] bucket_key_enabled = true + +minimum_tls_version = "1.2" diff --git a/examples/complete/main.tf b/examples/complete/main.tf index e116968d..d10e4f4c 100644 --- a/examples/complete/main.tf +++ b/examples/complete/main.tf @@ -34,6 +34,7 @@ module "s3_bucket" { block_public_policy = var.block_public_policy ignore_public_acls = var.ignore_public_acls restrict_public_buckets = var.restrict_public_buckets + minimum_tls_version = var.minimum_tls_version access_key_enabled = var.access_key_enabled store_access_key_in_ssm = var.store_access_key_in_ssm diff --git a/examples/complete/variables.tf b/examples/complete/variables.tf index 4a45c2c2..2f0430ba 100644 --- a/examples/complete/variables.tf +++ b/examples/complete/variables.tf @@ -315,3 +315,9 @@ variable "transfer_acceleration_enabled" { default = true description = "Set true to enable Transfer Acceleration (many regions not supported)" } + +variable "minimum_tls_version" { + type = string + default = null + description = "Set the minimum TLS version for in-transit traffic" +} diff --git a/main.tf b/main.tf index cb2bf133..d15faef5 100644 --- a/main.tf +++ b/main.tf @@ -424,6 +424,28 @@ data "aws_iam_policy_document" "bucket_policy" { } } + dynamic "statement" { + for_each = var.minimum_tls_version != null ? toset([var.minimum_tls_version]) : toset([]) + + content { + sid = "EnforceTLSVersion" + effect = "Deny" + actions = ["s3:*"] + resources = [local.bucket_arn, "${local.bucket_arn}/*"] + + principals { + identifiers = ["*"] + type = "*" + } + + condition { + test = "NumericLessThan" + values = [statement.value] + variable = "s3:TlsVersion" + } + } + } + dynamic "statement" { for_each = length(var.s3_replication_source_roles) > 0 ? [1] : [] diff --git a/variables.tf b/variables.tf index 4191f1cc..10eb6b03 100644 --- a/variables.tf +++ b/variables.tf @@ -139,6 +139,12 @@ variable "allow_ssl_requests_only" { nullable = false } +variable "minimum_tls_version" { + type = string + default = null + description = "Set the minimum TLS version for in-transit traffic" +} + variable "lifecycle_configuration_rules" { type = list(object({ enabled = optional(bool, true)