diff --git a/pkg/teams/team_map_test_invalid_arn.json b/pkg/teams/team_map_test_invalid_arn.json index 83ea89a..c19a14b 100644 --- a/pkg/teams/team_map_test_invalid_arn.json +++ b/pkg/teams/team_map_test_invalid_arn.json @@ -9,7 +9,8 @@ }, { "environment": "prod", - "id": "account 2" + "id": "account 2", + "roleArn": "arn:aws:iam::000000000011:role/CustomRole" } ], "name": "Test Team 1" diff --git a/pkg/teams/teams.go b/pkg/teams/teams.go index 7799298..dc05be7 100644 --- a/pkg/teams/teams.go +++ b/pkg/teams/teams.go @@ -19,6 +19,14 @@ func (e *duplicateAccountIDError) Error() string { return e.message } +type invalidRoleARNError struct { + message string +} + +func (e *invalidRoleARNError) Error() string { + return e.message +} + // Teams is a struct describing the format we expect in the JSON file // describing the team mappings type Teams struct { @@ -34,9 +42,9 @@ type Team struct { // Account is a struct describing a single account for a team type Account struct { - ID string `json:"id"` - Environment string `json:"environment"` - RoleARN string `json:"roleArn"` + ID string `json:"id"` + Environment string `json:"environment"` + RoleARN string `json:"roleArn"` } // ParseTeamMap takes a path to a team mapping JSON file, reads the file, and returns a Go map of Accounts to team names @@ -106,7 +114,9 @@ func (t *Teams) accountsToTeamNames() (map[Account]string, error) { } if !arn.IsARN(account.RoleARN) { - return nil, fmt.Errorf("invalid role ARN override for account %s: %s Input must be a valid Role ARN", account.ID, account.RoleARN) + return nil, &invalidRoleARNError{ + message: fmt.Sprintf("invalid role ARN for account %s: %s Input must be a valid Role ARN", account.ID, account.RoleARN), + } } a[account] = team.Name } diff --git a/pkg/teams/teams_test.go b/pkg/teams/teams_test.go index 4a1ab46..bbb446f 100644 --- a/pkg/teams/teams_test.go +++ b/pkg/teams/teams_test.go @@ -4,7 +4,6 @@ import ( "errors" "fmt" "reflect" - "strings" "testing" ) @@ -27,7 +26,7 @@ func TestParseTeamMap(t *testing.T) { // this test checks that a duplicate account ID is caught _, err = ParseTeamMap("team_map_test_duplicate.json") - fmt.Printf("err: %v", err) + fmt.Printf("err: %v\n", err) var duplicateAccountIDError *duplicateAccountIDError if err == nil || !errors.As(err, &duplicateAccountIDError) { t.Error("ERROR: didn't get expected error for duplicate account ID", err) @@ -35,9 +34,9 @@ func TestParseTeamMap(t *testing.T) { // Test invalid ARN _, err = ParseTeamMap("team_map_test_invalid_arn.json") - if err == nil { - t.Error("ERROR: expected error for invalid ARN, but got nil") - } else if !strings.Contains(err.Error(), "invalid role ARN for account") { - t.Errorf("ERROR: unexpected error message for invalid ARN: %s", err) + fmt.Printf("err: %v\n", err) + var invalidRoleARNError *invalidRoleARNError + if err == nil || !errors.As(err, &invalidRoleARNError) { + t.Error("ERROR: didn't get expected error for invalid Role ARN", err) } } diff --git a/terraform/README.md b/terraform/README.md index 10ab5b2..c572f81 100644 --- a/terraform/README.md +++ b/terraform/README.md @@ -1,2 +1,2 @@ /collector contains the actual collector application so that we can deploy to verify customer issues. -/ecr contains the terraform that sets up the infrastrcture to deploy the image and share it to customers +/ecr contains the terraform that sets up the infrastructure to deploy the image and share it to customers diff --git a/terraform/collector/main.tf b/terraform/collector/main.tf index cdb1dc7..d846626 100644 --- a/terraform/collector/main.tf +++ b/terraform/collector/main.tf @@ -1,6 +1,6 @@ ########## Building in us-east-1 ########## provider "aws" { - region = "us-east-1" + region = "us-east-1" allowed_account_ids = ["037370603820"] default_tags { @@ -29,22 +29,47 @@ terraform { ########## Create s3 bucket for storing the collected findings ########## resource "aws_s3_bucket" "security_hub_collector" { bucket = var.security_hub_collector_results_bucket_name - #acl = "private" +} - #lifecycle_rule { - # enabled = true +resource "aws_s3_bucket_server_side_encryption_configuration" "security_hub_collector" { + bucket = aws_s3_bucket.security_hub_collector.id - # expiration { - # days = 10 - # } - #} + rule { + apply_server_side_encryption_by_default { + sse_algorithm = "AES256" + } + } +} - tags = { - Automation = "Terraform" - Environment = "dev" +resource "aws_s3_bucket_lifecycle_configuration" "security_hub_collector" { + bucket = aws_s3_bucket.security_hub_collector.id + + rule { + id = "security-hub-collector" + status = "Enabled" + + expiration { + days = 90 + } } } +resource "aws_s3_bucket_public_access_block" "security_hub_collector" { + bucket = aws_s3_bucket.security_hub_collector.id + + # Block new public ACLs and uploading public objects + block_public_acls = true + + # Retroactively remove public access granted through public ACLs + ignore_public_acls = true + + # Block new public bucket policies + block_public_policy = true + + # Retroactively block public and cross-account access if bucket has public policies + restrict_public_buckets = true +} + resource "aws_s3_bucket_policy" "security_hub_collector_bucket_policy" { bucket = aws_s3_bucket.security_hub_collector.id policy = jsonencode({ @@ -62,19 +87,19 @@ resource "aws_s3_bucket_policy" "security_hub_collector_bucket_policy" { ] }, { - Action = "s3:*" + Action = "s3:*" Condition = { - Bool = { - "aws:SecureTransport" = "false", - } + Bool = { + "aws:SecureTransport" = "false", + } } Effect = "Deny" Principal = "*" - Resource = [ - aws_s3_bucket.security_hub_collector.arn, - "${aws_s3_bucket.security_hub_collector.arn}/*", + Resource = [ + aws_s3_bucket.security_hub_collector.arn, + "${aws_s3_bucket.security_hub_collector.arn}/*", ] - Sid = "AllowSSLRequestsOnly" + Sid = "AllowSSLRequestsOnly" } ] }) @@ -87,30 +112,30 @@ resource "aws_cloudwatch_log_group" "aws-scanner-inspec" { } resource "aws_ecs_cluster" "security_hub_collector_runner" { - name = "security_hub_collector" - } + name = "security-hub-collector" +} ########## Use the securityhub collector runner module ########## module "security_hub_collector_runner" { - source = "/home/acremins/corbalt/github.com/Enterprise-CMCS/mac-fc-security-hub-collector-ecs-runner" + source = "/home/acremins/corbalt/github.com/Enterprise-CMCS/mac-fc-security-hub-collector-ecs-runner" #source = "github.com/CMSgov/security-hub-collector-ecs-runner?ref=9b76aea273ce9c27c50257c10b23ae921ab99416" TODO: Remove hardcoded path and update ref once security-hub-collector-ecs-runner PR is merged - app_name = "security-hub" - environment = "dev" - task_name = "scheduled-collector" - repo_arn = "arn:aws:ecr:us-east-1:037370603820:repository/security-hub-collector" - repo_url = "037370603820.dkr.ecr.us-east-1.amazonaws.com/security-hub-collector" - repo_tag = "d93a473" - ecs_vpc_id = var.ecs_vpc_id - ecs_subnet_ids = var.ecs_subnet_ids + app_name = "security-hub" + environment = "dev" + task_name = "scheduled-collector" + repo_arn = "arn:aws:ecr:us-east-1:037370603820:repository/security-hub-collector" + repo_url = "037370603820.dkr.ecr.us-east-1.amazonaws.com/security-hub-collector" + repo_tag = "d93a473" + ecs_vpc_id = var.ecs_vpc_id + ecs_subnet_ids = var.ecs_subnet_ids schedule_task_expression = var.schedule_task_expression logs_cloudwatch_group_arn = aws_cloudwatch_log_group.aws-scanner-inspec.arn ecs_cluster_arn = aws_ecs_cluster.security_hub_collector_runner.arn - output_path = var.output_path //optional - s3_results_bucket = var.security_hub_collector_results_bucket_name - s3_key = var.s3_key //optional - assign_public_ip = var.assign_public_ip - role_path = "/delegatedadmin/developer/" - permissions_boundary = "arn:aws:iam::037370603820:policy/cms-cloud-admin/developer-boundary-policy" - team_map = base64encode(file("${path.module}/team_map.json")) - scheduled_task_state = "ENABLED" #Set to DISABLED to stop scheduled execution + output_path = var.output_path //optional + s3_results_bucket = var.security_hub_collector_results_bucket_name + s3_key = var.s3_key //optional + assign_public_ip = var.assign_public_ip + role_path = "/delegatedadmin/developer/" + permissions_boundary = "arn:aws:iam::037370603820:policy/cms-cloud-admin/developer-boundary-policy" + team_map = base64encode(file("${path.module}/team_map.json")) + scheduled_task_state = "DISABLED" #Set to DISABLED to stop scheduled execution } diff --git a/terraform/collector/team_map.json b/terraform/collector/team_map.json deleted file mode 100644 index d22edda..0000000 --- a/terraform/collector/team_map.json +++ /dev/null @@ -1,19 +0,0 @@ -{ - "teams": [ - { - "accounts": [ - { - "id": "116229642442", - "environment": "dev", - "roleArn": "arn:aws:iam::116229642442:role/security-hub-collector" - }, - { - "id": "989324938326", - "environment": "impl", - "roleArn": "arn:aws:iam::989324938326:role/security-hub-collector" - } - ], - "name": "My Team" - } - ] -} diff --git a/terraform/collector/team_map.json.example b/terraform/collector/team_map.json.example new file mode 100644 index 0000000..50dc9cc --- /dev/null +++ b/terraform/collector/team_map.json.example @@ -0,0 +1,19 @@ +{ + "teams": [ + { + "accounts": [ + { + "id": "XXXXXXXXX", + "environment": "dev", + "roleArn": "arn:aws:iam::XXXXXXXXX:role/security-hub-collector" + }, + { + "id": "YYYYYYYYY", + "environment": "impl", + "roleArn": "arn:aws:iam::YYYYYYYYY:role/security-hub-collector" + } + ], + "name": "My Team" + } + ] +} diff --git a/terraform/collector/terraform.tfvars b/terraform/collector/terraform.tfvars index 89a32ea..d35527a 100644 --- a/terraform/collector/terraform.tfvars +++ b/terraform/collector/terraform.tfvars @@ -1,7 +1,7 @@ ecs_vpc_id = "vpc-07f4de56f6970729d" -ecs_subnet_ids = ["subnet-06bbdc0b680091dd1","subnet-02d08271e8ac413b0"] +ecs_subnet_ids = ["subnet-06bbdc0b680091dd1", "subnet-02d08271e8ac413b0"] security_hub_collector_results_bucket_name = "securityhub-collector-results-037370603820s" -schedule_task_expression = "cron(*/5 * * * ? *)" +schedule_task_expression = "cron(*/5 * * * ? *)" output_path = "" s3_key = "" assume_role = "security-hub-collector" diff --git a/terraform/collector/variables.tf b/terraform/collector/variables.tf index dbb2e0b..dcba1cd 100644 --- a/terraform/collector/variables.tf +++ b/terraform/collector/variables.tf @@ -1,9 +1,44 @@ -variable "ecs_vpc_id" {} -variable "ecs_subnet_ids" {} -variable "security_hub_collector_results_bucket_name" {} -variable "schedule_task_expression" {} -variable "output_path" {} -variable "s3_key" {} -variable "assume_role" {} -variable "aws_cloudwatch_log_group_name" {} -variable "assign_public_ip" {} +variable "ecs_vpc_id" { + description = "The ID of the VPC where the ECS tasks will run" + type = string +} + +variable "ecs_subnet_ids" { + description = "A list of subnet IDs where the ECS tasks will be placed" + type = list(string) +} + +variable "security_hub_collector_results_bucket_name" { + description = "The name of the S3 bucket where Security Hub collector results will be stored" + type = string +} + +variable "schedule_task_expression" { + description = "The schedule expression for when the ECS task should run (e.g., cron or rate expression)" + type = string +} + +variable "output_path" { + description = "The path where output files will be saved" + type = string +} + +variable "s3_key" { + description = "The S3 key (path) where files will be stored in the S3 bucket" + type = string +} + +variable "assume_role" { + description = "The ARN of the IAM role to be assumed by the ECS task" + type = string +} + +variable "aws_cloudwatch_log_group_name" { + description = "The name of the CloudWatch log group where ECS task logs will be sent" + type = string +} + +variable "assign_public_ip" { + description = "Whether to assign a public IP address to the ECS task" + type = bool +}