From e574e1e86eaa646b0cf8b598cbc5365ec8d60b92 Mon Sep 17 00:00:00 2001 From: Edd Almond Date: Fri, 11 Jul 2025 11:36:10 +0100 Subject: [PATCH 1/3] eli-327 removing duplicated policies --- .../modules/kinesis_firehose/kms.tf | 3 --- infrastructure/modules/s3/kms.tf | 18 ------------------ .../stacks/api-layer/iam_policies.tf | 7 ------- .../stacks/api-layer/truststore_s3_bucket.tf | 19 +++++++++++++++++++ 4 files changed, 19 insertions(+), 28 deletions(-) diff --git a/infrastructure/modules/kinesis_firehose/kms.tf b/infrastructure/modules/kinesis_firehose/kms.tf index a725c4fab..a200569da 100644 --- a/infrastructure/modules/kinesis_firehose/kms.tf +++ b/infrastructure/modules/kinesis_firehose/kms.tf @@ -45,7 +45,6 @@ data "aws_iam_policy_document" "firehose_kms_key_policy" { resources = ["*"] } - # Your existing statements below... statement { sid = "AllowFirehoseAccess" effect = "Allow" @@ -110,5 +109,3 @@ data "aws_iam_policy_document" "firehose_kms_key_policy" { resources = [aws_kms_key.firehose_cmk.arn] } } - - diff --git a/infrastructure/modules/s3/kms.tf b/infrastructure/modules/s3/kms.tf index decb78315..56321c4ab 100644 --- a/infrastructure/modules/s3/kms.tf +++ b/infrastructure/modules/s3/kms.tf @@ -14,21 +14,3 @@ resource "aws_kms_alias" "storage_bucket_cmk" { name = "alias/${terraform.workspace == "default" ? "" : "${terraform.workspace}-"}${var.bucket_name}-cmk" target_key_id = aws_kms_key.storage_bucket_cmk.key_id } - -resource "aws_kms_key_policy" "storage_bucket_cmk" { - key_id = aws_kms_key.storage_bucket_cmk.id - policy = data.aws_iam_policy_document.storage_bucket_cmk.json -} - -data "aws_iam_policy_document" "storage_bucket_cmk" { - statement { - sid = "Enable IAM User Permissions for s3 buckets" - effect = "Allow" - principals { - type = "AWS" - identifiers = ["arn:aws:iam::${data.aws_caller_identity.current.account_id}:root"] - } - actions = ["kms:*"] - resources = [aws_kms_key.storage_bucket_cmk.arn] - } -} diff --git a/infrastructure/stacks/api-layer/iam_policies.tf b/infrastructure/stacks/api-layer/iam_policies.tf index 336903385..9b730345c 100644 --- a/infrastructure/stacks/api-layer/iam_policies.tf +++ b/infrastructure/stacks/api-layer/iam_policies.tf @@ -235,7 +235,6 @@ data "aws_iam_policy_document" "s3_audit_kms_key_policy" { actions = ["kms:*"] resources = ["*"] } - statement { sid = "AllowLambdaFullWrite" effect = "Allow" @@ -277,9 +276,3 @@ resource "aws_iam_role_policy" "lambda_firehose_policy" { role = aws_iam_role.eligibility_lambda_role.id policy = data.aws_iam_policy_document.lambda_firehose_write_policy.json } - - - - - - diff --git a/infrastructure/stacks/api-layer/truststore_s3_bucket.tf b/infrastructure/stacks/api-layer/truststore_s3_bucket.tf index d78ad6f9c..7c54710eb 100644 --- a/infrastructure/stacks/api-layer/truststore_s3_bucket.tf +++ b/infrastructure/stacks/api-layer/truststore_s3_bucket.tf @@ -13,6 +13,25 @@ resource "aws_s3_bucket_policy" "truststore" { } data "aws_iam_policy_document" "truststore_api_gateway" { + # Deny non-SSL + statement { + sid = "AllowSslRequestsOnly" + actions = ["s3:*"] + effect = "Deny" + resources = [ + module.s3_truststore_bucket.storage_bucket_arn, + "${module.s3_truststore_bucket.storage_bucket_arn}/*" + ] + principals { + type = "*" + identifiers = ["*"] + } + condition { + test = "Bool" + variable = "aws:SecureTransport" + values = ["false"] + } + } statement { sid = "Enable S3 access permissions for API Gateway" effect = "Allow" From 5d100209c89cf0e550561a32b1398faf31a16be2 Mon Sep 17 00:00:00 2001 From: Edd Almond Date: Fri, 11 Jul 2025 16:03:15 +0100 Subject: [PATCH 2/3] eli-327 fixing SSL transport as default, changing arn to id to avoid terraform constantly updating CMKs --- infrastructure/modules/s3/outputs.tf | 4 ++ infrastructure/modules/s3/s3.tf | 33 --------- .../stacks/api-layer/iam_policies.tf | 68 ++++++++++++++++++- 3 files changed, 70 insertions(+), 35 deletions(-) diff --git a/infrastructure/modules/s3/outputs.tf b/infrastructure/modules/s3/outputs.tf index 407876513..7a6d35722 100644 --- a/infrastructure/modules/s3/outputs.tf +++ b/infrastructure/modules/s3/outputs.tf @@ -21,3 +21,7 @@ output "storage_bucket_id" { output "storage_bucket_kms_key_arn" { value = aws_kms_key.storage_bucket_cmk.arn } + +output "storage_bucket_kms_key_id" { + value = aws_kms_key.storage_bucket_cmk.id +} diff --git a/infrastructure/modules/s3/s3.tf b/infrastructure/modules/s3/s3.tf index eb65cdfe5..e0138c065 100644 --- a/infrastructure/modules/s3/s3.tf +++ b/infrastructure/modules/s3/s3.tf @@ -14,39 +14,6 @@ resource "aws_s3_bucket_versioning" "storage_bucket_versioning_config" { } } -# ensure only secure transport is allowed - -resource "aws_s3_bucket_policy" "storage_bucket" { - bucket = aws_s3_bucket.storage_bucket.id - policy = data.aws_iam_policy_document.storage_s3_bucket_policy.json -} - -data "aws_iam_policy_document" "storage_s3_bucket_policy" { - statement { - sid = "AllowSslRequestsOnly" - actions = [ - "s3:*", - ] - effect = "Deny" - resources = [ - aws_s3_bucket.storage_bucket.arn, - "${aws_s3_bucket.storage_bucket.arn}/*", - ] - principals { - type = "*" - identifiers = ["*"] - } - condition { - test = "Bool" - values = [ - "false", - ] - - variable = "aws:SecureTransport" - } - } -} - # Block public access to the bucket resource "aws_s3_bucket_public_access_block" "storage_bucket_block_public_access" { bucket = aws_s3_bucket.storage_bucket.id diff --git a/infrastructure/stacks/api-layer/iam_policies.tf b/infrastructure/stacks/api-layer/iam_policies.tf index 9b730345c..00b5d914f 100644 --- a/infrastructure/stacks/api-layer/iam_policies.tf +++ b/infrastructure/stacks/api-layer/iam_policies.tf @@ -71,6 +71,70 @@ data "aws_iam_policy_document" "s3_rules_bucket_policy" { } } +# ensure only secure transport is allowed + +resource "aws_s3_bucket_policy" "rules_s3_bucket" { + bucket = module.s3_rules_bucket.storage_bucket_id + policy = data.aws_iam_policy_document.rules_s3_bucket_policy.json +} + +data "aws_iam_policy_document" "rules_s3_bucket_policy" { + statement { + sid = "AllowSslRequestsOnly" + actions = [ + "s3:*", + ] + effect = "Deny" + resources = [ + module.s3_rules_bucket.storage_bucket_arn, + "${module.s3_rules_bucket.storage_bucket_arn}/*", + ] + principals { + type = "*" + identifiers = ["*"] + } + condition { + test = "Bool" + values = [ + "false", + ] + + variable = "aws:SecureTransport" + } + } +} + +resource "aws_s3_bucket_policy" "audit_s3_bucket" { + bucket = module.s3_audit_bucket.storage_bucket_id + policy = data.aws_iam_policy_document.audit_s3_bucket_policy.json +} + +data "aws_iam_policy_document" "audit_s3_bucket_policy" { + statement { + sid = "AllowSslRequestsOnly" + actions = [ + "s3:*", + ] + effect = "Deny" + resources = [ + module.s3_audit_bucket.storage_bucket_arn, + "${module.s3_audit_bucket.storage_bucket_arn}/*", + ] + principals { + type = "*" + identifiers = ["*"] + } + condition { + test = "Bool" + values = [ + "false", + ] + + variable = "aws:SecureTransport" + } + } +} + # Attach s3 read policy to Lambda role resource "aws_iam_role_policy" "lambda_s3_read_policy" { name = "S3ReadAccess" @@ -216,7 +280,7 @@ data "aws_iam_policy_document" "s3_rules_kms_key_policy" { } resource "aws_kms_key_policy" "s3_rules_kms_key" { - key_id = module.s3_rules_bucket.storage_bucket_kms_key_arn + key_id = module.s3_rules_bucket.storage_bucket_kms_key_id policy = data.aws_iam_policy_document.s3_rules_kms_key_policy.json } @@ -253,7 +317,7 @@ data "aws_iam_policy_document" "s3_audit_kms_key_policy" { } resource "aws_kms_key_policy" "s3_audit_kms_key" { - key_id = module.s3_audit_bucket.storage_bucket_kms_key_arn + key_id = module.s3_audit_bucket.storage_bucket_kms_key_id policy = data.aws_iam_policy_document.s3_audit_kms_key_policy.json } From 4e58423f9cf74c1fe633272803c6f064e5033ee0 Mon Sep 17 00:00:00 2001 From: Edd Almond Date: Fri, 11 Jul 2025 16:11:26 +0100 Subject: [PATCH 3/3] eli-327 adding checkov skip --- infrastructure/modules/s3/kms.tf | 1 + 1 file changed, 1 insertion(+) diff --git a/infrastructure/modules/s3/kms.tf b/infrastructure/modules/s3/kms.tf index 56321c4ab..a4841f3c1 100644 --- a/infrastructure/modules/s3/kms.tf +++ b/infrastructure/modules/s3/kms.tf @@ -1,4 +1,5 @@ resource "aws_kms_key" "storage_bucket_cmk" { + #checkov:skip=CKV2_AWS_64: KMS key policy is defined in api-layer iam_policies.tf description = "${terraform.workspace == "default" ? "" : "${terraform.workspace}-"}${var.bucket_name} Master Key" deletion_window_in_days = 14 is_enabled = true