Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow users to encrypt wallet / wallet encryption/decryption user flow #762

Closed
tzarebczan opened this issue Nov 20, 2017 · 13 comments

Comments

@tzarebczan
Copy link
Member

commented Nov 20, 2017

The Issue

Once @akinwale is done with the protocol side of this which allows wallet encryption, we need to have a discussion about what this should look/work like from a user experience standpoint.

Some quick points:

  1. This needs to be as simple as possible - we'll have many non-crypto / non-tech users
  2. We need to be explicit about not being able to recover the wallet if the encryption password is lost
  3. Do we allow some type of pin which allows wallet to be unlocked via the auth token?
  4. When is the wallet unlocked?
  5. Can it be re-locked?
  6. Possible to integrate some type of 2FA?

I'd like to hear @kauffj 's thoughts on what this user flow should look like too :)

@kauffj

This comment has been minimized.

Copy link
Member

commented Nov 20, 2017

IMO this is something we ought to avoid introducing on first-run unless we have to. If it must be introduced on first-run, do it in a way that a user can put it off or do it later.

When the user is first running the software, they don't have any credits, or have very few. Being told you must write something down and if you forget it you lose your credits forever is a very intimidating introduction to something new.

Instead, I'd propose we introduce this either once the wallet crosses a certain threshold, or only if the user specifically indicates a desire for this type of security.

@tzarebczan

This comment has been minimized.

Copy link
Member Author

commented Dec 18, 2017

@akinwale feel free to use this issue or close/re-open a new one.

@akinwale

This comment has been minimized.

Copy link
Member

commented Dec 19, 2017

Wallet Encryption Commands

There are three new daemon CLI and JSON RPC commands related to wallet encryption.

  • wallet_encrypt
  • wallet_unlock
  • wallet_decrypt

wallet_encrypt

Encrypts the wallet private key, thereby requiring having to unlock the wallet before transactions can be sent from the wallet. The master_private_keys field in the default_wallet file will be updated with the encrypted private key string. After a wallet has been encrypted, you will need to use the wallet_unlock command to allow the wallet to be used. An encrypted wallet also needs to be unlocked upon daemon start up. There is a log entry during the daemon start up process which reads "Waiting for wallet password". Using wallet_unlock with the correct password at this point will allow the daemon to launch successfully.

CLI

lbrynet-cli wallet_encrypt <new_password>

JSON RPC

curl -X POST -d '{"method":"wallet_encrypt", "params":{"new_password":"<new_password>"}}' http://localhost:5279

wallet_unlock

Unlocks the wallet if the specified password matches the password that was used to encrypt the wallet. As stated above, the wallet needs to be unlocked to send transactions, or in order to allow the to daemon start up completely.

CLI

lbrynet-cli wallet_unlock <password>

JSON RPC

curl -X POST -d '{"method":"wallet_unlock", "params":{"password":"<password>"}}' http://localhost:5279

wallet_decrypt

Removes encryption from the wallet private key. The master_private_keys field in the default_wallet file will be updated with plain private key string. An encrypted wallet can only be decrypted after it has been unlocked.

CLI

lbrynet-cli wallet_decrypt

JSON RPC

curl -X POST -d '{"method":"wallet_decrypt", "params":{}}' http://localhost:5279

How to determine if a wallet is encrypted?

Running the status command will return a number of fields, including wallet_is_encrypted which is boolean, with a value of true if the wallet is encrypted, or false otherwise.

CLI

lbrynet-cli status

JSON RPC

curl -X POST -d '{"method":"status", "params":{}}' http://localhost:5279

@seanyesmunt

This comment has been minimized.

Copy link
Member

commented Dec 19, 2017

Looks good to me. I'm guessing we will just have a check on app startup and, if its encrypted, show a password field to unlock before loading anything else?

@akinwale

This comment has been minimized.

Copy link
Member

commented Dec 19, 2017

Yes, looks like it. You can run the status command on app startup (if the daemon is being started by the app) to check if the wallet is encrypted. If the wallet is waiting for a password, you also get

"startup_status": {
    "code": "loading_wallet",
    "message": "Catching up with the blockchain"
}

in the response. Although we probably need to change the code and the message at this point to make it more meaningful.

@LavRadis

This comment has been minimized.

Copy link
Contributor

commented Jan 10, 2018

I've been testing this and at one point the console returned "true" when I did the unlock command with a false password. Not sure if I followed the correct protocol. I've attached a .txt of that session. The return is on line 88.
lbry encrypt test 2.txt

@tzarebczan

This comment has been minimized.

Copy link
Member Author

commented Jan 10, 2018

@akinwale per our discussion on Slack, we should have a wallet_lock or similar which will prevent transactions from being signed. The use case would be users running the daemon for hosting purposes, but they aren't necessarily transacting.

This would require some overhead on the app side - would need a way to allow lock and then unlock again - prompt user to do so when a transaction needs to occur.

@tzarebczan

This comment has been minimized.

Copy link
Member Author

commented Jan 15, 2018

@akinwale / @jackrobison

We should also be encrypting channel certificate private keys during this process as well, right? I ran through a quick test and it doesn't look like we are.

@tzarebczan

This comment has been minimized.

Copy link
Member Author

commented Jan 30, 2018

@akinwale did you have a chance to review the comments from @LavRadis per our discussion?

Also, can you comment on using the keyring to store the encryption keys?

@tzarebczan

This comment has been minimized.

Copy link
Member Author

commented Feb 26, 2018

@hackrush01 think you connected this to the wrong PR (or should I say...you connected your PR to the wrong issue :p )

@skhameneh

This comment has been minimized.

Copy link
Contributor

commented May 29, 2018

Working on this

@tzarebczan tzarebczan changed the title Allow users to encrypt walllet / wallet encryption/decryption user flow Allow users to encrypt wallet / wallet encryption/decryption user flow May 29, 2018

@tzarebczan

This comment has been minimized.

@alyssaoc alyssaoc added this to the July 23rd (desktop) milestone Jul 9, 2018

@alyssaoc alyssaoc closed this Jul 16, 2018

@alyssaoc

This comment has been minimized.

Copy link
Contributor

commented Jul 16, 2018

a duplicate of #1097, this issue closed in of another issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.