diff --git a/packages/profile-sync-controller/src/controllers/user-storage/__fixtures__/mockResponses.ts b/packages/profile-sync-controller/src/controllers/user-storage/__fixtures__/mockResponses.ts index 218eb9a997..fc81ff44b3 100644 --- a/packages/profile-sync-controller/src/controllers/user-storage/__fixtures__/mockResponses.ts +++ b/packages/profile-sync-controller/src/controllers/user-storage/__fixtures__/mockResponses.ts @@ -58,15 +58,16 @@ export async function createMockGetStorageResponse( export async function createMockAllFeatureEntriesResponse( dataArr: string[] = [MOCK_STORAGE_DATA], ): Promise { - return await Promise.all( - dataArr.map(async function (d) { - const encryptedData = await MOCK_ENCRYPTED_STORAGE_DATA(d); - return { - HashedKey: 'HASHED_KEY', - Data: encryptedData, - }; - }), - ); + const decryptedData = []; + + for (const data of dataArr) { + decryptedData.push({ + HashedKey: 'HASHED_KEY', + Data: await MOCK_ENCRYPTED_STORAGE_DATA(data), + }); + } + + return decryptedData; } /** diff --git a/packages/profile-sync-controller/src/controllers/user-storage/services.test.ts b/packages/profile-sync-controller/src/controllers/user-storage/services.test.ts index 2c839b143a..ec2b698b0c 100644 --- a/packages/profile-sync-controller/src/controllers/user-storage/services.test.ts +++ b/packages/profile-sync-controller/src/controllers/user-storage/services.test.ts @@ -1,6 +1,8 @@ import encryption, { createSHA256Hash } from '../../shared/encryption'; +import { SHARED_SALT } from '../../shared/encryption/constants'; import type { UserStorageFeatureKeys } from '../../shared/storage-schema'; import { USER_STORAGE_FEATURE_NAMES } from '../../shared/storage-schema'; +import { createMockGetStorageResponse } from './__fixtures__'; import { mockEndpointGetUserStorage, mockEndpointUpsertUserStorage, @@ -23,6 +25,7 @@ import { upsertUserStorage, deleteUserStorageAllFeatureEntries, deleteUserStorage, + batchUpsertUserStorageWithAlreadyHashedAndEncryptedEntries, } from './services'; describe('user-storage/services.ts - getUserStorage() tests', () => { @@ -81,6 +84,46 @@ describe('user-storage/services.ts - getUserStorage() tests', () => { mockGetUserStorage.done(); expect(result).toBeNull(); }); + + it('re-encrypts data if received entry was encrypted with a random salt, and saves it back to user storage', async () => { + const DECRYPED_DATA = 'data1'; + const INITIAL_ENCRYPTED_DATA = { + HashedKey: 'entry1', + Data: '{"v":"1","t":"scrypt","d":"HIu+WgFBCtKo6rEGy0R8h8t/JgXhzC2a3AF6epahGY2h6GibXDKxSBf6ppxM099Gmg==","o":{"N":131072,"r":8,"p":1,"dkLen":16},"saltLen":16}', + }; + // Encrypted with a random salt + const mockResponse = INITIAL_ENCRYPTED_DATA; + + const mockGetUserStorage = await mockEndpointGetUserStorage( + `${USER_STORAGE_FEATURE_NAMES.notifications}.notification_settings`, + { + status: 200, + body: JSON.stringify(mockResponse), + }, + ); + + const mockUpsertUserStorage = mockEndpointUpsertUserStorage( + `${USER_STORAGE_FEATURE_NAMES.notifications}.notification_settings`, + undefined, + async (requestBody) => { + if (typeof requestBody === 'string') { + return; + } + + const isEncryptedUsingSharedSalt = + encryption.getSalt(requestBody.data).toString() === + SHARED_SALT.toString(); + + expect(isEncryptedUsingSharedSalt).toBe(true); + }, + ); + + const result = await actCallGetUserStorage(); + + mockGetUserStorage.done(); + mockUpsertUserStorage.done(); + expect(result).toBe(DECRYPED_DATA); + }); }); describe('user-storage/services.ts - getUserStorageAllFeatureEntries() tests', () => { @@ -103,6 +146,68 @@ describe('user-storage/services.ts - getUserStorageAllFeatureEntries() tests', ( expect(result).toStrictEqual([MOCK_STORAGE_DATA]); }); + it('re-encrypts data if received entries were encrypted with random salts, 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 entries with 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 createMockGetStorageResponse(), + ]; + + const mockGetUserStorageAllFeatureEntries = + await mockEndpointGetUserStorageAllFeatureEntries( + USER_STORAGE_FEATURE_NAMES.notifications, + { + status: 200, + body: JSON.stringify(mockResponse), + }, + ); + + const mockBatchUpsertUserStorage = mockEndpointBatchUpsertUserStorage( + USER_STORAGE_FEATURE_NAMES.notifications, + undefined, + async (_uri, requestBody) => { + if (typeof requestBody === 'string') { + 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() === + SHARED_SALT.toString(), + ); + + expect(doEntriesUseSharedSalt).toBe(true); + + const wereOnlyNonEmptySaltEntriesUploaded = + Object.entries(requestBody.data).length === 2; + + expect(wereOnlyNonEmptySaltEntriesUploaded).toBe(true); + }, + ); + + const result = await actCallGetUserStorageAllFeatureEntries(); + + mockGetUserStorageAllFeatureEntries.done(); + mockBatchUpsertUserStorage.done(); + expect(result).toStrictEqual(['data1', 'data2', MOCK_STORAGE_DATA]); + }); + it('returns null if endpoint does not have entry', async () => { const mockGetUserStorage = await mockEndpointGetUserStorageAllFeatureEntries( @@ -275,6 +380,79 @@ describe('user-storage/services.ts - batchUpsertUserStorage() tests', () => { }); }); +describe('user-storage/services.ts - batchUpsertUserStorageWithAlreadyHashedAndEncryptedEntries() tests', () => { + let dataToStore: [string, string][]; + const getDataToStore = async (): Promise<[string, string][]> => + (dataToStore ??= [ + [ + createSHA256Hash(`0x123${MOCK_STORAGE_KEY}`), + await encryption.encryptString(MOCK_STORAGE_DATA, MOCK_STORAGE_KEY), + ], + [ + createSHA256Hash(`0x456${MOCK_STORAGE_KEY}`), + await encryption.encryptString(MOCK_STORAGE_DATA, MOCK_STORAGE_KEY), + ], + ]); + + const actCallBatchUpsertUserStorage = async () => { + return await batchUpsertUserStorageWithAlreadyHashedAndEncryptedEntries( + await getDataToStore(), + { + bearerToken: 'MOCK_BEARER_TOKEN', + path: USER_STORAGE_FEATURE_NAMES.accounts, + storageKey: MOCK_STORAGE_KEY, + }, + ); + }; + + it('invokes upsert endpoint with no errors', async () => { + const mockUpsertUserStorage = mockEndpointBatchUpsertUserStorage( + USER_STORAGE_FEATURE_NAMES.accounts, + undefined, + async (_uri, requestBody) => { + if (typeof requestBody === 'string') { + return; + } + + const expectedBody = Object.fromEntries(await getDataToStore()); + + expect(requestBody.data).toStrictEqual(expectedBody); + }, + ); + + await actCallBatchUpsertUserStorage(); + + expect(mockUpsertUserStorage.isDone()).toBe(true); + }); + + it('throws error if unable to upsert user storage', async () => { + const mockUpsertUserStorage = mockEndpointBatchUpsertUserStorage( + USER_STORAGE_FEATURE_NAMES.accounts, + { + status: 500, + }, + ); + + await expect(actCallBatchUpsertUserStorage()).rejects.toThrow( + expect.any(Error), + ); + mockUpsertUserStorage.done(); + }); + + it('does nothing if empty data is provided', async () => { + const mockUpsertUserStorage = + mockEndpointBatchUpsertUserStorage('accounts_v2'); + + await batchUpsertUserStorage([], { + bearerToken: 'MOCK_BEARER_TOKEN', + path: 'accounts_v2', + storageKey: MOCK_STORAGE_KEY, + }); + + expect(mockUpsertUserStorage.isDone()).toBe(false); + }); +}); + describe('user-storage/services.ts - deleteUserStorage() tests', () => { const actCallDeleteUserStorage = async () => { return await deleteUserStorage({ diff --git a/packages/profile-sync-controller/src/controllers/user-storage/services.ts b/packages/profile-sync-controller/src/controllers/user-storage/services.ts index 5dcb48b51a..04032396c2 100644 --- a/packages/profile-sync-controller/src/controllers/user-storage/services.ts +++ b/packages/profile-sync-controller/src/controllers/user-storage/services.ts @@ -1,6 +1,7 @@ import log from 'loglevel'; import encryption, { createSHA256Hash } from '../../shared/encryption'; +import { SHARED_SALT } from '../../shared/encryption/constants'; import { Env, getEnvUrls } from '../../shared/env'; import type { UserStoragePathWithFeatureAndKey, @@ -93,6 +94,12 @@ export async function getUserStorage( nativeScryptCrypto, ); + // Re-encrypt and re-upload the entry if the salt is random + const salt = encryption.getSalt(encryptedData); + if (salt.toString() !== SHARED_SALT.toString()) { + await upsertUserStorage(decryptedData, opts); + } + return decryptedData; } catch (e) { log.error('Failed to get user storage', e); @@ -137,6 +144,7 @@ export async function getUserStorageAllFeatureEntries( } const decryptedData: string[] = []; + const reEncryptedEntries: [string, string][] = []; for (const entry of userStorage) { /* istanbul ignore if - unreachable if statement, but kept as edge case */ @@ -151,11 +159,32 @@ export async function getUserStorageAllFeatureEntries( nativeScryptCrypto, ); decryptedData.push(data); + + // Re-encrypt the entry if the salt is different from the shared one + const salt = encryption.getSalt(entry.Data); + if (salt.toString() !== SHARED_SALT.toString()) { + reEncryptedEntries.push([ + entry.HashedKey, + await encryption.encryptString( + data, + opts.storageKey, + nativeScryptCrypto, + ), + ]); + } } catch { // do nothing } } + // Re-upload the re-encrypted entries + if (reEncryptedEntries.length) { + await batchUpsertUserStorageWithAlreadyHashedAndEncryptedEntries( + reEncryptedEntries, + opts, + ); + } + return decryptedData; } catch (e) { log.error('Failed to get user storage', e); @@ -241,6 +270,41 @@ export async function batchUpsertUserStorage( } } +/** + * User Storage Service - Set multiple storage entries for one specific feature. + * You cannot use this method to set multiple features at once. + * + * @param encryptedData - data to store, in the form of an array of [hashedKey, encryptedData] pairs + * @param opts - storage options + */ +export async function batchUpsertUserStorageWithAlreadyHashedAndEncryptedEntries( + encryptedData: [string, string][], + opts: UserStorageBatchUpsertOptions, +): Promise { + if (!encryptedData.length) { + return; + } + + const { bearerToken, path } = opts; + + const url = new URL(`${USER_STORAGE_ENDPOINT}/${path}`); + + const formattedData = Object.fromEntries(encryptedData); + + const res = await fetch(url.toString(), { + method: 'PUT', + headers: { + 'Content-Type': 'application/json', + Authorization: `Bearer ${bearerToken}`, + }, + body: JSON.stringify({ data: formattedData }), + }); + + if (!res.ok) { + throw new Error('user-storage - unable to batch upsert data'); + } +} + /** * User Storage Service - Delete Storage Entry. * diff --git a/packages/profile-sync-controller/src/sdk/__fixtures__/mock-userstorage.ts b/packages/profile-sync-controller/src/sdk/__fixtures__/mock-userstorage.ts index 6b37dc41d0..4b7a8fbe44 100644 --- a/packages/profile-sync-controller/src/sdk/__fixtures__/mock-userstorage.ts +++ b/packages/profile-sync-controller/src/sdk/__fixtures__/mock-userstorage.ts @@ -1,6 +1,6 @@ import nock from 'nock'; -import encryption from '../../shared/encryption'; +import encryption, { createSHA256Hash } from '../../shared/encryption'; import { Env } from '../../shared/env'; import { USER_STORAGE_FEATURE_NAMES } from '../../shared/storage-schema'; import { STORAGE_URL } from '../user-storage'; @@ -20,19 +20,19 @@ const MOCK_STORAGE_URL_ALL_FEATURE_ENTRIES = STORAGE_URL( USER_STORAGE_FEATURE_NAMES.notifications, ); -export const MOCK_STORAGE_KEY = 'MOCK_STORAGE_KEY'; +export const MOCK_STORAGE_KEY = createSHA256Hash('mockStorageKey'); // TODO: Either fix this lint violation or explain why it's necessary to ignore. // eslint-disable-next-line @typescript-eslint/naming-convention -export const MOCK_NOTIFICATIONS_DATA = { is_compact: false }; -export const MOCK_NOTIFICATIONS_DATA_ENCRYPTED = async () => +export const MOCK_NOTIFICATIONS_DATA = '{ is_compact: false }'; +export const MOCK_NOTIFICATIONS_DATA_ENCRYPTED = async (data?: string) => await encryption.encryptString( - JSON.stringify(MOCK_NOTIFICATIONS_DATA), + data ?? MOCK_NOTIFICATIONS_DATA, MOCK_STORAGE_KEY, ); -export const MOCK_STORAGE_RESPONSE = async () => ({ +export const MOCK_STORAGE_RESPONSE = async (data?: string) => ({ HashedKey: '8485d2c14c333ebca415140a276adaf546619b0efc204586b73a5d400a18a5e2', - Data: await MOCK_NOTIFICATIONS_DATA_ENCRYPTED(), + Data: await MOCK_NOTIFICATIONS_DATA_ENCRYPTED(data), }); export const handleMockUserStorageGet = async (mockReply?: MockReply) => { diff --git a/packages/profile-sync-controller/src/sdk/user-storage.test.ts b/packages/profile-sync-controller/src/sdk/user-storage.test.ts index 7782d4c239..fb2c7dbb35 100644 --- a/packages/profile-sync-controller/src/sdk/user-storage.test.ts +++ b/packages/profile-sync-controller/src/sdk/user-storage.test.ts @@ -1,4 +1,5 @@ import encryption, { createSHA256Hash } from '../shared/encryption'; +import { SHARED_SALT } from '../shared/encryption/constants'; import { Env } from '../shared/env'; import type { UserStorageFeatureKeys } from '../shared/storage-schema'; import { USER_STORAGE_FEATURE_NAMES } from '../shared/storage-schema'; @@ -12,6 +13,7 @@ import { handleMockUserStorageDeleteAllFeatureEntries, handleMockUserStorageDelete, handleMockUserStorageBatchDelete, + MOCK_STORAGE_RESPONSE, } from './__fixtures__/mock-userstorage'; import { arrangeAuth, typedMockFn } from './__fixtures__/test-utils'; import type { IBaseAuth } from './authentication-jwt-bearer/types'; @@ -40,7 +42,7 @@ describe('User Storage', () => { const mockGet = await handleMockUserStorageGet(); // Test Set - const data = JSON.stringify(MOCK_NOTIFICATIONS_DATA); + const data = MOCK_NOTIFICATIONS_DATA; await userStorage.setItem( `${USER_STORAGE_FEATURE_NAMES.notifications}.notification_settings`, data, @@ -71,7 +73,7 @@ describe('User Storage', () => { const mockGet = await handleMockUserStorageGet(); // Test Set - const data = JSON.stringify(MOCK_NOTIFICATIONS_DATA); + const data = MOCK_NOTIFICATIONS_DATA; await userStorage.setItem( `${USER_STORAGE_FEATURE_NAMES.notifications}.notification_settings`, data, @@ -87,13 +89,50 @@ describe('User Storage', () => { expect(response).toBe(data); }); + it('re-encrypts data if received entry was encrypted with a random salt, and saves it back to user storage', async () => { + const { auth } = arrangeAuth('SRP', MOCK_SRP); + const { userStorage } = arrangeUserStorage(auth); + + // This corresponds to 'data1' + // Encrypted with a random 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}', + }; + + const mockGet = await handleMockUserStorageGet({ + status: 200, + body: JSON.stringify(mockResponse), + }); + const mockPut = handleMockUserStoragePut( + undefined, + async (_, requestBody) => { + if (typeof requestBody === 'string') { + return; + } + + const isEncryptedUsingSharedSalt = + encryption.getSalt(requestBody.data).toString() === + SHARED_SALT.toString(); + + expect(isEncryptedUsingSharedSalt).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); const mockGetAll = await handleMockUserStorageGetAllFeatureEntries(); - const data = JSON.stringify(MOCK_NOTIFICATIONS_DATA); + const data = MOCK_NOTIFICATIONS_DATA; const responseAllFeatureEntries = await userStorage.getAllFeatureItems( USER_STORAGE_FEATURE_NAMES.notifications, ); @@ -101,6 +140,66 @@ describe('User Storage', () => { expect(responseAllFeatureEntries).toStrictEqual([data]); }); + it('re-encrypts data if received entries were encrypted with random salts, 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) => { + if (typeof requestBody === 'string') { + 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() === + SHARED_SALT.toString(), + ); + + expect(doEntriesUseSharedSalt).toBe(true); + + const wereOnlyNonEmptySaltEntriesUploaded = + Object.entries(requestBody.data).length === 2; + + expect(wereOnlyNonEmptySaltEntriesUploaded).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: [ UserStorageFeatureKeys, diff --git a/packages/profile-sync-controller/src/sdk/user-storage.ts b/packages/profile-sync-controller/src/sdk/user-storage.ts index 9376e90641..ad2c52e39f 100644 --- a/packages/profile-sync-controller/src/sdk/user-storage.ts +++ b/packages/profile-sync-controller/src/sdk/user-storage.ts @@ -1,4 +1,5 @@ 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 { @@ -28,7 +29,7 @@ export type UserStorageOptions = { storage?: StorageOptions; }; -type GetUserStorageAllFeatureEntriesResponse = { +export type GetUserStorageAllFeatureEntriesResponse = { // eslint-disable-next-line @typescript-eslint/naming-convention HashedKey: string; // eslint-disable-next-line @typescript-eslint/naming-convention @@ -87,9 +88,9 @@ export class UserStorage { return this.#deleteUserStorageAllFeatureEntries(path); } - async batchDeleteItems( - path: FeatureName, - values: UserStorageFeatureKeys[], + async batchDeleteItems( + path: UserStoragePathWithFeatureOnly, + values: string[], ) { return this.#batchDeleteUserStorage(path, values); } @@ -149,9 +150,9 @@ export class UserStorage { } } - async #batchUpsertUserStorage( - path: FeatureName, - data: [UserStorageFeatureKeys, string][], + async #batchUpsertUserStorage( + path: UserStoragePathWithFeatureOnly, + data: [string, string][], ): Promise { try { if (!data.length) { @@ -200,6 +201,47 @@ export class UserStorage { } } + async #batchUpsertUserStorageWithAlreadyHashedAndEncryptedEntries( + path: UserStoragePathWithFeatureOnly, + encryptedData: [string, string][], + ): Promise { + try { + if (!encryptedData.length) { + return; + } + + const headers = await this.#getAuthorizationHeader(); + + const url = new URL(STORAGE_URL(this.env, path)); + + const response = await fetch(url.toString(), { + method: 'PUT', + headers: { + 'Content-Type': 'application/json', + ...headers, + }, + body: JSON.stringify({ data: Object.fromEntries(encryptedData) }), + }); + + if (!response.ok) { + const responseBody: ErrorMessage = await response.json().catch(() => ({ + message: 'unknown', + error: 'unknown', + })); + throw new Error( + `HTTP error message: ${responseBody.message}, error: ${responseBody.error}`, + ); + } + } catch (e) { + /* istanbul ignore next */ + const errorMessage = + e instanceof Error ? e.message : JSON.stringify(e ?? ''); + throw new UserStorageError( + `failed to batch upsert user storage for path '${path}'. ${errorMessage}`, + ); + } + } + async #getUserStorage( path: UserStoragePathWithFeatureAndKey, ): Promise { @@ -231,7 +273,18 @@ export class UserStorage { } const { Data: encryptedData } = await response.json(); - return encryption.decryptString(encryptedData, storageKey); + const decryptedData = await encryption.decryptString( + encryptedData, + storageKey, + ); + + // Re-encrypt the entry if it was encrypted with a random salt + const salt = encryption.getSalt(encryptedData); + if (salt.toString() !== SHARED_SALT.toString()) { + await this.#upsertUserStorage(path, decryptedData); + } + + return decryptedData; } catch (e) { if (e instanceof NotFoundError) { throw e; @@ -281,17 +334,40 @@ export class UserStorage { return null; } - const decryptedData = userStorage.flatMap((entry) => { + const decryptedData: string[] = []; + const reEncryptedEntries: [string, string][] = []; + + for (const entry of userStorage) { if (!entry.Data) { - return []; + continue; } - return encryption.decryptString(entry.Data, storageKey); - }); + try { + const data = await encryption.decryptString(entry.Data, storageKey); + 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), + ]); + } + } catch { + // do nothing + } + } + + // Re-upload the re-encrypted entries + if (reEncryptedEntries.length) { + await this.#batchUpsertUserStorageWithAlreadyHashedAndEncryptedEntries( + path, + reEncryptedEntries, + ); + } - return (await Promise.allSettled(decryptedData)) - .map((d) => (d.status === 'fulfilled' ? d.value : undefined)) - .filter((d): d is string => d !== undefined); + return decryptedData; } catch (e) { if (e instanceof NotFoundError) { throw e; @@ -393,9 +469,9 @@ export class UserStorage { } } - async #batchDeleteUserStorage( - path: FeatureName, - data: UserStorageFeatureKeys[], + async #batchDeleteUserStorage( + path: UserStoragePathWithFeatureOnly, + data: string[], ): Promise { try { if (!data.length) { diff --git a/packages/profile-sync-controller/src/shared/encryption/cache.ts b/packages/profile-sync-controller/src/shared/encryption/cache.ts index 3b14925b13..7552ef60d1 100644 --- a/packages/profile-sync-controller/src/shared/encryption/cache.ts +++ b/packages/profile-sync-controller/src/shared/encryption/cache.ts @@ -1,4 +1,5 @@ -import { base64ToByteArray, byteArrayToBase64 } from './utils'; +import { SHARED_SALT } from './constants'; +import { byteArrayToBase64 } from './utils'; type CachedEntry = { salt: Uint8Array; @@ -47,31 +48,27 @@ export function getCachedKeyBySalt( } /** - * Gets any cached key for a given hashed password + * Gets the cached key that was generated without a salt, if it exists. + * This is unique per hashed password. * * @param hashedPassword - hashed password for cache lookup - * @returns any (the first) cached key + * @returns the cached key */ -export function getAnyCachedKey( +export function getCachedKeyGeneratedWithSharedSalt( hashedPassword: string, ): CachedEntry | undefined { const cache = getPasswordCache(hashedPassword); + const base64Salt = byteArrayToBase64(SHARED_SALT); + const cachedKey = cache.get(base64Salt); - // Takes 1 item from an Iterator via Map.entries() - const cachedEntry: [string, Uint8Array] | undefined = cache - .entries() - .next().value; - - if (!cachedEntry) { + if (!cachedKey) { return undefined; } - const base64Salt = cachedEntry[0]; - const bytesSalt = base64ToByteArray(base64Salt); return { - salt: bytesSalt, + salt: SHARED_SALT, base64Salt, - key: cachedEntry[1], + key: cachedKey, }; } diff --git a/packages/profile-sync-controller/src/shared/encryption/constants.ts b/packages/profile-sync-controller/src/shared/encryption/constants.ts new file mode 100644 index 0000000000..b39d1aa8a9 --- /dev/null +++ b/packages/profile-sync-controller/src/shared/encryption/constants.ts @@ -0,0 +1,16 @@ +// Nonce/Key Sizes +export const ALGORITHM_NONCE_SIZE = 12; // 12 bytes +export const ALGORITHM_KEY_SIZE = 16; // 16 bytes + +// Scrypt settings +// 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) +// eslint-disable-next-line @typescript-eslint/naming-convention +export const SCRYPT_r = 8; // Block size parameter +// eslint-disable-next-line @typescript-eslint/naming-convention +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, +]); diff --git a/packages/profile-sync-controller/src/shared/encryption/encryption.test.ts b/packages/profile-sync-controller/src/shared/encryption/encryption.test.ts index 0b63381685..434706b58d 100644 --- a/packages/profile-sync-controller/src/shared/encryption/encryption.test.ts +++ b/packages/profile-sync-controller/src/shared/encryption/encryption.test.ts @@ -5,6 +5,13 @@ describe('encryption tests', () => { const DATA1 = 'Hello World'; const DATA2 = JSON.stringify({ foo: 'bar' }); + const ACCOUNTS_PASSWORD = + '0d55d30da233959674d14076737198c05ae3fb8631a17e20d3c28c60dddd82f7'; + const ACCOUNTS_ENCRYPTED_DATA_WITH_SALT = + '{"v":"1","t":"scrypt","d":"1yC/ZXarV57HbqEZ46nH0JWgXfPl86nTHD7kai2g5gm290FM9tw5QjOaAAwIuQESEE8TIM/J9pIj7nmlGi+BZrevTtK3DXWXwnUQsCP7amKd5Q4gs3EEQgXpA0W+WJUgyElj869rwIv/C6tl5E2pK4j/0EAjMSIm1TGoj9FPohyRgZsOIt8VhZfb7w0GODsjPwPIkN6zazvJ3gAFYFPh7yRtebFs86z3fzqCWZ9zakdCHntchC2oZiaApXR9yzaPlGgnPg==","o":{"N":131072,"r":8,"p":1,"dkLen":16},"saltLen":16}'; + const ACCOUNTS_DECRYPTED_DATA = + '{"v":"1","a":"0xd2a4afe5c2ff0a16bf81f77ba4201a8107aa874b","i":"c590ab50-add6-4de4-8af7-9b696b5e9c6a","n":"My Second Synced Account","nlu":1729234343749}'; + it('should encrypt and decrypt data', async () => { const actEncryptDecrypt = async (data: string) => { const encryptedString = await encryption.encryptString(data, PASSWORD); @@ -37,4 +44,72 @@ describe('encryption tests', () => { const hash2 = createSHA256Hash(DATA); expect(hash1).toBe(hash2); }); + + it('should be able to get the salt from an encrypted string', async () => { + const encryptedData = `{"v":"1","t":"scrypt","d":"d9k8wRtOOq97OyNqRNnTCa3ct7+z9nRjV75Am+ND9yMoV/bMcnrzZqO2EhjL3viJyA==","o":{"N":131072,"r":8,"p":1,"dkLen":16},"saltLen":16}`; + const saltUsedToPreviouslyEncryptData = new Uint8Array([ + 119, 217, 60, 193, 27, 78, 58, 175, 123, 59, 35, 106, 68, 217, 211, 9, + ]); + + const salt = encryption.getSalt(encryptedData); + expect(salt).toBeInstanceOf(Uint8Array); + expect(salt).toHaveLength(16); + expect(salt).toStrictEqual(saltUsedToPreviouslyEncryptData); + }); + + it('should throw an error when trying to get the salt from an unsupported encrypted string', () => { + const encryptedData = `{"v":"1","t":"unsupported","d":"d9k8wRtOOq97OyNqRNnTCa3ct7+z9nRjV75Am+ND9yMoV/bMcnrzZqO2EhjL3viJyA==","o":{"N":131072,"r":8,"p":1,"dkLen":16},"saltLen":16}`; + expect(() => encryption.getSalt(encryptedData)).toThrow( + `Unsupported encrypted data payload - ${encryptedData}`, + ); + }); + + it('should be able to decrypt an entry that has no salt', async () => { + const encryptedData = await encryption.encryptString(DATA1, PASSWORD); + const decryptedData = await encryption.decryptString( + encryptedData, + PASSWORD, + ); + expect(decryptedData).toBe(DATA1); + }); + + it('should be able to decrypt an entry that has a salt', async () => { + const decryptedData = await encryption.decryptString( + ACCOUNTS_ENCRYPTED_DATA_WITH_SALT, + ACCOUNTS_PASSWORD, + ); + expect(decryptedData).toBe(ACCOUNTS_DECRYPTED_DATA); + }); + + describe('getIfEntriesHaveDifferentSalts()', () => { + it('should return true if entries have different salts', () => { + const entries = [ + '{"v":"1","t":"scrypt","d":"1yC/ZXarV57HbqEZ46nH0JWgXfPl86nTHD7kai2g5gm290FM9tw5QjOaAAwIuQESEE8TIM/J9pIj7nmlGi+BZrevTtK3DXWXwnUQsCP7amKd5Q4gs3EEQgXpA0W+WJUgyElj869rwIv/C6tl5E2pK4j/0EAjMSIm1TGoj9FPohyRgZsOIt8VhZfb7w0GODsjPwPIkN6zazvJ3gAFYFPh7yRtebFs86z3fzqCWZ9zakdCHntchC2oZiaApXR9yzaPlGgnPg==","o":{"N":131072,"r":8,"p":1,"dkLen":16},"saltLen":16}', + '{"v":"1","t":"scrypt","d":"x7QqsdqsdEtUo7q/jG+UNkD/HOxQARGGRXsGPrLsDlkwDfgfoYlPI0To/M3pJRBlKD0RLEFIPHtHBEA5bv/2izB21VljvhMnhHfo0KgQ+e8Uq1t7grwa+r+ge3qbPNY+w78Xt8GtC+Hkrw5fORKvCn+xjzaCHYV6RxKYbp1TpyCJq7hDrr1XiyL8kqbpE0hAHALrrQOoV9/WXJi9pC5J118kquXx8CNA1P5wO/BXKp1AbryGR6kVW3lsp1sy3lYE/TApa5lTj+","o":{"N":131072,"r":8,"p":1,"dkLen":16},"saltLen":16}', + ]; + + const result = encryption.getIfEntriesHaveDifferentSalts(entries); + expect(result).toBe(true); + }); + + it('should return false if entries have the same salts', () => { + const entries = [ + '{"v":"1","t":"scrypt","d":"+nhJkMMjQljyyyytsnhO4dIzFL/hGR4Y6hb2qUGrPb/hjxHVJUk1jcJAyHP9eUzgZQ==","o":{"N":131072,"r":8,"p":1,"dkLen":16},"saltLen":16}', + '{"v":"1","t":"scrypt","d":"+nhJkMMjQljyyyytsnhO4XYxpF0N3IXuhCpPM9dAyw5pO2gcqcXNucJs60rBtgKttA==","o":{"N":131072,"r":8,"p":1,"dkLen":16},"saltLen":16}', + ]; + + const result = encryption.getIfEntriesHaveDifferentSalts(entries); + expect(result).toBe(false); + }); + + it('should return false if entries do not have salts', () => { + const entries = [ + '{"v":"1","t":"scrypt","d":"CgHcOM6xCaaNFnPCr0etqyxCq4xoJNQ9gfP9+GRn94hGtKurbOuXzyDoHJgzaJxDKd1zQHJhDwLjnH6oCZvC8XKvZZ6RcrN9BicZHpzpojon+HwpcPHceM/pvoMabYfiXqbokYHXZymGTxE5X+TjFo+HB7/Y6xOCU1usz47bru9vfyZrdQ66qGlMO2MUFx00cnh8xHOksDNC","o":{"N":131072,"r":8,"p":1,"dkLen":16},"saltLen":0}', + '{"v":"1","t":"scrypt","d":"OCrYnCFkt7a33cjaAL65D/WypM+oVxIiGVwMk+mjijcpnG4r3vzPl6OzFpx2LNKHj6YN59wcLje3QK2hISU0R8iXyZubdkeAiY89SsI7owLda96ysF+q6PuyxnWfNfWe+5a1+4O8BVkR8p/9PYimwTN0QGhX2lkfLt5r0aYgsLnWld/5k9G7cB4yqoduIopzpojS5ZGI8PFW","o":{"N":131072,"r":8,"p":1,"dkLen":16},"saltLen":0}', + ]; + + const result = encryption.getIfEntriesHaveDifferentSalts(entries); + expect(result).toBe(false); + }); + }); }); diff --git a/packages/profile-sync-controller/src/shared/encryption/encryption.ts b/packages/profile-sync-controller/src/shared/encryption/encryption.ts index 969f613751..7b36a15e64 100644 --- a/packages/profile-sync-controller/src/shared/encryption/encryption.ts +++ b/packages/profile-sync-controller/src/shared/encryption/encryption.ts @@ -5,7 +5,20 @@ import { sha256 } from '@noble/hashes/sha256'; import { utf8ToBytes, concatBytes, bytesToHex } from '@noble/hashes/utils'; import type { NativeScrypt } from '../types/encryption'; -import { getAnyCachedKey, getCachedKeyBySalt, setCachedKey } from './cache'; +import { + getCachedKeyBySalt, + getCachedKeyGeneratedWithSharedSalt, + setCachedKey, +} from './cache'; +import { + ALGORITHM_KEY_SIZE, + ALGORITHM_NONCE_SIZE, + SCRYPT_N, + SCRYPT_p, + SCRYPT_r, + SCRYPT_SALT_SIZE, + SHARED_SALT, +} from './constants'; import { base64ToByteArray, byteArrayToBase64, @@ -36,19 +49,6 @@ export type EncryptedPayload = { saltLen: number; }; -// Nonce/Key Sizes -const ALGORITHM_NONCE_SIZE = 12; // 12 bytes -const ALGORITHM_KEY_SIZE = 16; // 16 bytes - -// Scrypt settings -// see: https://cheatsheetseries.owasp.org/cheatsheets/Password_Storage_Cheat_Sheet.html#scrypt -const SCRYPT_SALT_SIZE = 16; // 16 bytes -const SCRYPT_N = 2 ** 17; // CPU/memory cost parameter (must be a power of 2, > 1) -// eslint-disable-next-line @typescript-eslint/naming-convention -const SCRYPT_r = 8; // Block size parameter -// eslint-disable-next-line @typescript-eslint/naming-convention -const SCRYPT_p = 1; // Parallelization parameter - class EncryptorDecryptor { async encryptString( plaintext: string, @@ -171,6 +171,47 @@ class EncryptorDecryptor { return bytesToUtf8(this.#decrypt(ciphertextAndNonce, key)); } + getSalt(encryptedDataStr: string) { + try { + const encryptedData: EncryptedPayload = JSON.parse(encryptedDataStr); + if (encryptedData.v === '1') { + if (encryptedData.t === 'scrypt') { + const { d: base64CiphertextAndNonceAndSalt, saltLen } = encryptedData; + + // Decode the base64. + const ciphertextAndNonceAndSalt = base64ToByteArray( + base64CiphertextAndNonceAndSalt, + ); + + // Create buffers of salt and ciphertextAndNonce. + const salt = ciphertextAndNonceAndSalt.slice(0, saltLen); + return salt; + } + } + throw new Error( + `Unsupported encrypted data payload - ${encryptedDataStr}`, + ); + } catch (e) { + const errorMessage = e instanceof Error ? e.message : JSON.stringify(e); + throw new Error(`Unable to get salt - ${errorMessage}`); + } + } + + getIfEntriesHaveDifferentSalts(entries: string[]): boolean { + const salts = entries + .map((e) => { + try { + return this.getSalt(e); + } catch { + return undefined; + } + }) + .filter((s): s is Uint8Array => s !== undefined); + + const strSet = new Set(salts.map((arr) => arr.toString())); + return strSet.size === salts.length; + } + #encrypt(plaintext: Uint8Array, key: Uint8Array): Uint8Array { const nonce = randomBytes(ALGORITHM_NONCE_SIZE); @@ -201,7 +242,7 @@ class EncryptorDecryptor { const hashedPassword = createSHA256Hash(password); const cachedKey = salt ? getCachedKeyBySalt(hashedPassword, salt) - : getAnyCachedKey(hashedPassword); + : getCachedKeyGeneratedWithSharedSalt(hashedPassword); if (cachedKey) { return { @@ -210,7 +251,7 @@ class EncryptorDecryptor { }; } - const newSalt = salt ?? randomBytes(SCRYPT_SALT_SIZE); + const newSalt = salt ?? SHARED_SALT; let newKey: Uint8Array; @@ -231,6 +272,7 @@ class EncryptorDecryptor { dkLen: o.dkLen, }); } + setCachedKey(hashedPassword, newSalt, newKey); return {