From 9d28a3885f08684c1cc321b1cf19d70152e8290e Mon Sep 17 00:00:00 2001 From: max-huneshagen Date: Thu, 9 Nov 2023 10:05:46 +0100 Subject: [PATCH 01/21] Create new stack name rule. --- cfripper/config/regex.py | 15 +++++++ cfripper/rules/__init__.py | 2 + cfripper/rules/stack_name_matches_regex.py | 41 +++++++++++++++++++ tests/rules/test_StackNameMatchesRegexRule.py | 39 ++++++++++++++++++ 4 files changed, 97 insertions(+) create mode 100644 cfripper/rules/stack_name_matches_regex.py create mode 100644 tests/rules/test_StackNameMatchesRegexRule.py diff --git a/cfripper/config/regex.py b/cfripper/config/regex.py index 39bb40e2..e5cebfc6 100644 --- a/cfripper/config/regex.py +++ b/cfripper/config/regex.py @@ -173,3 +173,18 @@ - sns:Get* """ REGEX_HAS_STAR_OR_STAR_AFTER_COLON = re.compile(r"^(\w*:)*[*?]+$") + + +""" +Check that stack name only consists of alphanumerical characters and hyphens. +Valid: +- abcdefg +- ABCDEFG +- abcdEFG +- aBc-DeFG +Invalid: +- abc_defg +- AB:cdefg +- !@£$$%aA +""" +REGEX_ALPHANUMERICAL_OR_HYPHEN = re.compile(r"^[A-Za-z0-9\-]+$") diff --git a/cfripper/rules/__init__.py b/cfripper/rules/__init__.py index 0522094f..23970fad 100644 --- a/cfripper/rules/__init__.py +++ b/cfripper/rules/__init__.py @@ -38,6 +38,7 @@ SQSQueuePolicyNotPrincipalRule, SQSQueuePolicyPublicRule, ) +from cfripper.rules.stack_name_matches_regex import StackNameMatchesRegexRule from cfripper.rules.wildcard_policies import ( GenericResourceWildcardPolicyRule, S3BucketPolicyWildcardActionRule, @@ -94,6 +95,7 @@ SQSQueuePolicyNotPrincipalRule, SQSQueuePolicyPublicRule, SQSQueuePolicyWildcardActionRule, + StackNameMatchesRegexRule, WildcardResourceRule, ) } diff --git a/cfripper/rules/stack_name_matches_regex.py b/cfripper/rules/stack_name_matches_regex.py new file mode 100644 index 00000000..4d19da85 --- /dev/null +++ b/cfripper/rules/stack_name_matches_regex.py @@ -0,0 +1,41 @@ +import re +from typing import Dict, Optional + +from pycfmodel.model.cf_model import CFModel + +from cfripper.config.regex import REGEX_ALPHANUMERICAL_OR_HYPHEN +from cfripper.model.enums import RuleMode, RuleRisk +from cfripper.model.result import Result +from cfripper.rules.base_rules import Rule + + +class StackNameMatchesRegexRule(Rule): + """ + Checks that a given stack follows the naming convention given by a regex. + """ + + RULE_MODE = RuleMode.DEBUG + RISK_VALUE = RuleRisk.LOW + REASON = ( + "The stack name {} does not follow the naming convention (only alphanumerical characters and hyphens allowed)." + ) + + def _stack_name_matches_regex(self, stack_name: str) -> bool: + """Check that stack name follows naming convention.""" + return bool(REGEX_ALPHANUMERICAL_OR_HYPHEN.match(stack_name)) + + def invoke(self, cfmodel: CFModel, extras: Optional[Dict] = None) -> Result: + result = Result() + stack_name = self._config.stack_name + if not stack_name: + return result + if not extras: + extras = {} + + if not self._stack_name_matches_regex(stack_name): + self.add_failure_to_result( + result, + self.REASON.format(stack_name), + context={"config": self._config, "extras": extras}, + ) + return result diff --git a/tests/rules/test_StackNameMatchesRegexRule.py b/tests/rules/test_StackNameMatchesRegexRule.py new file mode 100644 index 00000000..92bf6127 --- /dev/null +++ b/tests/rules/test_StackNameMatchesRegexRule.py @@ -0,0 +1,39 @@ +import pytest +from pycfmodel.model.cf_model import CFModel + +from cfripper.config.config import Config +from cfripper.rules import StackNameMatchesRegexRule + + +@pytest.mark.parametrize( + "stack_name, expected_result", + [ + ("justlowercase", True), + ("lowercase-with-hyphens", True), + ("lowercaseANDUPPERCASE", True), + ("lowercase-AND-UPPERCASE-with-hyphens", True), + ("including_underscore", False), + ("including space", False), + ("including-other-symbols!@£$%^&*()", False), + ], +) +def test_stack_name_matches_regex(stack_name, expected_result): + rule = StackNameMatchesRegexRule(Config(stack_name=stack_name, rules=["StackNameMatchesRegexRule"])) + assert rule._stack_name_matches_regex(stack_name) == expected_result + + +def test_works_with_extras(): + rule = StackNameMatchesRegexRule(Config(stack_name="some-valid-stack-name", rules=["StackNameMatchesRegexRule"])) + extras = {"stack": {"tags": [{"key": "project", "value": "some_project"}]}} + result = rule.invoke(cfmodel=CFModel(), extras=extras) + assert result.valid + + +def test_failure_is_added_for_invalid_stack_name(): + rule = StackNameMatchesRegexRule(Config(stack_name="some_invalid_stack_name", rules=["StackNameMatchesRegexRule"])) + result = rule.invoke(cfmodel=CFModel()) + assert result.failures + assert ( + result.failures[0].reason + == "The stack name some_invalid_stack_name does not follow the naming convention (only alphanumerical characters and hyphens allowed)." + ) From 9cb974a210f7921428011f0ae018e286db47d795 Mon Sep 17 00:00:00 2001 From: max-huneshagen Date: Thu, 9 Nov 2023 10:24:04 +0100 Subject: [PATCH 02/21] remove unused import --- cfripper/rules/stack_name_matches_regex.py | 1 - 1 file changed, 1 deletion(-) diff --git a/cfripper/rules/stack_name_matches_regex.py b/cfripper/rules/stack_name_matches_regex.py index 4d19da85..632b4cc2 100644 --- a/cfripper/rules/stack_name_matches_regex.py +++ b/cfripper/rules/stack_name_matches_regex.py @@ -1,4 +1,3 @@ -import re from typing import Dict, Optional from pycfmodel.model.cf_model import CFModel From fa89e2b8c04b9883f6521c34ccd52bb06f2ad405 Mon Sep 17 00:00:00 2001 From: max-huneshagen Date: Fri, 10 Nov 2023 11:59:04 +0100 Subject: [PATCH 03/21] review comments --- cfripper/config/regex.py | 1 + cfripper/rules/stack_name_matches_regex.py | 16 +++++++++++----- tests/rules/test_StackNameMatchesRegexRule.py | 17 +++++++++++++++++ 3 files changed, 29 insertions(+), 5 deletions(-) diff --git a/cfripper/config/regex.py b/cfripper/config/regex.py index e5cebfc6..7bd69ea6 100644 --- a/cfripper/config/regex.py +++ b/cfripper/config/regex.py @@ -182,6 +182,7 @@ - ABCDEFG - abcdEFG - aBc-DeFG +- a1b2c3 Invalid: - abc_defg - AB:cdefg diff --git a/cfripper/rules/stack_name_matches_regex.py b/cfripper/rules/stack_name_matches_regex.py index 632b4cc2..c2f0a935 100644 --- a/cfripper/rules/stack_name_matches_regex.py +++ b/cfripper/rules/stack_name_matches_regex.py @@ -3,29 +3,33 @@ from pycfmodel.model.cf_model import CFModel from cfripper.config.regex import REGEX_ALPHANUMERICAL_OR_HYPHEN -from cfripper.model.enums import RuleMode, RuleRisk +from cfripper.model.enums import RuleGranularity, RuleMode, RuleRisk from cfripper.model.result import Result from cfripper.rules.base_rules import Rule class StackNameMatchesRegexRule(Rule): """ - Checks that a given stack follows the naming convention given by a regex. + Checks that a given stack follows the naming convention given by a regex. For this to work, + the stack name must be given either in the config or in the extras using the key + "stack_name". """ - RULE_MODE = RuleMode.DEBUG + RULE_MODE = RuleMode.DEBUG # for demonstration purposes RISK_VALUE = RuleRisk.LOW + GRANULARITY = RuleGranularity.STACK REASON = ( "The stack name {} does not follow the naming convention (only alphanumerical characters and hyphens allowed)." ) + REGEX = REGEX_ALPHANUMERICAL_OR_HYPHEN def _stack_name_matches_regex(self, stack_name: str) -> bool: """Check that stack name follows naming convention.""" - return bool(REGEX_ALPHANUMERICAL_OR_HYPHEN.match(stack_name)) + return bool(self.REGEX.match(stack_name)) def invoke(self, cfmodel: CFModel, extras: Optional[Dict] = None) -> Result: result = Result() - stack_name = self._config.stack_name + stack_name = self._config.stack_name or extras.get("stack_name", "") if not stack_name: return result if not extras: @@ -35,6 +39,8 @@ def invoke(self, cfmodel: CFModel, extras: Optional[Dict] = None) -> Result: self.add_failure_to_result( result, self.REASON.format(stack_name), + self.GRANULARITY, + risk_value=self.RISK_VALUE, context={"config": self._config, "extras": extras}, ) return result diff --git a/tests/rules/test_StackNameMatchesRegexRule.py b/tests/rules/test_StackNameMatchesRegexRule.py index 92bf6127..60025ecf 100644 --- a/tests/rules/test_StackNameMatchesRegexRule.py +++ b/tests/rules/test_StackNameMatchesRegexRule.py @@ -12,6 +12,7 @@ ("lowercase-with-hyphens", True), ("lowercaseANDUPPERCASE", True), ("lowercase-AND-UPPERCASE-with-hyphens", True), + ("also-123-including-456-numbers", True), ("including_underscore", False), ("including space", False), ("including-other-symbols!@£$%^&*()", False), @@ -28,6 +29,12 @@ def test_works_with_extras(): result = rule.invoke(cfmodel=CFModel(), extras=extras) assert result.valid +def test_stack_name_from_extras(): + rule = StackNameMatchesRegexRule(Config(stack_name="some-valid-stack-name", rules=["StackNameMatchesRegexRule"])) + extras = {"stack": {"tags": [{"key": "project", "value": "some_project"}]}, "stack_name": "some_invalid_name"} + result = rule.invoke(cfmodel=CFModel(), extras=extras) + assert result.valid + def test_failure_is_added_for_invalid_stack_name(): rule = StackNameMatchesRegexRule(Config(stack_name="some_invalid_stack_name", rules=["StackNameMatchesRegexRule"])) @@ -37,3 +44,13 @@ def test_failure_is_added_for_invalid_stack_name(): result.failures[0].reason == "The stack name some_invalid_stack_name does not follow the naming convention (only alphanumerical characters and hyphens allowed)." ) + +def failure_is_added_for_invalid_stack_name_from_extras(): + rule = StackNameMatchesRegexRule(Config(rules=["StackNameMatchesRegexRule"])) + extras = {"stack": {"tags": [{"key": "project", "value": "some_project"}]}, "stack_name": "some_invalid_stack_name"} + result = rule.invoke(cfmodel=CFModel(), extras=extras) + assert result.failures + assert ( + result.failures[0].reason + == "The stack name some_invalid_stack_name does not follow the naming convention (only alphanumerical characters and hyphens allowed)." + ) From 1cff27846e7ed01f179dec54726b7ae0b9d65b20 Mon Sep 17 00:00:00 2001 From: max-huneshagen Date: Fri, 10 Nov 2023 12:22:18 +0100 Subject: [PATCH 04/21] lint error --- tests/rules/test_StackNameMatchesRegexRule.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/tests/rules/test_StackNameMatchesRegexRule.py b/tests/rules/test_StackNameMatchesRegexRule.py index 60025ecf..90eae654 100644 --- a/tests/rules/test_StackNameMatchesRegexRule.py +++ b/tests/rules/test_StackNameMatchesRegexRule.py @@ -29,6 +29,7 @@ def test_works_with_extras(): result = rule.invoke(cfmodel=CFModel(), extras=extras) assert result.valid + def test_stack_name_from_extras(): rule = StackNameMatchesRegexRule(Config(stack_name="some-valid-stack-name", rules=["StackNameMatchesRegexRule"])) extras = {"stack": {"tags": [{"key": "project", "value": "some_project"}]}, "stack_name": "some_invalid_name"} @@ -42,15 +43,18 @@ def test_failure_is_added_for_invalid_stack_name(): assert result.failures assert ( result.failures[0].reason - == "The stack name some_invalid_stack_name does not follow the naming convention (only alphanumerical characters and hyphens allowed)." + == "The stack name some_invalid_stack_name does not follow the naming convention (only alphanumerical " + "characters and hyphens allowed)." ) -def failure_is_added_for_invalid_stack_name_from_extras(): + +def test_failure_is_added_for_invalid_stack_name_from_extras(): rule = StackNameMatchesRegexRule(Config(rules=["StackNameMatchesRegexRule"])) extras = {"stack": {"tags": [{"key": "project", "value": "some_project"}]}, "stack_name": "some_invalid_stack_name"} result = rule.invoke(cfmodel=CFModel(), extras=extras) assert result.failures assert ( - result.failures[0].reason - == "The stack name some_invalid_stack_name does not follow the naming convention (only alphanumerical characters and hyphens allowed)." + result.failures[0].reason + == "The stack name some_invalid_stack_name does not follow the naming convention (only alphanumerical " + "characters and hyphens allowed)." ) From 383223c494eb22942c99c563c1119906ac7a98bd Mon Sep 17 00:00:00 2001 From: max-huneshagen Date: Fri, 10 Nov 2023 17:52:37 +0100 Subject: [PATCH 05/21] add new db encryption rule --- cfripper/rules/__init__.py | 2 + cfripper/rules/storage_encrypted_rule.py | 34 ++++++++ tests/rules/test_StorageEncryptedRule.py | 78 +++++++++++++++++++ .../encrypted_db_resource.yml | 18 +++++ .../missing_storage_encrypted_flag.yml | 17 ++++ .../StorageEncryptedRule/no_db_resource.yml | 16 ++++ .../two_resources_not_encrypted.yml | 35 +++++++++ 7 files changed, 200 insertions(+) create mode 100644 cfripper/rules/storage_encrypted_rule.py create mode 100644 tests/rules/test_StorageEncryptedRule.py create mode 100644 tests/test_templates/rules/StorageEncryptedRule/encrypted_db_resource.yml create mode 100644 tests/test_templates/rules/StorageEncryptedRule/missing_storage_encrypted_flag.yml create mode 100644 tests/test_templates/rules/StorageEncryptedRule/no_db_resource.yml create mode 100644 tests/test_templates/rules/StorageEncryptedRule/two_resources_not_encrypted.yml diff --git a/cfripper/rules/__init__.py b/cfripper/rules/__init__.py index 23970fad..46e4d5d2 100644 --- a/cfripper/rules/__init__.py +++ b/cfripper/rules/__init__.py @@ -39,6 +39,7 @@ SQSQueuePolicyPublicRule, ) from cfripper.rules.stack_name_matches_regex import StackNameMatchesRegexRule +from cfripper.rules.storage_encrypted_rule import StorageEncryptedRule from cfripper.rules.wildcard_policies import ( GenericResourceWildcardPolicyRule, S3BucketPolicyWildcardActionRule, @@ -88,6 +89,7 @@ S3BucketPublicReadAclRule, S3CrossAccountTrustRule, S3ObjectVersioningRule, + StorageEncryptedRule, SNSTopicDangerousPolicyActionsRule, SNSTopicPolicyNotPrincipalRule, SNSTopicPolicyWildcardActionRule, diff --git a/cfripper/rules/storage_encrypted_rule.py b/cfripper/rules/storage_encrypted_rule.py new file mode 100644 index 00000000..6d726d31 --- /dev/null +++ b/cfripper/rules/storage_encrypted_rule.py @@ -0,0 +1,34 @@ +from typing import Dict, Optional + +from pycfmodel.model.cf_model import CFModel + +from cfripper.model.enums import RuleGranularity, RuleMode, RuleRisk +from cfripper.model.result import Result +from cfripper.rules.base_rules import Rule + + +class StorageEncryptedRule(Rule): + RULE_MODE = RuleMode.DEBUG + RISK_VALUE = RuleRisk.LOW + REASON = ( + "The database {} does not seem to be encrypted. Database resources should be encrypted and have the property " + "StorageEncrypted set to True." + ) + GRANULARITY = RuleGranularity.RESOURCE + + def invoke(self, cfmodel: CFModel, extras: Optional[Dict] = None) -> Result: + result = Result() + + for resource in cfmodel.Resources.values(): + is_encrypted = getattr(resource.Properties, "StorageEncrypted", False) + db_name = getattr(resource.Properties, "DBName", "(could not get DB name)") + if resource.Type == "AWS::RDS::DBInstance" and not is_encrypted: + + self.add_failure_to_result( + result, + self.REASON.format(db_name), + context={"config": self._config, "extras": extras}, + resource_types={resource.Type}, + ) + + return result diff --git a/tests/rules/test_StorageEncryptedRule.py b/tests/rules/test_StorageEncryptedRule.py new file mode 100644 index 00000000..780fb935 --- /dev/null +++ b/tests/rules/test_StorageEncryptedRule.py @@ -0,0 +1,78 @@ +import pytest + +from cfripper.model.enums import RuleGranularity, RuleMode, RuleRisk +from cfripper.model.result import Failure +from cfripper.rules.storage_encrypted_rule import StorageEncryptedRule +from tests.utils import get_cfmodel_from + + +def test_storage_encrypted_rule_valid_results(): + rule = StorageEncryptedRule(None) + model = get_cfmodel_from("rules/StorageEncryptedRule/encrypted_db_resource.yml") + resolved_model = model.resolve() + result = rule.invoke(resolved_model) + + assert result.valid + assert result.failures == [] + + +@pytest.mark.parametrize( + "template, failures", + [ + ( + "rules/StorageEncryptedRule/missing_storage_encrypted_flag.yml", + [ + Failure( + granularity=RuleGranularity.RESOURCE, + reason="The database some-name does not seem to be encrypted. Database resources should be " + "encrypted and have the property StorageEncrypted set to True.", + risk_value=RuleRisk.LOW, + rule="StorageEncryptedRule", + rule_mode=RuleMode.DEBUG, + actions=None, + resource_ids=None, + resource_types={"AWS::RDS::DBInstance"}, + ) + ], + ), + ( + "rules/StorageEncryptedRule/two_resources_not_encrypted.yml", + [ + Failure( + granularity=RuleGranularity.RESOURCE, + reason="The database some-name does not seem to be encrypted. Database resources should be " + "encrypted and have the property StorageEncrypted set to True.", + risk_value=RuleRisk.LOW, + rule="StorageEncryptedRule", + rule_mode=RuleMode.DEBUG, + actions=None, + resource_ids=None, + resource_types={"AWS::RDS::DBInstance"}, + ), + Failure( + granularity=RuleGranularity.RESOURCE, + reason="The database some-name-backup does not seem to be encrypted. Database resources should be " + "encrypted and have the property StorageEncrypted set to True.", + risk_value=RuleRisk.LOW, + rule="StorageEncryptedRule", + rule_mode=RuleMode.DEBUG, + actions=None, + resource_ids=None, + resource_types={"AWS::RDS::DBInstance"}, + ), + ], + ), + ( + "rules/StorageEncryptedRule/no_db_resource.yml", + [], + ), + ], +) +def test_add_failure_if_db_resource_not_encrypted(template, failures): + rule = StorageEncryptedRule(None) + model = get_cfmodel_from(template) + resolved_model = model.resolve() + result = rule.invoke(resolved_model) + + assert result.valid + assert result.failures == failures diff --git a/tests/test_templates/rules/StorageEncryptedRule/encrypted_db_resource.yml b/tests/test_templates/rules/StorageEncryptedRule/encrypted_db_resource.yml new file mode 100644 index 00000000..74f79294 --- /dev/null +++ b/tests/test_templates/rules/StorageEncryptedRule/encrypted_db_resource.yml @@ -0,0 +1,18 @@ +Resources: + DBMaster: + Type: AWS::RDS::DBInstance + Properties: + AllocatedStorage: "100" + AllowMajorVersionUpgrade: false + AutoMinorVersionUpgrade: true + BackupRetentionPeriod: 14 + DBInstanceIdentifier: !Sub ${AWS::StackName}-master + DBName: "some-name" + Engine: aurora-postgresql + EngineVersion: "13.2" + KmsKeyId: !GetAtt RDSKMSKey.Arn + MultiAZ: true + StorageEncrypted: true + Tags: + - Key: Name + Value: !Sub ${AWS::StackName}-master \ No newline at end of file diff --git a/tests/test_templates/rules/StorageEncryptedRule/missing_storage_encrypted_flag.yml b/tests/test_templates/rules/StorageEncryptedRule/missing_storage_encrypted_flag.yml new file mode 100644 index 00000000..799b5ebd --- /dev/null +++ b/tests/test_templates/rules/StorageEncryptedRule/missing_storage_encrypted_flag.yml @@ -0,0 +1,17 @@ +Resources: + DBMaster: + Type: AWS::RDS::DBInstance + Properties: + AllocatedStorage: "100" + AllowMajorVersionUpgrade: false + AutoMinorVersionUpgrade: true + BackupRetentionPeriod: 14 + DBInstanceIdentifier: !Sub ${AWS::StackName}-master + DBName: "some-name" + Engine: aurora-postgresql + EngineVersion: "13.2" + KmsKeyId: !GetAtt RDSKMSKey.Arn + MultiAZ: true + Tags: + - Key: Name + Value: !Sub ${AWS::StackName}-master \ No newline at end of file diff --git a/tests/test_templates/rules/StorageEncryptedRule/no_db_resource.yml b/tests/test_templates/rules/StorageEncryptedRule/no_db_resource.yml new file mode 100644 index 00000000..081ad730 --- /dev/null +++ b/tests/test_templates/rules/StorageEncryptedRule/no_db_resource.yml @@ -0,0 +1,16 @@ +Resources: + SomeResource: + Type: AWS::RDS::DBCluster + Properties: + AllocatedStorage: "100" + AutoMinorVersionUpgrade: true + BackupRetentionPeriod: 14 + DBClusterIdentifier: !Sub ${AWS::StackName}-master + DatabaseName: "some-name" + Engine: aurora-postgresql + EngineVersion: "13.2" + KmsKeyId: !GetAtt RDSKMSKey.Arn + StorageEncrypted: false + Tags: + - Key: Name + Value: !Sub ${AWS::StackName}-master \ No newline at end of file diff --git a/tests/test_templates/rules/StorageEncryptedRule/two_resources_not_encrypted.yml b/tests/test_templates/rules/StorageEncryptedRule/two_resources_not_encrypted.yml new file mode 100644 index 00000000..e20266f1 --- /dev/null +++ b/tests/test_templates/rules/StorageEncryptedRule/two_resources_not_encrypted.yml @@ -0,0 +1,35 @@ +Resources: + DBMaster: + Type: AWS::RDS::DBInstance + Properties: + AllocatedStorage: "100" + AllowMajorVersionUpgrade: false + AutoMinorVersionUpgrade: true + BackupRetentionPeriod: 14 + DBInstanceIdentifier: !Sub ${AWS::StackName}-master + DBName: "some-name" + Engine: aurora-postgresql + EngineVersion: "13.2" + KmsKeyId: !GetAtt RDSKMSKey.Arn + MultiAZ: true + StorageEncrypted: false + Tags: + - Key: Name + Value: !Sub ${AWS::StackName}-master + DBBackup: + Type: AWS::RDS::DBInstance + Properties: + AllocatedStorage: "100" + AllowMajorVersionUpgrade: true + AutoMinorVersionUpgrade: false + BackupRetentionPeriod: 7 + DBInstanceIdentifier: !Sub ${AWS::StackName}-backup + DBName: "some-name-backup" + Engine: aurora-postgresql + EngineVersion: "13.2" + KmsKeyId: !GetAtt RDSKMSKey.Arn + MultiAZ: true + StorageEncrypted: false + Tags: + - Key: Name + Value: !Sub ${AWS::StackName}-backup \ No newline at end of file From 5b83fae3396a5850554cc7b414e81611ad645218 Mon Sep 17 00:00:00 2001 From: max-huneshagen Date: Mon, 13 Nov 2023 09:49:26 +0100 Subject: [PATCH 06/21] remove changes from other pr --- cfripper/config/regex.py | 16 ----- cfripper/rules/__init__.py | 2 - cfripper/rules/stack_name_matches_regex.py | 46 -------------- tests/rules/test_StackNameMatchesRegexRule.py | 60 ------------------- 4 files changed, 124 deletions(-) delete mode 100644 cfripper/rules/stack_name_matches_regex.py delete mode 100644 tests/rules/test_StackNameMatchesRegexRule.py diff --git a/cfripper/config/regex.py b/cfripper/config/regex.py index 7bd69ea6..39bb40e2 100644 --- a/cfripper/config/regex.py +++ b/cfripper/config/regex.py @@ -173,19 +173,3 @@ - sns:Get* """ REGEX_HAS_STAR_OR_STAR_AFTER_COLON = re.compile(r"^(\w*:)*[*?]+$") - - -""" -Check that stack name only consists of alphanumerical characters and hyphens. -Valid: -- abcdefg -- ABCDEFG -- abcdEFG -- aBc-DeFG -- a1b2c3 -Invalid: -- abc_defg -- AB:cdefg -- !@£$$%aA -""" -REGEX_ALPHANUMERICAL_OR_HYPHEN = re.compile(r"^[A-Za-z0-9\-]+$") diff --git a/cfripper/rules/__init__.py b/cfripper/rules/__init__.py index 46e4d5d2..adf902a8 100644 --- a/cfripper/rules/__init__.py +++ b/cfripper/rules/__init__.py @@ -38,7 +38,6 @@ SQSQueuePolicyNotPrincipalRule, SQSQueuePolicyPublicRule, ) -from cfripper.rules.stack_name_matches_regex import StackNameMatchesRegexRule from cfripper.rules.storage_encrypted_rule import StorageEncryptedRule from cfripper.rules.wildcard_policies import ( GenericResourceWildcardPolicyRule, @@ -97,7 +96,6 @@ SQSQueuePolicyNotPrincipalRule, SQSQueuePolicyPublicRule, SQSQueuePolicyWildcardActionRule, - StackNameMatchesRegexRule, WildcardResourceRule, ) } diff --git a/cfripper/rules/stack_name_matches_regex.py b/cfripper/rules/stack_name_matches_regex.py deleted file mode 100644 index c2f0a935..00000000 --- a/cfripper/rules/stack_name_matches_regex.py +++ /dev/null @@ -1,46 +0,0 @@ -from typing import Dict, Optional - -from pycfmodel.model.cf_model import CFModel - -from cfripper.config.regex import REGEX_ALPHANUMERICAL_OR_HYPHEN -from cfripper.model.enums import RuleGranularity, RuleMode, RuleRisk -from cfripper.model.result import Result -from cfripper.rules.base_rules import Rule - - -class StackNameMatchesRegexRule(Rule): - """ - Checks that a given stack follows the naming convention given by a regex. For this to work, - the stack name must be given either in the config or in the extras using the key - "stack_name". - """ - - RULE_MODE = RuleMode.DEBUG # for demonstration purposes - RISK_VALUE = RuleRisk.LOW - GRANULARITY = RuleGranularity.STACK - REASON = ( - "The stack name {} does not follow the naming convention (only alphanumerical characters and hyphens allowed)." - ) - REGEX = REGEX_ALPHANUMERICAL_OR_HYPHEN - - def _stack_name_matches_regex(self, stack_name: str) -> bool: - """Check that stack name follows naming convention.""" - return bool(self.REGEX.match(stack_name)) - - def invoke(self, cfmodel: CFModel, extras: Optional[Dict] = None) -> Result: - result = Result() - stack_name = self._config.stack_name or extras.get("stack_name", "") - if not stack_name: - return result - if not extras: - extras = {} - - if not self._stack_name_matches_regex(stack_name): - self.add_failure_to_result( - result, - self.REASON.format(stack_name), - self.GRANULARITY, - risk_value=self.RISK_VALUE, - context={"config": self._config, "extras": extras}, - ) - return result diff --git a/tests/rules/test_StackNameMatchesRegexRule.py b/tests/rules/test_StackNameMatchesRegexRule.py deleted file mode 100644 index 90eae654..00000000 --- a/tests/rules/test_StackNameMatchesRegexRule.py +++ /dev/null @@ -1,60 +0,0 @@ -import pytest -from pycfmodel.model.cf_model import CFModel - -from cfripper.config.config import Config -from cfripper.rules import StackNameMatchesRegexRule - - -@pytest.mark.parametrize( - "stack_name, expected_result", - [ - ("justlowercase", True), - ("lowercase-with-hyphens", True), - ("lowercaseANDUPPERCASE", True), - ("lowercase-AND-UPPERCASE-with-hyphens", True), - ("also-123-including-456-numbers", True), - ("including_underscore", False), - ("including space", False), - ("including-other-symbols!@£$%^&*()", False), - ], -) -def test_stack_name_matches_regex(stack_name, expected_result): - rule = StackNameMatchesRegexRule(Config(stack_name=stack_name, rules=["StackNameMatchesRegexRule"])) - assert rule._stack_name_matches_regex(stack_name) == expected_result - - -def test_works_with_extras(): - rule = StackNameMatchesRegexRule(Config(stack_name="some-valid-stack-name", rules=["StackNameMatchesRegexRule"])) - extras = {"stack": {"tags": [{"key": "project", "value": "some_project"}]}} - result = rule.invoke(cfmodel=CFModel(), extras=extras) - assert result.valid - - -def test_stack_name_from_extras(): - rule = StackNameMatchesRegexRule(Config(stack_name="some-valid-stack-name", rules=["StackNameMatchesRegexRule"])) - extras = {"stack": {"tags": [{"key": "project", "value": "some_project"}]}, "stack_name": "some_invalid_name"} - result = rule.invoke(cfmodel=CFModel(), extras=extras) - assert result.valid - - -def test_failure_is_added_for_invalid_stack_name(): - rule = StackNameMatchesRegexRule(Config(stack_name="some_invalid_stack_name", rules=["StackNameMatchesRegexRule"])) - result = rule.invoke(cfmodel=CFModel()) - assert result.failures - assert ( - result.failures[0].reason - == "The stack name some_invalid_stack_name does not follow the naming convention (only alphanumerical " - "characters and hyphens allowed)." - ) - - -def test_failure_is_added_for_invalid_stack_name_from_extras(): - rule = StackNameMatchesRegexRule(Config(rules=["StackNameMatchesRegexRule"])) - extras = {"stack": {"tags": [{"key": "project", "value": "some_project"}]}, "stack_name": "some_invalid_stack_name"} - result = rule.invoke(cfmodel=CFModel(), extras=extras) - assert result.failures - assert ( - result.failures[0].reason - == "The stack name some_invalid_stack_name does not follow the naming convention (only alphanumerical " - "characters and hyphens allowed)." - ) From f210ffaae22789d5e41012defd05cbf844e15c0e Mon Sep 17 00:00:00 2001 From: Jordi Soucheiron Date: Mon, 13 Nov 2023 09:57:36 +0100 Subject: [PATCH 07/21] Update lint-and-test.yml (#247) * Update lint-and-test.yml * Update pyyaml dependency --- .github/workflows/lint-and-test.yml | 2 +- requirements.txt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/lint-and-test.yml b/.github/workflows/lint-and-test.yml index e5eae075..8677fe47 100644 --- a/.github/workflows/lint-and-test.yml +++ b/.github/workflows/lint-and-test.yml @@ -9,7 +9,7 @@ jobs: strategy: fail-fast: false matrix: - python-version: ['3.7', '3.8', '3.9', '3.10'] + python-version: ['3.7', '3.8', '3.9', '3.10', '3.11', '3.12'] name: Python ${{ matrix.python-version }} diff --git a/requirements.txt b/requirements.txt index 2ea0232e..fa70e777 100644 --- a/requirements.txt +++ b/requirements.txt @@ -14,7 +14,7 @@ pycfmodel==0.20.0 pydantic==1.9.0 pydash==6.0.0 python-dateutil==2.8.2 -pyyaml==6.0 +pyyaml==6.0.1 s3transfer==0.5.2 six==1.16.0 typing-extensions==4.1.1 From 9b3efd2fb7883d8f65dd61020855a33baad13c7e Mon Sep 17 00:00:00 2001 From: Jordi Soucheiron Date: Mon, 13 Nov 2023 09:57:50 +0100 Subject: [PATCH 08/21] Update README.md (#246) * Update README.md * Add license badge --- README.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/README.md b/README.md index 287364e6..f8658527 100644 --- a/README.md +++ b/README.md @@ -6,8 +6,7 @@ ![Build Status](https://github.com/Skyscanner/cfripper/workflows/PyPI%20release/badge.svg) [![PyPI version](https://badge.fury.io/py/cfripper.svg)](https://badge.fury.io/py/cfripper) -[![Total alerts](https://img.shields.io/lgtm/alerts/g/Skyscanner/cfripper.svg?logo=lgtm&logoWidth=18)](https://lgtm.com/projects/g/Skyscanner/cfripper/alerts/) -[![Language grade: Python](https://img.shields.io/lgtm/grade/python/g/Skyscanner/cfripper.svg?logo=lgtm&logoWidth=18)](https://lgtm.com/projects/g/Skyscanner/cfripper/context:python) +![License](https://img.shields.io/github/license/skyscanner/cfripper) CFRipper is a Library and CLI security analyzer for AWS CloudFormation templates. You can use CFRipper to prevent deploying insecure AWS resources into your Cloud environment. You can write your own compliance checks by adding new custom plugins. From 021739b1933d2ada83e59be0c47ab4a9a392f18e Mon Sep 17 00:00:00 2001 From: max-huneshagen Date: Mon, 13 Nov 2023 10:59:12 +0100 Subject: [PATCH 09/21] rebase --- cfripper/config/regex.py | 16 +++++ cfripper/rules/__init__.py | 2 + cfripper/rules/stack_name_matches_regex.py | 45 ++++++++++++++ tests/rules/test_StackNameMatchesRegexRule.py | 60 +++++++++++++++++++ 4 files changed, 123 insertions(+) create mode 100644 cfripper/rules/stack_name_matches_regex.py create mode 100644 tests/rules/test_StackNameMatchesRegexRule.py diff --git a/cfripper/config/regex.py b/cfripper/config/regex.py index 39bb40e2..7bd69ea6 100644 --- a/cfripper/config/regex.py +++ b/cfripper/config/regex.py @@ -173,3 +173,19 @@ - sns:Get* """ REGEX_HAS_STAR_OR_STAR_AFTER_COLON = re.compile(r"^(\w*:)*[*?]+$") + + +""" +Check that stack name only consists of alphanumerical characters and hyphens. +Valid: +- abcdefg +- ABCDEFG +- abcdEFG +- aBc-DeFG +- a1b2c3 +Invalid: +- abc_defg +- AB:cdefg +- !@£$$%aA +""" +REGEX_ALPHANUMERICAL_OR_HYPHEN = re.compile(r"^[A-Za-z0-9\-]+$") diff --git a/cfripper/rules/__init__.py b/cfripper/rules/__init__.py index adf902a8..46e4d5d2 100644 --- a/cfripper/rules/__init__.py +++ b/cfripper/rules/__init__.py @@ -38,6 +38,7 @@ SQSQueuePolicyNotPrincipalRule, SQSQueuePolicyPublicRule, ) +from cfripper.rules.stack_name_matches_regex import StackNameMatchesRegexRule from cfripper.rules.storage_encrypted_rule import StorageEncryptedRule from cfripper.rules.wildcard_policies import ( GenericResourceWildcardPolicyRule, @@ -96,6 +97,7 @@ SQSQueuePolicyNotPrincipalRule, SQSQueuePolicyPublicRule, SQSQueuePolicyWildcardActionRule, + StackNameMatchesRegexRule, WildcardResourceRule, ) } diff --git a/cfripper/rules/stack_name_matches_regex.py b/cfripper/rules/stack_name_matches_regex.py new file mode 100644 index 00000000..ca2c8e8f --- /dev/null +++ b/cfripper/rules/stack_name_matches_regex.py @@ -0,0 +1,45 @@ +from typing import Dict, Optional + +from pycfmodel.model.cf_model import CFModel + +from cfripper.config.regex import REGEX_ALPHANUMERICAL_OR_HYPHEN +from cfripper.model.enums import RuleGranularity, RuleMode, RuleRisk +from cfripper.model.result import Result +from cfripper.rules.base_rules import Rule + + +class StackNameMatchesRegexRule(Rule): + """ + Checks that a given stack follows the naming convention given by a regex. For this to work, + the stack name must be given either in the config or in the extras using the key + "stack_name". + """ + + RULE_MODE = RuleMode.DEBUG # for demonstration purposes + RISK_VALUE = RuleRisk.LOW + GRANULARITY = RuleGranularity.STACK + REASON = "The stack name {} does not follow the naming convention, reason: {}" + REGEX = REGEX_ALPHANUMERICAL_OR_HYPHEN + REGEX_REASON = "Only alphanumerical characters and hyphens allowed." + + def _stack_name_matches_regex(self, stack_name: str) -> bool: + """Check that stack name follows naming convention.""" + return bool(self.REGEX.match(stack_name)) + + def invoke(self, cfmodel: CFModel, extras: Optional[Dict] = None) -> Result: + result = Result() + stack_name = self._config.stack_name or extras.get("stack_name", "") + if not stack_name: + return result + if not extras: + extras = {} + + if not self._stack_name_matches_regex(stack_name): + self.add_failure_to_result( + result, + self.REASON.format(stack_name, self.REGEX_REASON), + self.GRANULARITY, + risk_value=self.RISK_VALUE, + context={"config": self._config, "extras": extras}, + ) + return result diff --git a/tests/rules/test_StackNameMatchesRegexRule.py b/tests/rules/test_StackNameMatchesRegexRule.py new file mode 100644 index 00000000..684f1377 --- /dev/null +++ b/tests/rules/test_StackNameMatchesRegexRule.py @@ -0,0 +1,60 @@ +import pytest +from pycfmodel.model.cf_model import CFModel + +from cfripper.config.config import Config +from cfripper.rules import StackNameMatchesRegexRule + + +@pytest.mark.parametrize( + "stack_name, expected_result", + [ + ("justlowercase", True), + ("lowercase-with-hyphens", True), + ("lowercaseANDUPPERCASE", True), + ("lowercase-AND-UPPERCASE-with-hyphens", True), + ("also-123-including-456-numbers", True), + ("including_underscore", False), + ("including space", False), + ("including-other-symbols!@£$%^&*()", False), + ], +) +def test_stack_name_matches_regex(stack_name, expected_result): + rule = StackNameMatchesRegexRule(Config(stack_name=stack_name, rules=["StackNameMatchesRegexRule"])) + assert rule._stack_name_matches_regex(stack_name) == expected_result + + +def test_works_with_extras(): + rule = StackNameMatchesRegexRule(Config(stack_name="some-valid-stack-name", rules=["StackNameMatchesRegexRule"])) + extras = {"stack": {"tags": [{"key": "project", "value": "some_project"}]}} + result = rule.invoke(cfmodel=CFModel(), extras=extras) + assert result.valid + + +def test_stack_name_from_extras(): + rule = StackNameMatchesRegexRule(Config(stack_name="some-valid-stack-name", rules=["StackNameMatchesRegexRule"])) + extras = {"stack": {"tags": [{"key": "project", "value": "some_project"}]}, "stack_name": "some_invalid_name"} + result = rule.invoke(cfmodel=CFModel(), extras=extras) + assert result.valid + + +def test_failure_is_added_for_invalid_stack_name(): + rule = StackNameMatchesRegexRule(Config(stack_name="some_invalid_stack_name", rules=["StackNameMatchesRegexRule"])) + result = rule.invoke(cfmodel=CFModel()) + assert result.failures + assert ( + result.failures[0].reason + == "The stack name some_invalid_stack_name does not follow the naming convention, reason: Only alphanumerical " + "characters and hyphens allowed." + ) + + +def test_failure_is_added_for_invalid_stack_name_from_extras(): + rule = StackNameMatchesRegexRule(Config(rules=["StackNameMatchesRegexRule"])) + extras = {"stack": {"tags": [{"key": "project", "value": "some_project"}]}, "stack_name": "some_invalid_stack_name"} + result = rule.invoke(cfmodel=CFModel(), extras=extras) + assert result.failures + assert ( + result.failures[0].reason + == "The stack name some_invalid_stack_name does not follow the naming convention, reason: Only alphanumerical " + "characters and hyphens allowed." + ) From db344b6bb0f4bf8f38611278a8edf7543c1be264 Mon Sep 17 00:00:00 2001 From: max-huneshagen Date: Fri, 10 Nov 2023 11:59:04 +0100 Subject: [PATCH 10/21] rebase onto master --- tests/rules/test_StackNameMatchesRegexRule.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/rules/test_StackNameMatchesRegexRule.py b/tests/rules/test_StackNameMatchesRegexRule.py index 684f1377..4ec99b29 100644 --- a/tests/rules/test_StackNameMatchesRegexRule.py +++ b/tests/rules/test_StackNameMatchesRegexRule.py @@ -29,6 +29,12 @@ def test_works_with_extras(): result = rule.invoke(cfmodel=CFModel(), extras=extras) assert result.valid +def test_stack_name_from_extras(): + rule = StackNameMatchesRegexRule(Config(stack_name="some-valid-stack-name", rules=["StackNameMatchesRegexRule"])) + extras = {"stack": {"tags": [{"key": "project", "value": "some_project"}]}, "stack_name": "some_invalid_name"} + result = rule.invoke(cfmodel=CFModel(), extras=extras) + assert result.valid + def test_stack_name_from_extras(): rule = StackNameMatchesRegexRule(Config(stack_name="some-valid-stack-name", rules=["StackNameMatchesRegexRule"])) @@ -58,3 +64,13 @@ def test_failure_is_added_for_invalid_stack_name_from_extras(): == "The stack name some_invalid_stack_name does not follow the naming convention, reason: Only alphanumerical " "characters and hyphens allowed." ) + +def failure_is_added_for_invalid_stack_name_from_extras(): + rule = StackNameMatchesRegexRule(Config(rules=["StackNameMatchesRegexRule"])) + extras = {"stack": {"tags": [{"key": "project", "value": "some_project"}]}, "stack_name": "some_invalid_stack_name"} + result = rule.invoke(cfmodel=CFModel(), extras=extras) + assert result.failures + assert ( + result.failures[0].reason + == "The stack name some_invalid_stack_name does not follow the naming convention (only alphanumerical characters and hyphens allowed)." + ) From 607b7f814dbaa8881750849d57c895a6ec6fd971 Mon Sep 17 00:00:00 2001 From: max-huneshagen Date: Mon, 13 Nov 2023 09:49:26 +0100 Subject: [PATCH 11/21] rebase --- cfripper/config/regex.py | 16 ---------------- cfripper/rules/__init__.py | 2 -- 2 files changed, 18 deletions(-) diff --git a/cfripper/config/regex.py b/cfripper/config/regex.py index 7bd69ea6..39bb40e2 100644 --- a/cfripper/config/regex.py +++ b/cfripper/config/regex.py @@ -173,19 +173,3 @@ - sns:Get* """ REGEX_HAS_STAR_OR_STAR_AFTER_COLON = re.compile(r"^(\w*:)*[*?]+$") - - -""" -Check that stack name only consists of alphanumerical characters and hyphens. -Valid: -- abcdefg -- ABCDEFG -- abcdEFG -- aBc-DeFG -- a1b2c3 -Invalid: -- abc_defg -- AB:cdefg -- !@£$$%aA -""" -REGEX_ALPHANUMERICAL_OR_HYPHEN = re.compile(r"^[A-Za-z0-9\-]+$") diff --git a/cfripper/rules/__init__.py b/cfripper/rules/__init__.py index 46e4d5d2..adf902a8 100644 --- a/cfripper/rules/__init__.py +++ b/cfripper/rules/__init__.py @@ -38,7 +38,6 @@ SQSQueuePolicyNotPrincipalRule, SQSQueuePolicyPublicRule, ) -from cfripper.rules.stack_name_matches_regex import StackNameMatchesRegexRule from cfripper.rules.storage_encrypted_rule import StorageEncryptedRule from cfripper.rules.wildcard_policies import ( GenericResourceWildcardPolicyRule, @@ -97,7 +96,6 @@ SQSQueuePolicyNotPrincipalRule, SQSQueuePolicyPublicRule, SQSQueuePolicyWildcardActionRule, - StackNameMatchesRegexRule, WildcardResourceRule, ) } From 6090f3ecd7afa8b74455a6f2b8f6bb86e263c942 Mon Sep 17 00:00:00 2001 From: max-huneshagen Date: Mon, 13 Nov 2023 11:18:36 +0100 Subject: [PATCH 12/21] make lint --- cfripper/rules/__init__.py | 2 -- tests/rules/test_StackNameMatchesRegexRule.py | 6 +++--- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/cfripper/rules/__init__.py b/cfripper/rules/__init__.py index 7df590ac..46e4d5d2 100644 --- a/cfripper/rules/__init__.py +++ b/cfripper/rules/__init__.py @@ -38,10 +38,8 @@ SQSQueuePolicyNotPrincipalRule, SQSQueuePolicyPublicRule, ) - from cfripper.rules.stack_name_matches_regex import StackNameMatchesRegexRule from cfripper.rules.storage_encrypted_rule import StorageEncryptedRule - from cfripper.rules.wildcard_policies import ( GenericResourceWildcardPolicyRule, S3BucketPolicyWildcardActionRule, diff --git a/tests/rules/test_StackNameMatchesRegexRule.py b/tests/rules/test_StackNameMatchesRegexRule.py index 74468e2d..0de63cb3 100644 --- a/tests/rules/test_StackNameMatchesRegexRule.py +++ b/tests/rules/test_StackNameMatchesRegexRule.py @@ -59,13 +59,13 @@ def test_failure_is_added_for_invalid_stack_name_from_extras(): "characters and hyphens allowed." ) - + def failure_is_added_for_invalid_stack_name_from_extras(): rule = StackNameMatchesRegexRule(Config(rules=["StackNameMatchesRegexRule"])) extras = {"stack": {"tags": [{"key": "project", "value": "some_project"}]}, "stack_name": "some_invalid_stack_name"} result = rule.invoke(cfmodel=CFModel(), extras=extras) assert result.failures assert ( - result.failures[0].reason - == "The stack name some_invalid_stack_name does not follow the naming convention (only alphanumerical characters and hyphens allowed)." + result.failures[0].reason + == "The stack name some_invalid_stack_name does not follow the naming convention (only alphanumerical characters and hyphens allowed)." ) From e79c3dd395ae3b789a278b299a719d41e98df1ba Mon Sep 17 00:00:00 2001 From: max-huneshagen Date: Mon, 13 Nov 2023 11:20:42 +0100 Subject: [PATCH 13/21] remove duplicate test --- tests/rules/test_StackNameMatchesRegexRule.py | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/tests/rules/test_StackNameMatchesRegexRule.py b/tests/rules/test_StackNameMatchesRegexRule.py index 0de63cb3..684f1377 100644 --- a/tests/rules/test_StackNameMatchesRegexRule.py +++ b/tests/rules/test_StackNameMatchesRegexRule.py @@ -58,14 +58,3 @@ def test_failure_is_added_for_invalid_stack_name_from_extras(): == "The stack name some_invalid_stack_name does not follow the naming convention, reason: Only alphanumerical " "characters and hyphens allowed." ) - - -def failure_is_added_for_invalid_stack_name_from_extras(): - rule = StackNameMatchesRegexRule(Config(rules=["StackNameMatchesRegexRule"])) - extras = {"stack": {"tags": [{"key": "project", "value": "some_project"}]}, "stack_name": "some_invalid_stack_name"} - result = rule.invoke(cfmodel=CFModel(), extras=extras) - assert result.failures - assert ( - result.failures[0].reason - == "The stack name some_invalid_stack_name does not follow the naming convention (only alphanumerical characters and hyphens allowed)." - ) From e9753dbba1adad242275dd8759a1068bddca8b39 Mon Sep 17 00:00:00 2001 From: max-huneshagen Date: Mon, 13 Nov 2023 11:48:20 +0100 Subject: [PATCH 14/21] update changelog --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 34dd0e8f..0e0fd091 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,11 @@ # Changelog All notable changes to this project will be documented in this file. +## [1.15.0] +### Additions +- New rules: `StackNameMatchesRegexRule` and `StorageEncryptedRule` +- New regex: `REGEX_ALPHANUMERICAL_OR_HYPHEN` to check if stack name only consists of alphanumerical characters and hyphens. + ## [1.14.0] ### Additions - `Config` includes a metrics logger, and it is called to register when a filter is used From 6a0bab966ad4a5d316e9113f6cd44c46abac8cc8 Mon Sep 17 00:00:00 2001 From: max-huneshagen Date: Mon, 13 Nov 2023 15:36:58 +0100 Subject: [PATCH 15/21] add comment as for stack name rule --- cfripper/rules/storage_encrypted_rule.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cfripper/rules/storage_encrypted_rule.py b/cfripper/rules/storage_encrypted_rule.py index 6d726d31..e5e96f44 100644 --- a/cfripper/rules/storage_encrypted_rule.py +++ b/cfripper/rules/storage_encrypted_rule.py @@ -8,7 +8,7 @@ class StorageEncryptedRule(Rule): - RULE_MODE = RuleMode.DEBUG + RULE_MODE = RuleMode.DEBUG # for demonstration purposes RISK_VALUE = RuleRisk.LOW REASON = ( "The database {} does not seem to be encrypted. Database resources should be encrypted and have the property " From 74a38a6b1f87ffde887757673d047fe41718433d Mon Sep 17 00:00:00 2001 From: max-huneshagen Date: Mon, 13 Nov 2023 15:38:58 +0100 Subject: [PATCH 16/21] make format --- cfripper/rules/storage_encrypted_rule.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cfripper/rules/storage_encrypted_rule.py b/cfripper/rules/storage_encrypted_rule.py index e5e96f44..1b946efb 100644 --- a/cfripper/rules/storage_encrypted_rule.py +++ b/cfripper/rules/storage_encrypted_rule.py @@ -8,7 +8,7 @@ class StorageEncryptedRule(Rule): - RULE_MODE = RuleMode.DEBUG # for demonstration purposes + RULE_MODE = RuleMode.DEBUG # for demonstration purposes RISK_VALUE = RuleRisk.LOW REASON = ( "The database {} does not seem to be encrypted. Database resources should be encrypted and have the property " From 81e8cc850dbe036ce874a1f0eb3397d4f020c29e Mon Sep 17 00:00:00 2001 From: max-huneshagen Date: Mon, 13 Nov 2023 16:54:28 +0100 Subject: [PATCH 17/21] rule not invoked for aurora --- cfripper/rules/storage_encrypted_rule.py | 6 +++++- tests/rules/test_StorageEncryptedRule.py | 10 ++++++++++ .../aurora_engine_used.yml | 18 ++++++++++++++++++ .../encrypted_db_resource.yml | 2 +- .../missing_storage_encrypted_flag.yml | 2 +- .../StorageEncryptedRule/no_db_resource.yml | 2 +- .../two_resources_not_encrypted.yml | 4 ++-- 7 files changed, 38 insertions(+), 6 deletions(-) create mode 100644 tests/test_templates/rules/StorageEncryptedRule/aurora_engine_used.yml diff --git a/cfripper/rules/storage_encrypted_rule.py b/cfripper/rules/storage_encrypted_rule.py index 1b946efb..ff6e3d1c 100644 --- a/cfripper/rules/storage_encrypted_rule.py +++ b/cfripper/rules/storage_encrypted_rule.py @@ -22,7 +22,11 @@ def invoke(self, cfmodel: CFModel, extras: Optional[Dict] = None) -> Result: for resource in cfmodel.Resources.values(): is_encrypted = getattr(resource.Properties, "StorageEncrypted", False) db_name = getattr(resource.Properties, "DBName", "(could not get DB name)") - if resource.Type == "AWS::RDS::DBInstance" and not is_encrypted: + if ( + resource.Type == "AWS::RDS::DBInstance" + and not is_encrypted + and not getattr(resource.Properties, "Engine", "").startswith("aurora") + ): self.add_failure_to_result( result, diff --git a/tests/rules/test_StorageEncryptedRule.py b/tests/rules/test_StorageEncryptedRule.py index 780fb935..0c7ce676 100644 --- a/tests/rules/test_StorageEncryptedRule.py +++ b/tests/rules/test_StorageEncryptedRule.py @@ -16,6 +16,16 @@ def test_storage_encrypted_rule_valid_results(): assert result.failures == [] +def test_rule_not_invoked_for_aurora(): + rule = StorageEncryptedRule(None) + model = get_cfmodel_from("rules/StorageEncryptedRule/aurora_engine_used.yml") + resolved_model = model.resolve() + result = rule.invoke(resolved_model) + + assert result.valid + assert result.failures == [] + + @pytest.mark.parametrize( "template, failures", [ diff --git a/tests/test_templates/rules/StorageEncryptedRule/aurora_engine_used.yml b/tests/test_templates/rules/StorageEncryptedRule/aurora_engine_used.yml new file mode 100644 index 00000000..ab74cd33 --- /dev/null +++ b/tests/test_templates/rules/StorageEncryptedRule/aurora_engine_used.yml @@ -0,0 +1,18 @@ +Resources: + DBMaster: + Type: AWS::RDS::DBInstance + Properties: + AllocatedStorage: "100" + AllowMajorVersionUpgrade: false + AutoMinorVersionUpgrade: true + BackupRetentionPeriod: 14 + DBInstanceIdentifier: !Sub ${AWS::StackName}-master + DBName: "some-name" + Engine: aurora-postgresql + EngineVersion: "13.2" + KmsKeyId: !GetAtt RDSKMSKey.Arn + MultiAZ: true + StorageEncrypted: false + Tags: + - Key: Name + Value: !Sub ${AWS::StackName}-master \ No newline at end of file diff --git a/tests/test_templates/rules/StorageEncryptedRule/encrypted_db_resource.yml b/tests/test_templates/rules/StorageEncryptedRule/encrypted_db_resource.yml index 74f79294..c89b7b63 100644 --- a/tests/test_templates/rules/StorageEncryptedRule/encrypted_db_resource.yml +++ b/tests/test_templates/rules/StorageEncryptedRule/encrypted_db_resource.yml @@ -8,7 +8,7 @@ Resources: BackupRetentionPeriod: 14 DBInstanceIdentifier: !Sub ${AWS::StackName}-master DBName: "some-name" - Engine: aurora-postgresql + Engine: mysql EngineVersion: "13.2" KmsKeyId: !GetAtt RDSKMSKey.Arn MultiAZ: true diff --git a/tests/test_templates/rules/StorageEncryptedRule/missing_storage_encrypted_flag.yml b/tests/test_templates/rules/StorageEncryptedRule/missing_storage_encrypted_flag.yml index 799b5ebd..2cb78a3f 100644 --- a/tests/test_templates/rules/StorageEncryptedRule/missing_storage_encrypted_flag.yml +++ b/tests/test_templates/rules/StorageEncryptedRule/missing_storage_encrypted_flag.yml @@ -8,7 +8,7 @@ Resources: BackupRetentionPeriod: 14 DBInstanceIdentifier: !Sub ${AWS::StackName}-master DBName: "some-name" - Engine: aurora-postgresql + Engine: mysql EngineVersion: "13.2" KmsKeyId: !GetAtt RDSKMSKey.Arn MultiAZ: true diff --git a/tests/test_templates/rules/StorageEncryptedRule/no_db_resource.yml b/tests/test_templates/rules/StorageEncryptedRule/no_db_resource.yml index 081ad730..9ff0384e 100644 --- a/tests/test_templates/rules/StorageEncryptedRule/no_db_resource.yml +++ b/tests/test_templates/rules/StorageEncryptedRule/no_db_resource.yml @@ -7,7 +7,7 @@ Resources: BackupRetentionPeriod: 14 DBClusterIdentifier: !Sub ${AWS::StackName}-master DatabaseName: "some-name" - Engine: aurora-postgresql + Engine: mysql EngineVersion: "13.2" KmsKeyId: !GetAtt RDSKMSKey.Arn StorageEncrypted: false diff --git a/tests/test_templates/rules/StorageEncryptedRule/two_resources_not_encrypted.yml b/tests/test_templates/rules/StorageEncryptedRule/two_resources_not_encrypted.yml index e20266f1..36438d11 100644 --- a/tests/test_templates/rules/StorageEncryptedRule/two_resources_not_encrypted.yml +++ b/tests/test_templates/rules/StorageEncryptedRule/two_resources_not_encrypted.yml @@ -8,7 +8,7 @@ Resources: BackupRetentionPeriod: 14 DBInstanceIdentifier: !Sub ${AWS::StackName}-master DBName: "some-name" - Engine: aurora-postgresql + Engine: mysql EngineVersion: "13.2" KmsKeyId: !GetAtt RDSKMSKey.Arn MultiAZ: true @@ -25,7 +25,7 @@ Resources: BackupRetentionPeriod: 7 DBInstanceIdentifier: !Sub ${AWS::StackName}-backup DBName: "some-name-backup" - Engine: aurora-postgresql + Engine: mysql EngineVersion: "13.2" KmsKeyId: !GetAtt RDSKMSKey.Arn MultiAZ: true From 8285fdde630077bc40a612cd45730f6e5666954e Mon Sep 17 00:00:00 2001 From: max-huneshagen Date: Tue, 14 Nov 2023 10:08:40 +0100 Subject: [PATCH 18/21] make templates valid cloud formations (except for aurora one) --- .../rules/StorageEncryptedRule/aurora_engine_used.yml | 4 +--- .../rules/StorageEncryptedRule/encrypted_db_resource.yml | 2 +- .../StorageEncryptedRule/missing_storage_encrypted_flag.yml | 2 +- .../rules/StorageEncryptedRule/no_db_resource.yml | 2 +- .../StorageEncryptedRule/two_resources_not_encrypted.yml | 4 ++-- 5 files changed, 6 insertions(+), 8 deletions(-) diff --git a/tests/test_templates/rules/StorageEncryptedRule/aurora_engine_used.yml b/tests/test_templates/rules/StorageEncryptedRule/aurora_engine_used.yml index ab74cd33..b7ad4815 100644 --- a/tests/test_templates/rules/StorageEncryptedRule/aurora_engine_used.yml +++ b/tests/test_templates/rules/StorageEncryptedRule/aurora_engine_used.yml @@ -2,15 +2,13 @@ Resources: DBMaster: Type: AWS::RDS::DBInstance Properties: - AllocatedStorage: "100" AllowMajorVersionUpgrade: false AutoMinorVersionUpgrade: true - BackupRetentionPeriod: 14 DBInstanceIdentifier: !Sub ${AWS::StackName}-master DBName: "some-name" Engine: aurora-postgresql EngineVersion: "13.2" - KmsKeyId: !GetAtt RDSKMSKey.Arn + KmsKeyId: "some-kms-key" MultiAZ: true StorageEncrypted: false Tags: diff --git a/tests/test_templates/rules/StorageEncryptedRule/encrypted_db_resource.yml b/tests/test_templates/rules/StorageEncryptedRule/encrypted_db_resource.yml index c89b7b63..8efac523 100644 --- a/tests/test_templates/rules/StorageEncryptedRule/encrypted_db_resource.yml +++ b/tests/test_templates/rules/StorageEncryptedRule/encrypted_db_resource.yml @@ -10,7 +10,7 @@ Resources: DBName: "some-name" Engine: mysql EngineVersion: "13.2" - KmsKeyId: !GetAtt RDSKMSKey.Arn + KmsKeyId: "some-kms-key" MultiAZ: true StorageEncrypted: true Tags: diff --git a/tests/test_templates/rules/StorageEncryptedRule/missing_storage_encrypted_flag.yml b/tests/test_templates/rules/StorageEncryptedRule/missing_storage_encrypted_flag.yml index 2cb78a3f..850e1c09 100644 --- a/tests/test_templates/rules/StorageEncryptedRule/missing_storage_encrypted_flag.yml +++ b/tests/test_templates/rules/StorageEncryptedRule/missing_storage_encrypted_flag.yml @@ -10,7 +10,7 @@ Resources: DBName: "some-name" Engine: mysql EngineVersion: "13.2" - KmsKeyId: !GetAtt RDSKMSKey.Arn + KmsKeyId: "some-kms-key" MultiAZ: true Tags: - Key: Name diff --git a/tests/test_templates/rules/StorageEncryptedRule/no_db_resource.yml b/tests/test_templates/rules/StorageEncryptedRule/no_db_resource.yml index 9ff0384e..2055a06e 100644 --- a/tests/test_templates/rules/StorageEncryptedRule/no_db_resource.yml +++ b/tests/test_templates/rules/StorageEncryptedRule/no_db_resource.yml @@ -9,7 +9,7 @@ Resources: DatabaseName: "some-name" Engine: mysql EngineVersion: "13.2" - KmsKeyId: !GetAtt RDSKMSKey.Arn + KmsKeyId: "some-kms-key" StorageEncrypted: false Tags: - Key: Name diff --git a/tests/test_templates/rules/StorageEncryptedRule/two_resources_not_encrypted.yml b/tests/test_templates/rules/StorageEncryptedRule/two_resources_not_encrypted.yml index 36438d11..a9b4eea1 100644 --- a/tests/test_templates/rules/StorageEncryptedRule/two_resources_not_encrypted.yml +++ b/tests/test_templates/rules/StorageEncryptedRule/two_resources_not_encrypted.yml @@ -10,7 +10,7 @@ Resources: DBName: "some-name" Engine: mysql EngineVersion: "13.2" - KmsKeyId: !GetAtt RDSKMSKey.Arn + KmsKeyId: "some-kms-key" MultiAZ: true StorageEncrypted: false Tags: @@ -27,7 +27,7 @@ Resources: DBName: "some-name-backup" Engine: mysql EngineVersion: "13.2" - KmsKeyId: !GetAtt RDSKMSKey.Arn + KmsKeyId: "some-kms-key" MultiAZ: true StorageEncrypted: false Tags: From 77cc202a85fed00a32155cc7056bd393c40ac8f0 Mon Sep 17 00:00:00 2001 From: max-huneshagen Date: Tue, 14 Nov 2023 10:08:40 +0100 Subject: [PATCH 19/21] make templates valid cloud formations --- .../rules/StorageEncryptedRule/aurora_engine_used.yml | 5 +---- .../rules/StorageEncryptedRule/encrypted_db_resource.yml | 2 +- .../StorageEncryptedRule/missing_storage_encrypted_flag.yml | 2 +- .../rules/StorageEncryptedRule/no_db_resource.yml | 2 +- .../StorageEncryptedRule/two_resources_not_encrypted.yml | 4 ++-- 5 files changed, 6 insertions(+), 9 deletions(-) diff --git a/tests/test_templates/rules/StorageEncryptedRule/aurora_engine_used.yml b/tests/test_templates/rules/StorageEncryptedRule/aurora_engine_used.yml index ab74cd33..cab515cb 100644 --- a/tests/test_templates/rules/StorageEncryptedRule/aurora_engine_used.yml +++ b/tests/test_templates/rules/StorageEncryptedRule/aurora_engine_used.yml @@ -2,17 +2,14 @@ Resources: DBMaster: Type: AWS::RDS::DBInstance Properties: - AllocatedStorage: "100" AllowMajorVersionUpgrade: false AutoMinorVersionUpgrade: true - BackupRetentionPeriod: 14 DBInstanceIdentifier: !Sub ${AWS::StackName}-master DBName: "some-name" Engine: aurora-postgresql EngineVersion: "13.2" - KmsKeyId: !GetAtt RDSKMSKey.Arn + KmsKeyId: "some-kms-key" MultiAZ: true - StorageEncrypted: false Tags: - Key: Name Value: !Sub ${AWS::StackName}-master \ No newline at end of file diff --git a/tests/test_templates/rules/StorageEncryptedRule/encrypted_db_resource.yml b/tests/test_templates/rules/StorageEncryptedRule/encrypted_db_resource.yml index c89b7b63..8efac523 100644 --- a/tests/test_templates/rules/StorageEncryptedRule/encrypted_db_resource.yml +++ b/tests/test_templates/rules/StorageEncryptedRule/encrypted_db_resource.yml @@ -10,7 +10,7 @@ Resources: DBName: "some-name" Engine: mysql EngineVersion: "13.2" - KmsKeyId: !GetAtt RDSKMSKey.Arn + KmsKeyId: "some-kms-key" MultiAZ: true StorageEncrypted: true Tags: diff --git a/tests/test_templates/rules/StorageEncryptedRule/missing_storage_encrypted_flag.yml b/tests/test_templates/rules/StorageEncryptedRule/missing_storage_encrypted_flag.yml index 2cb78a3f..850e1c09 100644 --- a/tests/test_templates/rules/StorageEncryptedRule/missing_storage_encrypted_flag.yml +++ b/tests/test_templates/rules/StorageEncryptedRule/missing_storage_encrypted_flag.yml @@ -10,7 +10,7 @@ Resources: DBName: "some-name" Engine: mysql EngineVersion: "13.2" - KmsKeyId: !GetAtt RDSKMSKey.Arn + KmsKeyId: "some-kms-key" MultiAZ: true Tags: - Key: Name diff --git a/tests/test_templates/rules/StorageEncryptedRule/no_db_resource.yml b/tests/test_templates/rules/StorageEncryptedRule/no_db_resource.yml index 9ff0384e..2055a06e 100644 --- a/tests/test_templates/rules/StorageEncryptedRule/no_db_resource.yml +++ b/tests/test_templates/rules/StorageEncryptedRule/no_db_resource.yml @@ -9,7 +9,7 @@ Resources: DatabaseName: "some-name" Engine: mysql EngineVersion: "13.2" - KmsKeyId: !GetAtt RDSKMSKey.Arn + KmsKeyId: "some-kms-key" StorageEncrypted: false Tags: - Key: Name diff --git a/tests/test_templates/rules/StorageEncryptedRule/two_resources_not_encrypted.yml b/tests/test_templates/rules/StorageEncryptedRule/two_resources_not_encrypted.yml index 36438d11..a9b4eea1 100644 --- a/tests/test_templates/rules/StorageEncryptedRule/two_resources_not_encrypted.yml +++ b/tests/test_templates/rules/StorageEncryptedRule/two_resources_not_encrypted.yml @@ -10,7 +10,7 @@ Resources: DBName: "some-name" Engine: mysql EngineVersion: "13.2" - KmsKeyId: !GetAtt RDSKMSKey.Arn + KmsKeyId: "some-kms-key" MultiAZ: true StorageEncrypted: false Tags: @@ -27,7 +27,7 @@ Resources: DBName: "some-name-backup" Engine: mysql EngineVersion: "13.2" - KmsKeyId: !GetAtt RDSKMSKey.Arn + KmsKeyId: "some-kms-key" MultiAZ: true StorageEncrypted: false Tags: From 51ae539e24aa30b0aab13705ab3bae7674308fe4 Mon Sep 17 00:00:00 2001 From: Max-Huneshagen <145039806+Max-Huneshagen@users.noreply.github.com> Date: Tue, 14 Nov 2023 10:19:54 +0100 Subject: [PATCH 20/21] Update tests/rules/test_StorageEncryptedRule.py Co-authored-by: Ignacio Bolonio --- tests/rules/test_StorageEncryptedRule.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/rules/test_StorageEncryptedRule.py b/tests/rules/test_StorageEncryptedRule.py index 0c7ce676..2e79ede1 100644 --- a/tests/rules/test_StorageEncryptedRule.py +++ b/tests/rules/test_StorageEncryptedRule.py @@ -16,7 +16,7 @@ def test_storage_encrypted_rule_valid_results(): assert result.failures == [] -def test_rule_not_invoked_for_aurora(): +def test_rule_not_failing_for_aurora(): rule = StorageEncryptedRule(None) model = get_cfmodel_from("rules/StorageEncryptedRule/aurora_engine_used.yml") resolved_model = model.resolve() From bfd22dd72bed972295f0b23e89d2ef41cb8cb35c Mon Sep 17 00:00:00 2001 From: max-huneshagen Date: Wed, 15 Nov 2023 17:29:30 +0100 Subject: [PATCH 21/21] add aurora comment --- cfripper/rules/storage_encrypted_rule.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/cfripper/rules/storage_encrypted_rule.py b/cfripper/rules/storage_encrypted_rule.py index ff6e3d1c..6864f191 100644 --- a/cfripper/rules/storage_encrypted_rule.py +++ b/cfripper/rules/storage_encrypted_rule.py @@ -25,7 +25,9 @@ def invoke(self, cfmodel: CFModel, extras: Optional[Dict] = None) -> Result: if ( resource.Type == "AWS::RDS::DBInstance" and not is_encrypted - and not getattr(resource.Properties, "Engine", "").startswith("aurora") + and not getattr(resource.Properties, "Engine", "").startswith( + "aurora" + ) # not applicable for aurora since the encryption for DB instances is managed by the DB cluster ): self.add_failure_to_result(