Fix S3 Bucket logging

Stretch96 · Stretch96 · commit 024e2cf59a80 · 2022-05-29T12:47:08.000+01:00
* Add the S3 policy to actually allow buckets to log
diff --git a/README.md b/README.md
@@ -56,7 +56,6 @@ It can in most cases be used to host any static site, however this module adds s
 | [aws_iam_policy.publii_s3_frontend](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_policy) | resource |
 | [aws_iam_user.publii_s3_frontend](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_user) | resource |
 | [aws_iam_user_policy_attachment.publii_s3_frontend](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_user_policy_attachment) | resource |
-| [aws_kms_key.logs](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/kms_key) | resource |
 | [aws_kms_key.s3_bucket_frontend_www_redirect](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/kms_key) | resource |
 | [aws_lambda_permission.cloudfront_invalidation_frontend_alllow_s3](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/lambda_permission) | resource |
 | [aws_route53_record.cloudfront_frontend_tls_certificate_dns_validation](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/route53_record) | resource |
@@ -87,6 +86,7 @@ It can in most cases be used to host any static site, however this module adds s
 | [aws_s3_bucket_website_configuration.frontend_www_redirect](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/s3_bucket_website_configuration) | resource |
 | [aws_wafv2_web_acl.cloudfront_waf](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/wafv2_web_acl) | resource |
 | [random_id.project](https://registry.terraform.io/providers/hashicorp/random/latest/docs/resources/id) | resource |
+| [aws_caller_identity.current](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/caller_identity) | data source |
 | [aws_route53_zone.default](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/route53_zone) | data source |
 | [template_file.cloudfront_frontend_viewer_request_function](https://registry.terraform.io/providers/hashicorp/template/latest/docs/data-sources/file) | data source |
 | [template_file.frontend_bucket_cloudfront_read](https://registry.terraform.io/providers/hashicorp/template/latest/docs/data-sources/file) | data source |
@@ -97,6 +97,7 @@ It can in most cases be used to host any static site, however this module adds s
 | [template_file.frontend_www_redirect_bucket_policy](https://registry.terraform.io/providers/hashicorp/template/latest/docs/data-sources/file) | data source |
 | [template_file.lambda_cloudfront_invalidation_frontend_policy](https://registry.terraform.io/providers/hashicorp/template/latest/docs/data-sources/file) | data source |
 | [template_file.logs_bucket_enforce_tls_statement](https://registry.terraform.io/providers/hashicorp/template/latest/docs/data-sources/file) | data source |
+| [template_file.logs_bucket_log_delivery_access_statement](https://registry.terraform.io/providers/hashicorp/template/latest/docs/data-sources/file) | data source |
 | [template_file.logs_bucket_policy](https://registry.terraform.io/providers/hashicorp/template/latest/docs/data-sources/file) | data source |
 | [template_file.publii_s3_frontend_policy](https://registry.terraform.io/providers/hashicorp/template/latest/docs/data-sources/file) | data source |
 
diff --git a/data.tf b/data.tf
@@ -3,3 +3,5 @@ data "aws_route53_zone" "default" {
 
   zone_id = local.route53_hosted_zone_options.id
 }
+
+data "aws_caller_identity" "current" {}
diff --git a/kms.tf b/kms.tf
@@ -5,9 +5,3 @@ resource "aws_kms_key" "s3_bucket_frontend_www_redirect" {
   deletion_window_in_days = 10
   enable_key_rotation     = true
 }
-
-resource "aws_kms_key" "logs" {
-  description             = "This key is used to encrypt bucket objects within ${aws_s3_bucket.logs.id}"
-  deletion_window_in_days = 10
-  enable_key_rotation     = true
-}
diff --git a/locals.tf b/locals.tf
@@ -1,4 +1,5 @@
 locals {
+  account_id                             = data.aws_caller_identity.current.account_id
   site_url                               = var.site_url
   project_random_id                      = random_id.project.dec
   project_name                           = "${replace(local.site_url, ".", "-")}-${local.project_random_id}"
diff --git a/policies/s3-bucket-policy-statements/log-delivery-access.json.tpl b/policies/s3-bucket-policy-statements/log-delivery-access.json.tpl
@@ -0,0 +1,18 @@
+{
+  "Principal": {
+    "Service": "logging.s3.amazonaws.com"
+  },
+  "Action": [
+    "s3:PutObject"
+  ],
+  "Effect": "Allow",
+  "Resource": "${log_bucket_arn}/*",
+  "Condition": {
+    "ArnLike": {
+      "aws:SourceArn": ${source_bucket_arns}
+    },
+    "StringEquals": {
+      "aws:SourceAccount": "${account_id}"
+    }
+  }
+}
diff --git a/s3-logs.tf b/s3-logs.tf
@@ -23,13 +23,15 @@ resource "aws_s3_bucket_public_access_block" "logs" {
   restrict_public_buckets = true
 }
 
+# If default encryption is enabled on the target bucket, AES256 (SSE-S3) must be selected as the encryption key
+# https://aws.amazon.com/premiumsupport/knowledge-center/s3-server-access-log-not-delivered/
+#tfsec:ignore:aws-s3-encryption-customer-key
 resource "aws_s3_bucket_server_side_encryption_configuration" "logs" {
   bucket = aws_s3_bucket.logs.bucket
 
   rule {
     apply_server_side_encryption_by_default {
-      kms_master_key_id = aws_kms_key.logs.arn
-      sse_algorithm     = "aws:kms"
+      sse_algorithm = "AES256"
     }
   }
 }
@@ -42,13 +44,29 @@ data "template_file" "logs_bucket_enforce_tls_statement" {
   }
 }
 
+data "template_file" "logs_bucket_log_delivery_access_statement" {
+  template = file("${path.module}/policies/s3-bucket-policy-statements/log-delivery-access.json.tpl")
+
+  vars = {
+    log_bucket_arn = aws_s3_bucket.logs.arn
+    source_bucket_arns = local.cloudfront_enable_apex_to_www_redirect ? jsonencode([
+      aws_s3_bucket.frontend.arn,
+      aws_s3_bucket.frontend_www_redirect.0.arn,
+      ]) : jsonencode([
+      aws_s3_bucket.frontend.arn,
+    ])
+    account_id = local.account_id
+  }
+}
+
 data "template_file" "logs_bucket_policy" {
   template = file("${path.module}/policies/s3-bucket-policy.json.tpl")
 
   vars = {
     statement = <<EOT
 [
-${data.template_file.logs_bucket_enforce_tls_statement.rendered}
+${data.template_file.logs_bucket_enforce_tls_statement.rendered},
+${data.template_file.logs_bucket_log_delivery_access_statement.rendered}
 ]
 EOT
   }

Original file line number	Diff line number	Diff line change
`@@ -3,3 +3,5 @@ data "aws_route53_zone" "default" {`
`3`	`3`
`4`	`4`	`zone_id = local.route53_hosted_zone_options.id`
`5`	`5`	`}`
	`6`	`+`
	`7`	`+data "aws_caller_identity" "current" {}`
Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,5 @@`
`1`	`1`	`locals {`
	`2`	`+ account_id = data.aws_caller_identity.current.account_id`
`2`	`3`	`site_url = var.site_url`
`3`	`4`	`project_random_id = random_id.project.dec`
`4`	`5`	`project_name = "${replace(local.site_url, ".", "-")}-${local.project_random_id}"`
Original file line number	Diff line number	Diff line change
`@@ -23,13 +23,15 @@ resource "aws_s3_bucket_public_access_block" "logs" {`
`23`	`23`	`restrict_public_buckets = true`
`24`	`24`	`}`
`25`	`25`
	`26`	`+# If default encryption is enabled on the target bucket, AES256 (SSE-S3) must be selected as the encryption key`
	`27`	`+# https://aws.amazon.com/premiumsupport/knowledge-center/s3-server-access-log-not-delivered/`
	`28`	`+#tfsec:ignore:aws-s3-encryption-customer-key`
`26`	`29`	`resource "aws_s3_bucket_server_side_encryption_configuration" "logs" {`
`27`	`30`	`bucket = aws_s3_bucket.logs.bucket`
`28`	`31`
`29`	`32`	`rule {`
`30`	`33`	`apply_server_side_encryption_by_default {`
`31`		`- kms_master_key_id = aws_kms_key.logs.arn`
`32`		`- sse_algorithm = "aws:kms"`
	`34`	`+ sse_algorithm = "AES256"`
`33`	`35`	`}`
`34`	`36`	`}`
`35`	`37`	`}`
`@@ -42,13 +44,29 @@ data "template_file" "logs_bucket_enforce_tls_statement" {`
`42`	`44`	`}`
`43`	`45`	`}`
`44`	`46`
	`47`	`+data "template_file" "logs_bucket_log_delivery_access_statement" {`
	`48`	`+ template = file("${path.module}/policies/s3-bucket-policy-statements/log-delivery-access.json.tpl")`
	`49`	`+`
	`50`	`+ vars = {`
	`51`	`+ log_bucket_arn = aws_s3_bucket.logs.arn`
	`52`	`+ source_bucket_arns = local.cloudfront_enable_apex_to_www_redirect ? jsonencode([`
	`53`	`+ aws_s3_bucket.frontend.arn,`
	`54`	`+ aws_s3_bucket.frontend_www_redirect.0.arn,`
	`55`	`+ ]) : jsonencode([`
	`56`	`+ aws_s3_bucket.frontend.arn,`
	`57`	`+ ])`
	`58`	`+ account_id = local.account_id`
	`59`	`+ }`
	`60`	`+}`
	`61`	`+`
`45`	`62`	`data "template_file" "logs_bucket_policy" {`
`46`	`63`	`template = file("${path.module}/policies/s3-bucket-policy.json.tpl")`
`47`	`64`
`48`	`65`	`vars = {`
`49`	`66`	`statement = <<EOT`
`50`	`67`	`[`
`51`		`-${data.template_file.logs_bucket_enforce_tls_statement.rendered}`
	`68`	`+${data.template_file.logs_bucket_enforce_tls_statement.rendered},`
	`69`	`+${data.template_file.logs_bucket_log_delivery_access_statement.rendered}`
`52`	`70`	`]`
`53`	`71`	`EOT`
`54`	`72`	`}`