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

Update blowfish imprementation to latest version #182

Merged
merged 2 commits into from Nov 8, 2018

Conversation

fonica
Copy link
Contributor

@fonica fonica commented Oct 29, 2018

Changes:

  • Update the C implementation to http://openwall.com/crypt/crypt_blowfish-1.3.tar.gz
  • Update the Java implementation to use the latest BCrypt.java from spring-security.
  • add more tests from the C library test suite
  • updated lib/bcrypt/engine.rb to pass the secret as java bytes; it seems jruby messes up the encoding for certain bytes if the secret is passed as a string.

This should help close some old PRs and issues addressing this problem. The main one I think is PR #91

@fonica
Copy link
Contributor Author

fonica commented Oct 29, 2018

build was timing out on rbx-3 while installing gems, it seems this did the trick

@fonica
Copy link
Contributor Author

fonica commented Nov 2, 2018

@tjschuck @tenderlove any chance someone will look at this? (Not sure if there are any active maintainers on the repo)

Copy link
Collaborator

@tjschuck tjschuck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fonica Thank you — this is excellent! I've been waiting for those JRuby changes to land in Spring for a while, so I'm glad to see it finally happened!

@tjschuck tjschuck merged commit 1980bc6 into bcrypt-ruby:master Nov 8, 2018
tjschuck added a commit that referenced this pull request Nov 8, 2018
These were deleted in #182, but are a part of the Openwall test vectors.  They’re important because they’re actually making the string longer than 72 characters, which is the test that the vector is going for.
tjschuck added a commit that referenced this pull request Nov 9, 2018
tjschuck added a commit that referenced this pull request Nov 9, 2018
These were deleted in #182, but are a part of the Openwall test vectors.  They’re important because they’re actually making the string longer than 72 characters, which is the test that the vector is going for.
@pichot
Copy link

pichot commented Nov 9, 2018

👏 👏

tjschuck added a commit that referenced this pull request Nov 13, 2018
These were deleted in #182, but are a part of the Openwall test vectors.  They’re important because they’re actually making the string longer than 72 characters, which is the test that the vector is going for.
tjschuck added a commit that referenced this pull request Nov 15, 2018
These were deleted in #182, but are a part of the Openwall test vectors.  They’re important because they’re actually making the string longer than 72 characters, which is the test that the vector is going for.
tjschuck added a commit that referenced this pull request Nov 15, 2018
These were deleted in #182, but are a part of the Openwall test vectors.  They’re important because they’re actually making the string longer than 72 characters, which is the test that the vector is going for.
@mohamedhafez
Copy link

mohamedhafez commented Jun 1, 2019

updated lib/bcrypt/engine.rb to pass the secret as java bytes; it seems jruby messes up the encoding for certain bytes if the secret is passed as a string.

@fonica would this fix make it so that secrets hashed with older gem versions produce a different hash than the one produced by this new, fixed version in some cases? If so, how common would those cases be do you think?

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.

None yet

4 participants