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.

Sign up for GitHub

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

Jump to bottom

SNS: Enforce SSL with Prop #29142

Closed
2 tasks
jlosito opened this issue Feb 16, 2024 · 5 comments · Fixed by #29144
Closed
2 tasks

SNS: Enforce SSL with Prop #29142

jlosito opened this issue Feb 16, 2024 · 5 comments · Fixed by #29144
Labels
@aws-cdk/aws-sns Related to Amazon Simple Notification Service effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p2

Comments

@jlosito
Copy link

jlosito commented Feb 16, 2024 •
edited
Loading

Describe the feature

I'd like the ability to enforce SSL on an SNS topic in a similar fashion as the Bucket construct. The Bucket construct has a property, enforceSSL, that will automatically update the bucket policy and enforce SSL. I'd like something similar that will automatically update the topic policy.

https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_s3.Bucket.html#enforcessl

Use Case

In the AwsSolutionsChecks within cdk-nag, there is a rule that requires SSL on SNS Topics, AwsSolutions-SNS3. Given that is a recommended practice, I believe setting an SNS Topic's policy so that it requires SSL would be a typical scenario. Since it's a typical scenario, I'd like a property to do it for me rather than having to write up a TopicPolicy every time.

https://github.com/cdklabs/cdk-nag/blob/main/RULES.md

Proposed Solution

new Topic(this, 'MyTopic', {
    enforceSSL: true,
});

Other Information

No response

Acknowledgements

  • I may be able to implement this feature request
  • This feature might incur a breaking change

CDK version used

2.126.0

Environment details (OS name and version, etc.)

macOS 14.3
@jlosito jlosito added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Feb 16, 2024
@github-actions github-actions bot added the @aws-cdk/aws-sns Related to Amazon Simple Notification Service label Feb 16, 2024
@jlosito jlosito changed the title (module name): (short issue description) SNS: Enforce SSL with Prop Feb 16, 2024
@msambol msambol mentioned this issue Feb 17, 2024
Merged
@pahud
Copy link
Contributor

pahud commented Feb 19, 2024

I am not sure if we should expose that to the L2 props but feel free to submit a PR to move this forward. Thank you!

@pahud pahud added p2 effort/medium Medium work item – several days of effort and removed needs-triage This issue or PR still needs to be triaged. labels Feb 19, 2024
@jlosito
Copy link
Author

jlosito commented Feb 19, 2024

@pahud there already seems to be a PR open for this.
#29144

@mergify mergify bot closed this as completed in #29144 Feb 20, 2024
mergify bot pushed a commit that referenced this issue Feb 20, 2024
@msambol
feat(sns): enforce ssl with topic policy (#29144)
a75f447 
Adds a statement to match the document in the [docs](https://docs.aws.amazon.com/sns/latest/dg/sns-security-best-practices.html#enforce-encryption-data-in-transit):

```
{
  "Id": "ExamplePolicy",
  "Version": "2012-10-17",
  "Statement": [
    {
      "Sid": "AllowPublishThroughSSLOnly",
      "Action": "SNS:Publish",
      "Effect": "Deny",
      "Resource": [
        "arn:aws:sns:us-east-1:1234567890:test-topic"
      ],
      "Condition": {
        "Bool": {
          "aws:SecureTransport": "false"
        }
      },
      "Principal": "*"
    }
  ]
}
```

Closes #29142.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@github-actions GitHub Actions
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.

GavinZZ pushed a commit that referenced this issue Feb 22, 2024
@msambol @GavinZZ
feat(sns): enforce ssl with topic policy (#29144)
1a92663 
Adds a statement to match the document in the [docs](https://docs.aws.amazon.com/sns/latest/dg/sns-security-best-practices.html#enforce-encryption-data-in-transit):

```
{
  "Id": "ExamplePolicy",
  "Version": "2012-10-17",
  "Statement": [
    {
      "Sid": "AllowPublishThroughSSLOnly",
      "Action": "SNS:Publish",
      "Effect": "Deny",
      "Resource": [
        "arn:aws:sns:us-east-1:1234567890:test-topic"
      ],
      "Condition": {
        "Bool": {
          "aws:SecureTransport": "false"
        }
      },
      "Principal": "*"
    }
  ]
}
```

Closes #29142.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@github-actions github-actions bot mentioned this issue Mar 1, 2024
Closed
@mrgum
Copy link

mrgum commented Jun 5, 2024

Hello,

Does this work?

I've tried both python and typescript, cdk latest and 2.129.0 (the release notes for the one it was added to) and neither add a topicpolicy when the enforceSSL/enforce_ssl property is set to true/True

The synth is the same whether I add that property or not

@Adiotastic
Copy link

Adiotastic commented Nov 5, 2024 •
edited
Loading

That is because the enforceSSL is only added if you use the "addToResourcePolicy" afterwards.
See Line Of Code

I would expect to have id added, in any case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
@aws-cdk/aws-sns Related to Amazon Simple Notification Service effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p2
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat(sns): enforce ssl with topic policy msambol/aws-cdk
4 participants
@pahud @mrgum @Adiotastic @jlosito