[BUGFIX] Fix CrossAccountRule when trust relationship contains a serv…

…ice (#57) * add check for service * add test * reformat code * bump version
Skyscanner · Oct 30, 2019 · 655661d · 655661d
1 parent be1170b
commit 655661d
Show file tree

Hide file tree

Showing 4 changed files with 70 additions and 4 deletions.
diff --git a/cfripper/rules/CrossAccountTrustRule.py b/cfripper/rules/CrossAccountTrustRule.py
@@ -39,6 +39,7 @@ def invoke(self, cfmodel):
                 for principal in resource.Properties.AssumeRolePolicyDocument.allowed_principals_with(
                     not_has_account_id
                 ):
-                    self.add_failure(
-                        type(self).__name__, self.REASON.format(logical_id, principal), resource_ids={logical_id}
-                    )
+                    if not principal.endswith(".amazonaws.com"):  # Checks if principal is an AWS service
+                        self.add_failure(
+                            type(self).__name__, self.REASON.format(logical_id, principal), resource_ids={logical_id}
+                        )
diff --git a/setup.py b/setup.py
@@ -16,7 +16,7 @@
 
 setup(
     name="cfripper",
-    version="0.9.0",
+    version="0.9.1",
     author="Skyscanner Product Security",
     author_email="security@skyscanner.net",
     long_description=long_description,

diff --git a/tests/rules/test_CrossAccountTrustRule.py b/tests/rules/test_CrossAccountTrustRule.py
@@ -35,6 +35,11 @@ def template_two_roles_dict():
     return get_cfmodel_from("rules/CrossAccountTrustRule/iam_root_role_cross_account_two_roles.json").resolve()
 
 
+@pytest.fixture()
+def template_valid_with_service():
+    return get_cfmodel_from("rules/CrossAccountTrustRule/valid_with_service.json").resolve()
+
+
 @pytest.fixture()
 def expected_result_two_roles():
     return [
@@ -156,3 +161,13 @@ def test_non_whitelisted_stacks_are_reported_normally(template_two_roles_dict, e
     processor.process_cf_template(template_two_roles_dict, mock_config, result)
     assert not result.valid
     assert result.failed_rules == expected_result_two_roles
+
+
+def test_service_is_not_blocked(template_valid_with_service):
+    result = Result()
+    rule = CrossAccountTrustRule(Config(), result)
+    rule.invoke(template_valid_with_service)
+
+    assert result.valid
+    assert len(result.failed_rules) == 0
+    assert len(result.failed_monitored_rules) == 0
diff --git a/tests/test_templates/rules/CrossAccountTrustRule/valid_with_service.json b/tests/test_templates/rules/CrossAccountTrustRule/valid_with_service.json
@@ -0,0 +1,50 @@
+{
+  "AWSTemplateFormatVersion": "2010-09-09",
+  "Parameters": {},
+  "Resources": {
+    "LambdaRole": {
+      "Type": "AWS::IAM::Role",
+      "Properties": {
+        "RoleName": {
+          "Fn::Sub": "${AWS::StackName}-lambda-role"
+        },
+        "AssumeRolePolicyDocument": {
+          "Version": "2012-10-17",
+          "Statement": [
+            {
+              "Effect": "Allow",
+              "Principal": {
+                "Service": [
+                  "lambda.amazonaws.com"
+                ]
+              },
+              "Action": [
+                "sts:AssumeRole"
+              ]
+            }
+          ]
+        },
+        "Path": "/",
+        "Policies": [
+          {
+            "PolicyName": "cloudwatch_logging",
+            "PolicyDocument": {
+              "Version": "2012-10-17",
+              "Statement": [
+                {
+                  "Effect": "Allow",
+                  "Action": [
+                    "logs:CreateLogGroup",
+                    "logs:CreateLogStream",
+                    "logs:PutLogEvents"
+                  ],
+                  "Resource": "*"
+                }
+              ]
+            }
+          }
+        ]
+      }
+    }
+  }
+}