FIPS compatibility

[op-ct]

bundler will fail when run in FIPS mode because it uses MD5:

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

Two changes are needed to fix this (mostly in the vendor code):

• It must be possible to use an alternative to Digest::MD5.hexdigest using a cipher that is permitted by FIPS-140-2 (e.g., Digest::SHA256.hexdigest).
  • This will affect the files:
    • lib/bundler/vendor/compact_index_client/lib/compact_index_client/cache.rb
    • lib/bundler/vendor/compact_index_client/lib/compact_index_client/updater.rb
    • lib/bundler/vendor/thor/lib/thor/runner.rb
    • lib/bundler/source/rubygems/remote.rb
• require "digest/md5"' should not be required by default, since that will also fail on a system with FIPS mode enabled.
  • This affects the files:
    • lib/bundler/vendor/compact_index_client/lib/compact_index_client/cache.rb
    • lib/bundler/vendor/thor/lib/thor/runner.rb

[segiddins]

What is FIPS mode? Bundler intentionally uses MD5 as a digest because that is what the server sends as the ETAG for compact index files -- it is in no way used as a cryptographic hash

[op-ct]

Hi @segiddins,

"FIPS mode" is a feature of the Linux kernel (and Windows) that only permits the use of cryptographic algorithms validated under the NIST standard FIPS-140-2. When FIPS mode is enabled, OpenSSL will recognize this and deny the use of non-validated algorithms.

MD5 is notably not a validated algorithm―so no matter what MD5 is being used for, OpenSSL will barf if an application tries to use it when FIPS mode is enabled.

[segiddins]

I don't see us being able to make this change, unfortunately, as it would be backwards-incompatible, requiring changing the hash used as the etag on the server

[coilysiren]

In addition to that ^, I get the feeling that switching to using a different security hashing function for etags would be a temporary fix. That is, some years from now there'll be a FIPS2 that requires Digest::SHA2048.hexdigest. This is fine for security issues, but for cache validation it just seems annoying.

[trevor-vaughan]

@segiddins and @lynnco I certainly get that this is a rolling issue (and yes FIPS 140-3 is in the works) but kernel level enforcement of this feature means that anything using Bundler will actively terminate without warning (and a healthy OpenSSL garbage error to boot).

At least being able to set this somewhere (or detect it via the underlying SSL lib) and use a known good hash, would be very helpful. BUNDLER_HASH=sha1 or some such thing.

At present, even doing a "require 'md5'" will destroy things in a horrible fashion and we just need a way to work around this hopefully without forking bundler for a 4 line change.

[segiddins]

We cant change it though, since the hash has to be what the server is using

[trevor-vaughan]

Maybe I'm missing something.

What exactly is this hitting? rubygems.org seems to be defaulting to sha256 across the board.

Probably in relation to this (which was probably related to md5) http://blog.rubygems.org/2016/04/06/gem-replacement-vulnerability-and-mitigation.html

[segiddins]

No, this is the etag, which is simply a header used for checking whether the server can return a 304 NOT MODIFIED

[trevor-vaughan]

FYI, I just switched everything over to require 'digest' and Digest::SHA256 and everything ran just fine.

[op-ct]

Thanks for patiently pointing out etags several times, @segiddins―we got there in the end. 😛

I wasn't familiar with how etags were implemented, but after reading through RFC 7616 it looks like the root of our problem is that the digest algorithm for the etag should be negotiated with the server, with a preference for SHA-512/256 over MD5:

• Digest Operation: https://tools.ietf.org/html/rfc7616#section-3.2.
• Example of server / client interaction: https://tools.ietf.org/html/rfc7616#section-3.9

If bundler deferred its MD5 operations (including require "digest/md5") until it was certain the server didn't support accept anything better, it should satisfy the needs of both legacy servers and FIPS-mode clients.

[indirect]

I don't even know how to respond to this, honestly.

First: Bundler is very serious about backwards compatibility. We're not going to change anything that means current users will no longer be able to continue doing the same thing.

Second, the RFC that you just linked to has nothing to do with Bundler whatsoever. We aren't using hashing for HTTP Digest Authentication.

Finally, we use MD5 to generate content checksums sent between Bundler and gem servers. That means migrating off of MD5 would mean changing both the client and the server, and breaking every single existing client. Last time I checked, there are roughly 120 million downloads of the existing client.

As a result of those two things, the answer is no. No, we can't delay requiring MD5, and no, we can't switch to another algorithm and fall back on MD5 if SHA256 isn't available.

If you choose to run Bundler on machines where MD5 is not available, you're welcome to patch Bundler to use a different algorithm. Please keep in mind that if you choose to do that, the rubygems.org server will still be using MD5, and so the checksums that Bundler uses to know that it was able to successfully download gem information will no longer function.

[openface]

I'm curious if anyone has created a fork of Bundler for this purpose. We're currently unable to deploy our Ruby applications in fed space due to FIPS 140-2 regulations.

[segiddins]

I think an acceptable solution we could merge would be to skip the compact index fetcher if it fails to load due to that require

[trevor-vaughan]

@openface If you go through the code, there are two places where MD5 is used and they're both in upstream projects. You can 100% replace both of these calls with SHA256+ and nothing bad happens. We've been running it that way for a while now and neither of those operations has anything to do with the reason that it couldn't be replaced that was mentioned in this thread.

We got some of the upstream projects to fix their materials so it'll show up here eventually.

[segiddins]

No you can't replace it with SHA256, it'll cause bundler to redownload every single compact index file on every install, as we pointed out upthread

[segiddins]

Something like this should probably be sufficient

diff --git a/lib/bundler/fetcher/compact_index.rb b/lib/bundler/fetcher/compact_index.rb
index 5d703a3..a498ef6 100644
--- a/lib/bundler/fetcher/compact_index.rb
+++ b/lib/bundler/fetcher/compact_index.rb
@@ -5,8 +5,6 @@ require "bundler/worker"
 module Bundler
   class Fetcher
     class CompactIndex < Base
-      require "bundler/compact_index_client"
-
       def self.compact_index_request(method_name)
         method = instance_method(method_name)
         undef_method(method_name)
@@ -61,6 +59,11 @@ module Bundler
       compact_index_request :fetch_spec
 
       def available?
+        begin
+          require "bundler/compact_index_client"
+        rescue # whatever exception having fips mode enable raises
+          return false
+        end
         user_home = Bundler.user_home
         return nil unless user_home.directory? && user_home.writable?
         # Read info file checksums out of /versions, so we can know if gems are up to date

[trevor-vaughan]

@segiddins As a note for the future, providing a link to the following would have saved me a LOT of confusion http://andre.arko.net/2014/03/28/the-new-rubygems-index-format/.

That said, where do we go to ask the RubyGems.org people to update that standard to allow the specification of different checksums? They're really easy to document and you could implement multiples to allow systems that can only handle certain checksums to work properly.

[segiddins]

Given that André and I implemented the format, you'll have to believe us when we say that supporting multiple checksums is non-trivial and not a project we're interested in tackling at this time

[colby-swandale]

@trevor-vaughan Please be considerate and appreciative of the already given expertise help rather then blaming them for not linking you a blog post. They're here on their own time and are kindly helping you the best they can.

[trevor-vaughan]

@colby-swandale This is the moment where I regret that text-based formats don't reflect facial expressions well.

@segiddins First, I very much appreciate the time and effort that you put into this software! It makes life for me, and many others much easier.

While I understood the statements that were written, I didn't understand that the crux of the issue was the protocol that was linked in the post that I found today. I just didn't understand that the fundamental issue is (I think) the load on the servers that would be caused by maintaining this fork!

Technologically, yes, it will work just fine but, if everyone in the world stops using MD5, we're going to bring the upstream servers to their knees. After finding that post, I now think that I understand this and understand where the Hash is located and can potentially start looking at working on PRs that can help fix the issue for the community at large. This is Open Source, and that is awesome! I apologize that I didn't actually understand the issue, even after looking at the code but, in trying to help find a solution, specifications of the underlying API are very useful.

My frustration came in getting an explanation of something that I did not realize was an internally generated API as opposed to the trails that were covered based on terms that had different meanings based on various RFC definitions, etc...

Finally, @openface if I read that article correctly, the code change will work for your purposes, but you're going to be putting a great deal of load on the upstream Gem server (Bundler metadata server?) every time you do a bundle update. Therefore, I would only suggest doing this if you have a Rubygems.org mirror to work from so that you don't swamp the upstream servers.

I would like to try to help solve this issue, in code, if possible. Pretty much all of my users want to use Bundler and, likewise, they need it to run in FIPS mode.If it can't do that, I think that my only recourse is to set up Gem mirrors so that we don't add too much load upstream. Is this correct?

[indirect]

@trevor-vaughan if you report exactly what exception is caused by FIPS mode, we can patch Bundler to not use the compact index at all. That will cause Bundler to fall back on the full index, which slows down bundle install but works otherwise.

[trevor-vaughan]

@indirect Apologies that it took this long to get here, but this is what I think needs to be done. Also, thank you for your patience.

The issue with handling an exception, is that FIPS mode failing doesn't throw an exception, instead it fails in the heart of OpenSSL and just destroys your running process outright.

Example:

begin
  require 'digest/md5'
  Digest::MD5.hexdigest('foo')
rescue Exception => e
  puts e.class
  puts e.to_s
  puts e.backtrace
end

Output:

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

Given this, I think that the appropriate course of action is to check OpenSSL itself and then perform your action based on that.

This appears to work successfully in my quick test:

 if OpenSSL::OPENSSL_FIPS
    puts 'Do the long version'
  else
    # The 'require' needs to be inside the else
    require 'digest/md5'

    Digest::MD5.hexdigest('foo')
  end

You may want to add a const_get() in there around the OpenSSL materials just in case someone isn't using OpenSSL for some reason (unlikely though).

[breisig]

I am having the same issue when running bundle on a server in fips mode. Can we get that patch/fix submitted and a new version hopefully released soon with it? Thanks.

[segiddins]

@breisig patch already submitted at #5222

[breisig]

@segiddins Awesome, I am getting anxious when your patch will be merged with master AND when a new bundler release will be made.

[wjordan]

The fix in PR #5222 incorrectly disables the compact-index for environments where OpenSSL is compiled with FIPS support even though it is not currently enabled (see issue #5433).

The only way I've been able to reliably detect whether FIPS mode is actually active is to fork the process and detect whether invoking the MD5 module causes the child process to abort (see PR #5440). I don't think this is a good solution, though, I would prefer to replace #5222 with an alternative solution to this issue.

Here are two alternative solutions I can think of:

1. Add a configuration option (e.g., --disable-compact-index or --compact-index=false) that allows the use of the compact-index API to be optionally disabled. Add documentation recommending that FIPS-mode users set this option in their Bundler config.
2. Migrate the compact-index info_checksum used by Rubygems.org away from MD5. Either switch to a FIPS-validated alternative such as SHA256, or if it's truly the case that the hash is only used as a checksum and not as a secure one-way hash for securing information, you can use Zlib#crc32 which makes it unambiguous that the info_checksum hash is only being used for file integrity, and should be available even when FIPS mode is enabled.
   • Note that due to versions checksums do not match /info for existing gems rubygems.org#1566, a great majority (~85%) of the info_checksum data currently in rubygems.org is currently incorrect, so the concern about backwards-compatibility is not really an issue here, as most of the etags are already wrong anyway.

[wjordan]

Looks like there's a alternative interface (OpenSSL::Digest::MD5) that allows detecting when FIPS mode disables MD5 without aborting the process, which should work for now.

Note that disabling the compact index format whenever MD5 is unavailable will prevent the old rubygems index format from being deprecated in the future. Unless it's an explicit goal to never deprecate the legacy format, I think that migrating the compact-index format's checksum algorithm away from MD5 (towards either SHA256 or CRC32, or a configurable algorithm) would still be a better future-proof solution.

[cwjenkins]

@wjordan I added OpenSSL.fips_mode to check if fips is 'enabled'. Not sure when it will make its way down. ruby/openssl@2958067
Regardless, the remainder of the gem uses Digest vs OpenSSL::Digest which will cause issues in FIPS if Ruby is built against openssl. I'm looking to update ruby's digest to use EVP interface within Digest as well to avoid the following error openssl/openssl@65300dc#diff-49ae58fe4e39a3aaeb4f2b6edb566262R562