Improve s3 bucket policy linting

[chizou]

cfn-lint version: (cfn-lint --version) cfn-lint 0.16.0

Description of issue. Add better linting for s3 bucket policies

I have the following template:

  ReportBucketPolicy:
    Properties:
      Bucket: !Ref 'ReportBucket'
      PolicyDocument:
        Statement:
          - Action:
              - s3:ListBucket
            Condition:
              StringEquals:
                aws:PrincipalOrgID: !Ref 'OrgId'
            Effect: Allow
            Principal: '*'
            Resource:
              - !Sub '${ReportBucket.Arn}/*'
            Sid: AllowOrgAccess
        Version: '2012-10-17'
    Type: AWS::S3::BucketPolicy

I initially had a typo so that I forgot the $ in the Sub() so that it was like this: - !Sub '${ReportBucket.Arn}/*'

Can cfn-lint be made to catch that the resource specified was invalid?

The above template also has an issue because the resource shouldn't have a * for s3:ListBucket. This is the error that CloudFormation returns: Action does not apply to any resource(s) in statement

Is it within cfn-lint's scope to validate PolicyDocuments?

[atkinsonm]

Technically the s3:ListBucket is caused by your reference to * objects in the ReportBucket rather than a bucket. * by itself is valid, but not recommended:

Action: s3:ListBucket
Resource: '*'

I recommend cfn_nag for checking PolicyDocuments for best practices rather than just technical validity.

Fully agree that cfn-lint should catch this in the sense that it's an illegal policy definition. / is not a legal character in a bucket name, so any resource containing / should throw an error if the Actions block contains actions which only apply to buckets.

[kddejong]

I'm adding a rule to validate resource policy resource ARNs. Resource policies seem to have varying acceptable ARNs like a S3 bucket policy requires the full ARN but a SQS policy allows an asterisk. Creating a new rule is the easiest way to switch the pattern based on the scenario because we are working in nested json objects.

if you forget the sub you would get:

E3514 '${Bucket.Arn}/*' does not match '^arn:aws[A-Za-z\\-]*?:[^:]+:[^:]*(:(?:\\d{12}|\\*|aws)?:.+|)$'
local/issue/753.yaml:16:17

This will not cover the full scenario you have listed. In general this becomes can we create a fake or real resource ARN and substitute the value so we can validate it or do we create a very specific rule for s3 bucket policies that handles the Sub.

Additionally, I updated rule W3037 to limit the services and actions available in resource policies (example: A s3 bucket policy can only use s3 based actions).