aws / aws-cdk

The AWS Cloud Development Kit is a framework for defining cloud infrastructure in code
https://aws.amazon.com/cdk
Apache License 2.0
11.56k stars 3.87k forks source link

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

Open tlakomy opened 1 year ago

tlakomy commented 1 year ago

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

peterwoodworth commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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: