Merge branch '29690-rotate-otp-key-base' into 'master'

changelogs/unreleased/29690-rotate-otp-key-base.yml

+---
+title: Add a Rake task to aid in rotating otp_key_base
+merge_request: 11881
+author:

doc/raketasks/user_management.md

@@ -71,6 +71,85 @@ sudo gitlab-rake gitlab:two_factor:disable_for_all_users
 bundle exec rake gitlab:two_factor:disable_for_all_users RAILS_ENV=production
 ```
 
+## Rotate Two-factor Authentication (2FA) encryption key
+
+GitLab stores the secret data enabling 2FA to work in an encrypted database
+column. The encryption key for this data is known as `otp_key_base`, and is
+stored in `config/secrets.yml`.
+
+
+If that file is leaked, but the individual 2FA secrets have not, it's possible
+to re-encrypt those secrets with a new encryption key. This allows you to change
+the leaked key without forcing all users to change their 2FA details.
+
+First, look up the old key. This is in the `config/secrets.yml` file, but
+**make sure you're working with the production section**. The line you're
+interested in will look like this:
+
+```yaml
+production:
+  otp_key_base: ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
+```
+
+Next, generate a new secret:
+
+```
+# omnibus-gitlab
+sudo gitlab-rake secret
+
+# installation from source
+bundle exec rake secret RAILS_ENV=production
+```
+
+Now you need to stop the GitLab server, back up the existing secrets file and
+update the database:
+
+```
+# omnibus-gitlab
+sudo gitlab-ctl stop
+sudo cp config/secrets.yml config/secrets.yml.bak
+sudo gitlab-rake gitlab:two_factor:rotate_key:apply filename=backup.csv old_key=<old key> new_key=<new key>
+
+# installation from source
+sudo /etc/init.d/gitlab stop
+cp config/secrets.yml config/secrets.yml.bak
+bundle exec rake gitlab:two_factor:rotate_key:apply filename=backup.csv old_key=<old key> new_key=<new key> RAILS_ENV=production
+```
+
+The `<old key>` value can be read from `config/secrets.yml`; `<new key>` was
+generated earlier. The **encrypted** values for the user 2FA secrets will be
+written to the specified `filename` - you can use this to rollback in case of
+error.
+
+Finally, change `config/secrets.yml` to set `otp_key_base` to `<new key>` and
+restart. Again, make sure you're operating in the **production** section.
+
+```
+# omnibus-gitlab
+sudo gitlab-ctl start
+
+# installation from source
+sudo /etc/init.d/gitlab start
+```
+
+If there are any problems (perhaps using the wrong value for `old_key`), you can
+restore your backup of `config/secrets.yml` and rollback the changes:
+
+```
+# omnibus-gitlab
+sudo gitlab-ctl stop
+sudo gitlab-rake gitlab:two_factor:rotate_key:rollback filename=backup.csv
+sudo cp config/secrets.yml.bak config/secrets.yml
+sudo gitlab-ctl start
+
+# installation from source
+sudo /etc/init.d/gitlab start
+bundle exec rake gitlab:two_factor:rotate_key:rollback filename=backup.csv RAILS_ENV=production
+cp config/secrets.yml.bak config/secrets.yml
+sudo /etc/init.d/gitlab start
+
+```
+
 ## Clear authentication tokens for all users. Important! Data loss!
 
 Clear authentication tokens for all users in the GitLab database. This

lib/gitlab/otp_key_rotator.rb

+module Gitlab
+  # The +otp_key_base+ param is used to encrypt the User#otp_secret attribute.
+  #
+  # When +otp_key_base+ is changed, it invalidates the current encrypted values
+  # of User#otp_secret. This class can be used to decrypt all the values with
+  # the old key, encrypt them with the new key, and and update the database
+  # with the new values.
+  #
+  # For persistence between runs, a CSV file is used with the following columns:
+  #
+  #   user_id, old_value, new_value
+  #
+  # Only the encrypted values are stored in this file.
+  #
+  # As users may have their 2FA settings changed at any time, this is only
+  # guaranteed to be safe if run offline.
+  class OtpKeyRotator
+    HEADERS = %w[user_id old_value new_value].freeze
+
+    attr_reader :filename
+
+    # Create a new rotator. +filename+ is used to store values by +calculate!+,
+    # and to update the database with new and old values in +apply!+ and
+    # +rollback!+, respectively.
+    def initialize(filename)
+      @filename = filename
+    end
+
+    def rotate!(old_key:, new_key:)
+      old_key ||= Gitlab::Application.secrets.otp_key_base
+
+      raise ArgumentError.new("Old key is the same as the new key") if old_key == new_key
+      raise ArgumentError.new("New key is too short! Must be 256 bits") if new_key.size < 64
+
+      write_csv do |csv|
+        ActiveRecord::Base.transaction do
+          User.with_two_factor.in_batches do |relation|
+            rows = relation.pluck(:id, :encrypted_otp_secret, :encrypted_otp_secret_iv, :encrypted_otp_secret_salt)
+            rows.each do |row|
+              user = %i[id ciphertext iv salt].zip(row).to_h
+              new_value = reencrypt(user, old_key, new_key)
+
+              User.where(id: user[:id]).update_all(encrypted_otp_secret: new_value)
+              csv << [user[:id], user[:ciphertext], new_value]
+            end
+          end
+        end
+      end
+    end
+
+    def rollback!
+      ActiveRecord::Base.transaction do
+        CSV.foreach(filename, headers: HEADERS, return_headers: false) do |row|
+          User.where(id: row['user_id']).update_all(encrypted_otp_secret: row['old_value'])
+        end
+      end
+    end
+
+    private
+
+    attr_reader :old_key, :new_key
+
+    def otp_secret_settings
+      @otp_secret_settings ||= User.encrypted_attributes[:otp_secret]
+    end
+
+    def reencrypt(user, old_key, new_key)
+      original = user[:ciphertext].unpack("m").join
+      opts = {
+        iv: user[:iv].unpack("m").join,
+        salt: user[:salt].unpack("m").join,
+        algorithm: otp_secret_settings[:algorithm],
+        insecure_mode: otp_secret_settings[:insecure_mode]
+      }
+
+      decrypted = Encryptor.decrypt(original, opts.merge(key: old_key))
+      encrypted = Encryptor.encrypt(decrypted, opts.merge(key: new_key))
+      [encrypted].pack("m")
+    end
+
+    def write_csv(&blk)
+      File.open(filename, "w") do |file|
+        yield CSV.new(file, headers: HEADERS, write_headers: false)
+      end
+    end
+  end
+end

lib/tasks/gitlab/two_factor.rake

@@ -19,5 +19,21 @@ namespace :gitlab do
         puts "There are currently no users with 2FA enabled.".color(:yellow)
       end
     end
+
+    namespace :rotate_key do
+      def rotator
+        @rotator ||= Gitlab::OtpKeyRotator.new(ENV['filename'])
+      end
+
+      desc "Encrypt user OTP secrets with a new encryption key"
+      task apply: :environment do |t, args|
+        rotator.rotate!(old_key: ENV['old_key'], new_key: ENV['new_key'])
+      end
+
+      desc "Rollback to secrets encrypted with the old encryption key"
+      task rollback: :environment do
+        rotator.rollback!
+      end
+    end
   end
 end

spec/lib/gitlab/otp_key_rotator_spec.rb

+require 'spec_helper'
+
+describe Gitlab::OtpKeyRotator do
+  let(:file) { Tempfile.new("otp-key-rotator-test") }
+  let(:filename) { file.path }
+  let(:old_key) { Gitlab::Application.secrets.otp_key_base }
+  let(:new_key) { "00" * 32 }
+  let!(:users) { create_list(:user, 5, :two_factor) }
+
+  after do
+    file.close
+    file.unlink
+  end
+
+  def data
+    CSV.read(filename)
+  end
+
+  def build_row(user, applied = false)
+    [user.id.to_s, encrypt_otp(user, old_key), encrypt_otp(user, new_key)]
+  end
+
+  def encrypt_otp(user, key)
+    opts = {
+      value: user.otp_secret,
+      iv: user.encrypted_otp_secret_iv.unpack("m").join,
+      salt: user.encrypted_otp_secret_salt.unpack("m").join,
+      algorithm: 'aes-256-cbc',
+      insecure_mode: true,
+      key: key
+    }
+    [Encryptor.encrypt(opts)].pack("m")
+  end
+
+  subject(:rotator) { described_class.new(filename) }
+
+  describe '#rotate!' do
+    subject(:rotation) { rotator.rotate!(old_key: old_key, new_key: new_key) }
+
+    it 'stores the calculated values in a spreadsheet' do
+      rotation
+
+      expect(data).to match_array(users.map {|u| build_row(u) })
+    end
+
+    context 'new key is too short' do
+      let(:new_key) { "00" * 31 }
+
+      it { expect { rotation }.to raise_error(ArgumentError) }
+    end
+
+    context 'new key is the same as the old key' do
+      let(:new_key) { old_key }
+
+      it { expect { rotation }.to raise_error(ArgumentError) }
+    end
+  end
+
+  describe '#rollback!' do
+    it 'updates rows to the old value' do
+      file.puts("#{users[0].id},old,new")
+      file.close
+
+      rotator.rollback!
+
+      expect(users[0].reload.encrypted_otp_secret).to eq('old')
+      expect(users[1].reload.encrypted_otp_secret).not_to eq('old')
+    end
+  end
+end