From 7fbb9d4d9f491b1e9dcd36325e6b304d3694a857 Mon Sep 17 00:00:00 2001 From: Ronaldo Macapobre Date: Thu, 29 Aug 2024 10:11:52 -0700 Subject: [PATCH] Resolved static code analysis issues found by tfsec (#10) * - Addressed tfscan errors - Comment out s3 test bucket related code * Removed soft_fail param * Changed to tfsec-sarif-action so that errors can be seen in Security tab. * Changed codeql to v2 * Changing upload-sarif to v3 * Add github_token to pass in upload sarif file step * added sec events write permission * Add kms policy * Allow aws_kms_policy to Cloudwatch only * Add kms policy for admins and cloudwatch * Fixed ecs web td policy * Fixed incorrect reference * Removed retention days * Revert back retention days * Adjust ecs policy to separate ecr:GetAuthorizationToken * Revert Resource=* first * Add ecr repo back * Revert prevent deletion in ecr * Add ecs web td log group arn back * ignore changes from encryption_config to prevent ECR to get deleted * - Add check_changes step in deploy_infra - Rename deploy to build when build-infra wf is running * Replacing hash to tagged version as per recommendation Co-authored-by: Wade Barnes * - Trigger Web CI when PR is created targeting master branch - Allow triggering of workflow via GH UI - Added lint and test step but skips it for now * - Added comments for WEB CI - Added Lint step for API CI * Changed path of API CI * Renamed to build-web and build-api * Initial changes for building web images * web yml file cleanup * Changed working dir when building docker image * Added Dockerfile.node in Web * Removed npm-force-resolutions and add permissions to /tmp/app/dist/ * Changed permissions to /opt/app-root/src * Update permission trial and error * Changed to root user * Push image to jasper gchr and pass to deploy2dev step * Separate push to another step * Move login to GCHR before docker tag * Adding logs for debugging * Rename image name * Fixed run command to us | * Changed deprecated code to new implementation * Use $image_digest * Rename WF back to App (Vue) * - Separate building web artifacts to make it reusable - Added temp code for deploy2test - Rename deploy-web to deploy-to-aws * Tweaks to missing variable references * Pass correct parameter * - Fixes to use inputs rather than env - Pass node version * Fixed indention * Moved node setup * Revert code * Fixed path * Pass the vars variable to composite action * - Renamed jasper-web to web - Declare env above runs section * Added shell: bash * display aws_account * Removed env variable * Fixed github_image_repo error * Added id-token: write * Add Deploy to TEST and PROD * - Added Web CI Workflow - Rename to deploy-web * Revert changes to API * - Moved web Dockerfile to /docker/web. - Pass node version as an argument rather than hardcoded. * Renamed to app-vue so it can be tested on feature branch * Moved --build-arg position to the front * Add major and minor node version. * Revert code changes in devcontainer.json * Add ls to see all files * Changed source file location * Run npm install as root user * Rename back to publish-web.yml * Added api ecs related resources * Allow permissions inside td log groups * Add retention days * Changed log group to api specific --------- Co-authored-by: Ronaldo Macapobre Co-authored-by: Wade Barnes --- .github/workflows/aws-template-terraform.yml | 17 ++-- .github/workflows/build-infra.yml | 2 +- .../cloud/environments/dev/webapp.tf | 15 ++-- infrastructure/cloud/modules/container/ecr.tf | 16 +++- infrastructure/cloud/modules/container/ecs.tf | 58 ++++++++++++- .../cloud/modules/container/outputs.tf | 4 + .../cloud/modules/container/variables.tf | 11 +++ .../cloud/modules/monitoring/logs.tf | 11 ++- .../cloud/modules/monitoring/outputs.tf | 12 +++ .../cloud/modules/monitoring/variables.tf | 6 ++ .../cloud/modules/networking/alb.tf | 13 +-- .../cloud/modules/networking/securitygroup.tf | 15 +++- infrastructure/cloud/modules/security/iam.tf | 21 +++-- infrastructure/cloud/modules/security/kms.tf | 35 ++++++++ .../cloud/modules/security/outputs.tf | 8 ++ .../cloud/modules/security/variables.tf | 15 ++++ .../cloud/modules/storage/s3buckets.tf | 82 +++++++++---------- 17 files changed, 270 insertions(+), 71 deletions(-) diff --git a/.github/workflows/aws-template-terraform.yml b/.github/workflows/aws-template-terraform.yml index e9b7ca4d..13e58d72 100644 --- a/.github/workflows/aws-template-terraform.yml +++ b/.github/workflows/aws-template-terraform.yml @@ -59,16 +59,23 @@ jobs: scan: name: Scan TF Code runs-on: ubuntu-latest + permissions: + actions: read + contents: read + security-events: write steps: - name: Checkout repository uses: actions/checkout@v4 - name: tfsec - uses: aquasecurity/tfsec-action@v1.0.3 + uses: aquasecurity/tfsec-sarif-action@v0.1.4 with: + sarif_file: tfsec.sarif working_directory: ${{ inputs.CONTEXT_FOLDER }} - additional_args: "--tfvars-file=${{ inputs.CONTEXT_FOLDER }}/${{ inputs.ENVIRONMENT_NAME }}.tfvars" - soft_fail: false - github_token: ${{ secrets.GITHUB_TOKEN }} + tfsec_args: "--tfvars-file=${{ inputs.CONTEXT_FOLDER }}/${{ inputs.ENVIRONMENT_NAME }}.tfvars" + - name: Upload SARIF file + uses: github/codeql-action/upload-sarif@v3 + with: + sarif_file: tfsec.sarif needs: [check_changes] deploy_infra: @@ -81,7 +88,7 @@ jobs: TF_VAR_environment: ${{ vars.ENVIRONMENT_NAME }} TF_VAR_kms_key_name: ${{ vars.KMS_KEY_NAME }} TF_VAR_vpc_id: ${{ vars.VPC_ID }} - needs: [scan] + needs: [check_changes, scan] steps: - name: Checkout repository uses: actions/checkout@v4 diff --git a/.github/workflows/build-infra.yml b/.github/workflows/build-infra.yml index b19582fe..2eaec906 100644 --- a/.github/workflows/build-infra.yml +++ b/.github/workflows/build-infra.yml @@ -22,7 +22,7 @@ on: - prod jobs: - deploy: + build: uses: ./.github/workflows/aws-template-terraform.yml with: CONTEXT_FOLDER: "./infrastructure/cloud/environments/${{ inputs.environment || 'dev' }}" diff --git a/infrastructure/cloud/environments/dev/webapp.tf b/infrastructure/cloud/environments/dev/webapp.tf index 33bacf23..0d8d3f0d 100644 --- a/infrastructure/cloud/environments/dev/webapp.tf +++ b/infrastructure/cloud/environments/dev/webapp.tf @@ -1,8 +1,11 @@ module "security" { - source = "../../modules/security" - environment = var.environment - app_name = var.app_name - kms_key_name = var.kms_key_name + source = "../../modules/security" + environment = var.environment + app_name = var.app_name + kms_key_name = var.kms_key_name + ecs_web_td_log_group_arn = module.monitoring.ecs_web_td_log_group_arn + ecs_api_td_log_group_arn = module.monitoring.ecs_api_td_log_group_arn + ecr_repository_arn = module.container.ecr_repository_arn } module "storage" { @@ -35,12 +38,14 @@ module "container" { sg_id = module.networking.ecs_sg_id lb_tg_arn = module.networking.lb_tg_arn ecs_web_td_log_group_name = module.monitoring.ecs_web_td_log_group_name + ecs_api_td_log_group_name = module.monitoring.ecs_api_td_log_group_name + kms_key_id = module.security.kms_key_id depends_on = [module.monitoring] - } module "monitoring" { source = "../../modules/monitoring" environment = var.environment app_name = var.app_name + kms_key_arn = module.security.kms_key_arn } diff --git a/infrastructure/cloud/modules/container/ecr.tf b/infrastructure/cloud/modules/container/ecr.tf index 4f5e7cc2..e22327b7 100644 --- a/infrastructure/cloud/modules/container/ecr.tf +++ b/infrastructure/cloud/modules/container/ecr.tf @@ -1,11 +1,23 @@ resource "aws_ecr_repository" "ecr_repository" { - name = "${var.app_name}-ecr-repo-${var.environment}" - force_delete = true + name = "${var.app_name}-ecr-repo-${var.environment}" + force_delete = true + image_tag_mutability = "IMMUTABLE" image_scanning_configuration { scan_on_push = true } + encryption_configuration { + encryption_type = "KMS" + kms_key = var.kms_key_id + } + + lifecycle { + ignore_changes = [ + encryption_configuration, + ] + } + tags = { name = "${var.app_name}-ecr-repo-${var.environment}" } diff --git a/infrastructure/cloud/modules/container/ecs.tf b/infrastructure/cloud/modules/container/ecs.tf index afd53971..2e0527da 100644 --- a/infrastructure/cloud/modules/container/ecs.tf +++ b/infrastructure/cloud/modules/container/ecs.tf @@ -1,6 +1,11 @@ resource "aws_ecs_cluster" "ecs_cluster" { name = "${var.app_name}-ecs-cluster-${var.environment}" + setting { + name = "containerInsights" + value = "enabled" + } + tags = { name = "${var.app_name}-ecs-cluster-${var.environment}" } @@ -18,7 +23,7 @@ resource "aws_ecs_task_definition" "ecs_web_task_definition" { container_definitions = jsonencode([ { name = "${var.app_name}-web-container-${var.environment}" - image = "${aws_ecr_repository.ecr_repository.repository_url}:${var.app_name}-web" + image = "${aws_ecr_repository.ecr_repository.repository_url}:web" essential = true portMappings = [ { @@ -56,3 +61,54 @@ resource "aws_ecs_service" "ecs_web_service" { container_port = 8080 } } + +# API +resource "aws_ecs_task_definition" "ecs_api_task_definition" { + family = "${var.app_name}-api-task-definition-${var.environment}" + network_mode = "awsvpc" + requires_compatibilities = ["FARGATE"] + cpu = 256 + memory = 512 + execution_role_arn = var.ecs_execution_role_arn + + container_definitions = jsonencode([ + { + name = "${var.app_name}-api-container-${var.environment}" + image = "${aws_ecr_repository.ecr_repository.repository_url}:api" + essential = true + portMappings = [ + { + containerPort = 5000 + } + ] + logConfiguration = { + logDriver = "awslogs" + options = { + "awslogs-group" = var.ecs_api_td_log_group_name + "awslogs-region" = var.region + "awslogs-stream-prefix" = "ecs" + } + } + } + ]) +} + +resource "aws_ecs_service" "ecs_api_service" { + name = "${var.app_name}-ecs-api-service-${var.environment}" + cluster = aws_ecs_cluster.ecs_cluster.id + task_definition = aws_ecs_task_definition.ecs_api_task_definition.arn + launch_type = "FARGATE" + desired_count = 1 + + network_configuration { + subnets = var.subnet_ids + security_groups = [var.sg_id] + assign_public_ip = true + } + + load_balancer { + target_group_arn = var.lb_tg_arn + container_name = "${var.app_name}-api-container-${var.environment}" + container_port = 5000 + } +} diff --git a/infrastructure/cloud/modules/container/outputs.tf b/infrastructure/cloud/modules/container/outputs.tf index 3f7af474..349fdf78 100644 --- a/infrastructure/cloud/modules/container/outputs.tf +++ b/infrastructure/cloud/modules/container/outputs.tf @@ -1,3 +1,7 @@ output "ecr_url" { value = try(aws_ecr_repository.ecr_repository.repository_url, "") } + +output "ecr_repository_arn" { + value = aws_ecr_repository.ecr_repository.arn +} diff --git a/infrastructure/cloud/modules/container/variables.tf b/infrastructure/cloud/modules/container/variables.tf index 8ecce735..4d775e11 100644 --- a/infrastructure/cloud/modules/container/variables.tf +++ b/infrastructure/cloud/modules/container/variables.tf @@ -37,3 +37,14 @@ variable "ecs_web_td_log_group_name" { description = "ECS Web Task Definition Log Group Name in CloudWatch" type = string } + +variable "ecs_api_td_log_group_name" { + description = "ECS API Task Definition Log Group Name in CloudWatch" + type = string +} + +variable "kms_key_id" { + description = "The KMS Key ID" + type = string +} + diff --git a/infrastructure/cloud/modules/monitoring/logs.tf b/infrastructure/cloud/modules/monitoring/logs.tf index 4088a014..61c74f30 100644 --- a/infrastructure/cloud/modules/monitoring/logs.tf +++ b/infrastructure/cloud/modules/monitoring/logs.tf @@ -1,4 +1,13 @@ resource "aws_cloudwatch_log_group" "ecs_web_td_log_group" { - name = "${var.app_name}-ecs-web-td-log-group-${var.environment}" + name = "/aws/ecs/${var.app_name}-ecs-web-td-log-group-${var.environment}" retention_in_days = 90 + + kms_key_id = var.kms_key_arn +} + +resource "aws_cloudwatch_log_group" "ecs_api_td_log_group" { + name = "/aws/ecs/${var.app_name}-ecs-api-td-log-group-${var.environment}" + retention_in_days = 90 + + kms_key_id = var.kms_key_arn } diff --git a/infrastructure/cloud/modules/monitoring/outputs.tf b/infrastructure/cloud/modules/monitoring/outputs.tf index 9a3980de..7b277a6b 100644 --- a/infrastructure/cloud/modules/monitoring/outputs.tf +++ b/infrastructure/cloud/modules/monitoring/outputs.tf @@ -1,3 +1,15 @@ output "ecs_web_td_log_group_name" { value = aws_cloudwatch_log_group.ecs_web_td_log_group.name } + +output "ecs_web_td_log_group_arn" { + value = aws_cloudwatch_log_group.ecs_web_td_log_group.arn +} + +output "ecs_api_td_log_group_name" { + value = aws_cloudwatch_log_group.ecs_api_td_log_group.name +} + +output "ecs_api_td_log_group_arn" { + value = aws_cloudwatch_log_group.ecs_api_td_log_group.arn +} diff --git a/infrastructure/cloud/modules/monitoring/variables.tf b/infrastructure/cloud/modules/monitoring/variables.tf index 7d95abc7..797fefad 100644 --- a/infrastructure/cloud/modules/monitoring/variables.tf +++ b/infrastructure/cloud/modules/monitoring/variables.tf @@ -7,3 +7,9 @@ variable "app_name" { description = "The name of the application" type = string } + +variable "kms_key_arn" { + description = "KMS Key ARN" + type = string +} + diff --git a/infrastructure/cloud/modules/networking/alb.tf b/infrastructure/cloud/modules/networking/alb.tf index 315fe53b..8ebcdbba 100644 --- a/infrastructure/cloud/modules/networking/alb.tf +++ b/infrastructure/cloud/modules/networking/alb.tf @@ -1,10 +1,11 @@ resource "aws_lb" "lb" { - name = "${var.app_name}-lb-${var.environment}" - subnets = local.web_subnets - security_groups = [aws_security_group.sg.id] - internal = true - load_balancer_type = "application" - enable_http2 = true + name = "${var.app_name}-lb-${var.environment}" + subnets = local.web_subnets + security_groups = [aws_security_group.sg.id] + internal = true + load_balancer_type = "application" + enable_http2 = true + drop_invalid_header_fields = true tags = { Name = "${var.app_name}-lb-${var.environment}" diff --git a/infrastructure/cloud/modules/networking/securitygroup.tf b/infrastructure/cloud/modules/networking/securitygroup.tf index 87c9ea85..c53849ff 100644 --- a/infrastructure/cloud/modules/networking/securitygroup.tf +++ b/infrastructure/cloud/modules/networking/securitygroup.tf @@ -1,9 +1,11 @@ # Load Balancer Security Group resource "aws_security_group" "sg" { - name = "${var.app_name}-lb-sg-${var.environment}" - vpc_id = data.aws_vpc.vpc.id + name = "${var.app_name}-lb-sg-${var.environment}" + vpc_id = data.aws_vpc.vpc.id + description = "May change once Network Architecture has been finalized." ingress { + description = "Allow inbound HTTP traffic on port 80" from_port = 80 to_port = 80 protocol = "tcp" @@ -11,6 +13,7 @@ resource "aws_security_group" "sg" { } ingress { + description = "Accept traffic on port 8080" from_port = 8080 to_port = 8080 protocol = "tcp" @@ -18,6 +21,7 @@ resource "aws_security_group" "sg" { } egress { + description = "Unrestricted" from_port = 0 to_port = 0 protocol = "-1" @@ -32,10 +36,12 @@ resource "aws_security_group" "sg" { # ECS Security Group resource "aws_security_group" "ecs_sg" { - name = "${var.app_name}-ecs-sg-${var.environment}" - vpc_id = data.aws_vpc.vpc.id + name = "${var.app_name}-ecs-sg-${var.environment}" + vpc_id = data.aws_vpc.vpc.id + description = "May change once Network Architecture has been finalized." ingress { + description = "Accept traffic on port 8080 and from specific Security Group" from_port = 8080 to_port = 8080 protocol = "tcp" @@ -44,6 +50,7 @@ resource "aws_security_group" "ecs_sg" { } egress { + description = "Unrestricted" from_port = 0 to_port = 0 protocol = "-1" diff --git a/infrastructure/cloud/modules/security/iam.tf b/infrastructure/cloud/modules/security/iam.tf index 2c0a892a..cde09d8a 100644 --- a/infrastructure/cloud/modules/security/iam.tf +++ b/infrastructure/cloud/modules/security/iam.tf @@ -22,9 +22,15 @@ resource "aws_iam_role_policy" "ecs_execution_policy" { policy = jsonencode({ Version = "2012-10-17", Statement = [ + { + Effect = "Allow", + Action = [ + "ecr:GetAuthorizationToken" + ], + Resource = "*" + }, { Action = [ - "ecr:GetAuthorizationToken", "ecr:BatchCheckLayerAvailability", "ecr:GetDownloadUrlForLayer", "ecr:GetRepositoryPolicy", @@ -37,8 +43,10 @@ resource "aws_iam_role_policy" "ecs_execution_policy" { "ecr:ListTagsForResource", "ecr:DescribeImageScanFindings" ], - Effect = "Allow", - Resource = "*" + Effect = "Allow", + Resource = [ + var.ecr_repository_arn + ] }, { Action = [ @@ -46,8 +54,11 @@ resource "aws_iam_role_policy" "ecs_execution_policy" { "logs:PutLogEvents", "logs:CreateLogGroup" ], - Effect = "Allow", - Resource = "arn:aws:logs:*:*:*" + Effect = "Allow", + Resource = [ + "${var.ecs_web_td_log_group_arn}:*", + "${var.ecs_api_td_log_group_arn}:*" + ] } ] }) diff --git a/infrastructure/cloud/modules/security/kms.tf b/infrastructure/cloud/modules/security/kms.tf index e06d1c7f..819f4884 100644 --- a/infrastructure/cloud/modules/security/kms.tf +++ b/infrastructure/cloud/modules/security/kms.tf @@ -18,3 +18,38 @@ resource "aws_kms_alias" "kms_alias" { name = "alias/${var.kms_key_name}-${var.environment}" target_key_id = aws_kms_key.kms_key.key_id } + +resource "aws_kms_key_policy" "kms_key_policy" { + key_id = aws_kms_key.kms_key.id + + policy = jsonencode({ + Version = "2012-10-17" + Statement = [ + # Allow full access to the key for administrators + { + Sid = "EnableIAMUserPermissions" + Effect = "Allow" + Principal = { + AWS = ["arn:aws:iam::${data.aws_caller_identity.current.account_id}:root"] + } + Action = "kms:*" + Resource = "*" + }, + + # Allow CloudWatch Logs to use the key + { + Sid = "AllowCloudWatchLogsUsage" + Effect = "Allow" + Principal = { + Service = "logs.amazonaws.com" + } + Action = [ + "kms:Decrypt", + "kms:Encrypt", + "kms:GenerateDataKey" + ] + Resource = "*" + } + ] + }) +} diff --git a/infrastructure/cloud/modules/security/outputs.tf b/infrastructure/cloud/modules/security/outputs.tf index cdcab9a5..1c722071 100644 --- a/infrastructure/cloud/modules/security/outputs.tf +++ b/infrastructure/cloud/modules/security/outputs.tf @@ -6,3 +6,11 @@ output "kms_key_alias" { output "ecs_execution_role_arn" { value = aws_iam_role.ecs_execution_role.arn } + +output "kms_key_id" { + value = aws_kms_key.kms_key.key_id +} + +output "kms_key_arn" { + value = aws_kms_key.kms_key.arn +} diff --git a/infrastructure/cloud/modules/security/variables.tf b/infrastructure/cloud/modules/security/variables.tf index 5b6b2611..818c9b45 100644 --- a/infrastructure/cloud/modules/security/variables.tf +++ b/infrastructure/cloud/modules/security/variables.tf @@ -12,3 +12,18 @@ variable "environment" { description = "The AWS environment to deploy to" type = string } + +variable "ecs_web_td_log_group_arn" { + description = "The ECS Web Task Definition Log Group ARN" + type = string +} + +variable "ecs_api_td_log_group_arn" { + description = "The ECS API Task Definition Log Group ARN" + type = string +} + +variable "ecr_repository_arn" { + description = "The ECR Repository ARN" + type = string +} diff --git a/infrastructure/cloud/modules/storage/s3buckets.tf b/infrastructure/cloud/modules/storage/s3buckets.tf index cbcfb222..39438eb6 100644 --- a/infrastructure/cloud/modules/storage/s3buckets.tf +++ b/infrastructure/cloud/modules/storage/s3buckets.tf @@ -1,42 +1,42 @@ -data "aws_caller_identity" "current" {} - -data "aws_kms_alias" "encryption_key_alias" { - name = var.kms_key_name -} - -# a test s3 bucket -resource "aws_s3_bucket" "test_s3_bucket" { - bucket = "${var.test_s3_bucket_name}-${var.environment}" - - tags = { - Application = "${var.app_name}-${var.environment}" - Name = "${var.test_s3_bucket_name}-${var.environment}" - Environment = "${var.environment}" - } -} - -resource "aws_s3_bucket_server_side_encryption_configuration" "test_bucket_encryption" { - bucket = aws_s3_bucket.test_s3_bucket.id - - rule { - apply_server_side_encryption_by_default { - kms_master_key_id = data.aws_kms_alias.encryption_key_alias.target_key_id - sse_algorithm = "aws:kms" - } - } -} - -resource "aws_s3_bucket_ownership_controls" "test_bucket_ownership_controls" { - bucket = aws_s3_bucket.test_s3_bucket.id - rule { - object_ownership = "BucketOwnerPreferred" - } -} - -resource "aws_s3_bucket_acl" "test_bucket_acl" { - depends_on = [aws_s3_bucket_ownership_controls.test_bucket_ownership_controls] - - bucket = aws_s3_bucket.test_s3_bucket.id - acl = "private" -} +# data "aws_caller_identity" "current" {} + +# data "aws_kms_alias" "encryption_key_alias" { +# name = var.kms_key_name +# } + +# # a test s3 bucket +# resource "aws_s3_bucket" "test_s3_bucket" { +# bucket = "${var.test_s3_bucket_name}-${var.environment}" + +# tags = { +# Application = "${var.app_name}-${var.environment}" +# Name = "${var.test_s3_bucket_name}-${var.environment}" +# Environment = "${var.environment}" +# } +# } + +# resource "aws_s3_bucket_server_side_encryption_configuration" "test_bucket_encryption" { +# bucket = aws_s3_bucket.test_s3_bucket.id + +# rule { +# apply_server_side_encryption_by_default { +# kms_master_key_id = data.aws_kms_alias.encryption_key_alias.target_key_id +# sse_algorithm = "aws:kms" +# } +# } +# } + +# resource "aws_s3_bucket_ownership_controls" "test_bucket_ownership_controls" { +# bucket = aws_s3_bucket.test_s3_bucket.id +# rule { +# object_ownership = "BucketOwnerPreferred" +# } +# } + +# resource "aws_s3_bucket_acl" "test_bucket_acl" { +# depends_on = [aws_s3_bucket_ownership_controls.test_bucket_ownership_controls] + +# bucket = aws_s3_bucket.test_s3_bucket.id +# acl = "private" +# }