Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Faulty json output on cfn_nag_scan #608

Open
isuftin opened this issue Sep 2, 2022 · 2 comments · May be fixed by #615
Open

Faulty json output on cfn_nag_scan #608

isuftin opened this issue Sep 2, 2022 · 2 comments · May be fixed by #615

Comments

@isuftin
Copy link

isuftin commented Sep 2, 2022

Running cfn-nag 0.8.10, scanning a nested CloudFormation template, outputting json and am seeing:

Experimental SPCM rule is failing. Please report undefined method `gsub' for {"Ref"=>"RDSKMSKeyAlias"}:Hash

      value = value.gsub("${#{special_character}}", '')
                   ^^^^^ with the violating template

[ ... rest of json output ... ]

While the CloudFormation template(s) may be erroneous, we would not expect plaintext errors to make their way into JSON output as this output gets scanned later for test reporting in GitLab.

@isuftin
Copy link
Author

isuftin commented Sep 2, 2022

The problem was found in the template and corrected which fixed the error in the output. Original template:

Wrong:

---
AWSTemplateFormatVersion: "2010-09-09"
Description: My IAM role
Resources:
  Role:
    Type: AWS::IAM::Role
    Properties:
      Description: My Description
      PermissionsBoundary: !Sub arn:${AWS::Partition}:iam::${AWS::AccountId}:policy/my-policy-boundary
      ManagedPolicyArns:
        - !Sub arn:${AWS::Partition}:iam::aws:policy/ReadOnlyAccess
        - !Sub arn:${AWS::Partition}:iam::aws:policy/AmazonSSMManagedInstanceCore
      AssumeRolePolicyDocument:
        Version: "2012-10-17"
        Statement:
          - Effect: Allow
            Principal:
              Service:
                - ec2.amazonaws.com
            Action:
              - sts:AssumeRole
      Policies:
        - PolicyName: AliasBasedKMSAccess
          PolicyDocument:
            Version: '2012-10-17'
            Statement:
              - Effect: Allow
                Action:
                  - kms:List*
                  - kms:Describe*
                  - kms:Decrypt
                  - kms:Encrypt
                Resource: !Sub arn:${AWS::Partition}:kms:*:${AWS::AccountId}:key/*
                Condition:
                  ForAnyValue:StringEquals:
                    kms:ResourceAliases:
                      - !Ref RDSKMSKeyAlias

Right:

---
AWSTemplateFormatVersion: "2010-09-09"
Description: My IAM role
Resources:
  Role:
    Type: AWS::IAM::Role
    Properties:
      Description: My Description
      PermissionsBoundary: !Sub arn:${AWS::Partition}:iam::${AWS::AccountId}:policy/my-policy-boundary
      ManagedPolicyArns:
        - !Sub arn:${AWS::Partition}:iam::aws:policy/ReadOnlyAccess
        - !Sub arn:${AWS::Partition}:iam::aws:policy/AmazonSSMManagedInstanceCore
      AssumeRolePolicyDocument:
        Version: "2012-10-17"
        Statement:
          - Effect: Allow
            Principal:
              Service:
                - ec2.amazonaws.com
            Action:
              - sts:AssumeRole
      Policies:
        - PolicyName: AliasBasedKMSAccess
          PolicyDocument:
            Version: '2012-10-17'
            Statement:
              - Effect: Allow
                Action:
                  - kms:List*
                  - kms:Describe*
                  - kms:Decrypt
                  - kms:Encrypt
                Resource: !Sub arn:${AWS::Partition}:kms:*:${AWS::AccountId}:key/*
                Condition:
                  ForAnyValue:StringEquals:
                    kms:ResourceAliases: !Ref RDSKMSKeyAlias

Specifically, cfn-nag was puking on this having a list instead of a string:

Condition:
  ForAnyValue:StringEquals:
    kms:ResourceAliases: ...

@connelldave connelldave linked a pull request Dec 19, 2022 that will close this issue
@connelldave
Copy link

Bumping as this caught me out today expecting valid JSON on stdout with -o.

I've raised a PR with the simplest fix I could see :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants