From 407401b946860b73e0ab8cf2c0b9c8390a49bd9b Mon Sep 17 00:00:00 2001 From: Ramon Date: Wed, 19 Jan 2022 12:28:52 +0100 Subject: [PATCH] FIX: CrossAccountCheckingRule checking resources without PROPERTY_WITH_POLICYDOCUMENT (#201) * fix - cross account rule checking resources without policydocument * bumping version * fix format Co-authored-by: Ramon --- CHANGELOG.md | 4 +++ cfripper/__version__.py | 2 +- cfripper/rules/cross_account_trust.py | 19 +++++++------- tests/rules/test_CrossAccountTrustRule.py | 26 +++++++++++++++++++ .../es_domain_without_access_policies.yml | 12 +++++++++ ...nsearch_domain_without_access_policies.yml | 12 +++++++++ 6 files changed, 65 insertions(+), 10 deletions(-) create mode 100644 tests/test_templates/rules/CrossAccountTrustRule/es_domain_without_access_policies.yml create mode 100644 tests/test_templates/rules/CrossAccountTrustRule/opensearch_domain_without_access_policies.yml diff --git a/CHANGELOG.md b/CHANGELOG.md index a9f35479..4173e1d6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,10 @@ # Changelog All notable changes to this project will be documented in this file. +## [1.3.1] - 2022-1-17 +### Fixes +- Fixes `CrossAccountCheckingRule` when checking resources without `PROPERTY_WITH_POLICYDOCUMENT`. + ## [1.3.0] - 2022-1-17 ### Improvements - Add `ElasticsearchDomainCrossAccountTrustRule` and `OpenSearchDomainCrossAccountTrustRule` diff --git a/cfripper/__version__.py b/cfripper/__version__.py index 553ce3ca..d00a693c 100644 --- a/cfripper/__version__.py +++ b/cfripper/__version__.py @@ -1,3 +1,3 @@ -VERSION = (1, 3, 0) +VERSION = (1, 3, 1) __version__ = ".".join(map(str, VERSION)) diff --git a/cfripper/rules/cross_account_trust.py b/cfripper/rules/cross_account_trust.py index d5b3ded2..62b0a84a 100644 --- a/cfripper/rules/cross_account_trust.py +++ b/cfripper/rules/cross_account_trust.py @@ -52,15 +52,16 @@ def invoke(self, cfmodel: CFModel, extras: Optional[Dict] = None) -> Result: if isinstance(resource, self.RESOURCE_TYPE): properties = resource.Properties policy_document = getattr(properties, self.PROPERTY_WITH_POLICYDOCUMENT) - for statement in policy_document._statement_as_list(): - filters_available_context = { - "config": self._config, - "extras": extras, - "logical_id": logical_id, - "resource": resource, - "statement": statement, - } - self._do_statement_check(result, logical_id, statement, filters_available_context) + if policy_document: + for statement in policy_document._statement_as_list(): + filters_available_context = { + "config": self._config, + "extras": extras, + "logical_id": logical_id, + "resource": resource, + "statement": statement, + } + self._do_statement_check(result, logical_id, statement, filters_available_context) return result def _do_statement_check( diff --git a/tests/rules/test_CrossAccountTrustRule.py b/tests/rules/test_CrossAccountTrustRule.py index 6c40a8e3..3ec5b1a7 100644 --- a/tests/rules/test_CrossAccountTrustRule.py +++ b/tests/rules/test_CrossAccountTrustRule.py @@ -65,6 +65,16 @@ def template_invalid_with_sts_opensearch_domain(): return get_cfmodel_from("rules/CrossAccountTrustRule/invalid_with_sts_opensearch_domain.yml").resolve() +@pytest.fixture() +def template_es_domain_without_access_policies(): + return get_cfmodel_from("rules/CrossAccountTrustRule/es_domain_without_access_policies.yml").resolve() + + +@pytest.fixture() +def template_opensearch_domain_without_access_policies(): + return get_cfmodel_from("rules/CrossAccountTrustRule/opensearch_domain_without_access_policies.yml").resolve() + + @pytest.fixture() def expected_result_two_roles(): return [ @@ -367,6 +377,14 @@ def test_sts_failure_es_domain(template_invalid_with_sts_es_domain): ) +def test_es_domain_without_access_policies(template_es_domain_without_access_policies): + rule = ElasticsearchDomainCrossAccountTrustRule(Config(aws_account_id="123456789", aws_principals=["999999999"])) + result = rule.invoke(template_es_domain_without_access_policies) + + assert result.valid + assert compare_lists_of_failures(result.failures, []) + + @pytest.mark.parametrize( "principal", [ @@ -442,3 +460,11 @@ def test_sts_failure_opensearch_domain(template_invalid_with_sts_opensearch_doma ) ], ) + + +def test_opensearch_domain_without_access_policies(template_opensearch_domain_without_access_policies): + rule = OpenSearchDomainCrossAccountTrustRule(Config(aws_account_id="123456789", aws_principals=["999999999"])) + result = rule.invoke(template_opensearch_domain_without_access_policies) + + assert result.valid + assert compare_lists_of_failures(result.failures, []) diff --git a/tests/test_templates/rules/CrossAccountTrustRule/es_domain_without_access_policies.yml b/tests/test_templates/rules/CrossAccountTrustRule/es_domain_without_access_policies.yml new file mode 100644 index 00000000..66e80537 --- /dev/null +++ b/tests/test_templates/rules/CrossAccountTrustRule/es_domain_without_access_policies.yml @@ -0,0 +1,12 @@ +AWSTemplateFormatVersion: "2010-09-09" +Description: Testing ES Domain + +Parameters: + Principal: + Type: String + +Resources: + TestDomain: + Type: AWS::Elasticsearch::Domain + Properties: + DomainName: "test" diff --git a/tests/test_templates/rules/CrossAccountTrustRule/opensearch_domain_without_access_policies.yml b/tests/test_templates/rules/CrossAccountTrustRule/opensearch_domain_without_access_policies.yml new file mode 100644 index 00000000..bda57e33 --- /dev/null +++ b/tests/test_templates/rules/CrossAccountTrustRule/opensearch_domain_without_access_policies.yml @@ -0,0 +1,12 @@ +AWSTemplateFormatVersion: "2010-09-09" +Description: Testing OpenSearchService Domain + +Parameters: + Principal: + Type: String + +Resources: + TestDomain: + Type: AWS::OpenSearchService::Domain + Properties: + DomainName: "test"