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: Support KMS CMK #7142

Closed
2 tasks
cmckni3 opened this issue Apr 2, 2020 · 3 comments · Fixed by #7425
Closed
2 tasks

dynamodb: Support KMS CMK #7142

cmckni3 opened this issue Apr 2, 2020 · 3 comments · Fixed by #7425
Assignees
Labels
@aws-cdk/aws-dynamodb Related to Amazon DynamoDB effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. in-progress This issue is being actively worked on.

Comments

@cmckni3
Copy link
Contributor

cmckni3 commented Apr 2, 2020

Support KMS server side encryption type and KMS Key Id.

Use Case

Use KMS customer managed keys with DynamoDB tables.

Proposed Solution

Add properties to DynamoDB Table construct and TableProps.

Other

TableProps should support the other SSE options in CloudFormation.

Current workaround is escape hatch.

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

This is a 🚀 Feature Request

@cmckni3 cmckni3 added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Apr 2, 2020
@SomayaB SomayaB added the @aws-cdk/aws-dynamodb Related to Amazon DynamoDB label Apr 3, 2020
@cmckni3
Copy link
Contributor Author

cmckni3 commented Apr 6, 2020

Escape hatch:

import * as cdk from '@aws-cdk/core';

import { AttributeType, BillingMode, CfnTable, Table } from '@aws-cdk/aws-dynamodb';
import { Key } from '@aws-cdk/aws-kms';

export class CdkDynamo7142Stack extends cdk.Stack {
  constructor(scope: cdk.Construct, id: string, props?: cdk.StackProps) {
    super(scope, id, props);

    const myKey = new Key(this, 'MyKey');

    const table = new Table(this, 'MyTable', {
      tableName: 'my-table',
      partitionKey: { name: 'pk', type: AttributeType.STRING },
      billingMode: BillingMode.PAY_PER_REQUEST,
    });

    const cfnTable = table.node.defaultChild as CfnTable;

    cfnTable.addPropertyOverride('SSESpecification', {
      SSEEnabled: true,
      KMSMasterKeyId: myKey.keyId,
      SSEType: 'KMS',
    });
  }
}

@RomainMuller RomainMuller added 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 Apr 6, 2020
@shreyasdamle
Copy link
Contributor

👋 I may be able to implement this feature request. Here's what I'm thinking:

  1. Change serverSideEncryption from boolean to TableEncryption enum and introduce new prop for encryptionKey
  /**
   * Whether server-side encryption with an AWS managed customer master key is enabled.
   * @default - server-side encryption is enabled with an AWS owned customer master key
   */
  readonly serverSideEncryption?: TableEncryption;

  /**
   * External KMS key to use for table encryption.
   *
   * @default - If encryption is set to "Customer managed" and this property is undefined,
   * a new KMS key will be created and associated with this table.
   */
  readonly encryptionKey?: kms.IKey;
export enum TableEncryption {
  /**
   * Server-side KMS encryption with a customer master key managed by customer.
   * If `encryptionKey` is specified, this key will be used, otherwise, one will be defined.
   */
  Customer_Managed = 'Customer_Managed',
  /**
   * Server-side KMS encryption with a master key managed by AWS.
   */
  AWS_Managed = 'AWS_Managed',
  /**
   * Server-side KMS encryption with a master key owned by AWS.
   */
  Default = 'AWS_Owned'
}
  1. There will be a need to add a new method to parse encryption options
/**
   * Set up key properties and return the Table encryption property from the
   * user's configuration.
   */
  private parseEncryption(props:TableProps): {sseSpecification: CfnTable.SSESpecificationProperty, encryptionKey?: kms.IKey} {
    let encryptionType = props.serverSideEncryption;

    if (encryptionType === undefined) {
      encryptionType = props.encryptionKey ? TableEncryption.Customer_Managed : TableEncryption.Default;
    }
    
    if (encryptionType !== TableEncryption.Customer_Managed && props.encryptionKey) {
      throw new Error(`encryptionKey is specified, so 'requireServerSideEncryption' must be set to KMS (value: ${encryptionType})`);
    }

    if (encryptionType === TableEncryption.Customer_Managed) {
      const encryptionKey = props.encryptionKey || new kms.Key(this, 'Key', {
          description: `Created by ${this.node.path}`,
          enableKeyRotation: true
      });

      const sseSpecification =  {
          sseEnabled: true,
          KMSMasterKeyId: encryptionKey.keyArn,
          SSEType: 'KMS'
        }

      return { sseSpecification, encryptionKey };
    }

    if (encryptionType === TableEncryption.AWS_Managed) {
      const sseSpecification = {
        sseEnabled: true,
        SSEType: 'KMS'
      }
      return { sseSpecification };
    }
    if (encryptionType === TableEncryption.Default) {
      const sseSpecification = {
        sseEnabled: false,
      }
      return { sseSpecification };
    }
    throw new Error(`Unexpected 'encryptionType': ${encryptionType}`);
  }
  1. Call this in constructor:
const { sseSpecification, encryptionKey } = this.parseEncryption(props);

    this.table = new CfnTable(this, 'Resource', {
      tableName: this.physicalName,
      keySchema: this.keySchema,
      attributeDefinitions: this.attributeDefinitions,
      globalSecondaryIndexes: Lazy.anyValue({ produce: () => this.globalSecondaryIndexes }, { omitEmptyArray: true }),
      localSecondaryIndexes: Lazy.anyValue({ produce: () => this.localSecondaryIndexes }, { omitEmptyArray: true }),
      pointInTimeRecoverySpecification: props.pointInTimeRecovery ? { pointInTimeRecoveryEnabled: props.pointInTimeRecovery } : undefined,
      billingMode: this.billingMode === BillingMode.PAY_PER_REQUEST ? this.billingMode : undefined,
      provisionedThroughput: this.billingMode === BillingMode.PAY_PER_REQUEST ? undefined : {
        readCapacityUnits: props.readCapacity || 5,
        writeCapacityUnits: props.writeCapacity || 5
      },
      sseSpecification: sseSpecification,
      streamSpecification,
      timeToLiveSpecification: props.timeToLiveAttribute ? { attributeName: props.timeToLiveAttribute, enabled: true } : undefined
    });

this.encryptionKey = encryptionKey;
  1. Modify grant methods like this:
  public grant(grantee: iam.IGrantable, tableActions: string[], keyActions: string[], ...otherResourceArns: string[]) {
    const resources = [this.tableArn, Lazy.stringValue({ produce: () => this.hasIndex ? `${this.tableArn}/index/*` : Aws.NO_VALUE }), ...otherResourceArns];
    const ret = iam.Grant.addToPrincipal({
      grantee,
      actions: tableActions,
      resourceArns: resources,
      scope: this
    });
    if (this.encryptionKey) {
      this.encryptionKey.grant(grantee, ...keyActions);
    }
    return ret;
  }

  public grantReadData(grantee: iam.IGrantable, objectsKeyPattern: any = '*'): iam.Grant {
    return this.grant(grantee, perms.READ_DATA_ACTIONS, perms.KEY_READ_ACTIONS, this.arnForObjects(objectsKeyPattern));
  }

What do you think about this approach? This will incur a breaking change.

@RomainMuller
Copy link
Contributor

RomainMuller commented Apr 20, 2020

@shreyasdamle - you cannot change serverSideEncryption's type, for this is a breaking change. You can @deprecate it and introduce a new, more versatile field.

The rest of the plan does however look sane. We might need to consider implications on Global Tables, though (I haven't read into your PR enough just yet to know whether we can or have to do anything there).

@SomayaB SomayaB added the in-progress This issue is being actively worked on. label Apr 20, 2020
@mergify mergify bot closed this as completed in #7425 May 14, 2020
mergify bot pushed a commit that referenced this issue May 14, 2020
Deprecated `serverSideEncryption` and introduced new `encryption` and
`encryptionKey` properties to support Customer-Managed CMK on
DynamoDB Tables. The purpose-specific `grant*` methods (`grantReadData`, ...)
have been updated to automatically extend the relevant KMS key grants
transparently, while the generic `grant` methods are only adding grants on the
table or stream (depending on the `grant` method used) itself.

The new `encryption` and `encryptionKey` properties are mutually exclusive
with the now-deprecated `serverSideEncryption` property.

Fixes #7142

Co-Authored-By: rmuller@amazon.com
karupanerura pushed a commit to karupanerura/aws-cdk that referenced this issue May 22, 2020
Deprecated `serverSideEncryption` and introduced new `encryption` and
`encryptionKey` properties to support Customer-Managed CMK on
DynamoDB Tables. The purpose-specific `grant*` methods (`grantReadData`, ...)
have been updated to automatically extend the relevant KMS key grants
transparently, while the generic `grant` methods are only adding grants on the
table or stream (depending on the `grant` method used) itself.

The new `encryption` and `encryptionKey` properties are mutually exclusive
with the now-deprecated `serverSideEncryption` property.

Fixes aws#7142

Co-Authored-By: rmuller@amazon.com
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 effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. in-progress This issue is being actively worked on.
Projects
None yet
4 participants