From 6261d9a1cf416927d10999cdf79ffa0e87f071c8 Mon Sep 17 00:00:00 2001 From: Jonathan Hooper Date: Wed, 28 Jun 2023 16:26:09 -0400 Subject: [PATCH 1/2] Retire the v2 session encryptor We have migrated from the v2 encryptor to the v3 encryptor so the v2 encryptor can be safely retired. changelog: Internal, Session encryption, The v2 session encryptor was removed in favor of the v3 session encryptor. --- config/application.yml.default | 1 - lib/identity_config.rb | 1 - lib/legacy_session_encryptor.rb | 150 -------------- lib/session_encryptor.rb | 11 - spec/lib/session_encryptor_spec.rb | 312 +++++++++++++---------------- 5 files changed, 137 insertions(+), 338 deletions(-) delete mode 100644 lib/legacy_session_encryptor.rb diff --git a/config/application.yml.default b/config/application.yml.default index 0d7086794ae..b8409480e14 100644 --- a/config/application.yml.default +++ b/config/application.yml.default @@ -316,7 +316,6 @@ service_provider_request_ttl_hours: 24 session_check_delay: 30 session_check_frequency: 30 session_encryptor_alert_enabled: false -session_encryptor_v3_enabled: false session_timeout_in_minutes: 15 session_timeout_warning_seconds: 150 session_total_duration_timeout_in_minutes: 720 diff --git a/lib/identity_config.rb b/lib/identity_config.rb index 14cd4fbe5fd..64360cfb9ad 100644 --- a/lib/identity_config.rb +++ b/lib/identity_config.rb @@ -432,7 +432,6 @@ def self.build_store(config_map) config.add(:session_check_frequency, type: :integer) config.add(:session_encryption_key, type: :string) config.add(:session_encryptor_alert_enabled, type: :boolean) - config.add(:session_encryptor_v3_enabled, type: :boolean) config.add(:session_timeout_in_minutes, type: :integer) config.add(:session_timeout_warning_seconds, type: :integer) config.add(:session_total_duration_timeout_in_minutes, type: :integer) diff --git a/lib/legacy_session_encryptor.rb b/lib/legacy_session_encryptor.rb deleted file mode 100644 index ce54b980219..00000000000 --- a/lib/legacy_session_encryptor.rb +++ /dev/null @@ -1,150 +0,0 @@ -# frozen_string_literal: true - -class LegacySessionEncryptor - CIPHERTEXT_HEADER = 'v2' - def load(value) - _v2, ciphertext = value.split(':') - decrypted = outer_decrypt(ciphertext) - - session = JSON.parse(decrypted, quirks_mode: true).with_indifferent_access - kms_decrypt_sensitive_paths!(session) - - session - end - - def dump(value) - value.deep_stringify_keys! - - kms_encrypt_pii!(value) - kms_encrypt_sensitive_paths!(value, SessionEncryptor::SENSITIVE_PATHS) - alert_or_raise_if_contains_sensitive_keys!(value) - plain = JSON.generate(value, quirks_mode: true) - alert_or_raise_if_contains_sensitive_value!(plain, value) - CIPHERTEXT_HEADER + ':' + outer_encrypt(plain) - end - - def kms_encrypt(text) - Base64.encode64(Encryption::KmsClient.new.encrypt(text, 'context' => 'session-encryption')) - end - - def kms_decrypt(text) - Encryption::KmsClient.new.decrypt( - Base64.decode64(text), 'context' => 'session-encryption' - ) - end - - def outer_encrypt(plaintext) - Encryption::Encryptors::AesEncryptor.new.encrypt(plaintext, session_encryption_key) - end - - def outer_decrypt(ciphertext) - Encryption::Encryptors::AesEncryptor.new.decrypt(ciphertext, session_encryption_key) - end - - private - - # The PII bundle is stored in the user session in the 'decrypted_pii' key. - # The PII is decrypted with the user's password when they successfully submit it and then - # stored in the session. Before saving the session, this method encrypts the PII with KMS and - # stores it in the 'encrypted_pii' key. - # - # The PII is not frequently needed in its KMS-decrypted state. To reduce the - # risks around holding plaintext PII in memory during requests, this PII is KMS-decrypted - # on-demand by the Pii::Cacher. - def kms_encrypt_pii!(session) - return unless session.dig('warden.user.user.session', 'decrypted_pii') - decrypted_pii = session['warden.user.user.session'].delete('decrypted_pii') - session['warden.user.user.session']['encrypted_pii'] = - kms_encrypt(decrypted_pii) - nil - end - - # This method extracts all of the sensitive paths that exist into a - # separate hash. This separate hash is then encrypted and placed in the session. - # We use #reduce to build the nested empty hash if needed. If Hash#bury - # (https://bugs.ruby-lang.org/issues/11747) existed, we could use that instead. - def kms_encrypt_sensitive_paths!(session, sensitive_paths) - sensitive_data = {} - - sensitive_paths.each do |path| - *all_but_last_key, last_key = path - - if all_but_last_key.blank? - value = session.delete(last_key) - else - value = session.dig(*all_but_last_key)&.delete(last_key) - end - - if value - all_but_last_key.reduce(sensitive_data) do |hash, key| - hash[key] ||= {} - - hash[key] - end - - if all_but_last_key.blank? - sensitive_data[last_key] = value - else - sensitive_data.dig(*all_but_last_key).store(last_key, value) - end - end - end - - raise "invalid session, 'sensitive_data' is reserved key" if session['sensitive_data'].present? - return if sensitive_data.blank? - session['sensitive_data'] = kms_encrypt(JSON.generate(sensitive_data)) - end - - # This method reverses the steps taken in #kms_encrypt_sensitive_paths! - # The encrypted hash is decrypted and then deep merged into the session hash. - # The merge must be a deep merge to avoid collisions with existing hashes in the - # session. - def kms_decrypt_sensitive_paths!(session) - sensitive_data = session.delete('sensitive_data') - return if sensitive_data.blank? - - sensitive_data = JSON.parse( - kms_decrypt(sensitive_data), quirks_mode: true - ) - - session.deep_merge!(sensitive_data) - end - - def alert_or_raise_if_contains_sensitive_value!(string, hash) - if SessionEncryptor::SENSITIVE_REGEX.match?(string) - exception = SessionEncryptor::SensitiveValueError.new - if IdentityConfig.store.session_encryptor_alert_enabled - NewRelic::Agent.notice_error( - exception, custom_params: { - session_structure: hash.deep_transform_values { |v| nil }, - } - ) - else - raise exception - end - end - end - - def alert_or_raise_if_contains_sensitive_keys!(hash) - hash.deep_transform_keys do |key| - if SessionEncryptor::SENSITIVE_KEYS.include?(key.to_s) - exception = SessionEncryptor::SensitiveKeyError.new( - "#{key} unexpectedly appeared in session", - ) - if IdentityConfig.store.session_encryptor_alert_enabled - NewRelic::Agent.notice_error( - exception, custom_params: { - session_structure: hash.deep_transform_values { |_v| '' }, - } - ) - else - raise exception - end - end - end - end - - def session_encryption_key - IdentityConfig.store.session_encryption_key - end -end diff --git a/lib/session_encryptor.rb b/lib/session_encryptor.rb index 7321410f9e2..0f09f540940 100644 --- a/lib/session_encryptor.rb +++ b/lib/session_encryptor.rb @@ -43,8 +43,6 @@ class SensitiveValueError < StandardError; end SENSITIVE_REGEX = %r{#{SENSITIVE_DEFAULT_FIELDS.join('|')}}i def load(value) - return LegacySessionEncryptor.new.load(value) if should_use_legacy_encryptor_for_read?(value) - payload = MessagePack.unpack(value) ciphertext = payload[CIPHERTEXT_KEY] compressed = payload[COMPRESSED_KEY] @@ -62,7 +60,6 @@ def load(value) end def dump(value) - return LegacySessionEncryptor.new.dump(value) if should_use_legacy_encryptor_for_write? value.deep_stringify_keys! kms_encrypt_pii!(value) @@ -206,18 +203,10 @@ def alert_or_raise_if_contains_sensitive_keys!(hash) end end - def should_use_legacy_encryptor_for_read?(value) - value.start_with?(LegacySessionEncryptor::CIPHERTEXT_HEADER) - end - def should_compress?(value) value.bytesize >= MINIMUM_COMPRESS_LIMIT end - def should_use_legacy_encryptor_for_write? - !IdentityConfig.store.session_encryptor_v3_enabled - end - def session_encryption_key IdentityConfig.store.session_encryption_key end diff --git a/spec/lib/session_encryptor_spec.rb b/spec/lib/session_encryptor_spec.rb index 62592372afa..7220adb8f7a 100644 --- a/spec/lib/session_encryptor_spec.rb +++ b/spec/lib/session_encryptor_spec.rb @@ -3,210 +3,172 @@ RSpec.describe SessionEncryptor do subject { SessionEncryptor.new } describe '#load' do - context 'with a legacy session ciphertext' do - it 'decrypts the legacy session' do - session = { 'foo' => 'bar' } + it 'decrypts the session' do + session = { 'foo' => 'bar' } - ciphertext = LegacySessionEncryptor.new.dump(session) + ciphertext = SessionEncryptor.new.dump(session) - result = subject.load(ciphertext) + result = subject.load(ciphertext) - expect(result).to eq(session) - end - end - - context 'with version 3 encryption enabled' do - before do - allow(IdentityConfig.store).to receive(:session_encryptor_v3_enabled).and_return(true) - end - - it 'decrypts the new version of the session' do - session = { 'foo' => 'bar' } - - ciphertext = SessionEncryptor.new.dump(session) - - result = subject.load(ciphertext) - - expect(result).to eq session - end + expect(result).to eq session end end describe '#dump' do - context 'with version 3 encryption enabled' do - before do - allow(IdentityConfig.store).to receive(:session_encryptor_v3_enabled).and_return(true) - end - - it 'transparently encrypts/decrypts sensitive elements of the session' do - session = { 'warden.user.user.session' => { + it 'transparently encrypts/decrypts sensitive elements of the session' do + session = { 'warden.user.user.session' => { + 'idv' => { 'ssn' => '666-66-6666' }, + 'idv/doc_auth' => { 'ssn' => '666-66-6666' }, + 'other_value' => 42, + } } + + ciphertext = subject.dump(session) + + result = subject.load(ciphertext) + expect(result).to eq( + { 'warden.user.user.session' => { 'idv' => { 'ssn' => '666-66-6666' }, 'idv/doc_auth' => { 'ssn' => '666-66-6666' }, 'other_value' => 42, - } } + } }, + ) + end - ciphertext = subject.dump(session) + it 'encrypts decrypted_pii bundle without automatically decrypting' do + session = { 'warden.user.user.session' => { + 'decrypted_pii' => { 'ssn' => '666-66-6666' }.to_json, + } } - result = subject.load(ciphertext) - expect(result).to eq( - { 'warden.user.user.session' => { - 'idv' => { 'ssn' => '666-66-6666' }, - 'idv/doc_auth' => { 'ssn' => '666-66-6666' }, - 'other_value' => 42, - } }, - ) - end + ciphertext = subject.dump(session) - it 'encrypts decrypted_pii bundle without automatically decrypting' do - session = { 'warden.user.user.session' => { - 'decrypted_pii' => { 'ssn' => '666-66-6666' }.to_json, - } } + result = subject.load(ciphertext) - ciphertext = subject.dump(session) + expect(result.fetch('warden.user.user.session')['decrypted_pii']).to eq nil + expect(result.fetch('warden.user.user.session')['encrypted_pii']).to_not eq nil + end - result = subject.load(ciphertext) + it 'can decrypt PII bundle with Pii::Cacher' do + session = { 'warden.user.user.session' => { + 'decrypted_pii' => { 'ssn' => '666-66-6666' }.to_json, + } } - expect(result.fetch('warden.user.user.session')['decrypted_pii']).to eq nil - expect(result.fetch('warden.user.user.session')['encrypted_pii']).to_not eq nil - end + ciphertext = subject.dump(session) - it 'can decrypt PII bundle with Pii::Cacher' do - session = { 'warden.user.user.session' => { - 'decrypted_pii' => { 'ssn' => '666-66-6666' }.to_json, - } } + result = subject.load(ciphertext) + pii_cacher = Pii::Cacher.new(nil, result.fetch('warden.user.user.session')) - ciphertext = subject.dump(session) + expect(result.fetch('warden.user.user.session')['decrypted_pii']).to eq nil + expect(result.fetch('warden.user.user.session')['encrypted_pii']).to_not eq nil - result = subject.load(ciphertext) - pii_cacher = Pii::Cacher.new(nil, result.fetch('warden.user.user.session')) + pii_cacher.fetch + expect(JSON.parse(result.fetch('warden.user.user.session')['decrypted_pii'])).to eq( + { + 'ssn' => '666-66-6666', + }, + ) + end - expect(result.fetch('warden.user.user.session')['decrypted_pii']).to eq nil - expect(result.fetch('warden.user.user.session')['encrypted_pii']).to_not eq nil + it 'KMS encrypts/decrypts doc auth elements of the session' do + session = { 'warden.user.user.session' => { + 'idv' => { 'ssn' => '666-66-6666' }, + 'idv/doc_auth' => { 'ssn' => '666-66-6666' }, + 'other_value' => 42, + } } + ciphertext = subject.dump(session) + + partially_decrypted = Zlib.gunzip( + subject.outer_decrypt(MessagePack.unpack(ciphertext)[SessionEncryptor::CIPHERTEXT_KEY]), + ) + partially_decrypted_json = JSON.parse(partially_decrypted) + + expect(partially_decrypted_json.fetch('warden.user.user.session')['idv']).to eq nil + expect(partially_decrypted_json.fetch('warden.user.user.session')['idv/doc_auth']).to eq nil + expect( + partially_decrypted_json.fetch('sensitive_data'), + ).to_not eq nil + + expect( + partially_decrypted_json.fetch('warden.user.user.session')['other_value'], + ).to eq 42 + end - pii_cacher.fetch - expect(JSON.parse(result.fetch('warden.user.user.session')['decrypted_pii'])).to eq( - { - 'ssn' => '666-66-6666', - }, - ) - end + it 'does not compress when payload is small' do + session = { 'a' => 0 } + ciphertext = subject.dump(session) - it 'KMS encrypts/decrypts doc auth elements of the session' do - session = { 'warden.user.user.session' => { - 'idv' => { 'ssn' => '666-66-6666' }, - 'idv/doc_auth' => { 'ssn' => '666-66-6666' }, + session_payload = MessagePack.unpack(ciphertext) + expect(session_payload[SessionEncryptor::COMPRESSED_KEY]).to eq 0 + session_decrypted = JSON.parse( + subject.outer_decrypt(session_payload[SessionEncryptor::CIPHERTEXT_KEY]), + ) + expect(session_decrypted).to eq session + end + + it 'raises if reserved key is used' do + session = { + 'sensitive_data' => 'test', + 'warden.user.user.session' => { 'other_value' => 42, - } } - ciphertext = subject.dump(session) - - partially_decrypted = Zlib.gunzip( - subject.outer_decrypt(MessagePack.unpack(ciphertext)[SessionEncryptor::CIPHERTEXT_KEY]), - ) - partially_decrypted_json = JSON.parse(partially_decrypted) - - expect(partially_decrypted_json.fetch('warden.user.user.session')['idv']).to eq nil - expect(partially_decrypted_json.fetch('warden.user.user.session')['idv/doc_auth']).to eq nil - expect( - partially_decrypted_json.fetch('sensitive_data'), - ).to_not eq nil - - expect( - partially_decrypted_json.fetch('warden.user.user.session')['other_value'], - ).to eq 42 - end - - it 'does not compress when payload is small' do - session = { 'a' => 0 } - ciphertext = subject.dump(session) - - session_payload = MessagePack.unpack(ciphertext) - expect(session_payload[SessionEncryptor::COMPRESSED_KEY]).to eq 0 - session_decrypted = JSON.parse( - subject.outer_decrypt(session_payload[SessionEncryptor::CIPHERTEXT_KEY]), - ) - expect(session_decrypted).to eq session - end - - it 'raises if reserved key is used' do - session = { - 'sensitive_data' => 'test', - 'warden.user.user.session' => { - 'other_value' => 42, - }, - } - - expect do - subject.dump(session) - end.to raise_error( - RuntimeError, "invalid session, 'sensitive_data' is reserved key" - ) - end - - it 'raises if PII key appears outside of expected areas when alerting is disabled' do - nested_session = { 'warden.user.user.session' => { - 'idv_new' => { 'nested' => { 'ssn' => '666-66-6666' } }, - } } - - nested_array_session = { 'warden.user.user.session' => { - 'idv_new' => [{ 'nested' => { 'ssn' => '666-66-6666' } }], - } } - - expect do - subject.dump(nested_session) - end.to raise_error( - SessionEncryptor::SensitiveKeyError, 'ssn unexpectedly appeared in session' - ) - - expect do - subject.dump(nested_array_session) - end.to raise_error( - SessionEncryptor::SensitiveKeyError, 'ssn unexpectedly appeared in session' - ) - end - - it 'sends alert if PII key appears outside of expected areas if alerting is enabled' do - allow(IdentityConfig.store).to receive(:session_encryptor_alert_enabled).and_return(true) - session = { 'warden.user.user.session' => { - 'idv_new' => { 'nested' => { 'ssn' => '666-66-6666' } }, - } } - - expect(NewRelic::Agent).to receive(:notice_error).with( - SessionEncryptor::SensitiveKeyError.new('ssn unexpectedly appeared in session'), - custom_params: { - session_structure: { 'warden.user.user.session' => { - 'idv_new' => { 'nested' => { 'ssn' => '' } }, - } }, - }, - ) + }, + } + expect do subject.dump(session) - end - - it 'raises if sensitive value is not KMS encrypted' do - session = { - 'new_key' => Idp::Constants::MOCK_IDV_APPLICANT[:last_name], - } - - expect do - subject.dump(session) - end.to raise_error( - SessionEncryptor::SensitiveValueError, - ) - end + end.to raise_error( + RuntimeError, "invalid session, 'sensitive_data' is reserved key" + ) end - context 'without version 3 encryption enabled' do - before do - allow(IdentityConfig.store).to receive(:session_encryptor_v3_enabled).and_return(false) - end + it 'raises if PII key appears outside of expected areas when alerting is disabled' do + nested_session = { 'warden.user.user.session' => { + 'idv_new' => { 'nested' => { 'ssn' => '666-66-6666' } }, + } } + + nested_array_session = { 'warden.user.user.session' => { + 'idv_new' => [{ 'nested' => { 'ssn' => '666-66-6666' } }], + } } + + expect do + subject.dump(nested_session) + end.to raise_error( + SessionEncryptor::SensitiveKeyError, 'ssn unexpectedly appeared in session' + ) + + expect do + subject.dump(nested_array_session) + end.to raise_error( + SessionEncryptor::SensitiveKeyError, 'ssn unexpectedly appeared in session' + ) + end - it 'encrypts the session with the legacy encryptor' do - session = { 'foo' => 'bar' } - ciphertext = subject.dump(session) - decrypted_session = LegacySessionEncryptor.new.load(ciphertext) + it 'sends alert if PII key appears outside of expected areas if alerting is enabled' do + allow(IdentityConfig.store).to receive(:session_encryptor_alert_enabled).and_return(true) + session = { 'warden.user.user.session' => { + 'idv_new' => { 'nested' => { 'ssn' => '666-66-6666' } }, + } } + + expect(NewRelic::Agent).to receive(:notice_error).with( + SessionEncryptor::SensitiveKeyError.new('ssn unexpectedly appeared in session'), + custom_params: { + session_structure: { 'warden.user.user.session' => { + 'idv_new' => { 'nested' => { 'ssn' => '' } }, + } }, + }, + ) + + subject.dump(session) + end - expect(decrypted_session).to eq(session) - end + it 'raises if sensitive value is not KMS encrypted' do + session = { + 'new_key' => Idp::Constants::MOCK_IDV_APPLICANT[:last_name], + } + + expect do + subject.dump(session) + end.to raise_error( + SessionEncryptor::SensitiveValueError, + ) end end From 003bbc6041f5ab5c9ef8c1feeb7b8919708f1422 Mon Sep 17 00:00:00 2001 From: Jonathan Hooper Date: Wed, 28 Jun 2023 16:38:35 -0400 Subject: [PATCH 2/2] whoops --- config/initializers/session_store.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/config/initializers/session_store.rb b/config/initializers/session_store.rb index 3e2c4d07385..46321d2b554 100644 --- a/config/initializers/session_store.rb +++ b/config/initializers/session_store.rb @@ -1,5 +1,4 @@ require 'session_encryptor' -require 'legacy_session_encryptor' APPLICATION_SESSION_COOKIE_KEY = '_identity_idp_session'.freeze