diff --git a/infrastructure/modules/s3/main.tf b/infrastructure/modules/s3/main.tf index d48cf86e..2e985787 100755 --- a/infrastructure/modules/s3/main.tf +++ b/infrastructure/modules/s3/main.tf @@ -57,7 +57,7 @@ module "s3" { } resource "aws_s3_bucket_policy" "enforce_kms_truststore" { - count = var.enable_kms_encryption ? 1 : 0 + count = var.enable_kms_encryption && var.enable_kms_bucket_policy ? 1 : 0 bucket = module.s3.s3_bucket_id policy = jsonencode({ Version = "2012-10-17" Statement = [ diff --git a/infrastructure/modules/s3/variables.tf b/infrastructure/modules/s3/variables.tf index a5fa0f58..bb5157f2 100755 --- a/infrastructure/modules/s3/variables.tf +++ b/infrastructure/modules/s3/variables.tf @@ -34,8 +34,18 @@ variable "bucket_name" { # description = "The path to the zip file" # type = string # } -variable "attach_policy" { default = false } -variable "policy" { default = null } +variable "attach_policy" { + description = "Whether to attach a policy to the s3 bucket" + type = bool + default = false +} + +variable "policy" { + description = "s3 bucket policy as a JSON string" + type = string + default = null +} + variable "lifecycle_rule_inputs" { default = [] } variable "target_access_logging_bucket" { @@ -82,3 +92,9 @@ variable "enable_kms_encryption" { type = bool default = false } + +variable "enable_kms_bucket_policy" { + description = "Whether to attach the KMS enforcement bucket policy. Disable if managing policy externally" + type = bool + default = true +} diff --git a/infrastructure/stacks/artefact_management/s3.tf b/infrastructure/stacks/artefact_management/s3.tf index ad928fa7..598c2983 100644 --- a/infrastructure/stacks/artefact_management/s3.tf +++ b/infrastructure/stacks/artefact_management/s3.tf @@ -34,22 +34,18 @@ locals { all_bucket_principals = flatten(values(local.principals_by_environment)) } - module "artefacts_bucket" { source = "../../modules/s3" bucket_name = local.artefacts_bucket - enable_kms_encryption = true - s3_encryption_key_arn = module.s3_artefacts_encryption_key.arn -} - - -resource "aws_s3_bucket_policy" "artefacts_bucket_policy" { - depends_on = [module.artefacts_bucket] - bucket = local.artefacts_bucket - policy = data.aws_iam_policy_document.artefacts_bucket_policy.json + enable_kms_encryption = true + s3_encryption_key_arn = module.s3_artefacts_encryption_key.arn + attach_policy = true + policy = data.aws_iam_policy_document.artefacts_bucket_policy.json + enable_kms_bucket_policy = false } data "aws_iam_policy_document" "artefacts_bucket_policy" { + statement { principals { type = "AWS" @@ -79,4 +75,36 @@ data "aws_iam_policy_document" "artefacts_bucket_policy" { "${module.artefacts_bucket.s3_bucket_arn}/*", ] } + + statement { + sid = "DenyUnencryptedUploads" + effect = "Deny" + principals { + type = "AWS" + identifiers = ["*"] + } + actions = ["s3:PutObject"] + resources = ["${module.artefacts_bucket.s3_bucket_arn}/*"] + condition { + test = "StringNotEquals" + variable = "s3:x-amz-server-side-encryption" + values = ["aws:kms"] + } + } + + statement { + sid = "DenyUnencryptedKMSUploads" + effect = "Deny" + principals { + type = "AWS" + identifiers = ["*"] + } + actions = ["s3:PutObject"] + resources = ["${module.artefacts_bucket.s3_bucket_arn}/*"] + condition { + test = "ArnNotEquals" + variable = "s3:x-amz-server-side-encryption-aws-kms-key-id" + values = ["module.s3_artefacts_encryption_key.arn"] + } + } } diff --git a/infrastructure/stacks/github_runner/account_github_runner_data.policy.json.tpl b/infrastructure/stacks/github_runner/account_github_runner_data.policy.json.tpl index ebdb1a5f..2f9dabe9 100644 --- a/infrastructure/stacks/github_runner/account_github_runner_data.policy.json.tpl +++ b/infrastructure/stacks/github_runner/account_github_runner_data.policy.json.tpl @@ -76,7 +76,8 @@ "Sid": "TerraformStateLockingDynamoTable", "Effect": "Allow", "Action": [ - "dynamodb:DeleteItem", + "dynamodb:Delete*", + "dynamodb:Create*", "dynamodb:Describe*", "dynamodb:Get*", "dynamodb:List*", diff --git a/infrastructure/stacks/github_runner/app_github_runner_data.policy.json.tpl b/infrastructure/stacks/github_runner/app_github_runner_data.policy.json.tpl index 99f16b89..5a7c2920 100644 --- a/infrastructure/stacks/github_runner/app_github_runner_data.policy.json.tpl +++ b/infrastructure/stacks/github_runner/app_github_runner_data.policy.json.tpl @@ -83,7 +83,8 @@ "Sid": "TerraformStateLockingDynamoTable", "Effect": "Allow", "Action": [ - "dynamodb:DeleteItem", + "dynamodb:Delete*", + "dynamodb:Create*", "dynamodb:Describe*", "dynamodb:Get*", "dynamodb:List*",