diff --git a/README.md b/README.md index d75568ec121..96865981328 100644 --- a/README.md +++ b/README.md @@ -633,6 +633,7 @@ linkStyle default opacity:0.5 profile_sync_controller --> address_book_controller; profile_sync_controller --> base_controller; profile_sync_controller --> controller_utils; + profile_sync_controller --> cryptography; profile_sync_controller --> keyring_controller; profile_sync_controller --> messenger; profile_sync_controller --> seedless_onboarding_controller; diff --git a/jest.config.packages.cjs b/jest.config.packages.cjs index a96314934e2..b65ce04b056 100644 --- a/jest.config.packages.cjs +++ b/jest.config.packages.cjs @@ -87,6 +87,7 @@ module.exports = { '/../json-rpc-engine/src/v2/index.ts', ], '^@metamask/utils/node$': ['/../utils/src/node.ts'], + '^@metamask/cryptography/(.+)$': ['/../cryptography/src/$1'], '^@metamask/(.+)$': [ '/../$1/src', // Some @metamask/* packages we are referencing aren't in this monorepo, diff --git a/oxlint-suppressions.json b/oxlint-suppressions.json index 50a72b60d16..626b1252624 100644 --- a/oxlint-suppressions.json +++ b/oxlint-suppressions.json @@ -5385,7 +5385,7 @@ "count": 1 }, "typescript/explicit-function-return-type": { - "count": 2 + "count": 1 }, "typescript/no-unsafe-assignment": { "count": 2 diff --git a/packages/profile-sync-controller/CHANGELOG.md b/packages/profile-sync-controller/CHANGELOG.md index d66c90c8213..2b3ca2ca5be 100644 --- a/packages/profile-sync-controller/CHANGELOG.md +++ b/packages/profile-sync-controller/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- Replace JS AES implementation with `@metamask/cryptography` ([#10621](https://github.com/MetaMask/core/pull/10621)) - Bump `immer` from `^9.0.21` to `^11.1.18` ([#10382](https://github.com/MetaMask/core/pull/10382)) ### Fixed diff --git a/packages/profile-sync-controller/package.json b/packages/profile-sync-controller/package.json index 15d40b690da..92a126d30ed 100644 --- a/packages/profile-sync-controller/package.json +++ b/packages/profile-sync-controller/package.json @@ -76,13 +76,13 @@ "@metamask/address-book-controller": "^8.0.0", "@metamask/base-controller": "^10.0.0", "@metamask/controller-utils": "^13.0.0", + "@metamask/cryptography": "^1.1.0", "@metamask/key-tree": "^10.1.1", "@metamask/keyring-controller": "^28.1.0", "@metamask/messenger": "^3.0.0", "@metamask/seedless-onboarding-controller": "^11.0.1", "@metamask/superstruct": "^3.4.1", "@metamask/utils": "^12.0.0", - "@noble/ciphers": "^1.3.0", "@noble/curves": "^1.9.2", "@noble/hashes": "^1.8.0", "@signinwithethereum/siwe": "^4.2.1", diff --git a/packages/profile-sync-controller/src/shared/encryption/cache.ts b/packages/profile-sync-controller/src/shared/encryption/cache.ts index 10c55d90b3f..37fadc92854 100644 --- a/packages/profile-sync-controller/src/shared/encryption/cache.ts +++ b/packages/profile-sync-controller/src/shared/encryption/cache.ts @@ -2,9 +2,9 @@ import { SHARED_SALT } from './constants.js'; import { byteArrayToBase64 } from './utils.js'; type CachedEntry = { - salt: Uint8Array; + salt: Uint8Array; base64Salt: string; - key: Uint8Array; + key: Uint8Array; }; const MAX_PASSWORD_CACHES = 100; @@ -14,7 +14,7 @@ const MAX_SALT_CACHES = 100; * In-Memory Caching derived keys based from a given salt and password. */ type PasswordMemCachedKDF = { - [hashedPassword: string]: Map; + [hashedPassword: string]: Map>; }; let inMemCachedKDF: PasswordMemCachedKDF = {}; const getPasswordCache = (hashedPassword: string) => { @@ -31,7 +31,7 @@ const getPasswordCache = (hashedPassword: string) => { */ export function getCachedKeyBySalt( hashedPassword: string, - salt: Uint8Array, + salt: Uint8Array, ): CachedEntry | undefined { const cache = getPasswordCache(hashedPassword); const base64Salt = byteArrayToBase64(salt); @@ -82,8 +82,8 @@ export function getCachedKeyGeneratedWithSharedSalt( */ export function setCachedKey( hashedPassword: string, - salt: Uint8Array, - key: Uint8Array, + salt: Uint8Array, + key: Uint8Array, ): void { // Max password caches if (Object.keys(inMemCachedKDF).length > MAX_PASSWORD_CACHES) { diff --git a/packages/profile-sync-controller/src/shared/encryption/constants.ts b/packages/profile-sync-controller/src/shared/encryption/constants.ts index 6539a63ebeb..3e205becad2 100644 --- a/packages/profile-sync-controller/src/shared/encryption/constants.ts +++ b/packages/profile-sync-controller/src/shared/encryption/constants.ts @@ -1,5 +1,3 @@ -// Nonce/Key Sizes -export const ALGORITHM_NONCE_SIZE = 12; // 12 bytes export const ALGORITHM_KEY_SIZE = 16; // 16 bytes // Scrypt settings 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 e83dd937c93..6ab27cbb93b 100644 --- a/packages/profile-sync-controller/src/shared/encryption/encryption.test.ts +++ b/packages/profile-sync-controller/src/shared/encryption/encryption.test.ts @@ -203,12 +203,12 @@ describe('encryption tests', () => { await expect( encryption.decryptString(results[0], password2), ).rejects.toThrow( - 'Unable to decrypt string - aes/gcm: invalid ghash tag', + 'Unable to decrypt string - The operation failed for an operation-specific reason', ); await expect( encryption.decryptString(results[1], password1), ).rejects.toThrow( - 'Unable to decrypt string - aes/gcm: invalid ghash tag', + 'Unable to decrypt string - The operation failed for an operation-specific reason', ); }); diff --git a/packages/profile-sync-controller/src/shared/encryption/encryption.ts b/packages/profile-sync-controller/src/shared/encryption/encryption.ts index d0f38fc7e6b..f18f194e5d8 100644 --- a/packages/profile-sync-controller/src/shared/encryption/encryption.ts +++ b/packages/profile-sync-controller/src/shared/encryption/encryption.ts @@ -1,8 +1,13 @@ -import { gcm } from '@noble/ciphers/aes'; -import { randomBytes } from '@noble/ciphers/webcrypto'; +import { decrypt, encrypt, IV_LENGTH } from '@metamask/cryptography/aes-gcm'; +import { + getErrorMessage, + stringToBytes, + bytesToHex, + concatBytes, + remove0x, +} from '@metamask/utils'; import { scryptAsync } from '@noble/hashes/scrypt'; import { sha256 } from '@noble/hashes/sha256'; -import { utf8ToBytes, concatBytes, bytesToHex } from '@noble/hashes/utils'; import type { NativeScrypt } from '../types/encryption.js'; import { @@ -12,7 +17,6 @@ import { } from './cache.js'; import { ALGORITHM_KEY_SIZE, - ALGORITHM_NONCE_SIZE, MAX_KDF_PROMISE_CACHE_SIZE, SCRYPT_N, SCRYPT_p, @@ -53,7 +57,7 @@ class EncryptorDecryptor { // Promise cache for ongoing KDF operations to prevent duplicate work readonly #kdfPromiseCache = new Map< string, - Promise<{ key: Uint8Array; salt: Uint8Array }> + Promise<{ key: Uint8Array; salt: Uint8Array }> >(); async encryptString( @@ -68,8 +72,7 @@ class EncryptorDecryptor { nativeScryptCrypto, ); } catch (e) { - const errorMessage = e instanceof Error ? e.message : JSON.stringify(e); - throw new Error(`Unable to encrypt string - ${errorMessage}`); + throw new Error(`Unable to encrypt string - ${getErrorMessage(e)}`); } } @@ -93,8 +96,7 @@ class EncryptorDecryptor { `Unsupported encrypted data payload - ${encryptedDataStr}`, ); } catch (e) { - const errorMessage = e instanceof Error ? e.message : JSON.stringify(e); - throw new Error(`Unable to decrypt string - ${errorMessage}`); + throw new Error(`Unable to decrypt string - ${getErrorMessage(e)}`); } } @@ -116,11 +118,11 @@ class EncryptorDecryptor { ); // Encrypt and prepend salt. - const plaintextRaw = utf8ToBytes(plaintext); - const ciphertextAndNonceAndSalt = concatBytes( + const plaintextRaw = stringToBytes(plaintext); + const ciphertextAndNonceAndSalt = concatBytes([ salt, - this.#encrypt(plaintextRaw, key), - ); + await this.#encrypt(plaintextRaw, key), + ]); // Convert to Base64 const encryptedData = byteArrayToBase64(ciphertextAndNonceAndSalt); @@ -174,7 +176,7 @@ class EncryptorDecryptor { ); // Decrypt and return result. - return bytesToUtf8(this.#decrypt(ciphertextAndNonce, key)); + return bytesToUtf8(await this.#decrypt(ciphertextAndNonce, key)); } getSalt(encryptedDataStr: string) { @@ -198,8 +200,7 @@ class EncryptorDecryptor { `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}`); + throw new Error(`Unable to get salt - ${getErrorMessage(e)}`); } } @@ -218,33 +219,37 @@ class EncryptorDecryptor { return strSet.size === salts.length; } - #encrypt(plaintext: Uint8Array, key: Uint8Array): Uint8Array { - const nonce = randomBytes(ALGORITHM_NONCE_SIZE); - + async #encrypt( + plaintext: Uint8Array, + key: Uint8Array, + ): Promise> { // Encrypt and prepend nonce. - const ciphertext = gcm(key, nonce).encrypt(plaintext); + const { ciphertext, iv } = await encrypt(key, plaintext); - return concatBytes(nonce, ciphertext); + return concatBytes([iv, ciphertext]); } - #decrypt(ciphertextAndNonce: Uint8Array, key: Uint8Array): Uint8Array { + async #decrypt( + ciphertextAndNonce: Uint8Array, + key: Uint8Array, + ): Promise> { // Create buffers of nonce and ciphertext. - const nonce = ciphertextAndNonce.slice(0, ALGORITHM_NONCE_SIZE); + const nonce = ciphertextAndNonce.slice(0, IV_LENGTH); const ciphertext = ciphertextAndNonce.slice( - ALGORITHM_NONCE_SIZE, + IV_LENGTH, ciphertextAndNonce.length, ); // Decrypt and return result. - return gcm(key, nonce).decrypt(ciphertext); + return decrypt(key, nonce, ciphertext); } async #getOrGenerateScryptKey( password: string, o: EncryptedPayload['o'], - salt?: Uint8Array, + salt?: Uint8Array, nativeScryptCrypto?: NativeScrypt, - ) { + ): Promise<{ key: Uint8Array; salt: Uint8Array }> { const hashedPassword = createSHA256Hash(password); // Check if we already have the key cached @@ -318,11 +323,11 @@ class EncryptorDecryptor { async #performKdfOperation( password: string, o: EncryptedPayload['o'], - salt: Uint8Array, + salt: Uint8Array, hashedPassword: string, nativeScryptCrypto?: NativeScrypt, - ): Promise<{ key: Uint8Array; salt: Uint8Array }> { - let newKey: Uint8Array; + ): Promise<{ key: Uint8Array; salt: Uint8Array }> { + let newKey: Uint8Array; if (nativeScryptCrypto) { newKey = await nativeScryptCrypto( @@ -334,12 +339,12 @@ class EncryptorDecryptor { o.dkLen, ); } else { - newKey = await scryptAsync(password, salt, { + newKey = (await scryptAsync(password, salt, { N: o.N, r: o.r, p: o.p, dkLen: o.dkLen, - }); + })) as Uint8Array; } setCachedKey(hashedPassword, salt, newKey); @@ -362,5 +367,5 @@ export default encryption; */ export function createSHA256Hash(data: string): string { const hashedData = sha256(data); - return bytesToHex(hashedData); + return remove0x(bytesToHex(hashedData)); } diff --git a/packages/profile-sync-controller/src/shared/types/encryption.ts b/packages/profile-sync-controller/src/shared/types/encryption.ts index e95ec41fbd9..84f99749880 100644 --- a/packages/profile-sync-controller/src/shared/types/encryption.ts +++ b/packages/profile-sync-controller/src/shared/types/encryption.ts @@ -5,4 +5,4 @@ export declare type NativeScrypt = ( r: number, p: number, size: number, -) => Promise; +) => Promise>; diff --git a/packages/profile-sync-controller/tsconfig.build.json b/packages/profile-sync-controller/tsconfig.build.json index 220af8e2b65..9e8726b42af 100644 --- a/packages/profile-sync-controller/tsconfig.build.json +++ b/packages/profile-sync-controller/tsconfig.build.json @@ -9,6 +9,9 @@ { "path": "../base-controller/tsconfig.build.json" }, + { + "path": "../cryptography/tsconfig.build.json" + }, { "path": "../keyring-controller/tsconfig.build.json" }, diff --git a/packages/profile-sync-controller/tsconfig.json b/packages/profile-sync-controller/tsconfig.json index 6542f7e1bf9..6d54bbacab5 100644 --- a/packages/profile-sync-controller/tsconfig.json +++ b/packages/profile-sync-controller/tsconfig.json @@ -4,6 +4,9 @@ { "path": "../base-controller" }, + { + "path": "../cryptography/" + }, { "path": "../controller-utils" }, diff --git a/packages/profile-sync-controller/tsconfig.lint.json b/packages/profile-sync-controller/tsconfig.lint.json index 0a9b3c044c2..b3a13f88a00 100644 --- a/packages/profile-sync-controller/tsconfig.lint.json +++ b/packages/profile-sync-controller/tsconfig.lint.json @@ -25,6 +25,9 @@ }, { "path": "../controller-utils/tsconfig.lint.json" + }, + { + "path": "../cryptography/tsconfig.lint.json" } ] } diff --git a/tsconfig.packages.json b/tsconfig.packages.json index 46fe96683e1..4fe27b7c168 100644 --- a/tsconfig.packages.json +++ b/tsconfig.packages.json @@ -22,6 +22,7 @@ "@metamask/profile-sync-controller/user-storage": [ "${configDir}../profile-sync-controller/src/controllers/user-storage/index.ts" ], + "@metamask/cryptography/*": ["${configDir}/../cryptography/src/*.ts"], "@metamask/utils/node": ["${configDir}/../utils/src/node.ts"], "@metamask/*": ["${configDir}/../*/src/index.ts"] } diff --git a/yarn.lock b/yarn.lock index d51a47c81cf..3714c9937d3 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6396,7 +6396,7 @@ __metadata: languageName: node linkType: hard -"@metamask/cryptography@workspace:packages/cryptography": +"@metamask/cryptography@npm:^1.1.0, @metamask/cryptography@workspace:packages/cryptography": version: 0.0.0-use.local resolution: "@metamask/cryptography@workspace:packages/cryptography" dependencies: @@ -8291,6 +8291,7 @@ __metadata: "@metamask/auto-changelog": "npm:^6.2.1" "@metamask/base-controller": "npm:^10.0.0" "@metamask/controller-utils": "npm:^13.0.0" + "@metamask/cryptography": "npm:^1.1.0" "@metamask/eth-hd-keyring": "npm:^15.0.0" "@metamask/key-tree": "npm:^10.1.1" "@metamask/keyring-api": "npm:^24.0.0" @@ -8302,7 +8303,6 @@ __metadata: "@metamask/seedless-onboarding-controller": "npm:^11.0.1" "@metamask/superstruct": "npm:^3.4.1" "@metamask/utils": "npm:^12.0.0" - "@noble/ciphers": "npm:^1.3.0" "@noble/curves": "npm:^1.9.2" "@noble/hashes": "npm:^1.8.0" "@signinwithethereum/siwe": "npm:^4.2.1"