[Enhancement] Good mechanism for allowing check suppression

[rileydakota]

Is your feature request related to a problem? Please describe.
As a Security Engineer, I want to give Developers the ability to suppress certain rules on particular resources so I can be more prescriptive in the rules we use in our CI/CD pipelines

Describe the solution you'd like
Ideally - I'd love users to be able to specify an override in the Metadata of a particular CFN Resource to override particular rules.

        "NewVolume" : {
            "Type" : "AWS::EC2::Volume",
            "Properties" : {
                "Size" : 10,
                "Encrypted": false,
                "AvailabilityZone" : "us-west-2b"
            },
            "Metadata" : {
                "CfnGuard" : {
                    "IgnoreChecks" : [
                        "checkId1",
                        "checkId2",
                        "checkId3"
                    ]
                }
            }
        },

The ideal solution would allow us to to allow it on a rule-by-rule basis - there may be some rules we don't want to allow overriding at all. Allowing us to specify this in the rules themselves as opposed to just hardcoding this into CfnGuard (ala CfnLint) would allow a good bit of power on the end user.

Describe alternatives you've considered
I did attempt this by trying to build rules that would use the WHEN functionality to run a check as such:
AWS::EC2::Volume WHEN .Metadata.CfnGuard.IgnoreChecks.* != checkId1 CHECK Encrypted == %encryption_flag
The challenge with this approach is that if the user doesn't specify the Metadata.CfnGuard.IgnoreChecks key - the check is completely ignored altogether (Even with using --strict-checks). The above check would pass resources that don't have the Metadata.CfnGuard.IgnoreChecks key, and adding another check to cover that scenario like AWS::EC2::Volume Encrypted == %encryption_flag would defeat the purpose of the alternative above.

Additional context
If an implementation can be agreed upon - would be happy to try and get a PR in for this.

[PatMyron]

Agreed, resource-level ignores are valuable in cfn-lint/cfn_nag:

Resources:
  Resource:
    Type: AWS::SNS::Topic
    Metadata:
      cfn-lint:
        config:
          ignore_checks:
          - E3030
  Resource2:
    Type: AWS::SNS::Topic
    Metadata:
      cfn_nag:
        rules_to_suppress:
          - id: W9

Syntax should be easily searchable for those that want to disallow it completely or find all override instances

[johnttompkins]

This would be super powerful, but to get the check suppression for specific rules, we'd need some additional functionality to really make it work, like being able to identify/name rules -- stay tuned on this one.

[johnttompkins]

See my comment here on check suppression: #111 (comment). We are planning to introduce named rules in our guard 2.0 release. It should be possible to craft something to allow check suppression using the suggestion in the comment.

[rileydakota]

@jotompki for clarification - are you saying that it will possible to submit a contribution that allows check suppression in 2.0 (because of named rules)? Or that check suppression will be included in 2.0 along with named rules?

[johnttompkins]

You'd be able to write a rule that can skip if metadata is included. Something like:

rule public_read_write_rule {
    AWS::S3::Bucket {
         "public_read_write_rule" not in Metadata.cfn-guard.SuppressedRules[*]  {
              Properties.AccessControl != "public-read-write"
          }
    }
}

This isn't as elegant as some implicit check suppression that occurs after the fact, but we are trying to make these rulesets unambiguous so that they will evaluate the same for a given input. This gives ruleset authors the ability to specify which rules can be suppressed and which can't. I can see in the future maybe creating some syntactic sugar for this metadata check, but we aren't targeting it for the initial launch of this enhanced language.

[kddejong]

I think this has to be re-written a little given the recent release.
An example:

let s3_buckets = Resources.*[ Type == /S3::Bucket/ ]

# Skip the checks if there are no S3 buckets present
rule s3_bucket_name_encryption_check when %s3_buckets !empty {
    %s3_buckets {
        # ignore this rule if its Suppressed
        let metadata = Metadata."cfn-guard".SuppressedRules[
            some this == "s3_bucket_name_encryption_check"
        ]

        when %metadata empty {
            Properties {
                # encryption MUST BE on
                BucketEncryption.ServerSideEncryptionConfiguration[*] {
                    # only KMS 
                    ServerSideEncryptionByDefault.SSEAlgorithm IN 
                        ["aws:kms"] 
                }
            }
        }
    }
}

correlating test file

---
- input: {}
  expectations:
    rules:
      s3_bucket_name_encryption_check: SKIP
- input:
     Resources: {}
  expectations:
    rules:
      s3_bucket_name_encryption_check: SKIP
- input:
    Resources: 
      bucket:
        Type: AWS::S3::Bucket
  expectations:
    rules:
      s3_bucket_name_encryption_check: FAIL
- input:
    Resources: 
      bucket:
        Type: AWS::S3::Bucket
        Properties:
          BucketEncryption:
            ServerSideEncryptionConfiguration:
            - ServerSideEncryptionByDefault:
                SSEAlgorithm: aws:kms
  expectations:
    rules:
      s3_bucket_name_encryption_check: PASS
- input:
    Resources: 
      bucket:
        Type: AWS::S3::Bucket
        Properties:
          BucketEncryption:
            ServerSideEncryptionConfiguration:
            - ServerSideEncryptionByDefault:
                SSEAlgorithm: AES256
  expectations:
    rules:
      s3_bucket_name_encryption_check: FAIL
- input:
    Resources: 
      bucket:
        Type: AWS::S3::Bucket
        Metadata:
          cfn-guard:
            SuppressedRules:
            - s3_bucket_name_encryption_check
        Properties:
          BucketEncryption:
            ServerSideEncryptionConfiguration:
            - ServerSideEncryptionByDefault:
                SSEAlgorithm: AES256
  expectations:
    rules:
      s3_bucket_name_encryption_check: SKIP
- input:
    Resources: 
      bucket:
        Type: AWS::S3::Bucket
        Metadata:
          cfn-guard:
            SuppressedRules:
            - another_rule
        Properties:
          BucketEncryption:
            ServerSideEncryptionConfiguration:
            - ServerSideEncryptionByDefault:
                SSEAlgorithm: AES256
  expectations:
    rules:
      s3_bucket_name_encryption_check: FAIL

[priyap286]

Thanks @kddejong for the solution. I am closing this issue as I think the issue is resolved with cfn-guard 2.0. @rileydakota , please reopen this issue if you think you issue has not been triaged correctly.