From a25984a61410ab1ee372b217e703f91cb9d6b99c Mon Sep 17 00:00:00 2001 From: Arpan Adhikari Date: Sat, 24 Jul 2021 17:01:36 +1000 Subject: [PATCH 1/9] replaced list with tolist --- iam-policy-documents.tf | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/iam-policy-documents.tf b/iam-policy-documents.tf index d89bc34..14dcaf0 100644 --- a/iam-policy-documents.tf +++ b/iam-policy-documents.tf @@ -8,7 +8,7 @@ data "aws_iam_policy_document" "backend_assume_role_all" { principals { type = "AWS" - identifiers = length(var.all_workspaces_details) > 0 ? var.all_workspaces_details : list(data.aws_caller_identity.current.account_id) + identifiers = length(var.all_workspaces_details) > 0 ? var.all_workspaces_details : tolist([data.aws_caller_identity.current.account_id]) } } } @@ -38,7 +38,7 @@ data "aws_iam_policy_document" "backend_assume_role_restricted" { principals { type = "AWS" - identifiers = "${length(each.value) > 0 ? each.value : list(data.aws_caller_identity.current.account_id)}" + identifiers = "${length(each.value) > 0 ? each.value : tolist([data.aws_caller_identity.current.account_id])}" } } } From 9af0787f3769e9ba477e7f7d526aeeb9fc8b1d4e Mon Sep 17 00:00:00 2001 From: Arpan Adhikari Date: Sat, 24 Jul 2021 17:15:02 +1000 Subject: [PATCH 2/9] removed interpolation char --- iam-policy-documents.tf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/iam-policy-documents.tf b/iam-policy-documents.tf index 14dcaf0..d6e62bc 100644 --- a/iam-policy-documents.tf +++ b/iam-policy-documents.tf @@ -38,7 +38,7 @@ data "aws_iam_policy_document" "backend_assume_role_restricted" { principals { type = "AWS" - identifiers = "${length(each.value) > 0 ? each.value : tolist([data.aws_caller_identity.current.account_id])}" + identifiers = "length(each.value) > 0 ? each.value : tolist([data.aws_caller_identity.current.account_id])" } } } From 12552e0520caad68bf75b8f16901f910dd7f76fe Mon Sep 17 00:00:00 2001 From: Arpan Adhikari Date: Sat, 24 Jul 2021 17:21:35 +1000 Subject: [PATCH 3/9] fixed typo --- iam-policy-documents.tf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/iam-policy-documents.tf b/iam-policy-documents.tf index d6e62bc..b7e68ae 100644 --- a/iam-policy-documents.tf +++ b/iam-policy-documents.tf @@ -38,7 +38,7 @@ data "aws_iam_policy_document" "backend_assume_role_restricted" { principals { type = "AWS" - identifiers = "length(each.value) > 0 ? each.value : tolist([data.aws_caller_identity.current.account_id])" + identifiers = ["length(each.value) > 0 ? each.value : tolist([data.aws_caller_identity.current.account_id])"] } } } From 2f627350bef473a39ac078c112500cb3cbb08d24 Mon Sep 17 00:00:00 2001 From: Arpan Adhikari Date: Sat, 24 Jul 2021 17:36:18 +1000 Subject: [PATCH 4/9] removed extra [] --- iam-policy-documents.tf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/iam-policy-documents.tf b/iam-policy-documents.tf index b7e68ae..7b7aa5e 100644 --- a/iam-policy-documents.tf +++ b/iam-policy-documents.tf @@ -38,7 +38,7 @@ data "aws_iam_policy_document" "backend_assume_role_restricted" { principals { type = "AWS" - identifiers = ["length(each.value) > 0 ? each.value : tolist([data.aws_caller_identity.current.account_id])"] + identifiers = length(each.value) > 0 ? each.value : tolist([data.aws_caller_identity.current.account_id]) } } } From 723a9d3e3ee59bf912c6bdad9e55d05860a9b489 Mon Sep 17 00:00:00 2001 From: Arpan Adhikari Date: Sat, 24 Jul 2021 21:14:16 +1000 Subject: [PATCH 5/9] added kms_key_id output. required for multi environment setups --- outputs.tf | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/outputs.tf b/outputs.tf index 899d9e4..a132879 100644 --- a/outputs.tf +++ b/outputs.tf @@ -9,3 +9,7 @@ output "dynamo_lock_table" { output "iam_roles" { value = concat(aws_iam_role.backend_all[*].arn, values(aws_iam_role.backend_restricted)[*].arn) } + +output "kms_key_id"{ + value = var.enable_customer_kms_key ? aws_kms_key.backend[0].arn: null +} \ No newline at end of file From 3ad0de066cd5cb0b332b82b48af09fbcb66379b5 Mon Sep 17 00:00:00 2001 From: Arpan Adhikari Date: Sat, 8 Jan 2022 11:45:30 +1100 Subject: [PATCH 6/9] option to disable acl --- s3.tf | 10 ++++++++++ variables.tf | 6 ++++++ 2 files changed, 16 insertions(+) diff --git a/s3.tf b/s3.tf index 7bed220..70b8990 100644 --- a/s3.tf +++ b/s3.tf @@ -34,6 +34,16 @@ resource "aws_s3_bucket" "backend" { tags = var.tags } +# Setting object_ownership="BucketOwnerEnforced" will effectively disable ACL on the bucket +# default option is to keep it enabled. +resource "aws_s3_bucket_ownership_controls" "acl_set" { + count = var.disable_acl ? 1 : 0 + bucket = aws_s3_bucket.backend.id + rule { + object_ownership = "BucketOwnerEnforced" + } +} + resource "aws_s3_bucket_public_access_block" "backend" { bucket = aws_s3_bucket.backend.id diff --git a/variables.tf b/variables.tf index 73ff5f0..7ac520f 100644 --- a/variables.tf +++ b/variables.tf @@ -37,3 +37,9 @@ variable "all_workspaces_details" { description = "A list of aws principles that will be allowed to assume the backend-all role" default = [] } + +variable "disable_acl" { + type = string + description = "The ACL to apply to the S3 bucket" + default = false +} \ No newline at end of file From 70b06702b90f32d37c74411a4b78ff1634b33657 Mon Sep 17 00:00:00 2001 From: Arpan Adhikari Date: Sat, 8 Jan 2022 11:46:33 +1100 Subject: [PATCH 7/9] upgrade aws provider version, required for object_ownership=BucketOwnerEnforced --- versions.tf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/versions.tf b/versions.tf index 57a584c..e5c887c 100644 --- a/versions.tf +++ b/versions.tf @@ -4,7 +4,7 @@ terraform { required_providers { aws = { source = "hashicorp/aws" - version = ">= 2.70.0" + version = ">= 3.69.0" } } } From d3b8b8cd092b14e1ef6a3df9bae05d8612e79241 Mon Sep 17 00:00:00 2001 From: Arpan Adhikari Date: Sat, 8 Jan 2022 11:47:16 +1100 Subject: [PATCH 8/9] added test for disabling bucket acl --- tests/main.tf | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/main.tf b/tests/main.tf index d9f528f..0b43ca9 100644 --- a/tests/main.tf +++ b/tests/main.tf @@ -74,3 +74,18 @@ module "tf-backend4" { Department = "Bar" } } + +# all default bucket acl disabled +module "tf-backend2" { + source = "../" + + resource_prefix = "backend-ci-test4-${var.resource_suffix}" + + disable_acl = true + + workspace_details = { + "prod" = [] + "nonprod" = [] + "sandpit" = [] + } +} \ No newline at end of file From 2a9a3756aa9d7c199eb02d6daa43f872566cf8fc Mon Sep 17 00:00:00 2001 From: Arpan Adhikari Date: Tue, 5 Nov 2024 23:05:36 +1100 Subject: [PATCH 9/9] experience uplift --- iam-policy-documents.tf | 48 +++++++++++++++++++++++++++++++++++++++-- iam.tf | 12 ++++------- kms.tf | 15 +++++++------ s3.tf | 2 +- s3_bucket_policy.tf | 21 ++++++++++++++++++ 5 files changed, 81 insertions(+), 17 deletions(-) create mode 100644 s3_bucket_policy.tf diff --git a/iam-policy-documents.tf b/iam-policy-documents.tf index 7b7aa5e..5ad6984 100644 --- a/iam-policy-documents.tf +++ b/iam-policy-documents.tf @@ -15,7 +15,18 @@ data "aws_iam_policy_document" "backend_assume_role_all" { data "aws_iam_policy_document" "iam_role_policy_all" { statement { - actions = ["s3:GetBucketVersioning", "s3:ListBucket"] + actions = [ + "s3:GetBucketVersioning", + "s3:ListBucket", + "s3:GetEncryptionConfiguration", + "s3:GetBucketPolicy", + "s3:GetBucketPublicAccessBlock", + "s3:GetBucketAcl", + "s3:GetBucketLocation", + "s3:GetBucketTagging", + "s3:GetBucketOwnershipControls", + "s3:GetBucketVersioning" + ] resources = ["arn:aws:s3:::${aws_s3_bucket.backend.id}"] } @@ -28,6 +39,17 @@ data "aws_iam_policy_document" "iam_role_policy_all" { actions = ["dynamodb:DescribeTable", "dynamodb:GetItem", "dynamodb:PutItem", "dynamodb:DeleteItem"] resources = ["arn:aws:dynamodb:*:*:table/${var.resource_prefix}-terraform-lock"] } + + statement { + actions = [ + "kms:Encrypt", + "kms:Decrypt", + "kms:ReEncrypt*", + "kms:GenerateDataKey*", + "kms:DescribeKey" + ] + resources = var.enable_customer_kms_key ? [aws_kms_key.backend[0].arn] : [] + } } data "aws_iam_policy_document" "backend_assume_role_restricted" { @@ -47,7 +69,18 @@ data "aws_iam_policy_document" "iam_role_policy_restricted" { for_each = var.workspace_details statement { - actions = ["s3:GetBucketVersioning", "s3:ListBucket"] + actions = [ + "s3:GetBucketVersioning", + "s3:ListBucket", + "s3:GetEncryptionConfiguration", + "s3:GetBucketPolicy", + "s3:GetBucketPublicAccessBlock", + "s3:GetBucketAcl", + "s3:GetBucketLocation", + "s3:GetBucketTagging", + "s3:GetBucketOwnershipControls", + "s3:GetBucketVersioning" + ] resources = ["arn:aws:s3:::${aws_s3_bucket.backend.id}"] } @@ -60,4 +93,15 @@ data "aws_iam_policy_document" "iam_role_policy_restricted" { actions = ["dynamodb:DescribeTable", "dynamodb:GetItem", "dynamodb:PutItem", "dynamodb:DeleteItem"] resources = ["arn:aws:dynamodb:*:*:table/${var.resource_prefix}-terraform-lock"] } + + statement { + actions = [ + "kms:Encrypt", + "kms:Decrypt", + "kms:ReEncrypt*", + "kms:GenerateDataKey*", + "kms:DescribeKey" + ] + resources = var.enable_customer_kms_key ? [aws_kms_key.backend[0].arn] : [] + } } diff --git a/iam.tf b/iam.tf index 3d6c36b..c15534c 100644 --- a/iam.tf +++ b/iam.tf @@ -8,10 +8,8 @@ resource "aws_iam_role" "backend_all" { resource "aws_iam_role_policy" "backend_all" { name = "${var.resource_prefix}-terraform-backend" - role = "${var.resource_prefix}-terraform-backend" + role = aws_iam_role.backend_all.id policy = data.aws_iam_policy_document.iam_role_policy_all.json - - depends_on = [aws_iam_role.backend_all] } #These roles are limited to their specific workspace through the use of S3 resource permissions @@ -20,7 +18,7 @@ resource "aws_iam_role" "backend_restricted" { name = "${var.resource_prefix}-terraform-backend-${each.key}" description = "Allows access to the ${each.key} workspace prefix" - assume_role_policy = data.aws_iam_policy_document.backend_assume_role_restricted["${each.key}"].json + assume_role_policy = data.aws_iam_policy_document.backend_assume_role_restricted[each.key].json tags = var.tags } @@ -28,8 +26,6 @@ resource "aws_iam_role_policy" "backend_restricted" { for_each = var.workspace_details name = "${var.resource_prefix}-terraform-backend-${each.key}" - policy = data.aws_iam_policy_document.iam_role_policy_restricted["${each.key}"].json - role = "${var.resource_prefix}-terraform-backend-${each.key}" - - depends_on = [aws_iam_role.backend_restricted] + role = aws_iam_role.backend_restricted[each.key].id + policy = data.aws_iam_policy_document.iam_role_policy_restricted[each.key].json } diff --git a/kms.tf b/kms.tf index 38f923a..e27981d 100644 --- a/kms.tf +++ b/kms.tf @@ -13,9 +13,9 @@ resource "aws_kms_alias" "backend" { data "aws_iam_policy_document" "kms" { count = var.enable_customer_kms_key ? 1 : 0 + statement { effect = "Allow" - principals { identifiers = [ "arn:aws:iam::${data.aws_caller_identity.current.account_id}:root", @@ -28,16 +28,19 @@ data "aws_iam_policy_document" "kms" { statement { effect = "Allow" - principals { - identifiers = concat(aws_iam_role.backend_all[*].arn, values(aws_iam_role.backend_restricted)[*].arn) - type = "AWS" + identifiers = concat( + aws_iam_role.backend_all[*].arn, + values(aws_iam_role.backend_restricted)[*].arn, + flatten(values(var.workspace_details)) + ) + type = "AWS" } actions = [ "kms:Encrypt", "kms:Decrypt", - "kms:ReEncrypt", - "kms:GenerateDataKey", + "kms:ReEncrypt*", + "kms:GenerateDataKey*", "kms:DescribeKey" ] resources = ["*"] diff --git a/s3.tf b/s3.tf index 70b8990..8f52de2 100644 --- a/s3.tf +++ b/s3.tf @@ -26,7 +26,7 @@ resource "aws_s3_bucket" "backend" { rule { apply_server_side_encryption_by_default { sse_algorithm = "aws:kms" - kms_master_key_id = var.enable_customer_kms_key ? aws_kms_key.backend[0].id : null + kms_master_key_id = var.enable_customer_kms_key ? aws_kms_key.backend[0].arn : null } } } diff --git a/s3_bucket_policy.tf b/s3_bucket_policy.tf new file mode 100644 index 0000000..9956e97 --- /dev/null +++ b/s3_bucket_policy.tf @@ -0,0 +1,21 @@ +resource "aws_s3_bucket_policy" "s3_bucket_policy_restricted" { + bucket = aws_s3_bucket.backend.id + policy = data.aws_iam_policy_document.allow_access_from_another_account.json +} + +data "aws_iam_policy_document" "allow_access_from_another_account" { + dynamic "statement" { + for_each = var.workspace_details + content { + actions = ["s3:*"] + resources = [ + "arn:aws:s3:::${aws_s3_bucket.backend.id}/${local.workspace_key_prefix}${statement.key}*", + "arn:aws:s3:::${aws_s3_bucket.backend.id}" + ] + principals { + type = "AWS" + identifiers = flatten(statement.value) + } + } + } +} \ No newline at end of file