[BUG] OR is not working

Aaron-Garrett commented 8 months ago

Describe the bug A clear and concise description of what the bug is. I am searching for Managed Policies that have !Sub ......AWS, but it errors out on any lines starting with !Ref, even if I include an OR statement.

To Reproduce Please supply:

Example rules and template that results in the error


let aws_iam_entities_no_managed_policy = Resources.*[
Type in [ /AWS::IAM::User/,
        /AWS::IAM::Role/,
        /AWS::IAM::Group/ ]
Metadata.guard.SuppressedRules not exists or
Metadata.guard.SuppressedRules.* != "IAM_ONLY_ALLOW_ManagedPolicies"
]

rule IAM_ONLY_ALLOW_ManagedPolicies when %aws_iam_entities_no_managed_policy !empty { when %aws_iam_entities_no_managed_policy.Properties.ManagedPolicyArns[]."Fn::Sub" exists { %aws_iam_entities_no_managed_policy.Properties.ManagedPolicyArns[]."Fn::Sub" not in [/^arn:aws:iam::\${AWS::AccountId}:policy\/AWS/,"arn:aws:iam::${AWS::AccountId}:policy/VMImportExportRoleForAWSConnector"] << Violation: Only select managed policies are allowed on IAM Users, Roles, or Groups. Fix: Remove the disallowed policies from any IAM Users, Roles, or Groups.

} }

And this, and other combinations of both of these

R7.0 Automatically inspect the code to ensure that the use of AWS managed policies is not being used and present an error if the code is using AWS managed policies.  For example: SecretsManagerReadWrite; ReadOnlyAccess; AWSLambdaRole; AWSLambdaVPCAccessExecutionRole; AmazonAthenaFullAccess; AmazonRDSReadOnlyAcces

let aws_iam_entities_no_managed_policy = Resources.*[
  Type in [ /AWS::IAM::User/,
            /AWS::IAM::Role/,
            /AWS::IAM::Group/ ]
  Metadata.guard.SuppressedRules not exists or
  Metadata.guard.SuppressedRules.* != "IAM_ONLY_ALLOW_ManagedPolicies"
]

rule IAM_ONLY_ALLOW_ManagedPolicies when %aws_iam_entities_no_managed_policy !empty {
  when %aws_iam_entities_no_managed_policy.Properties.ManagedPolicyArns[*] !empty {
    %aws_iam_entities_no_managed_policy.Properties.ManagedPolicyArns[*]."Fn::Sub" not in [/^arn:aws:iam::\${AWS::AccountId}:policy\/AWS/,"arn:aws:iam::${AWS::AccountId}:policy/VMImportExportRoleForAWSConnector"] OR %aws_iam_entities_no_managed_policy.Properties.ManagedPolicyArns[*]."Fn::Ref" exists
    <<
      Violation: Only select managed policies are allowed on IAM Users, Roles, or Groups.
      Fix: Remove the disallowed policies from any IAM Users, Roles, or Groups.
    >>
  }
}

The commands you used to invoke the tool OUTPUT=`RUST_BACKTRACE=1 ./cfn-guard-v3-ubuntu-latest/cfn-guard validate --show-summary pass,fail --data "$TEMPLATE_FILE" --rules .github/edb-pr-automation/lib/cfn-guard/rules/ 2>&1
The output of a -v log level if it's not related to cfn-guard-lambda, or the relevant CloudWatch log messages if it is related to the cfn-guard-lambda

ALL {
      ANY {
        Check =  %aws_iam_entities_no_managed_policy[*].Properties.ManagedPolicyArns[*].Fn::Sub not IN  ["/^arn:aws:iam::\\${AWS::AccountId}:policy/AWS/","arn:aws:iam::${AWS::AccountId}:policy/VMImportExportRoleForAWSConnector"] {
          RequiredPropertyError {
            PropertyPath = /Resources/rEDBIbudataloaderappFederatedRoleDev/Properties/ManagedPolicyArns/2[L:26,C:10]
            MissingProperty = Fn::Sub
            Reason = Could not find key Fn::Sub inside struct at path /Resources/rEDBIbudataloaderappFederatedRoleDev/Properties/ManagedPolicyArns/2[L:26,C:10]
            Code:
                 24.      ManagedPolicyArns:
                 25.        - !Sub "arn:aws:iam::${AWS::AccountId}:policy/AWS-edb-kms-s3-rw"
                 26.        - !Sub "arn:aws:iam::${AWS::AccountId}:policy/kms-secrets-rw"
                 27.        - !Ref rIbudataloaderappFederatedPermissions
                 28.        - !Ref rIbudataloaderappGlue
                 29.        - !Ref rIbudataloaderappLambda
          }
        }
        Check =  %aws_iam_entities_no_managed_policy[*].Properties.ManagedPolicyArns[*].Fn::Sub not IN  ["/^arn:aws:iam::\\${AWS::AccountId}:policy/AWS/","arn:aws:iam::${AWS::AccountId}:policy/VMImportExportRoleForAWSConnector"] {
          RequiredPropertyError {
            PropertyPath = /Resources/rEDBIbudataloaderappFederatedRoleDev/Properties/ManagedPolicyArns/3[L:27,C:10]
            MissingProperty = Fn::Sub
            Reason = Could not find key Fn::Sub inside struct at path /Resources/rEDBIbudataloaderappFederatedRoleDev/Properties/ManagedPolicyArns/3[L:27,C:10]
            Code:
                 25.        - !Sub "arn:aws:iam::${AWS::AccountId}:policy/AWS-edb-kms-s3-rw"
                 26.        - !Sub "arn:aws:iam::${AWS::AccountId}:policy/kms-secrets-rw"
                 27.        - !Ref rIbudataloaderappFederatedPermissions
                 28.        - !Ref rIbudataloaderappGlue
                 29.        - !Ref rIbudataloaderappLambda
          }
        }
        Check =  %aws_iam_entities_no_managed_policy[*].Properties.ManagedPolicyArns[*].Fn::Sub not IN  ["/^arn:aws:iam::\\${AWS::AccountId}:policy/AWS/","arn:aws:iam::${AWS::AccountId}:policy/VMImportExportRoleForAWSConnector"] {
          RequiredPropertyError {
            PropertyPath = /Resources/rEDBIbudataloaderappFederatedRoleDev/Properties/ManagedPolicyArns/4[L:28,C:10]
            MissingProperty = Fn::Sub
            Reason = Could not find key Fn::Sub inside struct at path /Resources/rEDBIbudataloaderappFederatedRoleDev/Properties/ManagedPolicyArns/4[L:28,C:10]
            Code:
                 26.        - !Sub "arn:aws:iam::${AWS::AccountId}:policy/kms-secrets-rw"
                 27.        - !Ref rIbudataloaderappFederatedPermissions
                 28.        - !Ref rIbudataloaderappGlue
                 29.        - !Ref rIbudataloaderappLambda
          }
        }
        Check =  %aws_iam_entities_no_managed_policy[*].Properties.ManagedPolicyArns[*].Fn::Sub not IN  ["/^arn:aws:iam::\\${AWS::AccountId}:policy/AWS/","arn:aws:iam::${AWS::AccountId}:policy/VMImportExportRoleForAWSConnector"] {
          ComparisonError {
            Error            = Check was not compliant as property [/Resources/rEDBIbudataloaderappFederatedRoleDev/Properties/ManagedPolicyArns/0/Fn::Sub[L:24,C:10]] was not present in [(resolved, Path=[L:0,C:0] Value=["/^arn:aws:iam::\\${AWS::AccountId}:policy/AWS/","arn:aws:iam::${AWS::AccountId}:policy/VMImportExportRoleForAWSConnector"])]
          }
            PropertyPath    = /Resources/rEDBIbudataloaderappFederatedRoleDev/Properties/ManagedPolicyArns/0/Fn::Sub[L:24,C:10]
            Operator        = NOT IN
            Value           = "arn:aws:iam::${AWS::AccountId}:policy/AWS-edb-kms-s3-rw"
            ComparedWith    = [["/^arn:aws:iam::\\${AWS::AccountId}:policy/AWS/","arn:aws:iam::${AWS::AccountId}:policy/VMImportExportRoleForAWSConnector"]]
            Code:
                 22.                "SAML:aud": "https://signin.aws.amazon.com/saml"
                 23.
                 24.      ManagedPolicyArns:
                 25.        - !Sub "arn:aws:iam::${AWS::AccountId}:policy/AWS-edb-kms-s3-rw"
                 26.        - !Sub "arn:aws:iam::${AWS::AccountId}:policy/kms-secrets-rw"
                 27.        - !Ref rIbudataloaderappFederatedPermissions

        }
        Check =  %aws_iam_entities_no_managed_policy[*].Properties.ManagedPolicyArns[*].Fn::Ref EXISTS   {
          Message {
            Violation: Only select managed policies are allowed on IAM Users, Roles, or Groups.
            Fix: Remove the disallowed policies from any IAM Users, Roles, or Groups.
          }
          RequiredPropertyError {
            PropertyPath = /Resources/rEDBIbudataloaderappFederatedRoleDev/Properties/ManagedPolicyArns/0[L:24,C:10]
            MissingProperty = Fn::Ref
            Reason = Could not find key Fn::Ref inside struct at path /Resources/rEDBIbudataloaderappFederatedRoleDev/Properties/ManagedPolicyArns/0[L:24,C:10]
            Code:
                 22.                "SAML:aud": "https://signin.aws.amazon.com/saml"
                 23.
                 24.      ManagedPolicyArns:
                 25.        - !Sub "arn:aws:iam::${AWS::AccountId}:policy/AWS-edb-kms-s3-rw"
                 26.        - !Sub "arn:aws:iam::${AWS::AccountId}:policy/kms-secrets-rw"
                 27.        - !Ref rIbudataloaderappFederatedPermissions
          }
        }
        Check =  %aws_iam_entities_no_managed_policy[*].Properties.ManagedPolicyArns[*].Fn::Ref EXISTS   {
          Message {
            Violation: Only select managed policies are allowed on IAM Users, Roles, or Groups.
            Fix: Remove the disallowed policies from any IAM Users, Roles, or Groups.
          }
          RequiredPropertyError {
            PropertyPath = /Resources/rEDBIbudataloaderappFederatedRoleDev/Properties/ManagedPolicyArns/1[L:25,C:10]
            MissingProperty = Fn::Ref
            Reason = Could not find key Fn::Ref inside struct at path /Resources/rEDBIbudataloaderappFederatedRoleDev/Properties/ManagedPolicyArns/1[L:25,C:10]
            Code:
                 23.
                 24.      ManagedPolicyArns:
                 25.        - !Sub "arn:aws:iam::${AWS::AccountId}:policy/AWS-edb-kms-s3-rw"
                 26.        - !Sub "arn:aws:iam::${AWS::AccountId}:policy/kms-secrets-rw"
                 27.        - !Ref rIbudataloaderappFederatedPermissions
                 28.        - !Ref rIbudataloaderappGlue
          }
        }

NOTE: Please be sure that the templates, rules and logs you provide as part of your bug report do not contain any sensitive information.

Expected behavior A clear and concise description of what you expected to happen.

I expected this to flag any Managed Policies that follow the pattern !Sub arn:aws:iam::{AWS::AccountId}:policy/AWS* as a violation and leave any lines that start with !Ref alone.

Operating System: [eg, MacOS, Windows, Ubuntu, etc] Ubuntu

OS Version [eg Catalina, 10, 18.04, etc] Latest on GitHub Actions Additional context Add any other context about the problem here.

dannyvassallo commented 8 months ago

Hey @Aaron-Garrett!

Thanks for the report.

Would you mind sharing with us the version of cfn-guard you are using as well as a sample template to reproduce the issue?

Aaron-Garrett commented 8 months ago

CFN-Guard version 3.0.2

AWSTemplateFormatVersion:` '2010-09-09'
Description: 'CloudFormation exports'

Resources:
  rEDBIbudataloaderappFederatedRoleDev:
    Type: AWS::IAM::Role
    Properties:
      RoleName: !Sub "aws_edb_data_loader_utility_${pDeployEnvironment}"
      Description: "EDB Service Role for Data Loader Utility Files Onboarding to EDB from Sharepoint"
      PermissionsBoundary: !Sub "arn:aws:iam::${AWS::AccountId}:policy/LZ-IAM-Boundary"
      MaxSessionDuration: 36000
      AssumeRolePolicyDocument:
        Version: "2012-10-17"
        Statement:
          - Effect: Allow          
            Principal:
              Federated:
                - !Sub "arn:aws:iam::${AWS::AccountId}:saml-provider/LillyMgmt"
            Action: sts:AssumeRoleWithSAML
            Condition:
              StringEquals:
                "SAML:aud": "https://signin.aws.amazon.com/saml"

      ManagedPolicyArns:
        - !Sub "arn:aws:iam::${AWS::AccountId}:policy/AWS-kms-s3-rw"
        # Above should get flagged
        - !Sub "arn:aws:iam::${AWS::AccountId}:policy/kms-secrets-rw"
        # Refs should be ignored
        - !Ref rIbudataloaderappFederatedPermissions
        - !Ref rIbudataloaderappGlue
        - !Ref rIbudataloaderappLambda

I recognize that I could have just done this wrong, and if so, please let me know what the correct method is.

joshfried-aws commented 8 months ago

Hi @Aaron-Garrett, after going over this issue with @dannyvassallo we were unable to produce an actual error. The logs you posted above point to a validation failure, not an error. Just want to be clear the 2 are not the same.

That said, I believe I noticed a few mistakes in the rules you have provided. I have updated your rule with the following, i made some small tweaks to implement best practices and make it easier to follow.

let aws_iam_entities_no_managed_policy = Resources.*[
  Type in [ /AWS::IAM::User/,
            /AWS::IAM::Role/,
            /AWS::IAM::Group/ ]
  Metadata.guard.SuppressedRules not exists or
  Metadata.guard.SuppressedRules.* != "IAM_ONLY_ALLOW_ManagedPolicies"
]

let INVALID_VALUES = [/^arn:aws:iam::\${AWS::AccountId}:policy\/AWS/, "arn:aws:iam::${AWS::AccountId}:policy/VMImportExportRoleForAWSConnector"]

rule IAM_ONLY_ALLOW_ManagedPolicies when %aws_iam_entities_no_managed_policy !empty {
  when %aws_iam_entities_no_managed_policy.Properties.ManagedPolicyArns[*] !empty {
    let sub_queries = %aws_iam_entities_no_managed_policy.Properties.ManagedPolicyArns.*[ keys == "Fn::Sub" ]

    when %sub_queries EXISTS {
        %sub_queries not in %INVALID_VALUES
    }
  }
}

This results in the following output from guard, which fails as is expected

test-files/490.yaml Status = FAIL
FAILED rules
490.guard/IAM_ONLY_ALLOW_ManagedPolicies    FAIL
---
Evaluating data test-files/490.yaml against rules 490.guard
Number of non-compliant resources 1
Resource = rEDBIbudataloaderappFederatedRoleDev {
  Type      = AWS::IAM::Role
  Rule = IAM_ONLY_ALLOW_ManagedPolicies {
    ALL {
      Check =  %sub_queries not IN  %INVALID_VALUES {
        ComparisonError {
          Error            = Check was not compliant as property [/Resources/rEDBIbudataloaderappFederatedRoleDev/Properties/ManagedPolicyArns/0/Fn::Sub[L:24,C:10]] was not present in [(resolved, Path=[L:0,C:0] Value=["/^arn:aws:iam::\${AWS::AccountId}:policy/AWS/","arn:aws:iam::${AWS::AccountId}:policy/VMImportExportRoleForAWSConnector"])]
        }
          PropertyPath    = /Resources/rEDBIbudataloaderappFederatedRoleDev/Properties/ManagedPolicyArns/0/Fn::Sub[L:24,C:10]
          Operator        = NOT IN
          Value           = "arn:aws:iam::${AWS::AccountId}:policy/AWS-kms-s3-rw"
          ComparedWith    = [["/^arn:aws:iam::\${AWS::AccountId}:policy/AWS/","arn:aws:iam::${AWS::AccountId}:policy/VMImportExportRoleForAWSConnector"]]
          Code:
               22.                "SAML:aud": "https://signin.aws.amazon.com/saml"
               23.
               24.      ManagedPolicyArns:
               25.        - !Sub "arn:aws:iam::${AWS::AccountId}:policy/AWS-kms-s3-rw"
               26.        # Above should get flagged
               27.        - !Sub "arn:aws:iam::${AWS::AccountId}:policy/kms-secrets-rw"

      }
    }
  }
}

NOTE: this rule will succeed (as expected) if you comment out line 25 in your sample yaml file.

Some things to take away here is that to query for a list that contains structs we want to follow a similar pattern to the first query in this rule. This means we want to do a wildcard filter, while specifying the field's value we are looking to compare, in this case its the keyword keys which evaluates all the keys of a key, value pair. Once we change the query to do just that we can simply write the check that you had first written, in this case we're comparing the values each key resolves to against a given list. One last thing, for future reference note that all intrinsic functions get expanded to their json equivalent. Meaning the !Ref intrinsic function doesn't get expanded to Fn::Ref the same way a lot of other intrinsic functions do, it is simply just Ref CloudFormation docs.

Please let me know if this unblocks you, so we can close out this issue. I am going to go ahead and remove the bug label, and change it to guidance. Feel free to change it back if you feel my above assumptions are incorrect.

Thanks

Aaron-Garrett commented 8 months ago

That appears to have done it!

Just wanting to make sure I am understanding you correctly, the keys filters out anything in the array that does not start with what you set the keys value equal to? I scoured the github repo looking for something like that and must have overlooked it. I knew me overlooking something was a possibility and appreciate your timely response. My apologies for putting it under Bug, I did not know where else to put it when opening and Issue from the Issue list as I did not see a help wanted option.

joshfried-aws commented 8 months ago

Hi @Aaron-Garrett sorry i realize i may not have been as clear as i thought before. In this case your ManagedPolicyArns object is essentially a list of key, value pairs. In guard to filter on key, value pairs we can filter keys using the keys keyword. Does that make sense?

Also no stress about the labels, totally understandable!

Aaron-Garrett commented 8 months ago

It does, I did not realize it was key value pairs. Is that the same for anything in an array? For future reference, is there a values keyword? Thank you so much for helping me to figure it out! I love this took and it has been a boon for my team! Thank you guys so much for putting this wonderful thing together!

joshfried-aws commented 8 months ago

I think this is an opportunity for us to improve on our docs. I wonder if the reason why this wasn't easier to spot for you is because cfn-guard expands intrinsic functions to their json syntax which results in k,v pairs.

As for the values keyword I do not believe this is possible currently.

Aaron-Garrett commented 8 months ago

Wonderful, thank you for all of your help.

aws-cloudformation / cloudformation-guard

[BUG] OR is not working #490