From 4d6a9e14f644c60f6253233675ba7756427f3ae6 Mon Sep 17 00:00:00 2001 From: githubjianli <51385385+githubjianli@users.noreply.github.com> Date: Wed, 28 Aug 2024 08:56:07 -0700 Subject: [PATCH] fix: fix deny exception policy (#272) * fix: fix deny exception policy * fix: change variable type * fix: change deny exception policy * fix: fix condition * fix: fix policy * fix: fix deny exception roles * fix: fix typo * fix: fix list * fix: update change log * fix: update varaible name --------- Co-authored-by: janli --- CHANGELOG.md | 7 ++++++ s3-other.tf | 34 ++++++++++++++++++++++--- s3.tf | 3 +-- templates/apiary-bucket-policy.json | 39 +++++++++++++++++++++++++---- variables.tf | 16 ++++++------ 5 files changed, 81 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index af58828..5595704 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,13 @@ 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). +## [7.3.2] - 2024-08-27 +### Fixed +- Fixed schema deny exception policy. +- Added `apiary_governance_iamroles` into deny exception policy. +- Added new variable `apiary_managed_service_iamroles` to handle tagging service IAM roles in deny exception policy. +- Added new variable `system_schema_producer_iamroles` to support system schema producer IAM roles. + ## [7.3.1] - 2024-08-26 ### Fixed - Fixed incorrect `s3-inventory` service account secret binding. diff --git a/s3-other.tf b/s3-other.tf index 1c79b36..fe58235 100644 --- a/s3-other.tf +++ b/s3-other.tf @@ -240,6 +240,34 @@ resource "aws_s3_bucket" "apiary_system" { "arn:aws:s3:::${local.apiary_system_bucket}/*" ] }, +%{endif} +%{if length(var.system_schema_producer_iamroles) > 0} + { + "Sid": "system schema customer account permissions", + "Effect": "Allow", + "Principal": { + "AWS": [ "${join("\",\"", var.system_schema_producer_iamroles)}" ] + }, + "Action": [ + "s3:GetBucketLocation", + "s3:GetObject", + "s3:GetObjectAcl", + "s3:GetBucketAcl", + "s3:ListBucket", + "s3:PutObject", + "s3:PutObjectAcl", + "s3:DeleteObject", + "s3:GetBucketVersioning", + "s3:PutBucketVersioning", + "s3:ReplicateObject", + "s3:ReplicateDelete", + "s3:ObjectOwnerOverrideToBucketOwner" + ], + "Resource": [ + "arn:aws:s3:::${local.apiary_system_bucket}", + "arn:aws:s3:::${local.apiary_system_bucket}/*" + ] + }, %{endif} { "Sid": "DenyUnSecureCommunications", @@ -247,9 +275,9 @@ resource "aws_s3_bucket" "apiary_system" { "Principal": {"AWS": "*"}, "Action": "s3:*", "Resource": [ - "arn:aws:s3:::${local.apiary_system_bucket}", - "arn:aws:s3:::${local.apiary_system_bucket}/*" - ], + "arn:aws:s3:::${local.apiary_system_bucket}", + "arn:aws:s3:::${local.apiary_system_bucket}/*" + ], "Condition": { "Bool": { "aws:SecureTransport": "false" diff --git a/s3.tf b/s3.tf index cb84f5f..b4d23dd 100644 --- a/s3.tf +++ b/s3.tf @@ -26,8 +26,7 @@ locals { governance_iamroles = join("\",\"", var.apiary_governance_iamroles) consumer_prefix_roles = lookup(var.apiary_consumer_prefix_iamroles, schema["schema_name"], {}) common_producer_iamroles = join("\",\"", var.apiary_common_producer_iamroles) - deny_global_write_access = lookup(schema, "deny_global_write_access", var.deny_global_write_access) - producer_roles = lookup(schema, "producer_roles", var.producer_roles) + deny_exception_iamroles = lookup(schema, "deny_exception_iamroles", "") == "" ? "" : join("\",\"", compact(concat(split(",", schema["deny_exception_iamroles"]), var.apiary_managed_service_iamroles, var.apiary_governance_iamroles))) }) } } diff --git a/templates/apiary-bucket-policy.json b/templates/apiary-bucket-policy.json index eb8e8f6..ac68909 100644 --- a/templates/apiary-bucket-policy.json +++ b/templates/apiary-bucket-policy.json @@ -85,7 +85,36 @@ %{endif} %{endfor ~} %{endif} -%{if deny_global_write_access == "true" && producer_roles != "" } +%{if deny_exception_iamroles != "" } + { + "Sid": "Allow write permissions to the exception roles", + "Effect": "Allow", + "Principal": "*", + "Action": [ + "s3:GetBucketLocation", + "s3:GetObject", + "s3:GetObjectAcl", + "s3:GetBucketAcl", + "s3:ListBucket", + "s3:PutObject", + "s3:PutObjectAcl", + "s3:DeleteObject", + "s3:GetBucketVersioning", + "s3:PutBucketVersioning", + "s3:ReplicateObject", + "s3:ReplicateDelete", + "s3:ObjectOwnerOverrideToBucketOwner" + ], + "Resource": [ + "arn:aws:s3:::${bucket_name}", + "arn:aws:s3:::${bucket_name}/*" + ], + "Condition": { + "StringLike": { + "aws:PrincipalArn": [ "${deny_exception_iamroles}" ] + } + } + }, { "Sid": "Deny write permissions to everything except the specified roles", "Effect": "Deny", @@ -97,7 +126,7 @@ "Resource": "arn:aws:s3:::${bucket_name}/*", "Condition": { "StringNotLike": { - "aws:PrincipalArn": [ "${producer_roles}" ] + "aws:PrincipalArn": [ "${deny_exception_iamroles}" ] } } }, @@ -139,7 +168,7 @@ } }, %{endif} -%{if producer_iamroles != ""} +%{if deny_exception_iamroles == "" && producer_iamroles != ""} { "Sid": "Apiary producer iamrole permissions", "Effect": "Allow", @@ -167,7 +196,7 @@ ] }, %{endif} -%{if common_producer_iamroles != ""} +%{if deny_exception_iamroles == "" && common_producer_iamroles != ""} { "Sid": "General read-write iamrole permissions", "Effect": "Allow", @@ -198,7 +227,7 @@ } }, %{endif} -%{if governance_iamroles != ""} +%{if deny_exception_iamroles == "" && governance_iamroles != ""} { "Sid": "Apiary governance iamrole permissions", "Effect": "Allow", diff --git a/variables.tf b/variables.tf index 58b10af..495981d 100644 --- a/variables.tf +++ b/variables.tf @@ -812,14 +812,14 @@ variable "tcp_keepalive_probes" { default = 2 } -variable "deny_global_write_access" { - description = "Deny all write permissions from the S3 bucket except producer_roles. See VARIABLES.md for more information." - type = bool - default = false +variable "system_schema_producer_iamroles" { + description = "AWS IAM roles allowed write access to APIARY system schema" + type = list(string) + default = [] } -variable "producer_roles" { - description = "Comma separated list of roles that are able to write into the bucket. See VARIABLES.md for more information." - type = string - default = "" +variable "apiary_managed_service_iamroles" { + description = "apiary managed service IAM Roles read-write access to managed Apiary S3 buckets." + type = list(string) + default = [] }