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

Why are you trying to hide the iteration count? #4

Closed
EvanCarroll opened this issue Mar 26, 2014 · 6 comments
Closed

Why are you trying to hide the iteration count? #4

EvanCarroll opened this issue Mar 26, 2014 · 6 comments

Comments

@EvanCarroll
Copy link

That's going to make it much more difficult to impliment in another language.

@ericelliott
Copy link
Owner

Why does hiding the iteration count make it difficult to implement in another language?

@EvanCarroll
Copy link
Author

How am I going to implement it in another language without knowing the iteration count? The user of this module is not going to know the actual number of iterations without reading the source and obfuscating whatever you're intending to do with work_key.

@EvanCarroll
Copy link
Author

I've actually implimented my own version of this called co-crypto-saltedhash

@ericelliott
Copy link
Owner

Your default iterations are dangerously low. Even cheap commodity hardware would be able to crack those hashes too quickly. You need closer to 60,000 iterations to get into safe territory for 2014, and next year, the standard will be different, so it's important that it's easy to reconfigure without breaking existing passwords (safest way). If you have to break user passwords, you'll have to have a secure password change mechanism to prevent user lock-outs.

That's why credential includes all the information necessary (minus the work secret) to check the hash:

{
  hash: hash,
  salt: salt,
  keyLength: keyLength,
  hashMethod: hashMethod,
  workUnits: workUnits
}

All this gets stored with the passwords. That way you can easily move the work units to keep up with changing technology without locking users out. Just remind them to change their password when they log in if they're using the outdated work unit standard.

Credential hides the work secret by default (though it does have a sensible fallback if you don't want to use that feature) because of security in layers. It's that much harder to breach the password database if you have no idea how many iterations to try.

@ericelliott
Copy link
Owner

I would not recommend rolling your own solution for this stuff. Credential was audited by as many security professionals as I could find before it was published, and I'm still nervous about it, for good reason. Password security is currently one of the largest weaknesses in online security -- a much more dangerous threat than even the most sophisticated XSS or injection attacks, because of the potential impact if there's a password database theft. Proceed with caution.

@ericelliott
Copy link
Owner

BTW, your verify function is vulnerable to a variable time equality attack.

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

2 participants