Merge pull request #284 from saikirankv/arn_paths

Unexpected output when working with ARNs that have a path in them

Author: kmcquade

policy_sentry/bin/cli.py

@@ -2,7 +2,7 @@
 """
     Policy Sentry is a tool for generating least-privilege IAM Policies.
 """
-__version__ = "0.10.0"
+__version__ = "0.11.0"
 import click
 from policy_sentry import command
 

policy_sentry/util/arns.py

@@ -39,6 +39,9 @@ def __init__(self, provided_arn):
             self.resource, self.resource_path = self.resource.split(":", 1)
         self.resource_string = self._resource_string()
 
+    def __repr__(self):
+        return self.arn
+
     # pylint: disable=too-many-return-statements
     def _resource_string(self):
         """
@@ -96,8 +99,18 @@ def same_resource_type(self, arn_in_database):
         # 4b: If we have a confusing resource string, the length of the split resource string list
         #  should at least be the same
         # Again, table/${TableName} (len of 2) should not match `table/${TableName}/backup/${BackupName}` (len of 4)
-        if len(split_resource_string_to_test) != len(arn_format_list):
-            return False
+        # if len(split_resource_string_to_test) != len(arn_format_list):
+        #     return False
+
+        non_empty_arn_format_list = []
+        for i in arn_format_list:
+            if i != "":
+                non_empty_arn_format_list.append(i)
+
+        lower_resource_string = list(map(lambda x:x.lower(),split_resource_string_to_test))
+        for i in non_empty_arn_format_list:
+            if i.lower() not in lower_resource_string:
+                return False
 
         # 4c: See if the non-normalized fields match
         for i in range(len(arn_format_list)):
@@ -116,14 +129,17 @@ def same_resource_type(self, arn_in_database):
 
         # 4. Special type for S3 bucket objects and CodeCommit repos
         # Note: Each service can only have one of these, so these are definitely exceptions
-        exclusion_list = ["${ObjectName}", "${RepositoryName}", "${BucketName}"]
+        exclusion_list = ["${ObjectName}", "${RepositoryName}", "${BucketName}", "table/${TableName}", "${BucketName}/${ObjectName}"]
         resource_path_arn_in_database = elements[5]
         if resource_path_arn_in_database in exclusion_list:
             logger.debug("Special type: %s", resource_path_arn_in_database)
+            # handling special case table/${TableName}
+            if resource_string_arn_in_database in ["table/${TableName}", "${BucketName}"]:
+                return len(self.resource_string.split('/')) == len(elements[5].split('/'))
             # If we've made it this far, then it is a special type
             # return True
             # Presence of / would mean it's an object in both so it matches
-            if "/" in self.resource_string and "/" in elements[5]:
+            elif "/" in self.resource_string and "/" in elements[5]:
                 return True
             # / not being present in either means it's a bucket in both so it matches
             elif "/" not in self.resource_string and "/" not in elements[5]:

test/files/test_gh_204_multiple_resource_types_wildcards.json

@@ -117,6 +117,7 @@
             "Effect": "Allow",
             "Action": [
                 "rds:AddRoleToDBCluster",
+                "rds:ApplyPendingMaintenanceAction",
                 "rds:BacktrackDBCluster",
                 "rds:CreateDBCluster",
                 "rds:CreateDBClusterEndpoint",
@@ -361,41 +362,42 @@
             "Effect": "Allow",
             "Action": [
                 "rds:DescribeDBClusterBacktracks",
+                "rds:DescribeDBClusterEndpoints",
                 "rds:DescribeDBClusters",
-                "rds:DescribeDBProxyTargets"
+                "rds:DescribeDBProxyTargets",
+                "rds:DescribePendingMaintenanceActions"
             ],
             "Resource": [
                 "arn:aws:rds:us-east-1:123456789012:*:*"
             ]
         },
         {
-            "Sid": "RdsListClusterpg",
+            "Sid": "RdsListClusterendpoint",
             "Effect": "Allow",
             "Action": [
-                "rds:DescribeDBClusterParameterGroups",
-                "rds:DescribeDBClusterParameters"
+                "rds:DescribeDBClusterEndpoints"
             ],
             "Resource": [
                 "arn:aws:rds:us-east-1:123456789012:*:*"
             ]
         },
         {
-            "Sid": "RdsListClustersnapshot",
+            "Sid": "RdsListClusterpg",
             "Effect": "Allow",
             "Action": [
-                "rds:DescribeDBClusterSnapshotAttributes"
+                "rds:DescribeDBClusterParameterGroups",
+                "rds:DescribeDBClusterParameters"
             ],
             "Resource": [
                 "arn:aws:rds:us-east-1:123456789012:*:*"
             ]
         },
         {
-            "Sid": "RdsListPg",
+            "Sid": "RdsListClustersnapshot",
             "Effect": "Allow",
             "Action": [
-                "rds:DescribeDBEngineVersions",
-                "rds:DescribeDBParameterGroups",
-                "rds:DescribeDBParameters"
+                "rds:DescribeDBClusterSnapshotAttributes",
+                "rds:DescribeDBClusterSnapshots"
             ],
             "Resource": [
                 "arn:aws:rds:us-east-1:123456789012:*:*"
@@ -405,6 +407,8 @@
             "Sid": "RdsListDb",
             "Effect": "Allow",
             "Action": [
+                "rds:DescribeDBInstanceAutomatedBackups",
+                "rds:DescribeDBInstances",
                 "rds:DescribeDBLogFiles",
                 "rds:DescribeDBProxyTargets",
                 "rds:DescribeDBSnapshots",
@@ -415,6 +419,17 @@
                 "arn:aws:rds:us-east-1:123456789012:*:*"
             ]
         },
+        {
+            "Sid": "RdsListPg",
+            "Effect": "Allow",
+            "Action": [
+                "rds:DescribeDBParameterGroups",
+                "rds:DescribeDBParameters"
+            ],
+            "Resource": [
+                "arn:aws:rds:us-east-1:123456789012:*:*"
+            ]
+        },
         {
             "Sid": "RdsListProxy",
             "Effect": "Allow",
@@ -478,6 +493,16 @@
                 "arn:aws:rds:us-east-1:123456789012:*:*"
             ]
         },
+        {
+            "Sid": "RdsListGlobalcluster",
+            "Effect": "Allow",
+            "Action": [
+                "rds:DescribeGlobalClusters"
+            ],
+            "Resource": [
+                "arn:aws:rds:us-east-1:123456789012:*:*"
+            ]
+        },
         {
             "Sid": "RdsListOg",
             "Effect": "Allow",

test/util/test_arns.py

@@ -145,3 +145,51 @@ def test_dynamodb_arn_matching_gh_215(self):
         self.assertFalse(does_arn_match(this_arn, stream))
         self.assertFalse(does_arn_match(this_arn, backup))
         self.assertFalse(does_arn_match(this_arn, global_table))
+
+
+class ArnPathTestCase(unittest.TestCase):
+    # When paths are used
+    def test_ssm_paths(self):
+        parameter_1 = ARN("arn:aws:ssm:::parameter/dev/foo/bar*")
+        parameter_2 = "arn:aws:ssm:::parameter/dev"
+        print(parameter_1.same_resource_type(parameter_2))
+        self.assertTrue(parameter_1.same_resource_type(parameter_2))
+
+
+    # When confusing ARNs that look like paths but are not actually paths are used
+    def test_dynamo_db_non_paths(self):
+        backup_arn = "arn:aws:dynamodb:us-east-1:123456789123:table/mytable/backup/mybackup"
+        backup_raw_arn = "arn:${Partition}:dynamodb:${Region}:${Account}:table/${TableName}/backup/${BackupName}"
+
+        table_arn = "arn:aws:dynamodb:us-east-1:123456789123:table/mytable"
+        table_raw_arn = "arn:${Partition}:dynamodb:${Region}:${Account}:table/${TableName}"
+
+        parameter_arn_with_path = "arn:aws:ssm:::parameter/dev/foo/bar*"
+        parameter_arn_without_path = "arn:aws:ssm:::parameter/dev"
+        parameter_raw_arn = "arn:${Partition}:ssm:${Region}:${Account}:parameter/${FullyQualifiedParameterName}"
+
+        s3_object_with_path = "arn:aws:s3:::foo/bar/baz"
+        s3_object_without_path = "arn:aws:s3:::foo/bar"
+
+        s3_object_raw_arn = "arn:${Partition}:s3:::${BucketName}/${ObjectName}"
+        s3_bucket_raw_arn = "arn:${Partition}:s3:::${BucketName}"
+
+        ecr_raw_arn = "arn:${Partition}:ecr:${Region}:${Account}:repository/${RepositoryName}"
+        ecr_arn_with_path = "arn:aws:ecr:*:*:repository/foo/bar"
+        ecr_arn_without_path = "arn:aws:ecr:*:*:repository/foo"
+
+        self.assertTrue(does_arn_match(backup_arn, backup_raw_arn))
+        self.assertTrue(does_arn_match(table_arn, table_raw_arn))
+        self.assertFalse(does_arn_match(table_arn, backup_raw_arn))
+        self.assertFalse(does_arn_match(backup_arn, table_raw_arn))
+
+        self.assertTrue(does_arn_match(parameter_arn_with_path, parameter_raw_arn))
+        self.assertTrue(does_arn_match(parameter_arn_without_path, parameter_raw_arn))
+
+        self.assertTrue(does_arn_match(s3_object_without_path, s3_object_raw_arn))
+        self.assertTrue(does_arn_match(s3_object_with_path, s3_object_raw_arn))
+        self.assertFalse(does_arn_match(s3_object_with_path, s3_bucket_raw_arn))
+        self.assertFalse(does_arn_match(s3_object_without_path, s3_bucket_raw_arn))
+
+        self.assertTrue(does_arn_match(ecr_arn_with_path, ecr_raw_arn))
+        self.assertTrue(does_arn_match(ecr_arn_without_path, ecr_raw_arn))

test/writing/test_write_policy_library_usage.py

@@ -7,6 +7,7 @@
     get_crud_template_dict,
     get_actions_template_dict,
 )
+from policy_sentry.util.arns import ARN
 
 desired_crud_policy = {
     "Version": "2012-10-17",
@@ -348,6 +349,15 @@ def test_gh_204_multiple_resource_types_wildcards(self):
             expected_results = json.load(yaml_file)
         result = write_policy_with_template(crud_template)
         # print(json.dumps(result, indent=4))
-        print(len(result.get("Statement")))
-        self.assertTrue(len(result.get("Statement")) > 40)
-        # self.assertDictEqual(result, expected_results)
+        self.assertDictEqual(result, expected_results)
+
+    def test_gh_237_ssm_arns_with_paths(self):
+        """test_gh_237_ssm_arns_with_paths: Test GitHub issue #204 with resource ARN paths"""
+        crud_template = {
+            "mode": "crud",
+            'read': ["arn:aws:ssm:::parameter/dev/foo/bar*"]
+        }
+        # result = write_policy_with_template(crud_template)
+        # print(json.dumps(result, indent=4))
+        arn = ARN("arn:aws:ssm:::parameter/dev/foo/bar*")
+        self.assertTrue(arn.same_resource_type("arn:aws:ssm:::parameter/dev"))