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

#409 from cfn_nag issues, add support for relationship between NACLs and egress and ingress entries. #74

Merged
4 commits merged into from
Apr 1, 2020

Conversation

thegonch
Copy link
Contributor

@thegonch thegonch commented Mar 25, 2020

In reference to stelligent/cfn_nag#409.

This adds a relationship to be parsed between EC2 Network ACLs and Network ACL Entries separated by egress and ingress. This is primarily because the items we are checking for in this issue affect ingress and egress separately, including repeating rule numbers and reusing ports.

Planned to be used in conjunction with: https://github.com/stelligent/cfn_nag/tree/feature/409_ineffective_nacl_rules

This also adds the truthy util to cfn-model, as previously used in cfn-nag.

@thegonch thegonch requested a review from a team March 25, 2020 18:58
def ingress_network_acl_entries(cfn_model)
network_acl_entries = cfn_model.resources_by_type 'AWS::EC2::NetworkAclEntry'
network_acl_entries.select do |network_acl_entry|
network_acl_entry.egress.nil? || !network_acl_entry.egress
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

beware of "weak typing" in cfn. this could be "true", true, "false", false. there should be a truthy? util in the model you can use instead of a straight boolean op

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There isn't currently a truthy? util in cfn_model, just cfn_nag, but I can add one to model as part of this PR.

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just fix the truthiness and should be good to go

@thegonch thegonch requested a review from a user March 26, 2020 15:58
@ghost ghost merged commit 4c7ee6c into master Apr 1, 2020
@thegonch thegonch deleted the feature/409_ineffective_nacl_rules branch April 1, 2020 15:53
ssteo pushed a commit to ssteo/cfn-model that referenced this pull request Oct 16, 2020
…w SAM transforms templates (stelligent#64)

* stelligent/cfn_nag#74 Reworking Serverless transform to more closely 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.

* Updating array syntax to use ruby's %w[].
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.

1 participant