Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

aws_ses: SES ReceiptRuleSet S3 action grants too wide permissions #29811

Open
markusl opened this issue Apr 12, 2024 · 4 comments · Fixed by #29833, rwlxxvii/containers#124 or rwlxxvii/containers#140 · May be fixed by NOUIY/aws-solutions-constructs#98 or NOUIY/aws-solutions-constructs#99
Labels
@aws-cdk/aws-ses Related to Amazon Simple Email Service bug This issue is a bug. effort/small Small work item – less than a day of effort p2

Comments

@markusl
Copy link
Contributor

markusl commented Apr 12, 2024

Describe the bug

SES ReceiptRuleSet S3 action grants too wide permissions

Expected Behavior

Should work as documented
https://docs.aws.amazon.com/ses/latest/dg/receiving-email-permissions.html#receiving-email-permissions-s3

Currently missing the following block:

      "Condition":{
        "StringEquals":{
          "AWS:SourceAccount":"111122223333",
          "AWS:SourceArn": "arn:aws:ses:region:111122223333:receipt-rule-set/rule_set_name:receipt-rule/receipt_rule_name"
        }
      }

Current Behavior

Template.fromStack(stack).hasResourceProperties('AWS::S3::BucketPolicy', {

Reproduction Steps

    const ruleSet = new ses.ReceiptRuleSet(this, 'RuleSet');

    const defaultRule = ruleSet.addRule('DefaultRule', {
      recipients: props.recipients,
      enabled: true,
    });

    defaultRule.addAction(new actions.S3({
      bucket: new s3.Bucket(this, 'EmailBucket', {
        removalPolicy: cdk.RemovalPolicy.DESTROY,
        bucketName: props.bucketName,
      }),
    }));

Possible Solution

No response

Additional Information/Context

No response

CDK CLI Version

2.137.0

Framework Version

No response

Node.js Version

20

OS

all

Language

TypeScript

Language Version

No response

Other information

No response

@markusl markusl added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Apr 12, 2024
@github-actions github-actions bot added the @aws-cdk/aws-ses Related to Amazon Simple Email Service label Apr 12, 2024
@pahud pahud self-assigned this Apr 12, 2024
@pahud
Copy link
Contributor

pahud commented Apr 12, 2024

Yes, it's missing the required conditions in the bucket policy.

// create a DummyStack
export class DummyStack extends Stack {
  constructor(scope: Construct, id: string, props: StackProps) {
    super(scope, id, props);

    const ruleSet = new ses.ReceiptRuleSet(this, 'RuleSet');

    const defaultRule = ruleSet.addRule('DefaultRule', {
      recipients: ['foo.com'],
      enabled: true,
    });

    defaultRule.addAction(new sesa.S3({
      bucket: new s3.Bucket(this, 'EmailBucket', {
        removalPolicy: RemovalPolicy.DESTROY,
        bucketName: 'mock-bucket-name',
      }),
    }));
  }
}

BucketPolicy:

  Type: AWS::S3::BucketPolicy
    Properties:
      Bucket:
        Ref: EmailBucket843A740F
      PolicyDocument:
        Statement:
          - Action: s3:PutObject
            Condition:
              StringEquals:
                aws:Referer:
                  Ref: AWS::AccountId
            Effect: Allow
            Principal:
              Service: ses.amazonaws.com
            Resource:
              Fn::Join:
                - ""
                - - Fn::GetAtt:
                      - EmailBucket843A740F
                      - Arn
                  - /*
        Version: "2012-10-17"

Guess we should fix here

conditions: {
StringEquals: {
'aws:Referer': cdk.Aws.ACCOUNT_ID,
},

@pahud pahud removed their assignment Apr 12, 2024
@pahud pahud added p1 effort/small Small work item – less than a day of effort and removed needs-triage This issue or PR still needs to be triaged. labels Apr 12, 2024
@mergify mergify bot closed this as completed in #29833 Apr 19, 2024
@mergify mergify bot closed this as completed in 2da544f Apr 19, 2024
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

1 similar comment
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

@samson-keung
Copy link
Contributor

samson-keung commented Jun 5, 2024

Since the PR for this issue is reverted, I am re-opening this issue, although I am not sure if there is a good way to fix without running into the same problem that caused the revert..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment