diff --git a/policy_sentry/bin/cli.py b/policy_sentry/bin/cli.py index fdf0ce8a5..2055ccc4b 100755 --- a/policy_sentry/bin/cli.py +++ b/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 diff --git a/policy_sentry/util/arns.py b/policy_sentry/util/arns.py index 730a0a4ee..c144c160c 100644 --- a/policy_sentry/util/arns.py +++ b/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]: diff --git a/test/files/test_gh_204_multiple_resource_types_wildcards.json b/test/files/test_gh_204_multiple_resource_types_wildcards.json index 93f83936f..a85b65138 100644 --- a/test/files/test_gh_204_multiple_resource_types_wildcards.json +++ b/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", diff --git a/test/util/test_arns.py b/test/util/test_arns.py index 123caba38..9fee8c19f 100644 --- a/test/util/test_arns.py +++ b/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)) diff --git a/test/writing/test_write_policy_library_usage.py b/test/writing/test_write_policy_library_usage.py index fe7a8e0b4..0323267ac 100644 --- a/test/writing/test_write_policy_library_usage.py +++ b/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"))