Make has_secure_password pluginable/extendable with different password hashing algorithms

[f3ndot]

Motivation / Background

Based on an idea by @rafaelfranca in #41420 (comment).

While BCrypt may be the desirable default password hashing algorithm in Rails for the foreseeable future, making the mechanism for hashing and verifying passwords in has_secure_password extendable/modular is valuable.

Developers may opt for different hashing algorithms and can now extend or plug in their own alongside BCrypt in a clean and maintainable way. From the referenced thread, there is discourse amongst people on what should be the de facto password algorithm and disagreement on whether Argon2 should be it. Rails should make it easier for the community to progress on best password hashing practices.

This also unlocks Rails shipping with more than BCrypt as an available hashing algorithm.

I could see a world where Rails offers PBKDF2 (for those wanting FIPS 140 compliance), BCrypt (the safe default), and Argon2 (since OWASP recommends it).

Detail

This PR takes BCrypt-specific code in ActiveModel::SecurePassword and extracts it out into an object ActiveModel::SecurePassword::BCrypt. The object adheres to a simple interface (ActiveModel::SecurePassword::Base) so developers can supplement with different algorithms (or different libraries of the same underlying algo).

A user of the Rails framework uses the newly added :algorithm option in has_secure_password to declare what password hashing algo should be used on a given attribute.

Developers can write libraries or even initializers to add different password hashing algorithm support following a very clean interface.

Below is an example of someone opting to use Argon2 using ruby-argon2:

# config/initializers/add_argon2_to_activemodel_securepassword.rb

require "active_model/secure_password/base"

module ActiveModel
  module SecurePassword
    class Argon2id < Base
      def initialize
        require "argon2"
      rescue LoadError
        warn "You don't have argon2 installed in your application. Please add it to your Gemfile and run bundle install."
        raise
      end

      def self.algorithm_name
        :argon2id
      end

      def hash_password(unencrypted_password, options = {})
        if options[:min_cost]
          hasher = Argon2::Password.new(t_cost: 1, m_cost: 3, p_cost: 1)
        else
          hasher = Argon2::Password.new(t_cost: 2, m_cost: 19, p_cost: 1)  # Latest OWASP recommended values
        end
        hasher.create(unencrypted_password)
      end

      def verify_password(password, digest)
        Argon2::Password.verify_password(password, digest)
      end

      def password_salt(digest)
        Argon2::HashFormat.new(digest).salt
      end
    end
  end
end

# app/models/user.rb

class User < ActiveRecord::Base
  has_secure_password :password, algorithm: :argon2
end

Additional information

I'm a bit of a novice when it comes to making Rails aware of different files and how to "register" a series of subclasses (some of which could be defined in libraries or the initializers folder) to a part of Rails. Open to whatever pattern is the most appropriate.

Checklist

Before submitting the PR make sure the following are checked:

• This Pull Request is related to one change. Changes that are unrelated should be opened in separate PRs.
• Commit message has a detailed description of what changed and why. If this PR fixes a related issue include it in the commit message. Ex: [Fix #issue-number]
• Tests are added or updated if you fix a bug or add a feature.
• CHANGELOG files are updated for the changed libraries if there is a behavior change or additional feature. Minor bug fixes and documentation changes should not be included.

[Flixt]

I think this is a great contribution to the framework and makes Rails more future proof.

What instantly comes to my mind is: When we provide the feature of setting the algorithm which is used for hashing per attribute, should we then also add tooling to make a transition from one hash algorithm to another? Is that something that could be achieved at all (can we now what algorithm was used for a hash)? Probably this is just out of scope for this PR and should be done in a separate PR, but thinking about it should be done beforehand. If the answer is "no, we don't provide tooling" we should at least add some documentation about it. Same for the feature itself.

[Flixt]

I'd say the "Rails way" would be to make this configurable so one can write something like

config.active_model.secure_password_algorithms << :some_algo

or

config.active_model.secure_password_algorithms << 'SomeAlgo'

The default value of that option would then be just all Algorithms that are supported by default (only BCrypt for now?).

An example of how to add configurable options can be seen in: #41158 (or just search for something like "add config" in the commit history, there are plenty of examples)

[f3ndot]

@Flixt I think you raise a good point. Migrating from one hash algorithm to another is fraught with security pitfalls and can drastically reduce the overall difficulty to crack passwords, so it's important to get right. To that end, either Rails endeavours to make that decision on behalf of the developer, or the documentation provides guidance that mirrors OWASP's Password Storage Cheat Sheet or security industry's best practices.

I may get this slightly wrong, but generally the (most accepted?) approach is:

1. Create a "pepper", a decently long global constant secret value kept outside of the DB layer (placing in secrets.yml can be good enough). The point is SQL Injections or DB compromise should not be able to obtain the secret.
2. Take all the rows of the old-hashed attribute (e.g. sha1) and layer/wrap in the new algorithm (e.g. bcrypt), suffixing the pepper. e.g. bcrypt(sha1($password).$pepper) or more accurately bcrypt($existing_attr_value.$pepper).
3. Business logic should first try to authenticate the user by comparing against just the new hash and then fallback to try the layered hash (e.g. try bcrypt($password) first then try bcrypt(sha1($password).$pepper) if first failed).
4. When a user signs in successfully on the fallback, replace the combined/layered hash with just the new algorithm's hash (e.g. bcrypt($password)).

The goal of the pepper is to prevent "password shucking" and is good defense in depth for very little effort.

Note that is just but one approach with pros and cons. Other approaches include forcefully resetting all passwords for users, requiring them to change or re-input their existing password to forcefully update all rows, or maintaining incremental hash changes but not bothering with layering. I'm sure there are also other approaches to peppering that isn't a simple suffixing, such as HMAC with the pepper as the key/secret.

I don't have a hard preference, but I think that password hash migration can be a deepish topic with different approaches and trying to shoehorn that into Rails may be too cumbersome. I lean towards documentation as an approach.

I agree whatever the outcome, be it providing migration tooling, documentation, or nothing at all it should take place outside this PR.

EDIT: I should quickly point out that while most discussions on updating password hashes are centred around migrating off of legacy or insecure hashing algos (e.g. md5 or sha1), their approaches and strategies can apply to migrating between two generally accepted and safe password hashing algos (e.g. bcrypt to argon2id and vice-versa). Though that may tip the scales on which acceptable migration approach is used, depending on pros/cons.

[Flixt]

I agree migrating from one password hashing algorithm to another can be cumbersome and contain security relevant pitfalls, but I also tend to say that this is exactly something Rails excels at. Making hard stuff less hard :) .. anyway.. I think we should hear some more pros/cons and then decide wether it is worth it to implement tooling around that problem in a separate PR.

I personally think that migrating password hashing algorithms is not something that is done every day / very often and that it is not worth it to maintain code for tooling around it to automate the process. We can just describe the process that is needed and link to other sources with correct approaches to the topic.

[Leont]

What instantly comes to my mind is: When we provide the feature of setting the algorithm which is used for hashing per attribute, should we then also add tooling to make a transition from one hash algorithm to another?

Yeah that is generally a very good idea.

(can we now what algorithm was used for a hash)?

That is simple, password hashes have a well defined header (e.g. $2b$ for bcrypt and $argonid for argon2) precisely because people need to do this; that part is a solved problem.

Is that something that could be achieved at all

Absolutely. The usual method is this:

Have exactly one object for creating new passwords (e.g. argon2). Have additional objects for validating other algorithms (e.g. bcrypt). Postel's Law applies here: "be conservative in what you do, be liberal in what you accept from others". So when verifying you just check what type of hash you're dealing with an route it to the right validator (e.g. "this starts with $2b$ so we use the bcrypt validator"). That way a mixed database keeps working just fine.

The next step is transitioning the values from old to new. You can do this by using a password_needs_rehash method. When a user logs in and they provide a valid password, you can check if the hash needs upgrading and if so you can recreate it with the new algorithm. That way you transparently upgrade your password database user-by-user.

[Leont]

I may get this slightly wrong, but generally the (most accepted?) approach is:

That is an approach you could use when transitioning from md5 or sha1 as a password hash, but that's not what we're dealing with here (and personally I find that approach dubious in general).

[Leont]

I personally think that migrating password hashing algorithms is not something that is done every day / very often and that it is not worth it to maintain code for tooling around it to automate the process. We can just describe the process that is needed and link to other sources with correct approaches to the topic.

This is security, you really don't want users to write homegrown solutions for that sort of thing because they'll get it almost right and therefore get pwned.

[eric-christian]

(and personally I find that approach dubious in general)

@Leont Can you elaborate on that?

[Leont]

@Leont Can you elaborate on that?

Sure. I dislike it for three reasons.

1. Combining two constructs in such a way has a risk of being less secure than it's components, such discoveries have been made before (e.g. password shucking). This sort of combination hasn't been proven to be secure.
2. You're producing something that declares to be bcrypt, but is actually not out-of-the-box compatible with bcrypt. This greatly complicates the sort of upgrade process that I describe in my first post because that all depends on the metadata to be correct (substituting the $2a$ header with something else could alleviate that). For example, if you want to reset all old-style password after a 1 year transition period, you can't easily do that because you can't distinguish them from the new ones.
3. During the transition checking a password is more expensive than it needs to be because you need up to two bcrypt operations.