diff --git a/src/cfnlint/context/context.py b/src/cfnlint/context/context.py index 8cd9332e94..e3bd457c5b 100644 --- a/src/cfnlint/context/context.py +++ b/src/cfnlint/context/context.py @@ -10,7 +10,7 @@ from dataclasses import InitVar, dataclass, field, fields from typing import Any, Deque, Dict, Iterator, List, Mapping, Sequence, Set, Tuple -from cfnlint.helpers import PSEUDOPARAMS, REGION_PRIMARY +from cfnlint.helpers import BOOLEAN_STRINGS_TRUE, PSEUDOPARAMS, REGION_PRIMARY from cfnlint.schema import PROVIDER_SCHEMA_MANAGER, AttributeDict _PSEUDOPARAMS_NON_REGION = ["AWS::AccountId", "AWS::NoValue", "AWS::StackName"] @@ -234,6 +234,7 @@ def __post_init__(self, parameter) -> None: self.allowed_values = [] self.min_value = None self.max_value = None + self.no_echo = False t = parameter.get("Type") if not isinstance(t, str): @@ -255,8 +256,11 @@ def __post_init__(self, parameter) -> None: else: self.default = parameter.get("Default") self.allowed_values = parameter.get("AllowedValues") - self.min_value = parameter.get("MinValue") - self.max_value = parameter.get("MaxValue") + + self.min_value = parameter.get("MinValue") + self.max_value = parameter.get("MaxValue") + if parameter.get("NoEcho") in list(BOOLEAN_STRINGS_TRUE) + [True]: + self.no_echo = True def ref(self, context: Context) -> Iterator[Tuple[Any, deque]]: if self.allowed_values: diff --git a/src/cfnlint/data/schemas/other/resources/configuration.json b/src/cfnlint/data/schemas/other/resources/configuration.json index 57f9571df8..9b3d55e9a3 100644 --- a/src/cfnlint/data/schemas/other/resources/configuration.json +++ b/src/cfnlint/data/schemas/other/resources/configuration.json @@ -34,15 +34,6 @@ }, "type": "object" }, - "Metadata": { - "additionalProperties": true, - "properties": { - "AWS::CloudFormation::Init": { - "$ref": "#/definitions/CloudFormationInitConfig" - } - }, - "type": "object" - }, "ResourceConfiguration": { "additionalProperties": false, "allOf": [ @@ -99,7 +90,7 @@ ] }, "Metadata": { - "$ref": "#/definitions/Metadata" + "awsType": "CfnResourceMetadata" }, "Properties": {}, "Type": { diff --git a/src/cfnlint/data/schemas/other/resources/metadata.json b/src/cfnlint/data/schemas/other/resources/metadata.json new file mode 100644 index 0000000000..5e2620129c --- /dev/null +++ b/src/cfnlint/data/schemas/other/resources/metadata.json @@ -0,0 +1,47 @@ +{ + "allOf": [ + { + "type": [ + "object", + "array" + ] + }, + { + "additionalProperties": true, + "properties": { + "AWS::CloudFormation::Init": { + "properties": { + "config": { + "properties": { + "commands": { + "awsType": "CfnInitCommands" + }, + "files": { + "awsType": "CfnInitFiles" + }, + "groups": { + "awsType": "CfnInitGroups" + }, + "packages": { + "awsType": "CfnInitPackages" + }, + "services": { + "awsType": "CfnInitServices" + }, + "sources": { + "awsType": "CfnInitSources" + }, + "users": { + "awsType": "CfnInitUsers" + } + }, + "type": "object" + } + }, + "type": "object" + } + }, + "type": "object" + } + ] +} diff --git a/src/cfnlint/data/schemas/providers/us_east_1/aws-directoryservice-microsoftad.json b/src/cfnlint/data/schemas/providers/us_east_1/aws-directoryservice-microsoftad.json index f18dc76536..78101ac1d4 100644 --- a/src/cfnlint/data/schemas/providers/us_east_1/aws-directoryservice-microsoftad.json +++ b/src/cfnlint/data/schemas/providers/us_east_1/aws-directoryservice-microsoftad.json @@ -65,6 +65,9 @@ "type": "string" }, "Password": { + "awsType": [ + "CfnDynamicReferenceSecret" + ], "type": "string" }, "ShortName": { diff --git a/src/cfnlint/helpers.py b/src/cfnlint/helpers.py index 508ac2a8cb..ba1188ea66 100644 --- a/src/cfnlint/helpers.py +++ b/src/cfnlint/helpers.py @@ -280,6 +280,10 @@ VALID_PARAMETER_TYPES = VALID_PARAMETER_TYPES_SINGLE + VALID_PARAMETER_TYPES_LIST +BOOLEAN_STRINGS_TRUE = frozenset(["true", "True"]) +BOOLEAN_STRINGS_FALSE = frozenset(["false", "False"]) +BOOLEAN_STRINGS = frozenset(list(BOOLEAN_STRINGS_TRUE) + list(BOOLEAN_STRINGS_FALSE)) + # pylint: disable=missing-class-docstring class RegexDict(dict): diff --git a/src/cfnlint/rules/functions/GetAtt.py b/src/cfnlint/rules/functions/GetAtt.py index c4807111ed..a56947b518 100644 --- a/src/cfnlint/rules/functions/GetAtt.py +++ b/src/cfnlint/rules/functions/GetAtt.py @@ -126,8 +126,8 @@ def iter_errors(type: str) -> ValidationResult: err.message + f" when {value[1]!r} is resolved" ) yield err - break - else: + continue + # because of the complexities of schemas ($ref, anyOf, allOf, etc.) # we will simplify the validator to just have a type check # then we will provide a simple value to represent the type from the diff --git a/src/cfnlint/rules/functions/Ref.py b/src/cfnlint/rules/functions/Ref.py index 1eaab0c833..bf375b24a3 100644 --- a/src/cfnlint/rules/functions/Ref.py +++ b/src/cfnlint/rules/functions/Ref.py @@ -31,9 +31,22 @@ def __init__(self) -> None: "Resources/AWS::EC2::LaunchTemplate/Properties/LaunchTemplateData/ImageId": "W2506", # noqa: E501 "Resources/AWS::EC2::SpotFleet/Properties/SpotFleetRequestConfigData/LaunchSpecifications/ImageId": "W2506", # noqa: E501 "Resources/AWS::ImageBuilder::Image/Properties/ImageId": "W2506", # noqa: E501 - "Resources/AWS::NimbleStudio::StreamingImage/Properties/Ec2ImageId": "W2506", # noqa: E501 + "Resources/AWS::DirectoryService::MicrosoftAD/Properties/Password": "W1011", # noqa: E501 + "Resources/AWS::DirectoryService::SimpleAD/Properties/Password": "W1011", # noqa: E501 + "Resources/AWS::ElastiCache::ReplicationGroup/Properties/AuthToken": "W1011", # noqa: E501 + "Resources/AWS::IAM::User/Properties/LoginProfile/Password": "W1011", # noqa: E501 + "Resources/AWS::KinesisFirehose::DeliveryStream/Properties/RedshiftDestinationConfiguration/Password": "W1011", # noqa: E501 + "Resources/AWS::OpsWorks::App/Properties/AppSource/Password": "W1011", # noqa: E501 + "Resources/AWS::OpsWorks::Stack/Properties/RdsDbInstances/DbPassword": "W1011", # noqa: E501 + "Resources/AWS::OpsWorks::Stack/Properties/CustomCookbooksSource/Password": "W1011", # noqa: E501 + "Resources/AWS::RDS::DBCluster/Properties/MasterUserPassword": "W1011", # noqa: E501 + "Resources/AWS::RDS::DBInstance/Properties/MasterUserPassword": "W1011", # noqa: E501 + "Resources/AWS::Redshift::Cluster/Properties/MasterUserPassword": "W1011", # noqa: E501 } - self.child_rules = dict.fromkeys(list(self.keywords.values())) + self._all_refs = [ + "W2010", + ] + self.child_rules = dict.fromkeys(list(self.keywords.values()) + self._all_refs) def schema(self, validator, instance) -> Dict[str, Any]: return { @@ -89,13 +102,14 @@ def ref(self, validator, subschema, instance, schema): yield ValidationError(f"{instance!r} is not of type {reprs}") return - keyword = self.get_keyword(validator) - for rule in self.child_rules.values(): - if rule is None: - continue - if not rule.id: - continue + for rule_id in self._all_refs: + rule = self.child_rules.get(rule_id) + if rule: + yield from rule.validate(validator, {}, instance, schema) - for rule_keyword in self.keywords: - if rule_keyword == keyword: - yield from rule.validate(validator, keyword, instance, schema) + keyword = self.get_keyword(validator) + rule_id = self.keywords.get(keyword) + if rule_id: + rule = self.child_rules.get(rule_id) + if rule: + yield from rule.validate(validator, keyword, instance, schema) diff --git a/src/cfnlint/rules/functions/Sub.py b/src/cfnlint/rules/functions/Sub.py index b6a0cf4d3d..cacc680121 100644 --- a/src/cfnlint/rules/functions/Sub.py +++ b/src/cfnlint/rules/functions/Sub.py @@ -8,10 +8,8 @@ import regex as re -from cfnlint.context.context import Parameter from cfnlint.helpers import REGEX_SUB_PARAMETERS from cfnlint.jsonschema import ValidationError, ValidationResult, Validator -from cfnlint.jsonschema._utils import ensure_list from cfnlint.rules.functions._BaseFn import BaseFn @@ -31,6 +29,19 @@ def __init__(self) -> None: "W1019": None, "W1020": None, } + self._functions = [ + "Fn::Base64", + "Fn::FindInMap", + "Fn::GetAZs", + "Fn::GetAtt", + "Fn::If", + "Fn::ImportValue", + "Fn::Join", + "Fn::Select", + "Fn::Sub", + "Ref", + "Fn::ToJsonString", + ] def schema(self, validator: Validator, instance: Any) -> Dict[str, Any]: return { @@ -42,19 +53,7 @@ def schema(self, validator: Validator, instance: Any) -> Dict[str, Any]: "schema": {"type": "string"}, }, { - "functions": [ - "Fn::Base64", - "Fn::FindInMap", - "Fn::GetAZs", - "Fn::GetAtt", - "Fn::If", - "Fn::ImportValue", - "Fn::Join", - "Fn::Select", - "Fn::Sub", - "Ref", - "Fn::ToJsonString", - ], + "functions": self._functions, "schema": { "type": ["object"], "patternProperties": { @@ -68,45 +67,36 @@ def schema(self, validator: Validator, instance: Any) -> Dict[str, Any]: ], } + def _clean_error( + self, err: ValidationError, instance: Any, param: Any + ) -> ValidationError: + err.message = err.message.replace(f"{instance!r}", f"{param!r}") + err.instance = param + err.path = deque([self.fn.name]) + err.schema_path = deque([]) + err.validator = self.fn.py + return err + def _validate_string(self, validator: Validator, instance: Any) -> ValidationResult: params = re.findall(REGEX_SUB_PARAMETERS, instance) + validator = validator.evolve( + context=validator.context.evolve( + functions=self._functions, + ) + ) for param in params: param = param.strip() - valid_params = [] if "." in param: - [name, attr] = param.split(".", 1) - if name in validator.context.resources: - if attr in validator.context.resources[name].get_atts: - tS = ensure_list( - validator.context.resources[name].get_atts[attr].type - ) - if not any(type in tS for type in self.sub_parameter_types): - reprs = ", ".join( - repr(type) for type in self.sub_parameter_types - ) - yield ValidationError( - message=f"{param!r} is not of type {reprs}", - validator=self.fn.py, - path=deque([self.fn.name]), - ) - continue - valid_params = [ - f"{name}.{attr}" - for attr in list( - validator.context.resources[name].get_atts.keys() - ) - ] - else: - valid_params = list(validator.context.resources.keys()) + for err in validator.descend( + {"Fn::GetAtt": param}, + {"type": ["string", "integer", "number", "boolean"]}, + ): + yield self._clean_error(err, {"Fn::GetAtt": param}, param) else: - valid_params = validator.context.refs - - if param not in valid_params: - yield ValidationError( - message=f"{param!r} is not one of {valid_params!r}", - validator=self.fn.py, - path=deque([self.fn.name]), - ) + for err in validator.descend( + {"Ref": param}, {"type": ["string", "integer", "number", "boolean"]} + ): + yield self._clean_error(err, {"Ref": param}, param) def fn_sub( self, validator: Validator, s: Any, instance: Any, schema: Any @@ -121,11 +111,7 @@ def fn_sub( return validator_string = validator.evolve( - context=validator.context.evolve( - ref_values=dict.fromkeys( - list(value[1].keys()), Parameter({"Type": "String"}) - ), - ) + context=validator.context.evolve(ref_values=value[1]) ) value = value[0] elif validator.is_type(value, "string"): diff --git a/src/cfnlint/rules/jsonschema/AwsType.py b/src/cfnlint/rules/jsonschema/AwsType.py index 07afcffd90..c07e844e6d 100644 --- a/src/cfnlint/rules/jsonschema/AwsType.py +++ b/src/cfnlint/rules/jsonschema/AwsType.py @@ -21,6 +21,7 @@ def __init__(self) -> None: self.types = { "CfnConditions": "E8001", "CfnCondition": "E8002", + "CfnDynamicReferenceSecret": "E1051", "CfnInitCommand": "E3009", "CfnInitFiles": "E3009", "CfnInitGroups": "E3009", @@ -35,6 +36,7 @@ def __init__(self) -> None: "CfnParameters": "E2001", "CfnResources": "E3001", "CfnResourceDeletionPolicy": "E3035", + "CfnResourceMetadata": "E3028", "CfnResourceType": "E3011", "CfnResourceProperties": "E3002", "CfnResourceUpdatePolicy": "E3016", diff --git a/src/cfnlint/rules/parameters/DynamicReferenceSecret.py b/src/cfnlint/rules/parameters/DynamicReferenceSecret.py new file mode 100644 index 0000000000..11a5622b46 --- /dev/null +++ b/src/cfnlint/rules/parameters/DynamicReferenceSecret.py @@ -0,0 +1,37 @@ +""" +Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +SPDX-License-Identifier: MIT-0 +""" + +from typing import Any + +from cfnlint.jsonschema import ValidationError, Validator +from cfnlint.rules import CloudFormationLintRule + + +class DynamicReferenceSecret(CloudFormationLintRule): + """ + Check if Dynamic Reference Secure Strings are + only used in the correct locations + """ + + id = "W1011" + shortdesc = "Instead of REFing a parameter for a secret use a dynamic reference" + description = ( + "Instead of REFing a parameter for a secret use a dynamic reference. " + "Solutions like SSM parameter store and secrets manager provide " + "better security of sercrets" + ) + source_url = "https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/security-best-practices.html#creds" + tags = ["functions", "dynamic reference", "ref"] + + def validate(self, validator: Validator, _, instance: Any, schema: Any): + value = instance.get("Ref") + + if not validator.is_type(value, "string"): + return + + if value in validator.context.parameters: + yield ValidationError( + "Use dynamic references over parameters for secrets", rule=self + ) diff --git a/src/cfnlint/rules/parameters/NoEcho.py b/src/cfnlint/rules/parameters/NoEcho.py new file mode 100644 index 0000000000..51ea64dacf --- /dev/null +++ b/src/cfnlint/rules/parameters/NoEcho.py @@ -0,0 +1,55 @@ +""" +Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +SPDX-License-Identifier: MIT-0 +""" + +from collections import deque +from typing import Any + +from cfnlint.jsonschema import ValidationError, Validator +from cfnlint.rules import CloudFormationLintRule + + +class NoEcho(CloudFormationLintRule): + id = "W2010" + shortdesc = "NoEcho parameters are not masked when used in Metadata and Outputs" + description = ( + "Using the NoEcho attribute does not mask any information stored " + "in the following: Metadata, Outputs, Resource Metadata" + ) + source_url = "https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/parameters-section-structure.html" + tags = ["functions", "dynamic reference", "ref"] + + def validate(self, validator: Validator, _, instance: Any, schema: Any): + value = instance.get("Ref") + + if not validator.is_type(value, "string"): + return + + parameter = validator.context.parameters.get(value) + if not parameter: + return + + if parameter.no_echo: + if len(validator.context.path) >= 3: + if ( + validator.context.path[0] == "Resources" + and validator.context.path[2] == "Metadata" + ): + yield ValidationError( + f"Don't use 'NoEcho' parameter {value!r} in resource metadata", + rule=self, + path=deque(["Ref"]), + ) + return + if len(validator.context.path) > 0: + if validator.context.path[0] in ["Metadata", "Outputs"]: + yield ValidationError( + ( + f"Don't use 'NoEcho' parameter {value!r} " + f"in {validator.context.path[0]!r}" + ), + rule=self, + path=deque(["Ref"]), + ) + return diff --git a/src/cfnlint/rules/resources/Metadata.py b/src/cfnlint/rules/resources/Metadata.py new file mode 100644 index 0000000000..2dc0fd8510 --- /dev/null +++ b/src/cfnlint/rules/resources/Metadata.py @@ -0,0 +1,46 @@ +""" +Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +SPDX-License-Identifier: MIT-0 +""" + +from typing import Any + +import cfnlint.helpers +from cfnlint.data.schemas.other import resources +from cfnlint.jsonschema import Validator +from cfnlint.rules.jsonschema.Base import BaseJsonSchema + + +class Metadata(BaseJsonSchema): + """Check Base Resource Configuration""" + + id = "E3028" + shortdesc = "Basic CloudFormation Resource Check" + description = ( + "Making sure the basic CloudFormation resources are properly configured" + ) + source_url = "https://github.com/aws-cloudformation/cfn-python-lint" + tags = ["resources"] + + def __init__(self): + super().__init__() + self._schema = cfnlint.helpers.load_resource(resources, "metadata.json") + + @property + def schema(self): + return self._schema + + # pylint: disable=unused-argument + def cfnresourcemetadata(self, validator: Validator, _, instance: Any, schema): + validator = self.extend_validator( + validator, + self.schema, + context=validator.context.evolve( + functions=cfnlint.helpers.FUNCTIONS, + ), + ) + for err in validator.iter_errors(instance): + if err.rule is None: + if not err.validator.startswith("fn") and err.validator != "ref": + err.rule = self + yield err diff --git a/src/cfnlint/rules/resources/NoEcho.py b/src/cfnlint/rules/resources/NoEcho.py deleted file mode 100644 index 1fbd278090..0000000000 --- a/src/cfnlint/rules/resources/NoEcho.py +++ /dev/null @@ -1,83 +0,0 @@ -""" -Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. -SPDX-License-Identifier: MIT-0 -""" - -from cfnlint.helpers import bool_compare -from cfnlint.rules import CloudFormationLintRule, RuleMatch - - -class NoEcho(CloudFormationLintRule): - id = "W4002" - shortdesc = "Check for NoEcho References" - description = ( - "Check if there is a NoEcho enabled parameter referenced within a resources" - " Metadata section" - ) - source_url = "https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/parameters-section-structure.html#parameters-section-structure-properties" - tags = ["resources", "NoEcho"] - - def _get_no_echo_params(self, cfn): - """Get no Echo Params""" - no_echo_params = [] - for parameter_name, parameter_value in cfn.get_parameters().items(): - if isinstance(parameter_value, dict): - noecho = parameter_value.get("NoEcho", default=False) - if bool_compare(noecho, True): - no_echo_params.append(parameter_name) - return no_echo_params - - def _check_ref(self, cfn, no_echo_params): - """Check Refs""" - matches = [] - refs = cfn.search_deep_keys("Ref") - for ref in refs: - if ref[-1] in no_echo_params: - if len(ref) > 3: - if ref[0] == "Resources" and ref[2] == "Metadata": - matches.append( - RuleMatch( - ref, - 'As the resource "metadata" section contains ' - + 'reference to a "NoEcho" parameter ' - + str(ref[-1]) - + ", CloudFormation will display the parameter" - " value in " + "plaintext", - ) - ) - - return matches - - def _check_sub(self, cfn, no_echo_params): - """Check Subs""" - matches = [] - subs = cfn.search_deep_keys("Fn::Sub") - for sub in subs: - if isinstance(sub[-1], str): - params = cfn.get_sub_parameters(sub[-1]) - for param in params: - if param in no_echo_params: - if len(sub) > 2: - if sub[0] == "Resources" and sub[2] == "Metadata": - matches.append( - RuleMatch( - sub[:-1], - 'As the resource "metadata" section contains ' - + 'reference to a "NoEcho" parameter ' - + str(param) - + ", CloudFormation will display the parameter" - " value in " + "plaintext", - ) - ) - - return matches - - def match(self, cfn): - matches = [] - no_echo_params = self._get_no_echo_params(cfn) - if not no_echo_params: - return matches - matches.extend(self._check_ref(cfn, no_echo_params)) - matches.extend(self._check_sub(cfn, no_echo_params)) - - return matches diff --git a/test/fixtures/results/quickstart/nist_application.json b/test/fixtures/results/quickstart/nist_application.json index f9266a585a..b99c787266 100644 --- a/test/fixtures/results/quickstart/nist_application.json +++ b/test/fixtures/results/quickstart/nist_application.json @@ -368,20 +368,19 @@ "Fn::Join", 1, 9, - "Ref", - "pDBPassword" + "Ref" ], "Start": { "ColumnNumber": 21, "LineNumber": 343 } }, - "Message": "As the resource \"metadata\" section contains reference to a \"NoEcho\" parameter pDBPassword, CloudFormation will display the parameter value in plaintext", + "Message": "Don't use 'NoEcho' parameter 'pDBPassword' in resource metadata", "Rule": { - "Description": "Check if there is a NoEcho enabled parameter referenced within a resources Metadata section", - "Id": "W4002", - "ShortDescription": "Check for NoEcho References", - "Source": "https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/parameters-section-structure.html#parameters-section-structure-properties" + "Description": "Using the NoEcho attribute does not mask any information stored in the following: Metadata, Outputs, Resource Metadata", + "Id": "W2010", + "ShortDescription": "NoEcho parameters are not masked when used in Metadata and Outputs", + "Source": "https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/parameters-section-structure.html" } }, { @@ -1603,6 +1602,33 @@ "Source": "https://aws.amazon.com/blogs/devops/optimize-aws-cloudformation-templates/" } }, + { + "Filename": "test/fixtures/templates/quickstart/nist_application.yaml", + "Level": "Warning", + "Location": { + "End": { + "ColumnNumber": 25, + "LineNumber": 1016 + }, + "Path": [ + "Resources", + "rRDSInstanceMySQL", + "Properties", + "MasterUserPassword" + ], + "Start": { + "ColumnNumber": 7, + "LineNumber": 1016 + } + }, + "Message": "Use dynamic references over parameters for secrets", + "Rule": { + "Description": "Instead of REFing a parameter for a secret use a dynamic reference. Solutions like SSM parameter store and secrets manager provide better security of sercrets", + "Id": "W1011", + "ShortDescription": "Instead of REFing a parameter for a secret use a dynamic reference", + "Source": "https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/security-best-practices.html#creds" + } + }, { "Filename": "test/fixtures/templates/quickstart/nist_application.yaml", "Level": "Error", diff --git a/test/fixtures/results/quickstart/non_strict/nist_application.json b/test/fixtures/results/quickstart/non_strict/nist_application.json index 24ecce2234..8ae5e47702 100644 --- a/test/fixtures/results/quickstart/non_strict/nist_application.json +++ b/test/fixtures/results/quickstart/non_strict/nist_application.json @@ -368,20 +368,19 @@ "Fn::Join", 1, 9, - "Ref", - "pDBPassword" + "Ref" ], "Start": { "ColumnNumber": 21, "LineNumber": 343 } }, - "Message": "As the resource \"metadata\" section contains reference to a \"NoEcho\" parameter pDBPassword, CloudFormation will display the parameter value in plaintext", + "Message": "Don't use 'NoEcho' parameter 'pDBPassword' in resource metadata", "Rule": { - "Description": "Check if there is a NoEcho enabled parameter referenced within a resources Metadata section", - "Id": "W4002", - "ShortDescription": "Check for NoEcho References", - "Source": "https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/parameters-section-structure.html#parameters-section-structure-properties" + "Description": "Using the NoEcho attribute does not mask any information stored in the following: Metadata, Outputs, Resource Metadata", + "Id": "W2010", + "ShortDescription": "NoEcho parameters are not masked when used in Metadata and Outputs", + "Source": "https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/parameters-section-structure.html" } }, { @@ -862,6 +861,33 @@ "Source": "https://aws.amazon.com/blogs/devops/optimize-aws-cloudformation-templates/" } }, + { + "Filename": "test/fixtures/templates/quickstart/nist_application.yaml", + "Level": "Warning", + "Location": { + "End": { + "ColumnNumber": 25, + "LineNumber": 1016 + }, + "Path": [ + "Resources", + "rRDSInstanceMySQL", + "Properties", + "MasterUserPassword" + ], + "Start": { + "ColumnNumber": 7, + "LineNumber": 1016 + } + }, + "Message": "Use dynamic references over parameters for secrets", + "Rule": { + "Description": "Instead of REFing a parameter for a secret use a dynamic reference. Solutions like SSM parameter store and secrets manager provide better security of sercrets", + "Id": "W1011", + "ShortDescription": "Instead of REFing a parameter for a secret use a dynamic reference", + "Source": "https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/security-best-practices.html#creds" + } + }, { "Filename": "test/fixtures/templates/quickstart/nist_application.yaml", "Level": "Warning", diff --git a/test/unit/module/test_api.py b/test/unit/module/test_api.py index ed57366c75..052ca0b487 100644 --- a/test/unit/module/test_api.py +++ b/test/unit/module/test_api.py @@ -31,7 +31,7 @@ def test_noecho_yaml_template(self): config=ManualArgs(regions=["us-east-1", "us-west-2", "eu-west-1"]), ) self.assertEqual( - ["W4002", "W4002"], + ["W2010", "W2010"], [match.rule.id for match in matches], f"Got matches: {matches!r}", ) @@ -121,7 +121,7 @@ def helper_lint_string_from_file( def test_noecho_yaml_template(self): filename = "test/fixtures/templates/bad/noecho.yaml" matches = self.helper_lint_string_from_file(filename) - self.assertEqual(["W4002", "W4002"], [match.rule.id for match in matches]) + self.assertEqual(["W2010", "W2010"], [match.rule.id for match in matches]) def test_noecho_yaml_template_warnings_ignored(self): filename = "test/fixtures/templates/bad/noecho.yaml" diff --git a/test/unit/rules/functions/test_sub.py b/test/unit/rules/functions/test_sub.py index 7d7414f393..81fbeb5cce 100644 --- a/test/unit/rules/functions/test_sub.py +++ b/test/unit/rules/functions/test_sub.py @@ -9,6 +9,8 @@ from cfnlint.context import create_context_for_template from cfnlint.jsonschema import CfnTemplateValidator, ValidationError +from cfnlint.rules.functions.GetAtt import GetAtt +from cfnlint.rules.functions.Ref import Ref from cfnlint.rules.functions.Sub import Sub from cfnlint.template import Template @@ -86,7 +88,7 @@ def context(cfn): [], ), ( - "Valid Fn::Sub with a valid Ref and a string escape", + "Valid Fn::Sub with a valid Ref and a string escape with parameter", {"Fn::Sub": "${!foo}=${bar}"}, {"type": "string"}, [ @@ -124,7 +126,7 @@ def context(cfn): ], ), ( - "Valid Fn::Sub with a Ref to parameter", + "Valid Fn::Sub with a Ref to a sub parameter", {"Fn::Sub": ["${foo}", {"foo": "bar"}]}, {"type": "string"}, [], @@ -181,6 +183,15 @@ def context(cfn): ), validator="fn_sub", ), + ValidationError( + ( + "'foo' is not of type 'string', 'integer', " + "'number', 'boolean' when 'Ref' is resolved" + ), + path=deque(["Fn::Sub"]), + schema_path=deque([]), + validator="fn_sub", + ), ], ), ( @@ -196,11 +207,11 @@ def context(cfn): [ ValidationError( ( - "'MyResource.Foo' is not one of ['MyResource.Arn', " - "'MyResource.DomainName', " - "'MyResource.DualStackDomainName', " - "'MyResource.RegionalDomainName', " - "'MyResource.WebsiteURL']" + "'Foo' is not one of ['Arn', " + "'DomainName', " + "'DualStackDomainName', " + "'RegionalDomainName', " + "'WebsiteURL']" ), path=deque(["Fn::Sub"]), schema_path=deque([]), @@ -214,7 +225,7 @@ def context(cfn): {"type": "string"}, [ ValidationError( - "'Foo.Bar' is not one of ['MyResource', 'MySimpleAd']", + "'Foo' is not one of ['MyResource', 'MySimpleAd']", path=deque(["Fn::Sub"]), schema_path=deque([]), validator="fn_sub", @@ -229,9 +240,9 @@ def context(cfn): ValidationError( ( "'MySimpleAd.DnsIpAddresses' is not " - "of type 'string', 'integer', " - "'number', 'boolean'" + "of type 'string', 'integer', 'number', 'boolean'" ), + instance="MySimpleAd.DnsIpAddresses", path=deque(["Fn::Sub"]), schema_path=deque([]), validator="fn_sub", @@ -241,6 +252,13 @@ def context(cfn): ], ) def test_validate(name, instance, schema, expected, rule, context, cfn): - validator = CfnTemplateValidator(context=context, cfn=cfn) + validator = CfnTemplateValidator({}).extend( + validators={ + "ref": Ref().ref, + "fn_getatt": GetAtt().fn_getatt, + } + )(context=context, cfn=cfn) errs = list(rule.fn_sub(validator, schema, instance, {})) + for err in errs: + print(err.schema_path) assert errs == expected, f"Test {name!r} got {errs!r}" diff --git a/test/unit/rules/parameters/test_no_echo.py b/test/unit/rules/parameters/test_no_echo.py new file mode 100644 index 0000000000..6ff405587a --- /dev/null +++ b/test/unit/rules/parameters/test_no_echo.py @@ -0,0 +1,135 @@ +""" +Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +SPDX-License-Identifier: MIT-0 +""" + +from collections import deque + +import pytest + +from cfnlint.context import create_context_for_template +from cfnlint.jsonschema import CfnTemplateValidator, ValidationError +from cfnlint.rules.parameters.NoEcho import NoEcho +from cfnlint.template import Template + + +@pytest.fixture(scope="module") +def rule(): + rule = NoEcho() + yield rule + + +@pytest.fixture(scope="module") +def cfn(): + return Template( + "", + { + "Parameters": { + "Echo": {"Type": "String", "NoEcho": "true"}, + "EchoTrue": { + "Type": "String", + "NoEcho": True, + }, + "NoEcho": {"Type": "String", "NoEcho": "false"}, + "MyParameter": {"Type": "String"}, + }, + "Resources": {}, + }, + regions=["us-east-1"], + ) + + +@pytest.fixture(scope="module") +def context(cfn): + return create_context_for_template(cfn) + + +@pytest.mark.parametrize( + "name,instance,path,expected", + [ + ( + "An Invalid Ref", + {"Ref": []}, + deque(["Metadata"]), + [], + ), + ( + "A ref to something that isn't a parameter", + {"Ref": "AWS::Region"}, + deque(["Metadata"]), + [], + ), + ( + "Using NoEcho in Resources Properties", + {"Ref": "Echo"}, + deque(["Resources", "MyResource", "Properties", "Name"]), + [], + ), + ( + "Using NoEcho in Resource Metadata", + {"Ref": "Echo"}, + deque(["Resources", "MyResource", "Metadata"]), + [ + ValidationError( + "Don't use 'NoEcho' parameter 'Echo' in resource metadata", + rule=NoEcho(), + path=deque(["Ref"]), + ) + ], + ), + ( + "Using NoEcho in Metadata", + {"Ref": "Echo"}, + deque(["Metadata"]), + [ + ValidationError( + "Don't use 'NoEcho' parameter 'Echo' in 'Metadata'", + rule=NoEcho(), + path=deque(["Ref"]), + ) + ], + ), + ( + "Using NoEcho in Metadata", + {"Ref": "EchoTrue"}, + deque(["Metadata"]), + [ + ValidationError( + "Don't use 'NoEcho' parameter 'EchoTrue' in 'Metadata'", + rule=NoEcho(), + path=deque(["Ref"]), + ) + ], + ), + ( + "Using Non NoEcho in Metadata", + {"Ref": "NoEcho"}, + deque(["Metadata"]), + [], + ), + ( + "Using a parameter in Metadata", + {"Ref": "MyParameter"}, + deque(["Metadata"]), + [], + ), + ( + "Using NoEcho in Outputs", + {"Ref": "Echo"}, + deque(["Outputs", "Name", "Value"]), + [ + ValidationError( + "Don't use 'NoEcho' parameter 'Echo' in 'Outputs'", + rule=NoEcho(), + path=deque(["Ref"]), + ) + ], + ), + ], +) +def test_validate(name, instance, path, expected, rule, context, cfn): + for p in path: + context = context.evolve(path=p) + validator = CfnTemplateValidator(context=context, cfn=cfn) + errs = list(rule.validate(validator, {}, instance, {})) + assert errs == expected, f"Test {name!r} got {errs!r}" diff --git a/test/unit/rules/resources/test_noecho.py b/test/unit/rules/resources/test_noecho.py deleted file mode 100644 index 7e1dd69aa0..0000000000 --- a/test/unit/rules/resources/test_noecho.py +++ /dev/null @@ -1,23 +0,0 @@ -""" -Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. -SPDX-License-Identifier: MIT-0 -""" - -from test.unit.rules import BaseRuleTestCase - -from cfnlint.rules.resources.NoEcho import NoEcho # pylint: disable=E0401 - - -class TestNoEcho(BaseRuleTestCase): - def setUp(self): - """Setup""" - super(TestNoEcho, self).setUp() - self.collection.register(NoEcho()) - - def test_file_positive(self): - """Test Positive""" - self.helper_file_positive() - - def test_file_negative(self): - """Test failure""" - self.helper_file_negative("test/fixtures/templates/bad/noecho.yaml", 2)