Private wallet data preferences should be encrypted #2555

bsclifton · 2018-12-13T20:48:01Z

Description

Carried over from brave/browser-laptop#10705

The wallet private key and passphrase are as sensitive as most passwords, so we should use the Brave password manager encryption key to encrypt them before saving to disk.

bridiver · 2018-12-13T20:58:41Z

I think the best way to handle this is to add something like LedgerClient::EncryptString/LedgerClient::DecryptString which internally uses os_crypt. It doesn't look like there is "keychain" storage available on all platforms so I think we just need to encrypt before saving to disk (base64 encoded if saving to json). @diracdeltas ?

diracdeltas · 2018-12-13T21:31:59Z

what does os_crypt use to store the encryption key if a keychain-like mechanism isn't available?

bridiver · 2018-12-13T21:32:43Z

@diracdeltas os_crypt just encrypts the data using the OS encryption key and then we would just store the output in a file. I'm just saying that it doesn't look like we have a good way to directly store the output in something like macos keychain vs storing the encrypted output in a local file

diracdeltas · 2018-12-13T21:38:56Z

@bridiver i think we're on the same page here; in the original issue i mean store the encryption key in keyring (or whatever is available on the OS*), not store the private key itself in the keyring. the encrypted output can be stored in regular storage.

* appears this would be keyring on macos / Gnome Keyring or KDE Wallet Manager on linux

bridiver · 2018-12-13T21:40:18Z

the only concern I have here is that 3rd-party clients may not properly encrypt the data and could make bat-native-ledger/ads look insecure through a poor implementation of the encrypt/decrypt methods. The alternative would be to embed os_crypt into the lib which should be ok because it has code for all platforms including IOS. cc @tmancey @SergeyZhukovsky

diracdeltas · 2018-12-18T21:07:42Z

Does this not encrypt on windows due to lack of an underlying OS keychain-like mechanism?

diracdeltas · 2020-07-24T23:22:58Z

we should also encrypt the Authorization token (stored in the Preferences file under external_wallets > token ). @ryanml originally pointed this out in february but shoutout to hihihou on HackerOne for reminding me to put it in this issue since it's part of the same overall problem (sensitive rewards data needs to be encrypted on disk).

mrose17 · 2020-09-09T19:13:46Z

@mrose17 - tracking

Resolves brave/brave-browser#2555

LaurenWags · 2020-10-02T19:25:13Z

Verified passed with

Brave	1.16.50 Chromium: 86.0.4240.55 (Official Build) dev (x86_64)
Revision	a6d625ef6f7fe8ea0675f1cf759155a05ee1be40-refs/branch-heads/4240@{#953}
OS	macOS Version 10.14.6 (Build 18G3020)

Verified plan 1 from brave/brave-core#6213. Confirmed able to connect to Uphold on clean profile and view balance. Confirmed able to 1 time tip. Confirmed able to perform AC and recurring tip.

Verified plan 2 from brave/brave-core#6213. Confirmed brave://rewards was displayed correctly after upgrade from 1.15.x to 1.16.x and Uphold wallet was still connected as expected. After upgrade, confirmed able to 1 time tip. Confirmed able to perform AC and recurring tip.

Verification passed on

Brave	1.16.52 Chromium: 86.0.4240.68 (Official Build) dev (64-bit)
Revision	ad72ee9aa8e15ed300df1238e76c7a8f4d686f97-refs/branch-heads/4240@{#1097}
OS	Ubuntu 18.04 LTS

Verified plan 1 from brave/brave-core#6213. Confirmed able to connect to Uphold on clean profile and view balance. Confirmed able to 1 time tip. Confirmed able to perform AC and recurring tip.

Verified plan 2 from brave/brave-core#6213. Confirmed brave://rewards was displayed correctly after upgrade from 1.15.x to 1.16.x and Uphold wallet was still connected as expected. After upgrade, confirmed able to 1 time tip. Confirmed able to perform AC and recurring tip.

Verification passed on


Brave | 1.16.58 Chromium: 86.0.4240.75 (Official Build) dev (64-bit)
-- | --
Revision | c69c33933bfc72a159aceb4aeca939eb0087416c-refs/branch-heads/4240@{#1149}
OS | Windows 10 OS Version 1903 (Build 18362.1082)

Verified plan 1 from brave/brave-core#6213. Confirmed able to connect to Uphold on clean profile and view balance. Confirmed able to 1 time tip. Confirmed able to perform AC and recurring tip.

Verified plan 2 from brave/brave-core#6213. Confirmed brave://rewards was displayed correctly after upgrade from 1.15.x to 1.16.x and Uphold wallet was still connected as expected. After upgrade, confirmed able to 1 time tip. Confirmed able to perform AC and recurring tip.

Resolves brave/brave-browser#2555

srirambv · 2020-10-16T12:05:56Z

Verification passed on OnePlus 6T with Android 10 running 1.16.62 x64 build

Verified scenario 1 from Adds encryption on important data brave-core#6213
Verified able to connect to uphold on a clean profile and able to perform tips/recurring tips/ac
Verified scenario 2 from Adds encryption on important data brave-core#6213
Verified able balance shows as expected after upgrading from 1.15.66
Verified able to still send one-time tip/recurring tip/ac after upgrading

Verification passed on Samsung Tab A with Android 10 running 1.16.62 x64 build

Verified scenario 1 from Adds encryption on important data brave-core#6213
Verified able to connect to uphold on a clean profile and able to perform tips/recurring tips/ac
Verified scenario 2 from Adds encryption on important data brave-core#6213
Verified able balance shows as expected after upgrading from 1.15.66
Verified able to still send one-time tip/recurring tip/ac after upgrading

bsclifton added security feature/rewards labels Dec 13, 2018

bsclifton mentioned this issue Dec 13, 2018

private wallet data in ledger state files should be encrypted using password manager brave/browser-laptop#10705

Closed

diracdeltas added the priority/P3 The next thing for us to work on. It'll ride the trains. label Dec 13, 2018

NejcZdovc added this to the 1.x Backlog milestone Dec 13, 2018

bridiver self-assigned this Dec 13, 2018

rebron modified the milestone: 1.x Backlog Feb 7, 2019

diracdeltas mentioned this issue May 24, 2019

Figure out where to pass in derived from sync seed data for seed key generation in metamask #4589

Closed

NejcZdovc unassigned bridiver Feb 27, 2020

NejcZdovc added suggestion and removed priority/P3 The next thing for us to work on. It'll ride the trains. labels Apr 10, 2020

diracdeltas added the priority/P2 A bad problem. We might uplift this to the next planned release. label Jul 24, 2020

NejcZdovc added the release-notes/include label Jul 28, 2020

srirambv changed the title ~~Private wallet data preferences should be encrypted~~ [Desktop] Private wallet data preferences should be encrypted Sep 10, 2020

NejcZdovc added a commit to brave/brave-core that referenced this issue Sep 21, 2020

Adds encryption on important data

7084633

Resolves brave/brave-browser#2555

NejcZdovc added this to the 1.16.x - Nightly milestone Sep 22, 2020

NejcZdovc added a commit to brave/brave-core that referenced this issue Sep 22, 2020

Adds encryption on important data

5d76ed4

Resolves brave/brave-browser#2555

NejcZdovc added a commit to brave/brave-core that referenced this issue Sep 23, 2020

Adds encryption on important data

4b8d8d3

Resolves brave/brave-browser#2555

NejcZdovc closed this as completed in brave/brave-core#6213 Sep 24, 2020

NejcZdovc added OS/Android Fixes related to Android browser functionality OS/Desktop labels Sep 25, 2020

LaurenWags added the QA Pass-macOS label Oct 2, 2020

btlechowski added the QA Pass-Linux label Oct 6, 2020

deeppandya pushed a commit to brave/brave-core that referenced this issue Oct 7, 2020

Adds encryption on important data

6b14707

Resolves brave/brave-browser#2555

GeetaSarvadnya added the QA Pass-Win64 label Oct 12, 2020

srirambv added QA Pass - Android ARM QA Pass - Android Tab labels Oct 16, 2020

LaurenWags mentioned this issue Oct 26, 2020

Release Notes for 1.16.x Release #1 (Release Channel) #12317

Closed

rebron changed the title ~~[Desktop] Private wallet data preferences should be encrypted~~ Private wallet data preferences should be encrypted Oct 26, 2020

kjozwiak mentioned this issue Oct 26, 2020

Release Notes for 1.16.x Release #1 (Release Channel) #12338

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Private wallet data preferences should be encrypted #2555

Private wallet data preferences should be encrypted #2555

bsclifton commented Dec 13, 2018

bridiver commented Dec 13, 2018 •

edited

Loading

diracdeltas commented Dec 13, 2018

bridiver commented Dec 13, 2018 •

edited

Loading

diracdeltas commented Dec 13, 2018 •

edited

Loading

bridiver commented Dec 13, 2018 •

edited

Loading

diracdeltas commented Dec 18, 2018

diracdeltas commented Jul 24, 2020 •

edited

Loading

mrose17 commented Sep 9, 2020

LaurenWags commented Oct 2, 2020 •

edited by GeetaSarvadnya

Loading

srirambv commented Oct 16, 2020

Private wallet data preferences should be encrypted #2555

Private wallet data preferences should be encrypted #2555

Comments

bsclifton commented Dec 13, 2018

Description

bridiver commented Dec 13, 2018 • edited Loading

diracdeltas commented Dec 13, 2018

bridiver commented Dec 13, 2018 • edited Loading

diracdeltas commented Dec 13, 2018 • edited Loading

bridiver commented Dec 13, 2018 • edited Loading

diracdeltas commented Dec 18, 2018

diracdeltas commented Jul 24, 2020 • edited Loading

mrose17 commented Sep 9, 2020

LaurenWags commented Oct 2, 2020 • edited by GeetaSarvadnya Loading

srirambv commented Oct 16, 2020

bridiver commented Dec 13, 2018 •

edited

Loading

bridiver commented Dec 13, 2018 •

edited

Loading

diracdeltas commented Dec 13, 2018 •

edited

Loading

bridiver commented Dec 13, 2018 •

edited

Loading

diracdeltas commented Jul 24, 2020 •

edited

Loading

LaurenWags commented Oct 2, 2020 •

edited by GeetaSarvadnya

Loading