feat: reduce KDF computing cost on clients

packages/profile-sync-controller/CHANGELOG.md

@@ -7,6 +7,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Changed
+
+- Dumb down the `N` `scrypt` parameter in order to increase performance on clients
+  - This decreases the impact of the KDF on all clients to around 99%
+  - Add backwards compatible migration logic
+  - Add encryption callbacks so we can measure the impact of future migrations through analytics
+
 ## [21.0.0]
 
 ### Added

packages/profile-sync-controller/src/controllers/user-storage/UserStorageController.test.ts

@@ -17,6 +17,13 @@ import * as AccountSyncControllerIntegrationModule from './account-syncing/contr
 import { BACKUPANDSYNC_FEATURES } from './constants';
 import { MOCK_STORAGE_DATA, MOCK_STORAGE_KEY } from './mocks/mockStorage';
 import UserStorageController, { defaultState } from './UserStorageController';
+import {
+  ALGORITHM_KEY_SIZE,
+  SCRYPT_N_V2,
+  SCRYPT_p,
+  SCRYPT_r,
+  SCRYPT_SALT_SIZE,
+} from '../../shared/encryption/constants';
 import { USER_STORAGE_FEATURE_NAMES } from '../../shared/storage-schema';
 
 describe('user-storage/user-storage-controller - constructor() tests', () => {
@@ -936,3 +943,59 @@ describe('user-storage/user-storage-controller - snap handling', () => {
     expect(await controller.getStorageKey()).toBe(MOCK_STORAGE_KEY);
   });
 });
+
+describe('user-storage/user-storage-controller - encryption callbacks', () => {
+  const arrangeMocks = async () => {
+    const messengerMocks = mockUserStorageMessenger();
+    const mockOnDecrypt = jest.fn();
+    const mockOnEncrypt = jest.fn();
+    const controller = new UserStorageController({
+      messenger: messengerMocks.messenger,
+      config: {
+        encryption: {
+          onDecrypt: mockOnDecrypt,
+          onEncrypt: mockOnEncrypt,
+        },
+      },
+    });
+
+    await Promise.all([
+      mockEndpointGetUserStorage(
+        `${USER_STORAGE_FEATURE_NAMES.notifications}.notification_settings`,
+      ),
+      mockEndpointUpsertUserStorage(
+        `${USER_STORAGE_FEATURE_NAMES.notifications}.notification_settings`,
+      ),
+    ]);
+
+    return { mockOnDecrypt, mockOnEncrypt, controller };
+  };
+
+  it('calls encryption callbacks if they are set', async () => {
+    const { mockOnEncrypt, mockOnDecrypt, controller } = await arrangeMocks();
+
+    await controller.performSetStorage(
+      `${USER_STORAGE_FEATURE_NAMES.notifications}.notification_settings`,
+      'new data',
+    );
+
+    await controller.performGetStorage(
+      `${USER_STORAGE_FEATURE_NAMES.notifications}.notification_settings`,
+    );
+
+    const scryptParams = {
+      o: {
+        N: SCRYPT_N_V2,
+        dkLen: ALGORITHM_KEY_SIZE,
+        p: SCRYPT_p,
+        r: SCRYPT_r,
+      },
+      saltLen: SCRYPT_SALT_SIZE,
+      t: 'scrypt',
+      v: '1',
+    };
+
+    expect(mockOnEncrypt).toHaveBeenCalledWith(scryptParams);
+    expect(mockOnDecrypt).toHaveBeenCalledWith(scryptParams);
+  });
+});

packages/profile-sync-controller/src/controllers/user-storage/UserStorageController.ts

@@ -44,6 +44,7 @@ import { BACKUPANDSYNC_FEATURES } from './constants';
 import { syncContactsWithUserStorage } from './contact-syncing/controller-integration';
 import { setupContactSyncingSubscriptions } from './contact-syncing/setup-subscriptions';
 import type {
+  EncryptedPayload,
   UserStorageGenericFeatureKey,
   UserStorageGenericPathWithFeatureAndKey,
   UserStorageGenericPathWithFeatureOnly,
@@ -147,6 +148,10 @@ const metadata: StateMetadata<UserStorageControllerState> = {
 
 type ControllerConfig = {
   env: Env;
+  encryption?: {
+    onEncrypt?: (encryptedData: Omit<EncryptedPayload, 'd'>) => Promise<void>;
+    onDecrypt?: (encryptedData: Omit<EncryptedPayload, 'd'>) => Promise<void>;
+  };
   accountSyncing?: {
     maxNumberOfAccountsToAdd?: number;
     /**
@@ -490,6 +495,8 @@ export default class UserStorageController extends BaseController<
     return await this.#userStorage.getItem(path, {
       nativeScryptCrypto: this.#nativeScryptCrypto,
       entropySourceId,
+      onDecrypt: this.#config.encryption?.onDecrypt,
+      onEncrypt: this.#config.encryption?.onEncrypt,
     });
   }
 
@@ -508,6 +515,8 @@ export default class UserStorageController extends BaseController<
     return await this.#userStorage.getAllFeatureItems(path, {
       nativeScryptCrypto: this.#nativeScryptCrypto,
       entropySourceId,
+      onDecrypt: this.#config.encryption?.onDecrypt,
+      onEncrypt: this.#config.encryption?.onEncrypt,
     });
   }
 
@@ -528,6 +537,8 @@ export default class UserStorageController extends BaseController<
     return await this.#userStorage.setItem(path, value, {
       nativeScryptCrypto: this.#nativeScryptCrypto,
       entropySourceId,
+      onDecrypt: this.#config.encryption?.onDecrypt,
+      onEncrypt: this.#config.encryption?.onEncrypt,
     });
   }
 
@@ -548,6 +559,8 @@ export default class UserStorageController extends BaseController<
     return await this.#userStorage.batchSetItems(path, values, {
       nativeScryptCrypto: this.#nativeScryptCrypto,
       entropySourceId,
+      onDecrypt: this.#config.encryption?.onDecrypt,
+      onEncrypt: this.#config.encryption?.onEncrypt,
     });
   }
 

packages/profile-sync-controller/src/sdk/user-storage.test.ts

@@ -1,5 +1,3 @@
-import type { UserStorageGenericFeatureKey } from 'src/shared/storage-schema';
-
 import { arrangeAuthAPIs } from './__fixtures__/auth';
 import { arrangeAuth, typedMockFn } from './__fixtures__/test-utils';
 import {
@@ -20,8 +18,9 @@ import {
 import type { StorageOptions } from './user-storage';
 import { STORAGE_URL, UserStorage } from './user-storage';
 import encryption, { createSHA256Hash } from '../shared/encryption';
-import { SHARED_SALT } from '../shared/encryption/constants';
+import { SCRYPT_N_V2, SHARED_SALT } from '../shared/encryption/constants';
 import { Env } from '../shared/env';
+import type { UserStorageGenericFeatureKey } from '../shared/storage-schema';
 import { USER_STORAGE_FEATURE_NAMES } from '../shared/storage-schema';
 
 const MOCK_SRP = '0x6265617665726275696c642e6f7267';
@@ -130,6 +129,43 @@ describe('User Storage', () => {
     expect(mockPut.isDone()).toBe(true);
   });
 
+  it('re-encrypts data if received entry was encrypted with the old scrypt N param, and saves it back to user storage', async () => {
+    const { auth } = arrangeAuth('SRP', MOCK_SRP);
+    const { userStorage } = arrangeUserStorage(auth);
+
+    // This corresponds to 'data1'
+    // Encrypted with the old scrypt N param
+    const mockResponse = {
+      HashedKey: 'entry1',
+      Data: '{"v":"1","t":"scrypt","d":"HIu+WgFBCtKo6rEGy0R8h8t/JgXhzC2a3AF6epahGY2h6GibXDKxSBf6ppxM099Gmg==","o":{"N":131072,"r":8,"p":1,"dkLen":16},"saltLen":16}',
+    };
+
+    const mockGet = await handleMockUserStorageGet({
+      status: 200,
+      body: JSON.stringify(mockResponse),
+    });
+    const mockPut = handleMockUserStoragePut(
+      undefined,
+      async (_, requestBody) => {
+        // eslint-disable-next-line jest/no-conditional-in-test
+        if (typeof requestBody === 'string') {
+          return;
+        }
+
+        const isEncryptedUsingNewScryptN =
+          JSON.parse(requestBody.data).o.N === SCRYPT_N_V2;
+
+        expect(isEncryptedUsingNewScryptN).toBe(true);
+      },
+    );
+
+    await userStorage.getItem(
+      `${USER_STORAGE_FEATURE_NAMES.notifications}.notification_settings`,
+    );
+    expect(mockGet.isDone()).toBe(true);
+    expect(mockPut.isDone()).toBe(true);
+  });
+
   it('gets all feature entries', async () => {
     const { auth } = arrangeAuth('SRP', MOCK_SRP);
     const { userStorage } = arrangeUserStorage(auth);
@@ -184,13 +220,6 @@ describe('User Storage', () => {
           return;
         }
 
-        const doEntriesHaveDifferentSalts =
-          encryption.getIfEntriesHaveDifferentSalts(
-            Object.entries(requestBody.data).map((entry) => entry[1] as string),
-          );
-
-        expect(doEntriesHaveDifferentSalts).toBe(false);
-
         const doEntriesUseSharedSalt = Object.entries(requestBody.data).every(
           ([_entryKey, entryValue]) =>
             encryption.getSalt(entryValue as string).toString() ===
@@ -213,6 +242,54 @@ describe('User Storage', () => {
     expect(mockPut.isDone()).toBe(true);
   });
 
+  it('re-encrypts data if received entries were encrypted with the old scrypt N param, and saves it back to user storage', async () => {
+    // This corresponds to [['entry1', 'data1'], ['entry2', 'data2'], ['HASHED_KEY', '{ "hello": "world" }']]
+    // Each entry has been encrypted with a random salt, except for the last entry
+    // The last entry is used to test if the function can handle payloads that contain both random salts and the shared salt
+    const mockResponse = [
+      {
+        HashedKey: 'entry1',
+        Data: '{"v":"1","t":"scrypt","d":"HIu+WgFBCtKo6rEGy0R8h8t/JgXhzC2a3AF6epahGY2h6GibXDKxSBf6ppxM099Gmg==","o":{"N":131072,"r":8,"p":1,"dkLen":16},"saltLen":16}',
+      },
+      {
+        HashedKey: 'entry2',
+        Data: '{"v":"1","t":"scrypt","d":"3ioo9bxhjDjTmJWIGQMnOlnfa4ysuUNeLYTTmJ+qrq7gwI6hURH3ooUcBldJkHtvuQ==","o":{"N":131072,"r":8,"p":1,"dkLen":16},"saltLen":16}',
+      },
+      await MOCK_STORAGE_RESPONSE('data3'),
+    ];
+
+    const { auth } = arrangeAuth('SRP', MOCK_SRP);
+    const { userStorage } = arrangeUserStorage(auth);
+
+    const mockGetAll = await handleMockUserStorageGetAllFeatureEntries({
+      status: 200,
+      body: mockResponse,
+    });
+
+    const mockPut = handleMockUserStoragePut(
+      undefined,
+      async (_, requestBody) => {
+        // eslint-disable-next-line jest/no-conditional-in-test
+        if (typeof requestBody === 'string') {
+          return;
+        }
+
+        const doEntriesUseNewScryptN = Object.entries(requestBody.data).every(
+          ([_entryKey, entryValue]) =>
+            JSON.parse(entryValue as string).o.N === SCRYPT_N_V2,
+        );
+
+        expect(doEntriesUseNewScryptN).toBe(true);
+      },
+    );
+
+    await userStorage.getAllFeatureItems(
+      USER_STORAGE_FEATURE_NAMES.notifications,
+    );
+    expect(mockGetAll.isDone()).toBe(true);
+    expect(mockPut.isDone()).toBe(true);
+  });
+
   it('batch set items', async () => {
     const dataToStore: [UserStorageGenericFeatureKey, string][] = [
       ['0x123', JSON.stringify(MOCK_NOTIFICATIONS_DATA)],

packages/profile-sync-controller/src/sdk/user-storage.ts

@@ -1,7 +1,7 @@
 import type { IBaseAuth } from './authentication-jwt-bearer/types';
 import { NotFoundError, UserStorageError } from './errors';
+import type { EncryptedPayload } from '../shared/encryption';
 import encryption, { createSHA256Hash } from '../shared/encryption';
-import { SHARED_SALT } from '../shared/encryption/constants';
 import type { Env } from '../shared/env';
 import { getEnvUrls } from '../shared/env';
 import type {
@@ -39,6 +39,8 @@ export type GetUserStorageAllFeatureEntriesResponse = {
 export type UserStorageMethodOptions = {
   nativeScryptCrypto?: NativeScrypt;
   entropySourceId?: string;
+  onEncrypt?: (encryptedData: Omit<EncryptedPayload, 'd'>) => Promise<void>;
+  onDecrypt?: (encryptedData: Omit<EncryptedPayload, 'd'>) => Promise<void>;
 };
 
 type ErrorMessage = {
@@ -141,11 +143,10 @@ export class UserStorage {
     try {
       const headers = await this.#getAuthorizationHeader(entropySourceId);
       const storageKey = await this.getStorageKey(entropySourceId);
-      const encryptedData = await encryption.encryptString(
-        data,
-        storageKey,
-        options?.nativeScryptCrypto,
-      );
+      const encryptedData = await encryption.encryptString(data, storageKey, {
+        nativeScryptCrypto: options?.nativeScryptCrypto,
+        onEncrypt: options?.onEncrypt,
+      });
       const encryptedPath = createEntryPath(path, storageKey);
 
       const url = new URL(STORAGE_URL(this.env, encryptedPath));
@@ -196,11 +197,10 @@ export class UserStorage {
         data.map(async (d) => {
           return [
             this.#createEntryKey(d[0], storageKey),
-            await encryption.encryptString(
-              d[1],
-              storageKey,
-              options?.nativeScryptCrypto,
-            ),
+            await encryption.encryptString(d[1], storageKey, {
+              nativeScryptCrypto: options?.nativeScryptCrypto,
+              onEncrypt: options?.onEncrypt,
+            }),
           ];
         }),
       );
@@ -315,12 +315,14 @@ export class UserStorage {
       const decryptedData = await encryption.decryptString(
         encryptedData,
         storageKey,
-        options?.nativeScryptCrypto,
+        {
+          nativeScryptCrypto: options?.nativeScryptCrypto,
+          onDecrypt: options?.onDecrypt,
+        },
       );
 
-      // Re-encrypt the entry if it was encrypted with a random salt
-      const salt = encryption.getSalt(encryptedData);
-      if (salt.toString() !== SHARED_SALT.toString()) {
+      // Data migration
+      if (encryption.doesEntryNeedReEncryption(encryptedData)) {
         await this.#upsertUserStorage(path, decryptedData, options);
       }
 
@@ -381,24 +383,23 @@ export class UserStorage {
         }
 
         try {
-          const data = await encryption.decryptString(
-            entry.Data,
-            storageKey,
-            options?.nativeScryptCrypto,
-          );
+          const data = await encryption.decryptString(entry.Data, storageKey, {
+            nativeScryptCrypto: options?.nativeScryptCrypto,
+            onDecrypt: options?.onDecrypt,
+          });
           decryptedData.push(data);
 
-          // Re-encrypt the entry was encrypted with a random salt
-          const salt = encryption.getSalt(entry.Data);
-          if (salt.toString() !== SHARED_SALT.toString()) {
-            reEncryptedEntries.push([
-              entry.HashedKey,
-              await encryption.encryptString(
-                data,
-                storageKey,
-                options?.nativeScryptCrypto,
-              ),
-            ]);
+          // Data migration
+          if (encryption.doesEntryNeedReEncryption(entry.Data)) {
+            const reEncryptedData = await encryption.encryptString(
+              data,
+              storageKey,
+              {
+                nativeScryptCrypto: options?.nativeScryptCrypto,
+                onEncrypt: options?.onEncrypt,
+              },
+            );
+            reEncryptedEntries.push([entry.HashedKey, reEncryptedData]);
           }
         } catch {
           // do nothing

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

@@ -6,6 +6,7 @@ export const ALGORITHM_KEY_SIZE = 16; // 16 bytes
 // see: https://cheatsheetseries.owasp.org/cheatsheets/Password_Storage_Cheat_Sheet.html#scrypt
 export const SCRYPT_SALT_SIZE = 16; // 16 bytes
 export const SCRYPT_N = 2 ** 17; // CPU/memory cost parameter (must be a power of 2, > 1)
+export const SCRYPT_N_V2 = 2;
 export const SCRYPT_r = 8; // Block size parameter
 export const SCRYPT_p = 1; // Parallelization parameter