Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(cloudformation): Add sensitive param check #6921

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@ def scan_resource_conf(self, conf: dict[str, Any]) -> CheckResult:
# if it is a resolved instrinsic function like !Ref: xyz, then it can't be a secret
continue

# Skip checking if the value starts with 'handler.'
if isinstance(value, str) and (value.startswith('handler.') or value.startswith('git.')):
continue

secrets = get_secrets_from_string(str(value), AWS, GENERAL)
if secrets:
self.evaluated_keys = [f"Properties/Environment/Variables/{var_name}"]
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
from __future__ import annotations

import re
from typing import Any

from checkov.cloudformation.checks.resource.base_resource_check import BaseResourceCheck
from checkov.common.models.enums import CheckResult, CheckCategories
from checkov.common.util.secrets import AWS, GENERAL, PASSWORD, get_secrets_from_string


class ParameterStoreCredentials(BaseResourceCheck):
def __init__(self) -> None:
name = "Ensure no hard-coded secrets exist in Parameter Store values"
id = "CKV_AWS_384"
supported_resources = ("AWS::SSM::Parameter",)
categories = (CheckCategories.GENERAL_SECURITY,)
super().__init__(name=name, id=id, categories=categories, supported_resources=supported_resources)

def is_dynamic_value(self, value: str) -> bool:
patterns = [
r"\$\{.*?\}", # ${...}
r"\{\{.*?\}\}", # {{...}}
r"\$\(.*?\)", # $(...)
r"!Ref\s+\w+", # !Ref SomeResource
r"!Sub\s+'.*?'", # !Sub '...'
]
return any(re.search(pattern, value) for pattern in patterns)

def scan_resource_conf(self, conf: dict[str, Any]) -> CheckResult:
self.evaluated_keys = ["Properties/Value"]
properties = conf.get("Properties")
if isinstance(properties, dict):
name = properties.get("Name")
if name and re.match("(?i).*secret.*|.*api_?key.*", name):
value = properties.get("Value")
if value:
# If unresolved variable, then pass
if isinstance(value, dict):
return CheckResult.PASSED
# If unresolved variable, then pass 2
if re.match(r".*\$\{.*}.*", value):
return CheckResult.PASSED
if (re.match("(?i)(.*test.*|.*example.*)", name) or
re.match("(?i)(.*test.*|.*example.*)", value)):
return CheckResult.PASSED
secret = get_secrets_from_string(str(value), AWS, GENERAL, PASSWORD)
if secret:
return CheckResult.FAILED
return CheckResult.PASSED


check = ParameterStoreCredentials()
14 changes: 11 additions & 3 deletions checkov/common/util/secrets.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
AZURE = 'azure'
GCP = 'gcp'
GENERAL = 'general'
PASSWORD = 'password' # nosec B105
ALL = 'all'

GENERIC_OBFUSCATION_LENGTH = 10
Expand Down Expand Up @@ -61,6 +62,13 @@

'general': [
"^-----BEGIN (RSA|EC|DSA|GPP) PRIVATE KEY-----$",
],

'password': [
r"[A-Za-z0-9+/]{40,}={0,2}", # Base64 encoded string
r"[0-9a-fA-F]{32,}", # MD5 hash or similar
r"(?=.*[a-z])(?=.*[A-Z])(?=.*\d)(?=.*[@$!%*?&])[A-Za-z\d@$!%*?&]{8,}", # Strong password pattern
r"[A-Za-z0-9]{20,}", # Long alphanumeric string
]
}

Expand Down Expand Up @@ -238,8 +246,8 @@ def get_secrets_from_string(s: str, *categories: str) -> list[str]:
if not categories or "all" in categories:
categories = ("all",)

secrets: list[str] = []
secrets: set[str] = set() # Change to a set for automatic deduplication
for c in categories:
for pattern in _patterns[c]:
secrets.extend(str(match.group()) for match in pattern.finditer(s))
return secrets
secrets.update(str(match.group()) for match in pattern.finditer(s))
return list(secrets) # Convert set back to list before returning
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
Resources:
Pass2:
Type: "AWS::Lambda::Function"
Properties:
FunctionName: "NameOfLambdaFunction"
Handler: "handler.handlerverylongcustomhandlernameforservi"
Runtime: "python3.9"
Role: !GetAtt LambdaExecutionRole.Arn
Code:
S3Bucket: "your-code-bucket"
S3Key: "path/to/your-code.zip"
Environment:
Variables:
STAGE: "staging"
LAMBDA: "handler.handlerverylongcustomhandlernameforservi"
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
Resources:
CDKSecret:
Type: AWS::SecretsManager::Secret
Properties:
Name: my-secret
CDKLambda:
Type: AWS::Lambda::Function
Properties:
Code:
S3Bucket: <bucket-containing-lambda-code>
S3Key: <key-to-lambda-code>
Handler: handler
Runtime: provided.al2
Timeout: 60
Architectures:
- arm64
Environment:
Variables:
APP_PRIVATE_KEY_SECRET_NAME: my-secret
TAGS: git.commit.sha:55e7e7703f17c41f276caf8f1a1b744d674259f8
Role: <lambda-execution-role-arn>
CDKSecretPolicy:
Type: AWS::IAM::Policy
Properties:
PolicyName: SecretAccessPolicy
Roles:
- <lambda-execution-role-arn>
PolicyDocument:
Statement:
- Effect: Allow
Action:
- secretsmanager:GetSecretValue
Resource:
- !Ref CDKSecret
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
Resources:

FailAPIKey:
Type: 'AWS::SSM::Parameter'
Properties:
Name: '/myapp/api_key'
Type: 'String'
Value: 'akdfaksdfjkasdfjskafjdkfajsdfk345'

Bad1:
Type: 'AWS::SSM::Parameter'
Properties:
Name: '/myapp/secret'
Type: 'String'
Value: 'akdfaksdfjkasdfjskafjdkfajsdfk345'

GoodNoKeyword:
Type: 'AWS::SSM::Parameter'
Properties:
Name: '/myapp/foo'
Type: 'String'
Value: 'akdfaksdfjkasdfjskafjdkfajsdfk345'

GoodVariable:
Type: 'AWS::SSM::Parameter'
Properties:
Name: '/myapp/secret2'
Type: 'String'
Value: "${aws_iam_role.lambda[count.index].arn}"

GoodFnSub:
Type: AWS::SSM::Parameter
Metadata:
cfn-lint:
config:
ignore_checks:
- E1019
Properties:
Type: String
Name: secret
Value:
Fn::Sub: '/cdk-bootstrap/${Qualifier}/version'

GoodRef:
Type: AWS::SSM::Parameter
Properties:
Name: SuperSecret
Type: String
Value: !Ref GoodFnSub

Bad2:
Type: 'AWS::SSM::Parameter'
Properties:
Name: 'MYSECRET'
Type: 'String'
Value: 'akdfaksdfjkasdfjskafjdkfajsdfk345'

PassTestName:
Type: 'AWS::SSM::Parameter'
Properties:
Name: 'MYSECRET_TEST'
Type: 'String'
Value: 'akdfaksdfjkasdfjskafjdkfajsdfk345'

PassTestVALUE:
Type: 'AWS::SSM::Parameter'
Properties:
Name: 'MYSECRET2'
Type: 'String'
Value: 'akdfaksdfjkaEXAMPLEsdfjskafjdkfajsdfk345'
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ def test_summary(self):
"AWS::Serverless::Function.NoEnv",
"AWS::Serverless::Function.NoProperties",
"AWS::Serverless::Function.NoSecret",
"AWS::Lambda::Function.Pass2",
"AWS::Lambda::Function.CDKLambda",
}
failing_resources = {
"AWS::Lambda::Function.Secret",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
import unittest
from pathlib import Path

from checkov.cloudformation.checks.resource.aws.ParameterStoreCredentials import check
from checkov.cloudformation.runner import Runner
from checkov.runner_filter import RunnerFilter


class TestParameterStoreCredentials(unittest.TestCase):
def test_summary(self):
test_files_dir = Path(__file__).parent / "example_ParameterStoreCredentials"

report = Runner().run(root_folder=str(test_files_dir), runner_filter=RunnerFilter(checks=[check.id]))
summary = report.get_summary()

passing_resources = {
"AWS::SSM::Parameter.GoodNoKeyword",
"AWS::SSM::Parameter.GoodVariable",
"AWS::SSM::Parameter.GoodFnSub",
"AWS::SSM::Parameter.GoodRef",
"AWS::SSM::Parameter.PassTestName",
"AWS::SSM::Parameter.PassTestVALUE",
}
failing_resources = {
"AWS::SSM::Parameter.FailAPIKey",
"AWS::SSM::Parameter.Bad1",
"AWS::SSM::Parameter.Bad2",
}

passed_check_resources = {c.resource for c in report.passed_checks}
failed_check_resources = {c.resource for c in report.failed_checks}

self.assertEqual(summary["passed"], len(passing_resources))
self.assertEqual(summary["failed"], len(failing_resources))
self.assertEqual(summary["skipped"], 0)
self.assertEqual(summary["parsing_errors"], 0)

self.assertEqual(passing_resources, passed_check_resources)
self.assertEqual(failing_resources, failed_check_resources)


if __name__ == "__main__":
unittest.main()
Loading