Skip to content

Commit

Permalink
refactor: remove crypto-js which is now deprecated
Browse files Browse the repository at this point in the history
  • Loading branch information
jeanregisser committed Aug 30, 2024
1 parent 3ffb507 commit aead8c2
Show file tree
Hide file tree
Showing 12 changed files with 137 additions and 46 deletions.
8 changes: 0 additions & 8 deletions __mocks__/crypto-js.ts

This file was deleted.

2 changes: 0 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,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",
Expand Down Expand Up @@ -221,7 +220,6 @@
"@sentry/types": "^7.111.0",
"@testing-library/jest-native": "^5.4.3",
"@testing-library/react-native": "^12.4.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",
Expand Down
7 changes: 3 additions & 4 deletions src/backup/utils.ts
Original file line number Diff line number Diff line change
@@ -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'
Expand All @@ -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'

Expand Down Expand Up @@ -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)
}
8 changes: 6 additions & 2 deletions src/keylessBackup/keychain.test.ts
Original file line number Diff line number Diff line change
@@ -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')
Expand Down Expand Up @@ -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')
})
Expand Down
51 changes: 41 additions & 10 deletions src/pincode/authentication.test.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
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'
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 {
Expand Down Expand Up @@ -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')
Expand All @@ -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',
Expand All @@ -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()
Expand Down Expand Up @@ -338,18 +361,22 @@ 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)
}
if (options?.service === 'mnemonic') {
return Promise.resolve({
username: 'some username',
password: 'mockEncryptedValue',
password: encryptedMnemonicOldPin,
service: 'some service',
storage: 'some string',
})
Expand Down Expand Up @@ -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 () => {
Expand All @@ -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(
Expand All @@ -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()

Expand Down
36 changes: 36 additions & 0 deletions src/utils/aes.test.ts
Original file line number Diff line number Diff line change
@@ -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)
})
})
46 changes: 46 additions & 0 deletions src/utils/aes.ts
Original file line number Diff line number Diff line change
@@ -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')
}
3 changes: 0 additions & 3 deletions src/viem/getLockableWallet.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down
3 changes: 0 additions & 3 deletions src/web3/KeychainAccounts.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(() => {
Expand Down
7 changes: 3 additions & 4 deletions src/web3/KeychainAccounts.ts
Original file line number Diff line number Diff line change
@@ -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 {
Expand All @@ -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'
Expand Down Expand Up @@ -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)
Expand Down
7 changes: 2 additions & 5 deletions src/web3/KeychainWallet.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand All @@ -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'
Expand Down Expand Up @@ -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(),
},
})

Expand Down
5 changes: 0 additions & 5 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -4830,11 +4830,6 @@
resolved "https://registry.yarnpkg.com/@types/country-data/-/country-data-0.0.0.tgz#6f5563cae3d148780c5b6539803a29bd93f8f1a1"
integrity sha512-lIxCk6G7AwmUagQ4gIQGxUBnvAq664prFD9nSAz6dgd1XmBXBtZABV/op+QsJsIyaP1GZsf/iXhYKHX3azSRCw==

"@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"
Expand Down

0 comments on commit aead8c2

Please sign in to comment.