From 9183bf943b36f7505f4ec64c2db14dc3f641b617 Mon Sep 17 00:00:00 2001 From: Arturo Herrero Date: Tue, 5 Nov 2019 10:08:31 +0000 Subject: [PATCH] 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 | 64 ++++++++----- ...security-2943-encrypt-plaintext-tokens.yml | 5 + ...ncrypted_fields_to_application_settings.rb | 30 ++++++ ...text_attributes_on_application_settings.rb | 93 +++++++++++++++++++ db/schema.rb | 12 +++ ...attributes_on_application_settings_spec.rb | 58 ++++++++++++ spec/models/application_setting_spec.rb | 44 +++++++++ 7 files changed, 284 insertions(+), 22 deletions(-) create mode 100644 changelogs/unreleased/security-2943-encrypt-plaintext-tokens.yml create mode 100644 db/migrate/20191120084627_add_encrypted_fields_to_application_settings.rb create mode 100644 db/migrate/20191120115530_encrypt_plaintext_attributes_on_application_settings.rb create mode 100644 spec/migrations/encrypt_plaintext_attributes_on_application_settings_spec.rb 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 + 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 :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 - - 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' }