From 056038914410d45f366c36ea31ee7912f91ecf4b Mon Sep 17 00:00:00 2001
From: Chris Wright <chris@dxw.com>
Date: Fri, 27 Oct 2023 13:25:55 +0100
Subject: [PATCH] Fix KMS and S3 policies

* `jsonencode`s the s3 log bucket source arns
* Correctly formats `service-allow-decrypt.json.tpl`
* Uses the key arn rather than alias for cloudtrail
* Replaces the key arn with a wildcard, as ARNs can't contain wildcards
* Corrects the logs service name
* Adds the aws account ID to the logs bucket, so that it is globally
  unique
---
 cloudtrail.tf                                               | 6 +++---
 .../cloudwatch-logs-allow.json.tpl                          | 2 +-
 .../service-allow-decrypt.json.tpl                          | 2 +-
 s3-logs.tf                                                  | 4 ++--
 4 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/cloudtrail.tf b/cloudtrail.tf
index 3620c84..ef59fe8 100644
--- a/cloudtrail.tf
+++ b/cloudtrail.tf
@@ -28,7 +28,7 @@ resource "aws_kms_key" "cloudtrail_cloudwatch_logs" {
       ${templatefile("${path.root}/policies/kms-key-policy-statements/service-describe-key.json.tpl",
       {
         services   = jsonencode(["cloudtrail.amazonaws.com"])
-        key_arn    = "arn:aws:kms:${local.aws_region}:${local.aws_account_id}:key/*"
+        key_arn    = "*"
         source_arn = "arn:aws:cloudtrail:${local.aws_region}:${local.aws_account_id}:trail/${local.project_name}"
       }
       )},
@@ -55,7 +55,7 @@ resource "aws_cloudwatch_log_group" "cloudtrail" {
 
   name              = "${local.project_name}-cloudtrail"
   retention_in_days = local.cloudtrail_log_retention
-  kms_key_id        = local.cloudtrail_kms_encryption ? aws_kms_alias.cloudtrail_cloudwatch_logs[0].name : null
+  kms_key_id        = local.cloudtrail_kms_encryption ? aws_kms_key.cloudtrail_cloudwatch_logs[0].arn : null
   skip_destroy      = true
 }
 
@@ -99,7 +99,7 @@ resource "aws_cloudtrail" "cloudtrail" {
   cloud_watch_logs_role_arn     = aws_iam_role.cloudtrail_cloudwatch_logs[0].arn
   cloud_watch_logs_group_arn    = "${aws_cloudwatch_log_group.cloudtrail[0].arn}:*"
   enable_log_file_validation    = true
-  kms_key_id                    = local.cloudtrail_kms_encryption ? aws_kms_alias.cloudtrail_cloudwatch_logs[0].name : null
+  kms_key_id                    = local.cloudtrail_kms_encryption ? aws_kms_key.cloudtrail_cloudwatch_logs[0].arn : null
 
   depends_on = [
     aws_s3_bucket_policy.cloudtrail
diff --git a/policies/kms-key-policy-statements/cloudwatch-logs-allow.json.tpl b/policies/kms-key-policy-statements/cloudwatch-logs-allow.json.tpl
index ad2bb99..6cc028e 100644
--- a/policies/kms-key-policy-statements/cloudwatch-logs-allow.json.tpl
+++ b/policies/kms-key-policy-statements/cloudwatch-logs-allow.json.tpl
@@ -1,7 +1,7 @@
 {
   "Effect": "Allow",
   "Principal": {
-    "Service": "logs.region.amazonaws.com"
+    "Service": "logs.amazonaws.com"
   },
   "Action": [
     "kms:Encrypt*",
diff --git a/policies/kms-key-policy-statements/service-allow-decrypt.json.tpl b/policies/kms-key-policy-statements/service-allow-decrypt.json.tpl
index 101c660..579d4ad 100644
--- a/policies/kms-key-policy-statements/service-allow-decrypt.json.tpl
+++ b/policies/kms-key-policy-statements/service-allow-decrypt.json.tpl
@@ -1,7 +1,7 @@
 {
   "Effect": "Allow",
   "Principal": {
-    Service": ${services}
+    "Service": ${services}
   },
   "Action": "kms:Decrypt",
   "Resource": "*"
diff --git a/s3-logs.tf b/s3-logs.tf
index fdce7cf..dd405cb 100644
--- a/s3-logs.tf
+++ b/s3-logs.tf
@@ -3,7 +3,7 @@
 resource "aws_s3_bucket" "logs" {
   count = local.enable_logs_bucket ? 1 : 0
 
-  bucket = "${local.project_name}-logs"
+  bucket = "${local.aws_account_id}-${local.project_name}-logs"
 }
 
 resource "aws_s3_bucket_policy" "logs" {
@@ -18,7 +18,7 @@ resource "aws_s3_bucket_policy" "logs" {
       ${templatefile("${path.root}/policies/s3-bucket-policy-statements/enforce-tls.json.tpl", { bucket_arn = aws_s3_bucket.logs[0].arn })},
       ${templatefile("${path.root}/policies/s3-bucket-policy-statements/log-delivery-access.json.tpl", {
       log_bucket_arn     = aws_s3_bucket.logs[0].arn
-      source_bucket_arns = local.logs_bucket_source_arns
+      source_bucket_arns = jsonencode(local.logs_bucket_source_arns)
       account_id         = local.aws_account_id
 })}
       ]