diff --git a/app/services/encryption/encryptors/deprecated_attribute_encryptor.rb b/app/services/encryption/encryptors/deprecated_attribute_encryptor.rb index 9dda4049f97..50c3fb2904e 100644 --- a/app/services/encryption/encryptors/deprecated_attribute_encryptor.rb +++ b/app/services/encryption/encryptors/deprecated_attribute_encryptor.rb @@ -26,7 +26,9 @@ def self.load_or_init_user_access_key(key:, cost:) @_scypt_hashes_by_key ||= {} scrypt_hash = @_scypt_hashes_by_key["#{key}:#{cost}"] return UserAccessKey.new(scrypt_hash: scrypt_hash) if scrypt_hash.present? - uak = UserAccessKey.new(password: key, salt: key, cost: cost) + uak = UserAccessKey.new( + password: key, salt: OpenSSL::Digest::SHA256.hexdigest(key), cost: cost + ) @_scypt_hashes_by_key["#{key}:#{cost}"] = uak.as_scrypt_hash uak end diff --git a/app/services/encryption/encryptors/session_encryptor.rb b/app/services/encryption/encryptors/session_encryptor.rb index 748c0b1fd45..39e86ceeb56 100644 --- a/app/services/encryption/encryptors/session_encryptor.rb +++ b/app/services/encryption/encryptors/session_encryptor.rb @@ -17,7 +17,9 @@ def self.load_or_init_user_access_key end key = Figaro.env.session_encryption_key - user_access_key = UserAccessKey.new(password: key, salt: key) + user_access_key = UserAccessKey.new( + password: key, salt: OpenSSL::Digest::SHA256.hexdigest(key) + ) @user_access_key_scrypt_hash = user_access_key.as_scrypt_hash user_access_key end diff --git a/app/services/encryption/user_access_key.rb b/app/services/encryption/user_access_key.rb index 142e3cc47a9..5f16713ec80 100644 --- a/app/services/encryption/user_access_key.rb +++ b/app/services/encryption/user_access_key.rb @@ -64,11 +64,22 @@ def encrypted_password attr_writer :cost, :salt, :z1, :z2, :random_r, :masked_ciphertext, :cek def build_scrypt_password(password, salt, cost) - scrypt_salt = cost + OpenSSL::Digest::SHA256.hexdigest(salt) + scrypt_salt = cost + transform_password_salt_to_scrypt_salt(salt) scrypted = SCrypt::Engine.hash_secret password, scrypt_salt, 32 SCrypt::Password.new(scrypted) end + def transform_password_salt_to_scrypt_salt(salt) + # Legacy passwords had 20 byte salts, so we took a SHA256 digest to get + # to 32 bytes. While passwords exist with 20 byte salts, we will need this + # line, otherwise the passwords are effectively expired. + # + # Also note that the salt arg will have a length of 64 chars since it is + # a hex digest of 32 random bytes + return OpenSSL::Digest::SHA256.hexdigest(salt) if salt.length != 64 + salt + end + def kms_client KmsClient.new end diff --git a/spec/services/encryption/password_verifier_spec.rb b/spec/services/encryption/password_verifier_spec.rb index 36c09a39c33..e1d67eae9fd 100644 --- a/spec/services/encryption/password_verifier_spec.rb +++ b/spec/services/encryption/password_verifier_spec.rb @@ -41,6 +41,54 @@ expect(result).to eq(false) end + + it 'allows verification of a password with a 32 byte salt' do + # Once all new password digests are 32 bytes, this test can be torn down + # as it will be covered by the tests above + + password = 'saltypickles' + password_digest = { + encrypted_password: '8a5c5b165fd3a2fce81bc914d91a106b76dfdd9e8c2addf0e0f27424a32ca4cb', + encryption_key: 'VUl6QFRZeQZ5Xl9IUVsBZmRndkNkXHJDYgAFRX9nYVl8c3paUWhyX2p + oegBqaFgAeVpfWWpmcgRRXnJHfANAaFJdfQR/eHJbfXVASn0AYnx9dlRifAJ2BFF4dkFmdWZ + /Y3VASn0DfUlqZHp2aWdEAWZlWHRhWmZnY2dbAFMAfQFiW0hCVWNEAmFoan9TSnEEZUpcdmR + nanZ/dHZ/agJqR1VkWEd+XlgCUnRUXFFnYkdqXVQBZHRiUVMCantRAVRef3ZYZmNnW0JTdHJ + RfltYR353akVRAAV9VHZmAmUBZkBlXlxnZXVUe1RKWAJVXGp2Y1tqSmQBVFlnXWpiYkp6eWZ + dQEd/eFhHYWVYd2VKflhpA3lKZGZlSH5dal1+eHZJZWQACXlZR1lUd3ZeeVpfWVNnelhmewB + VZ2p7C3hqf3lhXV0XYDp0YmBnL0dlZQcKLQtUXA=='.gsub(/\s/, ''), + password_salt: '6bb7555423136772304b40c10afe11e459c4021a1a47dfd11fcc955e0a2161e2', + password_cost: '4000$8$4$', + }.to_json + + result = described_class.verify(password: password, digest: password_digest) + + expect(result).to eq(true) + end + + it 'allows verification of a legacy password with a 20 byte salt' do + # Legacy passwords had 20 bytes salts, which were SHA256 digested to get + # to a 32 byte salt (64 char hexdigest). This test verifies that the + # password verifier is capable of properly verifying those passwords + # using known values. + + password = 'saltypickles' + legacy_password_digest = { + encrypted_password: '6245274034d8d4dcc2d9930b431c4356aeaac9c7d0b7e2148ac19dcd12dfbc7a', + encryption_key: 'VUl6QFRZeQZ5XnZofXhTBGN3WABqdGZnamZTR30CeVl8c3paUWhyX2p + oegBqaFgAeVpfWWReZnhjd2pHZV1AX3wAQH1nW1hKY1t+BGd4agFnaF8AZFxASmd1SAV/ZXV + Kal1IUX50WAV7AFRpZ15hR1J4WANlZXZKZAB6BGYACQJSdURkYV1HA2JdanJRAFMEZQB+dGY + BVH5nWlhJZQNyflJnfgFqA2VJUV1IQ35dampVZVxbUwFmZWRZCWhSd0RyYwBEempnXFxmW3p + 8UQJ+An10XGJ/eGphU1kJY1FdZgJmaHpaZF5pSWV3XH5hZUhbfEp5SWp4cgRlZGpZY2ZUemV + bBXhnaFRqfgBDBGZnREN/aFx4fnZbR1J0aQJmZ0ABVEoACXlZR1lUd3ZeeVpfWX54VF10Gg0 + KZGA/XSt2L2YCbGMHYjNzXFBtDCUwMgdafV5RWA=='.gsub(/\s/, ''), + password_salt: 'u4KDouyiHwPgksKphAp1', + password_cost: '4000$8$4$', + }.to_json + + result = described_class.verify(password: password, digest: legacy_password_digest) + + expect(result).to eq(true) + end end it 'raises an encryption error when the password digest is nil' do diff --git a/spec/services/encryption/user_access_key_spec.rb b/spec/services/encryption/user_access_key_spec.rb index e39f3ffec5e..8a3f0a05c11 100644 --- a/spec/services/encryption/user_access_key_spec.rb +++ b/spec/services/encryption/user_access_key_spec.rb @@ -2,23 +2,22 @@ describe Encryption::UserAccessKey do let(:password) { 'this is a password' } - let(:password_salt) { 'this is a salt' } - + let(:salt) { '1' * 64 } # hex encoded 32 random bytes let(:cost) { '800$8$1$' } - let(:scrypt_salt) { 'bd305e29843227105aa7c820ddef8e2a6b4c88831abd84a8702370d401b44245' } - let(:z1) { '0a9bcfee214c15a6bbafef7204a0af88' } - let(:z2) { '8747755bcd92f295330e438059163eb1' } - let(:scrypt_hash) { "#{cost}#{scrypt_salt}$#{z1}#{z2}" } + + let(:z1) { 'a0db7e92c1cfe24df10cc1e1dbc17831' } + let(:z2) { '9fd0149eed6c9f42d3aa16ec23ae7317' } + let(:scrypt_hash) { "#{cost}#{salt}$#{z1}#{z2}" } let(:random_r) { '1' * 32 } let(:encrypted_random_r) { '2' * 128 } let(:encryption_key) do 'e31jSAICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgI - CAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgJTC1BRVFdXAAMGUQM - HUwRQUFNUV1QFAAIGUwJTVAoK'.gsub(/\s/, '') + CAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAlMCVlAFVwsAUQNRVFc + ABlZUAwJRUQNXA1ZQUQMFCgED'.gsub(/\s/, '') end - let(:cek) { '7374ebc97ba0f2b7a38b03fd76b2faafadded15c78c16dafaa0cf8e5e4740ff5' } - let(:encrypted_password) { '2e1ec56ad48694902e0b96d0979135363c14bd443707200c931358849bcb0a94' } + let(:cek) { 'a863b23a356db1619b1ae6fa565c6b6f4c6d52cdb8ec728b19306aeb9131ade5' } + let(:encrypted_password) { '8ffa77a1504706acfcb16d67d3cf5b8dc859f6bbbde14435223d73b0a5803eb2' } before do allow(FeatureManagement).to receive(:use_kms?).and_return(true) @@ -30,10 +29,10 @@ describe '.new' do it 'allows creation of a uak using password and salt' do - uak = described_class.new(password: password, salt: password_salt) + uak = described_class.new(password: password, salt: salt) expect(uak.cost).to eq(cost) - expect(uak.salt).to eq(scrypt_salt) + expect(uak.salt).to eq(salt) expect(uak.z1).to eq(z1) expect(uak.z2).to eq(z2) expect(uak.as_scrypt_hash).to eq(scrypt_hash) @@ -43,11 +42,36 @@ uak = described_class.new(scrypt_hash: scrypt_hash) expect(uak.cost).to eq(cost) - expect(uak.salt).to eq(scrypt_salt) + expect(uak.salt).to eq(salt) expect(uak.z1).to eq(z1) expect(uak.z2).to eq(z2) expect(uak.as_scrypt_hash).to eq(scrypt_hash) end + + context 'with a legacy password with a 20 byte salt' do + # Legacy passwords had 20 bytes salts, which were SHA256 digested to get + # to a 32 byte salt (64 char hexdigest). This test verifies that the + # UAK behaves properly when used to verify those legacy passwords + + let(:password) { 'this is a password' } + let(:salt) { '1' * 20 } + + let(:cost) { '800$8$1$' } + let(:digested_salt) { 'd1b3707fbdc6a22d16e95bf6b910646f5d9c2b3ed81bd637d454ffb9bb0948e4' } + let(:z1) { '76ad344efc442269ec28aaa28457ead2' } + let(:z2) { '75442e6f2354b60f4f2b40de8cdc92bb' } + let(:scrypt_hash) { "#{cost}#{digested_salt}$#{z1}#{z2}" } + + it 'can successfully create a uak using the password and the salt' do + uak = described_class.new(password: password, salt: salt) + + expect(uak.cost).to eq(cost) + expect(uak.z1).to eq(z1) + expect(uak.salt).to eq(digested_salt) + expect(uak.z2).to eq(z2) + expect(uak.as_scrypt_hash).to eq(scrypt_hash) + end + end end describe '#build' do