fix: Fixed Access Logging

Author: erastus.ndi

.pre-commit-config.yaml

@@ -8,7 +8,7 @@ repos:
         files: \.(tf|tfvars)$
 
   - repo: https://github.com/bridgecrewio/checkov.git
-    rev: '3.2.483'
+    rev: '3.2.484'
     hooks:
       - id: checkov
         name: checkov-aws

aws/modules/s3/main.tf

@@ -11,7 +11,7 @@ locals {
 #checkov:skip=CKV_AWS_144:Not required for this project
 resource "aws_s3_bucket" "this" {
   bucket              = var.bucket_name
-  object_lock_enabled = var.enable_object_lock
+  object_lock_enabled = var.enable_object_lock && !var.is_logging_target # Cannot have object lock on logging destination
   tags                = local.tags
 }
 
@@ -25,9 +25,9 @@ resource "aws_s3_bucket_versioning" "this" {
   }
 }
 
-#Object Lock (if enabled)
+#Object Lock (if enabled and not a logging target)
 resource "aws_s3_bucket_object_lock_configuration" "this" {
-  count  = var.enable_object_lock ? 1 : 0
+  count  = var.enable_object_lock && !var.is_logging_target ? 1 : 0
   bucket = aws_s3_bucket.this.id
 
   rule {
@@ -67,6 +67,11 @@ resource "aws_s3_bucket_logging" "this" {
   bucket        = aws_s3_bucket.this.id
   target_bucket = var.logging_target_bucket
   target_prefix = var.logging_target_prefix != null ? var.logging_target_prefix : "${var.environment}-bucket-logs/"
+
+  depends_on = [
+    aws_s3_bucket.this,
+    aws_s3_bucket_ownership_controls.this
+  ]
 }
 
 # Server-Side Encryption
@@ -110,7 +115,7 @@ resource "aws_s3_bucket_ownership_controls" "this" {
   bucket = aws_s3_bucket.this.id
 
   rule {
-    object_ownership = var.s3_object_ownership
+    object_ownership = var.is_logging_target ? "BucketOwnerPreferred" : var.s3_object_ownership
   }
 }
 

aws/modules/s3/variables.tf

@@ -115,6 +115,12 @@ variable "s3_object_ownership" {
   default     = "BucketOwnerPreferred"
 }
 
+variable "is_logging_target" {
+  description = "Whether this bucket is used as a logging target for other S3 buckets"
+  type        = bool
+  default     = false
+}
+
 variable "additional_bucket_policy_documents" {
   description = "Optional list of JSON policy documents to merge with the module's bucket policy"
   type        = list(string)

aws/non-prod-infra/dev/s3-policy.tf

@@ -52,5 +52,37 @@ data "aws_iam_policy_document" "s3_core_audit_logs_policy" {
       variable = "s3:x-amz-acl"
       values   = ["bucket-owner-full-control"]
     }
+    condition {
+      test     = "StringEquals"
+      variable = "aws:SourceAccount"
+      values   = [data.aws_caller_identity.current.account_id]
+    }
+    condition {
+      test     = "ArnLike"
+      variable = "aws:SourceArn"
+      values = [
+        "arn:aws:s3:::${local.s3_bucket_kfd_name}",
+        "arn:aws:s3:::${local.s3_bucket_static_files_name}",
+        "arn:aws:s3:::${local.s3_bucket_submitted_form_data_name}",
+        "arn:aws:s3:::${local.s3_bucket_zip_files_name}"
+      ]
+    }
+  }
+  statement {
+    sid    = "AllowS3LoggingGetBucketAcl"
+    effect = "Allow"
+    principals {
+      type        = "Service"
+      identifiers = ["logging.s3.amazonaws.com"]
+    }
+    actions = ["s3:GetBucketAcl"]
+    resources = [
+      "arn:aws:s3:::${local.s3_bucket_core_audit_logs_name}"
+    ]
+    condition {
+      test     = "StringEquals"
+      variable = "aws:SourceAccount"
+      values   = [data.aws_caller_identity.current.account_id]
+    }
   }
 }

aws/non-prod-infra/dev/s3.tf

@@ -115,18 +115,23 @@ module "s3_core_audit_logs" {
   kms_key_id                         = data.aws_kms_alias.s3.target_key_arn
   enable_public_access_block         = true
   enable_mfa_delete                  = false
-  enable_object_lock                 = true
+  enable_object_lock                 = false
   object_lock_mode                   = "GOVERNANCE"
   object_lock_retention_days         = 30
-  enable_lifecycle_expiration        = true
+  enable_lifecycle_expiration        = false
   expiration_days                    = 30
   additional_bucket_policy_documents = [data.aws_iam_policy_document.s3_core_audit_logs_policy.json]
+  is_logging_target                  = true
   # checkov:CKV2_AWS_62: Not required for this bucket
   # checkov:CKV2_AWS_61: Not required for this bucket
   # checkov:CKV_AWS_144: Not required for this project
 }
 
-resource "aws_s3_bucket_acl" "cloudfront_acl" {
-  bucket = local.s3_bucket_core_audit_logs_name
+resource "aws_s3_bucket_acl" "core_audit_logs_acl" {
+  bucket = module.s3_core_audit_logs.bucket_id
   acl    = "log-delivery-write"
+
+  depends_on = [
+    module.s3_core_audit_logs
+  ]
 }

aws/non-prod-infra/staging/s3-policy.tf

@@ -51,5 +51,37 @@ data "aws_iam_policy_document" "s3_core_audit_logs_policy" {
       variable = "s3:x-amz-acl"
       values   = ["bucket-owner-full-control"]
     }
+    condition {
+      test     = "StringEquals"
+      variable = "aws:SourceAccount"
+      values   = [data.aws_caller_identity.current.account_id]
+    }
+    condition {
+      test     = "ArnLike"
+      variable = "aws:SourceArn"
+      values = [
+        "arn:aws:s3:::${local.s3_bucket_kfd_name}",
+        "arn:aws:s3:::${local.s3_bucket_static_files_name}",
+        "arn:aws:s3:::${local.s3_bucket_submitted_form_data_name}",
+        "arn:aws:s3:::${local.s3_bucket_zip_files_name}"
+      ]
+    }
+  }
+  statement {
+    sid    = "AllowS3LoggingGetBucketAcl"
+    effect = "Allow"
+    principals {
+      type        = "Service"
+      identifiers = ["logging.s3.amazonaws.com"]
+    }
+    actions = ["s3:GetBucketAcl"]
+    resources = [
+      "arn:aws:s3:::${local.s3_bucket_core_audit_logs_name}"
+    ]
+    condition {
+      test     = "StringEquals"
+      variable = "aws:SourceAccount"
+      values   = [data.aws_caller_identity.current.account_id]
+    }
   }
 }

aws/non-prod-infra/staging/s3.tf

@@ -115,18 +115,23 @@ module "s3_core_audit_logs" {
   kms_key_id                         = data.aws_kms_alias.s3.target_key_arn
   enable_public_access_block         = true
   enable_mfa_delete                  = false
-  enable_object_lock                 = true
+  enable_object_lock                 = false
   object_lock_mode                   = "GOVERNANCE"
   object_lock_retention_days         = 30
   enable_lifecycle_expiration        = false
   expiration_days                    = 30
   additional_bucket_policy_documents = [data.aws_iam_policy_document.s3_core_audit_logs_policy.json]
+  is_logging_target                  = true
   # checkov:CKV2_AWS_62: Not required for this bucket
   # checkov:CKV2_AWS_61: Not required for this bucket
   # checkov:CKV_AWS_144: Not required for this project
 }
 
-resource "aws_s3_bucket_acl" "cloudfront_acl" {
-  bucket = local.s3_bucket_core_audit_logs_name
+resource "aws_s3_bucket_acl" "core_audit_logs_acl" {
+  bucket = module.s3_core_audit_logs.bucket_id
   acl    = "log-delivery-write"
+
+  depends_on = [
+    module.s3_core_audit_logs
+  ]
 }

aws/prod-infra/prod/s3-policy.tf

@@ -51,5 +51,37 @@ data "aws_iam_policy_document" "s3_core_audit_logs_policy" {
       variable = "s3:x-amz-acl"
       values   = ["bucket-owner-full-control"]
     }
+    condition {
+      test     = "StringEquals"
+      variable = "aws:SourceAccount"
+      values   = [data.aws_caller_identity.current.account_id]
+    }
+    condition {
+      test     = "ArnLike"
+      variable = "aws:SourceArn"
+      values = [
+        "arn:aws:s3:::${local.s3_bucket_kfd_name}",
+        "arn:aws:s3:::${local.s3_bucket_static_files_name}",
+        "arn:aws:s3:::${local.s3_bucket_submitted_form_data_name}",
+        "arn:aws:s3:::${local.s3_bucket_zip_files_name}"
+      ]
+    }
+  }
+  statement {
+    sid    = "AllowS3LoggingGetBucketAcl"
+    effect = "Allow"
+    principals {
+      type        = "Service"
+      identifiers = ["logging.s3.amazonaws.com"]
+    }
+    actions = ["s3:GetBucketAcl"]
+    resources = [
+      "arn:aws:s3:::${local.s3_bucket_core_audit_logs_name}"
+    ]
+    condition {
+      test     = "StringEquals"
+      variable = "aws:SourceAccount"
+      values   = [data.aws_caller_identity.current.account_id]
+    }
   }
 }

aws/prod-infra/prod/s3.tf

@@ -115,18 +115,23 @@ module "s3_core_audit_logs" {
   kms_key_id                         = data.aws_kms_alias.s3.target_key_arn
   enable_public_access_block         = true
   enable_mfa_delete                  = false
-  enable_object_lock                 = true
+  enable_object_lock                 = false
   object_lock_mode                   = "GOVERNANCE"
   object_lock_retention_days         = 30
   enable_lifecycle_expiration        = false
   expiration_days                    = 30
   additional_bucket_policy_documents = [data.aws_iam_policy_document.s3_core_audit_logs_policy.json]
+  is_logging_target                  = true
   # checkov:CKV2_AWS_62: Not required for this bucket
   # checkov:CKV2_AWS_61: Not required for this bucket
   # checkov:CKV_AWS_144: Not required for this project
 }
 
-resource "aws_s3_bucket_acl" "cloudfront_acl" {
-  bucket = local.s3_bucket_core_audit_logs_name
+resource "aws_s3_bucket_acl" "core_audit_logs_acl" {
+  bucket = module.s3_core_audit_logs.bucket_id
   acl    = "log-delivery-write"
+
+  depends_on = [
+    module.s3_core_audit_logs
+  ]
 }