feat(encryptor): abstract underlying encryption library + fix key parameter

There was also an error in the way we were using the actual key value.
We were passing an EncryptionKey to the native library... This would
have probably crash with the real library.

Author: ccharly

app/core/Encryptor/Encryptor.test.ts

@@ -140,12 +140,6 @@ describe('Encryptor', () => {
           lib,
         };
 
-        const expectedKey: EncryptionKey = {
-          key: expectedKeyValue,
-          keyMetadata: keyMetadata ?? KEY_DERIVATION_LEGACY_OPTIONS,
-          lib,
-        };
-
         const decryptedObject = await encryptor.decrypt(
           password,
           JSON.stringify(
@@ -160,7 +154,11 @@ describe('Encryptor', () => {
           lib === ENCRYPTION_LIBRARY.original
             ? decryptAesSpy
             : decryptAesForkedSpy,
-        ).toHaveBeenCalledWith(mockVault.cipher, expectedKey, mockVault.iv);
+        ).toHaveBeenCalledWith(
+          mockVault.cipher,
+          expectedKeyValue,
+          mockVault.iv,
+        );
         expect(
           lib === ENCRYPTION_LIBRARY.original
             ? pbkdf2AesSpy

app/core/Encryptor/Encryptor.ts

@@ -1,8 +1,6 @@
-import { NativeModules } from 'react-native';
 import { hasProperty, isPlainObject, Json } from '@metamask/utils';
 import {
   SALT_BYTES_COUNT,
-  SHA256_DIGEST_LENGTH,
   ENCRYPTION_LIBRARY,
   KEY_DERIVATION_LEGACY_OPTIONS,
   KeyDerivationIteration,
@@ -13,9 +11,7 @@ import type {
   EncryptionResult,
   KeyDerivationOptions,
 } from './types';
-
-const Aes = NativeModules.Aes;
-const AesForked = NativeModules.AesForked;
+import { getEncryptionLibrary } from './lib';
 
 /**
  * Checks if the provided object is a `KeyDerivationOptions`.
@@ -96,17 +92,6 @@ class Encryptor implements WithKeyEncryptor<EncryptionKey, Json> {
     return btoa(String.fromCharCode.apply(null, view as unknown as number[]));
   };
 
-  /**
-   * Generates a random IV.
-   *
-   * @param size - The number of bytes for the IV.
-   * @returns The generated IV.
-   */
-  private generateIV = async (size: number): Promise<unknown> =>
-    // Naming isn't perfect here, but this is how the library generates random IV (and encodes it the right way)
-    // See: https://www.npmjs.com/package/react-native-aes-crypto#example
-    await Aes.randomKey(size);
-
   /**
    * Generate an encryption key from a password and random salt, specifying
    * key derivation options.
@@ -123,15 +108,7 @@ class Encryptor implements WithKeyEncryptor<EncryptionKey, Json> {
     opts: KeyDerivationOptions,
     lib = ENCRYPTION_LIBRARY.original,
   ): Promise<EncryptionKey> => {
-    const key =
-      lib === ENCRYPTION_LIBRARY.original
-        ? await Aes.pbkdf2(
-            password,
-            salt,
-            opts.params.iterations,
-            SHA256_DIGEST_LENGTH,
-          )
-        : await AesForked.pbkdf2(password, salt);
+    const key = await getEncryptionLibrary(lib).deriveKey(password, salt, opts);
 
     return {
       key,
@@ -151,14 +128,18 @@ class Encryptor implements WithKeyEncryptor<EncryptionKey, Json> {
     key: EncryptionKey,
     data: Json,
   ): Promise<EncryptionResult> => {
-    const iv = await this.generateIV(16);
+    const text = JSON.stringify(data);
 
-    return Aes.encrypt(data, key, iv).then((cipher: string) => ({
+    const lib = getEncryptionLibrary(key.lib);
+    const iv = await lib.generateIV(16);
+    const cipher = await lib.encrypt(text, key.key, iv);
+
+    return {
       cipher,
       iv,
       keyMetadata: key.keyMetadata,
       lib: key.lib,
-    }));
+    };
   };
 
   /**
@@ -173,10 +154,10 @@ class Encryptor implements WithKeyEncryptor<EncryptionKey, Json> {
     payload: EncryptionResult,
   ): Promise<unknown> => {
     // TODO: Check for key and payload compatiblity?
-    const text =
-      payload.lib === ENCRYPTION_LIBRARY.original
-        ? await Aes.decrypt(payload.cipher, key, payload.iv)
-        : await AesForked.decrypt(payload.cipher, key, payload.iv);
+
+    // We assume that both `payload.lib` and `key.lib` are the same here!
+    const lib = getEncryptionLibrary(payload.lib);
+    const text = await lib.decrypt(payload.cipher, key.key, payload.iv);
 
     return JSON.parse(text);
   };
@@ -203,7 +184,7 @@ class Encryptor implements WithKeyEncryptor<EncryptionKey, Json> {
     // NOTE: When re-encrypting, we always use the original library and the KDF parameters from
     // the encryptor itself. This makes sure we always re-encrypt with the "latest" and "best"
     // setup possible.
-    const result = await this.encryptWithKey(key, JSON.stringify(data));
+    const result = await this.encryptWithKey(key, data);
     result.lib = key.lib; // Use the same library than the one used for key generation!
     result.salt = salt;
     result.keyMetadata = key.keyMetadata;

app/core/Encryptor/lib.test.ts

@@ -0,0 +1,20 @@
+import { getEncryptionLibrary, AesLib, AesForkedLib } from './lib';
+import { ENCRYPTION_LIBRARY } from './constants';
+
+describe('lib', () => {
+  describe('getLib', () => {
+    it('returns the original library', () => {
+      const lib = AesLib;
+
+      expect(getEncryptionLibrary(ENCRYPTION_LIBRARY.original)).toBe(lib);
+    });
+
+    it('returns the forked library in any other case', () => {
+      const lib = AesForkedLib;
+
+      expect(getEncryptionLibrary('random-lib')).toBe(lib);
+      // Some older vault might not have the `lib` field, so it is considered `undefined`
+      expect(getEncryptionLibrary(undefined)).toBe(lib);
+    });
+  });
+});

app/core/Encryptor/lib.ts

@@ -0,0 +1,63 @@
+import { NativeModules } from 'react-native';
+import { ENCRYPTION_LIBRARY, SHA256_DIGEST_LENGTH } from './constants';
+import { EncryptionLibrary, KeyDerivationOptions } from './types';
+
+// Actual native libraries
+const Aes = NativeModules.Aes;
+const AesForked = NativeModules.AesForked;
+
+class AesEncryptionLibrary implements EncryptionLibrary {
+  deriveKey = async (
+    password: string,
+    salt: string,
+    opts: KeyDerivationOptions,
+  ): Promise<string> =>
+    await Aes.pbkdf2(
+      password,
+      salt,
+      opts.params.iterations,
+      SHA256_DIGEST_LENGTH,
+    );
+
+  generateIV = async (size: number): Promise<string> =>
+    // Naming isn't perfect here, but this is how the library generates random IV (and encodes it the right way)
+    // See: https://www.npmjs.com/package/react-native-aes-crypto#example
+    await Aes.randomKey(size);
+
+  encrypt = async (data: string, key: string, iv: unknown): Promise<string> =>
+    await Aes.encrypt(data, key, iv);
+
+  decrypt = async (data: string, key: string, iv: unknown): Promise<string> =>
+    await Aes.decrypt(data, key, iv);
+}
+
+class AesForkedEncryptionLibrary implements EncryptionLibrary {
+  deriveKey = async (
+    password: string,
+    salt: string,
+    _opts: KeyDerivationOptions,
+  ): Promise<string> => await AesForked.pbkdf2(password, salt);
+
+  generateIV = async (size: number): Promise<string> =>
+    // NOTE: For some reason, we are not using the AesForked one here, so keep the previous behavior!
+    // Naming isn't perfect here, but this is how the library generates random IV (and encodes it the right way)
+    // See: https://www.npmjs.com/package/react-native-aes-crypto#example
+    await Aes.randomKey(size);
+
+  encrypt = async (data: string, key: string, iv: unknown): Promise<string> =>
+    // NOTE: For some reason, we are not using the AesForked one here, so keep the previous behavior!
+    await Aes.encrypt(data, key, iv);
+
+  decrypt = async (data: string, key: string, iv: unknown): Promise<string> =>
+    await AesForked.decrypt(data, key, iv);
+}
+
+// Those wrappers are stateless, we can build them only once!
+export const AesLib = new AesEncryptionLibrary();
+export const AesForkedLib = new AesForkedEncryptionLibrary();
+
+export function getEncryptionLibrary(
+  lib: string | undefined,
+): EncryptionLibrary {
+  return lib === ENCRYPTION_LIBRARY.original ? AesLib : AesForkedLib;
+}

app/core/Encryptor/types.ts

@@ -4,6 +4,50 @@ import type { KeyDerivationOptions } from '@metamask/browser-passworder';
 /** Key derivation function options. */
 export type { KeyDerivationOptions };
 
+/**
+ * Interface that needs to be implemented for the underlying library used by the `Encryptor`.
+ */
+export interface EncryptionLibrary {
+  /**
+   * Derive a key based on a password and some other parameters (KDF).
+   *
+   * @param password - The password used to generate the key.
+   * @param salt - The salt used during key generation.
+   * @param opts - KDF options used during key generation.
+   * @return The generated key.
+   */
+  deriveKey(
+    password: string,
+    salt: string,
+    opts: KeyDerivationOptions,
+  ): Promise<string>;
+  /**
+   * Generate IV (Initialization Vector) used during encryption/decryption.
+   *
+   * @param size - The IV size.
+   * @return The randomly generated IV.
+   */
+  generateIV(size: number): Promise<string>;
+  /**
+   * Encrypts data.
+   *
+   * @param data - The data to encrypt.
+   * @param key - The encryption key (generated by this same library).
+   * @param iv - The IV.
+   * @returns The encrypted data.
+   */
+  encrypt(data: string, key: string, iv: string): Promise<string>;
+  /**
+   * Decrypts encrypted data.
+   *
+   * @param data - The data to decrypt.
+   * @param key - The encryption key (generated by this same library).
+   * @param iv - The IV (the same one used during encryption).
+   * @returns The decrypted original data.
+   */
+  decrypt(data: string, key: string, iv: string): Promise<string>;
+}
+
 /**
  * The result of an encryption operation.
  * @interface EncryptionResult