Enable compact index when OpenSSL FIPS mode is enabled but not active #5440

wjordan · 2017-02-16T18:57:26Z

Fixes #5433. Since there is no easy accessor in Ruby to detect whether or not FIPS mode is currently active, the best approach I could come up with is to fork a separate process and attempt to generate a build MD5 object as a test of whether MD5 module is currently available.

Because fork approach won't work on some platforms (JRuby, Windows etc), md5_supported? returns false on any platforms where FIPS mode is enabled and Process.respond_to?(:fork) is false.

I've added a spec that simulates behavior when OpenSSL FIPS mode is active - an error message is output to STDERR and the process is killed with the ABRT signal.

colby-swandale · 2017-02-16T20:31:17Z

How does this behave on Windows environments? Specifically when you're writing to /dev/null

wjordan · 2017-02-16T21:35:15Z

I believe that md5_supported? will continue to return false on windows environments, assuming that Process.respond_to?(:fork) returns false in that environment. However, because Bundler's tests don't cover the Windows platform (at least doesn't appear to at first glance) that isn't something I can easily confirm.

segiddins · 2017-02-16T22:13:58Z

lib/bundler/fetcher/compact_index.rb

@@ -122,14 +122,24 @@ def call(path, headers)
      end

      def md5_available?
+        return true unless fips_enabled? && Process.respond_to?(:fork)
+        pid = fork do


I don't think forking on every Bundler process is going to be acceptable performance-wise (in addition to all the platforms that aren't fork-safe) -- is there no way to determine in-process if the MD5 digest is available?

Platforms that aren't fork-safe will not reach this line of code because Process.respond_to?(:fork) will be false.

Performance-wise, I expect a process fork to perform better than disabling the compact-index entirely, so for the case where FIPS mode is available but not active this should be a performance improvement. The tradeoff is that this causes the case where FIPS mode is active to perform the fork.

As far as I can tell, there is no way via Ruby's stdlib to determine in-process if the MD5 digest is available. The process is aborted as soon as OpenSSH's MD5 module is invoked, so in order to detect this failure without aborting the parent process, it's necessary to perform the test on a child process.

wjordan · 2017-02-16T23:30:12Z

I don't think this PR is a great solution, but it demonstrates the best I can do to extend #5222's detection-based approach to the underlying FIPS compatibility issue (#4989). I would actually prefer to see #5222 reverted and pursue an alternate solution to FIPS mode support in Bundler.

colby-swandale · 2017-02-16T23:41:34Z

The solution to #5222 is the best we have currently, some users were unable to use Bundler when FIPS was enabled so loosing Compact Index is a suitable compromise. I'll ask the OpenSSL gem maintainers if we can work to improve the API to better detect FIPS.

colby-swandale · 2017-02-16T23:56:48Z

Looking at the OpenSSL gem src there is a method called .fips_mode that looks like it can be used to check if FIPS is actually enabled.

simi · 2017-02-16T23:58:58Z

@colby-swandale it was introduced in Ruby 2.0 and it is just setter (fips_mode = true).

You can check if fips mode is enabled by this code:

require 'openssl'
require 'fiddle'
puts Fiddle::Function.new(Fiddle.dlopen(nil)["FIPS_mode"], [], Fiddle::TYPE_INT).call()

I can try to open PR to Ruby OpenSSL to add this as OpenSSL.fips_mode getter.

rhenium · 2017-02-17T04:25:49Z

Alternatively OpenSSL::Digest which uses the EVP interface can be used.

def md5_available?
  return true unless defined?(OpenSSL)

  OpenSSL::Digest::MD5.digest("")
  true
rescue OpenSSL::Digest::DigestError
  false
end

colby-swandale · 2017-02-17T04:34:38Z

@rhenium Does that catch the C error that gets printed? See #4989

md5_dgst.c(80): OpenSSL internal error, assertion failed: Digest MD5 forbidden in FIPS mode!

rhenium · 2017-02-17T05:12:32Z

@rhenium Does that catch the C error that gets printed? See #5222

Yes. That code will not reach MD5_Init() that aborts the process.

wjordan · 2017-02-17T15:27:23Z

Thanks @rhenium and @simi! I didn't know about those two Ruby alternatives. I've updated the PR to use OpenSSL::Digest::MD5 to test MD5 availability- this gives a more general MD5 check that doesn't require Bundler to fork a separate process or even reference the FIPS-mode API directly.

segiddins · 2017-02-17T15:44:26Z

spec/bundler/fetcher/compact_index_spec.rb

+              # OpenSSL writes to STDERR and kills the current process with SIGABRT
+              # when FIPS mode prevents MD5 from being used.
+              $stderr.write "Digest MD5 forbidden in FIPS mode!"
+              Process.kill("ABRT", Process.pid)


Is this still necessary?

No, this is not necessary anymore for the spec to run correctly, I can remove.

segiddins · 2017-02-17T22:18:24Z

Looks good to me!
@indirect r?

indirect · 2017-02-17T23:03:38Z

This is awesome teamwork, everyone. Thanks so much!

@bundlerbot r+

bundlerbot · 2017-02-17T23:03:39Z

📌 Commit 21b3358 has been approved by indirect

bundlerbot · 2017-02-18T11:07:36Z

⌛ Testing commit 21b3358 with merge 13f4cc1...

Enable compact index when OpenSSL FIPS mode is enabled but not active Fixes #5433. Since there is no easy accessor in Ruby to detect whether or not FIPS mode is currently active, the best approach I could come up with is to `fork` a separate process and attempt to generate a build MD5 object as a test of whether MD5 module is currently available. Because `fork` approach won't work on some platforms (JRuby, Windows etc), `md5_supported?` returns `false` on any platforms where FIPS mode is enabled and `Process.respond_to?(:fork)` is `false`. I've added a spec that simulates behavior when OpenSSL FIPS mode is active - an error message is output to STDERR and the process is killed with the `ABRT` signal.

bundlerbot · 2017-02-18T12:56:38Z

☀️ Test successful - status-travis
Approved by: indirect
Pushing 13f4cc1 to master...

Enable compact index when OpenSSL FIPS mode is enabled but not active Fixes #5433. Since there is no easy accessor in Ruby to detect whether or not FIPS mode is currently active, the best approach I could come up with is to `fork` a separate process and attempt to generate a build MD5 object as a test of whether MD5 module is currently available. Because `fork` approach won't work on some platforms (JRuby, Windows etc), `md5_supported?` returns `false` on any platforms where FIPS mode is enabled and `Process.respond_to?(:fork)` is `false`. I've added a spec that simulates behavior when OpenSSL FIPS mode is active - an error message is output to STDERR and the process is killed with the `ABRT` signal. (cherry picked from commit 13f4cc1)

wjordan force-pushed the fips_enabled_compact_index branch from f015d4d to fa448c7 Compare February 16, 2017 21:13

segiddins reviewed Feb 16, 2017

View reviewed changes

Enable compact index when OpenSSL FIPS mode is enabled but not active

873bac1

wjordan force-pushed the fips_enabled_compact_index branch from fa448c7 to 873bac1 Compare February 16, 2017 23:18

wjordan mentioned this pull request Feb 16, 2017

Bundler disabling Compact Index on all FIPS-supported OpenSSL installations #5433

Closed

wjordan mentioned this pull request Feb 16, 2017

FIPS compatibility #4989

Closed

simplify md5_available? test using OpenSSL::Digest:MD5.digest

9c389ff

segiddins reviewed Feb 17, 2017

View reviewed changes

remove unused Process.kill mock in FIPS-mode spec

21b3358

segiddins added this to the 1.14.5 milestone Feb 17, 2017

bundlerbot merged commit 21b3358 into rubygems:master Feb 18, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable compact index when OpenSSL FIPS mode is enabled but not active #5440

Enable compact index when OpenSSL FIPS mode is enabled but not active #5440

wjordan commented Feb 16, 2017 •

edited

Loading

colby-swandale commented Feb 16, 2017 •

edited

Loading

wjordan commented Feb 16, 2017

segiddins Feb 16, 2017

wjordan Feb 16, 2017

wjordan commented Feb 16, 2017 •

edited

Loading

colby-swandale commented Feb 16, 2017

colby-swandale commented Feb 16, 2017

simi commented Feb 16, 2017 •

edited

Loading

rhenium commented Feb 17, 2017

colby-swandale commented Feb 17, 2017 •

edited

Loading

rhenium commented Feb 17, 2017

wjordan commented Feb 17, 2017

segiddins Feb 17, 2017

wjordan Feb 17, 2017

segiddins commented Feb 17, 2017

indirect commented Feb 17, 2017

bundlerbot commented Feb 17, 2017

bundlerbot commented Feb 18, 2017

bundlerbot commented Feb 18, 2017

Enable compact index when OpenSSL FIPS mode is enabled but not active #5440

Enable compact index when OpenSSL FIPS mode is enabled but not active #5440

Conversation

wjordan commented Feb 16, 2017 • edited Loading

colby-swandale commented Feb 16, 2017 • edited Loading

wjordan commented Feb 16, 2017

segiddins Feb 16, 2017

Choose a reason for hiding this comment

wjordan Feb 16, 2017

Choose a reason for hiding this comment

wjordan commented Feb 16, 2017 • edited Loading

colby-swandale commented Feb 16, 2017

colby-swandale commented Feb 16, 2017

simi commented Feb 16, 2017 • edited Loading

rhenium commented Feb 17, 2017

colby-swandale commented Feb 17, 2017 • edited Loading

rhenium commented Feb 17, 2017

wjordan commented Feb 17, 2017

segiddins Feb 17, 2017

Choose a reason for hiding this comment

wjordan Feb 17, 2017

Choose a reason for hiding this comment

segiddins commented Feb 17, 2017

indirect commented Feb 17, 2017

bundlerbot commented Feb 17, 2017

bundlerbot commented Feb 18, 2017

bundlerbot commented Feb 18, 2017

wjordan commented Feb 16, 2017 •

edited

Loading

colby-swandale commented Feb 16, 2017 •

edited

Loading

wjordan commented Feb 16, 2017 •

edited

Loading

simi commented Feb 16, 2017 •

edited

Loading

colby-swandale commented Feb 17, 2017 •

edited

Loading