Skip to content

Commit

Permalink
feat(cloudformation): Add sensitive param check (#6921)
Browse files Browse the repository at this point in the history
* New check

* Add more fixes

* fix cfn-lint

* ignore lint

* Remove dupes

* skip security check

* fix flake8

* Limit for EC2 creds

* Limit Azure VMs
  • Loading branch information
tsmithv11 authored Dec 31, 2024
1 parent 91b57c3 commit b8839fe
Show file tree
Hide file tree
Showing 11 changed files with 237 additions and 9 deletions.
4 changes: 2 additions & 2 deletions checkov/arm/checks/resource/VMCredsInCustomData.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
from typing import List, Dict, Any

from checkov.common.models.enums import CheckResult, CheckCategories
from checkov.common.util.secrets import string_has_secrets
from checkov.common.util.secrets import string_has_secrets, AZURE, GENERAL
from checkov.arm.base_resource_value_check import BaseResourceCheck


Expand All @@ -20,7 +20,7 @@ def scan_resource_conf(self, conf: Dict[str, Any]) -> CheckResult:
if isinstance(os_profile, dict):
custom_data = os_profile.get("customData")
if isinstance(custom_data, str):
if string_has_secrets(custom_data):
if string_has_secrets(custom_data, AZURE, GENERAL):
conf[f'{self.id}_secret'] = custom_data
return CheckResult.FAILED
return CheckResult.PASSED
Expand Down
4 changes: 2 additions & 2 deletions checkov/cloudformation/checks/resource/aws/EC2Credentials.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

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


class EC2Credentials(BaseResourceCheck):
Expand All @@ -28,7 +28,7 @@ def scan_resource_conf(self, conf):
user_data_str = str(user_data)

if isinstance(user_data_str, str):
secrets = get_secrets_from_string(str(user_data_str))
secrets = get_secrets_from_string(str(user_data_str), GENERAL, AWS)
if secrets:
for idx, secret in enumerate(secrets):
conf[f'{self.id}_secret_{idx}'] = secret
Expand Down
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
@@ -1,7 +1,7 @@
from typing import List, Dict, Any

from checkov.common.models.enums import CheckResult, CheckCategories
from checkov.common.util.secrets import string_has_secrets
from checkov.common.util.secrets import string_has_secrets, AZURE, GENERAL
from checkov.terraform.checks.resource.base_resource_value_check import BaseResourceCheck


Expand All @@ -22,7 +22,7 @@ def scan_resource_conf(self, conf: Dict[str, List[Any]]) -> CheckResult:
if custom_data:
custom_data = custom_data[0]
if isinstance(custom_data, str):
if string_has_secrets(custom_data):
if string_has_secrets(custom_data, AZURE, GENERAL):
conf[f'{self.id}_secret'] = custom_data
return CheckResult.FAILED
return CheckResult.PASSED
Expand Down
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()

0 comments on commit b8839fe

Please sign in to comment.