From 4a8904fc24d6900ebb48f5032e360fa5a086014d Mon Sep 17 00:00:00 2001 From: Alan Nix <65611624+alannix-lw@users.noreply.github.com> Date: Mon, 10 Aug 2020 06:08:05 -0400 Subject: [PATCH] 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. --- aws/README.md | 5 ++ aws/modules/cloudtrail/main.tf | 86 ++++++++++++++++++++++++++--- aws/modules/cloudtrail/variables.tf | 26 +++++++++ 3 files changed, 110 insertions(+), 7 deletions(-) diff --git a/aws/README.md b/aws/README.md index 86b058f..c300a55 100644 --- 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 index 6c332b5..1024d3d 100644 --- 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 index 65c84ea..a33a01b 100644 --- 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 = ""