Bug 1537140 - rubygem-bcrypt does not work (is FTBFS)
Summary: rubygem-bcrypt does not work (is FTBFS)
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: libxcrypt
Version: rawhide
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Björn Esser (besser82)
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: 1537261
TreeView+ depends on / blocked
 
Reported: 2018-01-22 14:17 UTC by Vít Ondruch
Modified: 2018-02-16 09:48 UTC (History)
2 users (show)

Fixed In Version: rubygem-bcrypt-3.1.11-8.fc28
Clone Of:
Environment:
Last Closed: 2018-02-16 08:34:31 UTC
Type: Bug
Embargoed:


Attachments (Terms of Use)

Description Vít Ondruch 2018-01-22 14:17:47 UTC
Description of problem:
rubygem-bcrypt is broken by libxcrypt


Version-Release number of selected component (if applicable):
$ rpm -q libxcrypt
libxcrypt-4.0.0-0.203.20180120git3436e7b.fc28.x86_64


How reproducible:


Steps to Reproduce:
1.
2.
3.

Actual results:
The rubygem-bcrypt build fails.

What draw my attention from the build log is this test failure failure:

~~~
  2) Generating BCrypt hashes should be interoperable with other implementations
     Failure/Error: expect(BCrypt::Engine.hash_secret(secret, salt)).to eql(test_vector)
       expected: "$2a$05$CCCCCCCCCCCCCCCCCCCCC.E5YPO9kmyuRGyh0XouQYb4YMJKvyOeW"
            got: "$2a$05$CCCCCCCCCCCCCCCCCCCCC.E5YPO9kmyuRGyh0XouQYb4YMJKvyOeW\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00...x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"
       (compared using eql?)
     # /usr/share/gems/gems/rspec-support-3.7.0/lib/rspec/support.rb:97:in `block in <module:Support>'

... snip ...
~~~

As you can see, there are some unexpected null characters included at the end of the string. This is the code where libxcrypt is called:

https://github.com/codahale/bcrypt-ruby/blob/master/ext/mri/crypt.c#L37

To me, it seems that the crypt_ra function does not sanitize either the "data" parameter or probably the "size" is wrong.

Expected results:

The rubygem-bcrypt test suite and the build must pass.


Additional info:


[1] https://apps.fedoraproject.org/koschei/package/rubygem-bcrypt

Comment 1 Vít Ondruch 2018-01-30 08:41:47 UTC
Ping? Could you please look into it?

Comment 2 Björn Esser (besser82) 2018-02-16 08:34:31 UTC
https://koji.fedoraproject.org/koji/taskinfo?taskID=25086741

Comment 3 Vít Ondruch 2018-02-16 08:41:18 UTC
Hi Björn,

Thx for fixing this. Does it mean that the libxcrypt is not 100% compatible with libcrypt and changes the return values intentionally?

Comment 4 Björn Esser (besser82) 2018-02-16 09:41:17 UTC
(In reply to Vít Ondruch from comment #3)
> Hi Björn,
> 
> Thx for fixing this. Does it mean that the libxcrypt is not 100% compatible
> with libcrypt and changes the return values intentionally?


Hi Vit,

you're welcome!  Well, the actual problem was not about so much about libxcrypt;  there are no changes libxcrypt makes to any return value, but some internal implementations are different from glibc's libcrypt:

On pure C-level it doesn't make any difference to the consuming code (return values are 100% binary compatible on this level and are conforming to ANSI-C standards).

In this case the C-code of the built ruby-extension was the real problem, since it made the blunt assumption that the internal implementation of OpenWall's reference implementation would be the same for *any* other implementation as well:

 * The `bc_salt()` function was treating the return value of `crypt_gensalt()`
   as binary data instead as a valid C-string and thus it was full of
   trailing 0-bytes (libxcrypt uses a 192 byte sized char buffer as return
   value, which is perfectly valid for C-code as a 0-byte in a C-string
   terminates it).  This broke most of the code and tests.

 * The `bc_crypt()` function was discarding the return value of `crypt()` and
   instead assumed the opaque (and thus implementation dependant) void-pointer
   (void can be any structure of data by the C-standard) to `data` would just
   return a valid C-string, when in fact the return value *is* a valid
   C-string with the wanted information for all implementations of `crypt()`
   implied by POSIX.

I hope that this clarifies, what my patch does, a bit.

Comment 5 Vít Ondruch 2018-02-16 09:48:38 UTC
Thx a lot for insights.


Note You need to log in before you can comment on or make changes to this bug.