fix(race condition): Apply bucket policies to avoid race condition (#224

) (#230)

* Apply bucket policies in a different way, to temporarily get around cms active remediation race conditions

* Add a force override

* Make it clear that these are custom resources

src/services/uploads/serverless.yml

@@ -43,11 +43,19 @@ provider:
             - lambda:InvokeFunction
           Resource:
             - !Sub arn:aws:lambda:${self:provider.region}:${AWS::AccountId}:function:${self:service}-${sls:stage}-avDownloadDefinitions
+        - Effect: "Allow"
+          Action:
+            - "s3:GetBucketPolicy"
+            - "s3:PutBucketPolicy"
+          Resource:
+            - !Sub "arn:aws:s3:::${self:service}-${sls:stage}-attachments-${AWS::AccountId}"
+            - !Sub "arn:aws:s3:::${self:service}-${sls:stage}-avscan-${AWS::AccountId}"
 
 custom:
   project: ${env:PROJECT}
   accountId: !Sub "${AWS::AccountId}"
   stage: ${opt:stage, self:provider.stage}
+  forceApplyBucketPolicies: ${ssm:/aws/reference/secretsmanager/${self:custom.project}/${sls:stage}/forceApplyBucketPolicies, ssm:/aws/reference/secretsmanager/${self:custom.project}/default/forceApplyBucketPolicies, "blank"}
   serverlessTerminationProtection:
     stages: # Apply CloudFormation termination protection for these stages
       - master
@@ -93,6 +101,10 @@ functions:
     handler: src/triggerInitialDownload.handler
     timeout: 300 # 300 seconds = 5 minutes
     memorySize: 1024
+  applyPolicy:
+    handler: src/applyPolicy.handler
+    timeout: 300
+    memorySize: 1024
 resources:
   Resources:
     AttachmentsBucket:
@@ -120,12 +132,14 @@ resources:
             - Event: s3:ObjectCreated:*
               Function: !GetAtt AvScanLambdaFunction.Arn
       DependsOn: LambdaInvokePermission
-    S3CMSReadBucketPolicy:
-      Type: AWS::S3::BucketPolicy
+    AttachmentsBucketPolicyCustomResource:
+      Type: Custom::AttachmentsBucketPolicy
       Properties:
-        Bucket:
-          Ref: AttachmentsBucket
-        PolicyDocument:
+        ServiceToken: !GetAtt ApplyPolicyLambdaFunction.Arn
+        ForceApply: ${self:custom.forceApplyBucketPolicies}
+        Bucket: !Ref AttachmentsBucket
+        Policy:
+          Version: "2012-10-17"
           Statement:
             - Effect: Deny
               Principal: "*"
@@ -164,12 +178,14 @@ resources:
           ServerSideEncryptionConfiguration:
             - ServerSideEncryptionByDefault:
                 SSEAlgorithm: AES256
-    ClamDefsBucketPolicy:
-      Type: AWS::S3::BucketPolicy
+    ClamDefsBucketPolicyCustomResource:
+      Type: Custom::ClamDefsBucketPolicy
       Properties:
-        Bucket:
-          Ref: ClamDefsBucket
-        PolicyDocument:
+        ServiceToken: !GetAtt ApplyPolicyLambdaFunction.Arn
+        ForceApply: ${self:custom.forceApplyBucketPolicies}
+        Bucket: !Ref ClamDefsBucket
+        Policy:
+          Version: "2012-10-17"
           Statement:
             - Effect: Deny
               Principal: "*"

src/services/uploads/src/applyPolicy.ts

@@ -0,0 +1,25 @@
+import { Handler } from "aws-lambda";
+import { send, SUCCESS, FAILED } from "cfn-response-async";
+type ResponseStatus = typeof SUCCESS | typeof FAILED;
+import { S3Client, PutBucketPolicyCommand } from '@aws-sdk/client-s3';
+const s3 = new S3Client();
+
+export const handler: Handler = async (event, context) => {
+  console.log("request:", JSON.stringify(event, undefined, 2));
+  const responseData = {};
+  let responseStatus: ResponseStatus = SUCCESS;
+  let Bucket = event.ResourceProperties.Bucket;
+  let Policy = JSON.stringify(event.ResourceProperties.Policy);
+  try {
+    if (event.RequestType == "Create" || event.RequestType == "Update") {
+      let resp = await s3.send(new PutBucketPolicyCommand({ Bucket, Policy }));
+      console.log(resp)
+    }
+  } catch (error) {
+    console.log(error);
+    responseStatus = FAILED;
+  } finally {
+    console.log("finally");
+    await send(event, context, responseStatus, responseData);
+  }
+};