| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Openwall's crypt_blowfish library supports patching libc implementations. Part of this includes function definitions for `crypt` and `crypt_r`. Unfortunately, the function definition for `crypt_r` differs on FreeBSD >= 12 (and possibly others), which causes the extension to fail in building. This is due to the nature of how the extension works, where a C->Ruby extension is used, instead of just using a FFI such as Fiddle directly against `crypt_blowfish` and `crypt_gensalt`. The C->Ruby extension includes `ruby.h`, which in turn has a requirement on `unistd.h`, where the `crypt_r` function has already been defined, albeit with the different function signature. Defining the __SKIP_GNU token causes the `ow-crypt.h` header not to attempt defining(redefining) these two function definitions that may conflict with libc. We're not using either of them (only the re-entrant versions) so no harm done by skipping their definitions.
|
From what I gather, the core (bcrypt) functionality is in crypt_blowfish.{c,h} (plus x86.S) and the rest are meant for usage as libc. |
Sorry, something went wrong.
|
I wonder if it's the inclusion of wrapper.c in the ext folder that's making it look like the libc wrapper code is being used? In any case, both wrapper.o from $objs in extconf.rb and wrapper.c the file can be safely deleted from ext/ and the project will still build fine (with my patch). |
Sorry, something went wrong.
|
I couldn't get it to compile with that (maybe I did something wrong). I think it's just system libc (included by ruby.h) and ow-crypt.h aren't supposed to be included together because the latter supposedly functions to fulfill the lack of relevant functions in the former. I made two files (or whatever the correct term is) which proxy the required functions to ow-crypt.h and it seems to work fine as well (as in it passes the test). |
Sorry, something went wrong.
I tested your patch on FreeBSD 12 and it indeed works.
With my patch on FreeBSD 12?
That makes a bit more sense, tho I could of swore that ow-crypt.h was the header meant to be used in applications (per README). It would seem to me that applications would be expected to be linked to libc. |
Sorry, something went wrong.
The patch + removing wrapper.c & co.
yeah I was confused as well but it kind of makes sense after a while. |
Sorry, something went wrong.
The patch on it's own should be sufficient. I just tested it again thinking I had maybe a false positive. I'd be curious to know if it is indeed busted and why. Doubt it matters, but here's what I'm running: $ freebsd-version -kru 12.0-RELEASE 12.0-RELEASE 12.0-RELEASE-p1 |
Sorry, something went wrong.
|
I just tried your patch works for me. # freebsd-version -kru 12.0-RELEASE-p2 12.0-RELEASE-p2 12.0-RELEASE-p2 work/bcrypt-ruby $ ruby ./ext/mri/extconf.rb creating Makefile work/bcrypt-ruby $ make compiling ./ext/mri/bcrypt_ext.c compiling ./ext/mri/crypt_blowfish.c cc -fPIC -O2 -pipe -fstack-protector -isystem /usr/local/include -fno-strict-aliasing -fPIC -c ./ext/mri/x86.S -o x86.o compiling ./ext/mri/crypt_gensalt.c compiling ./ext/mri/wrapper.c linking shared-object bcrypt_ext.so Is there anything that needs to be done that this can be merged and released? (it breaks some projects for me) |
Sorry, something went wrong.
|
Can someone kick off Travis again? I think the 1 failure was a network timeout and not related to this patch. |
Sorry, something went wrong.
|
@adam12 apparently you can close an reopen the pull request (travis-ci/travis-ci#576 (comment)) which should retrigger the pull request |
Sorry, something went wrong.
@fliiiix right you were! |
Sorry, something went wrong.
|
@tjschuck what needs to be done to get this merged? |
Sorry, something went wrong.
|
Same issue here, FreeBSD-12.
|
Sorry, something went wrong.
|
For us bcrypt gets fetched & installed as part of bundle install. Any way to dig deeper and patch bcrypt source code before installing it? When I try to patch the expanded file /path/to/vendor/bundle/ruby/2.5.0/gems/bcrypt-3.1.10/ext/mri/extconf.rb with $defs << "-D__SKIP_GNU" and run bundle install again it gets overwritten with the unpatched copy first. |
Sorry, something went wrong.
|
I just point bcrypt to my own patched fork wherever I use it until this gets merged (or fixed in some other way). |
Sorry, something went wrong.
% pwd
/path/to/www/vendor/bundle/ruby/2.5.0/gems/bcrypt-3.1.10
% ruby ./ext/mri/extconf.rb
creating Makefile
% make
compiling ./ext/mri/wrapper.c
./ext/mri/wrapper.c:188:7: error: conflicting types for 'crypt_r'
char *crypt_r(__CONST char *key, __CONST char *setting, void *data)
^
/usr/include/unistd.h:499:7: note: previous declaration is here
char *crypt_r(const char *, const char *, struct crypt_data *);
^
1 error generated.
*** Error code 1
Stop.
make: stopped in /path/to/www/vendor/bundle/ruby/2.5.0/gems/bcrypt-3.1.10
% freebsd-version -kru
12.0-RELEASE-p3
12.0-RELEASE-p3
12.0-RELEASE-p3
|
Sorry, something went wrong.
Sorry, but how do you do that? I'm assuming you're using a rails project. |
Sorry, something went wrong.
|
Something like this in Gemfile (or make your own fork if you want use specific version and adjust accordingly) gem 'bcrypt', git: 'https://github.com/adam12/bcrypt-ruby', ref: '613daca' |
Sorry, something went wrong.
|
Sorry, but aren't you referring to adam's patch which you (and myself) had problems building with? |
Sorry, something went wrong.
|
That was referring to
The original patch (this PR) works fine (also it's the first thing I tried before even looking at this). |
Sorry, something went wrong.
|
Wow! I removed mentions of wrapper.c & wrapper.o from Makefile & ext/mri/Makefile (without deleting wrapper.c itself) and now the build succeeded! Thanks a lot! % make linking shared-object bcrypt_ext.so % |
Sorry, something went wrong.
|
Building it one thing, installing it is another ) I tried copying the resultant bcrypt_ext.so to vendor/bundle/ruby/2.5.0/extensions/x86_64-freebsd-12/2.5.0-static/bcrypt-3.1.10 and created an empty file gem.build_complete there, like in other successfully built gems there, but bundle check still thinks bcrypt is missing :( |
Sorry, something went wrong.
|
What are you trying to do? If it's just installing bcrypt for rails or other ruby app using bundler/Gemfile, have you tried switching (by updating Gemfile) to the patched version I mentioned above? |
Sorry, something went wrong.
|
Replacing that line locally is harder than I thought :) You are trying to install in deployment mode after changing your Gemfile. Run `bundle install` elsewhere and add the updated Gemfile.lock to version control. The list of sources changed The dependencies in your gemfile changed You have added to the Gemfile: * source: https://github.com/adam12/bcrypt-ruby (at 613daca) * bcrypt You have deleted from the Gemfile: * bcrypt (~> 3.1.10) Most of our other servers still use FreeBSD 10.x where there's no problem building bcrypt. |
Sorry, something went wrong.
|
As written there, you need to either update the lockfile somewhere else or (temporarily) disable deployment mode. |
Sorry, something went wrong.
|
Merge? |
Sorry, something went wrong.
|
@tjschuck @tenderlove were there any concerns with this PR? or anything need clarifying? Thanks! |
Sorry, something went wrong.
|
🎉 nice |
Sorry, something went wrong.
|
Is there an ETA when this will be in a release? I would love to upgrade the FreeBSD package. |
Sorry, something went wrong.
|
@tenderlove I hope there'll be a new release soon! |
Sorry, something went wrong.
|
@metalefty tbh I didn't want to step on @tjschuck's toes. @tjschuck do you have time for a release? If not, I will do it 😊 |
Sorry, something went wrong.
|
@tenderlove We FreeBSD users really appreciate if you guys make a new release 💯 |
Sorry, something went wrong.
|
@metalefty seems like @tjschuck is busy, so I'll just ship it today. Sorry this is taking so long |
Sorry, something went wrong.
|
I shipped 3.1.13. Please let me know if there's anything else we need! |
Sorry, something went wrong.
|
Thanks! bcrypt 3.1.13 got successfully installed without an error on my FreeBSD 12 host. |
Sorry, something went wrong.
|
Thanks you all! |
Sorry, something went wrong.
|
Thank you for the release 👍 |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Defining the SKIP_GNU token as a compile flag forces the build of crypt_blowfish not to attempt redefinition of crypt and crypt_r which possibly conflict with the system libc depending on their function definition. This allows bcrypt-ruby to build again on FreeBSD >= 12.0.
I spent a bunch of time debugging the real reason for why this occurred:
The fine FreeBSD folk just patched out the conflicting function definitions from the vendored C source.
Solardiz suggested that bcrypt-ruby was interacting with crypt_blowfish incorrectly, which is entirely possible (I'll humbly defer to Solar Designer here) but I didn't see how/why during my investigation.
Compiling bcrypt_blowfish independently works fine on FreeBSD 12. It only fails when linked against anything that's included unistd.h.
bcrypt-ruby doesn't use crypt or crypt_r so I think we're safe defining this token and building without those function definitions. The only downside I see is we're now dependent on this token which might be internal only. I don't see it disappearing so I suspect the risk is minimal.