Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[0.10.0-rc2] user passwords not changeable in cluster #5532

Closed
dgnorton opened this issue Feb 3, 2016 · 2 comments
Closed

[0.10.0-rc2] user passwords not changeable in cluster #5532

dgnorton opened this issue Feb 3, 2016 · 2 comments
Assignees
Milestone

Comments

@dgnorton
Copy link
Contributor

dgnorton commented Feb 3, 2016

Issue #5505 was fixed for single node in RC2 by PR #5520 but it is still broken for clusters. The fix for single node was to:

  • update user's password
  • on successful update, meta.Client deletes that user from meta.Client.authCache.

The problem in a cluster is that only the meta.Client on the node that executed the update knows to update its authCache. The other nodes still have an out of date authCache containing the hash for the old password.

A quick hack would be to deleted the entire authCache every time the loop in meta.Client.retryUntilSnapshot gets a new snapshot. A better way would be to individually update each user in the authCache from the user data in the new snapshot.

@jwilder jwilder modified the milestone: 0.11.0 Feb 3, 2016
@joelegasse
Copy link
Contributor

/cc @jwilder

I think we're going about authentication the wrong way. bcrypt is intended to slow down brute-force attempts at guessing passwords, and the authCache is just a way to make checking passwords faster(weaker). Once a user has successfully authenticated to a node, future attempts for that user will return near-instantly, just about eliminating the initial perceived benefit of using bcrypt.

What this means is that for a long-running node, authentication attempts for any active user can be done in bulk in a short amount of time.

If we really want bcrypt, and are willing to take the timing hit, we should just drop the cost factor down to an acceptable value. If speed is critical (and I would think it is), and we want to continue with our current authentication scheme of sending passwords over the wire, then I would suggest that we just store the passwords as salted hashes. It would be no less secure than what we currently have, other than the on-disk storage. This would require a slightly larger change, and code to gracefully transition user data in the meta store.

I'd be okay with fixing this quickly so that it works, and revisiting our authentication scheme afterward. The fix would be to fall-through the cache on an invalid password, instead of treating the cache as authoritative. If we're going to keep bcrypt, then it will still penalize invalid passwords while allowing the correct password through quickly (after the first attempt). This also means we can revert the change in #5520 that treats the local authCache differently than remote nodes.

@joelegasse
Copy link
Contributor

On second thought, the old passwords would still be valid, so we would have to also store the bcrypt hash and compare them when a new snapshot arrives.

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

No branches or pull requests

3 participants