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

Only parse 64 bit int #11

Closed
wants to merge 1 commit into from
Closed

Conversation

nmhancock
Copy link

I ran into an issue while implementing balloon hashing in javascript using this as a reference.

The issue resolved down to getting different values for other, and that was because the python implementation generates a bigint from the entire buffer, rather than reading a 64 bit integer as in the C reference implementation.

Note: This commit breaks all the unit tests, so if those test vectors are known good then the code below is a mistake. But if they're not this may be an issue.

@daxpedda
Copy link
Collaborator

daxpedda commented Jul 6, 2023

For various reasons, see RustCrypto/password-hashes#232, we are not following the C reference implementation.

There are already a bunch of libraries out there that have used this implementation with the test vectors so breaking that isn't really an option unless it's for a good reason.

I don't remember all the details, but looking at the paper it's not exactly clear how this should be done. I'm trying to retrace the steps I originally took in my mind and it seems to me that how it's currently done discards the least amount of data, which is probably why I decided to go that route.

If you are interested, you could adjust the reference implementation to use this algorithm, which was approved by the author: henrycg/balloon#1 (comment).

@daxpedda daxpedda closed this Jul 6, 2023
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

Successfully merging this pull request may close these issues.

2 participants