Add support to EC2 rules for use case where ports are not defined in …

…the template (#215)

* Add support to  for use cases where ports are not defined in the CloudFormation template. By default, this means all ports are included

* Add MIN_PORT_NUMBER constant

Co-authored-by: Oliver Crawford <oliver.crawford@skyscanner.net>
Co-authored-by: Oscar Blanco <oscarbc1996@gmail.com>

CHANGELOG.md

@@ -4,6 +4,7 @@ All notable changes to this project will be documented in this file.
 ## [1.7.1]
 ### Fixes
 - `EBSVolumeHasSSERule` can now understand `encrypted_status` if modelled as a `bool`.
+- Add support to `EC2SecurityGroupOpenToWorldRule` for use cases where ports are not defined in the CloudFormation template. By default, this means all ports are included.
 ### Updates
 - Updated `EBSVolumeHasSSERule` to iterate only over `AWS::EC2::Volume` resources.
 

cfripper/config/constants.py

@@ -88,3 +88,6 @@
     "40fa568277ad703bd160f66ae4f83fc9dfdfd06c2f1b5060ca22442ac3ef8be6",  # AWS GovCloud (US-West) Region
     "540804c33a284a299d2547575ce1010f2312ef3da9b3a053c8bc45bf233e4353",  # All other AWS Regions
 ]
+
+MIN_PORT_NUMBER = 0
+MAX_PORT_NUMBER = 65535

cfripper/rules/ec2_security_group.py

@@ -14,6 +14,7 @@
 from pycfmodel.model.resources.security_group import SecurityGroup
 from pycfmodel.model.resources.security_group_ingress import SecurityGroupIngress, SecurityGroupIngressProperties
 
+from cfripper.config.constants import MAX_PORT_NUMBER, MIN_PORT_NUMBER
 from cfripper.model.enums import RuleGranularity
 from cfripper.model.result import Result
 from cfripper.rules.base_rules import Rule
@@ -37,7 +38,7 @@ def analyse_ingress(
         resource_type: str,
     ):
         if self.non_compliant_ip_range(ingress=ingress):
-            open_ports = list(range(ingress.FromPort, ingress.ToPort + 1))
+            open_ports = list(range(ingress.FromPort or MIN_PORT_NUMBER, (ingress.ToPort or MAX_PORT_NUMBER) + 1))
             non_allowed_open_ports = sorted(set(open_ports))
 
             if non_allowed_open_ports:

tests/rules/test_EC2SecurityGroupOpenToWorldRule.py

@@ -54,6 +54,13 @@ def invalid_security_group_port78_81():
     return get_cfmodel_from("rules/EC2SecurityGroupOpenToWorldRule/invalid_security_group_port78_81.json").resolve()
 
 
+@pytest.fixture()
+def invalid_security_group_no_ports_defined():
+    return get_cfmodel_from(
+        "rules/EC2SecurityGroupOpenToWorldRule/invalid_security_group_no_ports_defined.json"
+    ).resolve()
+
+
 def test_security_group_type_slash0(security_group_type_slash0):
     rule = EC2SecurityGroupOpenToWorldRule(None)
     result = rule.invoke(security_group_type_slash0)
@@ -143,6 +150,30 @@ def test_invalid_security_group_port78_81(invalid_security_group_port78_81):
     )
 
 
+def test_invalid_security_group_no_ports_defined(invalid_security_group_no_ports_defined):
+    rule = EC2SecurityGroupOpenToWorldRule(
+        Config(rules=["EC2SecurityGroupOpenToWorldRule"], aws_account_id="123456789", stack_name="mockstack",)
+    )
+    result = rule.invoke(invalid_security_group_no_ports_defined)
+
+    assert not result.valid
+    assert compare_lists_of_failures(
+        result.failures,
+        [
+            Failure(
+                granularity=RuleGranularity.RESOURCE,
+                reason="Port(s) 0-65535 open to public IPs: (23.45.67.88/29) in security group 'SecurityGroup'",
+                risk_value=RuleRisk.MEDIUM,
+                rule="EC2SecurityGroupOpenToWorldRule",
+                rule_mode=RuleMode.BLOCKING,
+                actions=None,
+                resource_ids={"SecurityGroup"},
+                resource_types={"AWS::EC2::SecurityGroup"},
+            )
+        ],
+    )
+
+
 def test_invalid_security_group_cidripv6(invalid_security_group_cidripv6):
     rule = EC2SecurityGroupOpenToWorldRule(None)
     result = rule.invoke(invalid_security_group_cidripv6)

tests/test_templates/rules/EC2SecurityGroupOpenToWorldRule/invalid_security_group_no_ports_defined.json

@@ -0,0 +1,18 @@
+{
+  "AWSTemplateFormatVersion": "2010-09-09",
+  "Resources": {
+    "SecurityGroup": {
+      "Type": "AWS::EC2::SecurityGroup",
+      "Properties": {
+        "GroupDescription": "description",
+        "SecurityGroupIngress": [
+          {
+            "Description": "Some Office IP",
+            "IpProtocol": "-1",
+            "CidrIp": "23.45.67.88/29"
+          }
+        ]
+      }
+    }
+  }
+}