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-kms): circular dependency when encrypted bucket notifications are setup with encrypted SQS queue #17786

Closed
ryparker opened this issue Nov 30, 2021 · 5 comments
Labels
@aws-cdk/aws-kms Related to AWS Key Management bug This issue is a bug. closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. effort/small Small work item – less than a day of effort p1

Comments

@ryparker
Copy link
Contributor

ryparker commented Nov 30, 2021

What is the problem?

A circular dependency error is thrown during cdk deploy when using a KMS key for encrypting an S3 bucket and an SQS queue, then setup a notification on the S3 bucket to message the SQS queue.

Reproduction Steps

See full repro code

# Create Role
AwsCdkPythonSampleKmsStack.__Role = IamService.create_role(self)

# Get KMS policy Document
kms_policy_document = IamService.get_kms_policy_documents(self)

kms_key = kms.Key(self, 'ssl_s3_sqs_kms_key',
                  alias='sslS3SqsKmsKey',
                  description='This is a kms key',
                  enabled=True,
                  enable_key_rotation=True,
                  policy=kms_policy_document,
                  )

# Create the S3 bucket
bucket = s3.Bucket(
    self,
    id="ssl_s3_bucket_raw_kms",
    bucket_name="ssl-s3-bucket-kms-raw",
    encryption=s3.BucketEncryption.KMS,
    encryption_key=kms_key,
)

# Create the SQS queue
queue = sqs.Queue(
    self,
    id="ssl_sqs_event_queue",
    queue_name="ssl-sqs-kms-event-queue",
    encryption=sqs.QueueEncryption.KMS,
    encryption_master_key=kms_key,
)

# Create S3 notification object which points to SQS
notification = s3notif.SqsDestination(queue)
filter1 = s3.NotificationKeyFilter(prefix="home/")

# Attach notification event to S3 bucket
bucket.add_event_notification(s3.EventType.OBJECT_CREATED, notification, filter1) # <-- Causes circular dep error

What did you expect to happen?

No circular dependency error.

What actually happened?

On cdk deploy:

 ❌  AwsCdkPythonSampleKmsStack failed: Error [ValidationError]: Circular dependency between resources: [ssls3sqskmskey83E47315, ssls3bucketrawkms4B1E1122, ssls3sqskmskeyAlias49ACAD8B, sslsqseventqueuePolicy196D842E, ssls3bucketrawkmsNotificationsDE94AE6A, sslsqseventqueue5BCAC5CE]

CDK CLI Version

2.0.0-rc.33 (build 336ff5e)

Framework Version

No response

Node.js Version

v16.13.0

OS

macOS Montery 12.0.1 (21A559)

Language

Python

Language Version

v3.9.8

Other information

This bug was discovered from a StackOverflow question
Related GitHub issue: #11158

It looks like the root problem is that the following condition is added to the KMS key:

KMS key resource Properties.KeyPolicy.Statement[1]:

{
	"Action": [
		"kms:Decrypt",
	 	"kms:Encrypt",
		 "kms:ReEncrypt*",
		 "kms:GenerateDataKey*"
	 ],
	 "Condition": {
	 	"ArnLike": {
	    	"aws:SourceArn": {
	      		"Fn::GetAtt": [
	        		"ssls3bucketrawkms4B1E1122",
	        		"Arn"
	     		 ]
	   		 }
	 	 }
 	 },
},

The S3 bucket depends on the KMS key for encryption, and the KMS key has a condition that depends on the S3 bucket.

Workaround

I was able to deploy the stack after using escape hatches to delete the condition:

# Delete the circular reference
cfn_kms_key = kms_key.node.default_child
cfn_kms_key.add_property_deletion_override("KeyPolicy.Statement.1")

workaround code available here

@ryparker ryparker added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Nov 30, 2021
@github-actions github-actions bot added the @aws-cdk/aws-kms Related to AWS Key Management label Nov 30, 2021
@ryparker ryparker added effort/small Small work item – less than a day of effort p1 and removed needs-triage This issue or PR still needs to be triaged. labels Nov 30, 2021
@njlynch njlynch removed their assignment Dec 29, 2021
@bearrito
Copy link

bearrito commented May 7, 2022

For folks googling, this is still broken in V2.22. Also this does not effect only SQS but also S3 buckets in a similar way.

@github-actions
Copy link

github-actions bot commented May 8, 2023

This issue has not received any attention in 1 year. If you want to keep this issue open, please leave a comment below and auto-close will be canceled.

@github-actions github-actions bot added closing-soon This issue will automatically close in 4 days unless further comments are made. closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. and removed closing-soon This issue will automatically close in 4 days unless further comments are made. labels May 8, 2023
@privateGenish
Copy link

this is an unbelievable issue.


  GetPublicKeyFunction:
    Type: AWS::Serverless::Function
    Properties:
      CodeUri: ./src/routes/
      Handler: getPublicKey.handler
      Events:
        RootGet:
          Type: HttpApi
          Properties:
            Path: /publicKey
            Method: GET
            ApiId: !Ref HttpApi
      Role: !GetAtt GetPublicKeyFunctionRole.Arn
  
  GetPublicKeyFunctionRole:
    Type: AWS::IAM::Role
    Properties:
      AssumeRolePolicyDocument:
        Version: '2012-10-17'
        Statement:
          - Effect: Allow
            Principal:
              Service: lambda.amazonaws.com
            Action: sts:AssumeRole
      Policies:
        - PolicyName: LambdaKMSAccess
          PolicyDocument:
            Version: '2012-10-17'
            Statement:
              - Effect: Allow
                Action: kms:GetPublicKey
                Resource: !GetAtt RSASigningKey.Arn

  RSASigningKey:
    Type: 'AWS::KMS::Key'
    Properties:
      Description: RSA-3072 asymmetric KMS key for signing and verification
      KeySpec: RSA_3072
      KeyUsage: SIGN_VERIFY
      KeyPolicy:
        Version: '2012-10-17'
        Statement:
          - Sid: GetKey
            Effect: Allow
            Principal: 
            - AWS: !GetAtt GetPublicKeyFunctionRole.Arn 
            Action: kms:GetKey*
            Resource: '*'

such simple procedure cant be done due to circular dependency... an unused feature

@kornicameister
Copy link
Contributor

Is creating an alias for key a workaround? at least it does seem to help me out :D

@lukemsmyth
Copy link

Per @kornicameister suggestion, the below worked for me. The trick was to give the KMS Key Alias as a SAM Parameter and reference that value throughout:

Parameters:
  KmsKeyAliasName:
    Type: String
    Default: alias/KmsKey

Resources:
  KmsKeyAlias:
    Type: 'AWS::KMS::Alias'
    Properties:
      AliasName: !Ref KmsKeyAliasName
      TargetKeyId: !Ref KmsKey
  KmsKey:
    Type: AWS::KMS::Key
    Properties:
      Enabled: True
      KeySpec: SYMMETRIC_DEFAULT
      KeyPolicy:
        Version: 2012-10-17
        Id: key-default-1
        Statement:
          - Sid: Allow use of the key
            Effect: Allow
            Principal:
              AWS: !Join ["", ["arn:aws:sts::", !Ref "AWS::AccountId", ":assumed-role/", !Ref MyRole, "/", !Ref MyFunction]]
            Action:
              - 'kms:Encrypt'
              - 'kms:Decrypt'
            Resource: '*'
  MyFunction:
    Type: AWS::Serverless::Function 
    Properties:
      Environment:
        Variables:
          KMS_KEY_ID: !Ref KmsKeyAliasName
      Events:
        QueueEvent:
          Type: SQS
          Properties:
            BatchSize: 1
            Enabled: True
            Queue: !GetAtt MyQueue.Arn

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-kms Related to AWS Key Management bug This issue is a bug. closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. effort/small Small work item – less than a day of effort p1
Projects
None yet
Development

No branches or pull requests

6 participants