Read Pii encryption salt/cost from ciphertext

.reek

@@ -69,7 +69,6 @@ LongParameterList:
     - Idv::ProoferJob#perform
     - Idv::VendorResult#initialize
     - JWT
-    - Pii::Attributes#self.new_from_encrypted
 RepeatedConditional:
   exclude:
     - Users::ResetPasswordsController

app/models/profile.rb

@@ -33,39 +33,27 @@ def deactivate(reason)
   def decrypt_pii(password)
     Pii::Attributes.new_from_encrypted(
       encrypted_pii,
-      password: password,
-      salt: user.password_salt,
-      cost: user.password_cost
+      password: password
     )
   end
 
   def recover_pii(personal_key)
     Pii::Attributes.new_from_encrypted(
       encrypted_pii_recovery,
-      password: personal_key,
-      salt: user.recovery_salt,
-      cost: user.recovery_cost
+      password: personal_key
     )
   end
 
   def encrypt_pii(pii, password)
     ssn = pii.ssn
     self.ssn_signature = Pii::Fingerprinter.fingerprint(ssn) if ssn
-    self.encrypted_pii = pii.encrypted(
-      password: password,
-      salt: user.password_salt,
-      cost: user.password_cost
-    )
+    self.encrypted_pii = pii.encrypted(password)
     encrypt_recovery_pii(pii)
   end
 
   def encrypt_recovery_pii(pii)
     personal_key = personal_key_generator.create
-    self.encrypted_pii_recovery = pii.encrypted(
-      password: personal_key_generator.normalize(personal_key),
-      salt: user.recovery_salt,
-      cost: user.recovery_cost
-    )
+    self.encrypted_pii_recovery = pii.encrypted(personal_key_generator.normalize(personal_key))
     @personal_key = personal_key
   end
 

app/services/encryption/encryptors/pii_encryptor.rb

@@ -1,39 +1,66 @@
 module Encryption
   module Encryptors
     class PiiEncryptor
-      include Pii::Encodable
+      Ciphertext = Struct.new(:encrypted_data, :salt, :cost) do
+        include Pii::Encodable
+        class << self
+          include Pii::Encodable
+        end
 
-      def initialize(password:, salt:, cost: nil)
-        cost ||= Figaro.env.scrypt_cost
+        def self.parse_from_string(ciphertext_string)
+          parsed_json = JSON.parse(ciphertext_string)
+          new(extract_encrypted_data(parsed_json), parsed_json['salt'], parsed_json['cost'])
+        rescue JSON::ParserError
+          raise Pii::EncryptionError, 'ciphertext is not valid JSON'
+        end
+
+        def to_s
+          {
+            encrypted_data: encode(encrypted_data),
+            salt: salt,
+            cost: cost,
+          }.to_json
+        end
+
+        def self.extract_encrypted_data(parsed_json)
+          encoded_encrypted_data = parsed_json['encrypted_data']
+          raise Pii::EncryptionError, 'ciphertext invalid' unless valid_base64_encoding?(
+            encoded_encrypted_data
+          )
+          decode(encoded_encrypted_data)
+        end
+      end
+
+      def initialize(password)
+        @password = password
         @aes_cipher = Pii::Cipher.new
         @kms_client = KmsClient.new
-        @scrypt_password_digest = build_scrypt_password(password, salt, cost).digest
       end
 
       def encrypt(plaintext)
+        salt = Devise.friendly_token[0, 20]
+        cost = Figaro.env.scrypt_cost
+        aes_encryption_key = scrypt_password_digest(salt: salt, cost: cost)
         aes_encrypted_ciphertext = aes_cipher.encrypt(plaintext, aes_encryption_key)
         kms_encrypted_ciphertext = kms_client.encrypt(aes_encrypted_ciphertext)
-        encode(kms_encrypted_ciphertext)
+        Ciphertext.new(kms_encrypted_ciphertext, salt, cost).to_s
       end
 
-      def decrypt(ciphertext)
-        raise Pii::EncryptionError, 'ciphertext invalid' unless valid_base64_encoding?(ciphertext)
-        decoded_ciphertext = decode(ciphertext)
-        aes_encrypted_ciphertext = kms_client.decrypt(decoded_ciphertext)
+      def decrypt(ciphertext_string)
+        ciphertext = Ciphertext.parse_from_string(ciphertext_string)
+        aes_encrypted_ciphertext = kms_client.decrypt(ciphertext.encrypted_data)
+        aes_encryption_key = scrypt_password_digest(salt: ciphertext.salt, cost: ciphertext.cost)
         aes_cipher.decrypt(aes_encrypted_ciphertext, aes_encryption_key)
       end
 
       private
 
-      attr_reader :aes_cipher, :kms_client, :scrypt_password_digest
+      attr_reader :password, :aes_cipher, :kms_client
 
-      def build_scrypt_password(password, salt, cost)
+      def scrypt_password_digest(salt:, cost:)
         scrypt_salt = cost + OpenSSL::Digest::SHA256.hexdigest(salt)
         scrypted = SCrypt::Engine.hash_secret password, scrypt_salt, 32
-        SCrypt::Password.new(scrypted)
-      end
-
-      def aes_encryption_key
+        scrypt_password_digest = SCrypt::Password.new(scrypted).digest
         scrypt_password_digest[0...32]
       end
     end

app/services/pii/attributes.rb

@@ -18,12 +18,8 @@ def self.new_from_hash(hash)
       attrs
     end
 
-    def self.new_from_encrypted(encrypted, password:, salt:, cost:)
-      encryptor = Encryption::Encryptors::PiiEncryptor.new(
-        password: password,
-        salt: salt,
-        cost: cost
-      )
+    def self.new_from_encrypted(encrypted, password:)
+      encryptor = Encryption::Encryptors::PiiEncryptor.new(password)
       decrypted = encryptor.decrypt(encrypted)
       new_from_json(decrypted)
     end
@@ -39,12 +35,8 @@ def initialize(*args)
       assign_all_members
     end
 
-    def encrypted(password:, salt:, cost:)
-      encryptor = Encryption::Encryptors::PiiEncryptor.new(
-        password: password,
-        salt: salt,
-        cost: cost
-      )
+    def encrypted(password)
+      encryptor = Encryption::Encryptors::PiiEncryptor.new(password)
       encryptor.encrypt(to_json)
     end
 

spec/controllers/users/sessions_controller_spec.rb

@@ -271,7 +271,9 @@
       it 'deactivates profile if not de-cryptable' do
         user = create(:user, :signed_up)
         profile = create(:profile, :active, :verified, user: user, pii: { ssn: '1234' })
-        profile.update!(encrypted_pii: Base64.strict_encode64('nonsense'))
+        profile.update!(
+          encrypted_pii: { encrypted_data: Base64.strict_encode64('nonsense') }.to_json
+        )
 
         stub_analytics
         analytics_hash = {

spec/services/encryption/encryptors/pii_encryptor_spec.rb

@@ -2,10 +2,9 @@
 
 describe Encryption::Encryptors::PiiEncryptor do
   let(:password) { 'password' }
-  let(:salt) { 'n-pepa' }
   let(:plaintext) { 'Oooh baby baby' }
 
-  subject { described_class.new(password: password, salt: salt) }
+  subject { described_class.new(password) }
 
   describe '#encrypt' do
     it 'returns encrypted text' do
@@ -15,6 +14,9 @@
     end
 
     it 'uses the user access key encryptor to encrypt the plaintext' do
+      salt = '0' * 20
+      allow(Devise).to receive(:friendly_token).and_return(salt)
+
       scrypt_digest = '1' * 64
 
       scrypt_password = instance_double(SCrypt::Password)
@@ -37,7 +39,11 @@
 
       ciphertext = subject.encrypt(plaintext)
 
-      expect(ciphertext).to eq(expected_ciphertext)
+      expect(ciphertext).to eq({
+        encrypted_data: expected_ciphertext,
+        salt: salt,
+        cost: '800$8$1$',
+      }.to_json)
     end
   end
 
@@ -51,12 +57,15 @@
 
     it 'requires the same password used for encrypt' do
       ciphertext = subject.encrypt(plaintext)
-      new_encryptor = described_class.new(password: 'This is not the passowrd', salt: salt)
+      new_encryptor = described_class.new('This is not the passowrd')
 
       expect { new_encryptor.decrypt(ciphertext) }.to raise_error Pii::EncryptionError
     end
 
     it 'uses layered AES and KMS to decrypt the contents' do
+      salt = '0' * 20
+      allow(Devise).to receive(:friendly_token).and_return(salt)
+
       scrypt_digest = '1' * 64
 
       scrypt_password = instance_double(SCrypt::Password)
@@ -75,7 +84,11 @@
         with('aes_ciphertext', scrypt_digest[0...32]).
         and_return(plaintext)
 
-      result = subject.decrypt(Base64.strict_encode64('kms_ciphertext'))
+      result = subject.decrypt({
+        encrypted_data: Base64.strict_encode64('kms_ciphertext'),
+        salt: salt,
+        cost: '800$8$1$',
+      }.to_json)
 
       expect(result).to eq(plaintext)
     end

spec/services/pii/attributes_spec.rb

@@ -3,8 +3,6 @@
 describe Pii::Attributes do
   # let(:user_access_key) { Encryption::UserAccessKey.new(password: 'sekrit', salt: SecureRandom.uuid) }
   let(:password) { 'I am the password' }
-  let(:salt) { 'I am the salt' }
-  let(:cost) { '800$8$1$' }
 
   describe '#new_from_hash' do
     it 'initializes from plain Hash' do
@@ -34,20 +32,16 @@
   describe '#new_from_encrypted' do
     it 'inflates from encrypted string' do
       orig_attrs = described_class.new_from_hash(first_name: 'Jane')
-      encrypted_pii = orig_attrs.encrypted(password: password, salt: salt, cost: cost)
-      pii_attrs = described_class.new_from_encrypted(
-        encrypted_pii, password: password, salt: salt, cost: cost
-      )
+      encrypted_pii = orig_attrs.encrypted(password)
+      pii_attrs = described_class.new_from_encrypted(encrypted_pii, password: password)
 
       expect(pii_attrs.first_name).to eq 'Jane'
     end
 
     it 'allows deprecated attributes that are no longer added to the hash schema' do
       deprecated_atts = described_class.new_from_hash(otp: '123abc')
-      encrypted_pii = deprecated_atts.encrypted(password: password, salt: salt, cost: cost)
-      pii_attrs = described_class.new_from_encrypted(
-        encrypted_pii, password: password, salt: salt, cost: cost
-      )
+      encrypted_pii = deprecated_atts.encrypted(password)
+      pii_attrs = described_class.new_from_encrypted(encrypted_pii, password: password)
 
       expect(pii_attrs[:otp]).to eq('123abc')
     end
@@ -71,7 +65,7 @@
     it 'returns the object as encrypted string' do
       pii_attrs = described_class.new_from_hash(first_name: 'Jane')
 
-      encrypted = pii_attrs.encrypted(password: password, salt: salt, cost: cost)
+      encrypted = pii_attrs.encrypted(password)
       expect(encrypted).to_not match 'Jane'
     end
   end