(aws-key): Secret with KMS that calls secret.grantRead() in another stack causes cyclic reference

[prli]

Calling secret.grantRead() in another stack on a secret encrypted with KMS would result in cyclic reference error.

Reproduction Steps

#!/usr/bin/env node
import cdk = require("monocdk");
import iam = require("monocdk/aws-iam");
import s3 = require("monocdk/aws-s3");
import secret = require("monocdk/aws-secretsmanager");
import key = require("monocdk/aws-kms");

class S3HostStack extends cdk.Stack {
    public readonly bucket: s3.Bucket;
    public readonly key: key.Key;
    public readonly secret: secret.Secret;

    constructor(scope: cdk.Construct, id: string, props?: cdk.StackProps) {
        super(scope, id, props);
        this.key = new key.Key(this, 'test-secret-key', {
            alias: "test-secret-key",
            removalPolicy: cdk.RemovalPolicy.DESTROY
        });
        this.secret = new secret.Secret(this, 'test_secret', {
            secretName: 'test_secret',
            encryptionKey: this.key,
            removalPolicy: cdk.RemovalPolicy.DESTROY,
        });
    }
}

class S3ConsumerStack extends cdk.Stack {
    constructor(scope: cdk.Construct, id: string, s3HostStack: S3HostStack) {
        super(scope, id);

        const accessRole = new iam.Role(this, "Access", {
            assumedBy: new iam.ServicePrincipal("ecs-tasks.amazonaws.com"),
        });

        this.addDependency(s3HostStack); //required as seen at https://github.com/aws/aws-cdk/issues/3732#issuecomment-571708554

        // s3HostStack.secret.grantRead(accessRole); //this creates cyclic reference
        //workaround since secret.grantRead() have issues
        accessRole.addToPolicy(new iam.PolicyStatement({
            effect: iam.Effect.ALLOW,
            actions: ["secretsmanager:*", "kms:*"],
            resources: [s3HostStack.secret.secretArn, s3HostStack.key.keyArn],
        }));
    }
}

const app = new cdk.App();
const host = new S3HostStack(app, 'S3HostStack');
new S3ConsumerStack(app, 'S3ConsumerStack', host);

What did you expect to happen?

able to grant read permission to secrets encrypted with KMS successfully

What actually happened?

Error: 'S3HostStack' depends on 'S3ConsumerStack' (S3HostStack -> S3ConsumerStack/Access/Resource.Arn). Adding this dependency (S3ConsumerStack -> S3HostStack/test_secret/Resource.Ref) would create a cyclic reference.

Environment

• CDK CLI Version : 1.96.0 (build 39f3df8)
• Framework Version: 1.96.0 (build 39f3df8)
• Node.js Version: v14.12.0
• OS : macOS Catalina 10.15.7
• Language (Version): TypeScript (3.6.4)

Other

Similar issue - #3732

As a workaround, i am calling addToPolicy() to grant permission on the secret and key using their ARN. Any better workaround suggestions are welcome.

This is 🐛 Bug Report

[njlynch]

Thanks for the bug report and reproduction steps!

I agree #3732 is related, but also #5765 and also #10160 (due to interactions with ViaServicePrincipal).

The cyclic reference is caused by the Key policy being updated with the grant for the user in the S3ConsumerStack. Technically, this could be placed entirely on the user policy (no need for the key policy to be altered), but because of the way the ViaServicePrincipal currently operates, no grant is associated with the user policy and the key policy is instead altered.

From a quick look and repro, I believe the fix here would likely be shared with the fix for #10160, where the ViaServicePrincipal needs to get a bit smarter about delegating to the underlying grant principal. However, this is a non-trivial change due to its broad impact, so it will need to be approached carefully.

In the mean time, I agree with your workaround, although you could certainly tighten up those permissions a bit. I believe the below is the minimal set of permissions necessary:

        accessRole.addToPolicy(new iam.PolicyStatement({
            effect: iam.Effect.ALLOW,
            actions: ['secretsmanager:GetSecretValue', 'secretsmanager:DescribeSecret'],
            resources: [s3HostStack.secret.secretArn],
        }));
        accessRole.addToPolicy(new iam.PolicyStatement({
            effect: iam.Effect.ALLOW,
            actions: ['kms:Decrypt'],
            resources: [s3HostStack.key.keyArn],
            conditions: { 'kms:ViaService': `secretsmanager.${Stack.of(this).region}.amazonaws.com` },
        }));

[prli]

I ended up using an alternate workaround before seeing the response.

const secretReimport = secret.Secret.fromSecretCompleteArn(this, "reimportSecret",
            s3HostStack.secret.secretFullArn!.toString()!);
        secretReimport.grantRead(accessRole);

[misirlou-tg]

I saw this also when using ecs.Secret.fromSecretsManager() to pass a secret into a container. One difference is that I was not directly calling secret.grantRead(), the ECS method was calling it "on my behalf".

I had to use the alternate workaround from @prli above of "reimporting" the secret via secret.Secret.fromSecretCompleteArn() instead of directly referencing the secret created in another stack.