From 25e15fe373a1496760b780d43b14e7332e01fa0d Mon Sep 17 00:00:00 2001 From: Kostya Date: Mon, 29 Mar 2021 12:24:00 +0800 Subject: [PATCH 1/5] feat: Add ability to create deny insecure transport policy --- README.md | 1 + examples/complete/main.tf | 9 +++---- main.tf | 49 ++++++++++++++++++++++++++++++++++++--- variables.tf | 6 +++++ 4 files changed, 58 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index b684853a..0fbcced5 100644 --- a/README.md +++ b/README.md @@ -112,6 +112,7 @@ No modules. | [acceleration\_status](#input\_acceleration\_status) | (Optional) Sets the accelerate configuration of an existing bucket. Can be Enabled or Suspended. | `string` | `null` | no | | [acl](#input\_acl) | (Optional) The canned ACL to apply. Defaults to 'private'. Conflicts with `grant` | `string` | `"private"` | no | | [attach\_elb\_log\_delivery\_policy](#input\_attach\_elb\_log\_delivery\_policy) | Controls if S3 bucket should have ELB log delivery policy attached | `bool` | `false` | no | +| [attach\_deny\_insecure\_transport\_policy](#input\_attach\_deny\_insecure\_transport\_policy) | Controls if S3 bucket should have [deny non-encrypted transport policy](https://aws.amazon.com/premiumsupport/knowledge-center/s3-bucket-policy-for-config-rule/) attached | `bool` | `false`| no | | [attach\_policy](#input\_attach\_policy) | Controls if S3 bucket should have bucket policy attached (set to `true` to use value of `policy` as bucket policy) | `bool` | `false` | no | | [attach\_public\_policy](#input\_attach\_public\_policy) | Controls if a user defined public bucket policy will be attached (set to `false` to allow upstream to apply defaults to the bucket) | `bool` | `true` | no | | [block\_public\_acls](#input\_block\_public\_acls) | Whether Amazon S3 should block public ACLs for this bucket. | `bool` | `false` | no | diff --git a/examples/complete/main.tf b/examples/complete/main.tf index 90bd6f9b..adff7c71 100644 --- a/examples/complete/main.tf +++ b/examples/complete/main.tf @@ -51,10 +51,11 @@ data "aws_iam_policy_document" "bucket_policy" { module "log_bucket" { source = "../../" - bucket = "logs-${random_pet.this.id}" - acl = "log-delivery-write" - force_destroy = true - attach_elb_log_delivery_policy = true + bucket = "logs-${random_pet.this.id}" + acl = "log-delivery-write" + force_destroy = true + attach_elb_log_delivery_policy = true + attach_deny_insecure_transport_policy = true } module "cloudfront_log_bucket" { diff --git a/main.tf b/main.tf index 98d288af..8837bd8d 100644 --- a/main.tf +++ b/main.tf @@ -231,10 +231,23 @@ resource "aws_s3_bucket" "this" { } resource "aws_s3_bucket_policy" "this" { - count = var.create_bucket && (var.attach_elb_log_delivery_policy || var.attach_policy) ? 1 : 0 + count = var.create_bucket && (var.attach_elb_log_delivery_policy || var.attach_deny_insecure_transport_policy || var.attach_policy) ? 1 : 0 bucket = aws_s3_bucket.this[0].id - policy = var.attach_elb_log_delivery_policy ? data.aws_iam_policy_document.elb_log_delivery[0].json : var.policy + policy = ( + var.attach_elb_log_delivery_policy && var.attach_deny_insecure_transport_policy ? data.aws_iam_policy_document.combined[0].json + : var.attach_elb_log_delivery_policy ? data.aws_iam_policy_document.elb_log_delivery[0].json + : var.attach_deny_insecure_transport_policy ? data.aws_iam_policy_document.deny_insecure_transport[0].json + : var.policy + ) +} + +data "aws_iam_policy_document" "combined" { + count = var.create_bucket && var.attach_elb_log_delivery_policy && var.attach_deny_insecure_transport_policy ? 1 : 0 + source_policy_documents = [ + data.aws_iam_policy_document.elb_log_delivery[0].json, + data.aws_iam_policy_document.deny_insecure_transport[0].json + ] } # AWS Load Balancer access log delivery policy @@ -265,12 +278,42 @@ data "aws_iam_policy_document" "elb_log_delivery" { } } +data "aws_iam_policy_document" "deny_insecure_transport" { + count = var.create_bucket && var.attach_deny_insecure_transport_policy ? 1 : 0 + statement { + sid = "denyInsecureTransport" + effect = "Deny" + + actions = [ + "s3:*", + ] + + resources = [ + aws_s3_bucket.this[0].arn, + "${aws_s3_bucket.this[0].arn}/*", + ] + + principals { + type = "*" + identifiers = ["*"] + } + + condition { + test = "Bool" + variable = "aws:SecureTransport" + values = [ + "false" + ] + } + } +} + resource "aws_s3_bucket_public_access_block" "this" { count = var.create_bucket && var.attach_public_policy ? 1 : 0 # Chain resources (s3_bucket -> s3_bucket_policy -> s3_bucket_public_access_block) # to prevent "A conflicting conditional operation is currently in progress against this resource." - bucket = (var.attach_elb_log_delivery_policy || var.attach_policy) ? aws_s3_bucket_policy.this[0].id : aws_s3_bucket.this[0].id + bucket = (var.attach_elb_log_delivery_policy || var.attach_deny_insecure_transport_policy || var.attach_policy) ? aws_s3_bucket_policy.this[0].id : aws_s3_bucket.this[0].id block_public_acls = var.block_public_acls block_public_policy = var.block_public_policy diff --git a/variables.tf b/variables.tf index 9e2b7b1d..3fe50580 100644 --- a/variables.tf +++ b/variables.tf @@ -10,6 +10,12 @@ variable "attach_elb_log_delivery_policy" { default = false } +variable "attach_deny_insecure_transport_policy" { + description = "Controls if S3 bucket should have deny non-encrypted transport policy attached" + type = bool + default = false +} + variable "attach_policy" { description = "Controls if S3 bucket should have bucket policy attached (set to `true` to use value of `policy` as bucket policy)" type = bool From f21002b9d7f1af66bee720cf887b4fb3b009fcb3 Mon Sep 17 00:00:00 2001 From: Kostya Date: Mon, 29 Mar 2021 17:23:57 +0800 Subject: [PATCH 2/5] Fix readme typo --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 0fbcced5..1b1a093e 100644 --- a/README.md +++ b/README.md @@ -112,7 +112,7 @@ No modules. | [acceleration\_status](#input\_acceleration\_status) | (Optional) Sets the accelerate configuration of an existing bucket. Can be Enabled or Suspended. | `string` | `null` | no | | [acl](#input\_acl) | (Optional) The canned ACL to apply. Defaults to 'private'. Conflicts with `grant` | `string` | `"private"` | no | | [attach\_elb\_log\_delivery\_policy](#input\_attach\_elb\_log\_delivery\_policy) | Controls if S3 bucket should have ELB log delivery policy attached | `bool` | `false` | no | -| [attach\_deny\_insecure\_transport\_policy](#input\_attach\_deny\_insecure\_transport\_policy) | Controls if S3 bucket should have [deny non-encrypted transport policy](https://aws.amazon.com/premiumsupport/knowledge-center/s3-bucket-policy-for-config-rule/) attached | `bool` | `false`| no | +| [attach\_deny\_insecure\_transport\_policy](#input\_attach\_deny\_insecure\_transport\_policy) | Controls if S3 bucket should have [deny non-encrypted transport policy](https://aws.amazon.com/premiumsupport/knowledge-center/s3-bucket-policy-for-config-rule/) attached | `bool` | `false`| no | | [attach\_policy](#input\_attach\_policy) | Controls if S3 bucket should have bucket policy attached (set to `true` to use value of `policy` as bucket policy) | `bool` | `false` | no | | [attach\_public\_policy](#input\_attach\_public\_policy) | Controls if a user defined public bucket policy will be attached (set to `false` to allow upstream to apply defaults to the bucket) | `bool` | `true` | no | | [block\_public\_acls](#input\_block\_public\_acls) | Whether Amazon S3 should block public ACLs for this bucket. | `bool` | `false` | no | From 543b604d11bcd4505d89ecde2f8d06fe8e5d8278 Mon Sep 17 00:00:00 2001 From: Kostya Date: Mon, 29 Mar 2021 17:33:23 +0800 Subject: [PATCH 3/5] Another attempt to fix TF docs check --- README.md | 4 +++- variables.tf | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 1b1a093e..f6266ecc 100644 --- a/README.md +++ b/README.md @@ -103,6 +103,8 @@ No modules. | [aws_s3_bucket_policy.this](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/s3_bucket_policy) | resource | | [aws_s3_bucket_public_access_block.this](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/s3_bucket_public_access_block) | resource | | [aws_elb_service_account.this](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/elb_service_account) | data source | +| [aws_iam_policy_document.combined](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/iam_policy_document) | data source | +| [aws_iam_policy_document.deny_insecure_transport](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/iam_policy_document) | data source | | [aws_iam_policy_document.elb_log_delivery](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/iam_policy_document) | data source | ## Inputs @@ -111,8 +113,8 @@ No modules. |------|-------------|------|---------|:--------:| | [acceleration\_status](#input\_acceleration\_status) | (Optional) Sets the accelerate configuration of an existing bucket. Can be Enabled or Suspended. | `string` | `null` | no | | [acl](#input\_acl) | (Optional) The canned ACL to apply. Defaults to 'private'. Conflicts with `grant` | `string` | `"private"` | no | +| [attach\_deny\_insecure\_transport\_policy](#input\_attach\_deny\_insecure\_transport\_policy) | Controls if S3 bucket should have deny non-SSL transport policy attached | `bool` | `false` | no | | [attach\_elb\_log\_delivery\_policy](#input\_attach\_elb\_log\_delivery\_policy) | Controls if S3 bucket should have ELB log delivery policy attached | `bool` | `false` | no | -| [attach\_deny\_insecure\_transport\_policy](#input\_attach\_deny\_insecure\_transport\_policy) | Controls if S3 bucket should have [deny non-encrypted transport policy](https://aws.amazon.com/premiumsupport/knowledge-center/s3-bucket-policy-for-config-rule/) attached | `bool` | `false`| no | | [attach\_policy](#input\_attach\_policy) | Controls if S3 bucket should have bucket policy attached (set to `true` to use value of `policy` as bucket policy) | `bool` | `false` | no | | [attach\_public\_policy](#input\_attach\_public\_policy) | Controls if a user defined public bucket policy will be attached (set to `false` to allow upstream to apply defaults to the bucket) | `bool` | `true` | no | | [block\_public\_acls](#input\_block\_public\_acls) | Whether Amazon S3 should block public ACLs for this bucket. | `bool` | `false` | no | diff --git a/variables.tf b/variables.tf index 3fe50580..f07d4169 100644 --- a/variables.tf +++ b/variables.tf @@ -11,7 +11,7 @@ variable "attach_elb_log_delivery_policy" { } variable "attach_deny_insecure_transport_policy" { - description = "Controls if S3 bucket should have deny non-encrypted transport policy attached" + description = "Controls if S3 bucket should have deny non-SSL transport policy attached" type = bool default = false } From 4727b171dbc46ea69aa420173e5a24c55d9ec94e Mon Sep 17 00:00:00 2001 From: Anton Babenko Date: Fri, 9 Apr 2021 12:51:33 +0200 Subject: [PATCH 4/5] Simplifying things a bit --- examples/complete/main.tf | 2 ++ main.tf | 28 +++++++++++++++------------- 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/examples/complete/main.tf b/examples/complete/main.tf index adff7c71..587a19c2 100644 --- a/examples/complete/main.tf +++ b/examples/complete/main.tf @@ -87,6 +87,8 @@ module "s3_bucket" { attach_policy = true policy = data.aws_iam_policy_document.bucket_policy.json + attach_deny_insecure_transport_policy = true + tags = { Owner = "Anton" } diff --git a/main.tf b/main.tf index 8837bd8d..650ff419 100644 --- a/main.tf +++ b/main.tf @@ -1,3 +1,7 @@ +locals { + attach_policy = var.attach_elb_log_delivery_policy || var.attach_deny_insecure_transport_policy || var.attach_policy +} + resource "aws_s3_bucket" "this" { count = var.create_bucket ? 1 : 0 @@ -231,23 +235,20 @@ resource "aws_s3_bucket" "this" { } resource "aws_s3_bucket_policy" "this" { - count = var.create_bucket && (var.attach_elb_log_delivery_policy || var.attach_deny_insecure_transport_policy || var.attach_policy) ? 1 : 0 + count = var.create_bucket && local.attach_policy ? 1 : 0 bucket = aws_s3_bucket.this[0].id - policy = ( - var.attach_elb_log_delivery_policy && var.attach_deny_insecure_transport_policy ? data.aws_iam_policy_document.combined[0].json - : var.attach_elb_log_delivery_policy ? data.aws_iam_policy_document.elb_log_delivery[0].json - : var.attach_deny_insecure_transport_policy ? data.aws_iam_policy_document.deny_insecure_transport[0].json - : var.policy - ) + policy = data.aws_iam_policy_document.combined[0].json } data "aws_iam_policy_document" "combined" { - count = var.create_bucket && var.attach_elb_log_delivery_policy && var.attach_deny_insecure_transport_policy ? 1 : 0 - source_policy_documents = [ - data.aws_iam_policy_document.elb_log_delivery[0].json, - data.aws_iam_policy_document.deny_insecure_transport[0].json - ] + count = var.create_bucket && local.attach_policy ? 1 : 0 + + source_policy_documents = compact([ + var.attach_elb_log_delivery_policy ? data.aws_iam_policy_document.elb_log_delivery[0].json : "", + var.attach_deny_insecure_transport_policy ? data.aws_iam_policy_document.deny_insecure_transport[0].json : "", + var.attach_policy ? var.policy : "" + ]) } # AWS Load Balancer access log delivery policy @@ -280,6 +281,7 @@ data "aws_iam_policy_document" "elb_log_delivery" { data "aws_iam_policy_document" "deny_insecure_transport" { count = var.create_bucket && var.attach_deny_insecure_transport_policy ? 1 : 0 + statement { sid = "denyInsecureTransport" effect = "Deny" @@ -313,7 +315,7 @@ resource "aws_s3_bucket_public_access_block" "this" { # Chain resources (s3_bucket -> s3_bucket_policy -> s3_bucket_public_access_block) # to prevent "A conflicting conditional operation is currently in progress against this resource." - bucket = (var.attach_elb_log_delivery_policy || var.attach_deny_insecure_transport_policy || var.attach_policy) ? aws_s3_bucket_policy.this[0].id : aws_s3_bucket.this[0].id + bucket = local.attach_policy ? aws_s3_bucket_policy.this[0].id : aws_s3_bucket.this[0].id block_public_acls = var.block_public_acls block_public_policy = var.block_public_policy From 500e534c75b95640ed9d6dd3bf8e5259563e9e07 Mon Sep 17 00:00:00 2001 From: Anton Babenko Date: Fri, 9 Apr 2021 12:54:22 +0200 Subject: [PATCH 5/5] Fixing terraform-docs version --- .github/workflows/pre-commit.yml | 38 ++++++++++++++++---------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/.github/workflows/pre-commit.yml b/.github/workflows/pre-commit.yml index 7eaa782b..59cd0a89 100644 --- a/.github/workflows/pre-commit.yml +++ b/.github/workflows/pre-commit.yml @@ -7,30 +7,30 @@ on: - master jobs: -# Min Terraform version(s) + # Min Terraform version(s) getDirectories: - name: Get root directories - runs-on: ubuntu-latest - steps: - - name: Checkout - uses: actions/checkout@v2 - - name: Install Python - uses: actions/setup-python@v2 - - name: Build matrix - id: matrix - run: | - DIRS=$(python -c "import json; import glob; print(json.dumps([x.replace('/versions.tf', '') for x in glob.glob('./**/versions.tf', recursive=True)]))") - echo "::set-output name=directories::$DIRS" - outputs: - directories: ${{ steps.matrix.outputs.directories }} + name: Get root directories + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@v2 + - name: Install Python + uses: actions/setup-python@v2 + - name: Build matrix + id: matrix + run: | + DIRS=$(python -c "import json; import glob; print(json.dumps([x.replace('/versions.tf', '') for x in glob.glob('./**/versions.tf', recursive=True)]))") + echo "::set-output name=directories::$DIRS" + outputs: + directories: ${{ steps.matrix.outputs.directories }} preCommitMinVersions: name: Min TF validate needs: getDirectories runs-on: ubuntu-latest strategy: - matrix: - directory: ${{ fromJson(needs.getDirectories.outputs.directories) }} + matrix: + directory: ${{ fromJson(needs.getDirectories.outputs.directories) }} steps: - name: Checkout uses: actions/checkout@v2 @@ -59,7 +59,7 @@ jobs: pre-commit run terraform_validate --color=always --show-diff-on-failure --files $(ls *.tf) -# Max Terraform version + # Max Terraform version getBaseVersion: name: Module max TF version runs-on: ubuntu-latest @@ -94,7 +94,7 @@ jobs: - name: Install pre-commit dependencies run: | pip install pre-commit - curl -L "$(curl -s https://api.github.com/repos/terraform-docs/terraform-docs/releases/latest | grep -o -E "https://.+?-v0.12.0-linux-amd64" | head -n1)" > terraform-docs && chmod +x terraform-docs && sudo mv terraform-docs /usr/bin/ + curl -L "$(curl -s https://api.github.com/repos/terraform-docs/terraform-docs/releases/latest | grep -o -E "https://.+?-v0.12\..+?-linux-amd64" | head -n1)" > terraform-docs && chmod +x terraform-docs && sudo mv terraform-docs /usr/bin/ curl -L "$(curl -s https://api.github.com/repos/terraform-linters/tflint/releases/latest | grep -o -E "https://.+?_linux_amd64.zip")" > tflint.zip && unzip tflint.zip && rm tflint.zip && sudo mv tflint /usr/bin/ - name: Execute pre-commit # Run all pre-commit checks on max version supported