awslabs / aws-cloudformation-iam-policy-validator

MIT No Attribution
133 stars 18 forks source link

ignore-findings not ignoring when Fn::If in trust policy #32

Closed scolebrook closed 9 months ago

scolebrook commented 9 months ago

I need to use an Fn::If function in a trust policy to handle a chicken/egg situation. I need to create a role, provide the arn to a vendor, they give the arn to a user that will assume the role, I update a stack parameter with the arn which puts it in the trust policy.

Since cfn-policy-validator doesn't yet support Fn::If, is there a way to make it skip that particular role in the template so it isn't evaluated at all, but still evaluate everything else as distinct from ignoring a finding on a resource that has been evaluated? This template has multiple roles, so ignore AWS::IAM::Role would solve the problem, but it's a blunt instrument.

mluttrell commented 9 months ago

You can use --ignore-finding and specify a specific resource name to ignore that resource for now. That should work for what you want to do here. Example:

cfn-policy-validator validate --template-path ./my-template.json --region us-east-1 --ignore-finding MyResource1
scolebrook commented 9 months ago

Unfortunately that doesn't work. I think it's because it's seen as invalid syntax. I initially tried using that with the resource.finding form using either of the two error codes that were in the output (INVALID_PRINCIPAL_KEY and INVALID_CONFIGURATION). When I couldn't get it ignored that way, I tried with just the resource name (the logical name in the template). For good measure I also tried with the actual role name since I wasn't sure if I was supposed to be using the logical resource name from the template. None of that worked. The only thing that worked for me was to ignore Roles entirely.

mluttrell commented 9 months ago

Does your role have an explicit RoleName? This needs to be documented more clearly, but the resource name here is the actual name of the resource, which falls back to the logical name of the CFN resource if none is specified.

The actual line of code is here: https://github.com/awslabs/aws-cloudformation-iam-policy-validator/blob/main/cfn_policy_validator/parsers/identity.py#L169

Let me know if that works. I'll add a task to make this clearer in the documentation regardless.

scolebrook commented 9 months ago

Yes. But the RoleName is constructed using Fn::Sub/Fn::FindInMap combination to meet naming convention standards. I used the resolved string which had appeared elsewhere in the output. But I tried both and neither of them worked in skipping this individual role, with or without the error code.

mluttrell commented 9 months ago

Are you able to share a redacted version of the template just the role and map?

scolebrook commented 9 months ago
Mappings:
  "123456789012":
    all:
      env: "sb"
    eu-central-1:
      abbrev: ec1
Resources:
  MyRole:
    Type: "AWS::IAM::Role"
    Properties:
      AssumeRolePolicyDocument:
        Version: "2012-10-17"
        Statement:
          - Effect: Allow
            Principal:
              !If
              - someCondition
              - AWS: !Ref ParameterWithARN
              - AWS: !Ref "AWS::AccountId"
            Action: "sts:AssumeRole"
      RoleName:
        Fn::Sub:
          - 'fancy-role-${reg}-${env}'
          - reg: !FindInMap [!Ref "AWS::AccountId", !Ref "AWS::Region", "abbrev"]
            env: !FindInMap [!Ref "AWS::AccountId", "all", "env"]

This includes all the relevant bits hopefully. Some condition will be true if ParameterWithArn has an ARN in it, false otherwise.

mluttrell commented 9 months ago

Thanks! Let me take a look and I'll get back to you.

mluttrell commented 9 months ago

I see what's happening. Because the tool doesn't understand Fn::If, it's treating it as raw input to the trust policy's validation/access preview call.

The access preview API rightly returns an error with this raw input. The tool then raises an exception because an error was returned and we never get to filter out the resources. The filtering happens after the validation calls since it may rely on the finding codes returned from the validation.

Let me get this fixed and I'll get a new version out. I think what should happen here is that the tool should return an ERROR finding here instead of throwing an exception.

scolebrook commented 9 months ago

Cool. If there's an error instead of an exception, will the ignore feature work so I can ignore just this one role until Fn::If is supported (which is not a small body of work I'm guessing)?

mluttrell commented 9 months ago

Yes, the ignore feature will work once this gets fixed. You're right that Fn:If is not a small amount of work because it requires support for a bunch of other conditional operators (as well as conditions themselves). We do plan to add it, though.

Thanks for reporting this!

mluttrell commented 9 months ago

I just pushed v0.0.26 which should report these as blocking error findings, rather than exceptions that stop execution. You should be able to ignore them now. Please give it a try and let me know if it doesn't work for you.

scolebrook commented 9 months ago

Thanks @mluttrell . Using --ignore-finding MyResource.INVALID_PRINCIPAL_KEY worked perfectly. Checked the rest of the policy and all the other roles in the template.