#273 - New rule to warn on ipProtocol -1

.gitignore

@@ -3,3 +3,14 @@
 /coverage
 /spec/aws_sample_templates/
 *.zip
+spec/test_templates/json/iot_policy/UNUSED_iot2_policy_with_wildcard_action.json
+mkmf.log
+simple.json
+simple+iam.json
+test.json
+update_cellar.pl
+.vscode/launch.json
+.vscode/settings.json
+.vscode-upload.json
+IotPolicy-test.json
+test-cfn-model.rb

lib/cfn-nag/custom_rules/SecurityGroupEgressAllProtocolsRule.rb

@@ -0,0 +1,37 @@
+# frozen_string_literal: true
+
+require 'cfn-nag/violation'
+require_relative 'base'
+
+class SecurityGroupEgressAllProtocolsRule < BaseRule
+  def rule_text
+    'Security Groups with an IpProtocol of -1 found'
+  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.to_i == -1
+      end
+
+      !violating_egresses.empty?
+    end
+
+    violating_egresses = cfn_model.standalone_egress.select do |standalone_egress|
+      standalone_egress.ipProtocol.to_i == -1
+    end
+
+    violating_security_groups.map(&:logical_resource_id) + violating_egresses.map(&:logical_resource_id)
+  end
+end

lib/cfn-nag/custom_rules/SecurityGroupIngressAllProtocolsRule.rb

@@ -0,0 +1,37 @@
+# frozen_string_literal: true
+
+require 'cfn-nag/violation'
+require_relative 'base'
+
+class SecurityGroupIngressAllProtocolsRule < BaseRule
+  def rule_text
+    'Security Groups ingress with an ipProtocol of -1 found '
+  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.to_i == -1
+      end
+
+      !violating_ingresses.empty?
+    end
+
+    violating_ingresses = cfn_model.standalone_ingress.select do |standalone_ingress|
+      standalone_ingress.ipProtocol.to_i == -1
+    end
+
+    violating_security_groups.map(&:logical_resource_id) + violating_ingresses.map(&:logical_resource_id)
+  end
+end

spec/custom_rules/SecurityGroupEgressAllProtocolsRule_spec.rb

@@ -0,0 +1,16 @@
+require 'spec_helper'
+require 'cfn-model'
+require 'cfn-nag/custom_rules/SecurityGroupEgressAllProtocolsRule'
+
+describe SecurityGroupEgressAllProtocolsRule 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_allprotocols_egress_rule.json')
+
+      actual_logical_resource_ids = SecurityGroupEgressAllProtocolsRule.new.audit_impl cfn_model
+      expected_logical_resource_ids = %w[securityGroupEgress2 IpProtocol_minus_1_str]
+
+      expect(actual_logical_resource_ids).to eq expected_logical_resource_ids
+    end
+  end
+end

spec/custom_rules/SecurityGroupIngressAllProtocolsRule_spec.rb

@@ -0,0 +1,17 @@
+require 'spec_helper'
+require 'cfn-model'
+require 'cfn-nag/custom_rules/SecurityGroupIngressAllProtocolsRule'
+
+describe SecurityGroupIngressAllProtocolsRule do
+
+  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_allprotocols_ingress_rule.json')
+
+      actual_logical_resource_ids = SecurityGroupIngressAllProtocolsRule.new.audit_impl cfn_model
+      expected_logical_resource_ids = %w[danglingIngress IpProtocol_minus_1_str]
+
+      expect(actual_logical_resource_ids).to eq expected_logical_resource_ids
+    end
+  end
+end

spec/test_templates/json/security_group/dangling_allprotocols_egress_rule.json

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

spec/test_templates/json/security_group/dangling_allprotocols_ingress_rule.json

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