diff options
author | GitLab Release Tools Bot <robert+release-tools@gitlab.com> | 2019-11-26 12:02:03 +0000 |
---|---|---|
committer | GitLab Release Tools Bot <robert+release-tools@gitlab.com> | 2019-11-26 12:02:03 +0000 |
commit | 7d028ae6a925c50033b14ada8495a244305e6df0 (patch) | |
tree | a30d3b8b6983c017a0108c0420dd8ba717928ffe | |
parent | 96d91c7885ec9cbaadf7f6ebd095f6e2b77941aa (diff) | |
parent | cc9a30c758d5fc6aeed79ff15ba402a88604cd49 (diff) | |
download | gitlab-ce-7d028ae6a925c50033b14ada8495a244305e6df0.tar.gz |
Merge branch 'security-2943-encrypt-plaintext-tokens-12-5' into '12-5-stable'
GitLab stores AWS, Slack, Askimet, reCaptcha tokens in plaintext
See merge request gitlab/gitlabhq!3543
7 files changed, 239 insertions, 29 deletions
diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index 4028d711fd1..72605af433f 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! 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..e6b9a40ad4f --- /dev/null +++ b/db/migrate/20191120115530_encrypt_plaintext_attributes_on_application_settings.rb @@ -0,0 +1,87 @@ +# 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) + 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/post_migrate/20191122135327_remove_plaintext_columns_from_application_settings.rb b/db/post_migrate/20191122135327_remove_plaintext_columns_from_application_settings.rb new file mode 100644 index 00000000000..b5cd58b10a8 --- /dev/null +++ b/db/post_migrate/20191122135327_remove_plaintext_columns_from_application_settings.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +class RemovePlaintextColumnsFromApplicationSettings < 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| + remove_column :application_settings, plaintext_attribute + end + end + + def down + PLAINTEXT_ATTRIBUTES.each do |plaintext_attribute| + add_column :application_settings, plaintext_attribute, :text + end + end +end diff --git a/db/schema.rb b/db/schema.rb index e3413722991..0dce19a29d7 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_11_15_091425) do +ActiveRecord::Schema.define(version: 2019_11_22_135327) do # These are extensions that must be enabled in order to support this database enable_extension "pg_trgm" @@ -180,11 +180,8 @@ ActiveRecord::Schema.define(version: 2019_11_15_091425) do t.integer "metrics_timeout", default: 10 t.integer "metrics_method_call_threshold", default: 10 t.boolean "recaptcha_enabled", default: false - t.string "recaptcha_site_key" - t.string "recaptcha_private_key" t.integer "metrics_port", default: 8089 t.boolean "akismet_enabled", default: false - t.string "akismet_api_key" t.integer "metrics_sample_interval", default: 15 t.boolean "email_author_in_body", default: false t.integer "default_group_visibility" @@ -231,7 +228,6 @@ ActiveRecord::Schema.define(version: 2019_11_15_091425) do t.boolean "elasticsearch_aws", default: false, null: false t.string "elasticsearch_aws_region", default: "us-east-1" t.string "elasticsearch_aws_access_key" - t.string "elasticsearch_aws_secret_access_key" t.integer "geo_status_timeout", default: 10 t.string "uuid" t.decimal "polling_interval_multiplier", default: "1.0", null: false @@ -247,8 +243,6 @@ ActiveRecord::Schema.define(version: 2019_11_15_091425) do t.string "help_page_support_url" t.boolean "slack_app_enabled", default: false t.string "slack_app_id" - t.string "slack_app_secret" - t.string "slack_app_verification_token" t.integer "performance_bar_allowed_group_id" t.boolean "allow_group_owners_to_manage_ldap", default: true, null: false t.boolean "hashed_storage_enabled", default: true, null: false @@ -355,6 +349,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" 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..122da7b3d72 --- /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 and saves it' 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]).not_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 |