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, 22 deletions

diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb
index 4028d711fd1..dae1235fa6b 100644
--- a/app/models/application_setting.rb
+++ b/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,
-                 mode: :per_attribute_iv,
-                 key: Settings.attr_encrypted_db_key_base_truncated,
-                 algorithm: 'aes-256-gcm',
-                 encode: true
-
-  attr_encrypted :lets_encrypt_private_key,
-                 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 :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
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 9dccceb79f0..29d812e250e 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -355,6 +355,18 @@ ActiveRecord::Schema.define(version: 2019_11_24_150431) 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"
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 ba3b99f4421..7b1ebe586cd 100644
--- a/spec/models/application_setting_spec.rb
+++ b/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' }