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_secretsmanager: Make cross-account secret sharing easier with KMS Key alias #28284

Open
2 tasks done
pergardebrink opened this issue Dec 7, 2023 · 3 comments
Open
2 tasks done
Labels
@aws-cdk/aws-secretsmanager Related to AWS Secrets Manager effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p3

Comments

@pergardebrink
Copy link

pergardebrink commented Dec 7, 2023

Describe the feature

Short description

I'd like a way to use secret.grantRead(role) that would automatically generate the appropriate policy using the alias condition for cross-account secret sharing**

How it works without this feature

Let's say I have a secret in AccountA (111111111111) and what to share that with AccountB (222222222222).

Account A CDK Stack

const encryptionKey = new kms.Key(this, "Key", {
    alias = "mysecretkeyalias"
});

const secret = new secretsmanager.Secret(this, "Secret", {
    encryptionKey: encryptionKey,
    secretName: "MySecret"
});

const accountB = new iam.AccountPrincipal("222222222222");

secret.grantRead(accountB);

To consume this secret from AccountB, I'd need to grant access to both the secret and the kms key, but I'd like to use the alias instead of the key name for many reasons (easier configuration for multi-region for example as I wouldn't have to pass different key arns), which currently have to look something like this:

Account B CDK Stack

const role = new iam.Role(this, "ConsumerRole", {
    // Setup the role
});

const secret = secretsmanager.Secret.FromSecretAttributes(this, "Secret", {
    secretPartialArn: "arn:aws:secretsmanager:us-east-1:11111111111:secret:MySecret"
});

secret.grantRead(role);

role.addToPrincipalPolicy({
    resources: ["arn:aws:kms:us-east-1:11111111111:key/*"],
    actions: ["kms:decrypt"],
    conditions: {
        "ForAnyValue:StringEquals": {
            "kms:ResourceAliases": "alias/mysecretkeyalias",
        },
    },
}):

Use Case

When it's not the same team/people/company owning the source AWS account and/or the secret/key as the consuming account, it would be convenient if it's possible to reference the key by alias to ease configuration as well as making it possible to switch kms key for the secret without having to change on the consuming side.

For multi-region deployments, it's also better as I'd not have to specify multiple secrets + kms keys, but could instead use the secret partialarn and the kms key alias, which could be the same across all regions.

Proposed Solution

It would be nice if there was some way to specify the alias and that it would produce a policy with a kms key alias condition

const role = new iam.Role(this, "ConsumerRole", {
    // Setup the role
});

const kmsAlias = kms.Alias.FromAliasArn(this, "Alias", "arn:aws:kms:us-east-1:111111111111:alias/mysecretkeyalias");

const secret = secretsmanager.Secret.FromSecretAttributes(this, "Secret", {
    encryptionKey: kmsAlias,
    secretPartialArn: "arn:aws:secretsmanager:us-east-1:11111111111:secret:MySecret"
});

secret.grantRead(role);

Expected policy:

{
  "Version": "2012-10-17",
  "Statement": [
    {
      "Effect": "Allow",
      "Action": [
        "kms:Decrypt",
      ],
      "Resource": [
        "arn:aws:kms:us-east-1:11111111111:key/*",
      ],
      "Condition": {
        "ForAnyValue:StringLike": {
          "kms:ResourceAliases": "alias/mysecretkeyalias"
        }
      }
    }
  ]
}

Other Information

I assume there could be side-effects with my proposed solution that could cause this feature to not be possible or a good solution

Acknowledgements

  • I may be able to implement this feature request
  • This feature might incur a breaking change

CDK version used

2.114.1

Environment details (OS name and version, etc.)

Windows 11

@pergardebrink pergardebrink added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Dec 7, 2023
@github-actions github-actions bot added the @aws-cdk/aws-secretsmanager Related to AWS Secrets Manager label Dec 7, 2023
@pergardebrink
Copy link
Author

I realize that this is similar to to this issue #23545

I see benefits of only having to specify the accountId as opposed to the full alias arn, but then I assume that the region needs to be inferred as it might not always be the intention to give access to all keys with the same alias in all regions in the other account (least privilege). But there should also be a way to specify cross region access as well if so.

I might be able to create a PR if the api from either mine or the linked issue (or both) is considered good.

@pahud
Copy link
Contributor

pahud commented Dec 11, 2023

Thanks for the feedback and we appreciate your pull requests.

@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 Dec 11, 2023
@pergardebrink
Copy link
Author

pergardebrink commented Dec 12, 2023

@pahud thanks. I'm willing to create a pull request for this, but I have some questions on decisions from the past in the CDK constructs for Alias, to make sure I don't break things, or that I break things in a good way (as I think there's going to have to be breaking changes here).

My main thoughts are about the following comment that suggest that it was intentional not to implement the grant* methods for an alias that is not created by the stack (an imported alias). Or at least that it was not given priority to implement it?

* This Alias will not have a direct reference to the KMS Key, so addAlias and grant* methods are not supported.

My proposal for Secret:

If the imported secret with FromSecretAttributes is cross-account, it won't add the ViaService condition, but instead defer grant the KMS permission to the IKey grant* functions (see below)

My proposal for KMS Key Alias:

  1. Add the following overload to Alias:
kms.Alias.fromAliasArn(scope: Construct, id: string, aliasArn: string): IAlias
  • Any call to grant* methods on the returned Alias will add to the principal policy (since we do not have access to the key we cannot add resource policy)
  • The policy will look like the "Expected policy" in my first post in this issue and use the kms:ResourceAliases condition with a resource based on the arn of the alias (same account and region)
  1. The kms.Alias.fromAliasName will start implementing the same grant* logic as the fromAliasArn in 1. (breaking change)
  2. Inspired by the issue (aws-kms): (Need cross-account ability in Alias construct) #23545, kms.Alias.fromAliasAttributes will have more options in the AliasAttributes (comments condensed here for readability);
// Properties of a reference to an existing KMS Alias
export interface AliasAttributes {
  // Specifies the alias name. This value must begin with alias/ followed by a name (i.e. alias/ExampleAlias)
  readonly aliasName: string;
  // Specifies the account where the alias is defined.
  readonly account: string;
  // Specifies the region where the alias is defined.
  readonly region: string;
  // The customer master key (CMK) to which the Alias refers (optional)
  readonly aliasTargetKey: IKey;
}

For 3. I think there's some things that would need to be addressed:

  • If aliasTargetKey is specified, all grant* methods would use the current behaviors, and delegate the grant* call to the actual KMS key and not the alias
  • if aliasTargetKey is not specified, all grant* methods would be using the same logic as 1. and 2. to use the alias condition instead

I can start in parallell to see if I can create a draft PR based on the above, or if opinions and concerns appear before that I can adapt.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-secretsmanager Related to AWS Secrets Manager effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p3
Projects
None yet
2 participants