#409 Ineffective NACL rules #423
Conversation
…and test for ec2 network acl where port ranges could be set to '-1' as it does not seem to be possible for Cloudformation. Other general cleanup per Eric Kascic's PR review.
… ports with appropriate tests and warning text updates
…orts, and duplicate rule numbers, along with their spec tests.
lib/cfn-nag/custom_rules/EC2NetworkAclEntryIneffectiveDenyRule.rb
Outdated
Show resolved
Hide resolved
lib/cfn-nag/custom_rules/EC2NetworkAclEntryIneffectiveDenyRule.rb
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
there a number of places in this code where select/each/map are/aren't being used the way they should be. i'd advocate using select/map where you can instead of each with outside state (the code is using select with outside state which doesn't make sense). i think the rules themselves make sense. if the port range stuff overlaps... think it's a good way to warn developers they could be making trouble for themselves.
lib/cfn-nag/custom_rules/EC2NetworkAclEntryIneffectiveDenyRule.rb
Outdated
Show resolved
Hide resolved
…p, add port overlap check
@erickascic I've updated the code here though it is pending the review/merge of stelligent/cfn-model#77 to pull in the updated cfn-model gem. |
lib/cfn-nag/custom_rules/EC2NetworkAclEntryIneffectiveDenyRule.rb
Outdated
Show resolved
Hide resolved
…ing rule numbers
…plicate rule spec test. Clean up duplicate and reused portRange methods using group_by
| if ports.nil? | ||
| nacl_entry_range | ||
| else | ||
| port_min = nacl_entry_range.min < ports.min ? nacl_entry_range.min : ports.min |
There was a problem hiding this comment.
i believe this code is same as [nacl_entry_rnage.min, ports.min].min
There was a problem hiding this comment.
not a big deal - but you save 1 point per statement on CCM
| violating_nacl_entries += violating_nacl_entries(nacl) | ||
| end | ||
|
|
||
| violating_nacl_entries.uniq.map(&:logical_resource_id) |
There was a problem hiding this comment.
is the use of uniq necessary and tested? i don't believe hash or eql? are defined for the nacl entry model element?
…edPortsRule.rb to lib/cfn-nag/custom_rules/EC2NetworkAclEntryOverlappingPortsRule.rb. Clean up overlap logic.
ghost
merged commit 
In reference to #409.