From df69441761834ecce757d6820f97c05b2456a143 Mon Sep 17 00:00:00 2001 From: Jonathan Hooper Date: Fri, 15 Dec 2023 10:51:55 -0500 Subject: [PATCH 1/2] Remove the passive encryption of encrypted PII in the session In #9754 the code that depended on the passive encryption of `decrypted_pii` in the session was removed. Some of the code removed here was left behind to ensure that PII remained encrypted. Once the code in #9754 is deployed to production this should be safe to merge. [skip changelog] --- app/services/pii/cacher.rb | 2 -- lib/session_encryptor.rb | 17 ----------------- spec/lib/session_encryptor_spec.rb | 13 ------------- 3 files changed, 32 deletions(-) diff --git a/app/services/pii/cacher.rb b/app/services/pii/cacher.rb index 4f39dbebeef..4d22331d1e8 100644 --- a/app/services/pii/cacher.rb +++ b/app/services/pii/cacher.rb @@ -36,8 +36,6 @@ def exists_in_session? end def delete - user_session.delete(:decrypted_pii) - user_session.delete(:encrypted_pii) user_session.delete(:encrypted_profiles) end diff --git a/lib/session_encryptor.rb b/lib/session_encryptor.rb index 77b94875441..919c06475d3 100644 --- a/lib/session_encryptor.rb +++ b/lib/session_encryptor.rb @@ -61,7 +61,6 @@ def load(value) def dump(value) value.deep_stringify_keys! - kms_encrypt_pii!(value) kms_encrypt_sensitive_paths!(value, SENSITIVE_PATHS) alert_or_raise_if_contains_sensitive_keys!(value) plain = JSON.generate(value) @@ -104,22 +103,6 @@ def outer_decrypt(ciphertext) 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 diff --git a/spec/lib/session_encryptor_spec.rb b/spec/lib/session_encryptor_spec.rb index 5c2c5e38a18..048351aa390 100644 --- a/spec/lib/session_encryptor_spec.rb +++ b/spec/lib/session_encryptor_spec.rb @@ -34,19 +34,6 @@ ) end - it 'encrypts decrypted_pii bundle without automatically decrypting' do - session = { 'warden.user.user.session' => { - 'decrypted_pii' => { 'ssn' => '666-66-6666' }.to_json, - } } - - ciphertext = subject.dump(session) - - result = subject.load(ciphertext) - - 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 - it 'KMS encrypts/decrypts doc auth elements of the session' do session = { 'warden.user.user.session' => { 'idv' => { 'ssn' => '666-66-6666' }, From 8962716ac0b6098cfbd358176af16ee3dff34650 Mon Sep 17 00:00:00 2001 From: Jonathan Hooper Date: Tue, 19 Dec 2023 17:10:45 -0500 Subject: [PATCH 2/2] a few touchups from one last once over --- app/services/out_of_band_session_accessor.rb | 1 - spec/controllers/idv/sessions_controller_spec.rb | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/app/services/out_of_band_session_accessor.rb b/app/services/out_of_band_session_accessor.rb index 556125e8cc6..319053c4659 100644 --- a/app/services/out_of_band_session_accessor.rb +++ b/app/services/out_of_band_session_accessor.rb @@ -64,7 +64,6 @@ def put_empty_user_session(expiration = 5.minutes) # @param [#to_s] profile_id def put_pii(profile_id:, pii:, expiration: 5.minutes) data = { - decrypted_pii: pii.to_h.to_json, encrypted_profiles: { profile_id.to_s => SessionEncryptor.new.kms_encrypt(pii.to_h.to_json) }, } diff --git a/spec/controllers/idv/sessions_controller_spec.rb b/spec/controllers/idv/sessions_controller_spec.rb index 909694f0ca1..ea5331427dd 100644 --- a/spec/controllers/idv/sessions_controller_spec.rb +++ b/spec/controllers/idv/sessions_controller_spec.rb @@ -35,7 +35,7 @@ expect(controller.user_session['idv/in_person']).to be_blank end - it 'clears the decrypted_pii session' do + it 'clears the encrypted_profiles session' do expect(controller.user_session[:encrypted_profiles]).to be_blank end end