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

[marcbransn]

First of all, thank you for providing such a great tool. We have been using it for a couple of weeks now and it has been a great help not only with creating policies but also with getting a better understanding of IAM as a whole.

Now, we've hit some cases where the tool produces output that differs from our expected output. It could either be due to a misunderstanding on our side or due to a bug in the tool. We looked at the documentation and past issues but couldn't find any mentions so far which probably indicates that we're doing something wrong on our side. In that case could you help us by suggesting what the correct YAML format should be? Thank you

The issue is as follows:

We would like to create a policy for a resource where one of the ARN's variables is a path, the way it can happen for S3 objects or ECR repositories.

For example, in ECR the ARN structure is arn:${Partition}:ecr:${Region}:${Account}:repository/${RepositoryName} but the ${RepositoryName} can actually be a path consisting of the namespace and the image name.

You can see from the output below that policy sentry works for the ARN arn:aws:ecr:*:*:repository/foo but for arn:aws:ecr:*:*:repository/foo/bar it just outputs an empty policy statement (at least for us).

$ policy_sentry --version 
policy_sentry, version 0.8.7

It works when the ARNs do not have paths in them:

$ cat test-nopaths.yaml
mode: crud
name: ''
# Specify resource ARNs
read:
- "arn:aws:s3:::foo/bar"
- "arn:aws:ecr:*:*:repository/foo"
write:
- ''
list:
- ''
tagging:
- ''
permissions-management:
- ''
# Actions that do not support resource constraints
wildcard-only:
  single-actions: # standalone actions
  - ''
  # Service-wide - like 's3' or 'ec2'
  service-read:
  - ''
  service-write:
  - ''
  service-list:
  - ''
  service-tagging:
  - ''
  service-permissions-management:
  - ''
# Skip resource constraint requirements by listing actions here.
skip-resource-constraints:
- ''
# Exclude actions from the output by specifying them here. Accepts wildcards, like kms:Delete*
exclude-actions:
- ''

$ policy_sentry write-policy --input-file test-nopaths.yaml
{
    "Version": "2012-10-17",
    "Statement": [
        {
            "Sid": "S3ReadObject",
            "Effect": "Allow",
            "Action": [
                "s3:GetObject",
                "s3:GetObjectAcl",
                "s3:GetObjectLegalHold",
                "s3:GetObjectRetention",
                "s3:GetObjectTagging",
                "s3:GetObjectTorrent",
                "s3:GetObjectVersion",
                "s3:GetObjectVersionAcl",
                "s3:GetObjectVersionForReplication",
                "s3:GetObjectVersionTagging",
                "s3:GetObjectVersionTorrent"
            ],
            "Resource": [
                "arn:aws:s3:::foo/bar"
            ]
        },
        {
            "Sid": "EcrReadRepository",
            "Effect": "Allow",
            "Action": [
                "ecr:BatchCheckLayerAvailability",
                "ecr:BatchGetImage",
                "ecr:DescribeImageScanFindings",
                "ecr:DescribeImages",
                "ecr:GetDownloadUrlForLayer",
                "ecr:GetLifecyclePolicy",
                "ecr:GetLifecyclePolicyPreview",
                "ecr:GetRepositoryPolicy",
                "ecr:ListTagsForResource"
            ],
            "Resource": [
                "arn:aws:ecr:*:*:repository/foo"
            ]
        }
    ]
}

It produces an empty policy statement if the ARNs have paths in them:

$ cat test-paths.yaml
mode: crud
name: ''
# Specify resource ARNs
read:
- "arn:aws:s3:::foo/bar/baz"
- "arn:aws:ecr:*:*:repository/foo/bar"
write:
- ''
list:
- ''
tagging:
- ''
permissions-management:
- ''
# Actions that do not support resource constraints
wildcard-only:
  single-actions: # standalone actions
  - ''
  # Service-wide - like 's3' or 'ec2'
  service-read:
  - ''
  service-write:
  - ''
  service-list:
  - ''
  service-tagging:
  - ''
  service-permissions-management:
  - ''
# Skip resource constraint requirements by listing actions here.
skip-resource-constraints:
- ''
# Exclude actions from the output by specifying them here. Accepts wildcards, like kms:Delete*
exclude-actions:
- ''

$ policy_sentry write-policy --input-file test-paths.yaml
{
    "Version": "2012-10-17",
    "Statement": []
}

[jeffzi]

I faced the same issue today with SSM parameters. Without paths, SSM parameter permissions can only be assigned to the first level in the hierarchy.

mode: crud
name: ''

read:
- 'arn:aws:ssm:::parameter/dev/foo/bar*'

results in:

{
    "Version": "2012-10-17",
    "Statement": []
}

[kmcquade]

Hey, thanks for opening! I'm glad you are finding the tool useful 😊

I agree that this definitely needs to be fixed. I figured out where this is failing:

policy_sentry/policy_sentry/util/arns.py

Lines 96 to 100 in 92b6b93

	# 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

This is under the ARN class and the same_resource_type method. That is the core function to doing all of this ARN matching craziness.

To replicate, I set up a unit test that used the does_arn_match function. The does_arn_match function requires that you pass in (1) an ARN to test, and (2) the RAW ARN format in the database (which can be queried with policy_sentry query arn-table --service serviceprefixname)

    def test_gh_237_ssm_arns_with_paths(self):
        arn_to_test = "arn:aws:ssm:::parameter/dev/foo/bar*"
        arn_in_database = "arn:${Partition}:ssm:${Region}:${Account}:parameter/${FullyQualifiedParameterName}"
        self.assertTrue(does_arn_match("arn:aws:ssm:::parameter/dev/foo/bar*"))
        # Should evaluate to True, but it evaluates to False :( 

And set the breakpoints at each of the return statements under the ARN.same_resource_type method. I saw that it failed at the lines indicated above.

I'll have to think of another way to determine if the ARN types match. AWS's confusing resource strings don't help us out here. We have to figure out a way to differentiate:

• table/${TableName} should not match * * table/${TableName}/backup/${BackupName}
• arn:aws:ssm:::parameter/dev should match arn:aws:ssm:::parameter/dev/foo/bar*

Any suggestions on how to test for these cases? Keep in mind that DynamoDB table vs. backup, SSM paths, and S3 paths are not the only cases here. Any input or suggestions would be greatly appreciated

[kmcquade]

Here's a good list of what should match vs not match, as we fix the does_arn_match function

{"arn:aws:dynamodb:us-east-1:123456789123:table/mytable/backup/mybackup", "arn:${Partition}:dynamodb:${Region}:${Account}:table/${TableName}/backup/${BackupName}", True}
{"arn:aws:dynamodb:us-east-1:123456789123:table/mytable", "arn:${Partition}:dynamodb:${Region}:${Account}:table/${TableName}", True}

{"arn:aws:dynamodb:us-east-1:123456789123:table/mytable/backup/mybackup", "arn:${Partition}:dynamodb:${Region}:${Account}:table/${TableName}", False}
{"arn:aws:dynamodb:us-east-1:123456789123:table/mytable", "arn:${Partition}:dynamodb:${Region}:${Account}:table/${TableName}/backup/${BackupName}", False}

{"arn:aws:ssm:::parameter/dev/foo/bar*", "arn:${Partition}:ssm:${Region}:${Account}:parameter/${FullyQualifiedParameterName}", True}
{"arn:aws:ssm:::parameter/dev", "arn:${Partition}:ssm:${Region}:${Account}:parameter/${FullyQualifiedParameterName}", True}

{"arn:aws:s3:::foo/bar/baz", "arn:${Partition}:s3:::${BucketName}/${ObjectName}", True}
{"arn:aws:s3:::foo/bar/baz", "arn:${Partition}:s3:::${BucketName}", False}

{"arn:aws:ecr:*:*:repository/foo/bar", "arn:${Partition}:ecr:${Region}:${Account}:repository/${RepositoryName}", True}
{"arn:aws:ecr:*:*:repository/foo", "arn:${Partition}:ecr:${Region}:${Account}:repository/${RepositoryName}", True}

[marcbransn]

Thank you for your response, I think I now understand why the bug occurs.

My initial (naive?) hypothesis would be that values containing slashes can only be used for the last variable in the ARN, somewhat similar to how function rest parameters work. This holds for all of the three examples we have found so far (S3, SSM and ECR) but I'm not sure if it's true for every service. But if it is we can replace the equals-check with a greater-than-check and match the "rest parameter" part of the ARN to the last variable.

If this hypothesis does not hold and variables in the middle of the ARN can contain slashes then we would run into a problem with ARNs of this format:

arn:aws:dynamodb:us-east-1:123456789123:table/mytable/backup/foo/backup/mybackup

Because now the mapping is not unique, right? (I did check that DynamoDB tables don't allow slashes in table names so this might not be a possible ARN for DynamoDB backups but it might be okay somewhere else (?)) In that case we would have to make a decision if we go with greedy or lazy matching of the variables.

One issue with assuming that the last variable in every ARN can contain slashes is that this would be considered a valid ARN (but we already said that DynamoDB table names cannot contain slashes):

arn:aws:dynamodb:us-east-1:123456789123:table/mytable/foo/bar

This might force us to make a distinction between the two cases with some new syntax in the ARN format:

arn:${Partition}:dynamodb:${Region}:${Account}:table/${TableName}

vs

arn:${Partition}:s3:::${BucketName}/${ObjectName**}

However, I think right now you're getting the ARN formats in a rather automated way and going through all of them to make this modification might be somewhat troublesome.

I think that's all I got for now and I'm just spitballing some ideas because I also don't know the implementation too well

[dgubitosi]

Nearly all the iam resources support paths as well, but the documentation usually calls this out in the ARN format with references like ${RoleNameWithPath} or ${UserNameWithPath}, etc.

[kmcquade]

Hey all,

I haven't had time to address this particular issue and our other internal contributors are preoccupied with some other tasks at the moment. Rather than radio silence, I wanted to ask if anyone from the OSS community is interested in taking this particular issue.

To aid you in your quest, I created some Unit Tests here kmcquade@0c15cc7. These provide the pass/fail criteria from my earlier comment #237 (comment). I did not merge those unit tests because they currently fail. But you can tinker with the code, knowing that once those tests pass, it should be good to go.

Let me know if you are interested. Thanks!

[samchecc]

Is there any update regarding to this issue? I'm facing same problem as well

[saikirankv]

we have started working on it, will provide fix by this week.