#273 - New rule to warn on ipProtocol -1

lib/cfn-nag/custom_rules/SecurityGroupEgressPortRangeRule2.rb

@@ -0,0 +1,38 @@
+# frozen_string_literal: true
+
+require 'cfn-nag/violation'
+require_relative 'base'
+
+class SecurityGroupEgressPortRangeRule2 < BaseRule
+  def rule_text
+    'Security Groups with an IpProtocol of -1 found egress with port range'
+  end
+
+  def rule_type
+    Violation::WARNING
+  end
+
+  def rule_id
+    'W40'
+  end
+
+  ##
+  # This will behave slightly different than the legacy jq based rule which was
+  # targeted against inline ingress only
+  def audit_impl(cfn_model)
+    violating_security_groups = cfn_model.security_groups.select do |security_group|
+      violating_egresses = security_group.egresses.select do |egress|
+        egress.ipProtocol == -1 && egress.fromPort != egress.toPort
+      end
+
+        !violating_egresses.empty?
+     end
+
+     # TODO: research standalone egress, is there a CF example?
+     violating_egresses = cfn_model.standalone_egress.select do |standalone_egress|
+        standalone_egress.ipProtocol == -1 && standalone_egress.fromPort != standalone_egress.toPort
+      end
+
+    violating_security_groups.map(&:logical_resource_id) + violating_egresses.map(&:logical_resource_id)
+  end
+end

lib/cfn-nag/custom_rules/SecurityGroupIngressPortRangeRule2.rb

@@ -0,0 +1,37 @@
+# frozen_string_literal: true
+
+require 'cfn-nag/violation'
+require_relative 'base'
+
+class SecurityGroupIngressPortRangeRule2 < BaseRule
+  def rule_text
+    'Security Groups with an ipProtocol of -1 found ingress with port range'
+  end
+
+  def rule_type
+    Violation::WARNING
+  end
+
+  def rule_id
+    'W41'
+  end
+
+  ##
+  # This will behave slightly different than the legacy jq based rule which was
+  # targeted against inline ingress only
+  def audit_impl(cfn_model)
+    violating_security_groups = cfn_model.security_groups.select do |security_group|
+      violating_ingresses = security_group.ingresses.select do |ingress|
+        ingress.ipProtocol == -1 && ingress.fromPort != ingress.toPort
+      end
+
+      !violating_ingresses.empty?
+    end
+
+    violating_ingresses = cfn_model.standalone_ingress.select do |standalone_ingress|
+      standalone_ingress.ipProtocol == -1 && standalone_ingress.fromPort != standalone_ingress.toPort
+    end
+
+    violating_security_groups.map(&:logical_resource_id) + violating_ingresses.map(&:logical_resource_id)
+  end
+end

scripts/deploy_local.sh

@@ -2,5 +2,6 @@
 
 echo "Installing cfn_nag from local source"
 gem uninstall cfn-nag -x
-GEM_VERSION=0.0.01 gem build cfn-nag.gemspec
+GEM_VERSION=0.0.01
+gem build cfn-nag.gemspec
 gem install cfn-nag-0.0.01.gem --no-document

spec/custom_rules/SecurityGroupEgressPortRangeRule2_spec.rb

@@ -0,0 +1,16 @@
+require 'spec_helper'
+require 'cfn-model'
+require 'cfn-nag/custom_rules/SecurityGroupEgressPortRangeRule2'
+
+describe SecurityGroupEgressPortRangeRule2 do
+  context 'security group with egress rules open to port range' do
+    it 'returns offending logical resource id' do
+      cfn_model = CfnParser.new.parse read_test_template('json/security_group/dangling_egress_rule_2.json')
+
+      actual_logical_resource_ids = SecurityGroupEgressPortRangeRule2.new.audit_impl cfn_model
+      expected_logical_resource_ids = %w[securityGroupEgress2]
+
+      expect(actual_logical_resource_ids).to eq expected_logical_resource_ids
+    end
+  end
+end

spec/custom_rules/SecurityGroupIngressPortRangeRule2_spec.rb

@@ -0,0 +1,27 @@
+require 'spec_helper'
+require 'cfn-model'
+require 'cfn-nag/custom_rules/SecurityGroupIngressPortRangeRule2'
+
+describe SecurityGroupIngressPortRangeRule2 do
+#   context 'security group with ingress rules open to port range' do
+#     it 'returns offending logical resource id' do
+#       cfn_model = CfnParser.new.parse read_test_template('json/security_group/single_security_group_one_cidr_ingress.json')
+
+#       actual_logical_resource_ids = SecurityGroupIngressPortRangeRule2.new.audit_impl cfn_model
+#       expected_logical_resource_ids = %w[sg]
+
+#       expect(actual_logical_resource_ids).to eq expected_logical_resource_ids
+#     end
+#   end
+
+  context 'dangling security group ingress rules open to port range' do
+    it 'returns offending logical resource id' do
+      cfn_model = CfnParser.new.parse read_test_template('json/security_group/dangling_ingress_rule_2.json')
+
+      actual_logical_resource_ids = SecurityGroupIngressPortRangeRule2.new.audit_impl cfn_model
+      expected_logical_resource_ids = %w[danglingIngress]
+
+      expect(actual_logical_resource_ids).to eq expected_logical_resource_ids
+    end
+  end
+end

spec/test_templates/json/security_group/dangling_egress_rule_2.json

@@ -0,0 +1,14 @@
+{
+  "Resources": {
+    "securityGroupEgress2" : {
+      "Type" : "AWS::EC2::SecurityGroupEgress",
+      "Properties" : {
+        "GroupId": "sg-09fb296e",
+        "CidrIp" : "1.2.3.5/32",
+        "FromPort" : 45,
+        "ToPort" : 46,
+        "IpProtocol" : -1
+      }
+    }
+  }
+}

spec/test_templates/json/security_group/dangling_ingress_rule_2.json

@@ -0,0 +1,14 @@
+{
+  "Resources": {
+    "danglingIngress" : {
+      "Type" : "AWS::EC2::SecurityGroupIngress",
+      "Properties" : {
+        "GroupId": "sg-09fb296e",
+        "CidrIp" : "1.2.3.5/16",
+        "FromPort" : 45,
+        "ToPort" : 46,
+        "IpProtocol" : -1
+      }
+    }
+  }
+}