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

Multiple issues with the password generation algorithm #26

Open
palant opened this issue Nov 12, 2018 · 1 comment
Open

Multiple issues with the password generation algorithm #26

palant opened this issue Nov 12, 2018 · 1 comment

Comments

@palant
Copy link

palant commented Nov 12, 2018

A user of my PfP pointed me to this app. Since I've done some (very cursory) analysis already, I thought that I would share the findings with you. The password generation algorithm as it is implemented right now has multiple issues:

  • Combining PBKDF2 and Bcrypt makes no sense whatsoever. With the former being a considerably weaker algorithm, it would make far more sense to use Bcrypt exclusively.
  • While the number of PBKDF2 iterations can be adjusted, even 10000 (the highest possible value) is far too low to be useful. See my blog post on the issue here.
  • The 10 Bcrypt rounds (hardcoded and not configurable) are better from what I can tell, but that's still a value which was considered acceptable five years ago - I guess that by today's standards it should be at least 12 rounds.
  • Selecting a hash algorithm for PBKDF2 only helps confuse users, the choice of hash algorithm has no security impact whatsoever.

Finally, it is concerning that passwords generated are device-specific, as there is no way to recover passwords should that device fail.

@Yonjuni
Copy link
Collaborator

Yonjuni commented Nov 12, 2018

@palant thanks for the comments.

  • Adjusting the number of BCrypt iterations was under discussion, but is not supported yet (see Issue 10).

  • Users can choose whether the passwords are device-specific and the binding string could also be used on other devices to recover the password, but yes it is not possible to add it in the UI in the current version - good point.

  • We did a pre-study with users and the hashing choice did not confuse them, probably because it's part of the expert section.

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

No branches or pull requests

3 participants