feat(aws): add CloudTrail bucket security

* feat(aws): improved CloudTrail bucket security settings: encryption, logging, versioning * bug(aws): properly implemented dynamic block configuration * refactor(aws): removed unnecessary 'depends_on' statement * bug(aws): prevent log bucket creation when not required * refactor(aws): disable encryption, access logging, and versioning by default * feat(aws): enable configuration for AWS KMS encryption.
lacework · Aug 10, 2020 · 4a8904f · 4a8904f
1 parent 1ca6b58
commit 4a8904f
Show file tree

Hide file tree

Showing 3 changed files with 110 additions and 7 deletions.
diff --git a/aws/README.md b/aws/README.md
@@ -139,13 +139,18 @@ resource "aws_cloudtrail" "lw_sub_account_cloudtrail" {
 |------|-------------|------|---------|:--------:|
 | bucket_force_destroy | Force destroy bucket (Required when bucket not empty) | `bool` | false | no |
 | bucket_name | Name of S3 bucket | `string` | lacework- | no |
+| bucket_enable_encryption | Set this to `true` to enable encryption on a created S3 bucket | `bool` | false | no |
+| bucket_enable_logs | Set this to `true` to enable access logging on a created S3 bucket | `bool` | false | no |
+| bucket_enable_versioning | Set this to `true` to enable access logging on a created S3 bucket | `bool` | false | no |
 | bucket_sse_algorithm | Name of the server-side encryption algorithm to use ("AES256" or "aws:kms") | `string` | AES256 | no |
+| bucket_sse_key_arn | The ARN of the KMS encryption key to be used (Required when using "aws:kms") | `string` | "" | no |
 | cloudtrail_name | Name of existing cloudtrail | `string` | "lacework-cloudtrail" | no |
 | external_id_length | Length of External ID (max 1224) | `number` | 16 | no |
 | iam_role_external_id | External ID for IAM Role | `string` | "" | no |
 | iam_role_name |  The IAM role name | `string` | "lacework_iam_role" | no |
 | lacework_account_id | The Lacework AWS account that the IAM role will grant access | `string` | 434813966438 | no |
 | lacework_integration_name | The name of the integration in Lacework. This input is available in both the config, and the cloudtrail module | `string` | TF config | no |
+| log_bucket_name | Name of the S3 bucket for access logs | `string` | "" | no |
 | prefix | The prefix that will be use at the beginning of every generated resource | `string` | lacework-ct | no |
 | sns_topic_name | SNS topic name. Can be used when generating a new resource or when using an existing resource. | `string` | "" | no |
 | sqs_queue_name | SQS queue name. Can be used when generating a new resource or when using an existing resource. | `string` | "" | no |

diff --git a/aws/modules/cloudtrail/main.tf b/aws/modules/cloudtrail/main.tf
@@ -1,7 +1,8 @@
 locals {
-	bucket_name    = length(var.bucket_name) > 0 ?    var.bucket_name :    "${var.prefix}-bucket-${random_id.uniq.hex}"
-	sns_topic_name = length(var.sns_topic_name) > 0 ? var.sns_topic_name : "${var.prefix}-sns-${random_id.uniq.hex}"
-	sqs_queue_name = length(var.sqs_queue_name) > 0 ? var.sqs_queue_name : "${var.prefix}-sqs-${random_id.uniq.hex}"
+	bucket_name     = length(var.bucket_name) > 0 ?     var.bucket_name :     "${var.prefix}-bucket-${random_id.uniq.hex}"
+	log_bucket_name = length(var.log_bucket_name) > 0 ? var.log_bucket_name : "${local.bucket_name}-access-logs"
+	sns_topic_name  = length(var.sns_topic_name) > 0 ?  var.sns_topic_name :  "${var.prefix}-sns-${random_id.uniq.hex}"
+	sqs_queue_name  = length(var.sqs_queue_name) > 0 ?  var.sqs_queue_name :  "${var.prefix}-sqs-${random_id.uniq.hex}"
 	cross_account_policy_name = (
 		length(var.cross_account_policy_name) > 0 ? var.cross_account_policy_name : "${var.prefix}-cross-acct-policy-${random_id.uniq.hex}"
 	)
@@ -20,6 +21,7 @@ resource "aws_cloudtrail" "lacework_cloudtrail" {
 	name                  = var.cloudtrail_name
 	is_multi_region_trail = true
 	s3_bucket_name        = local.bucket_name
+	kms_key_id            = var.bucket_sse_key_arn
 	sns_topic_name        = aws_sns_topic.lacework_cloudtrail_sns_topic.arn
 	depends_on            = [aws_s3_bucket.cloudtrail_bucket]
 }
@@ -30,10 +32,49 @@ resource "aws_s3_bucket" "cloudtrail_bucket" {
 	force_destroy = var.bucket_force_destroy
 	policy        = data.aws_iam_policy_document.cloudtrail_s3_policy.json
 
-	server_side_encryption_configuration {
-		rule {
-			apply_server_side_encryption_by_default {
-				sse_algorithm = var.bucket_sse_algorithm
+	versioning {
+		enabled = var.bucket_enable_versioning
+	}
+
+	dynamic "logging" {
+		for_each = var.bucket_enable_logs == true ? [1] : []
+		content {
+			target_bucket = aws_s3_bucket.cloudtrail_log_bucket[0].id
+			target_prefix = "log/"
+		}
+	}
+
+	dynamic "server_side_encryption_configuration" {
+		for_each = var.bucket_enable_encryption == true ? [1] : []
+		content {
+			rule {
+				apply_server_side_encryption_by_default {
+					kms_master_key_id = var.bucket_sse_key_arn
+					sse_algorithm = var.bucket_sse_algorithm
+				}
+			}
+		}
+	}
+}
+
+resource "aws_s3_bucket" "cloudtrail_log_bucket" {
+	count         = var.use_existing_cloudtrail ? 0 : (var.bucket_enable_logs ? 1 : 0)
+	bucket        = local.log_bucket_name
+	force_destroy = var.bucket_force_destroy
+	acl           = "log-delivery-write"
+
+	versioning {
+		enabled = var.bucket_enable_versioning
+	}
+
+	dynamic "server_side_encryption_configuration" {
+		for_each = var.bucket_enable_encryption == true ? [1] : []
+		content {
+			rule {
+				apply_server_side_encryption_by_default {
+					kms_master_key_id = var.bucket_sse_key_arn
+					sse_algorithm = var.bucket_sse_algorithm
+				}
 			}
 		}
 	}
@@ -71,6 +112,28 @@ data "aws_iam_policy_document" "cloudtrail_s3_policy" {
 			values   = ["bucket-owner-full-control"]
 		}
 	}
+
+	statement {
+		sid     = "ForceSSLOnlyAccess"
+		actions = ["s3:*"]
+		effect  = "Deny"
+
+		resources = [
+			"arn:aws:s3:::${local.bucket_name}",
+			"arn:aws:s3:::${local.bucket_name}/*",
+		]
+
+		principals {
+			type        = "AWS"
+			identifiers = ["*"]
+		}
+
+		condition {
+			test     = "Bool"
+			variable = "aws:SecureTransport"
+			values   = ["false"]
+		}
+	}
 }
 
 # we use this data source to point to the S3 ARN for the cross-account policy,
@@ -148,6 +211,15 @@ data "aws_iam_policy_document" "cross_account_policy" {
 		resources = ["${data.aws_s3_bucket.selected.arn}/*"]
 	}
 
+	dynamic "statement" {
+		for_each = var.bucket_enable_encryption == true ? (var.bucket_sse_algorithm == "aws:kms" ? [1] : []) : []
+		content {
+			sid = "DecryptLogFiles"
+			actions = ["kms:Decrypt"]
+			resources = [var.bucket_sse_key_arn]
+		}
+	}
+
 	statement { 
 		sid       = "GetAccountAlias"
 		actions   = ["iam:ListAccountAliases"]

diff --git a/aws/modules/cloudtrail/variables.tf b/aws/modules/cloudtrail/variables.tf
@@ -37,6 +37,21 @@ variable "bucket_name" {
 	default = ""
 }
 
+variable "bucket_enable_encryption" {
+	type    = bool
+	default = false
+}
+
+variable "bucket_enable_logs" {
+	type    = bool
+	default = false
+}
+
+variable "bucket_enable_versioning" {
+	type    = bool
+	default = false
+}
+
 variable "bucket_force_destroy" {
 	type    = bool
 	default = false
@@ -48,6 +63,17 @@ variable "bucket_sse_algorithm" {
 	description = "The encryption algorithm to use for S3 bucket server-side encryption"
 }
 
+variable "bucket_sse_key_arn" {
+	type        = string
+	default     = ""
+	description = "The ARN of the KMS encryption key to be used (Required when using 'aws:kms')"
+}
+
+variable "log_bucket_name" {
+	type    = string
+	default = ""
+}
+
 variable "sns_topic_name" {
 	type    = string
 	default = ""