Skip to content

Cleanup PBKDF2+SHA512 user password implementation

Drew Blessing requested to merge dblessing_pbkdf2_cleanup into master

What does this MR do and why?

Describe in detail what your merge request does and why.

Related to Tech debt: Update implementation of PBKDF2+SHA5... (#370450 - closed)

When this feature was initially developed there was confusion about its intended use. I thought it was going to be used on GitLab.com and eventually be made the default for all instances. Hence the feature flags. However, later it was understood that this would not ever be the default for GitLab, as BCrypt is arguably more secure. But BCrypt is not accepted by FIPS while PBKDF2+SHA512 is.

Now we're cleaning up the implementation so its only enabled with FIPS mode. However, in all cases we should have backward compatibility with the other type of encrypted password.

I moved the logic and specs to a concern for clarity, even though it's closely tied to the User model.

Screenshots or screen recordings

Screenshots are required for UI changes, and strongly recommended for all other merge requests.

How to set up and validate locally

Numbered steps to set up and validate the change are strongly suggested.

The easiest way to verify this is via the Rails console with and without FIPS mode enabled. Start with FIPS mode so you can then see how the password moves back to BCrypt once FIPS is disabled.

Of course, sign-in to the GDK web UI should also be successful in either mode. GDK Rails can be restarted in FIPS mode via FIPS_MODE=1 gdk restart. This is not required for the steps below.

With FIPS mode

  1. Start the console with FIPS mode: FIPS_MODE=1 bundle exec rails console
  2. Find a user for which you know the current password: user = User.find_by_username 'my_user'
  3. Observe the format of the user's encrypted password. It should begin with $2a$ to denote BCrypt: user.encrypted_password
  4. Verify the user's password, which should currently be stored using BCrypt, although the 'default' is now PBKDF2 since you're in FIPS mode: user.valid_password?('the_password') (this should yield true if you used the correct password)
  5. Observe the format of the encrypted password now. It should begin with $pbkdf2-sha512$: user.encrypted_password.
  6. Check that the password still validates: user.valid_password?('the_password')

Without FIPS mode

  1. Start the console without FIPS mode: bundle exec rails console
  2. Find the same user from the last section: user = User.find_by_username 'my_user'
  3. Observe the format of the user's encrypted password. It should begin with $pbkdf2-sha512$: user.encrypted_password
  4. Verify the user's password, which should currently be stored using PBKDF2 if you did the previous section first, although the 'default' is now BCrypt since you're not in FIPS mode: user.valid_password?('the_password') (this should yield true if you used the correct password)
  5. Observe the format of the encrypted password now. It should begin with $2a$: user.encrypted_password.
  6. Check that the password still validates: user.valid_password?('the_password')

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Drew Blessing

Merge request reports