From bb5661296ae4b8d032b24ac96749be69da79b653 Mon Sep 17 00:00:00 2001 From: Andres Montalban Date: Thu, 27 Jun 2024 14:32:56 -0300 Subject: [PATCH 1/5] fix: Update Terraform version to match the version in variables.tf --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 4401521e..9fd415bd 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 From 961e80547b100852ee839b0cb459f3c3868b58e9 Mon Sep 17 00:00:00 2001 From: Andres Montalban Date: Thu, 27 Jun 2024 14:33:24 -0300 Subject: [PATCH 2/5] feat: Enforce the usage of modern TLS versions (1.2 or higher) --- README.md | 1 + docs/terraform.md | 1 + main.tf | 22 ++++++++++++++++++++++ variables.tf | 7 +++++++ 4 files changed, 31 insertions(+) diff --git a/README.md b/README.md index 25c77b88..3580802b 100644 --- a/README.md +++ b/README.md @@ -279,6 +279,7 @@ Available targets: | [delimiter](#input\_delimiter) | Delimiter to be used between ID elements.
Defaults to `-` (hyphen). Set to `""` to use no delimiter at all. | `string` | `null` | no | | [descriptor\_formats](#input\_descriptor\_formats) | Describe additional descriptors to be output in the `descriptors` output map.
Map of maps. Keys are names of descriptors. Values are maps of the form
`{
format = string
labels = list(string)
}`
(Type is `any` so the map values can later be enhanced to provide additional options.)
`format` is a Terraform format string to be passed to the `format()` function.
`labels` is a list of labels, in order, to pass to `format()` function.
Label values will be normalized before being passed to `format()` so they will be
identical to how they appear in `id`.
Default is `{}` (`descriptors` output will be empty). | `any` | `{}` | no | | [enabled](#input\_enabled) | Set to false to prevent the module from creating any resources | `bool` | `null` | no | +| [enforce\_modern\_tls](#input\_enforce\_modern\_tls) | Set to `true` to require that requests use a modern encryption protocol for data in transit (TLS version 1.2 or higher) | `bool` | `false` | no | | [environment](#input\_environment) | ID element. Usually used for region e.g. 'uw2', 'us-west-2', OR role 'prod', 'staging', 'dev', 'UAT' | `string` | `null` | no | | [force\_destroy](#input\_force\_destroy) | When `true`, permits a non-empty S3 bucket to be deleted by first deleting all objects in the bucket.
THESE OBJECTS ARE NOT RECOVERABLE even if they were versioned and stored in Glacier. | `bool` | `false` | no | | [grants](#input\_grants) | A list of policy grants for the bucket, taking a list of permissions.
Conflicts with `acl`. Set `acl` to `null` to use this.
Deprecated by AWS in favor of bucket policies.
Automatically disabled if `s3_object_ownership` is set to "BucketOwnerEnforced". | 
list(object({
    id          = string
    type        = string
    permissions = list(string)
    uri         = string
  }))
| `[]` | no | diff --git a/docs/terraform.md b/docs/terraform.md index 77260f0c..8b1a834b 100644 --- a/docs/terraform.md +++ b/docs/terraform.md @@ -71,6 +71,7 @@ | [delimiter](#input\_delimiter) | Delimiter to be used between ID elements.
Defaults to `-` (hyphen). Set to `""` to use no delimiter at all. | `string` | `null` | no | | [descriptor\_formats](#input\_descriptor\_formats) | Describe additional descriptors to be output in the `descriptors` output map.
Map of maps. Keys are names of descriptors. Values are maps of the form
`{
format = string
labels = list(string)
}`
(Type is `any` so the map values can later be enhanced to provide additional options.)
`format` is a Terraform format string to be passed to the `format()` function.
`labels` is a list of labels, in order, to pass to `format()` function.
Label values will be normalized before being passed to `format()` so they will be
identical to how they appear in `id`.
Default is `{}` (`descriptors` output will be empty). | `any` | `{}` | no | | [enabled](#input\_enabled) | Set to false to prevent the module from creating any resources | `bool` | `null` | no | +| [enforce\_modern\_tls](#input\_enforce\_modern\_tls) | Set to `true` to require that requests use a modern encryption protocol for data in transit (TLS version 1.2 or higher) | `bool` | `false` | no | | [environment](#input\_environment) | ID element. Usually used for region e.g. 'uw2', 'us-west-2', OR role 'prod', 'staging', 'dev', 'UAT' | `string` | `null` | no | | [force\_destroy](#input\_force\_destroy) | When `true`, permits a non-empty S3 bucket to be deleted by first deleting all objects in the bucket.
THESE OBJECTS ARE NOT RECOVERABLE even if they were versioned and stored in Glacier. | `bool` | `false` | no | | [grants](#input\_grants) | A list of policy grants for the bucket, taking a list of permissions.
Conflicts with `acl`. Set `acl` to `null` to use this.
Deprecated by AWS in favor of bucket policies.
Automatically disabled if `s3_object_ownership` is set to "BucketOwnerEnforced". | 
list(object({
    id          = string
    type        = string
    permissions = list(string)
    uri         = string
  }))
| `[]` | no | diff --git a/main.tf b/main.tf index cb2bf133..cbd389ad 100644 --- a/main.tf +++ b/main.tf @@ -424,6 +424,28 @@ data "aws_iam_policy_document" "bucket_policy" { } } + dynamic "statement" { + for_each = var.enforce_modern_tls ? [1] : [] + + content { + sid = "EnforceTLSVersion" + effect = "Deny" + actions = ["s3:*"] + resources = [local.bucket_arn, "${local.bucket_arn}/*"] + + principals { + identifiers = ["*"] + type = "*" + } + + condition { + test = "NumericLessThan" + values = ["1.2"] + 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..58155c3f 100644 --- a/variables.tf +++ b/variables.tf @@ -139,6 +139,13 @@ variable "allow_ssl_requests_only" { nullable = false } +variable "enforce_modern_tls" { + type = bool + default = false + description = "Set to `true` to require that requests use a modern encryption protocol for data in transit (TLS version 1.2 or higher)" + nullable = false +} + variable "lifecycle_configuration_rules" { type = list(object({ enabled = optional(bool, true) From d7713b13c19b6d356ccda4581d58f6c65fbaeaeb Mon Sep 17 00:00:00 2001 From: Andres Montalban Date: Fri, 28 Jun 2024 08:36:41 -0300 Subject: [PATCH 3/5] refactor: Change input to minimum_tls_version --- README.md | 2 +- docs/terraform.md | 2 +- main.tf | 4 ++-- variables.tf | 9 ++++----- 4 files changed, 8 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index 3580802b..9766b74b 100644 --- a/README.md +++ b/README.md @@ -279,7 +279,6 @@ Available targets: | [delimiter](#input\_delimiter) | Delimiter to be used between ID elements.
Defaults to `-` (hyphen). Set to `""` to use no delimiter at all. | `string` | `null` | no | | [descriptor\_formats](#input\_descriptor\_formats) | Describe additional descriptors to be output in the `descriptors` output map.
Map of maps. Keys are names of descriptors. Values are maps of the form
`{
format = string
labels = list(string)
}`
(Type is `any` so the map values can later be enhanced to provide additional options.)
`format` is a Terraform format string to be passed to the `format()` function.
`labels` is a list of labels, in order, to pass to `format()` function.
Label values will be normalized before being passed to `format()` so they will be
identical to how they appear in `id`.
Default is `{}` (`descriptors` output will be empty). | `any` | `{}` | no | | [enabled](#input\_enabled) | Set to false to prevent the module from creating any resources | `bool` | `null` | no | -| [enforce\_modern\_tls](#input\_enforce\_modern\_tls) | Set to `true` to require that requests use a modern encryption protocol for data in transit (TLS version 1.2 or higher) | `bool` | `false` | no | | [environment](#input\_environment) | ID element. Usually used for region e.g. 'uw2', 'us-west-2', OR role 'prod', 'staging', 'dev', 'UAT' | `string` | `null` | no | | [force\_destroy](#input\_force\_destroy) | When `true`, permits a non-empty S3 bucket to be deleted by first deleting all objects in the bucket.
THESE OBJECTS ARE NOT RECOVERABLE even if they were versioned and stored in Glacier. | `bool` | `false` | no | | [grants](#input\_grants) | A list of policy grants for the bucket, taking a list of permissions.
Conflicts with `acl`. Set `acl` to `null` to use this.
Deprecated by AWS in favor of bucket policies.
Automatically disabled if `s3_object_ownership` is set to "BucketOwnerEnforced". | 
list(object({
    id          = string
    type        = string
    permissions = list(string)
    uri         = string
  }))
| `[]` | no | @@ -294,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 8b1a834b..9e584898 100644 --- a/docs/terraform.md +++ b/docs/terraform.md @@ -71,7 +71,6 @@ | [delimiter](#input\_delimiter) | Delimiter to be used between ID elements.
Defaults to `-` (hyphen). Set to `""` to use no delimiter at all. | `string` | `null` | no | | [descriptor\_formats](#input\_descriptor\_formats) | Describe additional descriptors to be output in the `descriptors` output map.
Map of maps. Keys are names of descriptors. Values are maps of the form
`{
format = string
labels = list(string)
}`
(Type is `any` so the map values can later be enhanced to provide additional options.)
`format` is a Terraform format string to be passed to the `format()` function.
`labels` is a list of labels, in order, to pass to `format()` function.
Label values will be normalized before being passed to `format()` so they will be
identical to how they appear in `id`.
Default is `{}` (`descriptors` output will be empty). | `any` | `{}` | no | | [enabled](#input\_enabled) | Set to false to prevent the module from creating any resources | `bool` | `null` | no | -| [enforce\_modern\_tls](#input\_enforce\_modern\_tls) | Set to `true` to require that requests use a modern encryption protocol for data in transit (TLS version 1.2 or higher) | `bool` | `false` | no | | [environment](#input\_environment) | ID element. Usually used for region e.g. 'uw2', 'us-west-2', OR role 'prod', 'staging', 'dev', 'UAT' | `string` | `null` | no | | [force\_destroy](#input\_force\_destroy) | When `true`, permits a non-empty S3 bucket to be deleted by first deleting all objects in the bucket.
THESE OBJECTS ARE NOT RECOVERABLE even if they were versioned and stored in Glacier. | `bool` | `false` | no | | [grants](#input\_grants) | A list of policy grants for the bucket, taking a list of permissions.
Conflicts with `acl`. Set `acl` to `null` to use this.
Deprecated by AWS in favor of bucket policies.
Automatically disabled if `s3_object_ownership` is set to "BucketOwnerEnforced". | 
list(object({
    id          = string
    type        = string
    permissions = list(string)
    uri         = string
  }))
| `[]` | no | @@ -86,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/main.tf b/main.tf index cbd389ad..de0826ed 100644 --- a/main.tf +++ b/main.tf @@ -425,7 +425,7 @@ data "aws_iam_policy_document" "bucket_policy" { } dynamic "statement" { - for_each = var.enforce_modern_tls ? [1] : [] + for_each = var.minimum_tls_version != null ? [var.minimum_tls_version] : [] content { sid = "EnforceTLSVersion" @@ -440,7 +440,7 @@ data "aws_iam_policy_document" "bucket_policy" { condition { test = "NumericLessThan" - values = ["1.2"] + values = [each.value] variable = "s3:TlsVersion" } } diff --git a/variables.tf b/variables.tf index 58155c3f..10eb6b03 100644 --- a/variables.tf +++ b/variables.tf @@ -139,11 +139,10 @@ variable "allow_ssl_requests_only" { nullable = false } -variable "enforce_modern_tls" { - type = bool - default = false - description = "Set to `true` to require that requests use a modern encryption protocol for data in transit (TLS version 1.2 or higher)" - nullable = false +variable "minimum_tls_version" { + type = string + default = null + description = "Set the minimum TLS version for in-transit traffic" } variable "lifecycle_configuration_rules" { From 039b86b8ba710bbb00cb720f841da5e028bd2935 Mon Sep 17 00:00:00 2001 From: Andres Montalban Date: Wed, 3 Jul 2024 08:51:54 -0300 Subject: [PATCH 4/5] fix: Correct for_each --- main.tf | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/main.tf b/main.tf index de0826ed..d15faef5 100644 --- a/main.tf +++ b/main.tf @@ -425,7 +425,7 @@ data "aws_iam_policy_document" "bucket_policy" { } dynamic "statement" { - for_each = var.minimum_tls_version != null ? [var.minimum_tls_version] : [] + for_each = var.minimum_tls_version != null ? toset([var.minimum_tls_version]) : toset([]) content { sid = "EnforceTLSVersion" @@ -440,7 +440,7 @@ data "aws_iam_policy_document" "bucket_policy" { condition { test = "NumericLessThan" - values = [each.value] + values = [statement.value] variable = "s3:TlsVersion" } } From 4fefc490e5c93fa438aed9c0cc0045a87fa05c07 Mon Sep 17 00:00:00 2001 From: Andres Montalban Date: Wed, 3 Jul 2024 08:52:31 -0300 Subject: [PATCH 5/5] fix: Correct tests and add var.minimum_tls_version --- Makefile | 4 ++-- examples/complete/fixtures.us-east-2.tfvars | 2 ++ examples/complete/main.tf | 1 + examples/complete/variables.tf | 6 ++++++ 4 files changed, 11 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 9fd415bd..2212b558 100644 --- a/Makefile +++ b/Makefile @@ -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/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" +}