Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 45 commits into from Nov 18, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
45 commits
Select commit Hold shift + click to select a range
0c15cc7
Non-working tests and adjustments to fix #237.
kmcquade Sep 29, 2020
8658db4
Update database (#241)
github-actions[bot] Oct 1, 2020
3b087a6
Speed improvements by changing IAM definition - fixes #240 (#242)
kmcquade Oct 1, 2020
1d3207f
update brew formula
actions-user Oct 1, 2020
bea972b
Fix homebrew publishing
kmcquade Oct 2, 2020
50a974c
renaming github action test.yml to ci.yml and name to continuous-inte…
reetasingh Oct 11, 2020
8ad8d6e
fix None type scenario in get_actions_for_service (#246)
reetasingh Oct 12, 2020
cfb6264
updating build badge to github actions CI badge in README.md (#249)
reetasingh Oct 13, 2020
7d26d96
adding unit tests for util/conditions.py is_condition_key_match metho…
reetasingh Oct 14, 2020
180ae03
Fix IAM definition update code (#254)
reetasingh Oct 16, 2020
6ab403a
Made updates to the scraping logic for the Actions, Resources, and Co…
kmcquade Oct 17, 2020
256242d
Improving minimization by grouping results based on arns (#252)
saikirankv Oct 17, 2020
909eb2c
Ensured minimization won't break if resource path contains special ch…
kmcquade Oct 17, 2020
ff4006f
Added click unit tests (#262)
kmcquade Oct 20, 2020
1704ab1
Add `--resource-type` flag to `policy_sentry query action-table` comm…
reetasingh Oct 20, 2020
4eb2312
fix cheat sheet for resource type (#265)
reetasingh Oct 21, 2020
b5ccd54
Updates to support calling policy_sentry from a terraform external da…
dgubitosi Oct 21, 2020
b1f931b
Provide fix for SID that contains dash (#267)
Alegrowin Oct 21, 2020
06e10b7
fix logging bug (#269)
reetasingh Oct 26, 2020
7cc7db3
Update CHANGELOG.md
kmcquade Oct 26, 2020
8e58303
Change minimize to a boolean flag with an option minimize-length argu…
dgubitosi Oct 27, 2020
88c5a7c
backward compatibility for --minimize
dgubitosi Oct 28, 2020
5cb5512
remove --no-minimize
dgubitosi Oct 28, 2020
56ce0bf
is_eager is not needed here
dgubitosi Oct 29, 2020
f1a95d8
use custom classes for proper backward compatibility for --minimize
dgubitosi Nov 1, 2020
4c396fd
ensure argument order remains consistent
dgubitosi Nov 1, 2020
3b46f5a
fix pylint errors
dgubitosi Nov 2, 2020
e2345a8
fix last pylint error
dgubitosi Nov 2, 2020
90e62d8
increment version to 0.10.0
dgubitosi Nov 3, 2020
d64f53c
update brew formula
actions-user Oct 27, 2020
9a598e1
adding release drafter github action (#260)
reetasingh Oct 27, 2020
83f49fe
Update database (#271)
github-actions[bot] Nov 1, 2020
e6f504b
update brew formula
actions-user Nov 4, 2020
7d08fa3
docs formatting (#273)
plaguna Nov 5, 2020
b7a98e8
removing CHANGELOG.md
reetasingh Nov 5, 2020
bc09697
Migrate terraform module (#277)
kmcquade Nov 10, 2020
a99674c
Terraform: Add support for skip_resource_constraints and exclude_acti…
kmcquade Nov 10, 2020
7435597
Adds support for Terraform 0.13
kmcquade Nov 10, 2020
1cab8ab
Minimize is set to true by default for the Terraform module
kmcquade Nov 10, 2020
b280b0d
Resolving conflicts
saikirankv Nov 18, 2020
a9d959d
Unexpected output when working with ARNs that have a path in them
saikirankv Nov 18, 2020
dc5c773
fixing failing testcases
saikirankv Nov 18, 2020
f984dd7
pylint changes
saikirankv Nov 18, 2020
3c0b398
version increment
saikirankv Nov 18, 2020
8e07aea
Review changes
saikirankv Nov 18, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion policy_sentry/bin/cli.py
Expand Up @@ -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

Expand Down
24 changes: 20 additions & 4 deletions policy_sentry/util/arns.py
Expand Up @@ -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):
"""
Expand Down Expand Up @@ -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)):
Expand All @@ -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]:
Expand Down
45 changes: 35 additions & 10 deletions test/files/test_gh_204_multiple_resource_types_wildcards.json
Expand Up @@ -117,6 +117,7 @@
"Effect": "Allow",
"Action": [
"rds:AddRoleToDBCluster",
"rds:ApplyPendingMaintenanceAction",
"rds:BacktrackDBCluster",
"rds:CreateDBCluster",
"rds:CreateDBClusterEndpoint",
Expand Down Expand Up @@ -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:*:*"
Expand All @@ -405,6 +407,8 @@
"Sid": "RdsListDb",
"Effect": "Allow",
"Action": [
"rds:DescribeDBInstanceAutomatedBackups",
"rds:DescribeDBInstances",
"rds:DescribeDBLogFiles",
"rds:DescribeDBProxyTargets",
"rds:DescribeDBSnapshots",
Expand All @@ -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",
Expand Down Expand Up @@ -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",
Expand Down
48 changes: 48 additions & 0 deletions test/util/test_arns.py
Expand Up @@ -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))
16 changes: 13 additions & 3 deletions test/writing/test_write_policy_library_usage.py
Expand Up @@ -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",
Expand Down Expand Up @@ -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"))