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

feat(stepfunctions): add support for EncryptionConfiguration #30959

Merged
merged 47 commits into from
Sep 20, 2024

Conversation

VaidSaraswat
Copy link
Contributor

@VaidSaraswat VaidSaraswat commented Jul 26, 2024

Reason for this change

Allow customers to specify a customer managed KMS key and data key reuse period to encrypt state machine definition and execution history and activity inputs. The underlying AWS::StepFunctions::StateMachine and AWS::StepFunctions::Activity resources currently expose this through an optional EncryptionConfiguration property.

Description of changes

Activity and StateMachine accept a new field called encryptionConfiguration of type EncryptionConfiguration in their respective props. We have two separate classes which inherit the base class: 1. CustomerManagedEncryptionConfiguration 2. AwsOwnedEncryptionConfiguration

CustomerManagedEncryptionConfiguration:

kmsKey

  • Type: IKey
  • Description: Symmetric customer managed KMS key for server-side encryption of the state machine definition and execution history (when provided in StateMachine Props), and Activity Inputs (when provided in Activity Props)
  • Default: undefined

kmsDataKeyReusePeriodSeconds:

  • Type: Duration
  • Description: Maximum duration that Step Functions will reuse customer managed data keys. When the period expires, Step Functions will call GenerateDataKey. Must be a value between 60 and 900 seconds.
  • Default: 300 sec

AwsOwnedEncryptionConfiguration

  • Doesn't accept any fields

Permission Changes

Activity:

  • When the customer provides kmsKey? the key policy will be updated with the following policy statement:
{
          "Effect": "Allow",
          "Principal": {
              "Service": "states.amazonaws.com"
          },
          "Action": [
              "kms:Decrypt",
              "kms:GenerateDataKey"
          ],
          "Resource": "*",
          "Condition": {
              "StringEquals": {
                  "kms:EncryptionContext:aws:states:activityArn": "arn:aws:states:<region><account_id>:activity:<activity_name>"
              }
          }
}

StateMachine:

  • When the customer provides kmsKey? the key policy will be updated with the following policy statement:
{
          "Effect": "Allow",
          "Principal": {
              "Service": "states.amazonaws.com"
          },
          "Action": [
              "kms:Decrypt",
              "kms:GenerateDataKey"
          ],
          "Resource": "*",
          "Condition": {
              "StringEquals": {
                  "kms:EncryptionContext:aws:states:stateMachineArn": "arn:aws:states:<region><account_id>:stateMachine:<statemachine_name>"
              }
          }
}
  • If the state machine contains an activity which uses KMS encryption, the state machine execution role will be updated with the following permissions
{
            "Effect": "Allow",
            "Principal": {
                "Service": "states.amazonaws.com"
            },
            "Action": [
                "kms:Decrypt",
                "kms:GenerateDataKey"
            ],
            "Resource": "<activity_kmskey_arn>",
}
  • Customers have the option to encrypt data sent to CloudWatch Logs. To support this, if the customer provides both the kmsKey? and logs? prop, the following key policy statement will be added to the key used by the StateMachine:
{
            "Effect": "Allow",
            "Principal": {
                "Service": "delivery.logs.amazonaws.com"
            },
            "Action": [
                "kms:Decrypt",
            ]
}

In addition the execution role will be updated to include a separate policy that includes kms actions and encryption context for logging (otherwise customer will not see logs)

{
            "Effect": "Allow",
            "Action": [
                "kms:GenerateDataKey"
            ],
            "Resource": "<state_machine_kms_key_arn>",
            "Condition": {
                "StringEquals": {
                    "kms:EncryptionContext:SourceArn": "arn:aws:logs:<region><account_id>:*"
                }
            }
}

Description of how you validated changes

Unit Test (scenarios):

  • Activity
    • Creating an Activity with a KMS Key and without specifying kmsDataKeyReusePeriodSeconds defaults to 300 secs
    • Creating an Activity with a KMS Key results in the correct KMS key policy being generated which allows only the Activity to perform 'kms:Decrypt', 'kms:GenerateDataKey' actions on the associated KMS key.
    • Creating an Activity with invalid kmsDataKeyReusePeriodSeconds throws an error
    • Creating an Activity with AwsOwnedEncryptionConfiguration uses AWS_OWNED_KEY encryption type.
  • StateMachine
    • Creating a State Machine with a KMS Key allows only StateMachine execution role to perform 'kms:Decrypt', 'kms:GenerateDataKey' actions on the key.
    • Creating a State Machine with logs? and kmsKey?:
      • Separate IAM policy statement (using encryption context for logging) being generated for the State Machine execution role
      • KMS key policy statement which enables log service delivery for integrations
    • Creating a State Machine which invokes an Activity using KMS encryption results in a IAM policy generated which allows the execution role to perform 'kms:Decrypt', 'kms:GenerateDataKey' actions on the Activity KMS key
    • Creating a StateMachine with a KMS Key and without specifying kmsDataKeyReusePeriodSeconds defaults to 300 secs
    • Creating a State Machine with a KMS key and invalid kmsDataKeyReusePeriodSeconds throws a validation error
    • Creating a StateMachine with AwsOwnedEncryptionConfiguration uses AWS_OWNED_KEY encryption type.

Integration tests

  • Create a State Machine and Activity which both have encryption enabled and assert the activity input is correct when calling getActivityTask API
  • Create a State Machine with encryption and logging enabled. Ensure decrypted logs are being pushed to the log group.

Code samples

  • Creating an Activity with Encryption using a Customer Managed Key

const kmsKey = new kms.Key(this, 'Key');
const activity = new sfn.Activity(this, 'ActivityWithCMKEncryptionConfiguration', {
  activityName: 'ActivityWithCMKEncryptionConfiguration',
  encryptionConfiguration: new sfn.CustomerManagedEncryptionConfiguration(kmsKey, cdk.Duration.seconds(75))
});
  • Creating a StateMachine with Encryption using a Customer Managed Key

const kmsKey = new kms.Key(this, 'Key');
const stateMachine = new sfn.StateMachine(this, 'StateMachineWithCMKEncryptionConfiguration', {
  stateMachineName: 'StateMachineWithCMKEncryptionConfiguration',
  definitionBody: sfn.DefinitionBody.fromChainable(sfn.Chain.start(new sfn.Pass(this, 'Pass'))),
  stateMachineType: sfn.StateMachineType.STANDARD,
  encryptionConfiguration: new sfn.CustomerManagedEncryptionConfiguration(kmsKey, cdk.Duration.seconds(60)),
});
  • Creating a StateMachine with CWL Encryption using a Customer Managed Key

const stateMachineKmsKey = new kms.Key(this, 'StateMachine Key');
const logGroupKey = new kms.Key(this, 'LogGroup Key');

// Required KMS key policy to enrypt the CloudWatch log group
logGroupKey.addToResourcePolicy(new cdk.aws_iam.PolicyStatement({
  resources: ['*'],
  actions: ['kms:Encrypt*', 'kms:Decrypt*', 'kms:ReEncrypt*', 'kms:GenerateDataKey*', 'kms:Describe*'],
  principals: [new cdk.aws_iam.ServicePrincipal(`logs.${cdk.Stack.of(this).region}.amazonaws.com`)],
  conditions: {
    ArnEquals: {
      'kms:EncryptionContext:aws:logs:arn': cdk.Stack.of(this).formatArn({
        service: 'logs',
        resource: 'log-group',
        sep: ':',
        resourceName: '/aws/vendedlogs/states/MyLogGroup',
      }),
    },
  },
}));

const logGroup = new logs.LogGroup(this, 'MyLogGroup', {
  logGroupName: '/aws/vendedlogs/states/MyLogGroup',
  encryptionKey: logGroupKey,
});

const stateMachine = new sfn.StateMachine(this, 'StateMachineWithCMKWithCWLEncryption', {
  stateMachineName: 'StateMachineWithCMKWithCWLEncryption',
  definitionBody: sfn.DefinitionBody.fromChainable(sfn.Chain.start(new sfn.Pass(this, 'PassState', {
    result: sfn.Result.fromString('Hello World'),
  }))),
  stateMachineType: sfn.StateMachineType.STANDARD,
  encryptionConfiguration: new sfn.CustomerManagedEncryptionConfiguration(stateMachineKmsKey)
  logs: {
    destination: logGroup,
    level: sfn.LogLevel.ALL,
    includeExecutionData: true,
  },
});

Checklist


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

@github-actions github-actions bot added p2 beginning-contributor [Pilot] contributed between 0-2 PRs to the CDK labels Jul 26, 2024
@aws-cdk-automation aws-cdk-automation requested a review from a team July 26, 2024 18:32
Copy link
Collaborator

@aws-cdk-automation aws-cdk-automation left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.

@VaidSaraswat VaidSaraswat changed the title feat(stepfunctions): Add Support for EncryptionConfiguration feat(stepfunctions): add Support for EncryptionConfiguration Jul 26, 2024
@VaidSaraswat VaidSaraswat changed the title feat(stepfunctions): add Support for EncryptionConfiguration feat(stepfunctions): add support for EncryptionConfiguration Jul 26, 2024
@aws-cdk-automation aws-cdk-automation dismissed their stale review August 1, 2024 01:38

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

@aws-cdk-automation aws-cdk-automation added the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Aug 1, 2024
@aws-cdk-automation aws-cdk-automation removed the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Aug 2, 2024
@VaidSaraswat VaidSaraswat force-pushed the feat-encryption-configuration branch 3 times, most recently from ce4774b to a3fecf0 Compare August 7, 2024 20:59
@VaidSaraswat VaidSaraswat force-pushed the feat-encryption-configuration branch from a3fecf0 to 09350fd Compare August 7, 2024 22:57
@wong-a
Copy link
Contributor

wong-a commented Aug 7, 2024

Can you describe how the StateMachine and Activity constructs are actually being changed in the Description of changes section of the PR description?

Perhaps an example of these properties in use as well.

@VaidSaraswat VaidSaraswat force-pushed the feat-encryption-configuration branch from 09350fd to 971290d Compare August 8, 2024 05:38
Copy link
Contributor

@wong-a wong-a left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comments on docs

packages/aws-cdk-lib/aws-stepfunctions/README.md Outdated Show resolved Hide resolved
VaidSaraswat and others added 2 commits August 8, 2024 16:53
Co-authored-by: Adam Wong <55506708+wong-a@users.noreply.github.com>
Co-authored-by: Adam Wong <55506708+wong-a@users.noreply.github.com>
packages/aws-cdk-lib/aws-stepfunctions/README.md Outdated Show resolved Hide resolved
* @default Duration.seconds(300)
*/
public readonly kmsDataKeyReusePeriodSeconds?;
constructor(kmsKey: kms.IKey, kmsDataKeyReusePeriodSeconds?: cdk.Duration) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: new lines?

packages/aws-cdk-lib/aws-stepfunctions/lib/util.ts Outdated Show resolved Hide resolved
VaidSaraswat and others added 4 commits September 2, 2024 11:15
Co-authored-by: Adam Wong <55506708+wong-a@users.noreply.github.com>
Co-authored-by: Adam Wong <55506708+wong-a@users.noreply.github.com>
…nagedEncryptionConfiguration & AwsOwnedEncryptionConfiguration extend

- Created new class CustomerManagedEncryptionConfiguration
- Created new class AwsOwnedEncryptionConfiguration
- Updated unit and integ tests to use either CustomerManagedEncryptionConfiguration or AwsOwnedEncryptionConfiguration when setting encryptionConfiguration
- Updated README to use CustomerManagedEncryptionConfiguration or AwsOwnedEncryptionConfiguration in the code samples
- Updated README to include specific comment on encrypting log group with link to relevant documentation
- Added example in README for switching between CustomerManagedEncryptionConfiguration and AwsOwnedEncryptionConfiguration
Copy link
Contributor

@paulhcsun paulhcsun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few more small changes. Also please add a line break to the end of the files that do not have one.

packages/aws-cdk-lib/aws-stepfunctions/lib/util.ts Outdated Show resolved Hide resolved
packages/aws-cdk-lib/aws-stepfunctions/lib/util.ts Outdated Show resolved Hide resolved
- Moved util.ts to a private directory
- Updated buildEncryptionConfiguration to to accept type EncryptionConfiguration
- Rename ckmencryptionconfiguration to customer-managed-key-encryption-configuration
- Rename aokencryptionconfiguration to aws-owned-key-encryption-configuration
- Rename encryptionconfiguration to encryption-configuration
@mergify mergify bot dismissed paulhcsun’s stale review September 6, 2024 00:28

Pull request has been modified.

@paulhcsun paulhcsun added needs-security-review Related to feature or issues that needs security review pr/do-not-merge This PR should not be merged at this time. labels Sep 6, 2024
@aws-cdk-automation
Copy link
Collaborator

This PR has been in the CHANGES REQUESTED state for 3 weeks, and looks abandoned. To keep this PR from being closed, please continue work on it. If not, it will automatically be closed in a week.

Copy link
Contributor

@paulhcsun paulhcsun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes have been appsec approved. Great work on this @VaidSaraswat!

Thanks to everyone for helping with the review on this!

@paulhcsun
Copy link
Contributor

@Mergifyio update

@paulhcsun paulhcsun removed pr/do-not-merge This PR should not be merged at this time. needs-security-review Related to feature or issues that needs security review labels Sep 20, 2024
Copy link
Contributor

mergify bot commented Sep 20, 2024

update

☑️ Nothing to do

  • #commits-behind > 0 [📌 update requirement]
  • -closed [📌 update requirement]
  • -conflict [📌 update requirement]
  • queue-position = -1 [📌 update requirement]

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
  • Commit ID: 17641e4
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

Copy link
Contributor

mergify bot commented Sep 20, 2024

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

@mergify mergify bot merged commit b49032b into aws:main Sep 20, 2024
16 checks passed
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 Sep 20, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
@aws-cdk/aws-stepfunctions Related to AWS StepFunctions beginning-contributor [Pilot] contributed between 0-2 PRs to the CDK p2
Projects
None yet
Development

Successfully merging this pull request may close these issues.