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(kinesisfirehose-alpha): enable server-side encryption for delivery stream by default #31858

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 19 additions & 1 deletion packages/@aws-cdk/aws-kinesisfirehose-alpha/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ before being written to the service's internal storage layer and decrypted after
received from the internal storage layer. The service manages keys and cryptographic
operations so that sources and destinations do not need to, as the data is encrypted and
decrypted at the boundaries of the service (i.e., before the data is delivered to a
destination). By default, delivery streams do not have SSE enabled.
destination). By default, delivery streams have SSE enabled with an AWS-owned key.

The Key Management Service keys (KMS keys) used for SSE can either be AWS-owned or
customer-managed. AWS-owned KMS keys are created, owned and managed by AWS for use in
Expand Down Expand Up @@ -172,6 +172,24 @@ new firehose.DeliveryStream(this, 'Delivery Stream with Customer Managed and Pro
See: [Data Protection](https://docs.aws.amazon.com/firehose/latest/dev/encryption.html)
in the *Amazon Data Firehose Developer Guide*.

### Restriction for Kinesis Data Stream as Source

When using a Kinesis Data Stream as the source for a Delivery Stream, server-side encryption (SSE) must be explicitly disabled on the Delivery Stream. This is because the encryption should be specified on the source Kinesis Data Stream instead.

```ts
declare const kinesisStream: kinesis.Stream;
declare const destination: firehose.IDestination;

// SSE must be explicitly disabled when using a Kinesis Data Stream as source
new firehose.DeliveryStream(this, 'Delivery Stream with Kinesis Source', {
source: new firehose.KinesisStreamSource(kinesisStream),
encryption: firehose.StreamEncryption.unencrypted(),
destination: destination,
});
```

It's recommended to specify SSE on the source Kinesis Data Stream if encryption is required.

## Monitoring

Amazon Data Firehose is integrated with CloudWatch, so you can monitor the performance of
Expand Down
20 changes: 13 additions & 7 deletions packages/@aws-cdk/aws-kinesisfirehose-alpha/lib/delivery-stream.ts
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,10 @@ export interface DeliveryStreamProps {
/**
* Indicates the type of customer master key (CMK) to use for server-side encryption, if any.
*
* @default StreamEncryption.unencrypted()
* If the source is a Kinesis Data Stream, encryption must be set to `StreamEncryption.unencrypted()`.
* To enable server-side encryption when using a Kinesis Data Stream as the source, apply the encryption settings directly on the data stream itself.
*
* @default StreamEncryption.awsOwnedKey()
*/
readonly encryption?: StreamEncryption;
}
Expand Down Expand Up @@ -328,17 +331,20 @@ export class DeliveryStream extends DeliveryStreamBase {
});
}

const encryption = props.encryption ?? StreamEncryption.awsOwnedKey();
const encryptionKey = encryption.encryptionKey ?? (encryption.type === StreamEncryptionType.CUSTOMER_MANAGED ? new kms.Key(this, 'Key') : undefined);
Copy link
Contributor

Choose a reason for hiding this comment

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

This encryptionKey feels a bit off to me. Generally if it's a customer managed key, I believe we should allow users to be able to specify a key themselve instead of always creating a key for them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For customer managed key, passing in their own key is optional so this only creates a key when customer managed key is set if they didn't pass in their own, otherwise const encryptionKey = encryption.encryptionKey would use the key provided.

const encryptionConfig = (encryptionKey || (encryption?.type === StreamEncryptionType.AWS_OWNED)) ? {
keyArn: encryptionKey?.keyArn,
keyType: encryptionKey ? 'CUSTOMER_MANAGED_CMK' : 'AWS_OWNED_CMK',
} : undefined;

if (
props.source &&
(props.encryption?.type === StreamEncryptionType.AWS_OWNED || props.encryption?.type === StreamEncryptionType.CUSTOMER_MANAGED)
(encryption.type === StreamEncryptionType.AWS_OWNED || encryption.type === StreamEncryptionType.CUSTOMER_MANAGED)
) {
throw new Error('Requested server-side encryption but delivery stream source is a Kinesis data stream. Specify server-side encryption on the data stream instead.');
}
const encryptionKey = props.encryption?.encryptionKey ?? (props.encryption?.type === StreamEncryptionType.CUSTOMER_MANAGED ? new kms.Key(this, 'Key') : undefined);
const encryptionConfig = (encryptionKey || (props.encryption?.type === StreamEncryptionType.AWS_OWNED)) ? {
keyArn: encryptionKey?.keyArn,
keyType: encryptionKey ? 'CUSTOMER_MANAGED_CMK' : 'AWS_OWNED_CMK',
} : undefined;

/*
* In order for the service role to have access to the encryption key before the delivery stream is created, the
* CfnDeliveryStream below should have a dependency on the grant returned by the function call below:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,10 @@ describe('delivery stream', () => {
});

Template.fromStack(stack).hasResourceProperties('AWS::KinesisFirehose::DeliveryStream', {
DeliveryStreamEncryptionConfigurationInput: Match.absent(),
DeliveryStreamEncryptionConfigurationInput: {
KeyARN: Match.absent(),
KeyType: 'AWS_OWNED_CMK',
},
DeliveryStreamName: Match.absent(),
DeliveryStreamType: 'DirectPut',
KinesisStreamSourceConfiguration: Match.absent(),
Expand Down Expand Up @@ -135,6 +138,7 @@ describe('delivery stream', () => {

new firehose.DeliveryStream(stack, 'Delivery Stream', {
destination: mockS3Destination,
encryption: StreamEncryption.unencrypted(),
source: new source.KinesisStreamSource(sourceStream),
});

Expand Down Expand Up @@ -182,6 +186,7 @@ describe('delivery stream', () => {
new firehose.DeliveryStream(stack, 'Delivery Stream', {
destination: mockS3Destination,
source: new source.KinesisStreamSource(sourceStream),
encryption: StreamEncryption.unencrypted(),
role: deliveryStreamRole,
});

Expand Down Expand Up @@ -298,6 +303,23 @@ describe('delivery stream', () => {
});
});

test('not setting encryption defaults to AWS-owned key and does not create key and creates configuration', () => {
new firehose.DeliveryStream(stack, 'Delivery Stream', {
destination: mockS3Destination,
role: deliveryStreamRole,
});

Template.fromStack(stack).resourceCountIs('AWS::KMS::Key', 0);
Template.fromStack(stack).resourceCountIs('AWS::IAM::Policy', 0);
Template.fromStack(stack).hasResourceProperties('AWS::KinesisFirehose::DeliveryStream', {
DeliveryStreamType: 'DirectPut',
DeliveryStreamEncryptionConfigurationInput: {
KeyARN: Match.absent(),
KeyType: 'AWS_OWNED_CMK',
},
});
});

test('requesting no encryption creates no configuration', () => {
new firehose.DeliveryStream(stack, 'Delivery Stream', {
destination: mockS3Destination,
Expand Down Expand Up @@ -331,6 +353,10 @@ describe('delivery stream', () => {
encryption: StreamEncryption.customerManagedKey(new kms.Key(stack, 'Key')),
source: new source.KinesisStreamSource(sourceStream),
})).toThrowError('Requested server-side encryption but delivery stream source is a Kinesis data stream. Specify server-side encryption on the data stream instead.');
expect(() => new firehose.DeliveryStream(stack, 'Delivery Stream 4', {
destination: mockS3Destination,
source: new source.KinesisStreamSource(sourceStream),
})).toThrowError('Requested server-side encryption but delivery stream source is a Kinesis data stream. Specify server-side encryption on the data stream instead.');
});

test('grant provides access to stream', () => {
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading