👑 Private keys import and accounts backup #3089

jagodarybacka · 2023-02-27T18:39:23Z

About this PR

This PR will contain all changes for features:

allow importing private keys in the keyring service (initial scope of this PR)
import private keys with plain text (Add private key onboarding flow #3119)
import private keys with JSON (Private key JSON import #3177)
backup account with mnemonic and private key export (📚 Accounts backup #3252)

Initial scope

Resolves #3070

What

Handle importing wallets with private keys in the keyring service:

save pk imported accounts in the encrypted vault
handle serialization and deserialization of pk imported wallets
handle signing messages with both keyrings and pk wallets
handle communication between service and redux store about pk wallets
fix types and field names in the redux store

Cleanup and add more tests:

move existing keyring integration tests to the right folder
write more unit tests to cover private key imported wallets
cleanup some duplicated tests
add util functions to the test's factories
add function to mock local storage

Testing

make sure unit tests and E2E are passing
test manually adding accounts and signing transactions

Latest build: extension-builds-3089 (as of Thu, 06 Jul 2023 12:56:07 GMT).

Handle importing wallets with private key in the keyring service: * save pk imported accounts in the encrypted vault * handle serialization and deserialization of pk imported wallets * handle siging messages with both keyrings and single wallets * fix types and field names in the redux store

* add redux method for pk wallets import * rename `getKeyringSourceForAddress` to `getSignerSourceForAddress` * ensure addresses are normalized

* add unit tests for keyring service * move existing tests to keyring service folder * add utils for local storage mocking

* add unit tests for each type of signing transactions * add new test utils * remove some duplicated unit tests

Make separate import seed form component to allow private key import page to reuse the same page layout.

Add page, new icons and link to the private key page on tabbed onboarding flow.

Shadowfiend · 2023-03-09T15:04:19Z

⚠️ Please do not merge without code owner's approval! ⚠️

Btw this will be enforced by GitHub, as long as the target branch is main.

Shadowfiend

Left some higher-level architectural thoughts on how to reconsider the addition of new methods and redux actions by instead adding new variants to the existing data structures, and reconsidering private keys as “simple” keyrings with only one key and no derivation capabilities.

background/services/keyring/encryption.ts

background/main.ts

background/services/keyring/index.ts

Shadowfiend · 2023-03-09T19:18:23Z

background/services/keyring/index.ts

  hiddenAccounts: { [address: HexString]: boolean }
 }

 interface Events extends ServiceLifecycleEvents {
  locked: boolean
  keyrings: {
+    wallets: WalletData[]


Why add this if WalletData (naming aside) conforms to Keyring, the type carried by the keyrings field?

In many ways it's useful to treat a private key as a keyring with only one key. Particularly if we distinguish an HDKeyringAccountSigner from a KeyringAccountSigner, and make address derivation available only to an HDKeyringAccountSigner, I think we continue to drive towards a simpler API surface.

- create single import method to import both HD wallets and pk wallets - rename `wallets` to `privateKeys`

jagodarybacka · 2023-03-14T16:39:10Z

did some renames and other minor improvements (naming ideas still appreciated)
created a single function for importing both HD wallets and private keys
right now I cannot see how merging HD keyrings and pk wallets into single array will be beneficial, both in the service and in the redux as they have different methods and properties. I need to merge it to the other PR to see how it will behave with selectors etc.

Moving this to draft for now, will notify when it is ready.

Closes #3468 ## What When users try to copy a recovery phrase/private key we should warn them that it's not safe to do so. ## UI https://github.com/tahowallet/extension/assets/23117945/ce1d143e-58ef-4f23-bad8-e1b355954a9d ## Testing - [x] Try to copy a recovery phrase/private key, warning should warn that it's not safe to do. - [x] Check the "Don’t ask me again" option. The warning should not be displayed again. Latest build: [extension-builds-3488](https://github.com/tahowallet/extension/suites/13780837887/artifacts/763854812) (as of Thu, 22 Jun 2023 06:03:38 GMT).

Closes #3471 Adds a dropdown on settings to allow changing the maximum amount of time the wallet can remain idle before locking signing. This new setting is updated through an async thunk which triggers an event to also handle the change on the keyring service. The data is persisted on preferences db and exposed through redux to the UI. ## Testing Env ``` SUPPORT_CUSTOM_AUTOLOCK=true ``` ## To Test - [x] Import an account - [x] Lock and unlock your wallet - [x] Head to settings, change auto-lock time - [x] Change your system's date or change the options [here](https://github.com/tahowallet/extension/blob/be6fae78bd7949cd5e4d8f37c882152d00e8141d/ui/pages/Settings.tsx#L308), test that different time options work correctly - [x] If the wallet has been idle for 10 minutes and the current auto lock setting changes to from 15 minutes to 5, the wallet should lock itself. Latest build: [extension-builds-3477](https://github.com/tahowallet/extension/suites/13795656037/artifacts/764974743) (as of Thu, 22 Jun 2023 16:11:15 GMT).

- add required packages - configure webpack to work with webAssembly - add neccessary config to `content_security_policy` to allow webAssembly

Let's set length to 32 bytes to match length expected by AES-GCM

Catch erros if vaults migration to Argon2 fails and allow to continue with old vaults encrypted with PBKDF2. Log analytics event when vaults are succesfully migrated.

- allowed Jest to fetch WebAssembly files - moved `crypto.subtle` mock to global setup

- for Jest to work with WebAssembly we need to update to next major version - to support dependencies for new Jest version we need to bump Typesript as well - let's fix problems found by new Typescript version

Allow destructuring objects to remove unwanted fields from the objects. This is pattern we are using often across the codebase.

- return `success` boolean - make sure decrypted vaults match - send event on migration fail

Vault migration is no longer tracked as Argon2 specifically, but generically for all migrations. Already-migrated vaults are not tracked, and the migration function return value reflects that no migration was performed. Additionally, error messages are bubbled out of the migration function and reported up to the caller. The main outcome here is that PostHog migration events include the migrated-to version, and PostHog migration failure events include the error message. This will leave us open to future migrations, and will let us know if there are certain failures that are happening broadly that we may be able to do something about. Notably, wrong passwords will be tracked as migration errors if a wrong password is typed with an older vault version in the mix. Mitigating this may or may not be a good idea.

Resolves #3470 ### What Let's use Argon2 instead of PBKDF2 🔑 What was already done: - [x] added required packages - [x] configured webpack config to work with webAssembly following [example from the docs](https://github.com/antelle/argon2-browser/tree/master/examples/webpack) - [x] add necessary config to `content_security_policy` to allow webAssembly - without `'wasm-eval'` we are not able to use argon implementation in the extension - [x] migration from old vaults to new vaults - [x] handling errors during migration and allowing users to continue with old implementation if needed - [x] added one time analytics event after successful migration - [x] updated Jest to allow unit tests to work with WebAssembly, updated Typescript as this was necessary to make new version of Jest work correctly and fixed a bunch of Typescript issues - most fixes are copied from #3415 ### Testing - [x] test migration from main - install extension on `main`, add some HD wallets, checkout this branch, reload and unlock the wallet, make sure you don't see the error about failed migration in the background console, check if analytics event has been emitted, lock and unlock more than one time - [x] install extension on this branch, test locking and unlocking the wallet - [x] try to make webassembly part break (throw Error [here](https://github.com/tahowallet/extension/blob/migrate-to-argon/background/services/internal-signer/encryption.ts#L118)) and make sure user is able to continue with old vaults Latest build: [extension-builds-3502](https://github.com/tahowallet/extension/suites/14011994578/artifacts/782054890) (as of Sun, 02 Jul 2023 21:28:25 GMT).

Shadowfiend

All righty, we are good to go here. Waiting on one more round of thoughts from the auditors and then we're going to land this puppy with great fanfare.

Shadowfiend

It is time 🏆

@jagodarybacka

## What's Changed * Add private key onboarding flow by @jagodarybacka in #3119 * Private key JSON import by @jagodarybacka in #3177 * Allow export of private keys and mnemonics by @jagodarybacka in #3248 * Export private key form by @jagodarybacka in #3255 * Unlock screen for the account backup by @kkosiorowska in #3257 * Show mnemonic menu by @jagodarybacka in #3259 * Fix background blur issue by @jagodarybacka in #3265 * Account backup UI fixes by @jagodarybacka in #3270 * Fix unhiding removed accounts by @jagodarybacka in #3282 * New error for incorrectly decrypted JSON file by @jagodarybacka in #3293 * Export private keys from HD wallet addresses by @jagodarybacka in #3253 * Refactor keyring redux slice to remove `importing` field by @jagodarybacka in #3309 * 📚 Accounts backup by @kkosiorowska in #3252 * Catch Enter keypress on Unlock screen by @jagodarybacka in #3355 * Rename `keyring` to `internal signer` and other improvements by @jagodarybacka in #3331 * 🗝 QA - Accounts backup and private key import by @jagodarybacka in #3266 * Remove private key signers if they are replaced by accounts from HD wallet by @jagodarybacka in #3377 * RFB 4: One-Off Keyring Design by @Shadowfiend in #3372 * Copy to clipboard warning by @kkosiorowska in #3488 * Allow setting custom auto-lock timer by @hyphenized in #3477 * Use Argon2 for encrypted vaults by @jagodarybacka in #3502 * 👑 Private keys import and accounts backup by @jagodarybacka in #3089 * Untrusted assets should not block the addition of custom tokens by @kkosiorowska in #3491 * Flip updated dApp connections flag by @Shadowfiend in #3492 * v0.41.0 by @Shadowfiend in #3531 * Switch to a given network if adding a network that is already added. by @0xDaedalus in #3154 * Remove waiting for Loading Doggo component in E2E tests by @jagodarybacka in #3541 * Squeeze content to better fit on Swaps page by @jagodarybacka in #3542 * Refactor of terms for verified/unverified assets by @kkosiorowska in #3528 * Fix ChainList styling by @fulldecent in #3547 * Update release checklist by @jagodarybacka in #3548 * Fix custom asset price fetching by @hyphenized in #3508 * Sticky Defaults: Make Taho-as-default replace MetaMask in almost all cases by @Shadowfiend in #3546 ## New Contributors * @fulldecent made their first contribution in #3547 **Full Changelog**: v0.41.0...v0.42.0 Latest build: [extension-builds-3549](https://github.com/tahowallet/extension/suites/14268975651/artifacts/801826435) (as of Thu, 13 Jul 2023 09:51:56 GMT).

jagodarybacka self-assigned this Feb 27, 2023

jagodarybacka added 4 commits March 2, 2023 17:28

Adjust keyring methods for private key wallets

0d0141d

* add redux method for pk wallets import * rename `getKeyringSourceForAddress` to `getSignerSourceForAddress` * ensure addresses are normalized

Adjust keyring tests for private keys import

abefd93

* add unit tests for keyring service * move existing tests to keyring service folder * add utils for local storage mocking

Add keyring service unit tests for signing transactions

d34b19c

* add unit tests for each type of signing transactions * add new test utils * remove some duplicated unit tests

Merge branch 'main' into keyring-with-pk

f01f26e

jagodarybacka marked this pull request as ready for review March 3, 2023 14:01

jagodarybacka requested review from Shadowfiend and mhluongo as code owners March 3, 2023 14:01

jagodarybacka requested a review from 0xDaedalus March 3, 2023 14:01

jagodarybacka added 11 commits March 6, 2023 12:01

Add redux migration for keyrings slice

c6a6d4b

Merge branch 'main' into keyring-with-pk

514ac59

Refactor tabbed onboarding form to separate component

e003420

Make separate import seed form component to allow private key import page to reuse the same page layout.

Add private key import page to onboarding

ad51b3b

Add page, new icons and link to the private key page on tabbed onboarding flow.

Mark pk import process as finished

1244666

Merge branch 'main' into keyring-with-pk

080d10f

Merge branch 'keyring-with-pk' into pk-import-on-onboarding

4e45de4

Update doggo icon

137217a

Allow wallet to recognize private key wallets as valid signers

17bf915

Show accounts imported with private key in new section

dc946ee

Add SUPPORT_PRIV_KEYS flag to hide priv keys from UI

e35366d

jagodarybacka added this to the Private Key Import milestone Mar 8, 2023

Shadowfiend requested changes Mar 9, 2023

View reviewed changes

jagodarybacka added 4 commits March 10, 2023 12:15

Merge branch 'main' into keyring-with-pk

1856cf7

Merge branch 'keyring-with-pk' into pk-import-on-onboarding

e39db89

Adjust keyring types

04e66ba

Refactor keyring service names and methods for pk wallets

c399671

- create single import method to import both HD wallets and pk wallets - rename `wallets` to `privateKeys`

jagodarybacka requested a review from a team as a code owner March 14, 2023 16:33

jagodarybacka and others added 22 commits June 22, 2023 10:38

Merge branch 'keyring-with-pk' into locked-and-loaded

298d19e

Add missing units to auto lock timer settings

86dd6d8

Merge branch 'main' into keyring-with-pk

4e3a7f9

Merge branch 'main' into keyring-with-pk

aa9df5f

Rewrite vault encryption function using argon2

e789c36

- add required packages - configure webpack to work with webAssembly - add neccessary config to `content_security_policy` to allow webAssembly

Specify argon hash length

2884bf8

Let's set length to 32 bytes to match length expected by AES-GCM

Merge branch 'main' into keyring-with-pk

4cd218b

Add migration strategy for vaults encrypted with PBKDF2

ccf69a8

Merge branch 'keyring-with-pk' into migrate-to-argon

cff31fd

Rename old.. to deprecated...

7de1fbf

Handle errors on vaults migration

9fb2145

Catch erros if vaults migration to Argon2 fails and allow to continue with old vaults encrypted with PBKDF2. Log analytics event when vaults are succesfully migrated.

Allow Jest to work with wasm

8ce4d3b

- allowed Jest to fetch WebAssembly files - moved `crypto.subtle` mock to global setup

Move sending migration event to Internal Signer service

b04ee01

Bump Jest and Typescript versions

1d3d1a6

- for Jest to work with WebAssembly we need to update to next major version - to support dependencies for new Jest version we need to bump Typesript as well - let's fix problems found by new Typescript version

Add new unit tests to test encryption with Argon2

1acca3b

Add ignoreRestSiblings to no-unused-vars lint rule

0216036

Allow destructuring objects to remove unwanted fields from the objects. This is pattern we are using often across the codebase.

Adjust no-unused-vars rule to use _ for unused elements

ac79ba4

Refactor Argon migration function

a569168

- return `success` boolean - make sure decrypted vaults match - send event on migration fail

Shadowfiend previously approved these changes Jul 5, 2023

View reviewed changes

Merge branch 'main' into keyring-with-pk

0a13649

jagodarybacka dismissed Shadowfiend’s stale review via 0a13649 July 6, 2023 12:47

Shadowfiend approved these changes Jul 7, 2023

View reviewed changes

Shadowfiend merged commit 713acec into main Jul 7, 2023
5 checks passed

Shadowfiend deleted the keyring-with-pk branch July 7, 2023 22:24

kkosiorowska mentioned this pull request Jul 13, 2023

v0.42.0 #3549

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

👑 Private keys import and accounts backup #3089

👑 Private keys import and accounts backup #3089

jagodarybacka commented Feb 27, 2023 •

edited by github-actions bot

Loading

Shadowfiend commented Mar 9, 2023

Shadowfiend left a comment

Shadowfiend Mar 9, 2023

jagodarybacka commented Mar 14, 2023 •

edited

Loading

Shadowfiend left a comment

Shadowfiend left a comment

👑 Private keys import and accounts backup #3089

👑 Private keys import and accounts backup #3089

Conversation

jagodarybacka commented Feb 27, 2023 • edited by github-actions bot Loading

About this PR

Initial scope

What

Testing

Shadowfiend commented Mar 9, 2023

Shadowfiend left a comment

Choose a reason for hiding this comment

Shadowfiend Mar 9, 2023

Choose a reason for hiding this comment

jagodarybacka commented Mar 14, 2023 • edited Loading

Shadowfiend left a comment

Choose a reason for hiding this comment

Shadowfiend left a comment

Choose a reason for hiding this comment

jagodarybacka commented Feb 27, 2023 •

edited by github-actions bot

Loading

jagodarybacka commented Mar 14, 2023 •

edited

Loading