Enforce the usage of modern TLS versions (1.2 or higher) for S3 connections

[amontalban]

what

This variables adds a policy to the bucket to deny connections that do not use TLS 1.2 or higher.

why

This is required by our security team.

references

https://repost.aws/knowledge-center/s3-enforce-modern-tls

[mergify]

Important

Cloud Posse Engineering Team Review Required

This pull request modifies files that require Cloud Posse's review. Please be patient, and a core maintainer will review your changes.

To expedite this process, reach out to us on Slack in the #pr-reviews channel.

[goruha]

@amontalban that's a nice feature.
Can we refactor code a bit. I think we need to parameterize tls minimum version to have code like this

  dynamic "statement" {
    for_each = var.minimum_tls_version != null? [var.minimum_tls_version] : []

    content {
      sid       = "EnforceTLSVersion"
      effect    = "Deny"
      actions   = ["s3:*"]
      resources = [local.bucket_arn, "${local.bucket_arn}/*"]

      principals {
        identifiers = ["*"]
        type        = "*"
      }

      condition {
        test     = "NumericLessThan"
        values   = [each.value]
        variable = "s3:TlsVersion"
      }
    }
  }

[goruha]

See comment

[amontalban]

@goruha thanks for reviewing the PR, what do you think about it now?

@amontalban that's a nice feature. Can we refactor code a bit. I think we need to parameterize tls minimum version to have code like this

  dynamic "statement" {
    for_each = var.minimum_tls_version != null? [var.minimum_tls_version] : []

    content {
      sid       = "EnforceTLSVersion"
      effect    = "Deny"
      actions   = ["s3:*"]
      resources = [local.bucket_arn, "${local.bucket_arn}/*"]

      principals {
        identifiers = ["*"]
        type        = "*"
      }

      condition {
        test     = "NumericLessThan"
        values   = [each.value]
        variable = "s3:TlsVersion"
      }
    }
  }

[goruha]

/terratest

[goruha]

@amontalban have you tested that the code works?

I am a bit confused that terraform hcl https://developer.hashicorp.com/terraform/language/meta-arguments/for_each#basic-syntax

use toset for list in for_each for_each = toset(["Todd", "James", "Alice", "Dottie"])

Can you add minimum_tls_version = 1.12 to
https://github.com/amontalban/terraform-aws-s3-bucket/blob/am/enforce_tls12/examples/complete/fixtures.us-east-2.tfvars ?

[amontalban]

@goruha yeah, sorry I rushed based on your recommendation because I was on a hurry and didn't properly test the change.

I have implemented the toset and also updated the tests to include minimum_tls_version and fixture paths so I can validate everything with make test/plan.

Let me know what do you think now.

Thanks and sorry for the hassle!

@amontalban have you tested that the code works?

I am a bit confused that terraform hcl https://developer.hashicorp.com/terraform/language/meta-arguments/for_each#basic-syntax

use toset for list in for_each for_each = toset(["Todd", "James", "Alice", "Dottie"])

Can you add minimum_tls_version = 1.12 to https://github.com/amontalban/terraform-aws-s3-bucket/blob/am/enforce_tls12/examples/complete/fixtures.us-east-2.tfvars ?

[goruha]

/terratest

[goruha]

/terratest

[github-actions]

These changes were released in v4.3.0.