Merge pull request #148 from Skyscanner/add-kinesis-data-firehose-ips…

…-filter

Rule Config for Kinesis Data Firehose IPs

README.md

@@ -14,39 +14,42 @@ Docs and more details available in https://cfripper.readthedocs.io/
 ## CLI Usage
 
 ### Normal execution
+
 ```bash
 $ cfripper /tmp/root.yaml /tmp/root_bypass.json --format txt
 Analysing /tmp/root.yaml...
 Not adding CrossAccountTrustRule failure in rootRole because no AWS Account ID was found in the config.
 Valid: False
 Issues found:
-	- FullWildcardPrincipalRule: rootRole should not allow wildcards in principals (principal: '*')
-	- IAMRolesOverprivilegedRule: Role 'rootRole' contains an insecure permission '*' in policy 'root'
+ - FullWildcardPrincipalRule: rootRole should not allow wildcards in principals (principal: '*')
+ - IAMRolesOverprivilegedRule: Role 'rootRole' contains an insecure permission '*' in policy 'root'
 Analysing /tmp/root_bypass.json...
 Valid: True
 ```
 
 ### Using the "resolve" flag
+
 ```bash
 $ cfripper /tmp/root.yaml /tmp/root_bypass.json --format txt --resolve
 Analysing /tmp/root.yaml...
 Not adding CrossAccountTrustRule failure in rootRole because no AWS Account ID was found in the config.
 Valid: False
 Issues found:
-	- FullWildcardPrincipalRule: rootRole should not allow wildcards in principals (principal: '*')
-	- IAMRolesOverprivilegedRule: Role 'rootRole' contains an insecure permission '*' in policy 'root'
+ - FullWildcardPrincipalRule: rootRole should not allow wildcards in principals (principal: '*')
+ - IAMRolesOverprivilegedRule: Role 'rootRole' contains an insecure permission '*' in policy 'root'
 Analysing /tmp/root_bypass.json...
 Not adding CrossAccountTrustRule failure in rootRole because no AWS Account ID was found in the config.
 Valid: False
 Issues found:
-	- IAMRolesOverprivilegedRule: Role 'rootRole' contains an insecure permission '*' in policy 'root'
+ - IAMRolesOverprivilegedRule: Role 'rootRole' contains an insecure permission '*' in policy 'root'
 Monitored issues found:
-	- PartialWildcardPrincipalRule: rootRole contains an unknown principal: 123456789012
-	- PartialWildcardPrincipalRule: rootRole should not allow wildcard in principals or account-wide principals 
+ - PartialWildcardPrincipalRule: rootRole contains an unknown principal: 123456789012
+ - PartialWildcardPrincipalRule: rootRole should not allow wildcard in principals or account-wide principals
 (principal: 'arn:aws:iam::123456789012:root')
 ```
 
 ### Using json format and output-folder argument
+
 ```bash
 $ cfripper /tmp/root.yaml /tmp/root_bypass.json --format json --resolve --output-folder /tmp
 Analysing /tmp/root.yaml...
@@ -57,7 +60,16 @@ Not adding CrossAccountTrustRule failure in rootRole because no AWS Account ID w
 Result saved in /tmp/root_bypass.json.cfripper.results.json
 ```
 
+### Using rules config file
+
+```bash
+$ cfripper tests/test_templates/config/security_group_firehose_ips.json --rules-config-file cfripper/config/rule_configs/example_rules_config_for_cli.py
+Analysing tests/test_templates/config/security_group_firehose_ips.json...
+Valid: True
+```
+
 ### Exit Codes
+
 ```python
 """
 Analyse AWS Cloudformation templates passed by parameter.

cfripper/config/rule_configs/example_rules_config_for_cli.py

@@ -0,0 +1,8 @@
+from cfripper.config.rule_config import RuleConfig
+from cfripper.config.rule_configs.firehose_ips import firehose_ips_rules_config_filter
+from cfripper.model.enums import RuleMode
+
+RULES_CONFIG = {
+    "EC2SecurityGroupMissingEgressRule": RuleConfig(rule_mode=RuleMode.DISABLED),
+    "EC2SecurityGroupOpenToWorldRule": RuleConfig(filters=[firehose_ips_rules_config_filter]),
+}

cfripper/config/rule_configs/firehose_ips.py

@@ -0,0 +1,56 @@
+from cfripper.config.filter import Filter
+from cfripper.model.enums import RuleMode
+
+"""
+To use this RuleConfig, or any RuleConfig, make sure to include it in the `Config` instantiation.
+
+```python
+RULES_CONFIG = {
+    "EC2SecurityGroupOpenToWorldRule": RuleConfig(
+        filters=[firehose_ips_rules_config_filter]
+    )
+}
+
+config = Config(
+    ...
+    rules_config=RULES_CONFIG,
+)
+```
+"""
+
+# Adapted from https://docs.aws.amazon.com/firehose/latest/dev/controlling-access.html
+FIREHOSE_IPS = {
+    "13.113.196.224/27",  # Asia Pacific (Tokyo)
+    "13.209.1.64/27",  # Asia Pacific (Seoul)
+    "13.210.67.224/27",  # Asia Pacific (Sydney)
+    "13.228.64.192/27",  # Asia Pacific (Singapore)
+    "13.232.67.32/27",  # Asia Pacific (Mumbai)
+    "13.244.121.224/277",  # Africa (Cape Town)
+    "13.53.63.224/27",  # Europe (Stockholm)
+    "13.57.135.192/27",  # US West (N. California)
+    "13.58.135.96/27",  # US East (Ohio)
+    "15.161.135.128/27",  # Europe (Milan)
+    "15.185.91.0/27",  # Middle East (Bahrain)
+    "161.189.23.64/27",  # China (Ningxia)
+    "18.130.1.96/27",  # Europe (London)
+    "18.162.221.32/27",  # Asia Pacific (Hong Kong)
+    "18.228.1.128/27",  # South America (São Paulo)
+    "18.253.138.96/27",  # AWS GovCloud (US-East)
+    "35.158.127.160/27",  # Europe (Frankfurt)
+    "35.180.1.96/27",  # Europe (Paris)
+    "35.183.92.128/27",  # Canada (Central)
+    "52.19.239.192/27",  # Europe (Ireland)
+    "52.61.204.160/27",  # AWS GovCloud (US-West)
+    "52.70.63.192/27",  # US East (N. Virginia)
+    "52.81.151.32/27",  # China (Beijing)
+    "52.89.255.224/27",  # US West (Oregon)
+}
+
+firehose_ips_rules_config_filter = Filter(
+    reason=(
+        "Exclude Kinesis Data Firehose IPs to allow access from Amazon Redshift Clusters. "
+        "See https://docs.aws.amazon.com/firehose/latest/dev/controlling-access.html"
+    ),
+    rule_mode=RuleMode.WHITELISTED,
+    eval={"and": [{"exists": {"ref": "ingress_ip"}}, {"in": [{"ref": "ingress_ip"}, FIREHOSE_IPS]}]},
+)

tests/config/test_filter.py

@@ -3,6 +3,7 @@
 from cfripper.config.config import Config
 from cfripper.config.filter import Filter
 from cfripper.config.rule_config import RuleConfig
+from cfripper.config.rule_configs.firehose_ips import firehose_ips_rules_config_filter
 from cfripper.model.enums import RuleMode
 from cfripper.rule_processor import RuleProcessor
 from cfripper.rules import DEFAULT_RULES
@@ -19,6 +20,11 @@ def template_cross_account_role_with_name():
     return get_cfmodel_from("config/cross_account_role_with_name.json").resolve()
 
 
+@pytest.fixture()
+def template_security_group_firehose_ips():
+    return get_cfmodel_from("config/security_group_firehose_ips.json").resolve()
+
+
 @pytest.mark.parametrize(
     "filter, args, expected_result",
     [
@@ -301,3 +307,18 @@ def test_exist_function_and_property_exists(template_cross_account_role_with_nam
     processor = RuleProcessor(*rules)
     result = processor.process_cf_template(template_cross_account_role_with_name, mock_config)
     assert result.valid
+
+
+@pytest.mark.parametrize("filters, valid", [(None, False), ([firehose_ips_rules_config_filter], True)])
+def test_externally_defined_rule_filter(filters, valid, template_security_group_firehose_ips):
+    mock_config = Config(
+        rules=["EC2SecurityGroupOpenToWorldRule"],
+        aws_account_id="123456789",
+        stack_name="mockstack",
+        rules_config={} if not filters else {"EC2SecurityGroupOpenToWorldRule": RuleConfig(filters=filters)},
+    )
+
+    rules = [DEFAULT_RULES.get(rule)(mock_config) for rule in mock_config.rules]
+    processor = RuleProcessor(*rules)
+    result = processor.process_cf_template(template_security_group_firehose_ips, mock_config)
+    assert result.valid == valid

tests/test_templates/config/security_group_firehose_ips.json

@@ -0,0 +1,20 @@
+{
+  "AWSTemplateFormatVersion": "2010-09-09",
+  "Resources": {
+    "RedshiftSecurityGroup": {
+      "Properties": {
+        "GroupDescription": "Enable TCP access on port 5439 from Firehose in eu-west-1",
+        "SecurityGroupIngress": [
+          {
+            "CidrIp": "52.19.239.192/27",
+            "FromPort": "5439",
+            "IpProtocol": "tcp",
+            "ToPort": "5439",
+            "Description": "Allows access from Firehose in eu-west-1"
+          }
+        ]
+      },
+      "Type": "AWS::EC2::SecurityGroup"
+    }
+  }
+}