[Enhancement] Validate Principal in S3 Bucket Policy is "*" or Map with Key as AWS

[shahamy]

Is this feature request related to a new rule or cfn-lint capabilities?

rules

Describe the feature you'd like to request

I would like to add a rule for validating the Principal value in Bucket Policy for S3 buckets

I had a repo where we have

- Sid:              AllowRole
  Effect:           Allow
  Principal:        arn:aws:iam::<ACCOUNT_ID>:role/<ROLE_NAME>
  Action:           s3:PutObject
  Resource:
    -  !Sub  arn:aws:s3:::${S3BucketName}/*

This failed after a good 20 minutes trying to create the S3 Bucket Policy with Invalid policy syntax which was not a very descriptive issue

Describe the solution you'd like

The rule we could add could validate that
Principal == "*"
OR
Principal is type map and has key AWS

The correct template what worked is:

- Sid:              AllowRole
  Effect:           Allow
  Principal:        
        AWS : arn:aws:iam::<ACCOUNT_ID>:role/<ROLE_NAME>
  Action:           s3:PutObject
  Resource:
    -  !Sub  arn:aws:s3:::${S3BucketName}/*

Additional context

I believe Principal can only be those 2 options -- please confirm!

Note: I am not sure where the schema nor rules for S3 Bucket Policy are implemented but I'd be happy to try to contribute

Is this something that you'd be interested in working on?

• 👋 I may be able to implement this feature request

Would this feature include a breaking change?

• ⚠️ This feature might incur a breaking change

[kddejong]

In v1 of cfn-lint this will be reported as

E3512 '*' was expected
template.yaml:12:13

this happens because you have it as a string but the only valid string is *

[kddejong]

To add more context in v1 we have added more schema validations for IAM policies of all varieties.

We did our best to build the logic based on docs which describes it like you have mentioned it has to be an * or a map with a key of ["AWS" | "Federated" | "Service" | "CanonicalUser"]

[shahamy]

Hm, I don't believe the error was reported with v1.
I have my CFT look like
Principal: arn:aws:iam::<ACCOUNT_ID>:role/<ROLE_NAME>

We are running v1.8.2

[kddejong]

@shahamy any chance you are using a Sub for the Principal instead of a string as provided above? Using a sub removes the error message for me. We should provide one here but since we don't know the value for all the substitution parameters we don't validate it.

- Sid:              AllowRole
  Effect:           Allow
  Principal:       !Sub "arn:${AWS::Principal}:iam::${AWS::AccountId}:role/${RoleName}"
  Action:           s3:PutObject
  Resource:
    -  !Sub  arn:aws:s3:::${S3BucketName}/*

[shahamy]

Yes you're right. I am using a FindInMap -- it really is
Principal: !FindInMap [!Ref EnvironmentGroup, !Ref 'AWS::Region', RoleArn]

To confirm, cfn-lint doesn't evaluate the string and just looks at the CFT at face value so can't tell it's type?

[kddejong]

Yea, its tricky for us. We are working to improve this but at this point you are correct. If we don't know the parameter values we can't evaluate the function but we try. For instance if it was !FindInMap ["MyMap", !Ref 'AWS::Region', RoleArn] we would resolve the FindInMap and show you an error. (default region is us-east-1 but can be changed by parameter)

We could try to go farther with FindInMap to do the resolution. For instance if you only have one mapping that has regions as the first key then we could probably assume what !Ref EnvironmentGroup is or could be. We could look at the allowed values or default value for !Ref EnvironmentGroup and validate all the possibilities.

These changes wouldn't solve all the possibilities but may allow us to catch this type of issue.

[shahamy]

I'm good if you want to close this as a won't fix