diff --git a/CHANGELOG.md b/CHANGELOG.md index 4259e644..7877480e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,12 @@ # Changelog All notable changes to this project will be documented in this file. +## [0.11.1] - 2019-11-25 +### Changed +- `HardcodedRDSPasswordRule` now reports two different messages when there is a missing echo or a readable password. +### Fixes +- `HardcodedRDSPasswordRule` was wrongly adding an error when a value is provided. + ## [0.11.0] - 2019-11-20 ### Breaking changes - Moved some files from model to rules, renamed rules to match pythonic style. Moved tons of classes around diff --git a/cfripper/rules/hardcoded_RDS_password.py b/cfripper/rules/hardcoded_RDS_password.py index 8c9b8d82..4dd5def5 100644 --- a/cfripper/rules/hardcoded_RDS_password.py +++ b/cfripper/rules/hardcoded_RDS_password.py @@ -19,7 +19,8 @@ class HardcodedRDSPasswordRule(Rule): - REASON = "Default RDS {} password parameter or missing NoEcho for {}." + REASON_DEFAULT = "Default RDS {} password parameter (readable in plain-text) for {}." + REASON_MISSING_NOECHO = "RDS {} password parameter missing NoEcho for {}." def invoke(self, cfmodel): password_protected_cluster_ids = [] @@ -27,21 +28,13 @@ def invoke(self, cfmodel): for logical_id, resource in cfmodel.Resources.items(): # flag insecure RDS Clusters. - if ( - resource.Type == "AWS::RDS::DBCluster" - and resource.Properties.get("MasterUserPassword", Parameter.NO_ECHO_NO_DEFAULT) - != Parameter.NO_ECHO_NO_DEFAULT - ): - self.add_failure(type(self).__name__, self.REASON.format("Cluster", logical_id)) - continue - - # keep track of secure RDS Clusters. if resource.Type == "AWS::RDS::DBCluster": - password_protected_cluster_ids.append(logical_id) - continue + failure_added = self._failure_added(logical_id, resource) + if not failure_added: + password_protected_cluster_ids.append(logical_id) # keep track of RDS instances so they can be examined in the code below. - if resource.Type == "AWS::RDS::DBInstance": + elif resource.Type == "AWS::RDS::DBInstance": instances_to_check.append((logical_id, resource)) # check each instance with the context of clusters. @@ -52,8 +45,16 @@ def invoke(self, cfmodel): ): continue - if ( - resource.Properties.get("MasterUserPassword", Parameter.NO_ECHO_NO_DEFAULT) - != Parameter.NO_ECHO_NO_DEFAULT - ): - self.add_failure(type(self).__name__, self.REASON.format("Instance", logical_id)) + self._failure_added(logical_id, resource) + + def _failure_added(self, logical_id, resource) -> bool: + master_user_password = resource.Properties.get("MasterUserPassword", Parameter.NO_ECHO_NO_DEFAULT) + resource_type = resource.Type.replace("AWS::RDS::DB", "") + if master_user_password == Parameter.NO_ECHO_WITH_DEFAULT: + self.add_failure(type(self).__name__, self.REASON_DEFAULT.format(resource_type, logical_id)) + return True + elif master_user_password not in (Parameter.NO_ECHO_NO_DEFAULT, Parameter.NO_ECHO_WITH_VALUE): + self.add_failure(type(self).__name__, self.REASON_MISSING_NOECHO.format(resource_type, logical_id)) + return True + + return False diff --git a/setup.py b/setup.py index e68026db..89a3f9cf 100644 --- a/setup.py +++ b/setup.py @@ -3,7 +3,7 @@ with open("README.md", "r") as fh: long_description = fh.read() -install_requires = ["boto3>=1.4.7,<2", "PyYAML>=4.2b1", "pycfmodel>=0.5.0", "cfn_flip>=1.2.0"] +install_requires = ["boto3>=1.4.7,<2", "PyYAML>=4.2b1", "pycfmodel>=0.5.1", "cfn_flip>=1.2.0"] dev_requires = [ "black==19.10b0", @@ -17,7 +17,7 @@ setup( name="cfripper", - version="0.11.0", + version="0.11.1", author="Skyscanner Product Security", author_email="security@skyscanner.net", long_description=long_description, diff --git a/tests/rules/test_HardcodedRDSPasswordRule.py b/tests/rules/test_HardcodedRDSPasswordRule.py index 27e7ce88..0e31fe1d 100644 --- a/tests/rules/test_HardcodedRDSPasswordRule.py +++ b/tests/rules/test_HardcodedRDSPasswordRule.py @@ -53,9 +53,11 @@ def test_failures_are_raised_for_instances(bad_template_instances): assert len(result.failed_rules) == 2 assert len(result.failed_monitored_rules) == 0 assert result.failed_rules[0].rule == "HardcodedRDSPasswordRule" - assert result.failed_rules[0].reason == "Default RDS Instance password parameter or missing NoEcho for BadDb3." + assert result.failed_rules[0].reason == "RDS Instance password parameter missing NoEcho for BadDb3." assert result.failed_rules[1].rule == "HardcodedRDSPasswordRule" - assert result.failed_rules[1].reason == "Default RDS Instance password parameter or missing NoEcho for BadDb5." + assert ( + result.failed_rules[1].reason == "Default RDS Instance password parameter (readable in plain-text) for BadDb5." + ) def test_failures_are_raised_for_clusters(bad_template_clusters): @@ -67,7 +69,7 @@ def test_failures_are_raised_for_clusters(bad_template_clusters): assert len(result.failed_rules) == 1 assert len(result.failed_monitored_rules) == 0 assert result.failed_rules[0].rule == "HardcodedRDSPasswordRule" - assert result.failed_rules[0].reason == "Default RDS Cluster password parameter or missing NoEcho for BadCluster1." + assert result.failed_rules[0].reason == "RDS Cluster password parameter missing NoEcho for BadCluster1." def test_passed_cluster_pw_protected(good_template_clusters_and_instances): @@ -87,7 +89,9 @@ def test_failures_are_raised_for_instances_without_protected_clusters(bad_templa assert len(result.failed_rules) == 1 assert len(result.failed_monitored_rules) == 0 assert result.failed_rules[0].rule == "HardcodedRDSPasswordRule" - assert result.failed_rules[0].reason == "Default RDS Instance password parameter or missing NoEcho for BadDb5." + assert ( + result.failed_rules[0].reason == "Default RDS Instance password parameter (readable in plain-text) for BadDb5." + ) def test_failures_are_raised_for_bad_instances_and_bad_clusters(bad_template_clusters_with_bad_instances): @@ -99,6 +103,9 @@ def test_failures_are_raised_for_bad_instances_and_bad_clusters(bad_template_clu assert len(result.failed_rules) == 2 assert len(result.failed_monitored_rules) == 0 assert result.failed_rules[0].rule == "HardcodedRDSPasswordRule" - assert result.failed_rules[0].reason == "Default RDS Cluster password parameter or missing NoEcho for BadCluster99." + assert ( + result.failed_rules[0].reason + == "Default RDS Cluster password parameter (readable in plain-text) for BadCluster99." + ) assert result.failed_rules[1].rule == "HardcodedRDSPasswordRule" - assert result.failed_rules[1].reason == "Default RDS Instance password parameter or missing NoEcho for BadDb33." + assert result.failed_rules[1].reason == "RDS Instance password parameter missing NoEcho for BadDb33."