From acf8cf7b11adac0b04f7dd49b5d1be1b60641895 Mon Sep 17 00:00:00 2001 From: Dusty <42273218+DustyStudy@users.noreply.github.com> Date: Fri, 18 Sep 2026 14:35:29 -0500 Subject: [PATCH] Second-pass review: fix event coverage gap, IAM scoping, and baseline hardening Correctness - auto-remediate-open-ssh-rdp (event-driven): also match ModifySecurityGroupRules, so editing an existing rule to 0.0.0.0/0 is remediated in seconds instead of waiting for the Config scan. The modify event has no resulting CIDR, so the Lambda re-checks the whole group. Lambda copies and both event patterns/READMEs updated. - claude-apps-gateway (Terraform): always take a final RDS snapshot on destroy. It was tied to deletion protection, which must be disabled before a destroy is possible, so the snapshot was skipped in exactly the case it was meant to protect. Now matches the CFN DeletionPolicy. - claude-apps-gateway: allow the GovCloud inference-profile prefix (us-gov.anthropic.*) in the task role, since the module claims GovCloud support. Least-privilege / hardening - member-baseline: Config bucket now has versioning, a noncurrent-version lifecycle rule and a TLS-only bucket policy (both the StackSet inline template and the Terraform copy); documented the one-Config-recorder conflict for accounts with Config already enabled (e.g. Control Tower). - ec2-isolation-runbook: scope ModifyInstanceAttribute to the security group being attached as well as the instance (AWS lists both). - bedrock-logging-enforcement: add the docs-recommended aws:SourceArn condition to the CloudWatch role trust policy, and a narrowly scoped iam:PassRole (that role, Bedrock only) for the Lambda. The PassRole is precautionary - AWS's docs don't state whether it's required. CI - Run Checkov on the Terraform-side member-baseline StackSet template, which neither existing Checkov step covered. Co-Authored-By: Claude Sonnet 5 --- .github/workflows/lint-and-scan.yml | 10 ++++++ README.md | 4 +-- .../event-driven/README.md | 17 +++++---- .../event-driven/template.yaml | 1 + .../lambda/remediate_open_ssh_rdp.py | 36 +++++++++++++------ .../bedrock-logging-enforcement/template.yaml | 12 +++++++ .../infrastructure/template.yaml | 3 ++ .../ec2-isolation-runbook/template.yaml | 8 +++++ .../member-baseline/README.md | 5 +++ .../member-baseline/template.yaml | 28 +++++++++++++++ .../event-driven/README.md | 16 +++++---- .../event-driven/main.tf | 4 +-- .../lambda/README.md | 4 +-- .../lambda/remediate_open_ssh_rdp.py | 36 +++++++++++++------ terraform/bedrock-logging-enforcement/main.tf | 11 ++++++ terraform/claude-apps-gateway/README.md | 2 +- terraform/claude-apps-gateway/main.tf | 21 +++++++---- terraform/claude-apps-gateway/variables.tf | 2 +- terraform/ec2-isolation-runbook/main.tf | 13 ++++++- .../member-baseline/README.md | 5 +++ .../member-baseline/baseline-template.yaml | 28 +++++++++++++++ 21 files changed, 215 insertions(+), 51 deletions(-) diff --git a/.github/workflows/lint-and-scan.yml b/.github/workflows/lint-and-scan.yml index 67b3045..b793c48 100644 --- a/.github/workflows/lint-and-scan.yml +++ b/.github/workflows/lint-and-scan.yml @@ -52,6 +52,16 @@ jobs: framework: cloudformation quiet: true + - name: Run Checkov on the Terraform member-baseline StackSet template + # Lives under terraform/ (so the directory scan above and the + # Terraform job's framework filter both skip it), but it's a + # CloudFormation template that gets deployed into every member account. + uses: bridgecrewio/checkov-action@a8664e3a0549367977f0cda990a34311835c87c0 # v12.3123.0 + with: + file: terraform/security-baseline-new-accounts/member-baseline/baseline-template.yaml + framework: cloudformation + quiet: true + python-and-policies: name: Python, policy JSON, and Lambda copy consistency runs-on: ubuntu-latest diff --git a/README.md b/README.md index 99c40dd..a53dbc5 100644 --- a/README.md +++ b/README.md @@ -103,8 +103,8 @@ or **RDP (3389)** to the entire internet (`0.0.0.0/0` / `::/0`). Ships as two complementary paths — deploy one or both: - **`event-driven/`** — EventBridge rule matching CloudTrail's - `AuthorizeSecurityGroupIngress` event, revokes the offending rule within - seconds of it being created. + `AuthorizeSecurityGroupIngress` and `ModifySecurityGroupRules` events, + revokes the offending rule within seconds of it being created or edited. - **`config-rule/`** — AWS Config managed rule (`RESTRICTED_INCOMING_TRAFFIC`) + SSM Automation remediation, re-evaluates all security groups on a schedule and catches rules that existed before deployment or slipped diff --git a/cloudformation/auto-remediate-open-ssh-rdp/event-driven/README.md b/cloudformation/auto-remediate-open-ssh-rdp/event-driven/README.md index 675a5de..c33a616 100644 --- a/cloudformation/auto-remediate-open-ssh-rdp/event-driven/README.md +++ b/cloudformation/auto-remediate-open-ssh-rdp/event-driven/README.md @@ -5,20 +5,23 @@ to `0.0.0.0/0` / `::/0`, within seconds of the rule being created. ## How it works -1. Someone (or something) calls `AuthorizeSecurityGroupIngress` and opens +1. Someone (or something) calls `AuthorizeSecurityGroupIngress` (a new + rule) or `ModifySecurityGroupRules` (an existing rule edited) and opens 22 or 3389 to the internet. 2. That management API call is automatically delivered to EventBridge's default event bus by CloudTrail — **no dedicated trail needs to be created** for this to work; management events are available on the default bus in every account. -3. An EventBridge rule matches on `eventName: AuthorizeSecurityGroupIngress` - and invokes a Lambda function. -4. The Lambda inspects exactly the rule(s) that were just added, and if - they match the risky pattern, revokes them and publishes an SNS +3. An EventBridge rule matches on those two `eventName`s and invokes a + Lambda function. +4. For a new rule, the Lambda inspects exactly the rule(s) that were just + added; for a modification (whose event only carries rule IDs, not the + resulting CIDR) it re-checks the whole group. Either way it revokes only + the rule entries that open 22/3389 to the internet and publishes an SNS notification. -Because it only acts on the rule just created, it won't touch other, -legitimate ingress rules on the same security group. +It never touches other, legitimate ingress rules on the same security +group. Also included for defense-in-depth / hygiene: a customer-managed KMS key encrypting the Lambda's log group and environment variables, a dead-letter diff --git a/cloudformation/auto-remediate-open-ssh-rdp/event-driven/template.yaml b/cloudformation/auto-remediate-open-ssh-rdp/event-driven/template.yaml index 2c6a89c..8bd25d0 100644 --- a/cloudformation/auto-remediate-open-ssh-rdp/event-driven/template.yaml +++ b/cloudformation/auto-remediate-open-ssh-rdp/event-driven/template.yaml @@ -198,6 +198,7 @@ Resources: detail: eventName: - AuthorizeSecurityGroupIngress + - ModifySecurityGroupRules State: ENABLED Targets: - Arn: !GetAtt RemediationFunction.Arn diff --git a/cloudformation/auto-remediate-open-ssh-rdp/lambda/remediate_open_ssh_rdp.py b/cloudformation/auto-remediate-open-ssh-rdp/lambda/remediate_open_ssh_rdp.py index 3eed306..f284f7e 100644 --- a/cloudformation/auto-remediate-open-ssh-rdp/lambda/remediate_open_ssh_rdp.py +++ b/cloudformation/auto-remediate-open-ssh-rdp/lambda/remediate_open_ssh_rdp.py @@ -7,7 +7,10 @@ Automation remediation paths: 1. EventBridge rule matching CloudTrail's AuthorizeSecurityGroupIngress - management event. Revokes only the specific rule(s) just added. + management event. Revokes only the specific rule(s) just added. A + ModifySecurityGroupRules event (an existing rule edited to be open) is + also handled, but its request is rule-ID based rather than describing the + resulting CIDR, so the whole group is re-scanned instead. 2. Direct invocation with {"security_group_id": "sg-xxxxxxxx"} (used by the SSM Automation document triggered from an AWS Config remediation). Describes the group and revokes any matching bad rules found on it @@ -136,6 +139,20 @@ def _extract_ip_permissions(container): return normalized +def _revoke_from_group(group_id, source): + """Describe a security group and revoke any SSH/RDP-to-the-internet + rules currently on it. Returns a small result dict.""" + resp = ec2.describe_security_groups(GroupIds=[group_id]) + groups = resp.get("SecurityGroups", []) + if not groups: + logger.warning("Security group %s not found", group_id) + return {"remediated": False, "reason": "security group not found"} + + ip_permissions = groups[0].get("IpPermissions", []) + revoked = _revoke_from_permissions(group_id, ip_permissions, source=source) + return {"remediated": bool(revoked), "revoked_rules": revoked} + + def _handle_cloudtrail_event(event): detail = event.get("detail", {}) request_params = detail.get("requestParameters", {}) or {} @@ -144,6 +161,10 @@ def _handle_cloudtrail_event(event): logger.warning("No groupId found in CloudTrail event detail, skipping") return + if detail.get("eventName") == "ModifySecurityGroupRules": + _revoke_from_group(group_id, source="cloudtrail-eventbridge-modify") + return + response_elements = detail.get("responseElements", {}) or {} ip_permissions = _extract_ip_permissions(response_elements) or _extract_ip_permissions(request_params) @@ -160,22 +181,15 @@ def _handle_direct_invocation(event): logger.warning("No security_group_id provided in direct invocation event") return {"remediated": False, "reason": "no security group id provided"} - resp = ec2.describe_security_groups(GroupIds=[group_id]) - groups = resp.get("SecurityGroups", []) - if not groups: - logger.warning("Security group %s not found", group_id) - return {"remediated": False, "reason": "security group not found"} - - ip_permissions = groups[0].get("IpPermissions", []) - revoked = _revoke_from_permissions(group_id, ip_permissions, source="config-ssm-remediation") - return {"remediated": bool(revoked), "revoked_rules": revoked} + return _revoke_from_group(group_id, source="config-ssm-remediation") def lambda_handler(event, context): logger.info("Event: %s", json.dumps(event, default=str)) + handled_events = ("AuthorizeSecurityGroupIngress", "ModifySecurityGroupRules") is_cloudtrail_event = event.get("detail-type") == "AWS API Call via CloudTrail" or ( - "detail" in event and event.get("detail", {}).get("eventName") == "AuthorizeSecurityGroupIngress" + "detail" in event and event.get("detail", {}).get("eventName") in handled_events ) if is_cloudtrail_event: diff --git a/cloudformation/bedrock-logging-enforcement/template.yaml b/cloudformation/bedrock-logging-enforcement/template.yaml index 29572ec..1725e1c 100644 --- a/cloudformation/bedrock-logging-enforcement/template.yaml +++ b/cloudformation/bedrock-logging-enforcement/template.yaml @@ -207,6 +207,8 @@ Resources: Condition: StringEquals: aws:SourceAccount: !Ref "AWS::AccountId" + ArnLike: + aws:SourceArn: !Sub "arn:${AWS::Partition}:bedrock:${AWS::Region}:${AWS::AccountId}:*" Policies: - PolicyName: BedrockToCloudWatchLogsPolicy PolicyDocument: @@ -247,6 +249,16 @@ Resources: Resource: "*" # Bedrock's account-level logging configuration APIs are # not resource-scopable - "*" is required. + - Effect: Allow + # The logging configuration hands Bedrock this role for + # CloudWatch delivery. Scoped to exactly that role and only + # to Bedrock. + Action: + - iam:PassRole + Resource: !GetAtt BedrockToCloudWatchRole.Arn + Condition: + StringEquals: + iam:PassedToService: bedrock.amazonaws.com - Effect: Allow Action: - sns:Publish diff --git a/cloudformation/claude-apps-gateway/infrastructure/template.yaml b/cloudformation/claude-apps-gateway/infrastructure/template.yaml index 2e41100..4735851 100644 --- a/cloudformation/claude-apps-gateway/infrastructure/template.yaml +++ b/cloudformation/claude-apps-gateway/infrastructure/template.yaml @@ -297,6 +297,9 @@ Resources: - bedrock:InvokeModelWithResponseStream Resource: - !Sub "arn:${AWS::Partition}:bedrock:${AWS::Region}:${AWS::AccountId}:inference-profile/us.anthropic.*" + # GovCloud's cross-region inference profiles use a + # us-gov. prefix instead; this never matches elsewhere. + - !Sub "arn:${AWS::Partition}:bedrock:${AWS::Region}:${AWS::AccountId}:inference-profile/us-gov.anthropic.*" - !Sub "arn:${AWS::Partition}:bedrock:*::foundation-model/anthropic.*" # Both ARN families are required: the built-in model # catalog resolves every Claude model to a cross-region diff --git a/cloudformation/ec2-isolation-runbook/template.yaml b/cloudformation/ec2-isolation-runbook/template.yaml index c48271f..8a640a8 100644 --- a/cloudformation/ec2-isolation-runbook/template.yaml +++ b/cloudformation/ec2-isolation-runbook/template.yaml @@ -93,8 +93,16 @@ Resources: - !Sub "arn:${AWS::Partition}:ec2:${AWS::Region}:${AWS::AccountId}:volume/*" - !Sub "arn:${AWS::Partition}:ec2:${AWS::Region}:${AWS::AccountId}:snapshot/*" - Effect: Allow + # Swapping security groups is authorized against the instance + # and the security group being attached (AWS lists both as + # resources of ModifyInstanceAttribute), so scope both. Action: - ec2:ModifyInstanceAttribute + Resource: + - !Sub "arn:${AWS::Partition}:ec2:${AWS::Region}:${AWS::AccountId}:instance/*" + - !Sub "arn:${AWS::Partition}:ec2:${AWS::Region}:${AWS::AccountId}:security-group/*" + - Effect: Allow + Action: - ec2:StopInstances Resource: !Sub "arn:${AWS::Partition}:ec2:${AWS::Region}:${AWS::AccountId}:instance/*" - Effect: Allow diff --git a/cloudformation/security-baseline-new-accounts/member-baseline/README.md b/cloudformation/security-baseline-new-accounts/member-baseline/README.md index 5aa3051..7ad03a3 100644 --- a/cloudformation/security-baseline-new-accounts/member-baseline/README.md +++ b/cloudformation/security-baseline-new-accounts/member-baseline/README.md @@ -79,3 +79,8 @@ For GovCloud: `RetainStacksOnAccountRemoval` is left `false` and the account simply moves elsewhere in the org — StackSets only manages what's currently in scope. +- **One AWS Config recorder and delivery channel per region per account.** + If a targeted account already has Config enabled (for example accounts + enrolled through AWS Control Tower, or set up by hand), creating the + baseline's recorder fails in that account and region. Exclude those OUs, + or remove the existing recorder first. diff --git a/cloudformation/security-baseline-new-accounts/member-baseline/template.yaml b/cloudformation/security-baseline-new-accounts/member-baseline/template.yaml index 202e4dc..d1adc6f 100644 --- a/cloudformation/security-baseline-new-accounts/member-baseline/template.yaml +++ b/cloudformation/security-baseline-new-accounts/member-baseline/template.yaml @@ -79,6 +79,14 @@ Resources: ConfigBucket: Type: AWS::S3::Bucket + Metadata: + checkov: + skip: + - id: CKV_AWS_18 + comment: >- + Server access logging needs a destination bucket in every + member account and region; deliberately left out of this + per-account baseline. Properties: BucketName: !Sub "aws-config-${AWS::AccountId}-${AWS::Region}" BucketEncryption: @@ -90,6 +98,16 @@ Resources: BlockPublicPolicy: true IgnorePublicAcls: true RestrictPublicBuckets: true + VersioningConfiguration: + Status: Enabled + LifecycleConfiguration: + Rules: + - Id: ExpireNoncurrentVersions + Status: Enabled + NoncurrentVersionExpiration: + NoncurrentDays: 365 + AbortIncompleteMultipartUpload: + DaysAfterInitiation: 7 ConfigBucketPolicy: Type: AWS::S3::BucketPolicy @@ -117,6 +135,16 @@ Resources: StringEquals: s3:x-amz-acl: bucket-owner-full-control aws:SourceAccount: !Ref "AWS::AccountId" + - Sid: DenyInsecureTransport + Effect: Deny + Principal: "*" + Action: s3:* + Resource: + - !GetAtt ConfigBucket.Arn + - !Sub "${ConfigBucket.Arn}/*" + Condition: + Bool: + aws:SecureTransport: "false" ConfigRole: Type: AWS::IAM::Role diff --git a/terraform/auto-remediate-open-ssh-rdp/event-driven/README.md b/terraform/auto-remediate-open-ssh-rdp/event-driven/README.md index e5480b4..421f459 100644 --- a/terraform/auto-remediate-open-ssh-rdp/event-driven/README.md +++ b/terraform/auto-remediate-open-ssh-rdp/event-driven/README.md @@ -5,14 +5,18 @@ to `0.0.0.0/0` / `::/0`, within seconds of the rule being created. ## How it works -1. Someone calls `AuthorizeSecurityGroupIngress` and opens 22 or 3389 to the - internet. +1. Someone calls `AuthorizeSecurityGroupIngress` (a new rule) or + `ModifySecurityGroupRules` (an existing rule edited) and opens 22 or 3389 + to the internet. 2. CloudTrail delivers that management event to EventBridge's default event bus automatically — no dedicated trail resource required. -3. An `aws_cloudwatch_event_rule` matches on - `eventName: AuthorizeSecurityGroupIngress` and invokes a Lambda. -4. The Lambda inspects exactly the rule(s) just added and revokes any that - match the risky pattern, then publishes an SNS notification. +3. An `aws_cloudwatch_event_rule` matches on those two `eventName`s and + invokes a Lambda. +4. For a new rule the Lambda inspects exactly the rule(s) just added; for a + modification (whose event only carries rule IDs, not the resulting CIDR) + it re-checks the whole group. Either way it revokes only the rule + entries that open 22/3389 to the internet, then publishes an SNS + notification. Pair this with the sibling `../config-rule/` module to also catch pre-existing open rules and drift on a schedule. diff --git a/terraform/auto-remediate-open-ssh-rdp/event-driven/main.tf b/terraform/auto-remediate-open-ssh-rdp/event-driven/main.tf index 6d0c91e..521ceb2 100644 --- a/terraform/auto-remediate-open-ssh-rdp/event-driven/main.tf +++ b/terraform/auto-remediate-open-ssh-rdp/event-driven/main.tf @@ -183,13 +183,13 @@ resource "aws_cloudwatch_log_group" "remediate" { resource "aws_cloudwatch_event_rule" "authorize_sg_ingress" { name = "${var.name_prefix}-authorize-sg-ingress" - description = "Matches AuthorizeSecurityGroupIngress API calls captured by CloudTrail." + description = "Matches AuthorizeSecurityGroupIngress and ModifySecurityGroupRules API calls captured by CloudTrail." event_pattern = jsonencode({ source = ["aws.ec2"] detail-type = ["AWS API Call via CloudTrail"] detail = { - eventName = ["AuthorizeSecurityGroupIngress"] + eventName = ["AuthorizeSecurityGroupIngress", "ModifySecurityGroupRules"] } }) } diff --git a/terraform/auto-remediate-open-ssh-rdp/lambda/README.md b/terraform/auto-remediate-open-ssh-rdp/lambda/README.md index 63a93cd..92d5e6f 100644 --- a/terraform/auto-remediate-open-ssh-rdp/lambda/README.md +++ b/terraform/auto-remediate-open-ssh-rdp/lambda/README.md @@ -4,8 +4,8 @@ Shared Lambda source used by both remediation paths in `auto-remediate-open-ssh-rdp/`: - **event-driven**: invoked directly by EventBridge with a CloudTrail - `AuthorizeSecurityGroupIngress` event. Revokes only the rule(s) just - added. + `AuthorizeSecurityGroupIngress` event (revokes only the rule(s) just + added) or `ModifySecurityGroupRules` event (re-checks the whole group). - **config-rule**: invoked by an SSM Automation document with `{"security_group_id": "sg-xxxxxxxx"}`. Describes the group and revokes any matching rule found. diff --git a/terraform/auto-remediate-open-ssh-rdp/lambda/remediate_open_ssh_rdp.py b/terraform/auto-remediate-open-ssh-rdp/lambda/remediate_open_ssh_rdp.py index 3eed306..f284f7e 100644 --- a/terraform/auto-remediate-open-ssh-rdp/lambda/remediate_open_ssh_rdp.py +++ b/terraform/auto-remediate-open-ssh-rdp/lambda/remediate_open_ssh_rdp.py @@ -7,7 +7,10 @@ Automation remediation paths: 1. EventBridge rule matching CloudTrail's AuthorizeSecurityGroupIngress - management event. Revokes only the specific rule(s) just added. + management event. Revokes only the specific rule(s) just added. A + ModifySecurityGroupRules event (an existing rule edited to be open) is + also handled, but its request is rule-ID based rather than describing the + resulting CIDR, so the whole group is re-scanned instead. 2. Direct invocation with {"security_group_id": "sg-xxxxxxxx"} (used by the SSM Automation document triggered from an AWS Config remediation). Describes the group and revokes any matching bad rules found on it @@ -136,6 +139,20 @@ def _extract_ip_permissions(container): return normalized +def _revoke_from_group(group_id, source): + """Describe a security group and revoke any SSH/RDP-to-the-internet + rules currently on it. Returns a small result dict.""" + resp = ec2.describe_security_groups(GroupIds=[group_id]) + groups = resp.get("SecurityGroups", []) + if not groups: + logger.warning("Security group %s not found", group_id) + return {"remediated": False, "reason": "security group not found"} + + ip_permissions = groups[0].get("IpPermissions", []) + revoked = _revoke_from_permissions(group_id, ip_permissions, source=source) + return {"remediated": bool(revoked), "revoked_rules": revoked} + + def _handle_cloudtrail_event(event): detail = event.get("detail", {}) request_params = detail.get("requestParameters", {}) or {} @@ -144,6 +161,10 @@ def _handle_cloudtrail_event(event): logger.warning("No groupId found in CloudTrail event detail, skipping") return + if detail.get("eventName") == "ModifySecurityGroupRules": + _revoke_from_group(group_id, source="cloudtrail-eventbridge-modify") + return + response_elements = detail.get("responseElements", {}) or {} ip_permissions = _extract_ip_permissions(response_elements) or _extract_ip_permissions(request_params) @@ -160,22 +181,15 @@ def _handle_direct_invocation(event): logger.warning("No security_group_id provided in direct invocation event") return {"remediated": False, "reason": "no security group id provided"} - resp = ec2.describe_security_groups(GroupIds=[group_id]) - groups = resp.get("SecurityGroups", []) - if not groups: - logger.warning("Security group %s not found", group_id) - return {"remediated": False, "reason": "security group not found"} - - ip_permissions = groups[0].get("IpPermissions", []) - revoked = _revoke_from_permissions(group_id, ip_permissions, source="config-ssm-remediation") - return {"remediated": bool(revoked), "revoked_rules": revoked} + return _revoke_from_group(group_id, source="config-ssm-remediation") def lambda_handler(event, context): logger.info("Event: %s", json.dumps(event, default=str)) + handled_events = ("AuthorizeSecurityGroupIngress", "ModifySecurityGroupRules") is_cloudtrail_event = event.get("detail-type") == "AWS API Call via CloudTrail" or ( - "detail" in event and event.get("detail", {}).get("eventName") == "AuthorizeSecurityGroupIngress" + "detail" in event and event.get("detail", {}).get("eventName") in handled_events ) if is_cloudtrail_event: diff --git a/terraform/bedrock-logging-enforcement/main.tf b/terraform/bedrock-logging-enforcement/main.tf index 0af77fc..772e16a 100644 --- a/terraform/bedrock-logging-enforcement/main.tf +++ b/terraform/bedrock-logging-enforcement/main.tf @@ -181,6 +181,7 @@ resource "aws_iam_role" "bedrock_to_cloudwatch" { Action = "sts:AssumeRole" Condition = { StringEquals = { "aws:SourceAccount" = data.aws_caller_identity.current.account_id } + ArnLike = { "aws:SourceArn" = "arn:${data.aws_partition.current.partition}:bedrock:${data.aws_region.current.region}:${data.aws_caller_identity.current.account_id}:*" } } }] }) @@ -239,6 +240,16 @@ resource "aws_iam_role_policy" "lambda_exec" { ] Resource = "*" }, + { + # The logging configuration hands Bedrock this role for CloudWatch + # delivery. Scoped to exactly that role and only to Bedrock. + Effect = "Allow" + Action = ["iam:PassRole"] + Resource = aws_iam_role.bedrock_to_cloudwatch.arn + Condition = { + StringEquals = { "iam:PassedToService" = "bedrock.amazonaws.com" } + } + }, { Effect = "Allow" Action = ["sns:Publish"] diff --git a/terraform/claude-apps-gateway/README.md b/terraform/claude-apps-gateway/README.md index 262fc4c..375fd35 100644 --- a/terraform/claude-apps-gateway/README.md +++ b/terraform/claude-apps-gateway/README.md @@ -124,7 +124,7 @@ your MDM's managed settings file — see | `desired_count` | Number of gateway tasks | `1` | | `db_instance_class` | RDS instance class | `db.t4g.micro` | | `db_allocated_storage_gb` | RDS storage in GB | `20` | -| `enable_deletion_protection` | RDS + ALB deletion protection + final RDS snapshot on destroy | `true` | +| `enable_deletion_protection` | RDS + ALB deletion protection (a final RDS snapshot is always taken on destroy) | `true` | | `enable_multi_az` | Enable RDS Multi-AZ - roughly doubles RDS cost | `false` | | `create_bedrock_vpc_endpoint` | Create the `bedrock-runtime` interface endpoint | `true` | diff --git a/terraform/claude-apps-gateway/main.tf b/terraform/claude-apps-gateway/main.tf index 32a0df2..e8f13fd 100644 --- a/terraform/claude-apps-gateway/main.tf +++ b/terraform/claude-apps-gateway/main.tf @@ -231,6 +231,9 @@ resource "aws_iam_role_policy" "bedrock_invoke" { Action = ["bedrock:InvokeModel", "bedrock:InvokeModelWithResponseStream"] Resource = [ "arn:${data.aws_partition.current.partition}:bedrock:${data.aws_region.current.region}:${data.aws_caller_identity.current.account_id}:inference-profile/us.anthropic.*", + # GovCloud's cross-region inference profiles use a us-gov. prefix + # instead; this never matches in other partitions. + "arn:${data.aws_partition.current.partition}:bedrock:${data.aws_region.current.region}:${data.aws_caller_identity.current.account_id}:inference-profile/us-gov.anthropic.*", "arn:${data.aws_partition.current.partition}:bedrock:*::foundation-model/anthropic.*", ] }] @@ -415,13 +418,17 @@ resource "aws_db_instance" "gateway" { parameter_group_name = aws_db_parameter_group.gateway.name vpc_security_group_ids = [aws_security_group.db.id] - storage_encrypted = true - publicly_accessible = false - backup_retention_period = 7 - copy_tags_to_snapshot = true - deletion_protection = var.enable_deletion_protection - skip_final_snapshot = !var.enable_deletion_protection - final_snapshot_identifier = var.enable_deletion_protection ? "${var.name_prefix}-db-final" : null + storage_encrypted = true + publicly_accessible = false + backup_retention_period = 7 + copy_tags_to_snapshot = true + deletion_protection = var.enable_deletion_protection + # Always snapshot on destroy (matches the CloudFormation flavor's + # DeletionPolicy: Snapshot). Deletion protection has to be turned off + # before a destroy is possible, so tying the snapshot to it meant the + # snapshot was skipped in exactly the case it was meant to protect. + skip_final_snapshot = false + final_snapshot_identifier = "${var.name_prefix}-db-final" auto_minor_version_upgrade = true multi_az = var.enable_multi_az diff --git a/terraform/claude-apps-gateway/variables.tf b/terraform/claude-apps-gateway/variables.tf index eb0548c..6e7107d 100644 --- a/terraform/claude-apps-gateway/variables.tf +++ b/terraform/claude-apps-gateway/variables.tf @@ -55,7 +55,7 @@ variable "db_allocated_storage_gb" { variable "enable_deletion_protection" { type = bool - description = "RDS and ALB deletion protection. Set to false only for throwaway/test deployments - when true, a final RDS snapshot is taken on destroy instead of skipped." + description = "RDS and ALB deletion protection. Set to false only when you intend to destroy the deployment (or for throwaway/test ones). A final RDS snapshot is always taken on destroy regardless of this setting." default = true } diff --git a/terraform/ec2-isolation-runbook/main.tf b/terraform/ec2-isolation-runbook/main.tf index ade2ad6..7048783 100644 --- a/terraform/ec2-isolation-runbook/main.tf +++ b/terraform/ec2-isolation-runbook/main.tf @@ -74,9 +74,20 @@ resource "aws_iam_role_policy" "automation" { "arn:${data.aws_partition.current.partition}:ec2:${data.aws_region.current.region}:${data.aws_caller_identity.current.account_id}:snapshot/*", ] }, + { + # Swapping security groups is authorized against the instance and + # the security group being attached (AWS lists both as resources + # of ModifyInstanceAttribute), so scope both. + Effect = "Allow" + Action = ["ec2:ModifyInstanceAttribute"] + Resource = [ + "arn:${data.aws_partition.current.partition}:ec2:${data.aws_region.current.region}:${data.aws_caller_identity.current.account_id}:instance/*", + "arn:${data.aws_partition.current.partition}:ec2:${data.aws_region.current.region}:${data.aws_caller_identity.current.account_id}:security-group/*", + ] + }, { Effect = "Allow" - Action = ["ec2:ModifyInstanceAttribute", "ec2:StopInstances"] + Action = ["ec2:StopInstances"] Resource = "arn:${data.aws_partition.current.partition}:ec2:${data.aws_region.current.region}:${data.aws_caller_identity.current.account_id}:instance/*" }, { diff --git a/terraform/security-baseline-new-accounts/member-baseline/README.md b/terraform/security-baseline-new-accounts/member-baseline/README.md index 9ae6fec..3d881f7 100644 --- a/terraform/security-baseline-new-accounts/member-baseline/README.md +++ b/terraform/security-baseline-new-accounts/member-baseline/README.md @@ -85,3 +85,8 @@ For GovCloud: for departed accounts. - One `aws_cloudformation_stack_set_instance` is created per region in `regions` (via `for_each`), each targeting the same OU list. +- **One AWS Config recorder and delivery channel per region per account.** + If a targeted account already has Config enabled (for example accounts + enrolled through AWS Control Tower, or set up by hand), creating the + baseline's recorder fails in that account and region. Exclude those OUs, + or remove the existing recorder first. diff --git a/terraform/security-baseline-new-accounts/member-baseline/baseline-template.yaml b/terraform/security-baseline-new-accounts/member-baseline/baseline-template.yaml index 988d32a..8bf3a2a 100644 --- a/terraform/security-baseline-new-accounts/member-baseline/baseline-template.yaml +++ b/terraform/security-baseline-new-accounts/member-baseline/baseline-template.yaml @@ -15,6 +15,14 @@ Resources: ConfigBucket: Type: AWS::S3::Bucket + Metadata: + checkov: + skip: + - id: CKV_AWS_18 + comment: >- + Server access logging needs a destination bucket in every + member account and region; deliberately left out of this + per-account baseline. Properties: BucketName: !Sub "aws-config-${AWS::AccountId}-${AWS::Region}" BucketEncryption: @@ -26,6 +34,16 @@ Resources: BlockPublicPolicy: true IgnorePublicAcls: true RestrictPublicBuckets: true + VersioningConfiguration: + Status: Enabled + LifecycleConfiguration: + Rules: + - Id: ExpireNoncurrentVersions + Status: Enabled + NoncurrentVersionExpiration: + NoncurrentDays: 365 + AbortIncompleteMultipartUpload: + DaysAfterInitiation: 7 ConfigBucketPolicy: Type: AWS::S3::BucketPolicy @@ -53,6 +71,16 @@ Resources: StringEquals: s3:x-amz-acl: bucket-owner-full-control aws:SourceAccount: !Ref "AWS::AccountId" + - Sid: DenyInsecureTransport + Effect: Deny + Principal: "*" + Action: s3:* + Resource: + - !GetAtt ConfigBucket.Arn + - !Sub "${ConfigBucket.Arn}/*" + Condition: + Bool: + aws:SecureTransport: "false" ConfigRole: Type: AWS::IAM::Role