Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

stelligent/cfn_nag#74 Reworking Serverless transform to more closely match how SAM transforms templates #64

Merged
2 commits merged into from
Feb 28, 2020

Conversation

pshelby
Copy link
Contributor

@pshelby pshelby commented Feb 27, 2020

Updates stelligent/cfn_nag#74

Description

In order to properly audit permissions assigned to serverless functions, the roles which are generated must closely match those coming directly from SAM. This PR changes the Serverless transform in cfn-model to:

  1. Generating an IAM role for each serverless function, if Role property not provided.
  2. Parsing serverless function properties to correctly populate generated role.
  3. Updating spec tests.

As you'll see in the Testing section below, this allows utilization of all cfn_nag custom rules against the resultant roles.

Testing

  1. All rspec tests pass.
  2. Test runs of cfn_nag against a test YAML template showed individual roles per serverless function (unless a Role property was specified) and more appropriate output of custom rule audits.

Before

------------------------------------------------------------
spec/test_templates/yaml/sam/serverless_rest_api_with_basepathmapping.yml
------------------------------------------------------------
Failures count: 0
Warnings count: 0

raw_model.before.txt

After

------------------------------------------------------------
spec/test_templates/yaml/sam/serverless_rest_api_with_basepathmapping.yml
------------------------------------------------------------------------------------------------------------------------
| WARN W43
|
| Resources: ["FunctionWithAdministratorAccessPolicyRole"]
| Line Numbers: [-1]
|
| IAM role should not have AdministratorAccess policy
------------------------------------------------------------
| WARN W44
|
| Resources: ["FunctionWithPowerUserPolicyAndPolicyTemplateRole"]
| Line Numbers: [-1]
|
| IAM role should not have Elevated Managed policy
------------------------------------------------------------
| FAIL F38
|
| Resources: ["FunctionWithInlineAdminPolicyRole"]
| Line Numbers: [-1]
|
| IAM role should not allow * resource with PassRole action on its permissions policy
------------------------------------------------------------
| FAIL F3
|
| Resources: ["FunctionWithInlineAdminPolicyRole"]
| Line Numbers: [-1]
|
| IAM role should not allow * action on its permissions policy
------------------------------------------------------------
| WARN W11
|
| Resources: ["FunctionWithInlineAdminPolicyRole"]
| Line Numbers: [-1]
|
| IAM role should not allow * resource on its permissions policy
------------------------------------------------------------
| WARN W58
|
| Resources: ["FunctionWithExternalRole"]
| Line Numbers: [-1]
|
| Lambda functions require permission to write CloudWatch Logs

Failures count: 2
Warnings count: 4

raw_model.after.txt

Patrick Shelby added 2 commits February 27, 2020 09:36
…match how SAM transforms templates.

1. Generating an IAM role for each serverless function, if Role property not provided.
2. Parsing serverless function properties to correctly populate generated role.
3. Updating spec tests.
@ghost ghost merged commit 6a18e6a into stelligent:master Feb 28, 2020
@pshelby pshelby deleted the feature/roles-per-sam-function branch March 5, 2020 16:12
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant