From 4c7ee6ccdc2488c9b958eb7fa0d0ee30bbcd6a1d Mon Sep 17 00:00:00 2001 From: Stephen Goncher Date: Wed, 1 Apr 2020 11:32:40 -0400 Subject: [PATCH] #409 from cfn_nag issues, add support for relationship between NACLs and egress and ingress entries. (#74) * #409 from cfn_nag issues, add support for relationship between NACLs and egress and ingress entries. * #409 Add truthy util, its spec tests, and apply usage for ec2_network_acl_parser --- lib/cfn-model/model/ec2_network_acl.rb | 15 ++++ .../parser/ec2_network_acl_parser.rb | 56 +++++++++++++ lib/cfn-model/parser/parser_registry.rb | 3 +- lib/cfn-model/util/truthy.rb | 11 +++ spec/factories/ec2_network_acl.rb | 42 ++++++++++ .../parser/cfn_parser_ec2_network_acl_spec.rb | 84 +++++++++++++++++++ ...nacl_with_one_egress_and_ingress_entry.yml | 38 +++++++++ .../nacl_with_one_egress_entry.yml | 25 ++++++ .../nacl_with_one_ingress_entry.yml | 25 ++++++ .../nacl_with_two_egress_entries.yml | 37 ++++++++ .../nacl_with_two_ingress_entries.yml | 38 +++++++++ spec/util/truthy_spec.rb | 19 +++++ 12 files changed, 392 insertions(+), 1 deletion(-) create mode 100644 lib/cfn-model/model/ec2_network_acl.rb create mode 100644 lib/cfn-model/parser/ec2_network_acl_parser.rb create mode 100644 lib/cfn-model/util/truthy.rb create mode 100644 spec/factories/ec2_network_acl.rb create mode 100644 spec/parser/cfn_parser_ec2_network_acl_spec.rb create mode 100644 spec/test_templates/yaml/ec2_network_acl/nacl_with_one_egress_and_ingress_entry.yml create mode 100644 spec/test_templates/yaml/ec2_network_acl/nacl_with_one_egress_entry.yml create mode 100644 spec/test_templates/yaml/ec2_network_acl/nacl_with_one_ingress_entry.yml create mode 100644 spec/test_templates/yaml/ec2_network_acl/nacl_with_two_egress_entries.yml create mode 100644 spec/test_templates/yaml/ec2_network_acl/nacl_with_two_ingress_entries.yml create mode 100644 spec/util/truthy_spec.rb diff --git a/lib/cfn-model/model/ec2_network_acl.rb b/lib/cfn-model/model/ec2_network_acl.rb new file mode 100644 index 0000000..962b5b3 --- /dev/null +++ b/lib/cfn-model/model/ec2_network_acl.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +require_relative 'model_element' + +class AWS::EC2::NetworkAcl < ModelElement + attr_accessor :network_acl_egress_entries + attr_accessor :network_acl_ingress_entries + + def initialize(cfn_model) + super + @network_acl_egress_entries = [] + @network_acl_ingress_entries = [] + @resource_type = 'AWS::EC2::NetworkAcl' + end +end \ No newline at end of file diff --git a/lib/cfn-model/parser/ec2_network_acl_parser.rb b/lib/cfn-model/parser/ec2_network_acl_parser.rb new file mode 100644 index 0000000..317b2a4 --- /dev/null +++ b/lib/cfn-model/parser/ec2_network_acl_parser.rb @@ -0,0 +1,56 @@ +# frozen_string_literal: true + +require_relative 'parser_error' +require 'cfn-model/model/ec2_network_acl' +require 'cfn-model/model/references' +require 'cfn-model/util/truthy' + +class Ec2NetworkAclParser + def parse(cfn_model:, resource:) + network_acl = resource + + attach_nacl_entries_to_nacl(cfn_model: cfn_model, network_acl: network_acl) + network_acl + end + + private + + def egress_network_acl_entries(cfn_model) + network_acl_entries = cfn_model.resources_by_type 'AWS::EC2::NetworkAclEntry' + network_acl_entries.select(&:egress) + end + + 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| + not_truthy?(network_acl_entry.egress) + end + end + + def egress_nacl_entries_for_nacl(cfn_model, logical_resource_id) + egress_nacl_entries = egress_network_acl_entries(cfn_model) + egress_nacl_entries.select do |egress_nacl_entry| + References.resolve_resource_id(egress_nacl_entry.networkAclId) == logical_resource_id + end + end + + def ingress_nacl_entries_for_nacl(cfn_model, logical_resource_id) + ingress_nacl_entries = ingress_network_acl_entries(cfn_model) + ingress_nacl_entries.select do |ingress_nacl_entry| + References.resolve_resource_id(ingress_nacl_entry.networkAclId) == logical_resource_id + end + end + + def attach_nacl_entries_for_nacl(cfn_model, network_acl) + egress_nacl_entries_for_nacl(cfn_model, network_acl.logical_resource_id).each do |egress_entry| + network_acl.network_acl_egress_entries << egress_entry.logical_resource_id + end + ingress_nacl_entries_for_nacl(cfn_model, network_acl.logical_resource_id).each do |ingress_entry| + network_acl.network_acl_ingress_entries << ingress_entry.logical_resource_id + end + end + + def attach_nacl_entries_to_nacl(cfn_model:, network_acl:) + attach_nacl_entries_for_nacl(cfn_model, network_acl) + end +end diff --git a/lib/cfn-model/parser/parser_registry.rb b/lib/cfn-model/parser/parser_registry.rb index d550e18..79788d2 100644 --- a/lib/cfn-model/parser/parser_registry.rb +++ b/lib/cfn-model/parser/parser_registry.rb @@ -23,7 +23,8 @@ def initialize 'AWS::SNS::TopicPolicy' => WithPolicyDocumentParser, 'AWS::SQS::QueuePolicy' => WithPolicyDocumentParser, 'AWS::ApiGateway::Stage' => ApiGatewayStageParser, - 'AWS::ApiGateway::Deployment' => ApiGatewayDeploymentParser + 'AWS::ApiGateway::Deployment' => ApiGatewayDeploymentParser, + 'AWS::EC2::NetworkAcl' => Ec2NetworkAclParser } end diff --git a/lib/cfn-model/util/truthy.rb b/lib/cfn-model/util/truthy.rb new file mode 100644 index 0000000..50db482 --- /dev/null +++ b/lib/cfn-model/util/truthy.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +# Checks a string for truthiness. Any cased 'true' will evaluate to a true boolean. +# Any other string _at all_ results in false. +def truthy?(string) + string.to_s.casecmp('true').zero? +end + +def not_truthy?(string) + string.nil? || string.to_s.casecmp('false').zero? +end diff --git a/spec/factories/ec2_network_acl.rb b/spec/factories/ec2_network_acl.rb new file mode 100644 index 0000000..fae316f --- /dev/null +++ b/spec/factories/ec2_network_acl.rb @@ -0,0 +1,42 @@ +require 'cfn-model/model/ec2_network_acl' +require 'cfn-model/model/cfn_model' + +def network_acl_with_one_egress_entry(cfn_model: CfnModel.new) + network_acl = AWS::EC2::NetworkAcl.new cfn_model + network_acl.vpcId = 'testvpc1' + network_acl.network_acl_egress_entries << 'EgressEntry1' + network_acl +end + +def network_acl_with_two_egress_entries(cfn_model: CfnModel.new) + network_acl = AWS::EC2::NetworkAcl.new cfn_model + network_acl.vpcId = 'testvpc1' + %w[EgressEntry1 EgressEntry2].each do |egress_entry| + network_acl.network_acl_egress_entries << egress_entry + end + network_acl +end + +def network_acl_with_one_ingress_entry(cfn_model: CfnModel.new) + network_acl = AWS::EC2::NetworkAcl.new cfn_model + network_acl.vpcId = 'testvpc1' + network_acl.network_acl_ingress_entries << 'IngressEntry1' + network_acl +end + +def network_acl_with_two_ingress_entries(cfn_model: CfnModel.new) + network_acl = AWS::EC2::NetworkAcl.new cfn_model + network_acl.vpcId = 'testvpc1' + %w[IngressEntry1 IngressEntry2].each do |ingress_entry| + network_acl.network_acl_ingress_entries << ingress_entry + end + network_acl +end + +def network_acl_with_egress_and_ingress_entries(cfn_model: CfnModel.new) + network_acl = AWS::EC2::NetworkAcl.new cfn_model + network_acl.vpcId = 'testvpc1' + network_acl.network_acl_egress_entries << 'EgressEntry1' + network_acl.network_acl_ingress_entries << 'IngressEntry1' + network_acl +end diff --git a/spec/parser/cfn_parser_ec2_network_acl_spec.rb b/spec/parser/cfn_parser_ec2_network_acl_spec.rb new file mode 100644 index 0000000..a66727f --- /dev/null +++ b/spec/parser/cfn_parser_ec2_network_acl_spec.rb @@ -0,0 +1,84 @@ +require 'spec_helper' +require 'cfn-model/parser/cfn_parser' + +describe CfnParser do + before :each do + @cfn_parser = CfnParser.new + end + + context 'Network ACL that has one egress entry' do + it 'returns a Network ACL with one egress entry' do + expected_nacls = network_acl_with_one_egress_entry(cfn_model: CfnModel.new) + yaml_test_templates('ec2_network_acl/nacl_with_one_egress_entry').each do |test_template| + cfn_model = @cfn_parser.parse IO.read(test_template) + nacls = cfn_model.resources_by_type 'AWS::EC2::NetworkAcl' + + expect(nacls.size).to eq 1 + expect(nacls[0]).to eq expected_nacls + expect(nacls[0].network_acl_egress_entries).to eq expected_nacls.network_acl_egress_entries + expect(nacls[0].network_acl_egress_entries).not_to be_empty + end + end + end + + context 'Network ACL that has one ingress entry' do + it 'returns a Network ACL with one ingress entry' do + expected_nacls = network_acl_with_one_ingress_entry(cfn_model: CfnModel.new) + yaml_test_templates('ec2_network_acl/nacl_with_one_ingress_entry').each do |test_template| + cfn_model = @cfn_parser.parse IO.read(test_template) + nacls = cfn_model.resources_by_type 'AWS::EC2::NetworkAcl' + + expect(nacls.size).to eq 1 + expect(nacls[0]).to eq expected_nacls + expect(nacls[0].network_acl_ingress_entries).to eq expected_nacls.network_acl_ingress_entries + expect(nacls[0].network_acl_ingress_entries).not_to be_empty + end + end + end + + context 'Network ACL that has two egress entries' do + it 'returns a Network ACL with two egress entries' do + expected_nacls = network_acl_with_two_egress_entries(cfn_model: CfnModel.new) + yaml_test_templates('ec2_network_acl/nacl_with_two_egress_entries').each do |test_template| + cfn_model = @cfn_parser.parse IO.read(test_template) + nacls = cfn_model.resources_by_type 'AWS::EC2::NetworkAcl' + + expect(nacls.size).to eq 1 + expect(nacls[0]).to eq expected_nacls + expect(nacls[0].network_acl_egress_entries).to eq expected_nacls.network_acl_egress_entries + expect(nacls[0].network_acl_egress_entries).not_to be_empty + end + end + end + + context 'Network ACL that has two ingress entries' do + it 'returns a Network ACL with two ingress entries' do + expected_nacls = network_acl_with_two_ingress_entries(cfn_model: CfnModel.new) + yaml_test_templates('ec2_network_acl/nacl_with_two_ingress_entries').each do |test_template| + cfn_model = @cfn_parser.parse IO.read(test_template) + nacls = cfn_model.resources_by_type 'AWS::EC2::NetworkAcl' + + expect(nacls.size).to eq 1 + expect(nacls[0]).to eq expected_nacls + expect(nacls[0].network_acl_ingress_entries).to eq expected_nacls.network_acl_ingress_entries + expect(nacls[0].network_acl_ingress_entries).not_to be_empty + end + end + end + context 'Network ACL that has one egress and ingress entry' do + it 'returns a Network ACL with one egress and ingress entry' do + expected_nacls = network_acl_with_egress_and_ingress_entries(cfn_model: CfnModel.new) + yaml_test_templates('ec2_network_acl/nacl_with_one_egress_and_ingress_entry').each do |test_template| + cfn_model = @cfn_parser.parse IO.read(test_template) + nacls = cfn_model.resources_by_type 'AWS::EC2::NetworkAcl' + + expect(nacls.size).to eq 1 + expect(nacls[0]).to eq expected_nacls + expect(nacls[0].network_acl_egress_entries).to eq expected_nacls.network_acl_egress_entries + expect(nacls[0].network_acl_ingress_entries).to eq expected_nacls.network_acl_ingress_entries + expect(nacls[0].network_acl_egress_entries).not_to be_empty + expect(nacls[0].network_acl_ingress_entries).not_to be_empty + end + end + end +end diff --git a/spec/test_templates/yaml/ec2_network_acl/nacl_with_one_egress_and_ingress_entry.yml b/spec/test_templates/yaml/ec2_network_acl/nacl_with_one_egress_and_ingress_entry.yml new file mode 100644 index 0000000..70c2fe3 --- /dev/null +++ b/spec/test_templates/yaml/ec2_network_acl/nacl_with_one_egress_and_ingress_entry.yml @@ -0,0 +1,38 @@ +--- +Resources: + testvpc1: + Type: AWS::EC2::VPC + Properties: + CidrBlock: "10.0.0.0/16" + EnableDnsSupport: true + EnableDnsHostnames: true + InstanceTenancy: "default" + myNetworkAcl: + Type: AWS::EC2::NetworkAcl + Properties: + VpcId: 'testvpc1' + EgressEntry1: + Type: AWS::EC2::NetworkAclEntry + Properties: + NetworkAclId: !Ref myNetworkAcl + Protocol: "6" + RuleAction: "allow" + RuleNumber: "100" + CidrBlock: "10.0.0.0/16" + Egress: true + PortRange: + From: '443' + To: '443' + IngressEntry1: + Type: AWS::EC2::NetworkAclEntry + Properties: + NetworkAclId: !Ref myNetworkAcl + Protocol: "6" + RuleAction: "allow" + RuleNumber: "100" + CidrBlock: "10.0.0.0/16" + Egress: false + PortRange: + From: '443' + To: '443' + diff --git a/spec/test_templates/yaml/ec2_network_acl/nacl_with_one_egress_entry.yml b/spec/test_templates/yaml/ec2_network_acl/nacl_with_one_egress_entry.yml new file mode 100644 index 0000000..a67d842 --- /dev/null +++ b/spec/test_templates/yaml/ec2_network_acl/nacl_with_one_egress_entry.yml @@ -0,0 +1,25 @@ +--- +Resources: + testvpc1: + Type: AWS::EC2::VPC + Properties: + CidrBlock: "10.0.0.0/16" + EnableDnsSupport: true + EnableDnsHostnames: true + InstanceTenancy: "default" + myNetworkAcl: + Type: AWS::EC2::NetworkAcl + Properties: + VpcId: 'testvpc1' + EgressEntry1: + Type: AWS::EC2::NetworkAclEntry + Properties: + NetworkAclId: !Ref myNetworkAcl + Protocol: "6" + RuleAction: "allow" + RuleNumber: "100" + CidrBlock: "10.0.0.0/16" + Egress: true + PortRange: + From: '443' + To: '443' diff --git a/spec/test_templates/yaml/ec2_network_acl/nacl_with_one_ingress_entry.yml b/spec/test_templates/yaml/ec2_network_acl/nacl_with_one_ingress_entry.yml new file mode 100644 index 0000000..f8a1b8a --- /dev/null +++ b/spec/test_templates/yaml/ec2_network_acl/nacl_with_one_ingress_entry.yml @@ -0,0 +1,25 @@ +--- +Resources: + testvpc1: + Type: AWS::EC2::VPC + Properties: + CidrBlock: "10.0.0.0/16" + EnableDnsSupport: true + EnableDnsHostnames: true + InstanceTenancy: "default" + myNetworkAcl: + Type: AWS::EC2::NetworkAcl + Properties: + VpcId: 'testvpc1' + IngressEntry1: + Type: AWS::EC2::NetworkAclEntry + Properties: + NetworkAclId: !Ref myNetworkAcl + Protocol: "6" + RuleAction: "allow" + RuleNumber: "100" + CidrBlock: "10.0.0.0/16" + Egress: false + PortRange: + From: '443' + To: '443' diff --git a/spec/test_templates/yaml/ec2_network_acl/nacl_with_two_egress_entries.yml b/spec/test_templates/yaml/ec2_network_acl/nacl_with_two_egress_entries.yml new file mode 100644 index 0000000..cb3b683 --- /dev/null +++ b/spec/test_templates/yaml/ec2_network_acl/nacl_with_two_egress_entries.yml @@ -0,0 +1,37 @@ +--- +Resources: + testvpc1: + Type: AWS::EC2::VPC + Properties: + CidrBlock: "10.0.0.0/16" + EnableDnsSupport: true + EnableDnsHostnames: true + InstanceTenancy: "default" + myNetworkAcl: + Type: AWS::EC2::NetworkAcl + Properties: + VpcId: 'testvpc1' + EgressEntry1: + Type: AWS::EC2::NetworkAclEntry + Properties: + NetworkAclId: !Ref myNetworkAcl + Protocol: "6" + RuleAction: "allow" + RuleNumber: "100" + CidrBlock: "10.0.0.0/16" + Egress: true + PortRange: + From: '443' + To: '443' + EgressEntry2: + Type: AWS::EC2::NetworkAclEntry + Properties: + NetworkAclId: !Ref myNetworkAcl + Protocol: "6" + RuleAction: "allow" + RuleNumber: "200" + CidrBlock: "10.0.0.0/16" + Egress: true + PortRange: + From: '80' + To: '80' diff --git a/spec/test_templates/yaml/ec2_network_acl/nacl_with_two_ingress_entries.yml b/spec/test_templates/yaml/ec2_network_acl/nacl_with_two_ingress_entries.yml new file mode 100644 index 0000000..c3602b0 --- /dev/null +++ b/spec/test_templates/yaml/ec2_network_acl/nacl_with_two_ingress_entries.yml @@ -0,0 +1,38 @@ +--- +Resources: + testvpc1: + Type: AWS::EC2::VPC + Properties: + CidrBlock: "10.0.0.0/16" + EnableDnsSupport: true + EnableDnsHostnames: true + InstanceTenancy: "default" + myNetworkAcl: + Type: AWS::EC2::NetworkAcl + Properties: + VpcId: 'testvpc1' + IngressEntry1: + Type: AWS::EC2::NetworkAclEntry + Properties: + NetworkAclId: !Ref myNetworkAcl + Protocol: "6" + RuleAction: "allow" + RuleNumber: "100" + CidrBlock: "10.0.0.0/16" + Egress: false + PortRange: + From: '443' + To: '443' + IngressEntry2: + Type: AWS::EC2::NetworkAclEntry + Properties: + NetworkAclId: !Ref myNetworkAcl + Protocol: "6" + RuleAction: "allow" + RuleNumber: "200" + CidrBlock: "10.0.0.0/16" + Egress: false + PortRange: + From: '80' + To: '80' + diff --git a/spec/util/truthy_spec.rb b/spec/util/truthy_spec.rb new file mode 100644 index 0000000..678bf09 --- /dev/null +++ b/spec/util/truthy_spec.rb @@ -0,0 +1,19 @@ +require 'spec_helper' +require 'cfn-model/util/truthy' + +describe 'truthy', :rule do + + context 'Strings map to boolean' do + it 'given any true string, to_boolean returns true boolean' do + %w[true TRUE tRUE True tRuE TrUe] + .map { |x| truthy?(x) } + .map { |x| expect(x).to eq true } + end + + it 'given any false string, to_boolean returns false boolean' do + %w[false FALSE fALSE False fAlSe FaLsE] + .map { |x| truthy?(x) } + .map { |x| expect(x).to eq false } + end + end +end