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

When wallet password is enabled private keys are still accessible and BSQ / BTC funds can be sent without the need to enter password #5276

Closed
pazza83 opened this issue Mar 6, 2021 · 12 comments

Comments

@pazza83
Copy link

pazza83 commented Mar 6, 2021

Description

In Bisq when you enable a wallet password you can still access your private keys and send funds without needing your password.

This is not good for security.

Users might comfort knowing if they leave Bisq running while away from computer a password will still be required to send funds.

Version

v1.5.9

Steps to reproduce

  • Press 'Ctrl + j' or 'alt + j' or 'cmd + j' to open wallet details window > check to include private keys > copy to clipboard
  • Press 'Ctrl + g' or 'alt + g' or 'cmd + g' to open manual payout tool
  • Press 'Ctrl + e' or 'alt + e' or 'cmd + e' to open BTC emergency wallet tool for both BTC and BSQ - did not want to try it but assume no password needed to send funds?

Expected behaviour

When user has wallet password enabled password prompt is shown when doing the above 3 actions

Actual behaviour

When user has wallet password enabled no password prompt is shown when doing the above 3 actions. Funds can be send without password.

@pazza83 pazza83 changed the title When wallet password is enabled private keys are still accessible and BSQ / BTC funds can be send without the need to enter password When wallet password is enabled private keys are still accessible and BSQ / BTC funds can be sent without the need to enter password Mar 6, 2021
@pazza83
Copy link
Author

pazza83 commented Apr 2, 2021

Hi are there any @bisq-network/bisq-devs able to take this on? Might be good to implement with #5152

@wallclockbuilder
Copy link
Contributor

I'll add this to my stack for the next cycle. @ripcurlx I'll take the assign

@sqrrm
Copy link
Member

sqrrm commented Apr 3, 2021

This is not an easy issue to solve and as I mentioned in #5152 (comment) it has been discussed how to handle the wallet decryption in memory. Before we start implementing anything we need a good model. To take into account is that we might want to allow a remote app handling specific keys for a wallet to execute trades, but perhaps it shouldn't have access to the full wallet.

@pazza83
Copy link
Author

pazza83 commented Apr 3, 2021

Hi @sqrrm has this changed recently?

The wiki refers to requiring your password to access the private keys: https://bisq.wiki/Manual_payout#Get_private_keys

Get private keys
To access your Bisq wallet private key

  • Press Ctrl + j, alt +j or cmd + j.
  • Check Include private keys and click COPY TO CLIPBOARD.
  • In the text editor, search for the public key value. You are looking for an entry that reads DeterministicKey{pub HEX= where is the public key.
  • When you've found this entry, select and copy the priv WIF= value that immediately follows the pub HEX value. This value is the private key.
  • Copy, paste and save that information in a bisqWallet.txt file.

Note: If your wallet uses a password, you will need to remove the password to view the private keys in wallet data.

Where is the discussion taking place about improving wallet security?

What makes it not an easy issue to solve. Can't a password prompt just be added when the user presses Press Ctrl + j, alt +j or cmd + j?

@sqrrm
Copy link
Member

sqrrm commented Apr 5, 2021

I don't think there has been any public discussions on this. There are different threat models at work here. One is the simple case where someone with access to an unlocked UI can send funds, that can easily be managed by requiring a password before funds are sent. This is the main case described in this issue.

The other is the handling of keys in memory and what keys need to be available to handle trades. We could improve here but it's tricky since some keys need to be available for open offers and trades while others would not be needed. This is what I was thinking about but you're right that handling the first case is important and quite easy to do.

@pazza83
Copy link
Author

pazza83 commented Apr 5, 2021

Ok thanks for the information. I think protecting the private keys though a password prompt when pressing Ctrl + j, alt +j or cmd + j is a good first step.

@pazza83
Copy link
Author

pazza83 commented May 16, 2021

Hi @ripcurlx can this be approved for @wallclockbuilder to work on?

@ripcurlx
Copy link
Contributor

Hi @ripcurlx can this be approved for @wallclockbuilder to work on?

He is already assigned to it.

@stale
Copy link

stale bot commented Aug 21, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the was:dropped label Aug 21, 2021
@pazza83
Copy link
Author

pazza83 commented Aug 21, 2021

This still needs to be implemented

@stale
Copy link

stale bot commented Apr 16, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the was:dropped label Apr 16, 2022
@stale
Copy link

stale bot commented Apr 28, 2022

This issue has been automatically closed because of inactivity. Feel free to reopen it if you think it is still relevant.

@stale stale bot closed this as completed Apr 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants