From cd814e7912a23510f88b4f8a936fc0f78d1b775c Mon Sep 17 00:00:00 2001 From: Kevin Formsma Date: Mon, 3 Jan 2022 15:55:26 -0500 Subject: [PATCH] Update PassRole rules to not trigger on deny statements. Fixes #584 --- .../IamRolePassRoleWildcardResourceRule.rb | 2 +- .../custom_rules/passrole_base_rule.rb | 2 +- spec/custom_rules/SPCMRule_spec.rb | 2 +- ...policy_passrole_resource_wildcard_pass.yml | 30 +++++++++++++++++-- ...policy_passrole_resource_wildcard_pass.yml | 19 +++++++++--- ...m_role_passrole_resource_wildcard_pass.yml | 28 +++++++++++++++-- 6 files changed, 71 insertions(+), 12 deletions(-) diff --git a/lib/cfn-nag/custom_rules/IamRolePassRoleWildcardResourceRule.rb b/lib/cfn-nag/custom_rules/IamRolePassRoleWildcardResourceRule.rb index 5e534cfc..9e942208 100644 --- a/lib/cfn-nag/custom_rules/IamRolePassRoleWildcardResourceRule.rb +++ b/lib/cfn-nag/custom_rules/IamRolePassRoleWildcardResourceRule.rb @@ -23,7 +23,7 @@ def audit_impl(cfn_model) violating_roles = cfn_model.resources_by_type('AWS::IAM::Role').select do |role| violating_policies = role.policy_objects.select do |policy| violating_statements = policy.policy_document.statements.select do |statement| - passrole_action?(statement) && wildcard_resource?(statement) + statement.effect == 'Allow' && passrole_action?(statement) && wildcard_resource?(statement) end !violating_statements.empty? end diff --git a/lib/cfn-nag/custom_rules/passrole_base_rule.rb b/lib/cfn-nag/custom_rules/passrole_base_rule.rb index 2630481a..171f52fa 100644 --- a/lib/cfn-nag/custom_rules/passrole_base_rule.rb +++ b/lib/cfn-nag/custom_rules/passrole_base_rule.rb @@ -16,7 +16,7 @@ def audit_impl(cfn_model) violating_policies = policies.select do |policy| violating_statements = policy.policy_document.statements.select do |statement| - passrole_action?(statement) && wildcard_resource?(statement) + statement.effect == 'Allow' && passrole_action?(statement) && wildcard_resource?(statement) end !violating_statements.empty? end diff --git a/spec/custom_rules/SPCMRule_spec.rb b/spec/custom_rules/SPCMRule_spec.rb index 7e1f1495..3d554e62 100644 --- a/spec/custom_rules/SPCMRule_spec.rb +++ b/spec/custom_rules/SPCMRule_spec.rb @@ -38,7 +38,7 @@ rule = SPCMRule.new rule.spcm_threshold = 1 actual_logical_resource_ids = rule.audit_impl cfn_model - expected_logical_resource_ids = %w[InlinePolicyPass] + expected_logical_resource_ids = %w[InlinePolicyPass InlinePolicyDenyPass] expect(actual_logical_resource_ids).to eq expected_logical_resource_ids end diff --git a/spec/test_templates/yaml/iam_passrole_resource_wildcard/iam_inline_policy_passrole_resource_wildcard_pass.yml b/spec/test_templates/yaml/iam_passrole_resource_wildcard/iam_inline_policy_passrole_resource_wildcard_pass.yml index 045a7d25..2896944c 100644 --- a/spec/test_templates/yaml/iam_passrole_resource_wildcard/iam_inline_policy_passrole_resource_wildcard_pass.yml +++ b/spec/test_templates/yaml/iam_passrole_resource_wildcard/iam_inline_policy_passrole_resource_wildcard_pass.yml @@ -3,7 +3,7 @@ Resources: GenericGroup: Type: AWS::IAM::Group - Properties: + Properties: GroupName: GenericGroup InlinePolicyPass: @@ -15,7 +15,7 @@ Resources: Statement: - Effect: "Allow" - Action: + Action: - "s3:ListBucket" - "s3:GetBucketLocation" Resource: "arn:aws:s3:::*" @@ -29,5 +29,31 @@ Resources: Effect: Allow Action: "iam:PassRole" Resource: "arn:aws:s3:::*" + Groups: + - !Ref GenericGroup + + InlinePolicyDenyPass: + Type: "AWS::IAM::Policy" + Properties: + PolicyName: WildcardDenyResourcePolicy + PolicyDocument: + Version: "2012-10-17" + Statement: + - + Effect: "Allow" + Action: + - "s3:ListBucket" + - "s3:GetBucketLocation" + Resource: "arn:aws:s3:::*" + - + Effect: Allow + Action: + - "s3:ListBucket" + - "s3:GetBucketLocation" + Resource: "*" + - + Effect: Deny + Action: "iam:PassRole" + Resource: "*" Groups: - !Ref GenericGroup \ No newline at end of file diff --git a/spec/test_templates/yaml/iam_passrole_resource_wildcard/iam_managed_policy_passrole_resource_wildcard_pass.yml b/spec/test_templates/yaml/iam_passrole_resource_wildcard/iam_managed_policy_passrole_resource_wildcard_pass.yml index 4d927038..7c8fef71 100644 --- a/spec/test_templates/yaml/iam_passrole_resource_wildcard/iam_managed_policy_passrole_resource_wildcard_pass.yml +++ b/spec/test_templates/yaml/iam_passrole_resource_wildcard/iam_managed_policy_passrole_resource_wildcard_pass.yml @@ -3,7 +3,7 @@ Resources: GenericGroup: Type: AWS::IAM::Group - Properties: + Properties: GroupName: GenericGroup ManagedPolicyPass1: @@ -14,7 +14,7 @@ Resources: Statement: - Effect: "Allow" - Action: + Action: - "s3:ListBucket" - "s3:GetBucketLocation" Resource: "arn:aws:s3:::*" @@ -31,7 +31,7 @@ Resources: - "s3:ListBucket" - "s3:GetBucketLocation" Resource: "*" - + ManagedPolicyPass3: Type: "AWS::IAM::ManagedPolicy" Properties: @@ -43,4 +43,15 @@ Resources: Action: "iam:PassRole" Resource: "arn:aws:s3:::*" Groups: - - !Ref GenericGroup \ No newline at end of file + - !Ref GenericGroup + + ManagedPolicyPass4: + Type: "AWS::IAM::ManagedPolicy" + Properties: + PolicyDocument: + Version: "2012-10-17" + Statement: + - + Effect: Deny + Action: "iam:PassRole" + Resource: "*" \ No newline at end of file diff --git a/spec/test_templates/yaml/iam_passrole_resource_wildcard/iam_role_passrole_resource_wildcard_pass.yml b/spec/test_templates/yaml/iam_passrole_resource_wildcard/iam_role_passrole_resource_wildcard_pass.yml index 0ded2c95..2a478c27 100644 --- a/spec/test_templates/yaml/iam_passrole_resource_wildcard/iam_role_passrole_resource_wildcard_pass.yml +++ b/spec/test_templates/yaml/iam_passrole_resource_wildcard/iam_role_passrole_resource_wildcard_pass.yml @@ -1,8 +1,30 @@ --- Resources: - + RoleDeny: + Type: AWS::IAM::Role + Properties: + AssumeRolePolicyDocument: + Version: "2012-10-17" + Statement: + - + Effect: Allow + Principal: + Service: + - cloudformation.amazonaws.com + Action: + - sts:AssumeRole + Policies: + - + PolicyName: PolicyDeny + PolicyDocument: + Version: "2012-10-17" + Statement: + - + Effect: Deny + Action: "iam:PassRole" + Resource: "*" RoleFail: - Type: AWS::IAM::Role + Type: AWS::IAM::Role Properties: AssumeRolePolicyDocument: Version: "2012-10-17" @@ -22,7 +44,7 @@ Resources: Statement: - Effect: "Allow" - Action: + Action: - "s3:ListBucket" - "s3:GetBucketLocation" Resource: "arn:aws:s3:::*"