diff --git a/prowler/changelog.d/kingfisher-jdbc-false-positive.fixed.md b/prowler/changelog.d/kingfisher-jdbc-false-positive.fixed.md new file mode 100644 index 0000000000..e94de00bfc --- /dev/null +++ b/prowler/changelog.d/kingfisher-jdbc-false-positive.fixed.md @@ -0,0 +1 @@ +Secret checks no longer report credential-free JDBC connection strings as embedded credentials diff --git a/prowler/lib/utils/kingfisher_rules/kingfisher_jdbc_1.yaml b/prowler/lib/utils/kingfisher_rules/kingfisher_jdbc_1.yaml new file mode 100644 index 0000000000..130e20b7cb --- /dev/null +++ b/prowler/lib/utils/kingfisher_rules/kingfisher_jdbc_1.yaml @@ -0,0 +1,97 @@ +# Override of Kingfisher's built-in `kingfisher.jdbc.1`. Loading this file with +# `--rules-path` replaces the built-in rule of the same id (see +# `_build_kingfisher_command` in prowler/lib/utils/utils.py). +# +# The built-in pattern matches a bare `jdbc::` prefix plus any 10 +# non-space characters, so every JDBC connection string is reported as an +# embedded credential even when it carries none. The defect is upstream +# (https://github.com/mongodb/kingfisher), still present in 1.110.0. +# +# Because this replaces the built-in rule rather than extending it, every field +# below other than `pattern` and `examples` is a verbatim copy of the built-in +# rule: dropping one would silently disable it. `validation` in particular is +# what makes `--scan-secrets-validate` confirm a JDBC credential is live, and +# `pattern_requirements` is what discards placeholder values. +# +# Drop this file when a `kingfisher-bin` bump makes the credential-free cases in +# `Test_detect_secrets_scan_batch_jdbc` pass without it. +rules: + - name: JDBC connection string with embedded credentials + id: kingfisher.jdbc.1 + # Only this and `examples` diverge from the built-in rule. The `{1,32}` + # scheme quantifier (upstream uses `{2,32}`) also lets two-character schemes + # such as `jdbc:h2:` match. + # + # `#` is escaped even inside character classes: under `(?x)` a bare one + # opens a comment there too, and Kingfisher then fails to compile the rule + # and aborts the whole scan. + pattern: | + (?xi) + ( + (?: + # Credential forms that any JDBC subprotocol can carry. + jdbc: + [a-z][a-z0-9+.-]{1,32} + (?:[:][a-z0-9+.-]{1,32})* + : + (?: + # URL userinfo, anchored to the `//` that opens the authority. + # Both halves also exclude `?&;#` so neither can reach into the + # query string or the property list looking for an `@`. + // [^\s"'<>/@:?&;\#]{1,64} : [^\s"'<>/@?&;\#]{1,64} @ + # Password as a query parameter or a `;`-delimited property. Only + # this one keeps a leading `.*?`, because its match starts at the + # delimiter before the keyword, anywhere in the string. + | [^\s"'<>,(){}\[\]]{0,384}? + [?&;] [ \t]* (?:password|passwd|pwd|secret) [ \t]* = [ \t]* [^\s"'<>&;]{1,128} + ) + # Oracle TNS userinfo, `jdbc:oracle::user/password@db`. + # Spelled out as its own top-level alternative rather than as a third + # branch above, because `user/password@` is a credential only after an + # Oracle prefix: every other subprotocol reads `a/b@c` as part of a + # path or a host, so sharing the branch reported credential-free + # strings such as `jdbc:derby:team/ops@corp.internal`. + | jdbc:oracle: + [a-z0-9+.-]{1,32} + : + [^\s"'<>/@:?&;\#]{1,64} / [^\s"'<>/@?&;\#]{1,64} @ + # MySQL Connector/J host-list credentials. Keep both forms anchored + # to the MySQL prefix so this syntax cannot affect other drivers. + | jdbc:mysql:// + (?: + \( + [ \t]* host [ \t]* = [ \t]* [^,()\s"'<>/]{1,128} + [ \t]* , [ \t]* user [ \t]* = [ \t]* [^,()\s"'<>/]{1,64} + [ \t]* , [ \t]* password [ \t]* = [ \t]* [^,()\s"'<>/]{1,128} + [ \t]* \) + | address [ \t]* = + [ \t]* \( [ \t]* host [ \t]* = [ \t]* [^()\s"'<>/]{1,128} [ \t]* \) + [ \t]* \( [ \t]* user [ \t]* = [ \t]* [^()\s"'<>/]{1,64} [ \t]* \) + [ \t]* \( [ \t]* password [ \t]* = [ \t]* [^()\s"'<>/]{1,128} [ \t]* \) + ) + ) + [^\s"'<>,(){}\[\]]{0,192} + ) + pattern_requirements: + min_special_chars: 2 + special_chars: ";=/?@&" + ignore_if_contains: + - "****" + - "xxxx" + - "example" + min_entropy: 3.3 + confidence: medium + validation: + type: Jdbc + tls_mode: lax + # Enforced at load time: Kingfisher rejects the rule if one does not match. + examples: + - "jdbc:mysql://admin:s3cr3t@prod.internal:3306/inventory" # trufflehog:ignore + - "jdbc:postgresql://db.example.com:5432/app?user=admin&password=s3cr3t" # trufflehog:ignore + - "jdbc:sqlserver://sql.example.org:1433;databaseName=inventory;user=sa;password=s3cr3t!" # trufflehog:ignore + - "jdbc:oracle:thin:scott/tiger@ora.example.net:1521:ORCLPDB1" # trufflehog:ignore + - "jdbc:h2:file:./data/store;CIPHER=AES;PASSWORD=filepwd" # trufflehog:ignore + references: + - https://docs.oracle.com/javase/8/docs/api/java/sql/DriverManager.html + - https://jdbc.postgresql.org/documentation/use/ + - https://github.com/pgjdbc/pgjdbc/blob/3a699d57d957ca0c2b86e619d001a8763a130027/docs/content/documentation/use.md diff --git a/prowler/lib/utils/utils.py b/prowler/lib/utils/utils.py index fdec495c25..909e067be1 100644 --- a/prowler/lib/utils/utils.py +++ b/prowler/lib/utils/utils.py @@ -47,6 +47,11 @@ default_secrets_batch_chunk_size = 500 # cannot block the audit indefinitely. default_secrets_scan_timeout = 300 +# Directory of Prowler-maintained Kingfisher rules, loaded with ``--rules-path`` +# on every scan. A rule here that reuses a built-in id replaces the built-in one +# (see kingfisher_rules/*.yaml for why each override exists). +secrets_rules_path = os.path.join(os.path.dirname(__file__), "kingfisher_rules") + class SecretsScanError(Exception): """The secret scanner could not produce a trustworthy result. @@ -86,6 +91,9 @@ def _build_kingfisher_command( "--no-update-check", "--confidence", confidence, + # Overrides for built-in rules that produce false positives. + "--rules-path", + secrets_rules_path, ] if validate: # Live-validate discovered secrets against provider APIs. Use diff --git a/tests/lib/utils/utils_test.py b/tests/lib/utils/utils_test.py index 342f5d4bd5..18fc8729c7 100644 --- a/tests/lib/utils/utils_test.py +++ b/tests/lib/utils/utils_test.py @@ -5,6 +5,7 @@ from datetime import datetime from time import mktime import pytest +import yaml from mock import patch from prowler.lib.utils.utils import ( @@ -17,6 +18,7 @@ from prowler.lib.utils.utils import ( open_file, outputs_unix_timestamp, parse_json_file, + secrets_rules_path, strip_ansi_codes, validate_ip_address, ) @@ -259,6 +261,139 @@ class Test_detect_secrets_scan_batch: assert "x" in results +JDBC_RULE = "JDBC connection string with embedded credentials" + + +class Test_detect_secrets_scan_batch_jdbc: + """The bundled override of Kingfisher's built-in ``kingfisher.jdbc.1``. + + The built-in rule matches a bare ``jdbc::`` prefix followed by any 10 + non-space characters, so every JDBC connection string was reported as an + embedded credential. The override in + ``prowler/lib/utils/kingfisher_rules/kingfisher_jdbc_1.yaml`` requires an + actual credential; these tests pin both halves of that behavior. + """ + + def _jdbc_findings(self, connection_string): + results = detect_secrets_scan_batch({"a": connection_string}) + return [f for f in results.get("a", []) if f["type"] == JDBC_RULE] + + def test_override_keeps_every_non_pattern_field_of_the_builtin(self): + """Replacing the built-in rule drops any field the override omits. + + Losing ``validation`` would silently stop ``--scan-secrets-validate`` + from confirming a JDBC credential is live, and losing + ``pattern_requirements`` would stop placeholder values being discarded — + neither of which any behavioral test would catch. Only ``pattern`` and + ``examples`` are meant to diverge. + """ + with open( + os.path.join(secrets_rules_path, "kingfisher_jdbc_1.yaml"), + encoding="utf-8", + ) as f: + rule = yaml.safe_load(f)["rules"][0] + + # Verbatim from crates/kingfisher-rules/data/rules/jdbc.yml upstream. + assert rule["id"] == "kingfisher.jdbc.1" + assert rule["name"] == JDBC_RULE + assert rule["confidence"] == "medium" + assert rule["min_entropy"] == 3.3 + assert rule["validation"] == {"type": "Jdbc"} + assert rule["tls_mode"] == "lax" + assert rule["pattern_requirements"] == { + "min_special_chars": 2, + "special_chars": ";=/?@&", + "ignore_if_contains": ["****", "xxxx", "example"], + } + assert rule["references"] + + def test_rules_path_is_passed_to_kingfisher(self): + """The override is only in effect if the directory is actually shipped + and handed to Kingfisher.""" + assert os.path.isdir(secrets_rules_path) + assert os.path.isfile( + os.path.join(secrets_rules_path, "kingfisher_jdbc_1.yaml") + ) + + with patch( + "prowler.lib.utils.utils.subprocess.run", + side_effect=_fake_kingfisher_run(output_content="{}"), + ) as mocked_run: + detect_secrets_scan_batch({"a": "data"}) + + command = mocked_run.call_args[0][0] + assert "--rules-path" in command + assert command[command.index("--rules-path") + 1] == secrets_rules_path + + @pytest.mark.parametrize( + "connection_string", + [ + "jdbc:postgresql://mydb.cluster-abc123.eu-west-1.rds.amazonaws.com:5432/appdb", # trufflehog:ignore + "jdbc:oracle:thin:@ora.corp.internal:1521/ORCLPDB1", # trufflehog:ignore + "jdbc:oracle:thin:@//ora.corp.internal:1521/SVC", # trufflehog:ignore + "jdbc:mysql://prod.internal:3306/inventory?useSSL=true", # trufflehog:ignore + "jdbc:sqlserver://sql.corp.internal:1433;databaseName=inv;integratedSecurity=true", # trufflehog:ignore + "jdbc:redshift://cluster.abc.us-east-1.redshift.amazonaws.com:5439/dev", # trufflehog:ignore + # A username alone is not a credential. + "jdbc:mysql://prod.internal:3306/inventory?user=admin", # trufflehog:ignore + # An empty password is not a credential. + "jdbc:postgresql://pg.corp.internal/app?password=", # trufflehog:ignore + "jdbc:mysql://(host=db.internal,user=alice,password=)/app", # trufflehog:ignore + "jdbc:mysql://address=(host=db.internal)(user=alice)(password=)/app", # trufflehog:ignore + # Connector/J host-list credentials require a non-empty username. + "jdbc:mysql://(host=db.internal,user=,password=Zq81ncPl42)/app", # trufflehog:ignore + "jdbc:mysql://address=(host=db.internal)(user=)(password=Zq81ncPl42)/app", # trufflehog:ignore + # Connector/J host-list syntax must not apply to other drivers. + "jdbc:postgresql://(host=db.internal,user=alice,password=Zq81ncPl42)/app", # trufflehog:ignore + # An `@` in the query string must not turn the host and port into + # `user:password`: without the userinfo alternative being anchored + # to `//`, `db.internal:3306?user=alice` reads as a credential. + "jdbc:mysql://db.internal:3306?user=alice@corp.internal", # trufflehog:ignore + # The same backtrack against the `user/password@` alternative. + "jdbc:mysql://db.internal:3306?owner=team/ops@corp.internal", # trufflehog:ignore + "jdbc:mysql://db.internal:3306?path=a:b/c@corp.internal", # trufflehog:ignore + # And against a `;`-delimited property list. + "jdbc:sqlserver://sql.corp.internal:1433;user=sa@corp.internal", # trufflehog:ignore + # `user/password@` is Oracle TNS syntax and a credential only after + # an Oracle prefix. Every other subprotocol reads `a/b@c` as part of + # a path or a host, so the alternative must not apply to them. + "jdbc:derby:team/ops@corp.internal", # trufflehog:ignore + "jdbc:sqlite:team/ops@corp.internal", # trufflehog:ignore + "jdbc:h2:file:team/ops@corp.internal", # trufflehog:ignore + # The exact payload shape of a CloudFormation Output + # ("OutputKey:OutputValue"), which is how this was reported. + "DatabaseUrl:jdbc:postgresql://mydb.eu-west-1.rds.amazonaws.com:5432/appdb", # trufflehog:ignore + ], + ) + def test_credential_free_connection_string_is_not_reported(self, connection_string): + assert self._jdbc_findings(connection_string) == [] + + @pytest.mark.parametrize( + "connection_string", + [ + # URL userinfo. + "jdbc:mysql://admin:s3cr3t@prod.internal:3306/inventory", # trufflehog:ignore + # MySQL Connector/J host-list credentials. + "jdbc:mysql://(host=db.internal,user=alice,password=Zq81ncPl42)/app", # trufflehog:ignore + "jdbc:mysql://address=(host=db.internal)(user=alice)(password=Zq81ncPl42)/app", # trufflehog:ignore + # Password as a query parameter. + "jdbc:postgresql://pg.corp.internal:5432/app?user=admin&password=Tr0ub4dor3", # trufflehog:ignore + "jdbc:postgresql://pg.corp.internal/app?password=Xk29fjWa02", # trufflehog:ignore + "jdbc:mysql://prod.internal/db?user=a&pwd=Zq81ncPl42", # trufflehog:ignore + # Password as a semicolon-delimited property. + "jdbc:sqlserver://sql.corp.internal:1433;databaseName=inv;user=sa;password=S3cr3t99", # trufflehog:ignore + "jdbc:sqlserver://sql.corp.internal:1433;Password=Vb73msQr18;user=sa", # trufflehog:ignore + # Oracle TNS userinfo, for each driver type. + "jdbc:oracle:thin:scott/tiger99@ora.corp.internal:1521:ORCL", # trufflehog:ignore + "jdbc:oracle:oci:scott/tiger99@ora.corp.internal:1521:ORCL", # trufflehog:ignore + # Two-character scheme, which the built-in pattern could not match. + "jdbc:h2:file:./data/store;CIPHER=AES;PASSWORD=Nf62kdTp07", # trufflehog:ignore + ], + ) + def test_embedded_credential_is_still_reported(self, connection_string): + assert self._jdbc_findings(connection_string) != [] + + class Test_detect_secrets_scan_batch_failures: """A scanner failure must surface as SecretsScanError, never as empty results (which a caller would read as 'no secrets found')."""