metamask-mobile feat: bump `react-native-aes-crypto`

Description

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 https://github.com/MetaMask/metamask-mobile/pull/9898

Related issues

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

Manual testing steps

Upgrade app - Wallet creation

Build the latest release of the app and create a wallet
Add an account via private key
Lock the wallet
Change branches to this one (feat/bump-rn-aes)
Unlock the wallet
Lock and Unlock again
Check SRP and private keys are accessible
Use the MM Test Dapp to sign messages
Use the send transaction feature

Upgrade app - Import SRP

Build the latest release of the app and import a SRP
Follow the steps from the previous test case

Create a wallet

Create a new wallet
Follow the steps from the previous test case

Import a SRP

Import a SRP
Follow the steps from the previous test case

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 and MetaMask Mobile Coding Standards.
[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 format if applicable
[ ] I’ve applied the right labels on the PR (see labeling guidelines). 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.

Jun 11 '24 22:06 gantunesr

#9540

Jun 11 '24 22:06 legobeat

Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: 075efe2681d0c6968350dfea705ff675557c4f43 Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/be9c25d5-5ade-4dce-8018-2293040585e5

Jun 12 '24 02:06 github-actions[bot]

New and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Package	New capabilities	Transitives	Size	Publisher
npm/[email protected]	None	`0`	45.9 kB	tectiv3

🚮 Removed packages: npm/[email protected]

View full report↗︎

Jun 12 '24 21:06 socket-security[bot]

Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: 19e482aa51d435048f8bae0a9b2a7f4f71a17cd1 Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/78faed31-5e32-47c7-afad-911abbc8d13c

Jun 12 '24 22:06 github-actions[bot]

Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: 32c145c7aed2036e5abeb6fa30d42139355e7a6f Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/871f66f6-c2c9-4df1-a498-394a53b5883f

Jun 13 '24 02:06 github-actions[bot]

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

Jun 17 '24 21:06 github-actions[bot]

Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: 48e9113e1853a6a8f8331e6c9af86d759fdb26a5 Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/693ac1ab-eb93-4ce8-9a78-7ab27437890b

Jun 17 '24 22:06 github-actions[bot]

Could you add a recording to the PR?

Jun 18 '24 14:06 tommasini

@tommasini a recording of what specifically? This PR does not have an UI/UX impact

Jun 18 '24 21:06 gantunesr

Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: d151a951ae9aa293dca148a5be1af227b7f25031 Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/be63b4a7-1e6c-4a50-811f-92f6f0af2f2c

Jun 18 '24 21:06 github-actions[bot]

I thought this library had an impact on the creating new wallet feature and importing a new wallet, I was mentioning a recording on those features sorry, I was not specific at all!

Just for history purposes, if you think it makes sense of course!

Jun 19 '24 00:06 tommasini

@tommasini thanks for the clarification, I'll post the videos for account creation and SRP import

Jun 19 '24 15:06 gantunesr

Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: 1f18b51e33c62cec6dab82b7ba5bd808db68ebca Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/c6724d0e-195b-431d-b7a2-f3c8ee336f9a

Jul 15 '24 14:07 github-actions[bot]

@gantunesr, for context, why is SHA-512/256 being used here? Is there any specific reason? (I'm not trying to imply that it's not ok, but just to understand the reason.)

Jul 16 '24 14:07 danroc

Note: I requested the security team to review this PR and check if it's ok to bump the version.

Jul 16 '24 14:07 danroc

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #9947      +/-   ##
==========================================
- Coverage   49.78%   49.64%   -0.15%     
==========================================
  Files        1437     1446       +9     
  Lines       34762    35032     +270     
  Branches     3921     3969      +48     
==========================================
+ Hits        17307    17390      +83     
- Misses      16354    16543     +189     
+ Partials     1101     1099       -2

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

Jul 16 '24 14:07 codecov-commenter

I can't say for sure @danroc, this was the initial implementation that was done years ago

Jul 16 '24 21:07 gantunesr

Not related to this PR, but it seems that we are using CBC to encrypt the vault without any kind of MAC, which is a bit concerning. IMO we should switch to a AEAD algorithm as soon as possible.

Jul 17 '24 07:07 danroc

Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: b346cd2ae75f33c4b8be09f3d1431ef4ce3dc1db Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/e8826ec5-244c-443d-a8b6-04941d427cfa

Jul 17 '24 16:07 github-actions[bot]

Devices and Authentication Methods

iPhone Xs (iOS 17.5.1, FaceID)
Pixel 5a (Android 14, PIN, Security Patch: May 1, 2024)
iPhone 13 mini (iOS 15.6.1, FaceID)
Samsung a515f (Android 12, Fingerprint, Security Patch: January 1, 2024)

Additional Tests

Migration tests with QR and Ledger hardware wallets. Tested revealing private key, SRP, and password changes.

Critical Issue:

Android 14 Devices: App crashes on unlock attempt after upgrade or new install (observed on Pixel 5a, Pixel 8, Pixel 8 Pro). Was not able to be reproduced with Android versions 9-13 on physical devices or browserstack, but was unavoidable on Android 14.

Issue with Biometric Authentication Post-Upgrade if wallet is locked (step 3 in manual tests)

Current Behavior: When a user locks the wallet while using biometric authentication for MetaMask, they are required to use the password for the subsequent unlock. The biometric icon is NOT shown in the password input box. Post-Upgrade Behavior: After locking the wallet and upgrading to the test branch from a QA build of release v7.24.4, the biometric icon appears in the password input box but does not function for unlocking. Impact User Experience: This inconsistency can cause confusion, frustration, and panic. Risk Assessment: Presenting the biometric icon without functionality does not increase the risk of funds loss. Users will either unlock with their password or reset the wallet using their SRP backup. Recommendation: Suppress the biometric icon in the password input box after an upgrade if the user upgraded while the wallet was locked. This will prevent user confusion and ensure a consistent experience. Of note: this was not observed when I upgraded from a QA build of release v7.24.4 to 7.26.0, but was observed upgrading from 7.24.4 to this branch in test. It appears this condition may be introduced here or could already on main.

Typical login screen after locking wallet while biometric authentication is active for MetaMask on the Left, and on the right is the login screen observed when updating from v7.24.4 to this branch:

Minor annoyance:

Import Private Key Error: "Something went wrong..." error on first attempt to import a private key via QR code observed initially on all 3 physical devices (exception being Android 14 due to critical issue above). Issue did not recur on subsequent attempts with the same QR code. This may affect freshly created wallets but not upgraded ones, and appears sporadic. As seen in the following screenshot the error may appear overtop the success Account imported successfully message.

Jul 18 '24 03:07 plasmacorral

I have reproduced the android crash on the same Pixel 5a using a QA build of the nightly main v7.26.1. Issue opened here.

Jul 19 '24 16:07 plasmacorral

@plasmacorral can you double check if the biometric bug is from this PR? That issue seems unrelated to these changes and I believe it is not being introduced here

Jul 20 '24 02:07 gantunesr

biometric observation was reproduced updating from QA build of production v 7.24.4 to a QA nightly build of main nightly main v7.26.1 from 7/18, so confirms that is not introduced here. Fresh issue reported here.

Jul 22 '24 17:07 plasmacorral

Update:

Android 14 issue is resolved with changes from main
Biometric observation also present on main and not introduced in this PR. Reported here.
Unable to repro error message while importing pk.

Jul 22 '24 21:07 plasmacorral

Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: f416c965563b2f352beffa2ac4dfe89a9cd40d79 Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/01384cb0-4894-4440-9c5e-a7741d337429

Jul 22 '24 21:07 github-actions[bot]

Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: 2f882030bb816569bb65a74628f847974507ced1 Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/3cbb694f-2bed-42c6-accd-d8a5e900abf0