aws / serverless-application-model

The AWS Serverless Application Model (AWS SAM) transform is a AWS CloudFormation macro that transforms SAM templates into CloudFormation templates.
https://aws.amazon.com/serverless/sam
Apache License 2.0
9.29k stars 2.37k forks source link

Security: default permission allow any service to invoke a lambda function #3599

Closed ananich closed 3 weeks ago

ananich commented 1 month ago

Description:

When a lambda function has S3 event, this lambda function can be invoked by any service (not just S3).

Steps to reproduce:

  MyLambdaFunction:
    Type: AWS::Serverless::Function
    Properties:
      FunctionName: !Sub ${AWS::StackName}-hello
      CodeUri: hello/
      Handler: lambda_function.lambda_handler
      Runtime: python3.12
      Role: !GetAtt MyLambdaFunctionRole.Arn
      Events:
        S3FileAdded:
          Type: S3
          Properties:
            Bucket: !Ref MyBucket
            Events:
              - s3:ObjectCreated:*
            Filter:
              S3Key:
                Rules:
                  - Name: prefix
                    Value: inbox/
                  - Name: suffix
                    Value: .txt

Observed result:

When permission is created, it looks like that:

{
  "StringEquals": {
    "AWS:SourceAccount": "123123123123"
  }
}

Expected result:

I'd like Type: S3 to be same as Type: Schedule produce:

{
  "ArnLike": {
    "AWS:SourceArn": "arn:aws:events:us-east-1:123123123123:rule/hello-world"
  }
}

Additional environment details (Ex: Windows, Mac, Amazon Linux etc)

  1. OS: macOS-***-x86_64-i386-64bit
  2. Python: 3.12.2
  3. sam --version: SAM CLI, version 1.116.0
  4. AWS region: us-east-1
lucashuy commented 1 month ago

Thanks for reporting this. This seems to be stemming from the transform library instead of SAM CLI. Let me move this issue over to the SAM repo. This behaviour is likely due to how the S3 event is setup and used.

It looks like an S3 bucket notification configuration to Lambda requires the bucket have permissions to invoke the Lambda, before it will setup the notification event. This means that the Lambda permission needs to be created first, but since the permissions is being created first, it has no idea what the bucket's ARN is, hence the broader permission. (ref: https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-s3-bucket-notificationconfiguration.html).

GavinZZ commented 1 month ago

Hi @ananich, thanks for creating an issue. As Lucas explained above, this is a known limitation with S3 bucket notification.

If you create the target resource and related permissions in the same template, you might have a circular dependency.

For example, you might use the AWS::Lambda::Permission resource to grant the bucket permission to invoke an AWS Lambda function. However, AWS CloudFormation can't create the bucket until the bucket has permission to invoke the function (AWS CloudFormation checks whether the bucket can invoke the function). If you're using Refs to pass the bucket name, this leads to a circular dependency.

To avoid this dependency, you can create all resources without specifying the notification configuration. Then, update the stack with a notification configuration.

When using SAM with s3 event source, we have to first create the bucket configuration prior to creating bucket, thus we cannot restrict the bucket in the Condition property.

github-actions[bot] commented 3 weeks ago

This issue is now closed. Comments on closed issues are hard for our team to see. If you need more assistance, please either tag a team member or open a new issue that references this one.