Merge pull request #824 from ministryofjustice/feature/refactor-guard…

…duty-bucket Refactor Guardduty bucket to use S3 module
ministryofjustice · Oct 13, 2023 · 30ad5a1 · 30ad5a1
2 parents 63527a3 + 38b859b
commit 30ad5a1
Show file tree

Hide file tree

Showing 4 changed files with 103 additions and 80 deletions.
diff --git a/modules/s3/main.tf b/modules/s3/main.tf
@@ -4,6 +4,7 @@
 #tfsec:ignore:aws-s3-enable-bucket-logging
 resource "aws_s3_bucket" "default" {
   bucket        = var.bucket_name
+  bucket_prefix = var.bucket_prefix
   force_destroy = var.force_destroy
 
   tags = var.additional_tags
@@ -65,6 +66,37 @@ resource "aws_s3_bucket_server_side_encryption_configuration" "default" {
   }
 }
 
+#############################
+# Object Lock Configuration #
+#############################
+resource "aws_s3_bucket_object_lock_configuration" "guardduty_bucket" {
+  for_each = var.object_lock_enabled ? toset(["enabled"]) : []
+  bucket   = aws_s3_bucket.default.id
+
+  # rule {
+  #   # There are two modes of retention: Governance, or Compliance
+  #   # Governance is a soft retention period, whereas Compliance is a legal hold
+  #   # that can't be bypassed and requires you to delete an AWS account in its entirety to bypass it
+  #   # See: https://docs.aws.amazon.com/AmazonS3/latest/dev/object-lock-overview.html
+  #   default_retention = var.object_lock_retention
+  # }
+
+  dynamic "rule" {
+    for_each = try(flatten([var.object_lock_retention["rule"]]), [])
+
+    content {
+      dynamic "default_retention" {
+        for_each = try([rule.value.default_retention], [])
+
+        content {
+          mode = default_retention.value.mode
+          days = default_retention.value.days
+        }
+      }
+    }
+  }
+}
+
 ###################
 # Bucket policies #
 ###################

diff --git a/modules/s3/variables.tf b/modules/s3/variables.tf
@@ -1,5 +1,11 @@
 variable "bucket_name" {
-  type = string
+  type    = string
+  default = null
+}
+
+variable "bucket_prefix" {
+  type    = string
+  default = null
 }
 
 variable "force_destroy" {
@@ -66,3 +72,13 @@ variable "object_ownership" {
   type    = string
   default = "BucketOwnerEnforced"
 }
+
+variable "object_lock_enabled" {
+  type    = bool
+  default = false
+}
+
+variable "object_lock_retention" {
+  type    = any
+  default = {}
+}
diff --git a/organisation-security/terraform/guardduty-publishing-destination.tf b/organisation-security/terraform/guardduty-publishing-destination.tf
@@ -26,7 +26,7 @@ data "aws_iam_policy_document" "guardduty_publishing_destination_s3_bucket_polic
     sid       = "Allow GuardDuty to use the getBucketLocation operation"
     effect    = "Allow"
     actions   = ["s3:GetBucketLocation"]
-    resources = [aws_s3_bucket.guardduty_bucket.arn]
+    resources = [module.guardduty_publishing_destination_s3_bucket.bucket.arn]
 
     principals {
       type        = "Service"
@@ -38,7 +38,7 @@ data "aws_iam_policy_document" "guardduty_publishing_destination_s3_bucket_polic
     sid       = "Allow GuardDuty to upload objects to the bucket"
     effect    = "Allow"
     actions   = ["s3:PutObject"]
-    resources = ["${aws_s3_bucket.guardduty_bucket.arn}/*"]
+    resources = ["${module.guardduty_publishing_destination_s3_bucket.bucket.arn}/*"]
 
     principals {
       type        = "Service"
@@ -50,7 +50,7 @@ data "aws_iam_policy_document" "guardduty_publishing_destination_s3_bucket_polic
     sid       = "Deny unencrypted object uploads"
     effect    = "Deny"
     actions   = ["s3:PutObject"]
-    resources = ["${aws_s3_bucket.guardduty_bucket.arn}/*"]
+    resources = ["${module.guardduty_publishing_destination_s3_bucket.bucket.arn}/*"]
 
     principals {
       type        = "Service"
@@ -68,7 +68,7 @@ data "aws_iam_policy_document" "guardduty_publishing_destination_s3_bucket_polic
     sid       = "Deny incorrect encryption header"
     effect    = "Deny"
     actions   = ["s3:PutObject"]
-    resources = ["${aws_s3_bucket.guardduty_bucket.arn}/*"]
+    resources = ["${module.guardduty_publishing_destination_s3_bucket.bucket.arn}/*"]
 
     principals {
       type        = "Service"
@@ -86,7 +86,7 @@ data "aws_iam_policy_document" "guardduty_publishing_destination_s3_bucket_polic
     sid       = "Deny non-HTTPS access"
     effect    = "Deny"
     actions   = ["s3:*"]
-    resources = ["${aws_s3_bucket.guardduty_bucket.arn}/*"]
+    resources = ["${module.guardduty_publishing_destination_s3_bucket.bucket.arn}/*"]
 
     principals {
       type        = "*"
@@ -101,66 +101,41 @@ data "aws_iam_policy_document" "guardduty_publishing_destination_s3_bucket_polic
   }
 }
 
-resource "aws_s3_bucket" "guardduty_bucket" {
+module "guardduty_publishing_destination_s3_bucket" {
+  source = "../../modules/s3"
 
   bucket_prefix = "moj-guardduty"
-  acl           = "private"
-
-  object_lock_configuration {
-    object_lock_enabled = "Enabled"
-    rule {
-      # There are two modes of retention: Governance, or Compliance
-      # Governance is a soft retention period, whereas Compliance is a legal hold
-      # that can't be bypassed and requires you to delete an AWS account in its entirety to bypass it
-      # See: https://docs.aws.amazon.com/AmazonS3/latest/dev/object-lock-overview.html
-      default_retention {
+
+  attach_policy = true
+  policy        = data.aws_iam_policy_document.guardduty_publishing_destination_s3_bucket_policy.json
+
+  enable_versioning   = true
+  object_lock_enabled = true
+  object_lock_retention = {
+    rule = {
+      default_retention = {
         mode = "GOVERNANCE"
         days = 60
       }
     }
   }
 
-  server_side_encryption_configuration {
-    rule {
-      apply_server_side_encryption_by_default {
+  server_side_encryption_configuration = {
+    rule = {
+      apply_server_side_encryption_by_default = {
         kms_master_key_id = aws_kms_key.guardduty.arn
         sse_algorithm     = "aws:kms"
       }
     }
   }
 
-  versioning {
-    enabled = true
-  }
-
-  tags = merge(
+  additional_tags = merge(
     local.tags_organisation_management, {
       component = "Security"
     }
   )
 }
 
-resource "aws_s3_bucket_public_access_block" "guardduty_bucket_public_access_block" {
-  bucket = aws_s3_bucket.guardduty_bucket.id
-
-  # Block public ACLs
-  block_public_acls = true
-
-  # Block public bucket policies
-  block_public_policy = true
-
-  # Ignore public ACLs
-  ignore_public_acls = true
-
-  # Restrict public bucket policies
-  restrict_public_buckets = true
-}
-
-resource "aws_s3_bucket_policy" "guardduty_bucket_policy" {
-  bucket = aws_s3_bucket.guardduty_bucket.id
-  policy = data.aws_iam_policy_document.guardduty_publishing_destination_s3_bucket_policy.json
-}
-
 #########################################
 # KMS policy                            #
 #########################################