Create new ELB checker rule (#249)

* create new rule * create new rule * refactor tests * Create kms example rule (#248) * Create new stack name rule. * remove unused import * review comments * lint error * add new db encryption rule * remove changes from other pr * Update lint-and-test.yml (#247) * Update lint-and-test.yml * Update pyyaml dependency * Update README.md (#246) * Update README.md * Add license badge * rebase * rebase onto master * rebase * make lint * remove duplicate test * update changelog * add comment as for stack name rule * make format * rule not invoked for aurora * make templates valid cloud formations (except for aurora one) * make templates valid cloud formations * Update tests/rules/test_StorageEncryptedRule.py Co-authored-by: Ignacio Bolonio <[email protected]> * add aurora comment --------- Co-authored-by: Jordi Soucheiron <[email protected]> Co-authored-by: Ignacio Bolonio <[email protected]> * update changelog --------- Co-authored-by: Jordi Soucheiron <[email protected]> Co-authored-by: Ignacio Bolonio <[email protected]>
Skyscanner · Nov 16, 2023 · 5d57047 · 5d57047
1 parent d7848ba
commit 5d57047
Show file tree

Hide file tree

Showing 8 changed files with 234 additions and 1 deletion.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -3,7 +3,7 @@ All notable changes to this project will be documented in this file.
 
 ## [1.15.0]
 ### Additions
-- New rules: `StackNameMatchesRegexRule` and `StorageEncryptedRule`
+- New rules: `PublicELBCheckerRule`, `StackNameMatchesRegexRule`, and `StorageEncryptedRule`
 - New regex: `REGEX_ALPHANUMERICAL_OR_HYPHEN` to check if stack name only consists of alphanumerical characters and hyphens.
 
 ## [1.14.0]

diff --git a/cfripper/rules/__init__.py b/cfripper/rules/__init__.py
@@ -23,6 +23,7 @@
 from cfripper.rules.managed_policy_on_user import ManagedPolicyOnUserRule
 from cfripper.rules.policy_on_user import PolicyOnUserRule
 from cfripper.rules.privilege_escalation import PrivilegeEscalationRule
+from cfripper.rules.public_elb_checker_rule import PublicELBCheckerRule
 from cfripper.rules.rds_security_group import RDSSecurityGroupIngressOpenToWorldRule
 from cfripper.rules.s3_bucket_policy import S3BucketPolicyPrincipalRule
 from cfripper.rules.s3_lifecycle_configuration import S3LifecycleConfigurationRule
@@ -80,6 +81,7 @@
         PartialWildcardPrincipalRule,
         PolicyOnUserRule,
         PrivilegeEscalationRule,
+        PublicELBCheckerRule,
         RDSSecurityGroupIngressOpenToWorldRule,
         S3BucketPolicyPrincipalRule,
         S3LifecycleConfigurationRule,

diff --git a/cfripper/rules/public_elb_checker_rule.py b/cfripper/rules/public_elb_checker_rule.py
@@ -0,0 +1,41 @@
+from typing import Dict, Optional
+
+from pycfmodel.model.resources.generic_resource import GenericResource
+
+from cfripper.model.enums import RuleGranularity, RuleMode, RuleRisk
+from cfripper.model.result import Result
+from cfripper.rules.base_rules import ResourceSpecificRule
+
+
+class PublicELBCheckerRule(ResourceSpecificRule):
+    """
+    Rule to check if a public facing ELB is being created.
+    """
+
+    RESOURCE_TYPES = (GenericResource,)
+    ELB_RESOURCE_TYPES = ["AWS::ElasticLoadBalancing::LoadBalancer", "AWS::ElasticLoadBalancingV2::LoadBalancer"]
+    RISK_VALUE = RuleRisk.LOW
+    RULE_MODE = RuleMode.BLOCKING
+    REASON = "Creation of public facing ELBs is restricted. LogicalId: {}"
+
+    def resource_invoke(self, resource: GenericResource, logical_id: str, extras: Optional[Dict] = None) -> Result:
+        result = Result()
+        if resource.Type in self.ELB_RESOURCE_TYPES:
+            elb_scheme = getattr(resource.Properties, "Scheme", "internal")
+
+            if elb_scheme == "internet-facing":
+                self.add_failure_to_result(
+                    result=result,
+                    reason=self.REASON.format(logical_id),
+                    resource_ids={logical_id},
+                    resource_types={resource.Type},
+                    context={
+                        "config": self._config,
+                        "extras": extras,
+                        "logical_id": logical_id,
+                        "resource": resource,
+                    },
+                    granularity=RuleGranularity.RESOURCE,
+                )
+
+        return result
diff --git a/tests/rules/test_PublicELBCheckerRule.py b/tests/rules/test_PublicELBCheckerRule.py
@@ -0,0 +1,58 @@
+import pytest
+
+from cfripper.model.result import Failure
+from cfripper.rules.public_elb_checker_rule import PublicELBCheckerRule
+from tests.utils import get_cfmodel_from
+
+
+@pytest.mark.parametrize(
+    "template",
+    [
+        "rules/PublicELBCheckerRule/private_elb_instance.yml",
+        "rules/PublicELBCheckerRule/private_elb_v2_instance.yml",
+    ],
+)
+def test_invoke_private_elbs_passes(template):
+    rule = PublicELBCheckerRule(None)
+    rule._config.stack_name = "stackname"
+    result = rule.invoke(cfmodel=get_cfmodel_from(template).resolve())
+
+    assert result.valid
+    assert result.failures == []
+
+
+@pytest.mark.parametrize(
+    "template, logical_id, resource_type, reason",
+    [
+        (
+            "rules/PublicELBCheckerRule/public_facing_elb_instance.yml",
+            "PublicLoadBalancer",
+            "AWS::ElasticLoadBalancing::LoadBalancer",
+            "Creation of public facing ELBs is restricted. LogicalId: PublicLoadBalancer",
+        ),
+        (
+            "rules/PublicELBCheckerRule/public_facing_elb_v2_instance.yml",
+            "PublicV2LoadBalancer",
+            "AWS::ElasticLoadBalancingV2::LoadBalancer",
+            "Creation of public facing ELBs is restricted. LogicalId: PublicV2LoadBalancer",
+        ),
+    ],
+)
+def test_invoke_public_elbs_fail(template, logical_id, resource_type, reason):
+    rule = PublicELBCheckerRule(None)
+    rule._config.stack_name = "stackname"
+    result = rule.invoke(cfmodel=get_cfmodel_from(template).resolve())
+
+    assert result.valid is False
+    assert result.failures == [
+        Failure(
+            granularity="RESOURCE",
+            reason=reason,
+            risk_value="LOW",
+            rule="PublicELBCheckerRule",
+            rule_mode="BLOCKING",
+            actions=None,
+            resource_ids={logical_id},
+            resource_types={resource_type},
+        )
+    ]
diff --git a/tests/test_templates/rules/PublicELBCheckerRule/private_elb_instance.yml b/tests/test_templates/rules/PublicELBCheckerRule/private_elb_instance.yml
@@ -0,0 +1,32 @@
+Resources:
+  PublicLoadBalancer:
+    Type: 'AWS::ElasticLoadBalancing::LoadBalancer'
+    Properties:
+      Name: 'AWS::StackName-extlb'
+      Scheme: internal
+      SecurityGroups:
+        - !GetAtt
+          - LoadBalancerHttpsSG
+          - GroupId
+      Subnets: !If
+        - ExtLoadBalancer
+        - - !ImportValue PublicSubnetA
+          - !ImportValue PublicSubnetB
+          - !ImportValue PublicSubnetC
+        - - !ImportValue PrivateSubnetA
+          - !ImportValue PrivateSubnetB
+          - !ImportValue PrivateSubnetC
+      ConnectionSettings:
+        - IdleTimeout: 3600
+      Tags:
+        - Key: Name
+          Value: !Join
+            - '-'
+            - - !Ref 'AWS::StackName'
+              - LoadBalancerv2
+        - Key: Project
+          Value: !Ref ProjectName
+        - Key: Contact
+          Value: !Ref ContactEmail
+        - Key: StackName
+          Value: !Ref 'AWS::StackName'
diff --git a/tests/test_templates/rules/PublicELBCheckerRule/private_elb_v2_instance.yml b/tests/test_templates/rules/PublicELBCheckerRule/private_elb_v2_instance.yml
@@ -0,0 +1,34 @@
+Resources:
+  PublicV2LoadBalancer:
+    Type: 'AWS::ElasticLoadBalancingV2::LoadBalancer'
+    Properties:
+      Name: 'AWS::StackName-extlb'
+      Scheme: internal
+      SecurityGroups:
+        - !GetAtt
+          - LoadBalancerHttpsSG
+          - GroupId
+      Subnets: !If
+        - ExtLoadBalancer
+        - - !ImportValue PublicSubnetA
+          - !ImportValue PublicSubnetB
+          - !ImportValue PublicSubnetC
+        - - !ImportValue PrivateSubnetA
+          - !ImportValue PrivateSubnetB
+          - !ImportValue PrivateSubnetC
+      Type: application
+      LoadBalancerAttributes:
+        - Key: idle_timeout.timeout_seconds
+          Value: '3600'
+      Tags:
+        - Key: Name
+          Value: !Join
+            - '-'
+            - - !Ref 'AWS::StackName'
+              - LoadBalancerv2
+        - Key: Project
+          Value: !Ref ProjectName
+        - Key: Contact
+          Value: !Ref ContactEmail
+        - Key: StackName
+          Value: !Ref 'AWS::StackName'
diff --git a/tests/test_templates/rules/PublicELBCheckerRule/public_facing_elb_instance.yml b/tests/test_templates/rules/PublicELBCheckerRule/public_facing_elb_instance.yml
@@ -0,0 +1,32 @@
+Resources:
+  PublicLoadBalancer:
+    Type: 'AWS::ElasticLoadBalancing::LoadBalancer'
+    Properties:
+      Name: 'AWS::StackName-extlb'
+      Scheme: internet-facing
+      SecurityGroups:
+        - !GetAtt
+          - LoadBalancerHttpsSG
+          - GroupId
+      Subnets: !If
+        - ExtLoadBalancer
+        - - !ImportValue PublicSubnetA
+          - !ImportValue PublicSubnetB
+          - !ImportValue PublicSubnetC
+        - - !ImportValue PrivateSubnetA
+          - !ImportValue PrivateSubnetB
+          - !ImportValue PrivateSubnetC
+      ConnectionSettings:
+        - IdleTimeout: 3600
+      Tags:
+        - Key: Name
+          Value: !Join
+            - '-'
+            - - !Ref 'AWS::StackName'
+              - LoadBalancerv2
+        - Key: Project
+          Value: !Ref ProjectName
+        - Key: Contact
+          Value: !Ref ContactEmail
+        - Key: StackName
+          Value: !Ref 'AWS::StackName'
diff --git a/tests/test_templates/rules/PublicELBCheckerRule/public_facing_elb_v2_instance.yml b/tests/test_templates/rules/PublicELBCheckerRule/public_facing_elb_v2_instance.yml
@@ -0,0 +1,34 @@
+Resources:
+  PublicV2LoadBalancer:
+    Type: 'AWS::ElasticLoadBalancingV2::LoadBalancer'
+    Properties:
+      Name: 'AWS::StackName-extlb'
+      Scheme: internet-facing
+      SecurityGroups:
+        - !GetAtt
+          - LoadBalancerHttpsSG
+          - GroupId
+      Subnets: !If
+        - ExtLoadBalancer
+        - - !ImportValue PublicSubnetA
+          - !ImportValue PublicSubnetB
+          - !ImportValue PublicSubnetC
+        - - !ImportValue PrivateSubnetA
+          - !ImportValue PrivateSubnetB
+          - !ImportValue PrivateSubnetC
+      Type: application
+      LoadBalancerAttributes:
+        - Key: idle_timeout.timeout_seconds
+          Value: '3600'
+      Tags:
+        - Key: Name
+          Value: !Join
+            - '-'
+            - - !Ref 'AWS::StackName'
+              - LoadBalancerv2
+        - Key: Project
+          Value: !Ref ProjectName
+        - Key: Contact
+          Value: !Ref ContactEmail
+        - Key: StackName
+          Value: !Ref 'AWS::StackName'