Skip to content

Add support for using ActiveRecord::Encryption

Rémy Coutablerequested to merge
490590-cells-1-0-setup-for-activerecord-encryption-framework-2 into master

What does this MR do and why?

The original MR was reverted by !175143 (merged). I have marked this MR as dependent of gitlab-com/gl-infra/k8s-workloads/gitlab-com!4014 (merged) to prevent an incident similar to what the original MR caused.

I took inspiration from !127186 (closed).

This reads & write (if missing) ActiveRecord::Encryption keys from/in config/secrets.yml.

To start using ActiveRecord::Encryption, all you need is a migration to add a JSONB column, e.g.

# frozen_string_literal: true

class AddOpenidConnectSigningKeyToApplicationSettings < Gitlab::Database::Migration[2.2]
  milestone '17.5'

  def change
    add_column :application_settings, :openid_connect_signing_key, :jsonb
    add_column :application_settings, :encrypted_identity_verification_secrets, :jsonb
  end
end

and a new encrypts :field line, e.g.

diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb
index 55f01e328451..4a3e0b303fba 100644
--- a/app/models/application_setting.rb
+++ b/app/models/application_setting.rb
@@ -57,6 +57,8 @@ class ApplicationSetting < ApplicationRecord
   add_authentication_token_field :static_objects_external_storage_auth_token, encrypted: :required # rubocop:todo -- https://gitlab.com/gitlab-org/gitlab/-/issues/439292
   add_authentication_token_field :error_tracking_access_token, encrypted: :required # rubocop:todo -- https://gitlab.com/gitlab-org/gitlab/-/issues/439292
 
+  encrypts :openid_connect_signing_key
+  jsonb_accessor :encrypted_identity_verification_secrets,
+    some_setting: [:string, { default: "some_setting_default" }]
+  encrypts :encrypted_identity_verification_secrets
+
   belongs_to :push_rule
   belongs_to :web_ide_oauth_application, class_name: 'Doorkeeper::Application'

Encrypting hash attributes (defined with jsonb_accessor above) is also supported:

> application_settings = ApplicationSetting.last
> application_settings.some_setting = "foo"
> application_settings.save!
  ApplicationSetting Update (0.7ms)  UPDATE "application_settings" SET "encrypted_identity_verification_secrets" = '{"p":"PKkplntKWXf43ckBARi0UR+jfDXlckZ5wJxEuExI","h":{"iv":"TWwldXXA5VcP/AmA","at":"OTJEscJNUiWt+HbB/Bbeqw=="}}', "updated_at" = '2024-10-08 07:31:41.788513' WHERE "application_settings"."id" = 1

# Reload the record, just in case
> application_settings = ApplicationSetting.last

# The attribute is a hash
> application_settings.encrypted_identity_verification_secrets
=> {"some_setting"=>"foo"}

# Individual accessors also work
> application_settings.some_setting
=> "foo"

# Nested attribute can also be set via the hash, with a small caveat...
> application_settings.encrypted_identity_verification_secrets["some_setting"] = 'bar'
=> "bar"

# its value isn't reflected in the individual accessor...
> application_settings.some_setting
=> "baz" # we'd expect "bar" here!

# but it's value is correct in the hash attribute
> application_settings.encrypted_identity_verification_secrets
=> {"some_setting"=>"bar"}

> application_settings.save!

# Reload the record, just in case
> application_settings = ApplicationSetting.last

# The new nested setting value is saved...
> application_settings.encrypted_identity_verification_secrets
=> {"some_setting"=>"bar"}

# And reflected in the individual accessor after save/reload
> application_settings.some_setting             
=> "bar"

By using ActiveRecord::Encryption, we'll be able to know what key was used to encrypt a specific field, e.g.

> legacy_key_id = ActiveRecord::Encryption::DerivedSecretKeyProvider.new(ActiveRecord::Encryption.config.primary_key.first).encryption_key.id
=> "08ae"

> ApplicationSetting.where("openid_connect_signing_key->'h'->'i' ? :value", value: ::Base64.strict_encode64(legacy_key_id)).count
=> 1

so that later we can rotate keys easily (this is already supported by ActiveRecord::Encryption but we'll need a process to automatize the re-encryption of data).

MR acceptance checklist

Please evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Screenshots or screen recordings

Screenshots are required for UI changes, and strongly recommended for all other merge requests.

Before After

How to set up and validate locally

Numbered steps to set up and validate the change are strongly suggested.

Related to #490590 (closed).

Edited by Rémy Coutable

Merge request reports