Encrypt application setting tokens

This is the plan to encrypt the plaintext tokens:

First release (this commit):
  1. Create new encrypted fields in the database.
  2. Start populating new encrypted fields, read the encrypted fields or
     fallback to the plaintext fields.
  3. Backfill the data removing the plaintext fields to the encrypted fields.

Second release:
  4. Remove the virtual attribute (created in step 2).
  5. Drop plaintext columns from the database (empty columns after step 3).

app/models/application_setting.rb

@@ -313,29 +313,25 @@ class ApplicationSetting < ApplicationRecord
                  algorithm: 'aes-256-cbc',
                  insecure_mode: true
 
-  attr_encrypted :external_auth_client_key,
-                 mode: :per_attribute_iv,
-                 key: Settings.attr_encrypted_db_key_base_truncated,
-                 algorithm: 'aes-256-gcm',
-                 encode: true
-
-  attr_encrypted :external_auth_client_key_pass,
+  private_class_method def self.encryption_options_base_truncated_aes_256_gcm
+    {
       mode: :per_attribute_iv,
       key: Settings.attr_encrypted_db_key_base_truncated,
       algorithm: 'aes-256-gcm',
       encode: true
+    }
+  end
 
-  attr_encrypted :lets_encrypt_private_key,
-                 mode: :per_attribute_iv,
-                 key: Settings.attr_encrypted_db_key_base_truncated,
-                 algorithm: 'aes-256-gcm',
-                 encode: true
-
-  attr_encrypted :eks_secret_access_key,
-                 mode: :per_attribute_iv,
-                 key: Settings.attr_encrypted_db_key_base_truncated,
-                 algorithm: 'aes-256-gcm',
-                 encode: true
+  attr_encrypted :external_auth_client_key, encryption_options_base_truncated_aes_256_gcm
+  attr_encrypted :external_auth_client_key_pass, encryption_options_base_truncated_aes_256_gcm
+  attr_encrypted :lets_encrypt_private_key, encryption_options_base_truncated_aes_256_gcm
+  attr_encrypted :eks_secret_access_key, encryption_options_base_truncated_aes_256_gcm
+  attr_encrypted :akismet_api_key, encryption_options_base_truncated_aes_256_gcm
+  attr_encrypted :elasticsearch_aws_secret_access_key, encryption_options_base_truncated_aes_256_gcm
+  attr_encrypted :recaptcha_private_key, encryption_options_base_truncated_aes_256_gcm
+  attr_encrypted :recaptcha_site_key, encryption_options_base_truncated_aes_256_gcm
+  attr_encrypted :slack_app_secret, encryption_options_base_truncated_aes_256_gcm
+  attr_encrypted :slack_app_verification_token, encryption_options_base_truncated_aes_256_gcm
 
   before_validation :ensure_uuid!
 
@@ -368,6 +364,30 @@ class ApplicationSetting < ApplicationRecord
     Gitlab::ThreadMemoryCache.cache_backend
   end
 
+  def akismet_api_key
+    decrypt(:akismet_api_key, self[:encrypted_akismet_api_key]) || self[:akismet_api_key]
+  end
+
+  def elasticsearch_aws_secret_access_key
+    decrypt(:elasticsearch_aws_secret_access_key, self[:encrypted_elasticsearch_aws_secret_access_key]) || self[:elasticsearch_aws_secret_access_key]
+  end
+
+  def recaptcha_private_key
+    decrypt(:recaptcha_private_key, self[:encrypted_recaptcha_private_key]) || self[:recaptcha_private_key]
+  end
+
+  def recaptcha_site_key
+    decrypt(:recaptcha_site_key, self[:encrypted_recaptcha_site_key]) || self[:recaptcha_site_key]
+  end
+
+  def slack_app_secret
+    decrypt(:slack_app_secret, self[:encrypted_slack_app_secret]) || self[:slack_app_secret]
+  end
+
+  def slack_app_verification_token
+    decrypt(:slack_app_verification_token, self[:encrypted_slack_app_verification_token]) || self[:slack_app_verification_token]
+  end
+
   def recaptcha_or_login_protection_enabled
     recaptcha_enabled || login_recaptcha_protection_enabled
   end

changelogs/unreleased/security-2943-encrypt-plaintext-tokens.yml

+---
+title: Encrypt application setting tokens
+merge_request:
+author:
+type: security

db/migrate/20191120084627_add_encrypted_fields_to_application_settings.rb

+# frozen_string_literal: true
+
+class AddEncryptedFieldsToApplicationSettings < ActiveRecord::Migration[5.2]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  PLAINTEXT_ATTRIBUTES = %w[
+    akismet_api_key
+    elasticsearch_aws_secret_access_key
+    recaptcha_private_key
+    recaptcha_site_key
+    slack_app_secret
+    slack_app_verification_token
+  ].freeze
+
+  def up
+    PLAINTEXT_ATTRIBUTES.each do |plaintext_attribute|
+      add_column :application_settings, "encrypted_#{plaintext_attribute}", :text
+      add_column :application_settings, "encrypted_#{plaintext_attribute}_iv", :string, limit: 255
+    end
+  end
+
+  def down
+    PLAINTEXT_ATTRIBUTES.each do |plaintext_attribute|
+      remove_column :application_settings, "encrypted_#{plaintext_attribute}"
+      remove_column :application_settings, "encrypted_#{plaintext_attribute}_iv"
+    end
+  end
+end

db/migrate/20191120115530_encrypt_plaintext_attributes_on_application_settings.rb

+# frozen_string_literal: true
+
+class EncryptPlaintextAttributesOnApplicationSettings < ActiveRecord::Migration[5.2]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  PLAINTEXT_ATTRIBUTES = %w[
+    akismet_api_key
+    elasticsearch_aws_secret_access_key
+    recaptcha_private_key
+    recaptcha_site_key
+    slack_app_secret
+    slack_app_verification_token
+  ].freeze
+
+  class ApplicationSetting < ActiveRecord::Base
+    self.table_name = 'application_settings'
+
+    def self.encryption_options_base_truncated_aes_256_gcm
+      {
+        mode: :per_attribute_iv,
+        key: Gitlab::Application.secrets.db_key_base[0..31],
+        algorithm: 'aes-256-gcm',
+        encode: true
+      }
+    end
+
+    attr_encrypted :akismet_api_key, encryption_options_base_truncated_aes_256_gcm
+    attr_encrypted :elasticsearch_aws_secret_access_key, encryption_options_base_truncated_aes_256_gcm
+    attr_encrypted :recaptcha_private_key, encryption_options_base_truncated_aes_256_gcm
+    attr_encrypted :recaptcha_site_key, encryption_options_base_truncated_aes_256_gcm
+    attr_encrypted :slack_app_secret, encryption_options_base_truncated_aes_256_gcm
+    attr_encrypted :slack_app_verification_token, encryption_options_base_truncated_aes_256_gcm
+
+    def akismet_api_key
+      decrypt(:akismet_api_key, self[:encrypted_akismet_api_key]) || self[:akismet_api_key]
+    end
+
+    def elasticsearch_aws_secret_access_key
+      decrypt(:elasticsearch_aws_secret_access_key, self[:encrypted_elasticsearch_aws_secret_access_key]) || self[:elasticsearch_aws_secret_access_key]
+    end
+
+    def recaptcha_private_key
+      decrypt(:recaptcha_private_key, self[:encrypted_recaptcha_private_key]) || self[:recaptcha_private_key]
+    end
+
+    def recaptcha_site_key
+      decrypt(:recaptcha_site_key, self[:encrypted_recaptcha_site_key]) || self[:recaptcha_site_key]
+    end
+
+    def slack_app_secret
+      decrypt(:slack_app_secret, self[:encrypted_slack_app_secret]) || self[:slack_app_secret]
+    end
+
+    def slack_app_verification_token
+      decrypt(:slack_app_verification_token, self[:encrypted_slack_app_verification_token]) || self[:slack_app_verification_token]
+    end
+  end
+
+  def up
+    ApplicationSetting.find_each do |application_setting|
+      # We are using the setter from attr_encrypted gem to encrypt the data.
+      # The gem updates the two columns needed to decrypt the value:
+      # - "encrypted_#{plaintext_attribute}"
+      # - "encrypted_#{plaintext_attribute}_iv"
+      application_setting.assign_attributes(
+        PLAINTEXT_ATTRIBUTES.each_with_object({}) do |plaintext_attribute, attributes|
+          attributes[plaintext_attribute] = application_setting.send(plaintext_attribute)
+        end
+      )
+      application_setting.save(validate: false)
+
+      application_setting.update_columns(
+        PLAINTEXT_ATTRIBUTES.each_with_object({}) do |plaintext_attribute, attributes|
+          attributes[plaintext_attribute] = nil
+        end
+      )
+    end
+  end
+
+  def down
+    ApplicationSetting.find_each do |application_setting|
+      application_setting.update_columns(
+        PLAINTEXT_ATTRIBUTES.each_with_object({}) do |plaintext_attribute, attributes|
+          attributes[plaintext_attribute] = application_setting.send(plaintext_attribute)
+          attributes["encrypted_#{plaintext_attribute}"] = nil
+          attributes["encrypted_#{plaintext_attribute}_iv"] = nil
+        end
+      )
+    end
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 2019_11_15_091425) do
+ActiveRecord::Schema.define(version: 2019_11_20_115530) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "pg_trgm"
@@ -355,6 +355,18 @@ ActiveRecord::Schema.define(version: 2019_11_15_091425) do
     t.boolean "sourcegraph_enabled", default: false, null: false
     t.string "sourcegraph_url", limit: 255
     t.boolean "sourcegraph_public_only", default: true, null: false
+    t.text "encrypted_akismet_api_key"
+    t.string "encrypted_akismet_api_key_iv", limit: 255
+    t.text "encrypted_elasticsearch_aws_secret_access_key"
+    t.string "encrypted_elasticsearch_aws_secret_access_key_iv", limit: 255
+    t.text "encrypted_recaptcha_private_key"
+    t.string "encrypted_recaptcha_private_key_iv", limit: 255
+    t.text "encrypted_recaptcha_site_key"
+    t.string "encrypted_recaptcha_site_key_iv", limit: 255
+    t.text "encrypted_slack_app_secret"
+    t.string "encrypted_slack_app_secret_iv", limit: 255
+    t.text "encrypted_slack_app_verification_token"
+    t.string "encrypted_slack_app_verification_token_iv", limit: 255
     t.index ["custom_project_templates_group_id"], name: "index_application_settings_on_custom_project_templates_group_id"
     t.index ["file_template_project_id"], name: "index_application_settings_on_file_template_project_id"
     t.index ["instance_administration_project_id"], name: "index_applicationsettings_on_instance_administration_project_id"

spec/migrations/encrypt_plaintext_attributes_on_application_settings_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+require Rails.root.join('db', 'migrate', '20191120115530_encrypt_plaintext_attributes_on_application_settings.rb')
+
+describe EncryptPlaintextAttributesOnApplicationSettings, :migration do
+  let(:migration) { described_class.new }
+  let(:application_settings) { table(:application_settings) }
+  let(:plaintext) { 'secret-token' }
+
+  PLAINTEXT_ATTRIBUTES = %w[
+    akismet_api_key
+    elasticsearch_aws_secret_access_key
+    recaptcha_private_key
+    recaptcha_site_key
+    slack_app_secret
+    slack_app_verification_token
+  ].freeze
+
+  describe '#up' do
+    it 'encrypts token, saves it and removes plaintext token' do
+      application_setting = application_settings.create
+      application_setting.update_columns(
+        PLAINTEXT_ATTRIBUTES.each_with_object({}) do |plaintext_attribute, attributes|
+          attributes[plaintext_attribute] = plaintext
+        end
+      )
+
+      migration.up
+
+      application_setting.reload
+      PLAINTEXT_ATTRIBUTES.each do |plaintext_attribute|
+        expect(application_setting[plaintext_attribute]).to be_nil
+        expect(application_setting["encrypted_#{plaintext_attribute}"]).not_to be_nil
+        expect(application_setting["encrypted_#{plaintext_attribute}_iv"]).not_to be_nil
+      end
+    end
+  end
+
+  describe '#down' do
+    it 'decrypts encrypted token and saves it' do
+      application_setting = application_settings.create(
+        PLAINTEXT_ATTRIBUTES.each_with_object({}) do |plaintext_attribute, attributes|
+          attributes[plaintext_attribute] = plaintext
+        end
+      )
+
+      migration.down
+
+      application_setting.reload
+      PLAINTEXT_ATTRIBUTES.each do |plaintext_attribute|
+        expect(application_setting[plaintext_attribute]).to eq(plaintext)
+        expect(application_setting["encrypted_#{plaintext_attribute}"]).to be_nil
+        expect(application_setting["encrypted_#{plaintext_attribute}_iv"]).to be_nil
+      end
+    end
+  end
+end

spec/models/application_setting_spec.rb

@@ -15,6 +15,50 @@ describe ApplicationSetting do
   it { expect(setting.uuid).to be_present }
   it { expect(setting).to have_db_column(:auto_devops_enabled) }
 
+  context "with existing plaintext attributes" do
+    before do
+      setting.update_columns(
+        akismet_api_key: "akismet_api_key",
+        elasticsearch_aws_secret_access_key: "elasticsearch_aws_secret_access_key",
+        recaptcha_private_key: "recaptcha_private_key",
+        recaptcha_site_key: "recaptcha_site_key",
+        slack_app_secret: "slack_app_secret",
+        slack_app_verification_token: "slack_app_verification_token"
+      )
+    end
+
+    it "returns the attributes" do
+      expect(setting.akismet_api_key).to eq("akismet_api_key")
+      expect(setting.elasticsearch_aws_secret_access_key).to eq("elasticsearch_aws_secret_access_key")
+      expect(setting.recaptcha_private_key).to eq("recaptcha_private_key")
+      expect(setting.recaptcha_site_key).to eq("recaptcha_site_key")
+      expect(setting.slack_app_secret).to eq("slack_app_secret")
+      expect(setting.slack_app_verification_token).to eq("slack_app_verification_token")
+    end
+  end
+
+  context "with encrypted attributes" do
+    before do
+      setting.update(
+        akismet_api_key: "akismet_api_key",
+        elasticsearch_aws_secret_access_key: "elasticsearch_aws_secret_access_key",
+        recaptcha_private_key: "recaptcha_private_key",
+        recaptcha_site_key: "recaptcha_site_key",
+        slack_app_secret: "slack_app_secret",
+        slack_app_verification_token: "slack_app_verification_token"
+      )
+    end
+
+    it "returns the attributes" do
+      expect(setting.akismet_api_key).to eq("akismet_api_key")
+      expect(setting.elasticsearch_aws_secret_access_key).to eq("elasticsearch_aws_secret_access_key")
+      expect(setting.recaptcha_private_key).to eq("recaptcha_private_key")
+      expect(setting.recaptcha_site_key).to eq("recaptcha_site_key")
+      expect(setting.slack_app_secret).to eq("slack_app_secret")
+      expect(setting.slack_app_verification_token).to eq("slack_app_verification_token")
+    end
+  end
+
   describe 'validations' do
     let(:http)  { 'http://example.com' }
     let(:https) { 'https://example.com' }