Skip to content

use constant-time comparison for session password in checkPasswd#2401

Open
jmestwa-coder wants to merge 1 commit into
apache:masterfrom
jmestwa-coder:session-passwd-constant-time
Open

use constant-time comparison for session password in checkPasswd#2401
jmestwa-coder wants to merge 1 commit into
apache:masterfrom
jmestwa-coder:session-passwd-constant-time

Conversation

@jmestwa-coder

Copy link
Copy Markdown

checkPasswd in ZooKeeperServer compares the client-supplied session password against the server-generated one with Arrays.equals, which returns as soon as two bytes differ. The password comes straight off the wire (processConnectRequest reads it and reopenSession hands it here), so the early-exit timing leaks how many leading bytes are correct and lets a reconnecting client recover the password and revalidate someone else's session. Switch to MessageDigest.isEqual, which compares in constant time.

@jmestwa-coder

Copy link
Copy Markdown
Author

any update?

@maoling

maoling commented Jun 24, 2026

Copy link
Copy Markdown
Member

+0

The "password" is essentially a lightweight session validation token that prevents clients from attaching to arbitrary sessions using only a sessionId.

It is not intended to provide strong security guarantees. The secret is hardcoded (0xB3415C00L), java.util.Random is not cryptographically secure, and the password can theoretically be derived from the sessionId. Its primary purpose is to prevent accidental session reconnection rather than defend against a determined attacker.

@jmestwa-coder

Copy link
Copy Markdown
Author

fair point. with the secret hardcoded and java.util.Random not being a csprng, the password is derivable from the sessionId anyway, so the timing oracle isn't the weak link and this isn't a real security improvement.

it's a cheap drop-in regardless: MessageDigest.isEqual matches Arrays.equals exactly on true/false/null, so no behavior change. fine to keep it as cleanup, or close it out if you'd rather not carry the diff. your call.

@anmolnar

Copy link
Copy Markdown
Contributor

@jmestwa-coder Please be aware that we cannot accept patches without a Jira ticket.
I'm letting you know, because I saw multiple patches opened by you recently without a ticket reference.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants