diff --git a/iam-policy-documents.tf b/iam-policy-documents.tf index 388c6bc..5ad6984 100644 --- a/iam-policy-documents.tf +++ b/iam-policy-documents.tf @@ -8,14 +8,25 @@ data "aws_iam_policy_document" "backend_assume_role_all" { principals { type = "AWS" - identifiers = length(var.all_workspaces_details) > 0 ? var.all_workspaces_details : [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]) } } } 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" { @@ -38,7 +60,7 @@ data "aws_iam_policy_document" "backend_assume_role_restricted" { principals { type = "AWS" - identifiers = length(each.value) > 0 ? each.value : [data.aws_caller_identity.current.account_id] + identifiers = length(each.value) > 0 ? each.value : tolist([data.aws_caller_identity.current.account_id]) } } } @@ -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/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 diff --git a/s3.tf b/s3.tf index 7bed220..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 } } } @@ -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/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 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 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 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" } } }