#335 Adding warning if a KMS key allows wildcarded principals in its policy

[pshelby]

#335

Depends On

stelligent/cfn-model PR #58
Once the above PR is merged in cfn-model and a new gem artifact created, this PR will be able to execute.

Description

Adding a failure to catch KMS key policy principals with wildcard values.

Testing

Rspec test suite includes both positive and negative test cases from JSON templates, which pass while covering all relevant logic in the rule. Added YAML templates for integration testing as well.

Example rspec Output

$ bundle exec rspec spec/custom_rules/KMSKeyWildcardPrincipalRule_spec.rb
Run options: exclude {:end_to_end=>true}
....

Finished in 0.01764 seconds (files took 0.34528 seconds to load)
4 examples, 0 failures

Coverage report generated for RSpec to cfn_nag/coverage. 33 / 66 LOC (50.0%) covered.
$

Example Integration Test Output

$  bundle exec cfn_nag --rule-directory lib spec/test_templates/yaml/kms/kms_key_without_wildcard_principal.yml
------------------------------------------------------------
spec/test_templates/yaml/kms/kms_key_without_wildcard_principal.yml
------------------------------------------------------------
Failures count: 0
Warnings count: 0
$

$ bundle exec cfn_nag --rule-directory lib spec/test_templates/yaml/kms/kms_key_with_aws_wildcard_principal.yml
------------------------------------------------------------
spec/test_templates/yaml/kms/kms_key_with_aws_wildcard_principal.yml
------------------------------------------------------------------------------------------------------------------------
| FAIL F76
|
| Resources: ["myKeyAwsWildcardPrincipal"]
| Line Numbers: [4]
|
| KMS key should not allow * principal (https://docs.aws.amazon.com/kms/latest/developerguide/key-policies.html)

Failures count: 1
Warnings count: 0
$

$ bundle exec cfn_nag --rule-directory lib spec/test_templates/yaml/kms/kms_key_with_wildcard_principal.yml
------------------------------------------------------------
spec/test_templates/yaml/kms/kms_key_with_wildcard_principal.yml
------------------------------------------------------------------------------------------------------------------------
| FAIL F76
|
| Resources: ["myKeyWildcardPrincipal"]
| Line Numbers: [4]
|
| KMS key should not allow * principal (https://docs.aws.amazon.com/kms/latest/developerguide/key-policies.html)

Failures count: 1
Warnings count: 0
$

$ bundle exec cfn_nag --rule-directory lib spec/test_templates/yaml/kms/kms_key_with_aws_array_wildcard_principal.yml
------------------------------------------------------------
spec/test_templates/yaml/kms/kms_key_with_aws_array_wildcard_principal.yml
------------------------------------------------------------------------------------------------------------------------
| FAIL F76
|
| Resources: ["myKeyAwsArrayWildcardPrincipal"]
| Line Numbers: [4]
|
| KMS key should not allow * principal (https://docs.aws.amazon.com/kms/latest/developerguide/key-policies.html)

Failures count: 1
Warnings count: 0
$

[ghost]

i believe {"AWS":"*"} is also legal and equivalent to "*"

[pshelby]

Good call! Latest commit adds logic for that contingency and changes this to a failure instead of warning.

[ghost]

i was just thinking about this.... we may need to enhanced the logic around SQS and SNS. i believe the following is legal syntax:

"AWS": [
"aws:arn:us-east-1:111111111:user/foo",
"*"
]

[pshelby]

Yeah, we may need to make use of the parsing in cfn-model for principals. I believe that parsing (and the wildcard_principal method) take the depth into account. Let's hold off merging this, and I'll start reworking this to use that model.
https://github.com/stelligent/cfn-model/blob/master/lib/cfn-model/model/principal.rb

[ghost]

need to bump the version of cfn-model for the policy object on kms key?

[pshelby]

@erickascic Updated cfn-model version and validated with rubobop.

$ bundle list
Gems included by the bundle:
  * ast (2.4.0)
  * bundler (2.0.2)
  * cfn-model (0.4.12)
  * cfn-nag (0.0.0)
  * diff-lcs (1.3)
  * docile (1.3.2)
  * jaro_winkler (1.5.4)
  * jmespath (1.3.1)
  * json (2.2.0)
  * kwalify (0.7.2)
  * little-plugger (1.1.4)
  * logging (2.2.2)
  * multi_json (1.14.1)
  * netaddr (2.0.4)
  * parallel (1.19.1)
  * parser (2.6.5.0)
  * psych (3.1.0)
  * rainbow (3.0.0)
  * rake (13.0.1)
  * rspec (3.9.0)
  * rspec-core (3.9.0)
  * rspec-expectations (3.9.0)
  * rspec-mocks (3.9.0)
  * rspec-support (3.9.0)
  * rubocop (0.77.0)
  * ruby-progressbar (1.10.1)
  * simplecov (0.17.1)
  * simplecov-html (0.10.2)
  * trollop (2.1.3)
  * unicode-display_width (1.6.0)
$

$ bundle exec rubocop
Inspecting 141 files
.............................................................................................................................................

141 files inspected, no offenses detected
$