From eadef9c942080f775cc89e93d019ea664541d69d Mon Sep 17 00:00:00 2001 From: Tom Hughes Date: Wed, 29 Apr 2026 18:27:23 +0100 Subject: [PATCH] Drop support for MD5 legacy passwords Remove support for validating MD5 passwords and migrate any such passwords in the database to an invalid password. Reported-by: Jorge Gonzalez Milla --- .rubocop.yml | 1 + lib/password_hash.rb | 9 ++------- lib/tasks/expire_md5_passwords.rake | 13 +++++++++++++ test/lib/password_hash_test.rb | 8 ++++---- 4 files changed, 20 insertions(+), 11 deletions(-) create mode 100644 lib/tasks/expire_md5_passwords.rake diff --git a/.rubocop.yml b/.rubocop.yml index c4c7f68f6..77f50acde 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -70,6 +70,7 @@ Rails/ReflectionClassName: Rails/SkipsModelValidations: Exclude: - 'db/migrate/*.rb' + - 'lib/tasks/expire_md5_passwords.rake' Style/CollectionQuerying: Exclude: diff --git a/lib/password_hash.rb b/lib/password_hash.rb index 69c47d6b9..7eae03723 100644 --- a/lib/password_hash.rb +++ b/lib/password_hash.rb @@ -2,7 +2,6 @@ require "argon2" require "base64" -require "digest/md5" require "openssl" module PasswordHash @@ -16,14 +15,10 @@ module PasswordHash def self.check(hash, salt, candidate) if Argon2::HashFormat.valid_hash?(hash) Argon2::Password.verify_password(candidate, hash) - elsif salt.nil? - ActiveSupport::SecurityUtils.secure_compare(hash, Digest::MD5.hexdigest(candidate)) - elsif salt.include?("!") + elsif salt&.include?("!") algorithm, iterations, salt = salt.split("!") size = Base64.strict_decode64(hash).length ActiveSupport::SecurityUtils.secure_compare(hash, pbkdf2(candidate, salt, iterations.to_i, size, algorithm)) - else - ActiveSupport::SecurityUtils.secure_compare(hash, Digest::MD5.hexdigest(salt + candidate)) end end @@ -40,7 +35,7 @@ module PasswordHash end def self.valid?(hash, salt) - Argon2::HashFormat.valid_hash?(hash) || salt&.include?("!") || hash =~ /^[0-9A-Z]{32}$/i + Argon2::HashFormat.valid_hash?(hash) || salt&.include?("!") end def self.pbkdf2(password, salt, iterations, size, algorithm) diff --git a/lib/tasks/expire_md5_passwords.rake b/lib/tasks/expire_md5_passwords.rake new file mode 100644 index 000000000..b1b7a685b --- /dev/null +++ b/lib/tasks/expire_md5_passwords.rake @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +namespace :db do + desc "Expire MD5 passwords" + task :expire_md5_passwords => :environment do + chunk_size = ENV["CHUNK_SIZE"]&.to_i || 10_000 + + User + .where("pass_crypt SIMILAR TO '[0-9a-z]{32}'") + .in_batches(:of => chunk_size) + .update_all(:pass_crypt => "expired password", :pass_salt => nil) + end +end diff --git a/test/lib/password_hash_test.rb b/test/lib/password_hash_test.rb index 1794c8a76..267f5a1b6 100644 --- a/test/lib/password_hash_test.rb +++ b/test/lib/password_hash_test.rb @@ -4,18 +4,18 @@ require "test_helper" class PasswordHashTest < ActiveSupport::TestCase def test_md5_without_salt - assert PasswordHash.check("5f4dcc3b5aa765d61d8327deb882cf99", nil, "password") + assert_not PasswordHash.check("5f4dcc3b5aa765d61d8327deb882cf99", nil, "password") assert_not PasswordHash.check("5f4dcc3b5aa765d61d8327deb882cf99", nil, "wrong") assert PasswordHash.upgrade?("5f4dcc3b5aa765d61d8327deb882cf99", nil) - assert PasswordHash.valid?("5f4dcc3b5aa765d61d8327deb882cf99", nil) + assert_not PasswordHash.valid?("5f4dcc3b5aa765d61d8327deb882cf99", nil) end def test_md5_with_salt - assert PasswordHash.check("67a1e09bb1f83f5007dc119c14d663aa", "salt", "password") + assert_not PasswordHash.check("67a1e09bb1f83f5007dc119c14d663aa", "salt", "password") assert_not PasswordHash.check("67a1e09bb1f83f5007dc119c14d663aa", "salt", "wrong") assert_not PasswordHash.check("67a1e09bb1f83f5007dc119c14d663aa", "wrong", "password") assert PasswordHash.upgrade?("67a1e09bb1f83f5007dc119c14d663aa", "salt") - assert PasswordHash.valid?("67a1e09bb1f83f5007dc119c14d663aa", "salt") + assert_not PasswordHash.valid?("67a1e09bb1f83f5007dc119c14d663aa", "salt") end def test_pbkdf2_1000_32_sha512 -- 2.47.3