active_storage fips 140-2 issue

[da-development-id]

Tried using active_storage to handle file upload, but encountered an issue when FIPS mode is enforced.

Tried the following configuration option with same issue:
Rails.application.config.active_support.use_sha1_digests = true

rail 5.2.1

ruby 2.5.0

[rafaelfranca]

Can you please provide a sample application that reproduces the error?

[da-development-id]

The issue is similar to what was described in #31203 and #31289.

The use of Digest::MD5 results in 'OpenSSL internal error, assertion failed: Digest MD5 forbidden in FIPS mode! Aborted'.

I understand why the MD5 checksum is being generated in the active storage blob entry and how it's being used, but was wondering if there could be other options for generating the checksum and maybe disable the verification with other services (like S3 since it only has a MD5 option) if it's being used.

[rafaelfranca]

oh. I see. Yeah, active storage is using MD5 internally. @georgeclaghorn do you think it is possible to use something that is FIPS 140-2 friendly like SHA1?

[georgeclaghorn]

We use MD5 because every service accepts an MD5 checksum. Support for alternative checksum algorithms is pretty sparse, unfortunately—Google Cloud Storage supports CRC32C, but as far as I can tell, no other services do.

It’d probably be easiest to add a config option that disables checksumming.

[rails-bot]

This issue has been automatically marked as stale because it has not been commented on for at least three months.
The resources of the Rails team are limited, and so we are asking for your help.
If you can still reproduce this error on the 5-2-stable branch or on master, please reply with all of the information you have about it in order to keep the issue open.
Thank you for all your contributions.

[deepakmahakale]

Any updates on this issue?

[josecoelho]

I believe we should be able to disable checksum using MD5. There are cases where the integrity of the uploaded file is not that important and we have no way to use it while FIPS is enabled.

We did a monkey patch to disable this checksum. I will try to create a PR adding a config to ActiveStorage. This might be useful for anyone else in the same situation.

# config/initializers/active_storage_disable_md5_patch.rb


# This is a monkey-patch to disable checksum calculations for active storage uploads. It uses MD5 and there's no 
# way to disable it from
#
# Our servers have [FIPS](https://en.wikipedia.org/wiki/Federal_Information_Processing_Standard) enabled 
# and MD5 is not allowed.
#
# This is not a breaking change as all services are not supposed to check for checksum unless a checksum is sent.
# And the migration db/migrate/20201109013136_disable_active_storage_checksum.rb is responsible to allow null 
# values on the checksum column
#
# Be aware that you might need to update this patch after upgrading to rails 6+
# https://github.com/rails/rails/blob/6-1-stable/activestorage/app/models/active_storage/blob.rb
ActiveSupport::Reloader.to_prepare do
  # Please check https://github.com/rails/rails/blob/5-2-stable/activestorage/app/models/active_storage/blob.rb
  ActiveStorage::Blob.redefine_method :compute_checksum_in_chunks do |_|
    nil
  end
end

A migration to allow null checksum in active_storage_blobs table

# db/migrate/20201109013136_disable_active_storage_checksum.rb

class DisableActiveStorageChecksum < ActiveRecord::Migration[5.2]
  def change
    # this is required to be able to store null checksum, as we are not able to calculate MD5 
    # on servers with FIPS enabled
    change_column_null :active_storage_blobs, :checksum, true
  end
end

And a comment to check this monkey patch on every upgrade

# Gemfile

# Warning: check config/initializers/active_storage_disable_md5_patch.rb before upgrading to rails 6
gem 'rails', '5.2.4.4'
...

And a test to make sure that the checksum is not calculated, even after an upgrade.

require 'test_helper'

class ActiveStorageDisabledChecksumTest < ActiveSupport::TestCase
  test 'ensure checksum is not calculated' do
    blob = ActiveStorage::Blob.create_after_upload! io: StringIO.new('Hello'),
                                                    filename: 'hello.txt',
                                                    content_type: 'text/plain'

    assert_nil blob.checksum, 'checksum should not be calculated because it uses MD5 and FIPS is enabled'
  end
end

[shayonj]

Feels, like a good candidate to use config.active_support.hash_digest_class when present? That is how we control FIPS env today.

    config.active_support.hash_digest_class = ::Digest::SHA1 if config.x.run_fips

Happy to propose a PR

rails/activestorage/app/models/active_storage/blob.rb

Line 351 in d437ae3

OpenSSL::Digest::MD5.new.tap do |checksum|

[mrpasquini]

Looking into this closer, I'm not sure config.active_support.hash_digest_class will work since only MD5 is common between cloud services. A config option per cloud service seems needed to support Mirror functionality at minimum.

AWS/GCS/Azure support MD5 and use Content-MD5 HTTP Header to trigger cloud side validation.

• AWS lists support for :CRC32, :CRC32C, :SHA1, :SHA256 with custom http headers to use for server side validations
• GCS lists CRC32C with a custom http header for server side validations
• Azure lists CRC64 with a custom http header (x-ms-content-crc64) for server side validation.

I have OpenSSL in FIPS mode with a 1 line monkey patch on ActiveStorage::Blob to use an alternate implementation of MD5. Since this is integrity checking and not a cryptographic usage, it is an allowable usage and we can control/show all uses of this alternative implementation. I can submit a PR to abstract out the OpenSSL::Digest::MD5 class and allow a configuration option to provide another implementation.

Longer term, I support the idea of refactoring the service classes to have support for all digests supported by the cloud service and a configuration by service (or instance of service) allowing selection of digest to use.