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

Projects
None yet
4 participants
@fonica
Copy link
Contributor

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 fonica force-pushed the fonica:master branch from f3f7e1c to d19ea48 Oct 29, 2018

@fonica

This comment has been minimized.

Copy link
Contributor Author

commented Oct 29, 2018

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

@fonica

This comment has been minimized.

Copy link
Contributor Author

commented Nov 2, 2018

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

@tjschuck
Copy link
Collaborator

left a comment

@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 codahale:master Nov 8, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

tjschuck added a commit that referenced this pull request Nov 8, 2018

Add back in missing chars from Openwall test vectors
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

Add back in missing chars from Openwall test vectors
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

This comment has been minimized.

Copy link

commented Nov 9, 2018

👏 👏

tjschuck added a commit that referenced this pull request Nov 13, 2018

Add back in missing chars from Openwall test vectors
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

Add back in missing chars from Openwall test vectors
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

Add back in missing chars from Openwall test vectors
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

This comment has been minimized.

Copy link

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
You can’t perform that action at this time.