Skip to content

Commit

Permalink
feat: bump react-native-aes-crypto (#9947)
Browse files Browse the repository at this point in the history
<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **Description**

<!--
Write a short description of the changes included in this pull request,
also include relevant motivation and context. Have in mind the following
questions:
1. What is the reason for the change?
2. What is the improvement/solution?
-->

The changes on this PR includes,

- Bump of the `aes-react-native` module from 1.3.9 to 3.0.2

This PR depends on #9898

## **Related issues**

Fixes: https://github.com/MetaMask/mobile-planning/issues/1551
- #9540

## **Manual testing steps**

### Upgrade app - Wallet creation

1. Build the latest release of the app and create a wallet
2. Add an account by importing the private key
3. Lock the wallet
4. Change branches to this one (`feat/bump-rn-aes`)
5. Unlock the wallet
6. Lock and Unlock again
7. Check SRP and private keys are accessible
8. Use the MM Test Dapp to sign messages
9. Use the send transaction feature with a testnet

### Upgrade app - Import SRP

1. Build the latest release of the app and import a SRP
2. Follow the steps from the first test case from step 2

### Create a wallet

1. Create a new wallet
2. Follow the steps from the first test case from step 2

### Import a SRP

1. Import a SRP
2. Follow the steps from the first test case from step 2

## **Screenshots/Recordings**


https://github.com/MetaMask/metamask-mobile/assets/17601467/982a879b-9d22-4afd-9b47-65db1b0120d7

## **Pre-merge author checklist**

- [x] I’ve followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile
Coding
Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've completed the PR template to the best of my ability
- [x] I’ve included tests if applicable
- [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [ ] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **Pre-merge reviewer checklist**

- [ ] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [ ] I confirm that this PR addresses all acceptance criteria described
in the ticket it closes and includes the necessary testing evidence such
as recordings and or screenshots.

---------

Co-authored-by: legobeat <[email protected]>
Co-authored-by: Daniel Rocha <[email protected]>
Co-authored-by: Owen Craston <[email protected]>
  • Loading branch information
4 people authored and devin-ai-integration[bot] committed Jul 23, 2024
1 parent 5334431 commit 0c45efa
Show file tree
Hide file tree
Showing 8 changed files with 71 additions and 70 deletions.
31 changes: 24 additions & 7 deletions app/core/Encryptor/Encryptor.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
import { NativeModules } from 'react-native';
import { Encryptor } from './Encryptor';
import { ENCRYPTION_LIBRARY, LEGACY_DERIVATION_OPTIONS } from './constants';
import {
ShaAlgorithm,
CipherAlgorithm,
ENCRYPTION_LIBRARY,
LEGACY_DERIVATION_OPTIONS,
} from './constants';

const Aes = NativeModules.Aes;
const AesForked = NativeModules.AesForked;
Expand Down Expand Up @@ -55,7 +60,13 @@ describe('Encryptor', () => {
{
lib: ENCRYPTION_LIBRARY.original,
expectedKeyValue: 'mockedKey',
expectedPBKDF2Args: ['testPassword', 'mockedSalt', 5000, 256],
expectedPBKDF2Args: [
'testPassword',
'mockedSalt',
5000,
256,
ShaAlgorithm.Sha512,
],
},
],
[
Expand Down Expand Up @@ -83,15 +94,21 @@ describe('Encryptor', () => {
);

expect(decryptedObject).toEqual(expect.any(Object));

const expectedDecryptionArgs =
lib === ENCRYPTION_LIBRARY.original
? [
mockVault.cipher,
expectedKeyValue,
mockVault.iv,
CipherAlgorithm.cbc,
]
: [mockVault.cipher, expectedKeyValue, mockVault.iv];
expect(
lib === ENCRYPTION_LIBRARY.original
? decryptAesSpy
: decryptAesForkedSpy,
).toHaveBeenCalledWith(
mockVault.cipher,
expectedKeyValue,
mockVault.iv,
);
).toHaveBeenCalledWith(...expectedDecryptionArgs);
expect(
lib === ENCRYPTION_LIBRARY.original
? pbkdf2AesSpy
Expand Down
2 changes: 1 addition & 1 deletion app/core/Encryptor/Encryptor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ class Encryptor implements WithKeyEncryptor<EncryptionKey, Json> {
key: EncryptionKey,
payload: EncryptionResult,
): Promise<unknown> => {
// TODO: Check for key and payload compatiblity?
// TODO: Check for key and payload compatibility?

// We assume that both `payload.lib` and `key.lib` are the same here!
const lib = getEncryptionLibrary(payload.lib);
Expand Down
26 changes: 25 additions & 1 deletion app/core/Encryptor/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,34 @@ export enum KeyDerivationIteration {
OWASP2023Default = 900_000,
}

/**
* Supported SHA algorithms in react-native-aes v3.0.3
*/
export enum ShaAlgorithm {
Sha256 = 'sha256',
Sha512 = 'sha512',
}

/**
* Supported cipher algorithms in react-native-aes v3.0.3
*
* Important Note: Make sure to validate the compatibility of the algorithm with the underlying library.
* The react-native-aes-crypto does a string validation for the algorithm, so it's important to make sure
* we're using the correct string.
*
* References:
* - encrypt: https://github.com/MetaMask/metamask-mobile/pull/9947#:~:text=When-,encrypting,-and%20decypting%20the
* - decrypt: https://github.com/MetaMask/metamask-mobile/pull/9947#:~:text=When%20encrypting%20and-,decypting,-the%20library%20uses
*/
export enum CipherAlgorithm {
cbc = 'aes-cbc-pkcs7padding',
ctr = 'aes-ctr-pkcs5padding',
}

/**
* Used as a "tag" to identify the underlying encryption library.
*
* When no tag is speficied, this means our "forked" encryption library has been used.
* When no tag is specified, this means our "forked" encryption library has been used.
*/
export const ENCRYPTION_LIBRARY = {
original: 'original',
Expand Down
17 changes: 13 additions & 4 deletions app/core/Encryptor/lib.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import { NativeModules } from 'react-native';
import {
ENCRYPTION_LIBRARY,
KDF_ALGORITHM,
LEGACY_DERIVATION_OPTIONS,
ShaAlgorithm,
CipherAlgorithm,
ENCRYPTION_LIBRARY,
SHA256_DIGEST_LENGTH,
LEGACY_DERIVATION_OPTIONS,
} from './constants';
import { EncryptionLibrary, KeyDerivationOptions } from './types';

Expand All @@ -29,7 +31,14 @@ class AesEncryptionLibrary implements EncryptionLibrary {
password,
salt,
opts.params.iterations,
// We're using SHA512 but returning a key with length 256 bits.
// Truncating the output to 256 bits is intentional and considered safe.
//
// References:
// - https://nvlpubs.nist.gov/nistpubs/FIPS/NIST.FIPS.180-4.pdf
// - https://eprint.iacr.org/2010/548.pdf
SHA256_DIGEST_LENGTH,
ShaAlgorithm.Sha512,
);
};

Expand All @@ -39,10 +48,10 @@ class AesEncryptionLibrary implements EncryptionLibrary {
await Aes.randomKey(size);

encrypt = async (data: string, key: string, iv: unknown): Promise<string> =>
await Aes.encrypt(data, key, iv);
await Aes.encrypt(data, key, iv, CipherAlgorithm.cbc);

decrypt = async (data: string, key: string, iv: unknown): Promise<string> =>
await Aes.decrypt(data, key, iv);
await Aes.decrypt(data, key, iv, CipherAlgorithm.cbc);
}

class AesForkedEncryptionLibrary implements EncryptionLibrary {
Expand Down
6 changes: 3 additions & 3 deletions ios/Podfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -500,8 +500,8 @@ PODS:
- React-jsinspector (0.72.15)
- React-logger (0.72.15):
- glog
- react-native-aes (1.3.9):
- React
- react-native-aes (3.0.3):
- React-Core
- react-native-background-timer (2.1.1):
- React
- react-native-ble-plx (3.1.2):
Expand Down Expand Up @@ -1199,7 +1199,7 @@ SPEC CHECKSUMS:
React-jsiexecutor: ce8ecfcd3b7dbc9cb65a661110be17f5afd18aa3
React-jsinspector: b86a8abae760c28d69366bbc1d991561e51341ed
React-logger: ed7c9e01e58529065e7da6bf8318baf15024283e
react-native-aes: ff31f0dd4c791eb423a631ee04570fcf3c618924
react-native-aes: 0143040f4e0cb19296b69b4acc7ddd8d3df9d62d
react-native-background-timer: 1b6e6b4e10f1b74c367a1fdc3c72b67c619b222b
react-native-ble-plx: f0557dbb6bd1f26cca75a67b5f33cfc7f7f9abed
react-native-blob-jsi-helper: 13c10135af4614dbc0712afba5960784cd44f043
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@
"randomfill": "^1.0.4",
"react": "18.2.0",
"react-native": "0.72.15",
"react-native-aes-crypto": "1.3.9",
"react-native-aes-crypto": "3.0.3",
"react-native-aes-crypto-forked": "git+https://github.com/MetaMask/react-native-aes-crypto-forked.git#397d5db5250e8e7408294807965b5b9fd4ca6a25",
"react-native-animatable": "^1.3.3",
"react-native-background-timer": "2.1.1",
Expand Down
49 changes: 0 additions & 49 deletions patches/react-native-aes-crypto+1.3.9.patch

This file was deleted.

8 changes: 4 additions & 4 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -25444,10 +25444,10 @@ react-mixin@^3.0.5:
version "1.2.1"
resolved "git+https://github.com/MetaMask/react-native-aes-crypto-forked.git#397d5db5250e8e7408294807965b5b9fd4ca6a25"

react-native-aes-crypto@1.3.9:
version "1.3.9"
resolved "https://registry.yarnpkg.com/react-native-aes-crypto/-/react-native-aes-crypto-1.3.9.tgz#6f068535462c1b7d7d70a4152d519ae9911c3e69"
integrity sha512-xMl/VCWC9riSyidFsCjXQbR+BIxCZyQtRfVR4dLZbHsk3AMZnVw8VojKVIXRmQtSm3uEMdTUvcLm9HaB3Xf+Cw==
react-native-aes-crypto@3.0.3:
version "3.0.3"
resolved "https://registry.yarnpkg.com/react-native-aes-crypto/-/react-native-aes-crypto-3.0.3.tgz#147a8240c92719ddb925ec6c3e46dcfb69f9f7db"
integrity sha512-6yJjAix83xMDMCDUQLVpoHQMkTfeWoKrDXWP2Yrl68o2053aHPk3hTW6sCTYLj5HmftZ9S9DQ3DelP7WVlIDtw==

[email protected], react-native-animatable@^1.3.3:
version "1.3.3"
Expand Down

0 comments on commit 0c45efa

Please sign in to comment.