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

(DynamoDB): Narrow global table policy permissions to use specific actions instead of a wildcard #23529

Open
tlakomy opened this issue Jan 2, 2023 · 4 comments
Labels
@aws-cdk/aws-dynamodb Related to Amazon DynamoDB ddb-legacy-table This issue has to do with DynamoDB's legacy Table construct. Close after migration guide is out. effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. p2

Comments

@tlakomy
Copy link
Contributor

tlakomy commented Jan 2, 2023

Describe the bug

When using Amazon DynamoDB Global Tables with AWS CDK as described in https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_dynamodb-readme.html#amazon-dynamodb-global-tables the generated IAM policy fails the [IAM.21] IAM customer managed policies that you create should not allow wildcard actions for services AWS Foundational Security Best Practices controls check.

SecurityHub does offer remediation instructions but I believe that CDK should not create non-SecurityHub compliant policies by default.

Expected Behavior

When using replicationRegions prop in Table construct the generated IAM policy should be fully SecurityHub compliant.

Current Behavior

The generated rule fails the following check:

[IAM.21] This control checks whether the IAM identity-based custom policies have Allow statements that grant permissions for all actions on a service. The control fails if any policy statement includes "Effect": "Allow" with "Action": "Service:*".

Example generated IAM rule looks like this:

{
    "Version": "2012-10-17",
    "Statement": [
        {
            "Action": "dynamodb:*",
            "Resource": [
                "arn:aws:dynamodb:us-east-1:ACCOUNT_NUMBER:table/TABLE_NAME"
            ],
            "Effect": "Allow"
        },
        {
            "Action": "dynamodb:*",
            "Resource": "arn:aws:dynamodb:us-west-2:ACCOUNT_NUMBER:table/TABLE_NAME",
            "Effect": "Allow"
        }
    ]
}

Reproduction Steps

Provision a DynamoDB table, for instance using the following snippet:

const myTable = new Table(this, 'my-table', {
        billingMode: BillingMode.PAY_PER_REQUEST,
        partitionKey: {
          name: "pk",
          type: AttributeType.STRING,
        },
        removalPolicy: cdk.RemovalPolicy.RETAIN,
        sortKey: {
          name: "sk",
          type: AttributeType.STRING,
        },
        pointInTimeRecovery: true,
        replicationRegions: ['us-west-2'],
});

Go to SecurityHub and notice that myTable will trigger a IAM customer managed policies that you create should not allow wildcard actions for services low severity check from AWS Foundational Security Best Practices v1.0.0 standard.

Possible Solution

Scope down the generated policy in order to avoid granting permission for all actions on provisioned DDB table.

According to Using IAM with global tables documentation, only the following permissions are required:

Screenshot 2023-01-02 at 11 43 11

Then again, a comment in the CDK codebase seems to suggest that this documentation is incorrect (?)

https://github.com/aws/aws-cdk/blob/main/packages/@aws-cdk/aws-dynamodb/lib/table.ts#L1621

Additional Information/Context

No response

CDK CLI Version

2.55.1

Framework Version

No response

Node.js Version

16.16.0

OS

MacOS 13.1 (22C65)

Language

Typescript

Language Version

No response

Other information

No response

@tlakomy tlakomy added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jan 2, 2023
@github-actions github-actions bot added the @aws-cdk/aws-dynamodb Related to Amazon DynamoDB label Jan 2, 2023
@peterwoodworth peterwoodworth added feature-request A feature should be added or improved. and removed bug This issue is a bug. labels Jan 4, 2023
@peterwoodworth peterwoodworth changed the title (DynamoDB): DynamoDB Global Tables generates policies not compliant with SecurityHub (DynamoDB): Narrow global table policy permissions to use specific actions instead of a wildcard Jan 4, 2023
@peterwoodworth peterwoodworth added p2 effort/small Small work item – less than a day of effort and removed needs-triage This issue or PR still needs to be triaged. labels Jan 4, 2023
@peterwoodworth
Copy link
Contributor

I believe that CDK should not create non-SecurityHub compliant policies by default.

We don't claim to support this, and also won't be doing this by default for a few reasons. Doing this by default would incur additional cost in some cases, additionally there are different levels of security standards which can be enabled. On top of this, there's no guarantee that SecurityHub rules won't change over time, which would come with a maintenance cost. A combination of CFN Guard and aspects may be a way to address this down the line, see this RFC for more info

As for this specific issue, I changed it to a feature request to narrow the permissions to only what is necessary just in this case. Thanks for the request 🙂

@clueleaf
Copy link
Contributor

clueleaf commented Jan 5, 2023

I found the code comments saying:

// Documentation at https://docs.aws.amazon.com/amazondynamodb/latest/developerguide/V2gt_IAM.html
// is currently incorrect. AWS Support recommends `dynamodb:*` in both source and destination regions

https://github.com/aws/aws-cdk/blob/main/packages/%40aws-cdk/aws-dynamodb/lib/table.ts#L1621-L1622

Does anyone have any idea where did the AWS Support recommendation come from?

@tlakomy
Copy link
Contributor Author

tlakomy commented Jan 5, 2023

We don't claim to support this, and also won't be doing this by default for a few reasons. Doing this by default would incur additional cost in some cases, additionally there are different levels of security standards which can be enabled. On top of this, there's no guarantee that SecurityHub rules won't change over time, which would come with a maintenance cost. A combination of CFN Guard and aspects may be a way to address this down the line, see this RFC for more info

Gotcha! - what I'd love to see is an opt-in way of telling CDK "do not deploy anything not compatible with XYZ SecurityHub standards". Thank you for the link to RFC, excited to see how it's going to evolve down the line 👀

@rix0rrr
Copy link
Contributor

rix0rrr commented Sep 21, 2023

This issue was for the existing Table construct, which used custom resources to implement table replication. We no longer recommend the use of the Table construct.

Instead, the TableV2 construct has been released in 2.95.1 (#27023) which maps to the AWS::DynamoDB::GlobalTable resource, has better support for replication and does not suffer from the issue described here.


Be aware that there are additional deployment steps involved in a migration from Table to TableV2. You need to do a RETAIN deployment, a delete deployment, then change the code to use TableV2 and then use cdk import. A link to a full guide will be posted once it is available.

Here are some other resources to get you started (using CfnGlobalTable instead of TableV2) if you want to get going on the migration:

@rix0rrr rix0rrr added the ddb-legacy-table This issue has to do with DynamoDB's legacy Table construct. Close after migration guide is out. label Sep 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-dynamodb Related to Amazon DynamoDB ddb-legacy-table This issue has to do with DynamoDB's legacy Table construct. Close after migration guide is out. effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. p2
Projects
None yet
Development

No branches or pull requests

5 participants