From a5c78650441c31a522b18e30177c717ffdd7f401 Mon Sep 17 00:00:00 2001 From: talyz <kim.lindberger@gmail.com> Date: Fri, 5 Feb 2021 18:49:13 +0100 Subject: [PATCH] Use the exactly 32 byte long version of db_key_base with aes-256-gcm Treewide, usage of a truncated version of db_key_base has been common and it works fairly well with `aes-256-cbc`. However, with `aes-256-gcm` the key must be exactly 32 bytes long - if it's longer it has to be truncated, but if it's shorter it has to be padded. For this reason the such a version of the key, `attr_encrypted_db_key_base_32`, was introduced in 777b6713bb473d2e09c8340ab9a96373fdbaae50. This changes all occurrences of `attr_encrypted_db_key_base_truncated` being used with `aes-256-gcm`. --- .../alert_management/http_integration.rb | 2 +- .../alerting/project_alerting_setting.rb | 2 +- app/models/application_setting.rb | 32 +++++++++---------- app/models/atlassian/identity.rb | 4 +-- app/models/bulk_imports/configuration.rb | 4 +-- .../clusters/applications/prometheus.rb | 2 +- app/models/clusters/providers/aws.rb | 2 +- .../concerns/packages/debian/distribution.rb | 2 +- .../project_error_tracking_setting.rb | 2 +- .../project_incident_management_setting.rb | 2 +- app/models/pages_domain_acme_order.rb | 2 +- app/models/serverless/domain_cluster.rb | 2 +- config/secrets.yml.example | 2 +- config/settings.rb | 14 ++++---- ...text_attributes_on_application_settings.rb | 16 +++++----- ...01008013434_generate_ci_jwt_signing_key.rb | 2 +- ee/app/models/geo_node.rb | 2 +- .../generate_ci_jwt_signing_key_spec.rb | 2 +- 18 files changed, 49 insertions(+), 47 deletions(-) diff --git a/app/models/alert_management/http_integration.rb b/app/models/alert_management/http_integration.rb index e98c770c36451..2caa9a184456d 100644 --- a/app/models/alert_management/http_integration.rb +++ b/app/models/alert_management/http_integration.rb @@ -10,7 +10,7 @@ class HttpIntegration < ApplicationRecord attr_encrypted :token, mode: :per_attribute_iv, - key: Settings.attr_encrypted_db_key_base_truncated, + key: Settings.attr_encrypted_db_key_base_32, algorithm: 'aes-256-gcm' default_value_for(:endpoint_identifier, allows_nil: false) { SecureRandom.hex(8) } diff --git a/app/models/alerting/project_alerting_setting.rb b/app/models/alerting/project_alerting_setting.rb index 8f8c38f11e40f..34fa27eb29be9 100644 --- a/app/models/alerting/project_alerting_setting.rb +++ b/app/models/alerting/project_alerting_setting.rb @@ -10,7 +10,7 @@ class ProjectAlertingSetting < ApplicationRecord attr_encrypted :token, mode: :per_attribute_iv, - key: Settings.attr_encrypted_db_key_base_truncated, + key: Settings.attr_encrypted_db_key_base_32, algorithm: 'aes-256-gcm' before_validation :ensure_token diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index f405f5ca5d318..bf4acb40d9d61 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -481,29 +481,29 @@ def self.kroki_formats_attributes algorithm: 'aes-256-cbc', insecure_mode: true - private_class_method def self.encryption_options_base_truncated_aes_256_gcm + private_class_method def self.encryption_options_base_32_aes_256_gcm { mode: :per_attribute_iv, - key: Settings.attr_encrypted_db_key_base_truncated, + key: Settings.attr_encrypted_db_key_base_32, algorithm: 'aes-256-gcm', encode: true } end - 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 - attr_encrypted :ci_jwt_signing_key, encryption_options_base_truncated_aes_256_gcm - attr_encrypted :secret_detection_token_revocation_token, encryption_options_base_truncated_aes_256_gcm - attr_encrypted :cloud_license_auth_token, encryption_options_base_truncated_aes_256_gcm - attr_encrypted :external_pipeline_validation_service_token, encryption_options_base_truncated_aes_256_gcm + attr_encrypted :external_auth_client_key, encryption_options_base_32_aes_256_gcm + attr_encrypted :external_auth_client_key_pass, encryption_options_base_32_aes_256_gcm + attr_encrypted :lets_encrypt_private_key, encryption_options_base_32_aes_256_gcm + attr_encrypted :eks_secret_access_key, encryption_options_base_32_aes_256_gcm + attr_encrypted :akismet_api_key, encryption_options_base_32_aes_256_gcm + attr_encrypted :elasticsearch_aws_secret_access_key, encryption_options_base_32_aes_256_gcm + attr_encrypted :recaptcha_private_key, encryption_options_base_32_aes_256_gcm + attr_encrypted :recaptcha_site_key, encryption_options_base_32_aes_256_gcm + attr_encrypted :slack_app_secret, encryption_options_base_32_aes_256_gcm + attr_encrypted :slack_app_verification_token, encryption_options_base_32_aes_256_gcm + attr_encrypted :ci_jwt_signing_key, encryption_options_base_32_aes_256_gcm + attr_encrypted :secret_detection_token_revocation_token, encryption_options_base_32_aes_256_gcm + attr_encrypted :cloud_license_auth_token, encryption_options_base_32_aes_256_gcm + attr_encrypted :external_pipeline_validation_service_token, encryption_options_base_32_aes_256_gcm validates :disable_feed_token, inclusion: { in: [true, false], message: _('must be a boolean value') } diff --git a/app/models/atlassian/identity.rb b/app/models/atlassian/identity.rb index 906f2be0fbf31..02bbe007e1b1f 100644 --- a/app/models/atlassian/identity.rb +++ b/app/models/atlassian/identity.rb @@ -11,14 +11,14 @@ class Identity < ApplicationRecord attr_encrypted :token, mode: :per_attribute_iv, - key: Settings.attr_encrypted_db_key_base_truncated, + key: Settings.attr_encrypted_db_key_base_32, algorithm: 'aes-256-gcm', encode: false, encode_iv: false attr_encrypted :refresh_token, mode: :per_attribute_iv, - key: Settings.attr_encrypted_db_key_base_truncated, + key: Settings.attr_encrypted_db_key_base_32, algorithm: 'aes-256-gcm', encode: false, encode_iv: false diff --git a/app/models/bulk_imports/configuration.rb b/app/models/bulk_imports/configuration.rb index 4c6f745c2686b..6d9f598583ea0 100644 --- a/app/models/bulk_imports/configuration.rb +++ b/app/models/bulk_imports/configuration.rb @@ -12,11 +12,11 @@ class BulkImports::Configuration < ApplicationRecord allow_nil: true attr_encrypted :url, - key: Settings.attr_encrypted_db_key_base_truncated, + key: Settings.attr_encrypted_db_key_base_32, mode: :per_attribute_iv, algorithm: 'aes-256-gcm' attr_encrypted :access_token, - key: Settings.attr_encrypted_db_key_base_truncated, + key: Settings.attr_encrypted_db_key_base_32, mode: :per_attribute_iv, algorithm: 'aes-256-gcm' end diff --git a/app/models/clusters/applications/prometheus.rb b/app/models/clusters/applications/prometheus.rb index b9c136abab406..435a970294da1 100644 --- a/app/models/clusters/applications/prometheus.rb +++ b/app/models/clusters/applications/prometheus.rb @@ -22,7 +22,7 @@ class Prometheus < ApplicationRecord attr_encrypted :alert_manager_token, mode: :per_attribute_iv, - key: Settings.attr_encrypted_db_key_base_truncated, + key: Settings.attr_encrypted_db_key_base_32, algorithm: 'aes-256-gcm' after_destroy do diff --git a/app/models/clusters/providers/aws.rb b/app/models/clusters/providers/aws.rb index bfd01775620d4..af2eba427219f 100644 --- a/app/models/clusters/providers/aws.rb +++ b/app/models/clusters/providers/aws.rb @@ -18,7 +18,7 @@ class Aws < ApplicationRecord attr_encrypted :secret_access_key, mode: :per_attribute_iv, - key: Settings.attr_encrypted_db_key_base_truncated, + key: Settings.attr_encrypted_db_key_base_32, algorithm: 'aes-256-gcm' validates :role_arn, diff --git a/app/models/concerns/packages/debian/distribution.rb b/app/models/concerns/packages/debian/distribution.rb index 08fb9ccf3ea37..267c7a4d201f5 100644 --- a/app/models/concerns/packages/debian/distribution.rb +++ b/app/models/concerns/packages/debian/distribution.rb @@ -84,7 +84,7 @@ def self.container_foreign_key attr_encrypted :signing_keys, mode: :per_attribute_iv, - key: Settings.attr_encrypted_db_key_base_truncated, + key: Settings.attr_encrypted_db_key_base_32, algorithm: 'aes-256-gcm', encode: false, encode_iv: false diff --git a/app/models/error_tracking/project_error_tracking_setting.rb b/app/models/error_tracking/project_error_tracking_setting.rb index 9a9fbc6a801ff..956b5d6470f02 100644 --- a/app/models/error_tracking/project_error_tracking_setting.rb +++ b/app/models/error_tracking/project_error_tracking_setting.rb @@ -38,7 +38,7 @@ class ProjectErrorTrackingSetting < ApplicationRecord attr_encrypted :token, mode: :per_attribute_iv, - key: Settings.attr_encrypted_db_key_base_truncated, + key: Settings.attr_encrypted_db_key_base_32, algorithm: 'aes-256-gcm' after_save :clear_reactive_cache! diff --git a/app/models/incident_management/project_incident_management_setting.rb b/app/models/incident_management/project_incident_management_setting.rb index 4887265be8889..3d4cb2f19122e 100644 --- a/app/models/incident_management/project_incident_management_setting.rb +++ b/app/models/incident_management/project_incident_management_setting.rb @@ -12,7 +12,7 @@ class ProjectIncidentManagementSetting < ApplicationRecord attr_encrypted :pagerduty_token, mode: :per_attribute_iv, - key: ::Settings.attr_encrypted_db_key_base_truncated, + key: ::Settings.attr_encrypted_db_key_base_32, algorithm: 'aes-256-gcm', encode: false, # No need to encode for binary column https://github.com/attr-encrypted/attr_encrypted#the-encode-encode_iv-encode_salt-and-default_encoding-options encode_iv: false diff --git a/app/models/pages_domain_acme_order.rb b/app/models/pages_domain_acme_order.rb index 411456cc237bf..8427176fa7206 100644 --- a/app/models/pages_domain_acme_order.rb +++ b/app/models/pages_domain_acme_order.rb @@ -14,7 +14,7 @@ class PagesDomainAcmeOrder < ApplicationRecord attr_encrypted :private_key, mode: :per_attribute_iv, - key: Settings.attr_encrypted_db_key_base_truncated, + key: Settings.attr_encrypted_db_key_base_32, algorithm: 'aes-256-gcm', encode: true diff --git a/app/models/serverless/domain_cluster.rb b/app/models/serverless/domain_cluster.rb index 9f914d5c3f89d..0d54a97370e2e 100644 --- a/app/models/serverless/domain_cluster.rb +++ b/app/models/serverless/domain_cluster.rb @@ -12,7 +12,7 @@ class DomainCluster < ApplicationRecord attr_encrypted :key, mode: :per_attribute_iv, - key: Settings.attr_encrypted_db_key_base_truncated, + key: Settings.attr_encrypted_db_key_base_32, algorithm: 'aes-256-gcm' validates :pages_domain, :knative, presence: true diff --git a/config/secrets.yml.example b/config/secrets.yml.example index 6b408ac6031d9..aadf13d53b2e6 100644 --- a/config/secrets.yml.example +++ b/config/secrets.yml.example @@ -1,7 +1,7 @@ production: # db_key_base is used to encrypt for Variables. Ensure that you don't lose it. # If you change or lose this key you will be unable to access variables stored in database. - # Make sure the secret is at least 30 characters and all random, + # Make sure the secret is at least 32 characters and all random, # no regular words or you'll be exposed to dictionary attacks. # db_key_base: diff --git a/config/settings.rb b/config/settings.rb index 3369f2a448053..a88f6d89ce421 100644 --- a/config/settings.rb +++ b/config/settings.rb @@ -126,16 +126,18 @@ def absolute(path) File.expand_path(path, Rails.root) end - # Ruby 2.4+ requires passing in the exact required length for OpenSSL keys - # (https://github.com/ruby/ruby/commit/ce635262f53b760284d56bb1027baebaaec175d1). - # Previous versions quietly truncated the input. - # - # Use this when using :per_attribute_iv mode for attr_encrypted. - # We have to truncate the string to 32 bytes for a 256-bit cipher. + # Don't use this in new code, use attr_encrypted_db_key_base_32 instead! def attr_encrypted_db_key_base_truncated Gitlab::Application.secrets.db_key_base[0..31] end + # Ruby 2.4+ requires passing in the exact required length for OpenSSL keys + # (https://github.com/ruby/ruby/commit/ce635262f53b760284d56bb1027baebaaec175d1). + # Previous versions quietly truncated the input. + # + # Makes sure the key is exactly 32 bytes long, either by + # truncating or right-padding it with ASCII 0s. Use this when + # using :per_attribute_iv mode for attr_encrypted. def attr_encrypted_db_key_base_32 Gitlab::Utils.ensure_utf8_size(attr_encrypted_db_key_base, bytes: 32.bytes) 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 index e6b9a40ad4f97..3ae5e3265e833 100644 --- a/db/migrate/20191120115530_encrypt_plaintext_attributes_on_application_settings.rb +++ b/db/migrate/20191120115530_encrypt_plaintext_attributes_on_application_settings.rb @@ -17,21 +17,21 @@ class EncryptPlaintextAttributesOnApplicationSettings < ActiveRecord::Migration[ class ApplicationSetting < ActiveRecord::Base self.table_name = 'application_settings' - def self.encryption_options_base_truncated_aes_256_gcm + def self.encryption_options_base_32_aes_256_gcm { mode: :per_attribute_iv, - key: Gitlab::Application.secrets.db_key_base[0..31], + key: Gitlab::Utils.ensure_utf8_size(Rails.application.secrets.db_key_base, bytes: 32.bytes), 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 + attr_encrypted :akismet_api_key, encryption_options_base_32_aes_256_gcm + attr_encrypted :elasticsearch_aws_secret_access_key, encryption_options_base_32_aes_256_gcm + attr_encrypted :recaptcha_private_key, encryption_options_base_32_aes_256_gcm + attr_encrypted :recaptcha_site_key, encryption_options_base_32_aes_256_gcm + attr_encrypted :slack_app_secret, encryption_options_base_32_aes_256_gcm + attr_encrypted :slack_app_verification_token, encryption_options_base_32_aes_256_gcm def akismet_api_key decrypt(:akismet_api_key, self[:encrypted_akismet_api_key]) || self[:akismet_api_key] diff --git a/db/migrate/20201008013434_generate_ci_jwt_signing_key.rb b/db/migrate/20201008013434_generate_ci_jwt_signing_key.rb index 7983a56f439be..5d7b6349fe611 100644 --- a/db/migrate/20201008013434_generate_ci_jwt_signing_key.rb +++ b/db/migrate/20201008013434_generate_ci_jwt_signing_key.rb @@ -8,7 +8,7 @@ class ApplicationSetting < ActiveRecord::Base attr_encrypted :ci_jwt_signing_key, { mode: :per_attribute_iv, - key: Rails.application.secrets.db_key_base[0..31], + key: Gitlab::Utils.ensure_utf8_size(Rails.application.secrets.db_key_base, bytes: 32.bytes), algorithm: 'aes-256-gcm', encode: true } diff --git a/ee/app/models/geo_node.rb b/ee/app/models/geo_node.rb index 09a2bdc40caac..bdb0e6804420f 100644 --- a/ee/app/models/geo_node.rb +++ b/ee/app/models/geo_node.rb @@ -55,7 +55,7 @@ class GeoNode < ApplicationRecord scope :ordered, -> { order(:id) } attr_encrypted :secret_access_key, - key: Settings.attr_encrypted_db_key_base_truncated, + key: Settings.attr_encrypted_db_key_base_32, algorithm: 'aes-256-gcm', mode: :per_attribute_iv, encode: true diff --git a/spec/migrations/generate_ci_jwt_signing_key_spec.rb b/spec/migrations/generate_ci_jwt_signing_key_spec.rb index 4cfaa8701aa88..249af3bcb5029 100644 --- a/spec/migrations/generate_ci_jwt_signing_key_spec.rb +++ b/spec/migrations/generate_ci_jwt_signing_key_spec.rb @@ -11,7 +11,7 @@ attr_encrypted :ci_jwt_signing_key, { mode: :per_attribute_iv, - key: Rails.application.secrets.db_key_base[0..31], + key: Gitlab::Utils.ensure_utf8_size(Rails.application.secrets.db_key_base, bytes: 32.bytes), algorithm: 'aes-256-gcm', encode: true } -- GitLab