Mikhail Fomichev created KAFKA-20774:
----------------------------------------
Summary: ScramDelta.apply() drops SCRAM users when processing
REMOVE records for absent users or multiple removals in one batch
Key: KAFKA-20774
URL: https://issues.apache.org/jira/browse/KAFKA-20774
Project: Kafka
Issue Type: Bug
Components: kraft
Reporter: Mikhail Fomichev
h3. Problem
In KRaft mode, {{ScramDelta.apply()}} can incorrectly remove SCRAM credentials
from the metadata image when applying a metadata batch.
A typical failing batch for the same {{ScramMechanism}} contains:
# several {{RemoveUserScramCredentialRecord}} entries for users not present in
the current {{{}ScramImage{}}};
# followed by a {{UserScramCredentialRecord}} UPSERT for a user that should
remain (e.g. {{user-admin}} used for inter-broker authentication).
In production, entire SCRAM user sets were lost from broker metadata.
h3. Steps to reproduce
Minimal unit-test reproduction:
{code:java}
ScramDelta delta = new ScramDelta(ScramImage.EMPTY);
delta.replay(remove("producer-kcwc", SCRAM_SHA_256));
delta.replay(remove("consumer-xyff", SCRAM_SHA_256));
delta.replay(upsert("user-admin", SCRAM_SHA_256));
delta.replay(remove("update_user_standalone_eryz", SCRAM_SHA_256));
delta.replay(remove("ext-user-dfrx", SCRAM_SHA_256));
delta.replay(upsert("cluster-admin", SCRAM_SHA_512));
ScramImage result = delta.apply();
// only user-admin is missing with the broken apply() logic {code}
h3. Expected behavior
No silent loss of users: after {{{}ScramDelta.apply(){}}}, every user that
should still exist in metadata must remain in the resulting {{{}ScramImage{}}}.
Users must not disappear as a side effect of batch ordering, absent REMOVE
records, or mechanism cleanup.
h3. Actual behavior
{{user-admin}} disappears from metadata (depends on the order in which changes
are processed).
h3. Root cause
In
[{{metadata/src/main/java/org/apache/kafka/image/ScramDelta.java}}|https://github.com/apache/kafka/blob/trunk/metadata/src/main/java/org/apache/kafka/image/ScramDelta.java#L82],
method {{{}apply(){}}}:
# REMOVE is applied even when the user is not in {{{}userMap{}}}:
{code:java}
userMap.remove(userName);
if (userMap.isEmpty()) {
newMechanisms.remove(mechanism); // fires on an empty map
}{code}
On an empty base image, each absent REMOVE removes the mechanism from
{{{}newMechanisms{}}}; a subsequent UPSERT may write into a detached
{{userMap}} no longer referenced by {{{}newMechanisms{}}}.
# {{userMap.isEmpty()}} is checked inside the per-user loop — when multiple
REMOVEs are processed in one batch, the mechanism can be dropped early and
users that should survive are lost.
h3. Proposed fix
Move after the loop over all user changes.
{code:java}
if (userMap.isEmpty()) newMechanisms.remove(mechanism) {code}
h3. Kafka version
4.x.x
{{}}
{{}}
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
