From 2b38aef220196f5b725e57c148fee8957eca1328 Mon Sep 17 00:00:00 2001 From: LoneRifle Date: Wed, 28 Aug 2024 17:22:02 +0800 Subject: [PATCH] feat: add virus scanning via ECS Reinstate virus scanning abilities by emulating Lambda on ECS. This allows us to avoid using Lambda through CloudFormation, and the awkward problem of ECR images that comes with it - Rename s3-suffix-secret -> suffix-secret - Define new ECS-based Construct for virus scanner - Correct IAM role policy errors discovered during dev testing --- lib/constructs/lambdas.ts | 4 +- lib/constructs/s3.ts | 16 +++- lib/constructs/virus-scanner-ecs.ts | 114 ++++++++++++++++++++++++++++ lib/formsg-on-cdk-stack.ts | 31 ++++---- 4 files changed, 147 insertions(+), 18 deletions(-) create mode 100644 lib/constructs/virus-scanner-ecs.ts diff --git a/lib/constructs/lambdas.ts b/lib/constructs/lambdas.ts index b08b9b4..1728929 100644 --- a/lib/constructs/lambdas.ts +++ b/lib/constructs/lambdas.ts @@ -48,7 +48,7 @@ export class FormsgLambdas extends Construct { 's3:DeleteObject', 's3:DeleteObjectVersion', ], - resources: [s3VirusScannerQuarantine.bucketArn], + resources: [`${s3VirusScannerQuarantine.bucketArn}/*`], }), ], })) @@ -59,7 +59,7 @@ export class FormsgLambdas extends Construct { 's3:PutObject', 's3:PutObjectTagging', ], - resources: [s3VirusScannerClean.bucketArn], + resources: [`${s3VirusScannerClean.bucketArn}/*`], }), ], })) diff --git a/lib/constructs/s3.ts b/lib/constructs/s3.ts index b0c6160..b110531 100644 --- a/lib/constructs/s3.ts +++ b/lib/constructs/s3.ts @@ -91,9 +91,21 @@ export class FormsgS3Buckets extends Construct { enforceSSL: true, removalPolicy: RemovalPolicy.DESTROY, versioned: true, - blockPublicAccess: BlockPublicAccess.BLOCK_ALL, - objectOwnership: ObjectOwnership.BUCKET_OWNER_ENFORCED, + objectOwnership: ObjectOwnership.OBJECT_WRITER, + blockPublicAccess: { + blockPublicAcls: false, + ignorePublicAcls: false, + blockPublicPolicy: false, + restrictPublicBuckets: false, + }, + cors: [ + { + allowedMethods: [HttpMethods.POST], + allowedOrigins: [origin], + } + ], }) + this.s3VirusScannerQuarantine.grantPublicAccess('*', 's3:PutObject', 's3:PutObjectAcl') this.s3VirusScannerClean = new Bucket(this, `virus-scanner-clean`, { bucketName: `${envVars.VIRUS_SCANNER_CLEAN_S3_BUCKET}-${s3Suffix}`, diff --git a/lib/constructs/virus-scanner-ecs.ts b/lib/constructs/virus-scanner-ecs.ts new file mode 100644 index 0000000..ef1004a --- /dev/null +++ b/lib/constructs/virus-scanner-ecs.ts @@ -0,0 +1,114 @@ +import { RemovalPolicy } from 'aws-cdk-lib' +import { BlockPublicAccess, Bucket, BucketAccessControl, HttpMethods, ObjectOwnership } from 'aws-cdk-lib/aws-s3' +import { Construct } from 'constructs' +import envVars from '../formsg-env-vars' +import * as ecs from 'aws-cdk-lib/aws-ecs'; +import { FormsgS3Buckets } from './s3'; +import { ApplicationLoadBalancer, ApplicationProtocol } from 'aws-cdk-lib/aws-elasticloadbalancingv2'; +import { PolicyStatement } from 'aws-cdk-lib/aws-iam'; +import { LogGroup } from 'aws-cdk-lib/aws-logs'; + +export class VirusScannerEcs extends Construct { + readonly service: ecs.FargateService + readonly hostname: string + + constructor( + scope: Construct, + { + cluster, + logGroupSuffix, + s3Buckets, + } : { + cluster: ecs.Cluster; + logGroupSuffix: string; + s3Buckets: FormsgS3Buckets + } + ) { + super(scope, 'virus-scanner') + const { vpc } = cluster + + // Hack together a virus scanner cluster in lieu of Lambda + const port = 8080 + const taskDefinition = new ecs.FargateTaskDefinition(this, 'task', { + memoryLimitMiB: 2048, + cpu: 1024, + }) + taskDefinition + .addContainer('task-container', { + image: ecs.ContainerImage.fromRegistry('opengovsg/lambda-virus-scanner:latest-ecs'), + containerName: 'web', + environment: { + NODE_ENV: 'production', + VIRUS_SCANNER_QUARANTINE_S3_BUCKET: s3Buckets.s3VirusScannerQuarantine.bucketName, + VIRUS_SCANNER_CLEAN_S3_BUCKET: s3Buckets.s3VirusScannerClean.bucketName, + }, + logging: ecs.LogDriver.awsLogs({ + logGroup: new LogGroup(this, 'cloudwatch', { + logGroupName: `/aws/ecs/logs/virus-scanner/${logGroupSuffix}`, + }), + streamPrefix: 'virus-scanner', + }), + portMappings: [ + { containerPort: port, hostPort: port }, + ], + }) + taskDefinition.addToTaskRolePolicy( + new PolicyStatement({ + actions: [ + 's3:GetObject', + 's3:GetObjectTagging', + 's3:GetObjectVersion', + 's3:DeleteObject', + 's3:DeleteObjectVersion', + ], + resources: [`${s3Buckets.s3VirusScannerQuarantine.bucketArn}/*`], + }) + ) + taskDefinition.addToTaskRolePolicy( + new PolicyStatement({ + actions: [ + 's3:PutObject', + 's3:PutObjectTagging', + ], + resources: [`${s3Buckets.s3VirusScannerClean.bucketArn}/*`], + }) + ) + + const service = new ecs.FargateService(this, 'service', { + cluster, + taskDefinition, + }) + + const loadBalancer = new ApplicationLoadBalancer(this, 'alb', { + loadBalancerName: 'alb', + internetFacing: false, + vpc, + vpcSubnets: { + subnets: vpc.privateSubnets, + }, + }) + const listener = loadBalancer.addListener('alb-listener', { + port: 80, + protocol: ApplicationProtocol.HTTP, + }) + + service.registerLoadBalancerTargets({ + containerName: 'web', + containerPort: port, + listener: ecs.ListenerConfig.applicationListener( + listener, + { + protocol: ApplicationProtocol.HTTP, + port, + healthCheck: { + healthyHttpCodes: ['200', '404'].join(',') + } + }, + ), + newTargetGroupId: 'ecs', + }) + + this.service = service + this.hostname = loadBalancer.loadBalancerDnsName + } +} diff --git a/lib/formsg-on-cdk-stack.ts b/lib/formsg-on-cdk-stack.ts index 1ff2168..0e5895e 100644 --- a/lib/formsg-on-cdk-stack.ts +++ b/lib/formsg-on-cdk-stack.ts @@ -15,6 +15,7 @@ import { FormsgEcr } from './constructs/ecr' import defaultEnvironment from './formsg-env-vars' import { LogGroup } from 'aws-cdk-lib/aws-logs' import { OriginVerify } from '@alma-cdk/origin-verify' +import { VirusScannerEcs } from './constructs/virus-scanner-ecs' export class FormsgOnCdkStack extends cdk.Stack { constructor(scope: Construct, id: string, props?: cdk.StackProps) { @@ -89,10 +90,6 @@ export class FormsgOnCdkStack extends cdk.Stack { // Create ECR const ecr = new FormsgEcr(this) - // Do not create Lambda Virus Scanner for now, until we figure out - // how to load the ECR image for this deployment - // const lambdas = new FormsgLambdas(this, { s3Buckets, ecr }) - // Create DocumentDB cluster const ddbPassSecret = new Secret(this, 'ddb-password', { secretName: 'ddb-password', @@ -202,8 +199,8 @@ export class FormsgOnCdkStack extends cdk.Stack { const distributionUrl = `https://${cloudFront.distributionDomainName}` // Create S3 buckets - const s3SuffixSecret = new Secret(this, 's3-suffix-secret', { - secretName: 's3-suffix-secret', + const suffixSecret = new Secret(this, 'suffix-secret', { + secretName: 'suffix-secret', removalPolicy: cdk.RemovalPolicy.DESTROY, generateSecretString: { excludePunctuation: true, @@ -213,9 +210,16 @@ export class FormsgOnCdkStack extends cdk.Stack { }, }) - const s3Suffix = s3SuffixSecret.secretValue.unsafeUnwrap() + const s3Suffix = suffixSecret.secretValue.unsafeUnwrap() const s3Buckets = new FormsgS3Buckets(this, { s3Suffix, origin: distributionUrl }) + // Create ECS Cluster + const logGroupSuffix = s3Suffix + const cluster = new ecs.Cluster(this, 'ecs', { vpc }) + + // Hack together a virus scanner cluster in lieu of Lambda + const virusScanner = new VirusScannerEcs(this, { cluster, logGroupSuffix, s3Buckets }) + const environment = { ...defaultEnvironment, APP_URL: distributionUrl, @@ -238,6 +242,7 @@ export class FormsgOnCdkStack extends cdk.Stack { STATIC_ASSETS_S3_BUCKET: s3Buckets.s3StaticAssets.bucketName, VIRUS_SCANNER_QUARANTINE_S3_BUCKET: s3Buckets.s3VirusScannerQuarantine.bucketName, VIRUS_SCANNER_CLEAN_S3_BUCKET: s3Buckets.s3VirusScannerClean.bucketName, + VIRUS_SCANNER_LAMBDA_ENDPOINT: `http://${virusScanner.hostname}`, } // Create Session Secret @@ -252,9 +257,7 @@ export class FormsgOnCdkStack extends cdk.Stack { }) ) - // Create ECS Cluster and Fargate Service - const logGroupSuffix = s3Suffix - const cluster = new ecs.Cluster(this, 'ecs', { vpc }) + // Create Fargate Service const fargate = new ecsPatterns.ApplicationLoadBalancedFargateService(this, 'app', { cluster, taskImageOptions: { @@ -303,7 +306,7 @@ export class FormsgOnCdkStack extends cdk.Stack { 's3:DeleteObject', 's3:PutObjectAcl', ], - resources: [bucketArn], + resources: [`${bucketArn}/*`], }) ) ) @@ -314,7 +317,7 @@ export class FormsgOnCdkStack extends cdk.Stack { 's3:PutObject', 's3:GetObject', ], - resources: [s3Buckets.s3PaymentProof.bucketArn], + resources: [`${s3Buckets.s3PaymentProof.bucketArn}/*`], }) ) @@ -323,7 +326,7 @@ export class FormsgOnCdkStack extends cdk.Stack { actions: [ 's3:PutObject', ], - resources: [s3Buckets.s3VirusScannerQuarantine.bucketArn], + resources: [`${s3Buckets.s3VirusScannerQuarantine.bucketArn}/*`], }) ) @@ -332,7 +335,7 @@ export class FormsgOnCdkStack extends cdk.Stack { actions: [ 's3:GetObjectVersion', ], - resources: [s3Buckets.s3VirusScannerClean.bucketArn], + resources: [`${s3Buckets.s3VirusScannerClean.bucketArn}/*`], }) )