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).

7 files changed, 284 insertions, 17 deletions

diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb
index a14445511a7..e7fedddbddf 100644
--- a/app/models/application_setting.rb
+++ b/app/models/application_setting.rb
@@ -277,23 +277,24 @@ class ApplicationSetting < ApplicationRecord
                  pass: :external_auth_client_key_pass,
                  if: -> (setting) { setting.external_auth_client_cert.present? }
 
-  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,
-                 mode: :per_attribute_iv,
-                 key: Settings.attr_encrypted_db_key_base_truncated,
-                 algorithm: 'aes-256-gcm',
-                 encode: true
+  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 :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 :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!
 
@@ -330,6 +331,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
diff --git a/changelogs/unreleased/security-2943-encrypt-plaintext-tokens.yml b/changelogs/unreleased/security-2943-encrypt-plaintext-tokens.yml
new file mode 100644
index 00000000000..d040565da73
--- /dev/null
+++ b/changelogs/unreleased/security-2943-encrypt-plaintext-tokens.yml
@@ -0,0 +1,5 @@
+---
+title: Encrypt application setting tokens
+merge_request:
+author:
+type: security
diff --git a/db/migrate/20191120084627_add_encrypted_fields_to_application_settings.rb b/db/migrate/20191120084627_add_encrypted_fields_to_application_settings.rb
new file mode 100644
index 00000000000..4e0886a5121
--- /dev/null
+++ b/db/migrate/20191120084627_add_encrypted_fields_to_application_settings.rb
@@ -0,0 +1,30 @@
+# 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
diff --git a/db/migrate/20191120115530_encrypt_plaintext_attributes_on_application_settings.rb b/db/migrate/20191120115530_encrypt_plaintext_attributes_on_application_settings.rb
new file mode 100644
index 00000000000..d7abb29fd75
--- /dev/null
+++ b/db/migrate/20191120115530_encrypt_plaintext_attributes_on_application_settings.rb
@@ -0,0 +1,93 @@
+# 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
diff --git a/db/schema.rb b/db/schema.rb
index 09f27d2f9cb..95c1e8811a7 100644
--- a/db/schema.rb
+++ b/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_09_18_104222) 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"
@@ -307,6 +307,18 @@ ActiveRecord::Schema.define(version: 2019_09_18_104222) do
     t.string "encrypted_asset_proxy_secret_key_iv"
     t.string "static_objects_external_storage_url", limit: 255
     t.string "static_objects_external_storage_auth_token", limit: 255
+    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"
diff --git a/spec/migrations/encrypt_plaintext_attributes_on_application_settings_spec.rb b/spec/migrations/encrypt_plaintext_attributes_on_application_settings_spec.rb
new file mode 100644
index 00000000000..6435e43f38c
--- /dev/null
+++ b/spec/migrations/encrypt_plaintext_attributes_on_application_settings_spec.rb
@@ -0,0 +1,58 @@
+# 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
diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb
index 855ce9cb4f9..46986c76ea1 100644
--- a/spec/models/application_setting_spec.rb
+++ b/spec/models/application_setting_spec.rb
@@ -13,6 +13,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' }