feat(user-storage-controller): use deferred promises cache for KDF operations

packages/profile-sync-controller/CHANGELOG.md

@@ -9,6 +9,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Changed
 
+- Use deferred promises for encryption/decryption KDF operations ([#6736](https://github.com/MetaMask/core/pull/6736))
+  - That will prevent duplicate KDF operations from being computed if one with the same options is already in progress.
+  - For operations that already completed, we use the already existing cache.
 - Bump `@metamask/utils` from `^11.8.0` to `^11.8.1` ([#6708](https://github.com/MetaMask/core/pull/6708))
 - Bump `@metamask/keyring-api` from `^20.1.0` to `^21.0.0` ([#6560](https://github.com/MetaMask/core/pull/6560))
 - Bump `@metamask/keyring-internal-api` from `^8.1.0` to `^9.0.0` ([#6560](https://github.com/MetaMask/core/pull/6560))

packages/profile-sync-controller/src/shared/encryption/constants.ts

@@ -12,3 +12,5 @@ export const SCRYPT_p = 1; // Parallelization parameter
 export const SHARED_SALT = new Uint8Array([
   0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15,
 ]);
+
+export const MAX_KDF_PROMISE_CACHE_SIZE = 20;

packages/profile-sync-controller/src/shared/encryption/encryption.test.ts

@@ -1,3 +1,4 @@
+import { MAX_KDF_PROMISE_CACHE_SIZE } from './constants';
 import encryption, { createSHA256Hash } from './encryption';
 
 describe('encryption tests', () => {
@@ -112,4 +113,157 @@ describe('encryption tests', () => {
       expect(result).toBe(false);
     });
   });
+
+  describe('Deferred Promise KDF Functionality', () => {
+    it('should handle concurrent encryption operations with same password', async () => {
+      const password = 'test-password-concurrent';
+      const plaintext = 'test-data';
+
+      // Start multiple concurrent encryption operations
+      const promises = Array(3)
+        .fill(0)
+        .map(async (_, i) => {
+          return encryption.encryptString(`${plaintext}-${i}`, password);
+        });
+
+      const results = await Promise.all(promises);
+      expect(results).toHaveLength(3);
+
+      // Verify all results can be decrypted
+      for (let i = 0; i < results.length; i++) {
+        const decrypted = await encryption.decryptString(results[i], password);
+        expect(decrypted).toBe(`${plaintext}-${i}`);
+      }
+    });
+
+    it('should handle concurrent encrypt/decrypt operations', async () => {
+      const password = 'test-concurrent-mixed';
+      const testData = 'concurrent-test-data';
+
+      // First encrypt some data
+      const encryptedData = await encryption.encryptString(testData, password);
+
+      // Start concurrent operations
+      const decryptPromises = Array(2)
+        .fill(0)
+        .map(() => {
+          return encryption.decryptString(encryptedData, password);
+        });
+
+      const encryptPromises = Array(2)
+        .fill(0)
+        .map((_, i) => {
+          return encryption.encryptString(`new-data-${i}`, password);
+        });
+
+      const allResults = await Promise.all([
+        ...decryptPromises,
+        ...encryptPromises,
+      ]);
+
+      // Verify decrypt results
+      expect(allResults[0]).toBe(testData);
+      expect(allResults[1]).toBe(testData);
+
+      // Verify encrypt results can be decrypted
+      const newDecrypted0 = await encryption.decryptString(
+        allResults[2],
+        password,
+      );
+      const newDecrypted1 = await encryption.decryptString(
+        allResults[3],
+        password,
+      );
+      expect(newDecrypted0).toBe('new-data-0');
+      expect(newDecrypted1).toBe('new-data-1');
+    });
+
+    it('should handle different passwords concurrently', async () => {
+      const password1 = 'password-one';
+      const password2 = 'password-two';
+      const testData = 'multi-password-test';
+
+      // Start concurrent operations with different passwords
+      const promises = [
+        encryption.encryptString(testData, password1),
+        encryption.encryptString(testData, password2),
+      ];
+
+      const results = await Promise.all(promises);
+      expect(results).toHaveLength(2);
+
+      // Verify decryption with correct passwords
+      const decrypted1 = await encryption.decryptString(results[0], password1);
+      const decrypted2 = await encryption.decryptString(results[1], password2);
+
+      expect(decrypted1).toBe(testData);
+      expect(decrypted2).toBe(testData);
+
+      // Cross-password decryption should fail
+      await expect(
+        encryption.decryptString(results[0], password2),
+      ).rejects.toThrow(
+        'Unable to decrypt string - aes/gcm: invalid ghash tag',
+      );
+      await expect(
+        encryption.decryptString(results[1], password1),
+      ).rejects.toThrow(
+        'Unable to decrypt string - aes/gcm: invalid ghash tag',
+      );
+    });
+
+    it('should work correctly under concurrent load', async () => {
+      const password = 'load-test-password';
+      const baseData = 'load-test-data';
+
+      // Create a larger number of concurrent operations
+      const encryptPromises = Array(10)
+        .fill(0)
+        .map((_, i) => encryption.encryptString(`${baseData}-${i}`, password));
+
+      const results = await Promise.all(encryptPromises);
+      expect(results).toHaveLength(10);
+
+      // Verify all can be decrypted
+      const decryptPromises = results.map((encrypted, i) =>
+        encryption.decryptString(encrypted, password).then((decrypted) => {
+          expect(decrypted).toBe(`${baseData}-${i}`);
+          return decrypted;
+        }),
+      );
+
+      await Promise.all(decryptPromises);
+    });
+
+    it('should limit KDF promise cache size and remove oldest entries when limit is reached', async () => {
+      // Create enough operations to exceed the actual cache limit
+      const numOperations = MAX_KDF_PROMISE_CACHE_SIZE + 5; // 25 operations to exceed the limit
+
+      const promises: Promise<string>[] = [];
+      for (let i = 0; i < numOperations; i++) {
+        // Use different passwords to create unique cache keys
+        const uniquePassword = `cache-test-${i}`;
+        promises.push(encryption.encryptString('test-data', uniquePassword));
+      }
+
+      // All operations should complete successfully despite cache limit
+      const results = await Promise.all(promises);
+      expect(results).toHaveLength(numOperations);
+
+      // Verify a sampling of results can be decrypted (testing all 25 would be slow)
+      const sampleIndices = [
+        0,
+        Math.floor(MAX_KDF_PROMISE_CACHE_SIZE / 2),
+        numOperations - 1,
+      ]; // Test first, middle, and last
+      for (const i of sampleIndices) {
+        const uniquePassword = `cache-test-${i}`;
+        const decrypted = await encryption.decryptString(
+          results[i],
+          uniquePassword,
+        );
+        expect(decrypted).toBe('test-data');
+      }
+    }, 30000);
+  });
 });

packages/profile-sync-controller/src/shared/encryption/encryption.ts

@@ -12,6 +12,7 @@ import {
 import {
   ALGORITHM_KEY_SIZE,
   ALGORITHM_NONCE_SIZE,
+  MAX_KDF_PROMISE_CACHE_SIZE,
   SCRYPT_N,
   SCRYPT_p,
   SCRYPT_r,
@@ -49,6 +50,12 @@ export type EncryptedPayload = {
 };
 
 class EncryptorDecryptor {
+  // Promise cache for ongoing KDF operations to prevent duplicate work
+  readonly #kdfPromiseCache = new Map<
+    string,
+    Promise<{ key: Uint8Array; salt: Uint8Array }>
+  >();
+
   async encryptString(
     plaintext: string,
     password: string,
@@ -239,6 +246,8 @@ class EncryptorDecryptor {
     nativeScryptCrypto?: NativeScrypt,
   ) {
     const hashedPassword = createSHA256Hash(password);
+
+    // Check if we already have the key cached
     const cachedKey = salt
       ? getCachedKeyBySalt(hashedPassword, salt)
       : getCachedKeyGeneratedWithSharedSalt(hashedPassword);
@@ -250,33 +259,94 @@ class EncryptorDecryptor {
       };
     }
 
+    // Create a unique cache key for this KDF operation
     const newSalt = salt ?? SHARED_SALT;
+    const cacheKey = this.#createKdfCacheKey(
+      hashedPassword,
+      o,
+      newSalt,
+      nativeScryptCrypto,
+    );
+
+    // Check if there's already an ongoing KDF operation with the same parameters
+    const existingPromise = this.#kdfPromiseCache.get(cacheKey);
+    if (existingPromise) {
+      return existingPromise;
+    }
+
+    // Limit cache size to prevent unbounded growth
+    if (this.#kdfPromiseCache.size >= MAX_KDF_PROMISE_CACHE_SIZE) {
+      // Remove the oldest entry (first inserted)
+      const firstKey = this.#kdfPromiseCache.keys().next().value;
+      if (firstKey) {
+        this.#kdfPromiseCache.delete(firstKey);
+      }
+    }
+
+    // Create and cache the promise for the KDF operation
+    const kdfPromise = this.#performKdfOperation(
+      password,
+      o,
+      newSalt,
+      hashedPassword,
+      nativeScryptCrypto,
+    );
 
+    // Cache the promise and set up cleanup
+    this.#kdfPromiseCache.set(cacheKey, kdfPromise);
+
+    // Clean up the cache after completion (both success and failure)
+    // eslint-disable-next-line no-void
+    void kdfPromise.finally(() => {
+      this.#kdfPromiseCache.delete(cacheKey);
+    });
+
+    return kdfPromise;
+  }
+
+  #createKdfCacheKey(
+    hashedPassword: string,
+    o: EncryptedPayload['o'],
+    salt: Uint8Array,
+    nativeScryptCrypto?: NativeScrypt,
+  ): string {
+    const saltStr = byteArrayToBase64(salt);
+    const hasNative = Boolean(nativeScryptCrypto);
+    return `${hashedPassword}:${o.N}:${o.r}:${o.p}:${o.dkLen}:${saltStr}:${hasNative}`;
+  }
+
+  async #performKdfOperation(
+    password: string,
+    o: EncryptedPayload['o'],
+    salt: Uint8Array,
+    hashedPassword: string,
+    nativeScryptCrypto?: NativeScrypt,
+  ): Promise<{ key: Uint8Array; salt: Uint8Array }> {
     let newKey: Uint8Array;
 
     if (nativeScryptCrypto) {
       newKey = await nativeScryptCrypto(
         stringToByteArray(password),
-        newSalt,
+        salt,
         o.N,
         o.r,
         o.p,
         o.dkLen,
       );
     } else {
-      newKey = await scryptAsync(password, newSalt, {
+      newKey = await scryptAsync(password, salt, {
         N: o.N,
         r: o.r,
         p: o.p,
         dkLen: o.dkLen,
       });
     }
 
-    setCachedKey(hashedPassword, newSalt, newKey);
+    setCachedKey(hashedPassword, salt, newKey);
 
     return {
       key: newKey,
-      salt: newSalt,
+      salt,
     };
   }
 }