amazon-archives / realworld-serverless-application

This project is inspired by the design and development of the AWS Serverless Application Repository - a production-grade AWS service. Learn how AWS built a production service using serverless technologies.
Apache License 2.0
515 stars 108 forks source link

Possible Security Problems #55

Open ctindall opened 4 years ago

ctindall commented 4 years ago

Hey there! I noticed some possible problems in some code in this repo. A quick summary of a few of them is below, but let me know if you're interested in seeing a full report or talking about cloud security in general.


severity: serious

filename: ./analytics/sam/cicd/template.yaml

line number(s): [184]

resource(s):

IAM policy should not allow * action


severity: warning

filename: ./analytics/src/test/resources/integ-test-env.template.yaml

line number(s): [19]

resource(s):

S3 Bucket should have access logging configured


severity: warning

filename: ./analytics/src/test/resources/integ-test-env.template.yaml

line number(s): [19]

resource(s):

S3 Bucket should have encryption option set


severity: warning

filename: ./analytics/sam/app/template.yaml

line number(s): [91]

resource(s):

IAM policy should not allow * resource


severity: warning

filename: ./analytics/sam/app/template.yaml

line number(s): [37]

resource(s):

S3 Bucket should have access logging configured


severity: warning

filename: ./analytics/sam/cicd/template.yaml

line number(s): [43, 184]

resource(s):

IAM policy should not allow * resource


severity: warning

filename: ./analytics/sam/cicd/template.yaml

line number(s): [377]

resource(s):

IAM role should not allow * resource on its permissions policy

jlhood commented 4 years ago

@ctindall Thanks for the report! What tool did you run to generate this output? Here are my responses on each:

./analytics/sam/cicd/template.yaml:184 - IAM policy should not allow * action

The only action I see is this one. API Gateway IAM actions are different than most AWS APIs, because the actions are just general HTTP methods which have the ability to read/write many different types of API Gateway resources. Since this role is used to give CloudFormation permission to create/update/delete API gateway resources, I think is actually appropriate here. However, we could replace it with each individual HTTP method to make this error go away.

./analytics/src/test/resources/integ-test-env.template.yaml:19 - S3 Bucket should have access logging configured

This is safe to ignore since this S3 bucket is only created for use by integration tests and is then torn down at the end of each test run.

./analytics/src/test/resources/integ-test-env.template.yaml:19 - S3 Bucket should have encryption option set

This is safe to ignore since this S3 bucket is only created for use by integration tests and is then torn down at the end of each test run. There will never be customer data in this bucket.

./analytics/sam/app/template.yaml:91 - IAM policy should not allow * resource

I'm assuming it's referring to this. According to the AWS Glue IAM documentation, these actions do not support anything other than Resource: *, which is why we did it this way. I can doublecheck with the service team to confirm that's the case.

./analytics/sam/app/template.yaml:37 - S3 Bucket should have access logging configured

Sure, we can add this.

./analytics/sam/cicd/template.yaml:43,184 - IAM policy should not allow * resource

I see * resources on lambda event source mapping read/write actions and glue actions for the first IAM policy.

For lambda:CreateEventSourceMapping, it only supports resources. For lambda:GetEventSourceMapping, resource makes sense since this is for a CloudFormation execution role and the event source it would get is the one it's going to create so you don't know what it is ahead of time. For the Glue operations, this is the same as above where the Glue documentation says these APIs don't support anything other than * resources.

For the second IAM policy being flagged, I see resources on cognito-idp:CreateUserPool and kms:CreateKey. Both of these actions only support resources according to the IAM documentation.

./analytics/sam/cicd/template.yaml:377 - IAM role should not allow * resource on its permissions policy

I see resources on Athena actions. According to the IAM docs, we should be able to scope these to a specific workgroup, so we could change it to at least be scoped down to workgroups in the same account/region: `!Sub arn:${AWS::Partition}:athena:${AWS::Region}:${AWS::Account}:workgroup/`

jlhood commented 4 years ago

Actionable updates:

  1. Enable access logging on this bucket.
  2. Change this * resource to !Sub arn:${AWS::Partition}:athena:${AWS::Region}:${AWS::Account}:workgroup/*
ctindall commented 4 years ago

The Ruby cfn_nag gem is the tool I used for this. I will take a look at the bogus flags you mentioned and possibly submit issues/patches for them upstream.