Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

kms: All principals in a PolicyStatement must have the same Conditions for cross-account policy #29125

Closed
pahud opened this issue Feb 15, 2024 · 3 comments
Assignees
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 p1

Comments

@pahud
Copy link
Contributor

pahud commented Feb 15, 2024

Describe the bug

CDK synth is having Resolution error: All principals in a PolicyStatement must have the same Conditions by adding a cross-account plicy.

Expected Behavior

synthesize with no error

Current Behavior

Error: Resolution error: Resolution error: Resolution error: All principals in a PolicyStatement must have the same Conditions (got '{"StringEquals":{"kms:ViaService":"secretsmanager.us-east-1.amazonaws.com"}}' and '{}'). Use multiple statements instead..
Object creation stack:
  at stack traces disabled..
    at PolicyStatement.addPrincipalConditions (/projects/issue-triage/node_modules/aws-cdk-lib/aws-iam/lib/policy-statement.js:2:7201)
    at PolicyStatement.addPrincipals (/projects/issue-triage/node_modules/aws-cdk-lib/aws-iam/lib/policy-statement.js:2:2407)
    at new PolicyStatement (/projects/issue-triage/node_modules/aws-cdk-lib/aws-iam/lib/policy-statement.js:2:663)
    at PolicyStatement.copy (/projects/issue-triage/node_modules/aws-cdk-lib/aws-iam/lib/policy-statement.js:2:6086)
    at mergeIfCombinable (/projects/issue-triage/node_modules/aws-cdk-lib/aws-iam/lib/private/merge-statements.js:1:1847)
    at onePass (/projects/issue-triage/node_modules/aws-cdk-lib/aws-iam/lib/private/merge-statements.js:1:1032)
    at mergeStatements (/projects/issue-triage/node_modules/aws-cdk-lib/aws-iam/lib/private/merge-statements.js:1:660)
    at PolicyDocument._maybeMergeStatements (/projects/issue-triage/node_modules/aws-cdk-lib/aws-iam/lib/policy-document.js:1:3033)
    at PolicyDocument.resolve (/projects/issue-triage/node_modules/aws-cdk-lib/aws-iam/lib/policy-document.js:1:1755)
    at RememberingTokenResolver.resolveToken (/projects/issue-triage/node_modules/aws-cdk-lib/core/lib/resolvable.js:1:1401)

Reproduction Steps

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

    const dummyKey = new kms.Key(this, 'Key', {
      alias: 'keyalias',
      enableKeyRotation: true,
      keyUsage: KeyUsage.ENCRYPT_DECRYPT,
      keySpec: KeySpec.SYMMETRIC_DEFAULT,
    });


    new secrets.Secret(this, 'Secret', {
      encryptionKey: dummyKey,
    });

    const crossAccountAccessPolicy = new iam.PolicyStatement({
      actions: [
        'kms:CreateGrant',
        'kms:Decrypt',
        'kms:DescribeKey',
        'kms:Encrypt',
        'kms:GenerateDataKey*',
        'kms:ReEncrypt*'
      ],
      principals: [
        new iam.AccountPrincipal('123456789012'),
      ],
      resources: ['*'],
      conditions: {
        'StringEquals': { 'kms:ViaService': 'secretsmanager.us-east-1.amazonaws.com'}
      }
    })

    dummyKey.addToResourcePolicy(crossAccountAccessPolicy);

  }
}
     

Possible Solution

No response

Additional Information/Context

No response

CDK CLI Version

2.126.0 (build fb74c41)

Framework Version

No response

Node.js Version

v20.6.1

OS

linux

Language

TypeScript

Language Version

No response

Other information

No response

@pahud pahud added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Feb 15, 2024
@github-actions github-actions bot added the @aws-cdk/aws-kms Related to AWS Key Management label Feb 15, 2024
@pahud
Copy link
Contributor Author

pahud commented Feb 15, 2024

internal tracking: V1241367978

@pahud pahud added p1 effort/medium Medium work item – several days of effort needs-review and removed needs-triage This issue or PR still needs to be triaged. labels Feb 15, 2024
@Leo10Gama Leo10Gama self-assigned this Jul 12, 2024
@Leo10Gama Leo10Gama removed their assignment Aug 6, 2024
@xazhao xazhao self-assigned this Aug 28, 2024
@xazhao
Copy link
Contributor

xazhao commented Aug 28, 2024

Can't reproduce this issue. Looking at the internal tracking ticket, this happens when encryptionKey: dummyKey creates the policy with same action/condition as crossAccountAccessPolicy which will cause conflicts.
When I run the synth, condition created by encryptionKey: dummyKey is

Condition:
  StringEquals:
    kms:ViaService:
      Fn::Join:
        - ""
        - - secretsmanager.
          - Ref: AWS::Region
          - .amazonaws.com

while the condition in crossAccountAccessPolicy is

{
  'StringEquals': { 'kms:ViaService': 'secretsmanager.us-east-1.amazonaws.com'}
}

The region is not resolved in the output template hence not causing conflicts. I'm going to close the issue for now. If this issue still exists, feel free to re-open it.

@xazhao xazhao closed this as completed Aug 28, 2024
Copy link

Comments on closed issues and PRs are hard for our team to see.
If you need help, please open a new issue that references this one.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 28, 2024
# for free to subscribe to this conversation on GitHub. Already have an account? #.
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 p1
Projects
None yet
Development

No branches or pull requests

4 participants