From 688f18f8c422dd69bfe4e250c3120b7d39e0110d Mon Sep 17 00:00:00 2001 From: Emile Swarts Date: Tue, 22 Oct 2024 11:18:24 +0100 Subject: [PATCH 1/4] test adding custom bucket policy --- s3/main.tf | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/s3/main.tf b/s3/main.tf index 05c8afc51..4af6e8fee 100644 --- a/s3/main.tf +++ b/s3/main.tf @@ -40,6 +40,22 @@ data "aws_iam_policy_document" "bucket-policy" { "${aws_s3_bucket.this.arn}/*", ] } + + dynamic "statement" { + for_each = var.config.cross_account_access_role_name ? 0 : 1 + + content { + actions = ["s3:PutObject"] + effect = "Allow" + + principals { + type = "AWS" + identifiers = [ + "arn:aws:iam::637423335187:role/cross-account-s3-export" + ] + } + } + } } resource "aws_s3_bucket_policy" "bucket-policy" { From ca0474e4f96b2fbddb905c7108c034f9c4664486 Mon Sep 17 00:00:00 2001 From: Emile Swarts Date: Tue, 22 Oct 2024 13:29:02 +0100 Subject: [PATCH 2/4] Debugging --- s3/main.tf | 32 ++++++++++++++++++-------------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/s3/main.tf b/s3/main.tf index 4af6e8fee..6bc80d5e4 100644 --- a/s3/main.tf +++ b/s3/main.tf @@ -41,21 +41,25 @@ data "aws_iam_policy_document" "bucket-policy" { ] } - dynamic "statement" { - for_each = var.config.cross_account_access_role_name ? 0 : 1 +# dynamic "statement" { +# for_each = [] +# +# content { +# actions = ["s3:PutObject"] +# effect = "Allow" +# +# principals { +# type = "AWS" +# identifiers = [ +# "arn:aws:iam::637423335187:role/cross-account-s3-export" +# ] +# } +# } +# } +} - content { - actions = ["s3:PutObject"] - effect = "Allow" - - principals { - type = "AWS" - identifiers = [ - "arn:aws:iam::637423335187:role/cross-account-s3-export" - ] - } - } - } +output "foo" { + value = var.config } resource "aws_s3_bucket_policy" "bucket-policy" { From 25759abfb9842299af84c1ea27820c964ee01a62 Mon Sep 17 00:00:00 2001 From: Emile Swarts Date: Tue, 22 Oct 2024 16:35:29 +0100 Subject: [PATCH 3/4] Create dynamic statement for bucket policy --- s3/main.tf | 35 +++++++++++++++++------------------ s3/variables.tf | 1 + 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/s3/main.tf b/s3/main.tf index 6bc80d5e4..c1425fcd7 100644 --- a/s3/main.tf +++ b/s3/main.tf @@ -41,25 +41,24 @@ data "aws_iam_policy_document" "bucket-policy" { ] } -# dynamic "statement" { -# for_each = [] -# -# content { -# actions = ["s3:PutObject"] -# effect = "Allow" -# -# principals { -# type = "AWS" -# identifiers = [ -# "arn:aws:iam::637423335187:role/cross-account-s3-export" -# ] -# } -# } -# } -} + dynamic "statement" { + for_each = try(compact([var.config.cross_account_access_role]), []) + + content { + actions = ["s3:PutObject"] + effect = "Allow" + + principals { + type = "AWS" + identifiers = [var.config.cross_account_access_role] + } -output "foo" { - value = var.config + resources = [ + aws_s3_bucket.this.arn, + "${aws_s3_bucket.this.arn}/*", + ] + } + } } resource "aws_s3_bucket_policy" "bucket-policy" { diff --git a/s3/variables.tf b/s3/variables.tf index db9bb588f..43348407e 100644 --- a/s3/variables.tf +++ b/s3/variables.tf @@ -23,6 +23,7 @@ variable "vpc_name" { variable "config" { type = object({ bucket_name = string + cross_account_access_role = optional(string) versioning = optional(bool) retention_policy = optional(object({ mode = string From df9c491e2565a642c59737f5adc021e2dabdb82a Mon Sep 17 00:00:00 2001 From: Emile Swarts Date: Wed, 23 Oct 2024 14:21:28 +0100 Subject: [PATCH 4/4] Force apply kms key and policy regardless of static content --- s3/main.tf | 39 ++++++++++++++++++++++++--------------- s3/variables.tf | 2 +- 2 files changed, 25 insertions(+), 16 deletions(-) diff --git a/s3/main.tf b/s3/main.tf index c1425fcd7..abeaf96b4 100644 --- a/s3/main.tf +++ b/s3/main.tf @@ -42,7 +42,7 @@ data "aws_iam_policy_document" "bucket-policy" { } dynamic "statement" { - for_each = try(compact([var.config.cross_account_access_role]), []) + for_each = var.config.additional_access_role != null ? [1] : [] content { actions = ["s3:PutObject"] @@ -50,7 +50,7 @@ data "aws_iam_policy_document" "bucket-policy" { principals { type = "AWS" - identifiers = [var.config.cross_account_access_role] + identifiers = [var.config.additional_access_role] } resources = [ @@ -62,8 +62,6 @@ data "aws_iam_policy_document" "bucket-policy" { } resource "aws_s3_bucket_policy" "bucket-policy" { - count = var.config.serve_static_content ? 0 : 1 - bucket = aws_s3_bucket.this.id policy = data.aws_iam_policy_document.bucket-policy.json } @@ -101,26 +99,37 @@ resource "aws_s3_bucket_lifecycle_configuration" "lifecycle-configuration" { } resource "aws_kms_key" "kms-key" { - count = var.config.serve_static_content ? 0 : 1 - # checkov:skip=CKV_AWS_7:We are not currently rotating the keys description = "KMS Key for S3 encryption" tags = local.tags +} +resource "aws_kms_key_policy" "kms-key-policy" { + count = var.config.additional_access_role && var.config.serve_static_content ? 1 : 0 + + key_id = aws_kms_key.kms-key.id policy = jsonencode({ - Id = "key-default-1" + Version = "2012-10-17" + Id = "key-default-1" Statement = [ { - "Sid" : "Enable IAM User Permissions", - "Effect" : "Allow", - "Principal" : { - "AWS" : "arn:aws:iam::${data.aws_caller_identity.current.account_id}:root" + Sid = "Enable IAM User Permissions" + Effect = "Allow" + Principal = { + AWS = "arn:aws:iam::${data.aws_caller_identity.current.account_id}:root" }, - "Action" : "kms:*", - "Resource" : "*" - } + Action = "kms:*" + Resource = "*" + }, { + Sid = "Allow cross account S3 uploads", + Effect = "Allow", + Principal = { + AWS = var.config.additional_access_role + }, + Action = "kms:GenerateDataKey" + Resource = "*" + } ] - Version = "2012-10-17" }) } diff --git a/s3/variables.tf b/s3/variables.tf index 43348407e..04bebf769 100644 --- a/s3/variables.tf +++ b/s3/variables.tf @@ -23,7 +23,7 @@ variable "vpc_name" { variable "config" { type = object({ bucket_name = string - cross_account_access_role = optional(string) + additional_access_role = optional(string) versioning = optional(bool) retention_policy = optional(object({ mode = string