Redact connection-URL credentials from exceptions; stop echoing ES failure detail to DLQ - #933
Open
Jainam Jain (jjain1259) wants to merge 1 commit into
Open
Redact connection-URL credentials from exceptions; stop echoing ES failure detail to DLQ#933Jainam Jain (jjain1259) wants to merge 1 commit into
Jainam Jain (jjain1259) wants to merge 1 commit into
Conversation
…ng ES failure detail to DLQ Two latent (not-observed-in-production) log-leak paths found by a systematic audit of this connector, both defense-in-depth follow-ups to CC-42849/CC-36331: - connection.url can carry an embedded user:pass@ credential. Four call sites parsed it via HttpHost.create()/new URI() with no redaction, so a malformed credentialed URL would echo the raw credential into an exception message: ElasticsearchClient's constructor, ConfigCallbackHandler.configureAuthentication(), Validator.createClient(), and ElasticsearchSinkConnectorConfig's UrlListValidator (whose ConfigException also surfaces via the Connect config-validate REST API, not just logs). Added ConfigCallbackHandler.createRedactedHttpHost() and reused the existing redactUserInfo() helper at all four sites. - ElasticsearchClient's DLQ report for malformed-document bulk failures passed Elasticsearch's own failure message through unstripped; ES's document_parsing_exception can include a preview of the offending field's value. The DLQ report now includes only op-type/index; the full detail is still available at DEBUG. Confirmed via production OpenSearch data that neither path has fired with sensitive content in the last 30 days -- these are hardening fixes, not incident response.
|
🎉 All Contributor License Agreements have been signed. Ready to merge. |
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



Summary
Two latent (defense-in-depth, not currently-observed-with-sensitive-content) log-leak paths found by a systematic log-leak audit of this connector on
15.0.x, confirmed to also exist unchanged on14.1.x. Both are follow-ups adjacent to the CC-42849/CC-36331 fixes, but outside the scope of what those covered.1. Connection-URL credentials can reach exception messages via 4 unredacted
HttpHost.create()/new URI()call sitesconnection.urlcan carry an embeddeduser:pass@hostcredential (the code already documents this — seeConfigCallbackHandler'sredactedConnectionUrls()javadoc). Four places parsed the raw URL with no redaction, so a malformed credentialed URL would echo the raw credential into an exception message:ElasticsearchClient's constructor (the initialHttpHost[]build, unguarded, fires on every task start)ConfigCallbackHandler.configureAuthentication()(same call, only reached when username/password auth is configured)Validator.createClient()(validation-time client build — logged at ERROR and returned via the Connect config-validate REST API)ElasticsearchSinkConnectorConfig'sUrlListValidator(wrapped and rethrown uncaught byElasticsearchSinkConnector.start())Added
ConfigCallbackHandler.createRedactedHttpHost(), which reuses the existing (already-correct, from CC-42849)redactUserInfo()helper to redact the URL before it can appear in a parse-failure exception's message, and applied it at all four sites.UrlListValidatoralso now passes a redacted URL list toConfigExceptionitself, since Kafka's own config-validation framework renders that argument verbatim in its generated message.Production check: queried
logs.json.connect(prod OpenSearch) for the last 30 days. The general catch-all this feeds into fired ~104,651 times at ERROR; sampled >99.9% of that volume (top 50 distinct exception messages, including 3 confirmedHttpHost.create()parse failures) — no embedded credential in any observed instance. Latent, not an incident, but real: customers who embed a credential in a malformed URL are exposed today across 4 code paths, one of which is directly returned by the REST API validate response, not just logs.2.
ElasticsearchClient's DLQ report echoed Elasticsearch's raw failure messageFor bulk items failing with a malformed-document error (
document_parsing_exception,mapper_parsing_exception, etc.), the connector reported"Indexing failed: " + response.getFailureMessage()to the errant-record reporter verbatim. Elasticsearch's owndocument_parsing_exceptioncan include a preview of the offending field's value in that message. The DLQ report now includes only the op-type and index name; the full ES failure detail is still available at DEBUG (matching the existing DEBUG/TRACE precedent from CC-42849).Production check: zero hits for
"Indexing failed:"across the whole index in the last 30 days — not observed firing recently.Test plan
mvn clean compileandmvn test-compilesucceedmvn test(excluding the Docker-dependentElasticsearchClientTest) — 196/196 passConfigCallbackHandlerTest(createRedactedHttpHostparses valid URLs, redacts credential on parse failure),ElasticsearchSinkConnectorConfigTest(UrlListValidatorredacts credential inConfigExceptionmessage)ElasticsearchClientTest(testReporterDoesNotLeakElasticsearchFailureMessage, modeled on the existingtestReporterWithFail) — compiles cleanly but couldn't be run locally (no Docker in this environment); will run in CI🤖 Generated with Claude Code