use constant-time comparison for session password in checkPasswd#2401
use constant-time comparison for session password in checkPasswd#2401jmestwa-coder wants to merge 1 commit into
Conversation
|
any update? |
|
+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. |
|
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. |
|
@jmestwa-coder Please be aware that we cannot accept patches without a Jira ticket. |
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.