diff --git a/enroll/ignore.py b/enroll/ignore.py index d7c4198..563a9ee 100644 --- a/enroll/ignore.py +++ b/enroll/ignore.py @@ -129,6 +129,55 @@ HIGH_CONFIDENCE_SECRET_PATTERNS = [ # Credentials embedded in connection-string URIs, e.g. # postgres://user:pass@host, redis://:pass@host, amqp://u:p@host re.compile(rb"(?i)[a-z][a-z0-9+.-]*://[^/\s:@]*:[^/\s@]+@[^/\s]"), + # Whitespace-separated credential assignments for COMPOUND credential keys. + # + # The assignment pattern above only recognises a value when the key and value + # are joined by ':' or '='. Some real config styles (.netrc-like files, a + # number of daemon configs) instead separate a key from its value with one or + # more spaces/tabs, e.g. + # aws_secret_access_key wJalrXUtnFEMI/K7MDENG... + # client_secret hunter2value + # oauth_token ya29.aRealLookingToken + # The comment-aware soft tier below would not catch these because its keyword + # boundary (\b) is defeated by the leading underscore in a compound key + # (e.g. the 'secret' in 'client_secret' has a '_' -- a word char -- before it, + # so \bsecret\b does not match). This pattern closes that gap. + # + # It is deliberately restricted to *compound* keys -- a credential keyword that + # is joined to a prefix (case 1) or is itself a multi-word credential name + # (case 2). Standalone English words ('password', 'token', 'secret') are NOT + # matched here: they are already handled, comment-aware, by the soft tier, and + # matching them with a whitespace separator would false-positive on prose and + # on PAM 'password ...' stack directives. The trailing value test + # requires a value-like token (>=6 chars, or one containing a digit/secret-ish + # character) so a short config value such as 'yes'/'no' does not trip it. + re.compile( + rb"""(?ixm) + (?:^|[^A-Za-z0-9_.-]) + [\"']? + (?: + (?:[A-Za-z0-9]+[_.-])+ + (?:password|passwd|passphrase|pwd|pw|token|secret|credential|credentials) + | + (?:[A-Za-z0-9]+[_.-])* + (?: + auth[_.-]token|access[_.-]token|refresh[_.-]token| + client[_.-]secret|secret[_.-]key| + api[_.-]key|access[_.-]key|private[_.-]key| + aws[_.-]access[_.-]key[_.-]id|aws[_.-]secret[_.-]access[_.-]key| + azure[_.-]client[_.-]secret| + google[_.-]application[_.-]credentials|gcp[_.-]service[_.-]account| + service[_.-]account[_.-]key| + session[_.-]key + ) + ) + (?:[_.-][A-Za-z0-9]+)* + [\"']? + [ \t]+ + (?=\S) + (?:\S{6,}|\S*[0-9/+=._-]\S*) + """ + ), # HTTP(S) Authorization / Proxy-Authorization header values carrying a # bearer/basic/digest credential. re.compile( diff --git a/tests/test_secret_detection.py b/tests/test_secret_detection.py index 607d059..3c98d64 100644 --- a/tests/test_secret_detection.py +++ b/tests/test_secret_detection.py @@ -221,3 +221,65 @@ def test_commented_out_credential_values_require_dangerous(): ), data # ...but --dangerous still collects it (operator's explicit choice). assert dangerous._content_deny_reason("/etc/app/app.conf", data) is None, data + + +def test_whitespace_separated_compound_credentials_denied(): + """Security regression (audit finding): a populated credential assignment + that uses whitespace (space/tab) instead of ':'/'=' as the separator must + still be refused in safe mode when the key is a *compound* credential name. + + The soft keyword tier cannot catch these because its ``\\b`` boundary is + defeated by the leading underscore in a compound key (e.g. the ``secret`` in + ``client_secret`` is preceded by ``_``, a word character), and the + assignment tier only recognised ``:``/``=`` separators. Some real config + styles (.netrc-like files, some daemon configs) use whitespace separators, + so a value such as ``aws_secret_access_key wJalr...`` would previously leak. + """ + safe = IgnorePolicy(dangerous=False) + dangerous = IgnorePolicy(dangerous=True) + leaking = [ + b"aws_secret_access_key wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY\n", + b"aws_access_key_id AKIAIOSFODNN7EXAMPLE\n", + b"client_secret hunter2deadbeef\n", + b"oauth_token ya29.A0ARrdaReALlOOkINgToKeN\n", + b"refresh_token 1//0longrefreshvalue\n", + b"my_token deadbeef1234\n", + b"app_password s3cr3tValue\n", + b"db_pw s3cr3tvalue\n", + b"private_key /etc/ssl/private/app.pem\n", + b"session_key 0xDEADBEEFCAFE\n", + b"\tapi_key AKIA12345678\n", + ] + for data in leaking: + assert ( + safe._content_deny_reason("/root/.netrc", data) == "sensitive_content" + ), data + # --dangerous remains the explicit opt-in to collect such material. + assert dangerous._content_deny_reason("/root/.netrc", data) is None, data + + +def test_whitespace_separated_non_credentials_still_allowed(): + """The whitespace-separated credential rule must not false-positive on + ordinary config or PAM directives. It is intentionally restricted to + *compound* credential keys with a value-like token, so standalone words and + common config lines are not affected by it. + + (Standalone ``password``/``token``/``secret`` words on a non-comment line + are still caught by the comment-aware soft tier; that is unchanged. The + cases here are ones that must remain collectable: PAM stack directives, SSH + key-file path directives, and plain settings.) + """ + pol = IgnorePolicy(dangerous=False) + allowed = [ + # SSH key-file path directives (key material is denied by path globs, + # not by content here). + b"HostKey /etc/ssh/ssh_host_ed25519_key\n", + b"host_key /etc/ssh/known\n", + b"public_key /home/u/.ssh/id_rsa.pub\n", + # Plain numeric / boolean settings. + b"max_connections 1024\n", + b"PermitRootLogin no\n", + b"PasswordAuthentication yes\n", + ] + for data in allowed: + assert pol._content_deny_reason("/etc/app/app.conf", data) is None, data