Feature/enforce object ownership (#211)

* enforce bucket ownership * configure BucketOwnerEnforced to disable bucket ACLs * cleanup bucket policy * update github workflow versions * configure aws required_providers Co-authored-by: Raj Poluri <[email protected]>
ExpediaGroup · Mar 2, 2022 · 1de9601 · 1de9601
1 parent 3c1a500
commit 1de9601
Show file tree

Hide file tree

Showing 6 changed files with 28 additions and 33 deletions.
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
@@ -11,9 +11,9 @@ jobs:
       uses: actions/checkout@v2
 
     - name: Setup
-      uses: hashicorp/setup-terraform@v1.2.1
+      uses: hashicorp/setup-terraform@v1.3.2
       with:
-        terraform_version: 0.12.29
+        terraform_version: 0.12.31
 
     - name: Init
       run: terraform init -backend=false

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -3,6 +3,10 @@ All notable changes to this project will be documented in this file.
 
 The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/) and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).
 
+## [6.11.5] - 2022-03-01
+### Changed
+- Disable S3 object ACLs.
+
 ## [6.11.4] - 2021-12-10
 ### Added
 - make rds_family as variable

diff --git a/s3-other.tf b/s3-other.tf
@@ -24,8 +24,7 @@ resource "aws_s3_bucket" "apiary_inventory_bucket" {
               "aws:SourceArn": "arn:aws:s3:::${local.apiary_bucket_prefix}-*"
            },
          "StringEquals": {
-             "aws:SourceAccount": "${data.aws_caller_identity.current.account_id}",
-             "s3:x-amz-acl": "bucket-owner-full-control"
+             "aws:SourceAccount": "${data.aws_caller_identity.current.account_id}"
           }
        }
     },
@@ -79,9 +78,9 @@ resource "aws_s3_bucket_public_access_block" "apiary_inventory_bucket" {
   count  = var.s3_enable_inventory == true ? 1 : 0
   bucket = aws_s3_bucket.apiary_inventory_bucket[0].bucket
 
-  block_public_acls   = true
-  block_public_policy = true
-  ignore_public_acls  = true
+  block_public_acls       = true
+  block_public_policy     = true
+  ignore_public_acls      = true
   restrict_public_buckets = true
 }
 
@@ -90,7 +89,7 @@ resource "aws_s3_bucket_ownership_controls" "apiary_inventory_bucket" {
   bucket = aws_s3_bucket.apiary_inventory_bucket[0].bucket
 
   rule {
-    object_ownership = "BucketOwnerPreferred"
+    object_ownership = "BucketOwnerEnforced"
   }
 }
 
@@ -149,9 +148,9 @@ resource "aws_s3_bucket_public_access_block" "apiary_managed_logs_bucket" {
   count  = local.enable_apiary_s3_log_management ? 1 : 0
   bucket = aws_s3_bucket.apiary_managed_logs_bucket[0].bucket
 
-  block_public_acls   = true
-  block_public_policy = true
-  ignore_public_acls  = true
+  block_public_acls       = true
+  block_public_policy     = true
+  ignore_public_acls      = true
   restrict_public_buckets = true
 }
 
@@ -210,9 +209,9 @@ resource "aws_s3_bucket_public_access_block" "apiary_access_logs_hive" {
   count  = local.enable_apiary_s3_log_hive ? 1 : 0
   bucket = aws_s3_bucket.apiary_access_logs_hive[0].bucket
 
-  block_public_acls   = true
-  block_public_policy = true
-  ignore_public_acls  = true
+  block_public_acls       = true
+  block_public_policy     = true
+  ignore_public_acls      = true
   restrict_public_buckets = true
 }
 
@@ -279,8 +278,8 @@ EOF
 resource "aws_s3_bucket_public_access_block" "apiary_system" {
   bucket = aws_s3_bucket.apiary_system.bucket
 
-  block_public_acls   = true
-  block_public_policy = true
-  ignore_public_acls  = true
+  block_public_acls       = true
+  block_public_policy     = true
+  ignore_public_acls      = true
   restrict_public_buckets = true
 }
diff --git a/s3.tf b/s3.tf
@@ -108,9 +108,9 @@ resource "aws_s3_bucket_public_access_block" "apiary_bucket" {
   }
   bucket = aws_s3_bucket.apiary_data_bucket[each.key].id
 
-  block_public_acls   = true
-  block_public_policy = true
-  ignore_public_acls  = true
+  block_public_acls       = true
+  block_public_policy     = true
+  ignore_public_acls      = true
   restrict_public_buckets = true
 }
 
@@ -121,7 +121,7 @@ resource "aws_s3_bucket_ownership_controls" "apiary_bucket" {
   bucket = aws_s3_bucket.apiary_data_bucket[each.key].id
 
   rule {
-    object_ownership = "BucketOwnerPreferred"
+    object_ownership = "BucketOwnerEnforced"
   }
 }
 

diff --git a/templates/apiary-bucket-policy.json b/templates/apiary-bucket-policy.json
@@ -116,18 +116,6 @@
                 "arn:aws:s3:::${bucket_name}/*"
             ]
         },
-        {
-            "Sid": "EnsureBucketOwnerFullControl",
-            "Effect":"Deny",
-            "Principal": {
-                "AWS": [ "${producer_iamroles}" ]
-            },
-            "Action":"s3:PutObject",
-            "Resource": "arn:aws:s3:::${bucket_name}/*",
-            "Condition": {
-                "StringNotEquals": {"s3:x-amz-acl":"bucket-owner-full-control"}
-            }
-        },
 %{endif}
 %{if governance_iamroles != ""}
         {

diff --git a/version.tf b/version.tf
@@ -11,5 +11,9 @@ terraform {
       source  = "hashicorp/kubernetes"
       version = "~> 1.0"
     }
+    aws = {
+      source  = "hashicorp/aws"
+      version = "~> 3.0"
+    }
   }
 }