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

zxcvbn compatibility #57

Closed
TomLottermann opened this issue Sep 29, 2020 · 9 comments
Closed

zxcvbn compatibility #57

TomLottermann opened this issue Sep 29, 2020 · 9 comments

Comments

@TomLottermann
Copy link
Contributor

We are currently building password rating into two separate clients, one Java, one C++ and would like to use native password rating libraries.

For Java we are already using nbvcxz, for C++ we want to use zxcvbn.

Ideally both clients would identify the same passwords as weak. However, reading https://github.com/GoSimpleLLC/nbvcxz#differentiating-features, I'm assuming this never was a goal of nbvcxz (which I understand).
What would it take to give nbvcxz a "compatability" mode that would make it produce the same (or at least almost the same) results as zxcvbn?

As a best-effort measure: are there some configurations that would get nbvcxz results closer aligned to the zxcvbn results?

@Tostino
Copy link
Collaborator

Tostino commented Sep 30, 2020

Hi there @TomLottermann, that observation it wasn't an overriding goal of nbvcxz to maintain complete compatibility with zxcvbn or the other ports is entirely correct.

When I started this project, there were no other Java ports and I really wanted this functionality for my company, and I wanted it in Java. By the time I was done with the initial version, zxcvbn4j was also released, and from my memory of looking over their code years ago, they did maintain closer compatibility with zxcvbn (at least at the time).

Now on to ways nbvcxz can be configured to generate closer output to zxcvbn:

  1. Disable the Levenshtein Distance (LD) calculation. This feature was very helpful in my analysis on helping identify passwords which were only slightly different than dictionary words but were not caught with the original implementation. This feature will be sure to cause nbvcxz to produce different results than zxcvbn for a large number of passwords.

  2. Make sure both implementations are using the same dictionaries. There are many additional leaked passwords in the nbvcxz dictionary than in zxcvbn. There are also additional dictionaries included in nbvcxz that are not in zxcvbn and vice versa. Simply different choices on what lists were important to include by default. With nbvcxz you can easily change what dictionaries are being used though, so it's easy to make the different implementations use the same dictionaries.

  3. The algorithm to find the best matches is different between nbvcxz and zxcvbn, that is likely to produce slightly different results in cases where zxcvbn is unable to find the best combination of matches due to the algorithm used. There were quite a few instances I noted that brought about the change to the algorithm used by nbvcxz where there were obviously "wrong" results for entropy based on the combination of matches because it got stuck in a local minimum. This is no longer an issue with nbvcxz, but will inherently produce different results for some passwords compared to the original algorithm used by zxcvbn. In the majority of cases both algorithms are able to figure out what the lowest entropy combination of matches on the password are, so I don't see this being too big of an issue.

Hope that helps, and i'm interested in your findings if you end up testing nbvcxz and another implementation to see how similar their outputs are over a sample of passwords.

@Tostino
Copy link
Collaborator

Tostino commented Sep 30, 2020

Another difference I thought of today is the separator match type support we have. It helps with passphrases detection a lot, but since zxcvbn doesn't support it, that would be something to also disable using the ConfigurationBuilder: setPasswordMatchers(List passwordMatchers).

@Tostino
Copy link
Collaborator

Tostino commented Oct 13, 2020

@TomLottermann I am going to close this issue, hopefully my answers were helpful. If you have any more questions or need anything clarified feel free to ask.

@Tostino Tostino closed this as completed Oct 13, 2020
@TomLottermann
Copy link
Contributor Author

Sorry for the late reply.

Thanks a ton for the detailed info! Might we worth documenting this in the README - maybe :)

@Tostino
Copy link
Collaborator

Tostino commented Oct 20, 2020

Good call, i'll re-open this as a reminder to add this info to the README.md under a "compatibility" section.

@Tostino Tostino reopened this Oct 20, 2020
@Tostino
Copy link
Collaborator

Tostino commented Oct 26, 2020

@TomLottermann I updated the readme with the info from this thread, let me know if you think anything is unclear or should be reworded if you wouldn't mind.

@Tostino Tostino closed this as completed Nov 9, 2020
@TomLottermann
Copy link
Contributor Author

Thanks for the documentation update! :)

@thomas-mc-work
Copy link

Is there a shorthand to simply remove the SeparatorMatcher from the existing list of default matchers within a configuration or during the configuration building process?

@Tostino
Copy link
Collaborator

Tostino commented Apr 13, 2021

Nope, no shorthand that I am aware of.

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