Repository: salesforce/policy_sentry
Pull request #204 (2020-07-18 06:14:58)

Add support for resource type wildcards
When specifying an ARN and setting a `*` on the resource type (e.g. instead of `db:instance` it is `*:*`), multiple ARNs can match. Previously, this would fail and return empty results. Now it correctly returns the full list of matching ARNs and corresponding actions.

Let me know what you think about this change. From what I can tell it is fairly minor. The only potential issue I identified was `does_arn_match` also being called in `policy_sentry/writing/sid_group.py:215`. This PR would actually add support for this kind of wildcard to that code path as well, but make sure that's actually desired.

salesforce-cla[bot]: Thanks for the contribution!  Before we can merge this, we need @fruechel to [sign the Salesforce.com Contributor License Agreement](https://cla.salesforce.com/sign-cla?prUrl=https%3A%2F%2Fgithub.com%2Fsalesforce%2Fpolicy_sentry%2Fpull%2F204).

Patch:
diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml
index b40f39d5..484c42d4 100644
--- a/.github/workflows/publish.yml
+++ b/.github/workflows/publish.yml
@@ -25,11 +25,11 @@ jobs:
 
       - run: invoke build.install-package
       - run: invoke test.format
-      - run: invoke test.lint
       - run: invoke integration.clean
       - run: invoke integration.version
       - run: invoke integration.initialize
       - run: invoke unit.pytest
+      - run: invoke test.lint
       - run: invoke test.security
       - run: invoke integration.query
       - run: invoke integration.write-policy
diff --git a/policy_sentry/command/query.py b/policy_sentry/command/query.py
index c3e77d1f..80660f7d 100644
--- a/policy_sentry/command/query.py
+++ b/policy_sentry/command/query.py
@@ -4,8 +4,8 @@ Allow users to use specific pre-compiled queries against the action, arn, and co
 import json
 import logging
 import click
-import yaml
 import click_log
+import yaml
 from policy_sentry.util.access_levels import transform_access_level_text
 from policy_sentry.querying.all import get_all_service_prefixes
 from policy_sentry.querying.arns import (
diff --git a/policy_sentry/command/write_policy.py b/policy_sentry/command/write_policy.py
index d6f67f07..0372c5b6 100755
--- a/policy_sentry/command/write_policy.py
+++ b/policy_sentry/command/write_policy.py
@@ -4,9 +4,9 @@ Given a Policy Sentry YML template, write a least-privilege IAM Policy in CRUD m
 import sys
 import json
 import logging
-import yaml
 import click
 import click_log
+import yaml
 from policy_sentry.util.file import read_yaml_file
 from policy_sentry.writing.sid_group import SidGroup
 
diff --git a/policy_sentry/querying/actions.py b/policy_sentry/querying/actions.py
index 646b3584..9e8890fe 100644
--- a/policy_sentry/querying/actions.py
+++ b/policy_sentry/querying/actions.py
@@ -6,7 +6,7 @@ import logging
 import functools
 from policy_sentry.shared.iam_data import iam_definition, get_service_prefix_data
 from policy_sentry.querying.all import get_all_service_prefixes
-from policy_sentry.querying.arns import get_matching_raw_arn, get_resource_type_name_with_raw_arn
+from policy_sentry.querying.arns import get_matching_raw_arns, get_resource_type_name_with_raw_arn
 from policy_sentry.util.arns import get_service_from_arn
 
 all_service_prefixes = get_all_service_prefixes()
@@ -218,16 +218,17 @@ def get_actions_matching_arn(arn):
     Returns:
         List: A list of all actions that can match it.
     """
-    raw_arn = get_matching_raw_arn(arn)
-    resource_type_name = get_resource_type_name_with_raw_arn(raw_arn)
-    service_prefix = get_service_from_arn(raw_arn)
-    service_prefix_data = get_service_prefix_data(service_prefix)
+    raw_arns = get_matching_raw_arns(arn)
     results = []
-    for some_action in service_prefix_data["privileges"]:
-        for some_resource_type in some_action["resource_types"]:
-            this_resource_type = some_resource_type["resource_type"].strip("*")
-            if this_resource_type.lower() == resource_type_name.lower():
-                results.append(f"{service_prefix}:{some_action['privilege']}")
+    for raw_arn in raw_arns:
+        resource_type_name = get_resource_type_name_with_raw_arn(raw_arn)
+        service_prefix = get_service_from_arn(raw_arn)
+        service_prefix_data = get_service_prefix_data(service_prefix)
+        for some_action in service_prefix_data["privileges"]:
+            for some_resource_type in some_action["resource_types"]:
+                this_resource_type = some_resource_type["resource_type"].strip("*")
+                if this_resource_type.lower() == resource_type_name.lower():
+                    results.append(f"{service_prefix}:{some_action['privilege']}")
     results = list(dict.fromkeys(results))
     results.sort()
     return results
diff --git a/policy_sentry/querying/arns.py b/policy_sentry/querying/arns.py
index 8196c1d2..52d455be 100644
--- a/policy_sentry/querying/arns.py
+++ b/policy_sentry/querying/arns.py
@@ -112,21 +112,21 @@ def get_resource_type_name_with_raw_arn(raw_arn):
             return resource["resource"]
 
 
-def get_matching_raw_arn(arn):
+def get_matching_raw_arns(arn):
     """
-    Given a user-supplied ARN, return the raw_arn since that is used as a unique identifier throughout this library
+    Given a user-supplied ARN, return the list of raw_arns since that is used as a unique identifier throughout this library
 
     Arguments:
         arn: The user-supplied arn, like arn:aws:s3:::mybucket
     Returns:
-        String: The raw ARN stored in the database, like 'arn:${Partition}:s3:::${BucketName}'
+        list(str): The list of raw ARNs stored in the database, like 'arn:${Partition}:s3:::${BucketName}'
     """
-    result = None
+    result = []
     service_in_scope = get_service_from_arn(arn)
     # Determine which resource it applies to
     all_raw_arns_for_service = get_raw_arns_for_service(service_in_scope)
     # Get the raw ARN specific to the provided one
     for raw_arn in all_raw_arns_for_service:
-        if does_arn_match(arn, raw_arn):
-            result = raw_arn
+        if does_arn_match(arn, raw_arn) and raw_arn not in result:
+            result.append(raw_arn)
     return result
diff --git a/policy_sentry/util/arns.py b/policy_sentry/util/arns.py
index b70bbafa..5345bccc 100644
--- a/policy_sentry/util/arns.py
+++ b/policy_sentry/util/arns.py
@@ -180,7 +180,7 @@ def does_arn_match(arn_to_test, arn_in_database):
         if get_service_from_arn(arn_in_database) != get_service_from_arn(arn_to_test):
             score += 10
             return False
-        if resource_type_arn_in_database == resource_type_arn_to_test:
+        if resource_type_arn_to_test in (resource_type_arn_in_database, '*'):
             return True
         else:
             score += 1


Test patch:
diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
index 863ac40c..813c6a2c 100644
--- a/.github/workflows/test.yml
+++ b/.github/workflows/test.yml
@@ -22,11 +22,11 @@ jobs:
 
       - run: invoke build.install-package
       - run: invoke test.format
-      - run: invoke test.lint
       - run: invoke integration.clean
       - run: invoke integration.version
       - run: invoke integration.initialize
       - run: invoke unit.pytest
+      - run: invoke test.lint
       - run: invoke test.security
       - run: invoke integration.query
       - run: invoke integration.write-policy
diff --git a/test/querying/test_query_arns.py b/test/querying/test_query_arns.py
index ca792975..4d69a96b 100644
--- a/test/querying/test_query_arns.py
+++ b/test/querying/test_query_arns.py
@@ -7,7 +7,7 @@ from policy_sentry.querying.arns import (
     get_arn_types_for_service,
     get_arn_type_details,
     get_resource_type_name_with_raw_arn,
-    get_matching_raw_arn
+    get_matching_raw_arns
 )
 
 
@@ -66,12 +66,14 @@ class QueryArnsTestCase(unittest.TestCase):
         self.assertTrue(get_resource_type_name_with_raw_arn(raw_arn), "environment")
 
     def test_get_matching_raw_arn(self):
-        """querying.arns.get_matching_raw_arn"""
-        self.assertEqual(get_matching_raw_arn("arn:aws:s3:::bucket_name"), "arn:${Partition}:s3:::${BucketName}")
-        self.assertEqual(get_matching_raw_arn("arn:aws:codecommit:us-east-1:123456789012:MyDemoRepo"), "arn:${Partition}:codecommit:${Region}:${Account}:${RepositoryName}")
-        self.assertEqual(get_matching_raw_arn("arn:aws:ssm:us-east-1:123456789012:parameter/test"), "arn:${Partition}:ssm:${Region}:${Account}:parameter/${FullyQualifiedParameterName}")
-        self.assertEqual(get_matching_raw_arn("arn:aws:batch:region:account-id:job-definition/job-name:revision"), "arn:${Partition}:batch:${Region}:${Account}:job-definition/${JobDefinitionName}:${Revision}")
-        self.assertEqual(get_matching_raw_arn("arn:aws:states:region:account-id:stateMachine:stateMachineName"), "arn:${Partition}:states:${Region}:${Account}:stateMachine:${StateMachineName}")
-        self.assertEqual(get_matching_raw_arn("arn:aws:states:region:account-id:execution:stateMachineName:executionName"), "arn:${Partition}:states:${Region}:${Account}:execution:${StateMachineName}:${ExecutionId}")
-        # self.assertEqual(get_matching_raw_arn("arn:aws:greengrass:region:account-id:/greengrass/definition/devices/1234567/versions/1"), "arn:aws:greengrass:${Region}:${Account}:/greengrass/definition/devices/${DeviceDefinitionId}/versions/${VersionId}")
-        self.assertEqual(get_matching_raw_arn("arn:${Partition}:rds:region:account-id:db:mydatabase"), "arn:${Partition}:rds:${Region}:${Account}:db:${DbInstanceName}")
+        """querying.arns.get_matching_raw_arns"""
+        self.assertEqual(get_matching_raw_arns("arn:aws:s3:::bucket_name"), ["arn:${Partition}:s3:::${BucketName}"])
+        self.assertEqual(get_matching_raw_arns("arn:aws:codecommit:us-east-1:123456789012:MyDemoRepo"), ["arn:${Partition}:codecommit:${Region}:${Account}:${RepositoryName}"])
+        self.assertEqual(get_matching_raw_arns("arn:aws:ssm:us-east-1:123456789012:parameter/test"), ["arn:${Partition}:ssm:${Region}:${Account}:parameter/${FullyQualifiedParameterName}"])
+        self.assertEqual(get_matching_raw_arns("arn:aws:batch:region:account-id:job-definition/job-name:revision"), ["arn:${Partition}:batch:${Region}:${Account}:job-definition/${JobDefinitionName}:${Revision}"])
+        self.assertEqual(get_matching_raw_arns("arn:aws:states:region:account-id:stateMachine:stateMachineName"), ["arn:${Partition}:states:${Region}:${Account}:stateMachine:${StateMachineName}"])
+        self.assertEqual(get_matching_raw_arns("arn:aws:states:region:account-id:execution:stateMachineName:executionName"), ["arn:${Partition}:states:${Region}:${Account}:execution:${StateMachineName}:${ExecutionId}"])
+        # self.assertEqual(get_matching_raw_arns("arn:aws:greengrass:region:account-id:/greengrass/definition/devices/1234567/versions/1"), ["arn:aws:greengrass:${Region}:${Account}:/greengrass/definition/devices/${DeviceDefinitionId}/versions/${VersionId}"])
+        self.assertEqual(get_matching_raw_arns("arn:${Partition}:rds:region:account-id:db:mydatabase"), ["arn:${Partition}:rds:${Region}:${Account}:db:${DbInstanceName}"])
+        self.assertIn("arn:${Partition}:rds:${Region}:${Account}:db:${DbInstanceName}", get_matching_raw_arns("arn:${Partition}:rds:region:account-id:*:*"))
+        self.assertEqual(get_matching_raw_arns("arn:${Partition}:rds:region:account-id:invalid-resource:*"), [])
diff --git a/test/util/test_arns.py b/test/util/test_arns.py
index e3c6d999..9839bdc9 100644
--- a/test/util/test_arns.py
+++ b/test/util/test_arns.py
@@ -99,3 +99,15 @@ class ArnsTestCase(unittest.TestCase):
         arn_in_database = "arn:${Partition}:rds:${Region}:${Account}:db:${DbInstanceName}"
         decision = does_arn_match(arn_to_test, arn_in_database)
         self.assertTrue(decision)
+
+    def test_does_arn_match_resource_wildcard(self):
+        arn_to_test = "arn:${Partition}:rds:${Region}:${Account}:*:*"
+        arn_in_database = "arn:${Partition}:rds:${Region}:${Account}:db:${DbInstanceName}"
+        decision = does_arn_match(arn_to_test, arn_in_database)
+        self.assertTrue(decision)
+
+        # Make sure wrong service yields False
+        arn_to_test = "arn:${Partition}:s3:${Region}:${Account}:*:*"
+        arn_in_database = "arn:${Partition}:rds:${Region}:${Account}:db:${DbInstanceName}"
+        decision = does_arn_match(arn_to_test, arn_in_database)
+        self.assertFalse(decision)