diff --git a/__mocks__/crypto-js.ts b/__mocks__/crypto-js.ts deleted file mode 100644 index b3cf92dd787..00000000000 --- a/__mocks__/crypto-js.ts +++ /dev/null @@ -1,8 +0,0 @@ -module.exports = { - ...(jest.requireActual('crypto-js') as any), - AES: { - ...(jest.requireActual('crypto-js').AES as any), - encrypt: jest.fn().mockReturnValue('mockEncryptedValue'), - decrypt: jest.fn().mockReturnValue('mockDecryptedValue'), - }, -} diff --git a/package.json b/package.json index 4a57e58338e..f0e0892260a 100644 --- a/package.json +++ b/package.json @@ -123,7 +123,6 @@ "bignumber.js": "^9.1.2", "clevertap-react-native": "^2.2.1", "country-data": "^0.0.31", - "crypto-js": "^4.2.0", "date-fns": "^3.6.0", "dot-prop-immutable": "^2.1.1", "fast-levenshtein": "^3.0.0", @@ -223,7 +222,6 @@ "@testing-library/jest-native": "^5.4.3", "@testing-library/react-native": "^12.4.5", "@types/country-data": "^0.0.5", - "@types/crypto-js": "^4.1.1", "@types/fast-levenshtein": "^0.0.2", "@types/fs-extra": "^9.0.13", "@types/hoist-non-react-statics": "^3.3.5", diff --git a/src/backup/utils.ts b/src/backup/utils.ts index 2024a88eaa9..d1f41667e8a 100644 --- a/src/backup/utils.ts +++ b/src/backup/utils.ts @@ -1,4 +1,3 @@ -import CryptoJS from 'crypto-js' import { useAsync } from 'react-async-hook' import { showError } from 'src/alert/actions' import AppAnalytics from 'src/analytics/AppAnalytics' @@ -9,6 +8,7 @@ import { useDispatch, useSelector } from 'src/redux/hooks' import { removeStoredItem, retrieveStoredItem, storeItem } from 'src/storage/keychain' import Logger from 'src/utils/Logger' import { CELO_DERIVATION_PATH_BASE, generateKeys } from 'src/utils/account' +import { aesDecrypt, aesEncrypt } from 'src/utils/aes' import { ETHEREUM_DERIVATION_PATH } from 'src/web3/consts' import { currentAccountSelector } from 'src/web3/selectors' @@ -95,10 +95,9 @@ export function isValidBackupPhrase(phrase: string) { } export async function encryptMnemonic(phrase: string, password: string) { - return CryptoJS.AES.encrypt(phrase, password).toString() + return aesEncrypt(phrase, password) } export async function decryptMnemonic(encryptedMnemonic: string, password: string) { - const bytes = CryptoJS.AES.decrypt(encryptedMnemonic, password) - return bytes.toString(CryptoJS.enc.Utf8) + return aesDecrypt(encryptedMnemonic, password) } diff --git a/src/keylessBackup/keychain.test.ts b/src/keylessBackup/keychain.test.ts index 6514b056f97..0f1cf007ddb 100644 --- a/src/keylessBackup/keychain.test.ts +++ b/src/keylessBackup/keychain.test.ts @@ -1,6 +1,7 @@ import { getSECP256k1PrivateKey, storeSECP256k1PrivateKey } from 'src/keylessBackup/keychain' import { getPassword } from 'src/pincode/authentication' import { retrieveStoredItem, storeItem } from 'src/storage/keychain' +import { encryptPrivateKey } from 'src/web3/KeychainAccounts' import { generatePrivateKey } from 'viem/accounts' jest.mock('src/pincode/authentication') @@ -33,8 +34,11 @@ describe(getSECP256k1PrivateKey, () => { it('gets the private key from the keychain', async () => { jest.mocked(getPassword).mockResolvedValue('password') - jest.mocked(retrieveStoredItem).mockResolvedValue(mockPrivateKey) - await getSECP256k1PrivateKey('0x1234') + jest + .mocked(retrieveStoredItem) + .mockResolvedValue(await encryptPrivateKey(mockPrivateKey, 'password')) + const privateKey = await getSECP256k1PrivateKey('0x1234') + expect(privateKey).toBe(mockPrivateKey) expect(getPassword).toHaveBeenCalledWith('0x1234') expect(retrieveStoredItem).toHaveBeenCalledWith('secp256k1PrivateKey') }) diff --git a/src/pincode/authentication.test.ts b/src/pincode/authentication.test.ts index 2d934723abc..29fc8b31264 100644 --- a/src/pincode/authentication.test.ts +++ b/src/pincode/authentication.test.ts @@ -1,4 +1,3 @@ -import CryptoJS from 'crypto-js' import * as Keychain from 'react-native-keychain' import { expectSaga } from 'redux-saga-test-plan' import { select } from 'redux-saga/effects' @@ -6,6 +5,7 @@ import { PincodeType } from 'src/account/reducer' import { pincodeTypeSelector } from 'src/account/selectors' import AppAnalytics from 'src/analytics/AppAnalytics' import { AuthenticationEvents } from 'src/analytics/Events' +import { encryptMnemonic } from 'src/backup/utils' import { storedPasswordRefreshed } from 'src/identity/actions' import { navigate, navigateBack } from 'src/navigator/NavigationService' import { @@ -33,10 +33,11 @@ import { } from 'src/pincode/authentication' import { store } from 'src/redux/store' import Logger from 'src/utils/Logger' +import { aesDecrypt, aesEncrypt } from 'src/utils/aes' import { ensureError } from 'src/utils/ensureError' import { getWalletAsync } from 'src/web3/contracts' import { getMockStoreData } from 'test/utils' -import { mockAccount } from 'test/values' +import { mockAccount, mockMnemonic } from 'test/values' jest.mock('src/web3/contracts') jest.unmock('src/pincode/authentication') @@ -51,6 +52,16 @@ jest.mock('src/utils/sleep', () => ({ sleep: jest.fn().mockResolvedValue(true), })) +jest.mock('src/utils/aes', () => { + const originalAes = jest.requireActual('src/utils/aes') + + return { + ...originalAes, + aesEncrypt: jest.fn().mockImplementation((...args) => originalAes.aesEncrypt(...args)), + aesDecrypt: jest.fn().mockImplementation((...args) => originalAes.aesDecrypt(...args)), + } +}) + const loggerErrorSpy = jest.spyOn(Logger, 'error') const mockPepper = { username: 'some username', @@ -75,6 +86,18 @@ const expectPincodeEntered = () => { expect(navigateBack).toHaveBeenCalled() } +// Keep initial mocked implementation from __mocks__/react-native-keychain.ts +const originalGetGenericPassword = mockedKeychain.getGenericPassword.getMockImplementation() +const originalSetGenericPassword = mockedKeychain.setGenericPassword.getMockImplementation() +const originalResetGenericPassword = mockedKeychain.resetGenericPassword.getMockImplementation() + +beforeEach(async () => { + // Reset to the original mocked implementation to get better isolation between tests + mockedKeychain.getGenericPassword.mockImplementation(originalGetGenericPassword) + mockedKeychain.setGenericPassword.mockImplementation(originalSetGenericPassword) + mockedKeychain.resetGenericPassword.mockImplementation(originalResetGenericPassword) +}) + describe(getPasswordSaga, () => { beforeEach(() => { jest.clearAllMocks() @@ -338,10 +361,14 @@ describe(updatePin, () => { const newPasswordHash = 'd9bb2d77ec27dc8bf4269a6241daaa0388e8908518458f6ce0314380d11411cd' // expectedAccountHash generated from normalizeAddress(mockAccount) const accountHash = 'PASSWORD_HASH-0000000000000000000000000000000000007e57' + let encryptedMnemonicOldPin: string - beforeEach(() => { + beforeEach(async () => { jest.clearAllMocks() clearPasswordCaches() + + encryptedMnemonicOldPin = await encryptMnemonic(mockMnemonic, oldPassword) + mockedKeychain.getGenericPassword.mockImplementation((options) => { if (options?.service === 'PEPPER') { return Promise.resolve(mockPepper) @@ -349,7 +376,7 @@ describe(updatePin, () => { if (options?.service === 'mnemonic') { return Promise.resolve({ username: 'some username', - password: 'mockEncryptedValue', + password: encryptedMnemonicOldPin, service: 'some service', storage: 'some string', }) @@ -390,13 +417,11 @@ describe(updatePin, () => { expect(mockedKeychain.setGenericPassword).toHaveBeenNthCalledWith( 2, 'CELO', - 'mockEncryptedValue', + expect.any(String), // Encrypted mnemonic new pin expect.objectContaining({ service: 'mnemonic' }) ) - // as we are mocking the outcome of encryption/decryption of mnemonic, check - // that they are called with the expected params - expect(CryptoJS.AES.decrypt).toHaveBeenCalledWith('mockEncryptedValue', oldPassword) - expect(CryptoJS.AES.encrypt).toHaveBeenCalledWith('mockDecryptedValue', newPassword) + expect(jest.mocked(aesDecrypt)).toHaveBeenCalledWith(encryptedMnemonicOldPin, oldPassword) + expect(jest.mocked(aesEncrypt)).toHaveBeenCalledWith(mockMnemonic, newPassword) }) it('should update the cached pin, stored password, store mnemonic, and stored pin if biometry is enabled', async () => { @@ -415,7 +440,7 @@ describe(updatePin, () => { expect(mockedKeychain.setGenericPassword).toHaveBeenNthCalledWith( 3, 'CELO', - 'mockEncryptedValue', + expect.any(String), // Encrypted mnemonic new pin expect.objectContaining({ service: 'mnemonic' }) ) expect(mockedKeychain.setGenericPassword).toHaveBeenNthCalledWith( @@ -432,7 +457,13 @@ describe(updatePin, () => { }) describe(removeStoredPin, () => { + beforeEach(() => { + jest.clearAllMocks() + clearPasswordCaches() + }) + it('should remove the item from keychain', async () => { + expect(mockedKeychain.resetGenericPassword).toHaveBeenCalledTimes(0) mockedKeychain.resetGenericPassword.mockResolvedValueOnce(true) await removeStoredPin() diff --git a/src/utils/aes.test.ts b/src/utils/aes.test.ts new file mode 100644 index 00000000000..c072ba62428 --- /dev/null +++ b/src/utils/aes.test.ts @@ -0,0 +1,36 @@ +// This test was initially copied from https://github.com/RaisinTen/aes-crypto-js/blob/2978af8e004d47539d767e751def003fe134b6e2/test.js +// And adapted for this project +import { aesDecrypt, aesEncrypt } from './aes' + +describe.each([ + [ + 'normal characters', + 'Hello, world!', + 'umm, shhh ...', + 'U2FsdGVkX1+W9o0WI1QJGehALRoGMaRfoN2YH36BGTk=', + ], + [ + 'weird characters in secret', + 'Hello, world!', + 'umm, ลกhhh ... ๐Ÿ˜€Dโ—Œฬ‡แ„…แ…กแ†ฑํƒ†๐ฟ ๐‘’นโ—Œฬดโ—Œ๐‘’บ', + 'U2FsdGVkX1/Eq6lXayqOFwfqTdefS3Zqi7LqOeWKrtA=', + ], + [ + 'bytes corresponding to a single character that are split between two buffers', + '\u{30a8}\u{30b9}\u{30af}\u{30fc}\u{30c8}\u{3099}', + 'umm, shhh ...', + 'U2FsdGVkX18JW+58n/s+37y5831hmabBUuwtVf+JkaDZjeVyRNDHc+I/1w8kpAEA', + ], +])('AES encryption and decryption: %s', (scenario, plainText, secret, encryptedByCryptoJS) => { + it('decrypts strings encrypted by crypto-js', () => { + // Note: encryptedByCryptoJS is the result of CryptoJS.AES.encrypt(plainText, secret).toString() + const decrypted = aesDecrypt(encryptedByCryptoJS, secret) + expect(decrypted).toBe(plainText) + }) + + it('decrypts strings encrypted with encryptAES', () => { + const encrypted = aesEncrypt(plainText, secret) + const decrypted = aesDecrypt(encrypted, secret) + expect(decrypted).toBe(plainText) + }) +}) diff --git a/src/utils/aes.ts b/src/utils/aes.ts new file mode 100644 index 00000000000..f2f77682dc3 --- /dev/null +++ b/src/utils/aes.ts @@ -0,0 +1,46 @@ +// This file was copied from https://github.com/RaisinTen/aes-crypto-js/blob/2978af8e004d47539d767e751def003fe134b6e2/index.js +// and modified slightly for TS compatibility. +import crypto from 'crypto' + +// Refs: https://github.com/brix/crypto-js/issues/468#issuecomment-2060562277 +export function aesEncrypt(plainText: string, secret: string) { + const salt = crypto.randomBytes(8) + const password = Buffer.concat([Buffer.from(secret), salt]) + const hash = [] + let digest = password + for (let i = 0; i < 3; i++) { + hash[i] = crypto.createHash('md5').update(digest).digest() + digest = Buffer.concat([hash[i], password]) + } + const keyDerivation = Buffer.concat(hash) + const key = keyDerivation.subarray(0, 32) + const iv = keyDerivation.subarray(32) + const cipher = crypto.createCipheriv('aes-256-cbc', key, iv) + return Buffer.concat([ + Buffer.from('Salted__', 'utf8'), + salt, + cipher.update(plainText), + cipher.final(), + ]).toString('base64') +} + +// Refs: https://github.com/brix/crypto-js/issues/468#issuecomment-1783351942 +export function aesDecrypt(encryptedText: string, secret: string) { + // From https://gist.github.com/schakko/2628689?permalink_comment_id=3321113#gistcomment-3321113 + // From https://gist.github.com/chengen/450129cb95c7159cb05001cc6bdbf6a1 + const cypher = Buffer.from(encryptedText, 'base64') + const salt = cypher.slice(8, 16) + const password = Buffer.concat([Buffer.from(secret), salt]) + const md5Hashes = [] + let digest = password + for (let i = 0; i < 3; i++) { + md5Hashes[i] = crypto.createHash('md5').update(digest).digest() + digest = Buffer.concat([md5Hashes[i], password]) + } + const key = Buffer.concat([md5Hashes[0], md5Hashes[1]]) + const iv = md5Hashes[2] + const contents = cypher.slice(16) + const decipher = crypto.createDecipheriv('aes-256-cbc', key, iv) + + return Buffer.concat([decipher.update(contents), decipher.final()]).toString('utf8') +} diff --git a/src/viem/getLockableWallet.test.ts b/src/viem/getLockableWallet.test.ts index c0c95a49937..6a19a544e9b 100644 --- a/src/viem/getLockableWallet.test.ts +++ b/src/viem/getLockableWallet.test.ts @@ -19,9 +19,6 @@ import { } from 'viem/actions' import { celoAlfajores, goerli as ethereumGoerli, sepolia as ethereumSepolia } from 'viem/chains' -// Use real encryption -jest.unmock('crypto-js') - jest.mock('src/viem', () => { return { viemTransports: { diff --git a/src/web3/KeychainAccounts.test.ts b/src/web3/KeychainAccounts.test.ts index 3b5e66e2ef7..efe5dc0b2a5 100644 --- a/src/web3/KeychainAccounts.test.ts +++ b/src/web3/KeychainAccounts.test.ts @@ -15,9 +15,6 @@ import { mockPrivateKey, } from 'test/values' -// Use real encryption -jest.unmock('crypto-js') - const MOCK_DATE = new Date('2016-12-21T23:36:07.071Z') beforeEach(() => { diff --git a/src/web3/KeychainAccounts.ts b/src/web3/KeychainAccounts.ts index 0cf8263c9c8..db5e3341736 100644 --- a/src/web3/KeychainAccounts.ts +++ b/src/web3/KeychainAccounts.ts @@ -1,5 +1,4 @@ import { isValidAddress, normalizeAddress, normalizeAddressWith0x } from '@celo/utils/lib/address' -import CryptoJS from 'crypto-js' import { ErrorMessages } from 'src/app/ErrorMessages' import { generateKeysFromMnemonic, getStoredMnemonic } from 'src/backup/utils' import { @@ -9,6 +8,7 @@ import { storeItem, } from 'src/storage/keychain' import Logger from 'src/utils/Logger' +import { aesDecrypt, aesEncrypt } from 'src/utils/aes' import { ViemKeychainAccount, keychainAccountToAccount } from 'src/viem/keychainAccountToAccount' import { Hex } from 'viem' import { Address, privateKeyToAddress } from 'viem/accounts' @@ -39,13 +39,12 @@ function accountStorageKey(account: KeychainAccount) { } export async function encryptPrivateKey(privateKey: string, password: string) { - return CryptoJS.AES.encrypt(privateKey, password).toString() + return aesEncrypt(privateKey, password) } export async function decryptPrivateKey(encryptedPrivateKey: string, password: string) { try { - const bytes = CryptoJS.AES.decrypt(encryptedPrivateKey, password) - return bytes.toString(CryptoJS.enc.Utf8) + return aesDecrypt(encryptedPrivateKey, password) } catch (e) { // decrypt can sometimes throw if the inputs are incorrect (encryptedPrivateKey or password) Logger.warn(TAG, 'Failed to decrypt private key', e) diff --git a/src/web3/KeychainWallet.test.ts b/src/web3/KeychainWallet.test.ts index f8109168fa1..a914cd22874 100644 --- a/src/web3/KeychainWallet.test.ts +++ b/src/web3/KeychainWallet.test.ts @@ -3,10 +3,10 @@ import { normalizeAddressWith0x, privateKeyToPublicKey } from '@celo/utils/lib/a import { Encrypt } from '@celo/utils/lib/ecies' import { verifySignature } from '@celo/utils/lib/signatureUtils' import { recoverTransaction, verifyEIP712TypedDataSigner } from '@celo/wallet-base' -import CryptoJS from 'crypto-js' import MockDate from 'mockdate' import * as Keychain from 'react-native-keychain' import { trimLeading0x } from 'src/utils/address' +import { aesEncrypt } from 'src/utils/aes' import { UNLOCK_DURATION } from 'src/web3/consts' import { KeychainAccounts } from 'src/web3/KeychainAccounts' import { KeychainWallet } from 'src/web3/KeychainWallet' @@ -20,9 +20,6 @@ import { mockPrivateKey2, } from 'test/values' -// Use real encryption -jest.unmock('crypto-js') - const CHAIN_ID = 44378 const ONE_CELO_IN_WEI = '1000000000000000000' @@ -401,7 +398,7 @@ describe('KeychainWallet', () => { // Setup mocked keychain content with a private key without the 0x prefix mockedKeychain.setItems({ 'account--2021-01-10T11:14:50.298Z--1be31a94361a391bbafb2a4ccd704f57dc04d4bb': { - password: await CryptoJS.AES.encrypt(mockPrivateKey, 'password').toString(), + password: await aesEncrypt(mockPrivateKey, 'password').toString(), }, }) diff --git a/yarn.lock b/yarn.lock index 1784df14b2b..1c1e4f941f6 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4835,11 +4835,6 @@ resolved "https://registry.yarnpkg.com/@types/country-data/-/country-data-0.0.5.tgz#75d27ccb80f40901d0c537fd95a3a6e3db90a843" integrity sha512-IHr8m91tilfp32svmvAIPs9zuuMKZcUk/HRsZPN7y4jafQdpXHLqiMJtlP1a+W4eE0AkUz1Y42/iMxxXOSNpTQ== -"@types/crypto-js@^4.1.1": - version "4.1.1" - resolved "https://registry.yarnpkg.com/@types/crypto-js/-/crypto-js-4.1.1.tgz#602859584cecc91894eb23a4892f38cfa927890d" - integrity sha512-BG7fQKZ689HIoc5h+6D2Dgq1fABRa0RbBWKBd9SP/MVRVXROflpm5fhwyATX5duFmbStzyzyycPB8qUYKDH3NA== - "@types/d3-array@^3.0.3": version "3.0.3" resolved "https://registry.yarnpkg.com/@types/d3-array/-/d3-array-3.0.3.tgz#87d990bf504d14ad6b16766979d04e943c046dac" @@ -17083,7 +17078,7 @@ string-length@^4.0.1: char-regex "^1.0.2" strip-ansi "^6.0.0" -"string-width-cjs@npm:string-width@^4.2.0": +"string-width-cjs@npm:string-width@^4.2.0", string-width@^4.1.0, string-width@^4.2.0, string-width@^4.2.3: version "4.2.3" resolved "https://registry.yarnpkg.com/string-width/-/string-width-4.2.3.tgz#269c7117d27b05ad2e536830a8ec895ef9c6d010" integrity sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g== @@ -17118,15 +17113,6 @@ string-width@^3.0.0, string-width@^3.1.0: is-fullwidth-code-point "^2.0.0" strip-ansi "^5.1.0" -string-width@^4.1.0, string-width@^4.2.0, string-width@^4.2.3: - version "4.2.3" - resolved "https://registry.yarnpkg.com/string-width/-/string-width-4.2.3.tgz#269c7117d27b05ad2e536830a8ec895ef9c6d010" - integrity sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g== - dependencies: - emoji-regex "^8.0.0" - is-fullwidth-code-point "^3.0.0" - strip-ansi "^6.0.1" - string-width@^5.0.1, string-width@^5.1.2: version "5.1.2" resolved "https://registry.yarnpkg.com/string-width/-/string-width-5.1.2.tgz#14f8daec6d81e7221d2a357e668cab73bdbca794" @@ -19356,7 +19342,7 @@ wordwrap@0.0.2: resolved "https://registry.yarnpkg.com/wordwrap/-/wordwrap-0.0.2.tgz#b79669bb42ecb409f83d583cad52ca17eaa1643f" integrity sha1-t5Zpu0LstAn4PVg8rVLKF+qhZD8= -"wrap-ansi-cjs@npm:wrap-ansi@^7.0.0": +"wrap-ansi-cjs@npm:wrap-ansi@^7.0.0", wrap-ansi@^7.0.0: version "7.0.0" resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-7.0.0.tgz#67e145cff510a6a6984bdf1152911d69d2eb9e43" integrity sha512-YVGIj2kamLSTxw6NsZjoBxfSwsn0ycdesmc4p+Q21c5zPuZ1pl+NfxVdxPtdHvmNVOQ6XSYG4AUtyt/Fi7D16Q== @@ -19391,15 +19377,6 @@ wrap-ansi@^6.2.0: string-width "^4.1.0" strip-ansi "^6.0.0" -wrap-ansi@^7.0.0: - version "7.0.0" - resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-7.0.0.tgz#67e145cff510a6a6984bdf1152911d69d2eb9e43" - integrity sha512-YVGIj2kamLSTxw6NsZjoBxfSwsn0ycdesmc4p+Q21c5zPuZ1pl+NfxVdxPtdHvmNVOQ6XSYG4AUtyt/Fi7D16Q== - dependencies: - ansi-styles "^4.0.0" - string-width "^4.1.0" - strip-ansi "^6.0.0" - wrap-ansi@^8.1.0: version "8.1.0" resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-8.1.0.tgz#56dc22368ee570face1b49819975d9b9a5ead214"