Repository: bridgecrewio/checkov

Issue:
False positive for CKV_AWS_46: "Ensure no hard coded AWS access key and secret key exists in EC2 user data" 1
The [`string_has_secrets`](https://github.com/bridgecrewio/checkov/blob/master/checkov/common/util/secrets.py) function returns `True` when passed any SHA1 hash. This is problematic because with Terraform resources similar to:

```
resource "aws_instance" "test" {
   // ...
   user_data = filebase64("provision.sh")
}
```

The plan is generated using the hash of the user data, e.g.:

```
10:28:13  #16 29.02       + user_data                    = "d1b0706c4879b31a195d01f3a967e8b35d95cbb7"
```

... which `string_has_secrets` considers as a secret. My suggest would be to have the function return `False` if the user_data is _only_ a SHA1 or MD5 hash

Discussion:
schosterbarak: hi @christophetd looks like test_plan_runner is failing. can you take a look before merging?
christophetd: For sure. The test failure seems unrelated though?

```
=================================== FAILURES ===================================
_____________________ TestRunnerValid.test_runner_root_dir _____________________

self = <tests.terraform.runner.test_plan_runner.TestRunnerValid testMethod=test_runner_root_dir>

    def test_runner_root_dir(self):
        current_dir = os.path.dirname(os.path.realpath(__file__))
        root_dir = current_dir + "/resources"
        runner = Runner()
        report = runner.run(root_folder=root_dir, files=None, external_checks_dir=None,
                            runner_filter=RunnerFilter(framework='all'))
        report_json = report.get_json()
        self.assertTrue(isinstance(report_json, str))
        self.assertIsNotNone(report_json)
        self.assertIsNotNone(report.get_test_suites())
        self.assertEqual(report.get_exit_code(soft_fail=False), 1)
        self.assertEqual(report.get_exit_code(soft_fail=True), 0)
    
>       self.assertEqual(41, report.get_summary()["failed"])
E       AssertionError: 41 != 40
```
schosterbarak: @christophetd  i belive that due to that change one of the checks is now returning a different result over that plan file
mikeurbanski1: @christophetd The `aws_instance.web_host` on line ~550 of `tests/terraform/runner/plan/tfplan.json` now passes the check you edited. I modified the expected pass / fail counts (and fixed a mysterious conflict?).
mikeurbanski1: This is definitely the proudest moment of my git career.

Patch:
diff --git a/checkov/common/util/secrets.py b/checkov/common/util/secrets.py
index 61f703467..af177902e 100644
--- a/checkov/common/util/secrets.py
+++ b/checkov/common/util/secrets.py
@@ -54,6 +54,16 @@ _patterns = {k: [re.compile(p, re.DOTALL) for p in v] for k, v in _secrets_regex
 # now combine all the compiled patterns into one long list
 _patterns['all'] = list(itertools.chain.from_iterable(_patterns.values()))
 
+_hash_patterns = list(map(lambda regex: re.compile(regex, re.IGNORECASE), ['^[a-f0-9]{32}$', '^[a-f0-9]{40}$']))
+def is_hash(s: str) -> bool:
+    """
+    Checks whether a string is a MD5 or SHA1 hash
+
+    :param s:
+    :return:
+    """
+    return any([pattern.search(s) for pattern in _hash_patterns])
+
 
 def string_has_secrets(s: str, *categories) -> bool:
     """
@@ -76,6 +86,9 @@ def string_has_secrets(s: str, *categories) -> bool:
     if not categories or 'all' in categories:
         categories = ['all']
 
+    if is_hash(s):
+        return False
+
     for c in categories:
         if any([pattern.search(s) for pattern in _patterns[c]]):
             return True


Test patch:
diff --git a/tests/terraform/runner/test_plan_runner.py b/tests/terraform/runner/test_plan_runner.py
index 652f274fa..7a5c1a962 100644
--- a/tests/terraform/runner/test_plan_runner.py
+++ b/tests/terraform/runner/test_plan_runner.py
@@ -117,8 +117,8 @@ class TestRunnerValid(unittest.TestCase):
         self.assertEqual(report.get_exit_code(soft_fail=False), 1)
         self.assertEqual(report.get_exit_code(soft_fail=True), 0)
 
-        self.assertEqual(61, report.get_summary()["failed"])
-        self.assertEqual(60, report.get_summary()["passed"])
+        self.assertEqual(60, report.get_summary()["failed"])
+        self.assertEqual(61, report.get_summary()["passed"])
 
         files_scanned = list(set(map(lambda rec: rec.file_path, report.failed_checks)))
         self.assertGreaterEqual(5, len(files_scanned))
diff --git a/tests/unit/test_secrets.py b/tests/unit/test_secrets.py
index 2545e12b3..382266dd2 100644
--- a/tests/unit/test_secrets.py
+++ b/tests/unit/test_secrets.py
@@ -28,3 +28,11 @@ class TestSecrets(unittest.TestCase):
 
         # check explicit all plus another category
         self.assertEqual(3, sum(1 for s in test_strings if string_has_secrets(s, ALL, AWS)))
+
+    # Regression test for https://github.com/bridgecrewio/checkov/issues/754
+    def test_does_not_consider_single_hash_as_a_secret(self):
+        # SHA1
+        self.assertFalse(string_has_secrets("b5a5b36b6be8d98c6f1bea655536d67abef23be8"))
+
+        # MD5
+        self.assertFalse(string_has_secrets("d9de48cf0676e9edb99bd8ee1ed44a21"))