Merge pull request rails#54040 from mrpasquini/md5_config

byroot · web-flow · commit e64e5d31cdea · 2025-01-04T13:09:08.000+01:00
Add config option for MD5 class to support FIPS compliance
diff --git a/activestorage/CHANGELOG.md b/activestorage/CHANGELOG.md
@@ -1,3 +1,10 @@
+*   Add support for alternative MD5 implementation through `config.active_storage.checksum_implementation`.
+
+    Also automatically degrade to using the slower `Digest::MD5` implementation if `OpenSSL::Digest::MD5`
+    is found to be disabled because of OpenSSL FIPS mode.
+
+    *Matt Pasquini*, *Jean Boussier*
+
 *   A Blob will no longer autosave associated Attachment.
 
     This fixes an issue where a record with an attachment would have
diff --git a/activestorage/app/models/active_storage/blob.rb b/activestorage/app/models/active_storage/blob.rb
@@ -348,7 +348,7 @@ def service
     def compute_checksum_in_chunks(io)
       raise ArgumentError, "io must be rewindable" unless io.respond_to?(:rewind)
 
-      OpenSSL::Digest::MD5.new.tap do |checksum|
+      ActiveStorage.checksum_implementation.new.tap do |checksum|
         read_buffer = "".b
         while io.read(5.megabytes, read_buffer)
           checksum << read_buffer
diff --git a/activestorage/lib/active_storage.rb b/activestorage/lib/active_storage.rb
@@ -363,6 +363,15 @@ module ActiveStorage
 
   mattr_accessor :track_variants, default: false
 
+  singleton_class.attr_accessor :checksum_implementation
+  @checksum_implementation = OpenSSL::Digest::MD5
+  begin
+    @checksum_implementation.hexdigest("test")
+  rescue # OpenSSL may have MD5 disabled
+    require "digest/md5"
+    @checksum_implementation = Digest::MD5
+  end
+
   mattr_accessor :video_preview_arguments, default: "-y -vframes 1 -f image2"
 
   module Transformers
diff --git a/activestorage/lib/active_storage/downloader.rb b/activestorage/lib/active_storage/downloader.rb
@@ -35,7 +35,7 @@ def download(key, file)
       end
 
       def verify_integrity_of(file, checksum:)
-        unless OpenSSL::Digest::MD5.file(file).base64digest == checksum
+        unless ActiveStorage.checksum_implementation.file(file).base64digest == checksum
           raise ActiveStorage::IntegrityError
         end
       end
diff --git a/activestorage/lib/active_storage/engine.rb b/activestorage/lib/active_storage/engine.rb
@@ -119,6 +119,9 @@ class Engine < Rails::Engine # :nodoc:
         ActiveStorage.binary_content_type = app.config.active_storage.binary_content_type || "application/octet-stream"
         ActiveStorage.video_preview_arguments = app.config.active_storage.video_preview_arguments || "-y -vframes 1 -f image2"
         ActiveStorage.track_variants = app.config.active_storage.track_variants || false
+        if app.config.active_storage.checksum_implementation
+          ActiveStorage.checksum_implementation = app.config.active_storage.checksum_implementation
+        end
       end
     end
 
diff --git a/activestorage/lib/active_storage/service/disk_service.rb b/activestorage/lib/active_storage/service/disk_service.rb
@@ -161,7 +161,7 @@ def make_path_for(key)
       end
 
       def ensure_integrity_of(key, checksum)
-        unless OpenSSL::Digest::MD5.file(path_for(key)).base64digest == checksum
+        unless ActiveStorage.checksum_implementation.file(path_for(key)).base64digest == checksum
           delete key
           raise ActiveStorage::IntegrityError
         end
diff --git a/activestorage/test/controllers/direct_uploads_controller_test.rb b/activestorage/test/controllers/direct_uploads_controller_test.rb
@@ -15,7 +15,7 @@ class ActiveStorage::S3DirectUploadsControllerTest < ActionDispatch::Integration
     end
 
     test "creating new direct upload" do
-      checksum = OpenSSL::Digest::MD5.base64digest("Hello")
+      checksum = ActiveStorage.checksum_implementation.base64digest("Hello")
       metadata = {
         "foo" => "bar",
         "my_key_1" => "my_value_1",
@@ -61,7 +61,7 @@ class ActiveStorage::GCSDirectUploadsControllerTest < ActionDispatch::Integratio
     end
 
     test "creating new direct upload" do
-      checksum = OpenSSL::Digest::MD5.base64digest("Hello")
+      checksum = ActiveStorage.checksum_implementation.base64digest("Hello")
       metadata = {
         "foo" => "bar",
         "my_key_1" => "my_value_1",
@@ -106,7 +106,7 @@ class ActiveStorage::AzureStorageDirectUploadsControllerTest < ActionDispatch::I
     end
 
     test "creating new direct upload" do
-      checksum = OpenSSL::Digest::MD5.base64digest("Hello")
+      checksum = ActiveStorage.checksum_implementation.base64digest("Hello")
       metadata = {
         "foo" => "bar",
         "my_key_1" => "my_value_1",
@@ -136,7 +136,7 @@ class ActiveStorage::AzureStorageDirectUploadsControllerTest < ActionDispatch::I
 
 class ActiveStorage::DiskDirectUploadsControllerTest < ActionDispatch::IntegrationTest
   test "creating new direct upload" do
-    checksum = OpenSSL::Digest::MD5.base64digest("Hello")
+    checksum = ActiveStorage.checksum_implementation.base64digest("Hello")
     metadata = {
       "foo" => "bar",
       "my_key_1" => "my_value_1",
@@ -161,7 +161,7 @@ class ActiveStorage::DiskDirectUploadsControllerTest < ActionDispatch::Integrati
   end
 
   test "creating new direct upload does not include root in json" do
-    checksum = OpenSSL::Digest::MD5.base64digest("Hello")
+    checksum = ActiveStorage.checksum_implementation.base64digest("Hello")
     metadata = {
       "foo" => "bar",
       "my_key_1" => "my_value_1",
diff --git a/activestorage/test/controllers/disk_controller_test.rb b/activestorage/test/controllers/disk_controller_test.rb
@@ -74,7 +74,7 @@ class ActiveStorage::DiskControllerTest < ActionDispatch::IntegrationTest
 
   test "directly uploading blob with integrity" do
     data = "Something else entirely!"
-    blob = create_blob_before_direct_upload byte_size: data.size, checksum: OpenSSL::Digest::MD5.base64digest(data)
+    blob = create_blob_before_direct_upload byte_size: data.size, checksum: ActiveStorage.checksum_implementation.base64digest(data)
 
     put blob.service_url_for_direct_upload, params: data, headers: { "Content-Type" => "text/plain" }
     assert_response :no_content
@@ -83,7 +83,7 @@ class ActiveStorage::DiskControllerTest < ActionDispatch::IntegrationTest
 
   test "directly uploading blob without integrity" do
     data = "Something else entirely!"
-    blob = create_blob_before_direct_upload byte_size: data.size, checksum: OpenSSL::Digest::MD5.base64digest("bad data")
+    blob = create_blob_before_direct_upload byte_size: data.size, checksum: ActiveStorage.checksum_implementation.base64digest("bad data")
 
     put blob.service_url_for_direct_upload, params: data
     assert_response :unprocessable_entity
@@ -92,7 +92,7 @@ class ActiveStorage::DiskControllerTest < ActionDispatch::IntegrationTest
 
   test "directly uploading blob with mismatched content type" do
     data = "Something else entirely!"
-    blob = create_blob_before_direct_upload byte_size: data.size, checksum: OpenSSL::Digest::MD5.base64digest(data)
+    blob = create_blob_before_direct_upload byte_size: data.size, checksum: ActiveStorage.checksum_implementation.base64digest(data)
 
     put blob.service_url_for_direct_upload, params: data, headers: { "Content-Type" => "application/octet-stream" }
     assert_response :unprocessable_entity
@@ -102,7 +102,7 @@ class ActiveStorage::DiskControllerTest < ActionDispatch::IntegrationTest
   test "directly uploading blob with different but equivalent content type" do
     data = "Something else entirely!"
     blob = create_blob_before_direct_upload(
-      byte_size: data.size, checksum: OpenSSL::Digest::MD5.base64digest(data), content_type: "application/x-gzip")
+      byte_size: data.size, checksum: ActiveStorage.checksum_implementation.base64digest(data), content_type: "application/x-gzip")
 
     put blob.service_url_for_direct_upload, params: data, headers: { "Content-Type" => "application/x-gzip" }
     assert_response :no_content
@@ -111,7 +111,7 @@ class ActiveStorage::DiskControllerTest < ActionDispatch::IntegrationTest
 
   test "directly uploading blob with mismatched content length" do
     data = "Something else entirely!"
-    blob = create_blob_before_direct_upload byte_size: data.size - 1, checksum: OpenSSL::Digest::MD5.base64digest(data)
+    blob = create_blob_before_direct_upload byte_size: data.size - 1, checksum: ActiveStorage.checksum_implementation.base64digest(data)
 
     put blob.service_url_for_direct_upload, params: data, headers: { "Content-Type" => "text/plain" }
     assert_response :unprocessable_entity
diff --git a/activestorage/test/models/attachment_test.rb b/activestorage/test/models/attachment_test.rb
@@ -41,7 +41,7 @@ class ActiveStorage::AttachmentTest < ActiveSupport::TestCase
   test "attaching a blob doesn't touch the record" do
     data = "Something else entirely!"
     io = StringIO.new(data)
-    blob = create_blob_before_direct_upload byte_size: data.size, checksum: OpenSSL::Digest::MD5.base64digest(data)
+    blob = create_blob_before_direct_upload byte_size: data.size, checksum: ActiveStorage.checksum_implementation.base64digest(data)
     blob.upload(io)
 
     user = User.create!(
diff --git a/activestorage/test/models/blob_test.rb b/activestorage/test/models/blob_test.rb
@@ -38,7 +38,7 @@ class ActiveStorage::BlobTest < ActiveSupport::TestCase
 
     assert_equal data, blob.download
     assert_equal data.length, blob.byte_size
-    assert_equal OpenSSL::Digest::MD5.base64digest(data), blob.checksum
+    assert_equal ActiveStorage.checksum_implementation.base64digest(data), blob.checksum
   end
 
   test "create_and_upload extracts content type from data" do
@@ -188,7 +188,7 @@ class ActiveStorage::BlobTest < ActiveSupport::TestCase
 
   test "open without integrity" do
     create_blob(data: "Hello, world!").tap do |blob|
-      blob.update! checksum: OpenSSL::Digest::MD5.base64digest("Goodbye, world!")
+      blob.update! checksum: ActiveStorage.checksum_implementation.base64digest("Goodbye, world!")
 
       assert_raises ActiveStorage::IntegrityError do
         blob.open { |file| flunk "Expected integrity check to fail" }
diff --git a/activestorage/test/service/azure_storage_public_service_test.rb b/activestorage/test/service/azure_storage_public_service_test.rb
@@ -21,7 +21,7 @@ class ActiveStorage::Service::AzureStoragePublicServiceTest < ActiveSupport::Tes
     test "direct upload" do
       key          = SecureRandom.base58(24)
       data         = "Something else entirely!"
-      checksum     = OpenSSL::Digest::MD5.base64digest(data)
+      checksum     = ActiveStorage.checksum_implementation.base64digest(data)
       content_type = "text/xml"
       url          = @service.url_for_direct_upload(key, expires_in: 5.minutes, content_type: content_type, content_length: data.size, checksum: checksum)
 
diff --git a/activestorage/test/service/azure_storage_service_test.rb b/activestorage/test/service/azure_storage_service_test.rb
@@ -12,7 +12,7 @@ class ActiveStorage::Service::AzureStorageServiceTest < ActiveSupport::TestCase
     test "direct upload with content type" do
       key          = SecureRandom.base58(24)
       data         = "Something else entirely!"
-      checksum     = OpenSSL::Digest::MD5.base64digest(data)
+      checksum     = ActiveStorage.checksum_implementation.base64digest(data)
       content_type = "text/xml"
       url          = @service.url_for_direct_upload(key, expires_in: 5.minutes, content_type: content_type, content_length: data.size, checksum: checksum)
 
@@ -34,7 +34,7 @@ class ActiveStorage::Service::AzureStorageServiceTest < ActiveSupport::TestCase
     test "direct upload with content disposition" do
       key      = SecureRandom.base58(24)
       data     = "Something else entirely!"
-      checksum = OpenSSL::Digest::MD5.base64digest(data)
+      checksum = ActiveStorage.checksum_implementation.base64digest(data)
       url      = @service.url_for_direct_upload(key, expires_in: 5.minutes, content_type: "text/plain", content_length: data.size, checksum: checksum)
 
       uri = URI.parse url
@@ -56,7 +56,7 @@ class ActiveStorage::Service::AzureStorageServiceTest < ActiveSupport::TestCase
       key      = SecureRandom.base58(24)
       data     = "Foobar"
 
-      @service.upload(key, StringIO.new(data), checksum: OpenSSL::Digest::MD5.base64digest(data), filename: ActiveStorage::Filename.new("test.txt"), content_type: "text/plain")
+      @service.upload(key, StringIO.new(data), checksum: ActiveStorage.checksum_implementation.base64digest(data), filename: ActiveStorage::Filename.new("test.txt"), content_type: "text/plain")
 
       url = @service.url(key, expires_in: 2.minutes, disposition: :attachment, content_type: nil, filename: ActiveStorage::Filename.new("test.html"))
       response = Net::HTTP.get_response(URI(url))
@@ -70,7 +70,7 @@ class ActiveStorage::Service::AzureStorageServiceTest < ActiveSupport::TestCase
       key  = SecureRandom.base58(24)
       data = "Foobar"
 
-      @service.upload(key, StringIO.new(data), checksum: OpenSSL::Digest::MD5.base64digest(data), filename: ActiveStorage::Filename.new("test.txt"), disposition: :inline)
+      @service.upload(key, StringIO.new(data), checksum: ActiveStorage.checksum_implementation.base64digest(data), filename: ActiveStorage::Filename.new("test.txt"), disposition: :inline)
 
       assert_equal("inline; filename=\"test.txt\"; filename*=UTF-8''test.txt", @service.client.get_blob_properties(@service.container, key).properties[:content_disposition])
 
@@ -85,7 +85,7 @@ class ActiveStorage::Service::AzureStorageServiceTest < ActiveSupport::TestCase
       key  = SecureRandom.base58(24)
       data = "Foobar"
 
-      @service.upload(key, StringIO.new(data), checksum: OpenSSL::Digest::MD5.base64digest(data), filename: ActiveStorage::Filename.new("test.txt"), custom_metadata: { "foo" => "baz" })
+      @service.upload(key, StringIO.new(data), checksum: ActiveStorage.checksum_implementation.base64digest(data), filename: ActiveStorage::Filename.new("test.txt"), custom_metadata: { "foo" => "baz" })
       url = @service.url(key, expires_in: 2.minutes, disposition: :inline, content_type: "text/html", filename: ActiveStorage::Filename.new("test.html"))
 
       response = Net::HTTP.get_response(URI(url))
diff --git a/activestorage/test/service/gcs_public_service_test.rb b/activestorage/test/service/gcs_public_service_test.rb
@@ -21,7 +21,7 @@ class ActiveStorage::Service::GCSPublicServiceTest < ActiveSupport::TestCase
     test "direct upload" do
       key      = SecureRandom.base58(24)
       data     = "Something else entirely!"
-      checksum = OpenSSL::Digest::MD5.base64digest(data)
+      checksum = ActiveStorage.checksum_implementation.base64digest(data)
       url      = @service.url_for_direct_upload(key, expires_in: 5.minutes, content_type: "text/plain", content_length: data.size, checksum: checksum)
 
       uri = URI.parse url
diff --git a/activestorage/test/service/gcs_service_test.rb b/activestorage/test/service/gcs_service_test.rb
@@ -16,7 +16,7 @@ class ActiveStorage::Service::GCSServiceTest < ActiveSupport::TestCase
     test "direct upload" do
       key      = SecureRandom.base58(24)
       data     = "Something else entirely!"
-      checksum = OpenSSL::Digest::MD5.base64digest(data)
+      checksum = ActiveStorage.checksum_implementation.base64digest(data)
       url      = @service.url_for_direct_upload(key, expires_in: 5.minutes, content_type: "text/plain", content_length: data.size, checksum: checksum)
 
       uri = URI.parse url
@@ -36,7 +36,7 @@ class ActiveStorage::Service::GCSServiceTest < ActiveSupport::TestCase
     test "direct upload with content disposition" do
       key      = SecureRandom.base58(24)
       data     = "Something else entirely!"
-      checksum = OpenSSL::Digest::MD5.base64digest(data)
+      checksum = ActiveStorage.checksum_implementation.base64digest(data)
       url      = @service.url_for_direct_upload(key, expires_in: 5.minutes, content_type: "text/plain", content_length: data.size, checksum: checksum)
 
       uri = URI.parse url
@@ -91,7 +91,7 @@ class ActiveStorage::Service::GCSServiceTest < ActiveSupport::TestCase
       key      = SecureRandom.base58(24)
       data     = "Something else entirely!"
 
-      @service.upload(key, StringIO.new(data), checksum: OpenSSL::Digest::MD5.base64digest(data), disposition: :attachment, filename: ActiveStorage::Filename.new("test.txt"), content_type: "text/plain")
+      @service.upload(key, StringIO.new(data), checksum: ActiveStorage.checksum_implementation.base64digest(data), disposition: :attachment, filename: ActiveStorage::Filename.new("test.txt"), content_type: "text/plain")
 
       url = @service.url(key, expires_in: 2.minutes, disposition: :inline, content_type: "text/html", filename: ActiveStorage::Filename.new("test.html"))
       response = Net::HTTP.get_response(URI(url))
@@ -105,7 +105,7 @@ class ActiveStorage::Service::GCSServiceTest < ActiveSupport::TestCase
       key      = SecureRandom.base58(24)
       data     = "Something else entirely!"
 
-      @service.upload(key, StringIO.new(data), checksum: OpenSSL::Digest::MD5.base64digest(data), content_type: "text/plain")
+      @service.upload(key, StringIO.new(data), checksum: ActiveStorage.checksum_implementation.base64digest(data), content_type: "text/plain")
 
       url = @service.url(key, expires_in: 2.minutes, disposition: :inline, content_type: "text/html", filename: ActiveStorage::Filename.new("test.html"))
       response = Net::HTTP.get_response(URI(url))
@@ -148,7 +148,7 @@ class ActiveStorage::Service::GCSServiceTest < ActiveSupport::TestCase
     test "update custom_metadata" do
       key      = SecureRandom.base58(24)
       data     = "Something else entirely!"
-      @service.upload(key, StringIO.new(data), checksum: OpenSSL::Digest::MD5.base64digest(data), disposition: :attachment, filename: ActiveStorage::Filename.new("test.html"), content_type: "text/html", custom_metadata: { "foo" => "baz" })
+      @service.upload(key, StringIO.new(data), checksum: ActiveStorage.checksum_implementation.base64digest(data), disposition: :attachment, filename: ActiveStorage::Filename.new("test.html"), content_type: "text/html", custom_metadata: { "foo" => "baz" })
 
       @service.update_metadata(key, disposition: :inline, filename: ActiveStorage::Filename.new("test.txt"), content_type: "text/plain", custom_metadata: { "foo" => "bar" })
       url = @service.url(key, expires_in: 2.minutes, disposition: :attachment, content_type: "text/html", filename: ActiveStorage::Filename.new("test.html"))
diff --git a/activestorage/test/service/mirror_service_test.rb b/activestorage/test/service/mirror_service_test.rb
@@ -29,7 +29,7 @@ class ActiveStorage::Service::MirrorServiceTest < ActiveSupport::TestCase
     key      = SecureRandom.base58(24)
     data     = "Something else entirely!"
     io       = StringIO.new(data)
-    checksum = OpenSSL::Digest::MD5.base64digest(data)
+    checksum = ActiveStorage.checksum_implementation.base64digest(data)
 
     assert_performed_jobs 1, only: ActiveStorage::MirrorJob do
       @service.upload key, io.tap(&:read), checksum: checksum
@@ -49,7 +49,7 @@ class ActiveStorage::Service::MirrorServiceTest < ActiveSupport::TestCase
   test "downloading from primary service" do
     key      = SecureRandom.base58(24)
     data     = "Something else entirely!"
-    checksum = OpenSSL::Digest::MD5.base64digest(data)
+    checksum = ActiveStorage.checksum_implementation.base64digest(data)
 
     @service.primary.upload key, StringIO.new(data), checksum: checksum
 
@@ -68,7 +68,7 @@ class ActiveStorage::Service::MirrorServiceTest < ActiveSupport::TestCase
   test "mirroring a file from the primary service to secondary services where it doesn't exist" do
     key      = SecureRandom.base58(24)
     data     = "Something else entirely!"
-    checksum = OpenSSL::Digest::MD5.base64digest(data)
+    checksum = ActiveStorage.checksum_implementation.base64digest(data)
 
     @service.primary.upload key, StringIO.new(data), checksum: checksum
     @service.mirrors.third.upload key, StringIO.new("Surprise!")
diff --git a/activestorage/test/service/s3_public_service_test.rb b/activestorage/test/service/s3_public_service_test.rb
@@ -35,7 +35,7 @@ class ActiveStorage::Service::S3PublicServiceTest < ActiveSupport::TestCase
     test "direct upload" do
       key      = SecureRandom.base58(24)
       data     = "Something else entirely!"
-      checksum = OpenSSL::Digest::MD5.base64digest(data)
+      checksum = ActiveStorage.checksum_implementation.base64digest(data)
       url      = @service.url_for_direct_upload(key, expires_in: 5.minutes, content_type: "text/plain", content_length: data.size, checksum: checksum)
 
       uri = URI.parse url
diff --git a/activestorage/test/service/s3_service_test.rb b/activestorage/test/service/s3_service_test.rb
diff --git a/activestorage/test/service/shared_service_tests.rb b/activestorage/test/service/shared_service_tests.rb
diff --git a/activestorage/test/test_helper.rb b/activestorage/test/test_helper.rb
diff --git a/guides/source/configuring.md b/guides/source/configuring.md
