(s3): dependency not added when key alias provided as encryptionKey to bucket

[thbrooks22]

Describe the bug

Our team has an S3 Bucket construct that uses a KMS Alias construct as its (the Bucket's) encryptionKey. When the CDK code is compiled down to CloudFormation, the BucketEncryption.ServerSideEncryptionConfiguration.KMSMasterKeyID compiles down to something like

{
 "Fn::Join": [
  "",
  [
   "arn:",
   {
    "Ref": "AWS::Partition"
   },
   ":kms:<REGION>:<ACCOUNT_ID>:alias/<ALIAS_NAME>"
  ]
 ]
}

instead of something like

{
 "Ref": "<ALIAS_LOGICAL_ID>"
}

which was unexpected.

This impacted us in production in the following way. We had already deployed our Bucket previously, and writes to the Bucket are on the critical path for some of our APIs. We wanted to deploy the Key and corresponding Alias as well, so we did so. However, during the deployment for about 2 minutes, the APIs for which the Bucket is on the critical path were giving Internal Server Errors due to AmazonS3Exception: Alias <ALIAS_ARN> is not found.. We found that this was because, without a logical dependency between Bucket and Alias, the Bucket's encryptionKey was updated before the Alias had been created.

Expected Behavior

We expected the BucketEncryption.ServerSideEncryptionConfiguration.KMSMasterKeyID to compile down to something like

{
 "Ref": "<ALIAS_LOGICAL_ID>"
}

so that there would be a logical dependency between the Bucket and the Alias, which is what you would expect from looking at the CDK code.

Current Behavior

The BucketEncryption.ServerSideEncryptionConfiguration.KMSMasterKeyID compiled down to something like

{
 "Fn::Join": [
  "",
  [
   "arn:",
   {
    "Ref": "AWS::Partition"
   },
   ":kms:<REGION>:<ACCOUNT_ID>:alias/<ALIAS_NAME>"
  ]
 ]
}

instead.

Reproduction Steps

Create a stack consisting only of the following CDK constructs:

const keyPolicy = new PolicyDocument({
  statements: [
    new PolicyStatement({
      sid: 'FooStmt',
      'actions': [
        'kms:Decrypt',
        'kms:GenerateDataKey'
      ],
      principals: [new AnyPrincipal()],
      resources: ['*'],
    })
  ]
})

const key = new Key(this, 'FooBucketKey', {
  enableKeyRotation: true,
  enabled: true,
  policy: keyPolicy,
  removalPolicy: RemovalPolicy.RETAIN
})

const alias = new Alias(this, 'FooBucketKeyAlias', {
  aliasName: 'fooBucketKeyAlias',
  targetKey: key
})

const bucket = new Bucket(this, 'FooBucket', {
  bucketName: 'fooBucket',
  removalPolicy: RemovalPolicy.RETAIN,
  blockPublicAccess: BlockPublicAccess.BLOCK_ALL,
  versioned: true,
  encryption: BucketEncryption.KMS,
  encryptionKey: alias,
  enforceSSL: true
})

Possible Solution

When attaching an IKey to a resource, check if the IKey is an Alias, and if so, compile down to a reference instead of an ARN.

Additional Information/Context

No response

CDK CLI Version

2.14.0

Framework Version

No response

Node.js Version

14.21.3

OS

Amazon Linux 2

Language

Typescript

Language Version

Typescript (4.3.0)

Other information

No response

[rix0rrr]

I think we change:

    this.aliasName = this.getResourceNameAttribute(resource.aliasName);

into

    this.aliasName = this.getResourceNameAttribute(resource.ref);

https://github.com/aws/aws-cdk/blob/main/packages/aws-cdk-lib/aws-kms/lib/alias.ts/#L227

I suggest we do that, but maybe under a feature flag for safety?

---

Motivation: resource.aliasName is the value of AliasName property as provided by the user (always a literal string).

resource.ref will be { Ref: MyAlias }, evaluating to the same value (alias name) but having a reference implied.

(This change may break cross-account workflows if people currently are relying on the literal string to make that reference work)

[github-actions]

⚠️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.