From 05e11b97199113987386dcae8021538b20e80f1b Mon Sep 17 00:00:00 2001 From: Ben Fellows Date: Mon, 3 Aug 2026 16:03:37 +1200 Subject: [PATCH] Delete the audit CFN template and the vendor sync script templates/iam.role.audit.account.cfn.yaml defaulted ParentAWSAccountId to 660228977852 -- Teem's own SaaS account -- and built the notifier's ServiceToken from AWS::Region, the child's region, rather than from a parent-region parameter. The sibling child template was fixed to take an explicit ParentDeploymentRegion; this one never was. Deployed with defaults it created a cross-account IAM role in the operator's account trusting an account they do not control, carrying ReadOnlyAccess plus securityhub:*, guardduty:*, macie:*, inspector:*, config:Put*, CloudTrail and CloudWatch Logs writes, and iam:PassRole on role/tops*. Nothing in the product referenced it. install-messaging.sh uploads only iam.role.child.account.cfn.yaml, and TOPS_CFN_TEMPLATE_URL points at that same file, so it was never on the onboarding path -- it was reachable only by a self-hosted operator browsing templates/ and picking the file whose name matched what they wanted. Deleted rather than repaired. A second, unexercised copy of the onboarding template was itself the defect: it drifted through an entire SaaS-to-self-hosted pivot without anyone noticing, because nothing ran it. Repairing it would have restored exactly that condition. templates/sync.sh goes with it. It ran aws s3api put-bucket-acl --bucket storage.teemops.com --acl public-read and synced the whole directory with --acl public-read, which is what made the broken template look canonical -- served from the vendor's domain next to the one that works. Vendor-era publishing has no meaning in a self-hosted install, where the installer uploads the template to the operator's own deployment bucket. That answers the third acceptance criterion on #102: storage.teemops.com should not be publicly readable on this project's behalf, because this project no longer publishes to it. Deleting the script does not un-publish what is already in that bucket. Objects synced there previously stay public until removed by hand. Also drops $topsType from ProcessSqsMessages, assigned and never read. It distinguished "ops" from "audit", and nothing branched on it even while the audit template existed. The installer test now guards the whole templates/ directory against a Default on ParentAWSAccountId, rather than the one file, so a future template cannot reintroduce the defect. Closes #102 Co-Authored-By: Claude Opus 5 --- .../Console/Commands/ProcessSqsMessages.php | 4 +- .../sns-topic-publish-authorization.md | 33 +- templates/iam.role.audit.account.cfn.yaml | 670 ------------------ templates/sync.sh | 5 - tests/install-messaging.test.sh | 20 +- 5 files changed, 48 insertions(+), 684 deletions(-) delete mode 100644 templates/iam.role.audit.account.cfn.yaml delete mode 100644 templates/sync.sh diff --git a/app/app/Console/Commands/ProcessSqsMessages.php b/app/app/Console/Commands/ProcessSqsMessages.php index 39e4620f..8e9317bd 100644 --- a/app/app/Console/Commands/ProcessSqsMessages.php +++ b/app/app/Console/Commands/ProcessSqsMessages.php @@ -211,7 +211,9 @@ private function processMessage(array $message, SqsClient $sqsClient, string $qu $roleArn = $resourceProperties['TopsRoleArn'] ?? null; $externalId = $resourceProperties['TopsExternalId'] ?? null; $uniqueId = $resourceProperties['TopsUniqueId'] ?? null; - $topsType = $resourceProperties['TopsType'] ?? null; + // TopsType is deliberately not read. It distinguished "ops" from "audit", and + // the audit template that sent "audit" was deleted in #102; nothing branched + // on it even while that template existed. Messages still carry the field. if (!$requestType || !$responseUrl || !$stackId || !$requestId) { $this->warn('Message missing required CloudFormation fields, deleting from queue'); diff --git a/docs/features/sns-topic-publish-authorization.md b/docs/features/sns-topic-publish-authorization.md index 012a75c0..a8a268fa 100644 --- a/docs/features/sns-topic-publish-authorization.md +++ b/docs/features/sns-topic-publish-authorization.md @@ -280,6 +280,9 @@ worthwhile reduction. It is not "the topic is now private". 4. **Both child templates need the parameter**, not just one: `iam.role.child.account.cfn.yaml` and `iam.role.audit.account.cfn.yaml`. + *(Resolved differently in the end: the audit template was deleted under + [#102](https://github.com/teemops/tops/issues/102) rather than maintained. There is now one + child template.)* 5. **Verify payload filtering works against a real CloudFormation message** before this is relied on (see the warning under [Research findings](#research-findings)). @@ -392,12 +395,30 @@ filter existed. Noticed while researching; not part of this work, but they touch the same files. -- **`iam.role.audit.account.cfn.yaml` hardcodes a parent account** — [#102](https://github.com/teemops/tops/issues/102). - `ParentAWSAccountId` defaults to `660228977852` (Teem's SaaS account) and the notifier's - `ServiceToken` is built from `AWS::Region` rather than a parent-region parameter — unlike - the child template, which was fixed to take `ParentDeploymentRegion`. As shipped, a - self-hosted user deploying the audit template with defaults points their stack at someone - else's account. Worth its own issue. +- ~~**`iam.role.audit.account.cfn.yaml` hardcodes a parent account**~~ — [#102](https://github.com/teemops/tops/issues/102), + **resolved 2026-08-03 by deleting the template.** `ParentAWSAccountId` defaulted to + `660228977852` (Teem's SaaS account) and the notifier's `ServiceToken` was built from + `AWS::Region` rather than a parent-region parameter — unlike the child template, which was + fixed to take `ParentDeploymentRegion`. A self-hosted operator deploying it with defaults + created a cross-account IAM role in their own account trusting an account they did not + control, carrying `ReadOnlyAccess` plus `securityhub:*`, `guardduty:*`, `macie:*`, + `inspector:*`, CloudTrail and CloudWatch writes, and `iam:PassRole` on `role/tops*`. + + Nothing in the product referenced it — the installer uploads only + `iam.role.child.account.cfn.yaml`, and `TOPS_CFN_TEMPLATE_URL` points at that same file — so + it was deleted rather than repaired. Keeping a second, unexercised copy of the onboarding + template was itself the defect: it drifted for a whole SaaS-to-self-hosted pivot without + anyone noticing, because nothing ran it. + + `templates/sync.sh` went with it. It ran + `aws s3api put-bucket-acl --bucket storage.teemops.com --acl public-read` and synced the + whole directory with `--acl public-read` — vendor-era publishing that has no meaning in a + self-hosted install, where the installer uploads the template to the operator's *own* + deployment bucket. That answers the third acceptance criterion on #102: `storage.teemops.com` + should not be publicly readable on this project's behalf, because this project no longer + publishes anything to it. **Deleting the script does not un-publish what is already in that + bucket** — objects previously synced there stay public until removed by hand, which only the + bucket's owner can do. - **`TopsMainSQSPolicy` grants `SQS:ReceiveMessage` to `Principal: "*"`.** Harmless in practice — the `aws:SourceArn` condition can never match a direct caller, so it is dead permission — but it reads alarmingly in a review. Drop `ReceiveMessage` from the statement. diff --git a/templates/iam.role.audit.account.cfn.yaml b/templates/iam.role.audit.account.cfn.yaml deleted file mode 100644 index 9ede72a7..00000000 --- a/templates/iam.role.audit.account.cfn.yaml +++ /dev/null @@ -1,670 +0,0 @@ -## Template for IAM Role for Cloud Auditing -AWSTemplateFormatVersion: "2010-09-09" -Description: "IAM role for an account to be accessed by Audit AWS Auditing Account" -Parameters: - # Parent AWS Account - ParentAWSAccountId: - Type: String - Description: "The AWS Account ID of tops cloud service" - AllowedPattern: "[0-9]{12}" - MinLength: 12 - MaxLength: 12 - Default: "660228977852" - ExternalId: - Type: String - Description: "External STS ID DO NOT CHANGE" - AllowedPattern: "[a-zA-Z0-9-]*" - MinLength: 10 - MaxLength: 200 - UniqueId: - Type: String - Description: "Unique GUID DO NOT CHANGE" - AllowedPattern: "[a-zA-Z0-9-]*" - MinLength: 10 - MaxLength: 200 - TopsInstallId: - Type: String - NoEcho: true - Description: >- - Identifies the TOPS installation you are connecting to DO NOT CHANGE. The - parent SNS subscription only accepts pings carrying this value, so a wrong or - missing one means this stack will sit waiting and then roll back. - AllowedPattern: "[a-zA-Z0-9-]*" - MinLength: 10 - MaxLength: 200 -Resources: - TopsCustomNotifier: - Type: Custom::TeemopsPingSNS - Version: 1.1 - Properties: - ServiceToken: !Join - - "" - - - "arn:aws:sns:" - - !Ref "AWS::Region" - - ":" - - !Ref ParentAWSAccountId - - ":teemops-sns" - TopsRoleArn: !GetAtt - - TopsAuditSetup - - Arn - TopsExternalId: !Ref ExternalId - TopsUniqueId: !Ref UniqueId - TopsType: "audit" - TopsVersion: "1.1" - # Lands in the custom-resource message body, where the parent topic's - # subscription filter matches on it (N-11). - TopsInstallId: !Ref TopsInstallId - TopsCWEventRole: - Type: "AWS::IAM::Role" - Properties: - AssumeRolePolicyDocument: - Statement: - - Effect: Allow - Principal: - Service: - - "events.amazonaws.com" - Action: - - "sts:AssumeRole" - Policies: - - PolicyDocument: - Statement: - - Action: - - "Events:PutEvents" - Effect: Allow - Resource: - Fn::Join: - - "" - - - "arn:aws:events:*:" - - !Ref ParentAWSAccountId - - ":event-bus/default" - PolicyName: CWPutEventsTeemOpsPolicy - # Auditing Read-Only - TopsAuditSetup: - Type: "AWS::IAM::Role" - Properties: - ManagedPolicyArns: - - "arn:aws:iam::aws:policy/ReadOnlyAccess" - - "arn:aws:iam::aws:policy/ResourceGroupsandTagEditorReadOnlyAccess" - AssumeRolePolicyDocument: - Version: "2012-10-17" - Statement: - - Effect: "Allow" - Principal: - AWS: !Ref ParentAWSAccountId - Action: - - "sts:AssumeRole" - Condition: - Bool: - aws:MultiFactorAuthPresent: true - - Action: - - "sts:AssumeRole" - Effect: Allow - Principal: - AWS: !Ref ParentAWSAccountId - Condition: - StringEquals: - "sts:ExternalId": !Ref ExternalId - #allow ability to passrole to another role - PassRole: - Type: "AWS::IAM::Policy" - Properties: - PolicyName: "TopsPassRole" - PolicyDocument: - Version: "2012-10-17" - Statement: - - Effect: "Allow" - Action: - - "iam:PassRole" - Resource: "arn:aws:iam::*:role/tops*" - Roles: - - !Ref TopsAuditSetup - #allow ability to create an S3 bucket for Config delivery - S3Bucket: - Type: "AWS::IAM::Policy" - Properties: - PolicyName: "TopsS3Bucket" - PolicyDocument: - Version: "2012-10-17" - Statement: - - Effect: "Allow" - Action: - - "s3:CreateBucket" - - "s3:DeleteBucket" - - "s3:ListAllMyBuckets" - - "s3:ListBucket" - - "s3:GetBucketLocation" - Resource: "arn:aws:s3:::tops-config-*" - Roles: - - !Ref TopsAuditSetup - #allow ability to create a KMS key - KMSKey: - Type: "AWS::IAM::Policy" - Properties: - PolicyName: "TopsKMSKey" - PolicyDocument: - Version: "2012-10-17" - Statement: - - Effect: "Allow" - Action: - - "kms:CreateKey" - - "kms:DeleteKey" - - "kms:DescribeKey" - - "kms:EnableKey" - - "kms:DisableKey" - - "kms:ScheduleKeyDeletion" - - "kms:CancelKeyDeletion" - - "kms:PutKeyPolicy" - - "kms:TagResource" - - "kms:UntagResource" - - "kms:CreateAlias" - Resource: "arn:aws:kms:*:*:key/*" - Roles: - - !Ref TopsAuditSetup - #allow ability to create, update and delete cloudformation stacks - CloudFormationAccess: - Type: "AWS::IAM::Policy" - Properties: - PolicyName: "TopsCloudFormationAccess" - PolicyDocument: - Version: "2012-10-17" - Statement: - - Effect: "Allow" - Action: - - "cloudformation:CreateStack" - - "cloudformation:DeleteStack" - - "cloudformation:DescribeStacks" - - "cloudformation:UpdateStack" - - "cloudformation:CreateChangeSet" - - "cloudformation:DeleteChangeSet" - - "cloudformation:DescribeChangeSet" - - "cloudformation:ExecuteChangeSet" - - "cloudformation:ListChangeSets" - - "cloudformation:ListStacks" - - "cloudformation:ListStackResources" - - "cloudformation:GetTemplate" - Resource: "arn:aws:cloudformation:*:*:stack/tops*" - Roles: - - !Ref TopsAuditSetup - CloudFormationListAccess: - Type: "AWS::IAM::Policy" - Properties: - PolicyName: "TopsCloudFormationListAccess" - PolicyDocument: - Version: "2012-10-17" - Statement: - - Effect: "Allow" - Action: - - "cloudformation:ListStacks" - - "cloudformation:ListStackResources" - - "cloudformation:DescribeStacks" - Resource: "*" - Roles: - - !Ref TopsAuditSetup - #allow ability to create a CloudTrail trail - CloudTrailTrail: - Type: "AWS::IAM::Policy" - Properties: - PolicyName: "TopsCloudTrailTrail" - PolicyDocument: - Version: "2012-10-17" - Statement: - - Effect: "Allow" - Action: - - "cloudtrail:CreateTrail" - - "cloudtrail:DeleteTrail" - - "cloudtrail:DescribeTrails" - - "cloudtrail:UpdateTrail" - - "cloudtrail:StartLogging" - - "cloudtrail:StopLogging" - - "cloudtrail:GetTrailStatus" - Resource: "*" - Roles: - - !Ref TopsAuditSetup - #allow ability to create a CloudWatch log group - CloudWatchLogGroup: - Type: "AWS::IAM::Policy" - Properties: - PolicyName: "TopsCloudWatchLogGroup" - PolicyDocument: - Version: "2012-10-17" - Statement: - - Effect: "Allow" - Action: - - "logs:CreateLogGroup" - - "logs:DeleteLogGroup" - - "logs:DescribeLogGroups" - Resource: "*" - Roles: - - !Ref TopsAuditSetup - #allow ability to create a CloudWatch log stream - CloudWatchLogStream: - Type: "AWS::IAM::Policy" - Properties: - PolicyName: "TopsCloudWatchLogStream" - PolicyDocument: - Version: "2012-10-17" - Statement: - - Effect: "Allow" - Action: - - "logs:CreateLogStream" - - "logs:DeleteLogStream" - - "logs:DescribeLogStreams" - Resource: "*" - Roles: - - !Ref TopsAuditSetup - #allow ability to create a CloudWatch log event - CloudWatchLogEvent: - Type: "AWS::IAM::Policy" - Properties: - PolicyName: "TopsCloudWatchLogEvent" - PolicyDocument: - Version: "2012-10-17" - Statement: - - Effect: "Allow" - Action: - - "logs:PutLogEvents" - Resource: "*" - Roles: - - !Ref TopsAuditSetup - #allow ability to create a CloudWatch log metric filter - CloudWatchLogMetricFilter: - Type: "AWS::IAM::Policy" - Properties: - PolicyName: "TopsCloudWatchLogMetricFilter" - PolicyDocument: - Version: "2012-10-17" - Statement: - - Effect: "Allow" - Action: - - "logs:PutMetricFilter" - - "logs:DeleteMetricFilter" - - "logs:DescribeMetricFilters" - Resource: "*" - Roles: - - !Ref TopsAuditSetup - #allow ability to create config rules - ConfigRule: - Type: "AWS::IAM::Policy" - Properties: - PolicyName: "TopsConfigRule" - PolicyDocument: - Version: "2012-10-17" - Statement: - - Effect: "Allow" - Action: - - "config:PutConfigRule" - - "config:DeleteConfigRule" - - "config:DescribeConfigRules" - Resource: "*" - Roles: - - !Ref TopsAuditSetup - #allow ability to create config configuration recorders - ConfigConfigurationRecorder: - Type: "AWS::IAM::Policy" - Properties: - PolicyName: "TopsConfigConfigurationRecorder" - PolicyDocument: - Version: "2012-10-17" - Statement: - - Effect: "Allow" - Action: - - "config:PutConfigurationRecorder" - - "config:DeleteConfigurationRecorder" - - "config:DescribeConfigurationRecorders" - Resource: "*" - Roles: - - !Ref TopsAuditSetup - #allow ability to create config delivery channels - ConfigDeliveryChannel: - Type: "AWS::IAM::Policy" - Properties: - PolicyName: "TopsConfigDeliveryChannel" - PolicyDocument: - Version: "2012-10-17" - Statement: - - Effect: "Allow" - Action: - - "config:PutDeliveryChannel" - - "config:DeleteDeliveryChannel" - - "config:DescribeDeliveryChannels" - Resource: "*" - Roles: - - !Ref TopsAuditSetup - #allow ability to create config configuration aggregators - ConfigConfigurationAggregator: - Type: "AWS::IAM::Policy" - Properties: - PolicyName: "TopsConfigConfigurationAggregator" - PolicyDocument: - Version: "2012-10-17" - Statement: - - Effect: "Allow" - Action: - - "config:PutConfigurationAggregator" - - "config:DeleteConfigurationAggregator" - - "config:DescribeConfigurationAggregators" - Resource: "*" - Roles: - - !Ref TopsAuditSetup - #allow ability to create config configuration snapshots - ConfigConfigurationSnapshot: - Type: "AWS::IAM::Policy" - Properties: - PolicyName: "TopsConfigConfigurationSnapshot" - PolicyDocument: - Version: "2012-10-17" - Statement: - - Effect: "Allow" - Action: - - "config:StartConfigurationRecorder" - - "config:StopConfigurationRecorder" - - "config:DescribeConfigurationRecorderStatus" - Resource: "*" - Roles: - - !Ref TopsAuditSetup - #allow ability to create config conformance packs - ConfigConformancePack: - Type: "AWS::IAM::Policy" - Properties: - PolicyName: "TopsConfigConformancePack" - PolicyDocument: - Version: "2012-10-17" - Statement: - - Effect: "Allow" - Action: - - "config:PutConformancePack" - - "config:DeleteConformancePack" - - "config:DescribeConformancePacks" - Resource: "*" - Roles: - - !Ref TopsAuditSetup - #allow ability to create config remediation configurations - ConfigRemediationConfiguration: - Type: "AWS::IAM::Policy" - Properties: - PolicyName: "TopsConfigRemediationConfiguration" - PolicyDocument: - Version: "2012-10-17" - Statement: - - Effect: "Allow" - Action: - - "config:PutRemediationConfigurations" - - "config:DeleteRemediationConfigurations" - - "config:DescribeRemediationConfigurations" - Resource: "*" - Roles: - - !Ref TopsAuditSetup - #allow ability to create config remediation exceptions - ConfigRemediationExceptions: - Type: "AWS::IAM::Policy" - Properties: - PolicyName: "TopsConfigRemediationExceptions" - PolicyDocument: - Version: "2012-10-17" - Statement: - - Effect: "Allow" - Action: - - "config:PutRemediationExceptions" - - "config:DeleteRemediationExceptions" - - "config:DescribeRemediationExceptions" - Resource: "*" - Roles: - - !Ref TopsAuditSetup - #allow ability to manage security hub - SecurityHub: - Type: "AWS::IAM::Policy" - Properties: - PolicyName: "TopsSecurityHub" - PolicyDocument: - Version: "2012-10-17" - Statement: - - Effect: "Allow" - Action: - - "securityhub:*" - Resource: "*" - Roles: - - !Ref TopsAuditSetup - #allow ability to manage guard duty - GuardDuty: - Type: "AWS::IAM::Policy" - Properties: - PolicyName: "TopsGuardDuty" - PolicyDocument: - Version: "2012-10-17" - Statement: - - Effect: "Allow" - Action: - - "guardduty:*" - Resource: "*" - Roles: - - !Ref TopsAuditSetup - #allow ability to manage macie - Macie: - Type: "AWS::IAM::Policy" - Properties: - PolicyName: "TopsMacie" - PolicyDocument: - Version: "2012-10-17" - Statement: - - Effect: "Allow" - Action: - - "macie:*" - Resource: "*" - Roles: - - !Ref TopsAuditSetup - #allow ability to manage inspector - Inspector: - Type: "AWS::IAM::Policy" - Properties: - PolicyName: "TopsInspector" - PolicyDocument: - Version: "2012-10-17" - Statement: - - Effect: "Allow" - Action: - - "inspector:*" - Resource: "*" - Roles: - - !Ref TopsAuditSetup - #create service linkedrole for config - ConfigServiceLinkedRole: - Type: "AWS::IAM::Role" - Properties: - RoleName: "tops-config-role" - Path: "/" - AssumeRolePolicyDocument: - Version: "2012-10-17" - Statement: - - Effect: "Allow" - Principal: - Service: - - "config.amazonaws.com" - Action: - - "sts:AssumeRole" - ManagedPolicyArns: - - "arn:aws:iam::aws:policy/service-role/AWS_ConfigRole" - ConfigServiceLinkedRolePolicy: - Type: "AWS::IAM::Policy" - Properties: - PolicyName: "TopsConfigServiceLinkedRolePolicy" - PolicyDocument: - Version: "2012-10-17" - Statement: - # - Effect: "Allow" - # Action: - # - "kms:Decrypt" - # - "kms:GenerateDataKey" - # Resource: !Ref s3configKey - - Effect: Allow - Action: - - "s3:*" - Resource: !Join - - "" - - - "arn:aws:s3:::" - - !Ref TopsAuditS3Bucket - - /* - - Effect: Allow - Action: - - "s3:PutObject" - - "s3:PutObjectAcl" - Resource: !Join - - "" - - - "arn:aws:s3:::" - - !Ref TopsAuditS3Bucket - - /AWSLogs/ - - !Ref "AWS::AccountId" - - /* - - Effect: Allow - Action: - - "s3:*" - Resource: !Join - - "" - - - "arn:aws:s3:::" - - !Ref TopsAuditS3Bucket - Roles: - - !Ref ConfigServiceLinkedRole - ConfigAggregatorRole: - Type: AWS::IAM::Role - Properties: - RoleName: "tops-config-aggregrator-role" - AssumeRolePolicyDocument: - Version: 2012-10-17 - Statement: - - Effect: Allow - Principal: - Service: - - config.amazonaws.com - Action: - - sts:AssumeRole - Path: / - ManagedPolicyArns: - - arn:aws:iam::aws:policy/service-role/AWSConfigRoleForOrganizations - # KMS S3 Config Service encryption key - s3configKey: - Type: AWS::KMS::Key - Properties: - KeyPolicy: - Version: 2012-10-17 - Id: key-s3config - Statement: - - Sid: Enable IAM User Permissions - Effect: Allow - Principal: - AWS: !Join - - "" - - - "arn:aws:iam::" - - !Ref "AWS::AccountId" - - ":root" - Action: "kms:*" - Resource: "*" - s3configKeyAlias: - Type: AWS::KMS::Alias - Properties: - AliasName: alias/tops-audit - TargetKeyId: - Ref: s3configKey - - # Build AWS Config Service S3 Bucket for Storage - TopsAuditS3Bucket: - Type: AWS::S3::Bucket - DeletionPolicy: Retain - UpdateReplacePolicy: Retain - Properties: - BucketName: !Join - - "-" - - - "tops-config-audit" - - !Select [2, !Split ["/", !Ref AWS::StackId]] - BucketEncryption: - ServerSideEncryptionConfiguration: - - ServerSideEncryptionByDefault: - SSEAlgorithm: AES256 - # BucketEncryption: - # ServerSideEncryptionConfiguration: - # - ServerSideEncryptionByDefault: - # KMSMasterKeyID: !Sub "arn:aws:kms:${AWS::Region}:${AWS::AccountId}:${s3configKeyAlias}" - # SSEAlgorithm: "aws:kms" - PublicAccessBlockConfiguration: - BlockPublicAcls: true - BlockPublicPolicy: true - IgnorePublicAcls: true - RestrictPublicBuckets: true - VersioningConfiguration: - Status: Enabled - LifecycleConfiguration: - Rules: - - Id: "AWSConfigBucketRule" - Status: Enabled - Prefix: AWSLogs/ - ExpirationInDays: 365 - TopsAuditS3ConfigPolicy: - Type: AWS::S3::BucketPolicy - Properties: - Bucket: - Ref: TopsAuditS3Bucket - PolicyDocument: - Version: 2012-10-17 - Statement: - - Sid: AllowMFADeleteOnly - Effect: Deny - Principal: "*" - Action: "s3:DeleteBucket" - Resource: - - !Sub "arn:aws:s3:::${TopsAuditS3Bucket}" - Condition: - Bool: - "aws:MultiFactorAuthPresent": false - - Sid: AWSConfigBucketPermissionsCheck - Effect: Allow - Principal: - Service: - - config.amazonaws.com - Action: - - s3:GetBucketAcl - Resource: - - !Sub "arn:aws:s3:::${TopsAuditS3Bucket}" - # Condition: - # StringEquals: - # "AWS:SourceAccount": - # - !Ref "AWS::AccountId" - - Sid: AWSConfigBucketAccessCheck - Effect: Allow - Principal: - Service: - - config.amazonaws.com - Action: - - s3:ListBucket - Resource: - - !Sub "arn:aws:s3:::${TopsAuditS3Bucket}" - # Condition: - # StringEquals: - # "AWS:SourceAccount": - # - !Ref "AWS::AccountId" - - Sid: AWSConfigBucketDelivery - Effect: Allow - Principal: - Service: - - config.amazonaws.com - Action: - - s3:PutObject - Resource: - - !Sub "arn:aws:s3:::${TopsAuditS3Bucket}/AWSLogs/${AWS::AccountId}/Config/*" - # Condition: - # StringEquals: - # "AWS:SourceAccount": - # - !Ref "AWS::AccountId" - # "s3:x-amz-acl": bucket-owner-full-control - -Outputs: - RoleArn: - Description: "The ARN of the role" - Value: !GetAtt - - TopsAuditSetup - - Arn - RoleName: - Description: "The name of the role" - Value: !Ref TopsAuditSetup - S3Bucket: - Description: "The ARN of the S3 bucket" - Value: !GetAtt - - TopsAuditS3Bucket - - Arn diff --git a/templates/sync.sh b/templates/sync.sh deleted file mode 100644 index 5ffb2d64..00000000 --- a/templates/sync.sh +++ /dev/null @@ -1,5 +0,0 @@ -#!/bin/bash -#make s3 bucket public -aws s3api put-bucket-acl --bucket storage.teemops.com --acl public-read -aws s3 sync . s3://storage.teemops.com/ --acl public-read --exclude ".git/*" --exclude "README.md" --exclude "sync.sh" - diff --git a/tests/install-messaging.test.sh b/tests/install-messaging.test.sh index b03ba37d..ad3f81e1 100755 --- a/tests/install-messaging.test.sh +++ b/tests/install-messaging.test.sh @@ -391,8 +391,11 @@ else fail "the installer does not persist TOPS_INSTALL_ID — the next re-run would mint a new one" fi -for template in "$REPO_ROOT/templates/iam.role.child.account.cfn.yaml" \ - "$REPO_ROOT/templates/iam.role.audit.account.cfn.yaml"; do +# One template, and it should stay that way. The audit variant was deleted in #102: +# it defaulted ParentAWSAccountId to the vendor's own AWS account, so deploying it +# unmodified created a cross-account role trusting an account the operator did not +# control. Nothing referenced it. +for template in "$REPO_ROOT/templates/iam.role.child.account.cfn.yaml"; do name="$(basename "$template")" if grep -q 'TopsInstallId: !Ref TopsInstallId' "$template"; then @@ -411,6 +414,19 @@ for template in "$REPO_ROOT/templates/iam.role.child.account.cfn.yaml" \ fi done +# A template with a Default on the parent account is deployable by a stranger against +# an account they do not own — the defect #102 was filed for. Guard the whole +# directory rather than the one file, so a future template cannot reintroduce it. +for template in "$REPO_ROOT"/templates/*.cfn.yaml; do + name="$(basename "$template")" + + if awk '/^ ParentAWSAccountId:/{f=1; next} f && /^ [A-Za-z]/{exit} f && /Default:/{print; exit}' "$template" | grep -q Default; then + fail "$name gives ParentAWSAccountId a Default — it would deploy against whatever account is baked in" + else + pass "$name makes ParentAWSAccountId explicit" + fi +done + # The dead condition this replaces must not come back as live YAML. Comments are # stripped first: the template explains at length why sns:MessageAttributes cannot # work, and that prose is the point — it is what stops the line being revived.