From 68a1064677f5d6096194cbe39e8141e17c348b7e Mon Sep 17 00:00:00 2001 From: Florian Schmidt Date: Tue, 26 Sep 2023 10:38:39 +0200 Subject: [PATCH 1/4] Enforce KMS encryption for Pipeline artifact bucket objects Fixes #5328 --- internal/pkg/template/templates/app/cf.yml | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/internal/pkg/template/templates/app/cf.yml b/internal/pkg/template/templates/app/cf.yml index bb2c4880eea..811fbe3ca0a 100644 --- a/internal/pkg/template/templates/app/cf.yml +++ b/internal/pkg/template/templates/app/cf.yml @@ -82,6 +82,20 @@ Resources: AWS: - !Sub arn:${AWS::Partition}:iam::${AWS::AccountId}:root{{range $accounts}} - !Sub arn:${AWS::Partition}:iam::{{.}}:root{{end}} + - Action: s3:PutObject + Effect: Deny + Resource: !Sub arn:${AWS::Partition}:s3:::${PipelineBuiltArtifactBucket}/* + Principal: '*' + Condition: + StringNotEquals: + s3:x-amz-server-side-encryption: 'aws:kms' + - Action: s3:PutObject + Effect: Deny + Resource: !Sub arn:${AWS::Partition}:s3:::${PipelineBuiltArtifactBucket}/* + Principal: '*' + Condition: + Null: + s3:x-amz-server-side-encryption: true PipelineBuiltArtifactBucket: Type: AWS::S3::Bucket Properties: @@ -90,7 +104,9 @@ Resources: BucketEncryption: ServerSideEncryptionConfiguration: - ServerSideEncryptionByDefault: - SSEAlgorithm: AES256 + SSEAlgorithm: 'aws:kms' + KMSMasterKeyID: !GetAtt KMSKey.Arn + BucketKeyEnabled: true PublicAccessBlockConfiguration: BlockPublicAcls: true BlockPublicPolicy: true From 0763b3fa1336318d86180147fbf61e32e0ef5199 Mon Sep 17 00:00:00 2001 From: Florian Schmidt Date: Wed, 27 Sep 2023 09:19:44 +0200 Subject: [PATCH 2/4] fix wrong policy to allow default server-side-encryption when uploading object --- internal/pkg/template/templates/app/cf.yml | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/internal/pkg/template/templates/app/cf.yml b/internal/pkg/template/templates/app/cf.yml index 811fbe3ca0a..fb6afffdb7a 100644 --- a/internal/pkg/template/templates/app/cf.yml +++ b/internal/pkg/template/templates/app/cf.yml @@ -87,15 +87,11 @@ Resources: Resource: !Sub arn:${AWS::Partition}:s3:::${PipelineBuiltArtifactBucket}/* Principal: '*' Condition: - StringNotEquals: + StringNotEqualsIfExists: s3:x-amz-server-side-encryption: 'aws:kms' - - Action: s3:PutObject - Effect: Deny - Resource: !Sub arn:${AWS::Partition}:s3:::${PipelineBuiltArtifactBucket}/* - Principal: '*' - Condition: Null: - s3:x-amz-server-side-encryption: true + s3:x-amz-server-side-encryption: false + PipelineBuiltArtifactBucket: Type: AWS::S3::Bucket Properties: From 619c6642ba9b07d5e6698f6818676696cb08e2d6 Mon Sep 17 00:00:00 2001 From: Florian Schmidt Date: Tue, 10 Oct 2023 09:58:58 +0200 Subject: [PATCH 3/4] allow CopyAssetsStateMachine to decrypt S3 objects from pipeline bucket --- .../templates/workloads/services/static-site/cf.yml | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/internal/pkg/template/templates/workloads/services/static-site/cf.yml b/internal/pkg/template/templates/workloads/services/static-site/cf.yml index f23f2d4b7a3..36afd9a2a78 100644 --- a/internal/pkg/template/templates/workloads/services/static-site/cf.yml +++ b/internal/pkg/template/templates/workloads/services/static-site/cf.yml @@ -327,6 +327,15 @@ Resources: Resource: - arn:aws:s3:::{{.AssetMappingFileBucket}}/{{.AssetMappingFilePath}} - arn:aws:s3:::{{.AssetMappingFileBucket}}/local-assets/* + - Effect: Allow + Action: kms:Decrypt + Resource: + Fn::ImportValue: + !Sub "${AppName}-ArtifactKey" + Condition: + StringEquals: + kms:EncryptionContext:aws:s3:arn: arn:aws:s3:::{{.AssetMappingFileBucket}} + kms:ViaService: !Sub 's3.${AWS::Region}.amazonaws.com' - PolicyName: ServiceBucketAccess PolicyDocument: Version: 2012-10-17 From 039fb75e6361189482bb3c35cd7dac98182a46c6 Mon Sep 17 00:00:00 2001 From: iamhopaul123 Date: Tue, 10 Oct 2023 10:37:36 -0700 Subject: [PATCH 4/4] Add integ test and nit --- .../cloudformation/stack/static_site_integration_test.go | 2 +- .../stack/testdata/workloads/static-site-test.stack.yml | 9 +++++++++ .../stack/testdata/workloads/static-site.stack.yml | 9 +++++++++ internal/pkg/template/templates/app/cf.yml | 2 +- 4 files changed, 20 insertions(+), 2 deletions(-) diff --git a/internal/pkg/deploy/cloudformation/stack/static_site_integration_test.go b/internal/pkg/deploy/cloudformation/stack/static_site_integration_test.go index 68eae0b1b6d..d0947e75151 100644 --- a/internal/pkg/deploy/cloudformation/stack/static_site_integration_test.go +++ b/internal/pkg/deploy/cloudformation/stack/static_site_integration_test.go @@ -101,7 +101,7 @@ func TestStaticSiteService_TemplateAndParamsGeneration(t *testing.T) { Version: "v1.29.0", Region: "us-west-2", }, - ArtifactBucketName: "bucket", + ArtifactBucketName: "stackset-bucket", AssetMappingURL: "s3://stackset-bucket/mappingfile", RootUserARN: "arn:aws:iam::123456789123:root", }) diff --git a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/static-site-test.stack.yml b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/static-site-test.stack.yml index 534b334b4e9..11c36e4fe9c 100644 --- a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/static-site-test.stack.yml +++ b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/static-site-test.stack.yml @@ -284,6 +284,15 @@ Resources: Resource: - arn:aws:s3:::stackset-bucket/mappingfile - arn:aws:s3:::stackset-bucket/local-assets/* + - Effect: Allow + Action: kms:Decrypt + Resource: + Fn::ImportValue: + !Sub "${AppName}-ArtifactKey" + Condition: + StringEquals: + kms:EncryptionContext:aws:s3:arn: arn:aws:s3:::stackset-bucket + kms:ViaService: !Sub 's3.${AWS::Region}.amazonaws.com' - PolicyName: ServiceBucketAccess PolicyDocument: Version: 2012-10-17 diff --git a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/static-site.stack.yml b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/static-site.stack.yml index 5d5a54a6fee..1ac23771999 100644 --- a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/static-site.stack.yml +++ b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/static-site.stack.yml @@ -290,6 +290,15 @@ Resources: Resource: - arn:aws:s3:::stackset-bucket/mappingfile - arn:aws:s3:::stackset-bucket/local-assets/* + - Effect: Allow + Action: kms:Decrypt + Resource: + Fn::ImportValue: + !Sub "${AppName}-ArtifactKey" + Condition: + StringEquals: + kms:EncryptionContext:aws:s3:arn: arn:aws:s3:::stackset-bucket + kms:ViaService: !Sub 's3.${AWS::Region}.amazonaws.com' - PolicyName: ServiceBucketAccess PolicyDocument: Version: 2012-10-17 diff --git a/internal/pkg/template/templates/app/cf.yml b/internal/pkg/template/templates/app/cf.yml index fb6afffdb7a..449b012ac9d 100644 --- a/internal/pkg/template/templates/app/cf.yml +++ b/internal/pkg/template/templates/app/cf.yml @@ -89,7 +89,7 @@ Resources: Condition: StringNotEqualsIfExists: s3:x-amz-server-side-encryption: 'aws:kms' - Null: + 'Null': s3:x-amz-server-side-encryption: false PipelineBuiltArtifactBucket: