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-cdk-lib: Replica Secrets with Replica KMS keys #29819

Open
justin-masse opened this issue Apr 12, 2024 · 20 comments
Open

aws-cdk-lib: Replica Secrets with Replica KMS keys #29819

justin-masse opened this issue Apr 12, 2024 · 20 comments
Labels
@aws-cdk/aws-kms Related to AWS Key Management bug This issue is a bug. effort/medium Medium work item – several days of effort p3

Comments

@justin-masse
Copy link

Describe the bug

Unable to attach CfnReplicaKey to replicated secrets in the ReplicaRegion[] because you cannot get an IKey for this resource across regions. This means replicated secrets that use KMS keys have to manually be updated in console after replica region deploys in order to attach the replica KMS key (or any KMS key I believe).

This is an inconsistency because of the way SecretsManager has decided to run all of it's logic from the "primary region" whereas KMS runs it's replication logic from the "secondary regions". This inconsistency between services is what leads to this issue.

Expected Behavior

Be able to attach a KMS key (specifically a replica key) when it is created in secondary regions to a secret replicated into that region.

Current Behavior

Sharing secrets across accounts, created a secret in us-east-1 with a defined KMS key and then replicated it into us-west-2. I replicated my KMS key as well but have no way to attach this back to the ReplicaRegions[] in my code.

Example of secret created in us-east-1: (defaultKmsKey obj

const defaultKmsKey = new aws_kms.CfnKey(
    this,
    'defaultProjectSecretsKMSKey',
    {
          description: 'KMS key for the secret secret',
          multiRegion: true,
          keyPolicy: getKmsKeyPolicy(),
    },
)
new aws_secretsmanager.Secret(this, 'defaultProjectSecrets', {
    secretName: 'mysecretsecret',
    // Have to convert CfnKey into IKey to use it as the encryption key to use the attrArn to retrieve it as an IKey
    encryptionKey: aws_kms.Key.fromKeyArn(
          this,
          'defaultKmsKeyAsKey',
          defaultKmsKey.attrArn,
    ),
    replicaRegions: [
          { region: 'us-west-2' }
    ],
})

In my secondary regions I am doing this:

new aws_kms.CfnReplicaKey(this, 'defaultProjectSecretsKMSKeyReplica', {
    description: 'Replicated KMS key for the secret secret',
    // This was deployed into us-east-1 first in order to pull this. Could store it in ParameterStore or something but this is fine for now.
    primaryKeyArn: 'my-us-east-1-kms-key-arn',
    keyPolicy: getKmsKeyPolicy(),
})

The problem here is that I cannot even attach this CfnReplicaKey back to my secret in replica regions because when I synth/deploy in that region the fromKeyArn() would fail since it's in a different region.

Reproduction Steps

  1. Create Secret with a KMS key in east-1 and set it to replicate to west-2
  2. Now secret in east-1 has your kms key, and secret in west-2 has the default AWS key.
  3. This secret can no longer be accessed across accounts now because it has the "default" KMS key on it in west-2.
  4. Replicate your KMS key into us-west-2 and you have no way to attach it (via cdk) back to the us-east-1 secret.

ReplicaRegion[] is expecting an IKey for encryptionKey prop but you cannot get an IKey because the KMS key you want to attach exists in another region and would fail to import via fromKeyArn() and even if it worked would be dirty.

Possible Solution

Allow a call on the Secret class from "replicated regions" that will allow you to update/attach a KMS key. Instead of declaring this @ create time.

Additional Information/Context

No response

CDK CLI Version

2.126

Framework Version

No response

Node.js Version

18

OS

macos

Language

TypeScript

Language Version

No response

Other information

No response

@justin-masse justin-masse added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Apr 12, 2024
@github-actions github-actions bot added the aws-cdk-lib Related to the aws-cdk-lib package label Apr 12, 2024
@pahud pahud self-assigned this Apr 16, 2024
@pahud pahud added the investigating This issue is being investigated and/or work is in progress to resolve the issue. label Apr 16, 2024
@pahud
Copy link
Contributor

pahud commented Apr 16, 2024

Thank you for bringing this up. This is a very great use case. I guess the code should be like:

stack.ts

export class PrimaryRegionStack extends Stack {
  readonly primaryKmsKeyArn: string;
  constructor(scope: Construct, id: string, props: StackProps) {
    super(scope, id, props);

    // create the kms key
    const defaultKmsKey = new kms.CfnKey(
      this,
      'defaultProjectSecretsKMSKey',
      {
        description: 'KMS key for the secret secret',
        multiRegion: true,
        keyPolicy: generateKeyPolicy(this),
      },
    )
    this.primaryKmsKeyArn = defaultKmsKey.attrArn;

    new secrets.Secret(this, 'defaultProjectSecrets', {
      // random secretName,
      // secretName: 'mysecretsecret',
      encryptionKey: kms.Key.fromKeyArn(
        this,
        'defaultKmsKeyAsKey',
        defaultKmsKey.attrArn,
      ),
      replicaRegions: [{ region: 'us-west-2' }],
    })

  }

}

export interface SecondaryRegionStackProps extends StackProps {
  readonly primaryKmsKeyArn: string;
}

export class SecondaryRegionStack extends Stack {
  constructor(scope: Construct, id: string, props: SecondaryRegionStackProps) {
    super(scope, id, props);

    const replicaKey = new kms.CfnReplicaKey(this, 'ReplicaKey', {
      description: 'Replicated KMS key for the secret secret',
      primaryKeyArn: props.primaryKmsKeyArn,
      keyPolicy: generateKeyPolicy(this),
  })
  }
}

app.ts

const app = new App();

const env = { region: process.env.CDK_DEFAULT_REGION, account: process.env.CDK_DEFAULT_ACCOUNT };
const uw2env = { region: 'us-west-2', account: process.env.CDK_DEFAULT_ACCOUNT };

const primaryStack = new PrimaryRegionStack(app, 'primary-stack', { env });
new SecondaryRegionStack(app, 'secondary-stack', { 
    env: uw2env,
    primaryKmsKeyArn: primaryStack.primaryKmsKeyArn,
    crossRegionReferences: true,
 });

But I didn't make it to deploy as some permissions are still missing but I believe this should be a correct pattern. I'll update here when I figure it out.

@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 Apr 16, 2024
@justin-masse
Copy link
Author

yeah @pahud the primary issue I was having was the need to present an IKey as the encryptionKey prop on ReplicaRegions[] but also the fact that Secrets Manager and KMS have different replication patterns. So run into the chicken and egg problem where I have dependency in us-east-1 on my us-west-2 stack being deployed in order to attach a KMS key to the secret in cdk.

If there was a method on ISecret to attach a KMS key (at least in replica regions) that would be phenomenal as I could get the replica secret in us-west-2 and then call an attach() type method. Or if KMS had a similar method on CfnReplicaKey() though that feels a little bit more hacky than having something on ISecret

@justin-masse
Copy link
Author

Though I do see you're passing in crossRegionReferences and maybe that allows me to access the IKey for a us-west-2 KMS key? That solves the problem of being able to do this in code, but still creates a race condition if I'm deploying to us-east-1 first then us-west-2 subsequently (or in parallel) and the only workaround would be to ALWAYS deploy east-1 + west-2 KMS key in a stack BEFORE the stack that creates the secret.

@pahud
Copy link
Contributor

pahud commented Apr 16, 2024

@justin-masse

Check my provided app.ts above:

const primaryStack = new PrimaryRegionStack(app, 'primary-stack', { env });
new SecondaryRegionStack(app, 'secondary-stack', { 
    env: uw2env,
    primaryKmsKeyArn: primaryStack.primaryKmsKeyArn,
    crossRegionReferences: true,
 });

This implicitly ensure the dependencies that primary stack would always be deployed first followed by the secondary. You can just deploy them with npx cdk deploy --all --require-approval=never. Sounds good?

@pahud
Copy link
Contributor

pahud commented Apr 16, 2024

If there was a method on ISecret to attach a KMS key

ISecret could be a reference of an existing Secret created outsides cloudformation and CDK/CFN generally can't modify the existing resource created out of CFN so I would doubt ISecret can attach the KMS key.

Are you able to deploy it using my sample above?

@pahud pahud added response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. and removed investigating This issue is being investigated and/or work is in progress to resolve the issue. labels Apr 16, 2024
@pahud pahud removed their assignment Apr 16, 2024
@justin-masse
Copy link
Author

@justin-masse

Check my provided app.ts above:

const primaryStack = new PrimaryRegionStack(app, 'primary-stack', { env });
new SecondaryRegionStack(app, 'secondary-stack', { 
    env: uw2env,
    primaryKmsKeyArn: primaryStack.primaryKmsKeyArn,
    crossRegionReferences: true,
 });

This implicitly ensure the dependencies that primary stack would always be deployed first followed by the secondary. You can just deploy them with npx cdk deploy --all --require-approval=never. Sounds good?

I'm not sure, I can try to add crossRegionReferences: true but we don't use cdk deploy instead we use the output CFN templates and run them through codepipeline... but I assume the dependencies are set in the CFN templates versus cdk deploy doing some sort of logic?

@pahud
Copy link
Contributor

pahud commented Apr 16, 2024

The generateKey() function for me is like this FYI. Definitely could be scoped down a little bit further. Just FYR.

function generateKeyPolicy(scope: Construct) {
  // return a dummy policy(do not use in production)
  const masterRegion = 'us-east-1';
  const secondaryRegion = 'us-west-2';
  const cfnExecRoleMaster = iam.Role.fromRoleName(scope, 'cfn-exec-role-m', `cdk-hnb659fds-cfn-exec-role-${Aws.ACCOUNT_ID}-${masterRegion}`);
  const cfnExecRoleSecondary = iam.Role.fromRoleName(scope, 'cfn-exec-role-s', `cdk-hnb659fds-cfn-exec-role-${Aws.ACCOUNT_ID}-${secondaryRegion}`);
  const adminRole = iam.Role.fromRoleName(scope, 'AdminRole', 'AdminRole');
  return new iam.PolicyDocument({
    statements: [
      // see https://docs.aws.amazon.com/secretsmanager/latest/userguide/security-encryption.html#security-encryption-authz
      // basic read-only permissions for the account
      new iam.PolicyStatement({
        principals: [ new iam.AccountRootPrincipal() ],
        actions: [
          'kms:Describe*',
          'kms:Get*',
        ],
        resources: ['*'],
      }),
      // for cfn-exec-role only
      new iam.PolicyStatement({
        principals: [ 
          cfnExecRoleMaster,
          cfnExecRoleSecondary,
        ],
        actions: [
          'kms:Describe*',
          'kms:GenerateDataKey',
          'kms:ReplicateKey',
          'kms:Decrypt',
        ],
        resources: ['*'],
      }),
      // secondary region should have replicateKey permission
      new iam.PolicyStatement({
        principals: [ 
          cfnExecRoleSecondary,
        ],
        actions: [
          'kms:ReplicateKey',
      ],
        resources: ['*'],
      }),
      // required for admin permission
      new iam.PolicyStatement({
        principals: [ 
          adminRole,
          cfnExecRoleMaster,
          cfnExecRoleSecondary,
        ],
        actions: [
          'kms:Create*',
          'kms:Describe*',
          'kms:Enable*',
          'kms:List*',
          'kms:Put*',
          'kms:Update*',
          'kms:Revoke*',
          'kms:Disable*',
          'kms:Get*',
          'kms:Delete*',
          'kms:TagResource',
          'kms:UntagResource',
          'kms:ScheduleKeyDeletion',
          'kms:CancelKeyDeletion'
      ],
        resources: ['*'],
      })
    ]
  })
}

@justin-masse
Copy link
Author

But i'm not sure how I get the KMS replica key back from us-west-2 into us-east-1 to attach to the replicaRegions? We are deploying us-east-1 first, then us-west-2 so the replica key won't exist yet @ secret replication time. I would have to do this in a series of 2 deployments:

  1. Create us-east-1 stack + create us-west-2 stack without KMS replica key attached to secret
  2. Deploy again to attach us-west-2 replica key to replicated secret

@pahud
Copy link
Contributor

pahud commented Apr 16, 2024

we don't use cdk deploy instead we use the output CFN templates and run them through codepipeline

Oh if you don't use cdk deploy then it would be another story. I think your pipeline should be able to deploy us-east-1 first and pass the primary key arn as a parameter down the pipeline to another stage to deploy us-west-2. I think this is still possible but would need some experiment.

@justin-masse
Copy link
Author

justin-masse commented Apr 16, 2024

If I have this:

new SecretsStack(app, 'SecretsStackEast', {
  stackName: 'SecretsEast',
  env: {
    region: 'us-east-1',
  },
})

new SecretsStack(app, 'SecretsStackWest', {
  stackName: 'SecretsWest',
  env: {
    region: 'us-west-2',
  },
  crossRegionReferences: true
})

Don't I still need to somehow export the KMS key arn for the CfnReplicaKey out of the "west stack" and import it back into the east stack? This would put a dependency that the West stack must be deployed before the east stack, correct? But if my secrets AND kms keys are created in this same stack that would never work?

I feel like without something really hacky there's just no way you can make this work in cdk just simply due to the disparity of how KMS and Secrets are replicated in different fashions (one being replicated from primary into secondary and one being created in secondary referencing an ARN from primary).

@pahud
Copy link
Contributor

pahud commented Apr 16, 2024

If you are deploying with CloudFormation deployment actions from the pipeline then you can control the deployment sequence from the pipeline. For example

  1. DeployActionUE1: deploy the us-east-1 stack
  2. DeployActionUW2: deploy the us-west-2 stack

The challenge is how to pass the keyArn between the two deployments. Off the top of my head we have two options:

  1. On the 1st stack deployment complete, update a SSM parameter store in us-east-1 to store that KeyArn and a custom resource from the 2nd stack in us-west-2 to retrieve that value from us-east-1 and pass that value as a prop to the construct in 2nd stack.

  2. codepipeline should allow you to transmit state/values between stages so I guess we should be able to pass that value from a CFN output from stack1 as a parameter to the 2nd CFN stack. See this doc for more details.

If you do not cdk deploy but use codepipeline to deploy two stacks in sequence, I guess option 2 might be a good solution to explore.

Another alternative option is using CodeBuild as your deploy action from the pipeline and run cdk deploy in your CodeBuild so you have full control using CDK CLI.

@pahud
Copy link
Contributor

pahud commented Apr 16, 2024

Not sure if you are on cdk.dev slack. Feel free to ping me on that slack if you need more ad-hoc discussion with me.

@justin-masse
Copy link
Author

I can handle it that way, but this means you MUST break your stacks apart and deploy KMS keys before secrets in order for the replica key to be present at the time when replica regions are populated. Is that correct?

@justin-masse
Copy link
Author

Will crossRegionReferences: true allow me to import a KMS key via ARN from another region? I'm still confused as to how you get the IKey object to provide in replica regions

@github-actions github-actions bot removed the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Apr 17, 2024
@dvd-88
Copy link

dvd-88 commented May 23, 2024

I am facing the same issue: I managed to create a secret replicated in two regions and the associated KMS key in the corresponding regions. However, the secret replicas are encrypted using the default encryption key, having the alias 'aws/secretsmanager'.

In order to update the encryption key, would it work to create an AwsCustomResource triggering a lambda function performing an update secret operation on the secret specifying the kms key id as input parameter using the aws sdk?

@dvd-88
Copy link

dvd-88 commented May 23, 2024

Unfortunatelly it seems that the update secret call has to be performed in the primary region. Trying in the secondary region it gives the following error:
Operation not permitted on a replica secret. Call must be made in primary secret's region.

The only other option that I see is to create an AWS custom resource in the primary region that performs a ReplicateSecretToRegionsCommand specifying the KmsKeyId and the Region in the request. However, the replicated key must exist in the secondary region before this call is performed. Thus it should require two deployments.

@dvd-88
Copy link

dvd-88 commented May 24, 2024

The second approach works!

I had to perform two deployments:

  1. Create the KMS key and replicate it cross-region assigning to it a human-readable alias, create the secret in the master region using the KMS key in the master region. The secret at this point should not be replicated cross-region otherwise the replicas will be encrypted with the default KMS key used by the secrets manager and they will not be re-encrypted performing the replicate secret to regions command using the SDK.
  2. Add an AWS custom resource that performs the replicate secret to regions command providing all the replica regions and the alias of the KMS key in the replicated regions.

@pahud pahud added p3 and removed p2 labels Jun 11, 2024
@justin-masse
Copy link
Author

@dvd-88 this is an option, but a custom resource for this seems a bit absurd. Especially just for the added complexity for an already complex scenario.

Likely this is just going to take some collaboration between the service teams for KMS/SecretsManager. There really shouldn't be this disparity between services imo

@dvd-88
Copy link

dvd-88 commented Jun 13, 2024

@justin-masse I agree with you that it should be easier to create replicated secrets encrypted with customer-managed keys.

However, it is really hard for me to find a "simple" solution to the circular dependency issue in CDK: the main stack containing the secret to be replicated depends on the secondary stacks that contain the customer-managed key replicas and of course, these stacks depend on the main one since the customer-managed key it has to exists before you create the replicas.
The only solution I can imagine is that everything can be managed automatically by Cloud Formation.

@moelasmar moelasmar added @aws-cdk/aws-kms Related to AWS Key Management and removed aws-cdk-lib Related to the aws-cdk-lib package labels Aug 28, 2024
@justin-masse
Copy link
Author

@dvd-88 yeah I don't know of a simple solution for this. The idea that some AWS services replicate from primary -> secondary and others replicate secondary -> from primary just really makes this frustrating.

This just happens to be the one weird scenario where this difference in replication collides due to these intermingling with KMS + Secrets.

I'm just trying to save time for future engineers (as i'm aware of this limitation now and it won't suck days of my time away anymore).

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. effort/medium Medium work item – several days of effort p3
Projects
None yet
Development

No branches or pull requests

4 participants