diff --git a/app/services/encryption/multi_region_kms_profile_migrator.rb b/app/services/encryption/multi_region_kms_profile_migrator.rb new file mode 100644 index 00000000000..31032b2636d --- /dev/null +++ b/app/services/encryption/multi_region_kms_profile_migrator.rb @@ -0,0 +1,73 @@ +# frozen_string_literal: true + +module Encryption + class MultiRegionKmsProfileMigrator + include ::NewRelic::Agent::MethodTracer + + attr_reader :profile + + def initialize(profile) + @profile = profile + end + + def migrate! + profile.with_lock do + if profile.encrypted_pii.blank? && profile.encrypted_pii_recovery.blank? + raise "Profile##{profile.id} is missing encrypted_pii or encrypted_pii_recovery" + end + + next if profile.encrypted_pii_multi_region.present? && + profile.encrypted_pii_recovery_multi_region.present? + + if profile.encrypted_pii.present? && profile.encrypted_pii_multi_region.blank? + encrypted_pii_multi_region = migrate_ciphertext(profile.encrypted_pii) + profile.update!( + encrypted_pii_multi_region: encrypted_pii_multi_region, + ) + end + if profile.encrypted_pii_recovery.present? && + profile.encrypted_pii_recovery_multi_region.blank? + encrypted_pii_recovery_multi_region = migrate_ciphertext(profile.encrypted_pii_recovery) + profile.update!( + encrypted_pii_recovery_multi_region: encrypted_pii_recovery_multi_region, + ) + end + + profile + end + end + + private + + def migrate_ciphertext(ciphertext_string) + ciphertext = Encryption::Encryptors::PiiEncryptor::Ciphertext.parse_from_string( + ciphertext_string, + ) + + aes_encrypted_data = multi_region_kms_client.decrypt( + ciphertext.encrypted_data, kms_encryption_context + ) + multi_region_kms_encrypted_data = multi_region_kms_client.encrypt( + aes_encrypted_data, kms_encryption_context + ) + Encryption::Encryptors::PiiEncryptor::Ciphertext.new( + multi_region_kms_encrypted_data, + ciphertext.salt, + ciphertext.cost, + ) + end + + def kms_encryption_context + { + 'context' => 'pii-encryption', + 'user_uuid' => profile.user.uuid, + } + end + + def multi_region_kms_client + @multi_region_kms_client ||= KmsClient.new( + kms_key_id: IdentityConfig.store.aws_kms_multi_region_key_id, + ) + end + end +end diff --git a/lib/tasks/backfill_profiles.rake b/lib/tasks/backfill_profiles.rake new file mode 100644 index 00000000000..0cdf843891d --- /dev/null +++ b/lib/tasks/backfill_profiles.rake @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +namespace :profiles do + desc 'Backfill the encrypted_pii_multi_region value column.' + + ## + # Usage: + # + # Print pending updates + # bundle exec rake profiles:backfill_encrypted_pii_multi_region + # + # Commit updates + # bundle exec rake profiles:backfill_encrypted_pii_multi_region UPDATE_PROFILES=true + # + task backfill_encrypted_pii_multi_region: :environment do |_task, _args| + profile_limit = ENV['PROFILE_LIMIT'].to_i + statement_timeout_seconds = ENV['STATEMENT_TIMEOUT_SECONDS'].to_i + update_profiles = ENV['UPDATE_PROFILES'] == 'true' + + profiles = Profile.transaction do + quoted_timeout = Profile.connection.quote(statement_timeout_seconds * 1000) + Profile.connection.execute("SET LOCAL statement_timeout = #{quoted_timeout}") + + Profile.where( + <<-SQL, + (encrypted_pii IS NOT NULL AND encrypted_pii_multi_region IS NULL) OR + (encrypted_pii_recovery IS NOT NULL AND encrypted_pii_recovery_multi_region IS NULL) + SQL + ).limit(profile_limit) + end + + Rails.logger.info("#{profiles.count} profiles found") + profiles.each do |profile| + Rails.logger.info(profile.id) + Encryption::MultiRegionKmsProfileMigrator.new(profile).migrate! if update_profiles + end + end +end diff --git a/spec/lib/tasks/backfill_profiles_rake_spec.rb b/spec/lib/tasks/backfill_profiles_rake_spec.rb new file mode 100644 index 00000000000..789d44bc40f --- /dev/null +++ b/spec/lib/tasks/backfill_profiles_rake_spec.rb @@ -0,0 +1,82 @@ +require 'rails_helper' +require 'rake' + +RSpec.describe 'backfill profiles tasks' do + let(:profile_limit) { '10' } + let(:update_profiles) { nil } + let(:env) do + { + 'PROFILE_LIMIT' => profile_limit, + 'STATEMENT_TIMEOUT_SECONDS' => '100', + 'UPDATE_PROFILES' => update_profiles, + } + end + + before do + Rake.application.rake_require 'tasks/backfill_profiles' + Rake::Task.define_task(:environment) + Rake::Task['profiles:backfill_encrypted_pii_multi_region'].reenable + stub_const('ENV', env) + end + + describe 'dev:backfill_encrypted_pii_multi_region' do + it 'logs data about profiles being migrated' do + profile = create_profile_that_needs_to_be_migrated + create(:profile, :with_pii) + + expect(Rails.logger).to receive(:info).with('1 profiles found') + expect(Rails.logger).to receive(:info).with(profile.id) + + Rake::Task['profiles:backfill_encrypted_pii_multi_region'].invoke + end + + context 'with update_profiles disabled' do + it 'does not update profiles' do + profile = create_profile_that_needs_to_be_migrated + + expect(profile.encrypted_pii).to be_present + expect(profile.encrypted_pii_recovery).to be_present + expect(profile.encrypted_pii_multi_region).to_not be_present + expect(profile.encrypted_pii_recovery_multi_region).to_not be_present + + Rake::Task['profiles:backfill_encrypted_pii_multi_region'].invoke + + expect(profile.encrypted_pii).to be_present + expect(profile.encrypted_pii_recovery).to be_present + expect(profile.encrypted_pii_multi_region).to_not be_present + expect(profile.encrypted_pii_recovery_multi_region).to_not be_present + end + end + + context 'with update_profiles enabled' do + let(:update_profiles) { 'true' } + + it 'does update profiles' do + profile = create_profile_that_needs_to_be_migrated + expect(profile.encrypted_pii).to be_present + expect(profile.encrypted_pii_recovery).to be_present + expect(profile.encrypted_pii_multi_region).to_not be_present + expect(profile.encrypted_pii_recovery_multi_region).to_not be_present + + Rake::Task['profiles:backfill_encrypted_pii_multi_region'].invoke + + profile.reload + expect(profile.encrypted_pii).to be_present + expect(profile.encrypted_pii_recovery).to be_present + expect(profile.encrypted_pii_multi_region).to be_present + expect(profile.encrypted_pii_recovery_multi_region).to be_present + end + end + end + + def create_profile_that_needs_to_be_migrated + profile = create(:profile, :with_pii) + profile.update( + encrypted_pii: profile.encrypted_pii_multi_region, + encrypted_pii_multi_region: nil, + encrypted_pii_recovery: profile.encrypted_pii_recovery_multi_region, + encrypted_pii_recovery_multi_region: nil, + ) + profile + end +end diff --git a/spec/services/encryption/kms_client_spec.rb b/spec/services/encryption/kms_client_spec.rb index 4b4a46e94a5..dab6a9771ca 100644 --- a/spec/services/encryption/kms_client_spec.rb +++ b/spec/services/encryption/kms_client_spec.rb @@ -12,29 +12,17 @@ ) # rubocop:disable Layout/LineLength - stub_mapped_aws_kms_client( - [ - { plaintext: 'a' * 3000, ciphertext: 'us-north-1:kms1', key_id: key_id, region: 'us-north-1' }, - { plaintext: 'b' * 3000, ciphertext: 'us-north-1:kms2', key_id: key_id, region: 'us-north-1' }, - { plaintext: 'c' * 3000, ciphertext: 'us-north-1:kms3', key_id: key_id, region: 'us-north-1' }, - ], - ) + if kms_enabled + stub_mapped_aws_kms_client( + [ + { plaintext: 'a' * 3000, ciphertext: 'us-north-1:kms1', key_id: key_id, region: 'us-north-1' }, + { plaintext: 'b' * 3000, ciphertext: 'us-north-1:kms2', key_id: key_id, region: 'us-north-1' }, + { plaintext: 'c' * 3000, ciphertext: 'us-north-1:kms3', key_id: key_id, region: 'us-north-1' }, + ], + ) + end # rubocop:enable Layout/LineLength - encryptor = Encryption::Encryptors::AesEncryptor.new - { - 'a' * 3000 => 'local1', - 'b' * 3000 => 'local2', - 'c' * 3000 => 'local3', - }.each do |plaintext, ciphertext| - allow(encryptor).to receive(:encrypt) - .with(plaintext, local_encryption_key) - .and_return(ciphertext) - allow(encryptor).to receive(:decrypt) - .with(ciphertext, local_encryption_key) - .and_return(plaintext) - end - allow(Encryption::Encryptors::AesEncryptor).to receive(:new).and_return(encryptor) allow(FeatureManagement).to receive(:use_kms?).and_return(kms_enabled) allow(IdentityConfig.store).to receive(:aws_region).and_return(aws_region) allow(IdentityConfig.store).to receive(:aws_kms_key_id).and_return(key_id) @@ -46,14 +34,6 @@ let(:encryption_context) { { 'context' => 'attribute-bundle', 'user_id' => '123-abc-456-def' } } let(:log_timestamp) { Time.utc(2025, 2, 28, 15, 30, 1) } - let(:local_encryption_key) do - OpenSSL::HMAC.digest( - 'sha256', - IdentityConfig.store.password_pepper, - '123-abc-456-defattribute-bundlecontextuser_id', - ) - end - let(:aws_region) { 'us-north-1' } let(:kms_ciphertext) do @@ -64,10 +44,6 @@ ].map { |c| Base64.strict_encode64(c) }.to_json end - let(:local_ciphertext) do - 'LOCc' + %w[local1 local2 local3].map { |c| Base64.strict_encode64(c) }.to_json - end - let(:kms_enabled) { true } describe '#encrypt' do @@ -111,7 +87,7 @@ it 'encrypts with a local key' do result = subject.encrypt(plaintext, encryption_context) - expect(result).to eq(local_ciphertext) + expect(result).to_not include(plaintext) end end @@ -136,8 +112,11 @@ end context 'with a ciphertext encrypted with a local key' do + let(:kms_enabled) { false } + it 'decrypts the ciphertext with a local key' do - result = subject.decrypt(local_ciphertext, encryption_context) + ciphertext = subject.encrypt(plaintext, encryption_context) + result = subject.decrypt(ciphertext, encryption_context) expect(result).to eq(plaintext) end diff --git a/spec/services/encryption/multi_region_kms_profile_migrator_spec.rb b/spec/services/encryption/multi_region_kms_profile_migrator_spec.rb new file mode 100644 index 00000000000..62c503890cb --- /dev/null +++ b/spec/services/encryption/multi_region_kms_profile_migrator_spec.rb @@ -0,0 +1,115 @@ +require 'rails_helper' + +RSpec.describe Encryption::MultiRegionKmsProfileMigrator do + let(:profile) { create(:profile, pii: pii) } + let(:user_password) { profile.user.password } + let(:personal_key) { PersonalKeyGenerator.new(profile.user).normalize(profile.personal_key) } + let(:pii) do + { + dob: '1920-01-01', + ssn: '666-66-1234', + first_name: 'Jane', + last_name: 'Doe', + zipcode: '20001', + } + end + + subject { described_class.new(profile) } + + before do + allow(IdentityConfig.store).to receive(:aws_kms_multi_region_read_enabled).and_return(true) + end + + describe '#migrate!' do + context 'for a user without multi-region ciphertexts' do + it 'migrates the single-region ciphertext and saves it to the profile' do + pii_encryptor = Encryption::Encryptors::PiiEncryptor.new(user_password) + profile.encrypted_pii = pii_encryptor.encrypt(pii.to_json, user_uuid: profile.user.uuid) + recovery_pii_encryptor = Encryption::Encryptors::PiiEncryptor.new(personal_key) + profile.encrypted_pii_recovery = recovery_pii_encryptor.encrypt( + pii.to_json, user_uuid: profile.user.uuid + ) + profile.encrypted_pii_multi_region = nil + profile.encrypted_pii_recovery_multi_region = nil + profile.save + subject.migrate! + + pii_encryptor = Encryption::Encryptors::PiiEncryptor.new(user_password) + + single_region_pii = pii_encryptor.decrypt( + profile.encrypted_pii, + user_uuid: profile.user.uuid, + ) + multi_region_pii = pii_encryptor.decrypt( + profile.encrypted_pii_multi_region, + user_uuid: profile.user.uuid, + ) + + expect(profile.encrypted_pii).to_not be_blank + expect(profile.encrypted_pii_multi_region).to_not be_blank + expect(profile.encrypted_pii).to_not eq(profile.encrypted_pii_multi_region) + expect(single_region_pii).to eq(multi_region_pii) + + pii_recovery_encryptor = Encryption::Encryptors::PiiEncryptor.new(personal_key) + + single_region_pii_recovery = pii_recovery_encryptor.decrypt( + profile.encrypted_pii_recovery, + user_uuid: profile.user.uuid, + ) + multi_region_pii_recovery = pii_recovery_encryptor.decrypt( + profile.encrypted_pii_recovery_multi_region, + user_uuid: profile.user.uuid, + ) + + expect(profile.encrypted_pii_recovery).to_not be_blank + expect(profile.encrypted_pii_recovery_multi_region).to_not be_blank + expect(profile.encrypted_pii_recovery).to_not eq( + profile.encrypted_pii_recovery_multi_region, + ) + + expect(single_region_pii_recovery).to eq(multi_region_pii_recovery) + end + end + + context 'for a user with multi-region ciphertexts' do + it 'does not modify the profile record' do + expect do + pii_encryptor = Encryption::Encryptors::PiiEncryptor.new(user_password) + profile.encrypted_pii = pii_encryptor.encrypt(pii.to_json, user_uuid: profile.user.uuid) + recovery_pii_encryptor = Encryption::Encryptors::PiiEncryptor.new(personal_key) + profile.encrypted_pii_recovery = recovery_pii_encryptor.encrypt( + pii.to_json, user_uuid: profile.user.uuid + ) + profile.save + + subject.migrate! + end.to_not change { + profile.attributes.slice( + :encrypted_pii, + :encrypted_pii_multi_region, + :encrypted_pii_recovery, + :encrypted_pii_recovery_multi_region, + ) + } + end + end + + context 'for a user without multi-region or single-region ciphertexts' do + before do + profile.update!( + encrypted_pii: nil, + encrypted_pii_multi_region: nil, + encrypted_pii_recovery: nil, + encrypted_pii_recovery_multi_region: nil, + ) + end + + it 'does not modify the profile record' do + expect { subject.migrate! }.to raise_error( + RuntimeError, + "Profile##{profile.id} is missing encrypted_pii or encrypted_pii_recovery", + ) + end + end + end +end