aws / aws-cdk

The AWS Cloud Development Kit is a framework for defining cloud infrastructure in code
https://aws.amazon.com/cdk
Apache License 2.0
11.64k stars 3.91k forks source link

(aws-iot-actions): SNS-Topic-Action missing master-key policies #24848

Open cponfick opened 1 year ago

cponfick commented 1 year ago

Describe the bug

When creating an SNS topic action with a master-key the action does not work, because it does not have the permission to use the KMS-Key.

Expected Behavior

I would expect the action to work.

Current Behavior

It does not work, because of missing KMS-Key permissions.

Reproduction Steps

# kms_key

sns_topic = aws_sns.Topic(self, 'MyTopic', master_key=kms_key)
aws_iot_alpha.TopicRule(
          self,
          f'MyTopicRule',
          actions=[
              iot_actions.SnsTopicAction(
                  sns_topic,
                  message_format=iot_actions.SnsActionMessageFormat.RAW,
              )
          ],
          error_action=iot_actions.CloudWatchLogsAction(
              aws_logs.LogGroup(self, 'ErrorTopicRuleMyAction')
          ),
          sql=iot.IotSql.from_string_as_ver20160323(
              f'SELECT * FROM "$aws/events/presence/connected/#"'
          ),
      )

Possible Solution

I did not look into the source code yet, but I guess it should be possible to grant the required permissions to the sns topic action role.

The following is a workaround I currently use:

# kms_key
# iam_role

ksm_key.grant_encrypt_decrypt(iam_role)
sns_topic = aws_sns.Topic(self, 'MyTopic', master_key=kms_key)
aws_iot_alpha.TopicRule(
          self,
          f'MyTopicRule',
          actions=[
              iot_actions.SnsTopicAction(
                  sns_topic,
                  message_format=iot_actions.SnsActionMessageFormat.RAW,
                  role= iam_role
              )
          ],
          error_action=iot_actions.CloudWatchLogsAction(
              aws_logs.LogGroup(self, 'ErrorTopicRuleMyAction')
          ),
          sql=iot.IotSql.from_string_as_ver20160323(
              f'SELECT * FROM "$aws/events/presence/connected/#"'
          ),
      )

Additional Information/Context

No response

CDK CLI Version

2.70

Framework Version

No response

Node.js Version

16.15.0

OS

MacOS

Language

Python

Language Version

3.9

Other information

No response

pahud commented 1 year ago

Hi

 iot_actions.SnsTopicAction(
                  sns_topic,
                  message_format=iot_actions.SnsActionMessageFormat.RAW,
                  role= iam_role
              )

I believe you need manually grant the read permission to the iam role before you are allowed to to that because SnsTopicAction has no idea if the sns_topic comes with a master key property.

cponfick commented 1 year ago

Hi

 iot_actions.SnsTopicAction(
                  sns_topic,
                  message_format=iot_actions.SnsActionMessageFormat.RAW,
                  role= iam_role
              )

I believe you need manually grant the read permission to the iam role before you are allowed to to that.

Yes, that's how I currently do it. But if no role is given I would expect that the grant happens automatically inside SnsTopicAction (on the generated role).

cponfick commented 1 year ago

@pahud I guess this is not really a bug, but an inconsistency in how different grants work.

Looking at grantWrite of a s3 bucket. It is stated that it grants the required permission to encrypt the data. SnsTopicAction just calls grantPublish. This does not grant the permission to encrypt data. Maybe I am missing something, but this seems odd to me.

As a user I would expect that grantWrite and grantPublish would either both grant the required policies for encryption, or none would.

peterwoodworth commented 1 year ago

This is something that could be better handled by our codebase - it would be great if this could automatically detect the key on the Topic. We'd need to expose this on the Topic construct first, and then handle it appropriately after that, but it should be possible!

yamatatsu commented 1 year ago

I'll work on it.