Skip to content

KAFKA-20964: Do not echo salt or saltedPassword in ScramParser errors - #23312

Open
shantanushukla wants to merge 1 commit into
apache:trunkfrom
shantanushukla:KAFKA-20964
Open

KAFKA-20964: Do not echo salt or saltedPassword in ScramParser errors#23312
shantanushukla wants to merge 1 commit into
apache:trunkfrom
shantanushukla:KAFKA-20964

Conversation

@shantanushukla

@shantanushukla shantanushukla commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

JIRA: https://issues.apache.org/jira/browse/KAFKA-20964

kafka-storage format --add-scram parses SCRAM credentials in ScramParser. On Base64 decode failure, PerMechanismData built a FormatterException whose message included the user-supplied saltedpassword (and the same for salt). StorageTool prints getMessage() to stderr, so credential-adjacent input was echoed in the CLI error.

Fix

Use a generic decode-failure message and keep the IllegalArgumentException as the cause. Do not concatenate the user-supplied value.

Failed to decode given saltedPassword
Failed to decode given salt

This matches the existing request-side practice of not printing salt or salted password (AlterUserScramCredentialsRequest). Decode details remain on the cause if it is logged.

Why existing tests missed this

ScramParserTest covered valid Base64 saltedpassword/salt and missing-field errors (name, password, salt required with saltedpassword, extra keys). None of those paths throw on Base64 failure, so the decode-error message was never asserted.

Tests

These failed on trunk (message included the raw input) and pass after this change:

  • testParsePerMechanismDataFailsWithInvalidSaltedPasswordBase64 — valid salt so construction reaches the salted-password decoder; saltedpassword="not-valid-base64!!!". Trunk: Failed to decode given saltedPassword: not-valid-base64!!!. After: Failed to decode given saltedPassword.
  • testParsePerMechanismDataFailsWithInvalidSaltBase64salt="%%%". Trunk: Failed to decode given salt: %%%. After: Failed to decode given salt.

Related existing tests still pass: full ScramParserTest (including valid saltedpassword parse and toRecord()).

./gradlew :metadata:test --tests org.apache.kafka.metadata.storage.ScramParserTest :metadata:checkstyleMain :metadata:checkstyleTest :metadata:spotlessCheck

BUILD SUCCESSFUL.

On Base64 decode failure, FormatterException included the user-supplied
value. kafka-storage format --add-scram prints that message to stderr.
Keep a generic message and the IllegalArgumentException cause.
@github-actions github-actions Bot added triage PRs from the community kraft small Small PRs labels Aug 30, 2026
@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown

A label of 'needs-attention' was automatically added to this PR in order to raise the
attention of the committers. Once this issue has been triaged, the triage label
should be removed to prevent this automation from happening again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kraft needs-attention small Small PRs triage PRs from the community

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant