Extra functions + fix to filters (#112)

* implement_filters_in_crossaccount: refactor + add support for filters

* implement_filters_in_crossaccount: update docs

* implement_filters_in_crossaccount: add tests

* implement_filters_in_crossaccount: update changelog and version

* implement_filters_in_crossaccount: implement exists and empty

* implement_filters_in_crossaccount: format code

* implement_filters_in_crossaccount: update changelog and version

* implement_filters_in_crossaccount: more tests

* implement_filters_in_crossaccount: fix format

CHANGELOG.md

@@ -1,6 +1,13 @@
 # Changelog
 All notable changes to this project will be documented in this file.
 
+## [0.17.1] - 2020-03-30
+### Improvements
+- Add `exists` and `empty` functions to filters
+- Add `param_resolver` to filters to evaluate just necessary params
+### Fixed
+- Add protection when a filter is evaluated to catch the exception and continue
+
 ## [0.17.0] - 2020-03-27
 ### Improvements
 - `CrossAccountCheckingRule`, `CrossAccountTrustRule`, `S3CrossAccountTrustRule` and `KMSKeyCrossAccountTrustRule` include support for filters.

cfripper/__version__.py

@@ -1,3 +1,3 @@
-VERSION = (0, 17, 0)
+VERSION = (0, 17, 1)
 
 __version__ = ".".join(map(str, VERSION))

cfripper/config/filter.py

@@ -1,4 +1,3 @@
-import operator
 import re
 from typing import Any, Callable, Dict, List, Optional, Union
 
@@ -7,19 +6,29 @@
 
 from cfripper.model.enums import RuleMode, RuleRisk
 
+
+def param_resolver(f):
+    def wrap(*args, **kwargs):
+        return f(*(arg(kwargs) for arg in args), **kwargs)
+
+    return wrap
+
+
 IMPLEMENTED_FILTER_FUNCTIONS = {
-    "eq": lambda *args, **kwargs: operator.eq(*args),
-    "ne": lambda *args, **kwargs: operator.ne(*args),
-    "lt": lambda *args, **kwargs: operator.lt(*args),
-    "gt": lambda *args, **kwargs: operator.gt(*args),
-    "le": lambda *args, **kwargs: operator.le(*args),
-    "ge": lambda *args, **kwargs: operator.ge(*args),
-    "not": lambda *args, **kwargs: operator.not_(*args),
-    "or": lambda *args, **kwargs: any(args),
-    "and": lambda *args, **kwargs: all(args),
-    "in": lambda a, b, *args, **kwargs: operator.contains(b, a),
-    "regex": lambda *args, **kwargs: bool(re.match(*args)),
-    "ref": lambda param_name, *args, **kwargs: get(kwargs, param_name),
+    "eq": param_resolver(lambda a, b, **kwargs: a == b),
+    "ne": param_resolver(lambda a, b, **kwargs: a != b),
+    "lt": param_resolver(lambda a, b, **kwargs: a < b),
+    "gt": param_resolver(lambda a, b, **kwargs: a > b),
+    "le": param_resolver(lambda a, b, **kwargs: a <= b),
+    "ge": param_resolver(lambda a, b, **kwargs: a >= b),
+    "not": param_resolver(lambda a, **kwargs: not a),
+    "or": lambda *args, **kwargs: any(arg(kwargs) for arg in args),
+    "and": lambda *args, **kwargs: all(arg(kwargs) for arg in args),
+    "in": param_resolver(lambda a, b, **kwargs: a in b),
+    "regex": param_resolver(lambda *args, **kwargs: bool(re.match(*args))),
+    "exists": param_resolver(lambda a, **kwargs: a is not None),
+    "empty": param_resolver(lambda *args, **kwargs: len(args) == 0),
+    "ref": param_resolver(lambda param_name, **kwargs: get(kwargs, param_name)),
 }
 
 
@@ -34,7 +43,7 @@ def build_evaluator(tree: Union[str, int, float, bool, List, Dict]) -> Callable:
             nodes = [nodes]
         nodes = [build_evaluator(node) for node in nodes]
         function_resolver = IMPLEMENTED_FILTER_FUNCTIONS[function_name]
-        return lambda kwargs: function_resolver(*[node(kwargs) for node in nodes], **kwargs)
+        return lambda kwargs: function_resolver(*nodes, **kwargs)
 
     return lambda kwargs: tree
 

cfripper/rules/base_rules.py

@@ -64,9 +64,13 @@ def add_failure_to_result(
         rule_mode = rule_mode or self.rule_mode
         risk_value = risk_value or self.risk_value
         for fltr in self.rule_config.filters:
-            if fltr(**context):
-                risk_value = fltr.risk_value or risk_value
-                rule_mode = fltr.rule_mode or rule_mode
+            try:
+                if fltr(**context):
+                    risk_value = fltr.risk_value or risk_value
+                    rule_mode = fltr.rule_mode or rule_mode
+            except Exception:
+                logger.exception(f"Exception raised while evaluating filter for `{fltr.reason}`", extra=context)
+
         if rule_mode not in (RuleMode.DISABLED, RuleMode.WHITELISTED):
             result.add_failure(
                 rule=type(self).__name__,

tests/config/test_filter.py

@@ -1,6 +1,22 @@
 import pytest
 
+from cfripper.config.config import Config
 from cfripper.config.filter import Filter
+from cfripper.config.rule_config import RuleConfig
+from cfripper.model.enums import RuleMode
+from cfripper.rule_processor import RuleProcessor
+from cfripper.rules import DEFAULT_RULES
+from tests.utils import get_cfmodel_from
+
+
+@pytest.fixture()
+def template_cross_account_role_no_name():
+    return get_cfmodel_from("config/cross_account_role_no_name.json").resolve()
+
+
+@pytest.fixture()
+def template_cross_account_role_with_name():
+    return get_cfmodel_from("config/cross_account_role_with_name.json").resolve()
 
 
 @pytest.mark.parametrize(
@@ -123,6 +139,22 @@
         (Filter(eval={"regex": [r"\w*\d\s*\d\s*\d\w*", "xx123xx"]}), {}, True),
         (Filter(eval={"regex": [r"^b\w+", "foobar"]}), {}, False),
         (Filter(eval={"regex": [r"\w*b\w+", "foobar"]}), {}, True),
+        (Filter(eval={"exists": None}), {}, False),
+        (Filter(eval={"exists": "string"}), {}, True),
+        (Filter(eval={"exists": 1}), {}, True),
+        (Filter(eval={"exists": -1}), {}, True),
+        (Filter(eval={"exists": 1.0}), {}, True),
+        (Filter(eval={"exists": -1.0}), {}, True),
+        (Filter(eval={"exists": True}), {}, True),
+        (Filter(eval={"exists": False}), {}, True),
+        (Filter(eval={"empty": []}), {}, True),
+        (Filter(eval={"empty": ["string"]}), {}, False),
+        (Filter(eval={"empty": [1]}), {}, False),
+        (Filter(eval={"empty": [-1]}), {}, False),
+        (Filter(eval={"empty": [1.0]}), {}, False),
+        (Filter(eval={"empty": [-1.0]}), {}, False),
+        (Filter(eval={"empty": [True]}), {}, False),
+        (Filter(eval={"empty": [False]}), {}, False),
         (Filter(eval={"ref": "param_a"}), {"param_a": "a"}, "a"),
         (Filter(eval={"ref": "param_a"}), {"param_a": 1}, 1),
         (Filter(eval={"ref": "param_a"}), {"param_a": -1}, -1),
@@ -172,7 +204,83 @@
         (Filter(eval={"eq": [{"ref": "param_a"}, {"ref": "param_b"}]}), {"param_a": "a", "param_b": "a"}, True),
         (Filter(eval={"eq": [{"ref": "param_a"}, {"ref": "param_b"}]}), {"param_a": "a", "param_b": "b"}, False),
         (Filter(eval={"eq": [{"ref": "param_a"}, {"ref": "param_b"}]}), {"param_a": "b", "param_b": "a"}, False),
+        (
+            Filter(eval={"and": [{"exists": {"ref": "param_a.param_b"}}, {"eq": [{"ref": "param_a.param_b"}, "b"]}]}),
+            {},
+            False,
+        ),
+        (
+            Filter(eval={"and": [{"exists": {"ref": "param_a.param_b"}}, {"eq": [{"ref": "param_a.param_b"}, "b"]}]}),
+            {"param_a": {"param_b": "b"}},
+            True,
+        ),
     ],
 )
 def test_filter(filter, args, expected_result):
     assert filter(**args) == expected_result
+
+
+def test_exist_function_and_property_does_not_exist(template_cross_account_role_no_name):
+    mock_config = Config(
+        rules=["CrossAccountTrustRule"],
+        aws_account_id="123456789",
+        stack_name="mockstack",
+        rules_config={
+            "CrossAccountTrustRule": RuleConfig(
+                filters=[
+                    Filter(
+                        rule_mode=RuleMode.WHITELISTED,
+                        eval={
+                            "and": [
+                                {
+                                    "and": [
+                                        {"exists": {"ref": "resource.Properties.RoleName"}},
+                                        {"regex": ["^prefix-.*$", {"ref": "resource.Properties.RoleName"}]},
+                                    ]
+                                },
+                                {"eq": [{"ref": "principal"}, "arn:aws:iam::999999999:role/someuser@bla.com"]},
+                            ]
+                        },
+                    ),
+                ]
+            )
+        },
+    )
+
+    rules = [DEFAULT_RULES.get(rule)(mock_config) for rule in mock_config.rules]
+    processor = RuleProcessor(*rules)
+    result = processor.process_cf_template(template_cross_account_role_no_name, mock_config)
+    assert not result.valid
+
+
+def test_exist_function_and_property_exists(template_cross_account_role_with_name):
+    mock_config = Config(
+        rules=["CrossAccountTrustRule"],
+        aws_account_id="123456789",
+        stack_name="mockstack",
+        rules_config={
+            "CrossAccountTrustRule": RuleConfig(
+                filters=[
+                    Filter(
+                        rule_mode=RuleMode.WHITELISTED,
+                        eval={
+                            "and": [
+                                {
+                                    "and": [
+                                        {"exists": {"ref": "resource.Properties.RoleName"}},
+                                        {"regex": ["^prefix-.*$", {"ref": "resource.Properties.RoleName"}]},
+                                    ]
+                                },
+                                {"eq": [{"ref": "principal"}, "arn:aws:iam::999999999:role/someuser@bla.com"]},
+                            ]
+                        },
+                    ),
+                ]
+            )
+        },
+    )
+
+    rules = [DEFAULT_RULES.get(rule)(mock_config) for rule in mock_config.rules]
+    processor = RuleProcessor(*rules)
+    result = processor.process_cf_template(template_cross_account_role_with_name, mock_config)
+    assert result.valid

tests/test_templates/config/cross_account_role_no_name.json

@@ -0,0 +1,24 @@
+{
+  "AWSTemplateFormatVersion": "2010-09-09",
+  "Resources": {
+    "RootRole": {
+      "Type": "AWS::IAM::Role",
+      "Properties": {
+        "AssumeRolePolicyDocument": {
+          "Version": "2012-10-17",
+          "Statement": [
+            {
+              "Effect": "Allow",
+              "Principal": {
+                "AWS": "arn:aws:iam::999999999:role/someuser@bla.com"
+              },
+              "Action": "sts:AssumeRole"
+            }
+          ]
+        },
+        "Path": "/",
+        "Policies": []
+      }
+    }
+  }
+}

tests/test_templates/config/cross_account_role_with_name.json

@@ -0,0 +1,25 @@
+{
+  "AWSTemplateFormatVersion": "2010-09-09",
+  "Resources": {
+    "RootRole": {
+      "Type": "AWS::IAM::Role",
+      "Properties": {
+        "RoleName": "prefix-test-root-role",
+        "AssumeRolePolicyDocument": {
+          "Version": "2012-10-17",
+          "Statement": [
+            {
+              "Effect": "Allow",
+              "Principal": {
+                "AWS": "arn:aws:iam::999999999:role/someuser@bla.com"
+              },
+              "Action": "sts:AssumeRole"
+            }
+          ]
+        },
+        "Path": "/",
+        "Policies": []
+      }
+    }
+  }
+}