Repository: duo-labs/parliament

Issue:
RESOURCE_MISMATCH not correct for redshift:GetClusterCredentials
It seems like there is a problem with the logic in `is_arn_match`. I ran into an issue with `redshift:GetClusterCredentials`.

Simple repro =>

```
$ parliament --string '
{
    "Statement": [
        {
            "Action": "redshift:GetClusterCredentials",
            "Effect": "Allow",
            "Resource": "arn:aws:redshift:us-west-2:123456789012:dbuser:the_cluster/the_user"
        }
    ],
    "Version": "2012-10-17"
}' --json | jq .
{
  "issue": "RESOURCE_MISMATCH",
  "title": "No resources match for the given action",
  "severity": "MEDIUM",
  "description": "Double check that the actions are valid for the specified resources (and vice versa). See https://iam.cloudonaut.io/reference/ or the AWS docs for details",
  "detail": [
    {
      "action": "redshift:GetClusterCredentials",
      "required_format": "arn:*:redshift:*:*:dbuser:*/*"
    }
  ],
  "location": {
    "actions": [
      "redshift:GetClusterCredentials"
    ],
    "filepath": null
  }
}
```

The expected and actual pattern looks correct to me =>
```
(Pdb) arn_parts
['arn',    '*',   'redshift',          '*',                   '*',              'dbuser',                  '*/*']
(Pdb) resource_parts
['arn', 'aws', 'redshift', 'us-west-2', '123456789012', 'dbuser', 'the_cluster/the_user']
```

All the logic in  `is_arn_match` confuses me. I feel like there could be a simple glob check at the top to circumvent all that custom logic for special things like s3 buckets. However, interestingly, that doesn't work either???

```
>>> fnmatch.fnmatch('arn:*:redshift:*:*:dbuser:*/*', 'arn:aws:redshift:us-west-2:528741615426:dbuser:the_cluster/the_user')
False
```

Which is weird, because translating to regex, then using that does work... 

```
>>> import re
>>> re.match(fnmatch.translate('arn:*:redshift:*:*:dbuser:*/*'), 'arn:aws:redshift:us-west-2:528741615426:dbuser:the_cluster/the_user').group()
'arn:aws:redshift:us-west-2:528741615426:dbuser:the_cluster/the_user'
```

Discussion:

Patch:
diff --git a/parliament/__init__.py b/parliament/__init__.py
index cfa5602..fee4d08 100644
--- a/parliament/__init__.py
+++ b/parliament/__init__.py
@@ -39,7 +39,7 @@ def enhance_finding(finding):
     finding.severity = config_settings["severity"]
     finding.title = config_settings["title"]
     finding.description = config_settings.get("description", "")
-    finding.ignore_locations =  config_settings.get("ignore_locations", None)
+    finding.ignore_locations = config_settings.get("ignore_locations", None)
     return finding
 
 
@@ -90,7 +90,7 @@ def is_arn_match(resource_type, arn_format, resource):
     The problem can be simplified because we only care about globbed strings, not full regexes,
     but there are no Python libraries, but here is one in Go: https://github.com/Pathgather/glob-intersection
 
-    We can some cheat because after the first sections of the arn match, meaning until the 5th colon (with some
+    We can cheat some because after the first sections of the arn match, meaning until the 5th colon (with some
     rules there to allow empty or asterisk sections), we only need to match the ID part.
     So the above is simplified to "*/*" and "*personalize*".
 
@@ -110,12 +110,15 @@ def is_arn_match(resource_type, arn_format, resource):
         if "/" in resource:
             return False
 
+    # The ARN has at least 6 parts, separated by a colon. Ensure these exist.
     arn_parts = arn_format.split(":")
     if len(arn_parts) < 6:
         raise Exception("Unexpected format for ARN: {}".format(arn_format))
     resource_parts = resource.split(":")
     if len(resource_parts) < 6:
         raise Exception("Unexpected format for resource: {}".format(resource))
+
+    # For the first 5 parts (ex. arn:aws:SERVICE:REGION:ACCOUNT:), ensure these match appropriately
     for position in range(0, 5):
         if arn_parts[position] == "*" or arn_parts[position] == "":
             continue
@@ -126,13 +129,21 @@ def is_arn_match(resource_type, arn_format, resource):
         else:
             return False
 
-    arn_id = "".join(arn_parts[5:])
-    resource_id = "".join(resource_parts[5:])
+    # Everything up to and including the account ID section matches, so now try to match the remainder
+
+    arn_id = ":".join(arn_parts[5:])
+    resource_id = ":".join(resource_parts[5:])
+
+    print("Checking {} matches {}".format(arn_id, resource_id))  # TODO REMOVE
 
     # At this point we might have something like:
     # log-group:* for arn_id and
     # log-group:/aws/elasticbeanstalk* for resource_id
 
+    # Alternatively we might have multiple colon separated sections, such as:
+    # dbuser:*/* for arn_id and
+    # dbuser:the_cluster/the_user for resource_id
+
     # Look for exact match
     # Examples:
     # "mybucket", "mybucket" -> True
@@ -149,6 +160,25 @@ def is_arn_match(resource_type, arn_format, resource):
     if "*" not in arn_id and "*" not in resource_id:
         return False
 
+    # Remove the start of a string that matches.
+    # "dbuser:*/*", "dbuser:the_cluster/the_user" -> "*/*", "the_cluster/the_user"
+    # "layer:*:*", "layer:sol-*:*" -> "*:*", "sol-*:*"
+
+    def get_starting_match(s1, s2):
+        match = ""
+        for i in range(len(s1)):
+            if i > len(s2) - 1:
+                break
+            if s1[i] == "*" or s2[i] == "*":
+                break
+            if s1[i] == s2[i]:
+                match += s1[i]
+        return match
+
+    match_len = len(get_starting_match(arn_id, resource_id))
+    arn_id = arn_id[match_len:]
+    resource_id = resource_id[match_len:]
+
     # If either is an asterisk it matches
     # Examples:
     # "*", "mybucket" -> True
@@ -165,7 +195,7 @@ def is_arn_match(resource_type, arn_format, resource):
 
     # If one begins with an asterisk and the other ends with one, it should match
     # Examples:
-    # "*/*" and "*personalize*" -> True
+    # "*/" and "personalize*" -> True
     if (arn_id[0] == "*" and resource_id[-1] == "*") or (
         arn_id[-1] == "*" and resource_id[0] == "*"
     ):
@@ -182,6 +212,7 @@ def is_arn_match(resource_type, arn_format, resource):
 
     # Check situation where it begins and ends with asterisks, such as "*/*"
     if arn_id[0] == "*" and arn_id[-1] == "*":
+        # Now only check the middle matches
         if arn_id[1:-1] in resource_id:
             return True
     if resource_id[0] == "*" and resource_id[-1] == "*":


Test patch:
diff --git a/tests/unit/test_formatting.py b/tests/unit/test_formatting.py
index 748c5c1..fb434a8 100644
--- a/tests/unit/test_formatting.py
+++ b/tests/unit/test_formatting.py
@@ -267,7 +267,7 @@ class TestFormatting(unittest.TestCase):
         )
         print(policy.findings)
         assert_equal(len(policy.findings), 0)
-    
+
     def test_condition_with_null(self):
         policy = analyze_policy_string(
             """{
@@ -287,7 +287,7 @@ class TestFormatting(unittest.TestCase):
         )
         print(policy.findings)
         assert_equal(len(policy.findings), 0)
-    
+
     def test_condition_with_MultiFactorAuthAge(self):
         policy = analyze_policy_string(
             """{
@@ -305,4 +305,43 @@ class TestFormatting(unittest.TestCase):
  }"""
         )
         print(policy.findings)
-        assert_equal(len(policy.findings), 0)
\ No newline at end of file
+        assert_equal(len(policy.findings), 0)
+
+    def test_redshift_GetClusterCredentials(self):
+        policy = analyze_policy_string(
+            """{
+    "Version": "2012-10-17",
+    "Id": "123",
+    "Statement": [
+        {
+            "Action": "redshift:GetClusterCredentials",
+            "Effect": "Allow",
+            "Resource": "arn:aws:redshift:us-west-2:123456789012:dbuser:the_cluster/the_user"
+        }
+    ]
+ }"""
+        )
+
+        # This privilege has a required format of arn:*:redshift:*:*:dbuser:*/*
+        print(policy.findings)
+        assert_equal(len(policy.findings), 0)
+
+    def test_lambda_AddLayerVersionPermission(self):
+        policy = analyze_policy_string(
+            """{
+    "Version": "2012-10-17",
+    "Id": "123",
+    "Statement": [
+        {
+            "Sid": "TestPol",
+            "Effect": "Allow",
+            "Action": "lambda:AddLayerVersionPermission",
+            "Resource": "arn:aws:lambda:*:123456789012:layer:sol-*:*"
+        }
+    ]
+ }"""
+        )
+
+        # This privilege has a required format of arn:*:redshift:*:*:dbuser:*/*
+        print(policy.findings)
+        assert_equal(len(policy.findings), 0)