Merge pull request #742 from cisagov/bug/adjust_rules_bucket_access

Fix the rules publication Terraform configuration
cisagov · Dec 29, 2023 · bd817ec · bd817ec
2 parents 041a3ae + 0904ddd
commit bd817ec
Show file tree

Hide file tree

Showing 4 changed files with 62 additions and 12 deletions.
diff --git a/terraform_egress_pub/cloudfront.tf b/terraform_egress_pub/cloudfront.tf
@@ -69,10 +69,20 @@ module "security_header_lambda" {
   tags                   = { "Application" = "Egress Publish" }
 }
 
+resource "aws_cloudfront_origin_access_control" "rules_s3_distribution" {
+  description = var.distribution_oac_description
+  name        = var.distribution_oac_name
+
+  origin_access_control_origin_type = "s3"
+  signing_behavior                  = "always"
+  signing_protocol                  = "sigv4"
+}
+
 resource "aws_cloudfront_distribution" "rules_s3_distribution" {
   origin {
-    domain_name = aws_s3_bucket.rules_bucket.bucket_regional_domain_name
-    origin_id   = local.s3_origin_id
+    domain_name              = aws_s3_bucket.rules_bucket.bucket_regional_domain_name
+    origin_access_control_id = aws_cloudfront_origin_access_control.rules_s3_distribution.id
+    origin_id                = local.s3_origin_id
   }
 
   enabled             = true

diff --git a/terraform_egress_pub/s3.tf b/terraform_egress_pub/s3.tf
@@ -38,14 +38,46 @@ resource "aws_s3_bucket_ownership_controls" "rules_bucket" {
   }
 }
 
-resource "aws_s3_bucket_website_configuration" "rules_bucket" {
-  bucket = aws_s3_bucket.rules_bucket.id
+data "aws_iam_policy_document" "cloudfront_read_rules_bucket" {
+  statement {
+    actions   = ["s3:GetObject"]
+    resources = ["${aws_s3_bucket.rules_bucket.arn}/*"]
+
+    condition {
+      test     = "StringEquals"
+      variable = "AWS:SourceArn"
 
-  error_document {
-    key = "error.html"
+      values = [
+        aws_cloudfront_distribution.rules_s3_distribution.arn
+      ]
+    }
+    principals {
+      identifiers = ["cloudfront.amazonaws.com"]
+      type        = "Service"
+    }
   }
 
-  index_document {
-    suffix = "all.txt"
+  statement {
+    actions   = ["s3:ListBucket"]
+    resources = [aws_s3_bucket.rules_bucket.arn]
+
+    condition {
+      test     = "StringEquals"
+      variable = "AWS:SourceArn"
+
+      values = [
+        aws_cloudfront_distribution.rules_s3_distribution.arn
+      ]
+    }
+
+    principals {
+      identifiers = ["cloudfront.amazonaws.com"]
+      type        = "Service"
+    }
   }
 }
+
+resource "aws_s3_bucket_policy" "cloudfront_read_rules_bucket" {
+  bucket = aws_s3_bucket.rules_bucket.id
+  policy = data.aws_iam_policy_document.cloudfront_read_rules_bucket.json
+}
diff --git a/terraform_egress_pub/scripts/egress_pub.py b/terraform_egress_pub/scripts/egress_pub.py
@@ -193,10 +193,6 @@ def update_bucket(bucket_name, filename, bucket_contents):
         ContentType="text/plain",
     )
 
-    # by default new objects cannot be read by public
-    # allow public reads of this object
-    b_object.Acl().put(ACL="public-read")
-
 
 def main():
     """Get the list of IPs to publish and upload them to the S3 bucket."""

diff --git a/terraform_egress_pub/variables.tf b/terraform_egress_pub/variables.tf
@@ -28,6 +28,18 @@ variable "distribution_domain" {
   type        = string
 }
 
+variable "distribution_oac_description" {
+  default     = "Allow CloudFront to read from the rules bucket."
+  description = "The description to apply to the CloudFront Origin Access Control."
+  type        = string
+}
+
+variable "distribution_oac_name" {
+  default     = "rules-s3-distribution"
+  description = "The name for the CloudFront Origin Access Control."
+  type        = string
+}
+
 variable "root_object" {
   default     = "all.txt"
   description = "The root object to serve when no path is provided, or an error occurs."