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

Encrypt or remove saved trader chats and trade data on local Bisq instances #5396

Closed
pazza83 opened this issue Apr 5, 2021 · 18 comments · Fixed by #6001
Closed

Encrypt or remove saved trader chats and trade data on local Bisq instances #5396

pazza83 opened this issue Apr 5, 2021 · 18 comments · Fixed by #6001

Comments

@pazza83
Copy link

pazza83 commented Apr 5, 2021

Description

I have created this issue from the discussion on the Bisq community for forum Is trader chat saved?.

Currently the following information is avlaiable on users local Bisq instances

  • Unencrypted trader chats between themselves and peers
  • Unencrypted trade data (names, account numbers, trade amounts etc) between themselves and peers

I am unsure what trade information is unencrypted on mediators' or arbitrators' Bisq instances.

Having trade chats and trade data saved on local Bisq instances is a security concern for both traders and everyone they have traded with.

Having trade chats and trade data saved on mediators' or arbitrators' Bisq instances is a security concern for everyone they have mediated / arbitrated.

Traders with lots of trades, mediators and arbitrators will end up being a centralized source of unencrypted data. This puts users of Bisq at risk.

Version

v1.6.2

Steps to reproduce

open \Bisq\btc_mainnet\db\

  • ClosedTrades
  • FailedTrades
  • MaiboxMessageList

There might be more. I have not checked all the files for unencrypted data.

Expected behaviour

Chat and trade data to be encrypted.

Not sure if there should be a time limit for how long this data is kept?

Actual behaviour

Chats and trade data are stored unencrypted.

Screenshots

Taken from: https://bisq.community/t/is-trader-chat-saved/10539/14

SEPA���������:�
>XXXCENSORED_BANK COUNTRY CODE_XXX"�
XXXCENSORED_Full name_XXXeXXXCENSORED_IBAN_XXXXXXCENSORED_bic_XXX*AT*BE*BG*CH*CY*CZ*DE*DK*EE*ES*FI*FR*GB*GR*HR*HU*IE*IS*IT*LI*LT*LU*LV*MC*MT*NL*NO*PL*PT*RO*SE*SI*SKzH
salt@XXXCENSORED_XXX*XXXCENSORED_btcADRESS_XXX"�#{
  "offerPayload": {
"id": "XXXCENSORED_TRADEID_XXX",
"date": XXXCENSORED_TIMESTAMP_XXX,
"ownerNodeAddress": {
  "hostName": "XXXCENSORED_XXX.onion",
  "port": 9999
},
"direction": "BUY",
"price": 0,
"marketPriceMargin": 0.005,
"useMarketBasedPrice": true,
"amount": 700000,
"minAmount": 300000,
"baseCurrencyCode": "BTC",
"counterCurrencyCode": "CHF",
"arbitratorNodeAddresses": [],
"mediatorNodeAddresses": [
  {
    "hostName": "apbp7ubuyezav4hy.onion",
    "port": 9999
  },
  {
    "hostName": "a56olqlmmpxrn5q34itq5g5tb5d3fg7vxekpbceq7xqvfl3cieocgsyd.onion",
    "port": 9999
  },
  {
    "hostName": "sjlho4zwp3gecspf.onion",
    "port": 9999
  }
],
"paymentMethodId": "SEPA",
"makerPaymentAccountId": "XXXCENSORED_XXX",
"offerFeePaymentTxId": "XXXCENSORED_XXX",
"countryCode": "XXXCENSORED_BANKCC_XXX",
"acceptedCountryCodes": [
  "AT",
  "BE",
  "BG",
  "CH",
  "CY",
  "CZ",
  "DE",
  "DK",
  "EE",
  "ES",
  "FI",
  "FR",
  "GB",
  "GR",
  "HR",
  "HU",
  "IE",
  "IS",
  "IT",
  "LI",
  "LT",
  "LU",
  "LV",
  "MC",
  "MT",
  "NL",
  "NO",
  "PL",
  "PT",
  "RO",
  "SE",
  "SI",
  "SK"
],
"bankId": "XXXCENSORED_XXX",
"versionNr": "1.5.3",
"blockHeightAtOfferCreation": XXXCENSORED_XXX,
"txFee": XXXCENSORED_XXX,
"makerFee": 5000,
"isCurrencyForMakerFeeBtc": true,
"buyerSecurityDeposit": 600000,
"sellerSecurityDeposit": 600000,
"maxTradeLimit": 1000000,
"maxTradePeriod": XXXCENSORED_XXX,
"useAutoClose": false,
"useReOpenAfterAutoClose": false,
"lowerClosePrice": 0,
"upperClosePrice": 0,
"isPrivateOffer": false,
"extraDataMap": {
  "capabilities": "0, 1, 2, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16",
  "accountAgeWitnessHash": "XXXCENSORED_XXX"
},
"protocolVersion": 3
  },
  "tradeAmount": 300000,
  "tradePrice": XXXCENSORED_XXX,
  "takerFeeTxID": "XXXCENSORED_XXX",
  "buyerNodeAddress": {
"hostName": "XXXCENSORED_XXX.onion",
"port": 9999
  },
  "sellerNodeAddress": {
"hostName": "XXXCENSORED_XXX.onion",
"port": 9999
  },
  "mediatorNodeAddress": {
"hostName": "sjlho4zwp3gecspf.onion",
"port": 9999
  },
  "isBuyerMakerAndSellerTaker": true,
  "makerAccountId": "XXXCENSORED_XXX",
  "takerAccountId": "XXXCENSORED_XXX",
  "makerPaymentAccountPayload": {
"holderName": "XXXCENSORED_XXX",
"iban": "XXXCENSORED_XXX",
"bic": "XXXCENSORED_XXX",
"email": "",
"acceptedCountryCodes": [
  "AT",
  "BE",
  "BG",
  "CH",
  "CY",
  "CZ",
  "DE",
  "DK",
  "EE",
  "ES",
  "FI",
  "FR",
  "GB",
  "GR",
  "HR",
  "HU",
  "IE",
  "IS",
  "IT",
  "LI",
  "LT",
  "LU",
  "LV",
  "MC",
  "MT",
  "NL",
  "NO",
  "PL",
  "PT",
  "RO",
  "SE",
  "SI",
  "SK"
],
"countryCode": "XXXCENSORED_XXX",
"paymentMethodId": "SEPA",
"id": "XXXCENSORED_XXX",
"maxTradePeriod": -1
  },
  "takerPaymentAccountPayload": {
"holderName": "XXXCENSORED_XXX",
"iban": "XXXCENSORED_XXX",
"bic": "XXXCENSORED_XXX",
"email": "",
"acceptedCountryCodes": [
  "AT",
  "BE",
  "BG",
  "CH",
  "CY",
  "CZ",
  "DE",
  "DK",
  "EE",
  "ES",
  "FI",
  "FR",
  "GB",
  "GR",
  "HR",
  "HU",
  "IE",
  "IS",
  "IT",
  "LI",
  "LT",
  "LU",
  "LV",
  "MC",
  "MT",
  "NL",
  "NO",
  "PL",
  "PT",
  "RO",
  "SE",
  "SI",
  "SK"
],
"countryCode": "XXXCENSORED_XXX",
"paymentMethodId": "SEPA",
"id": "XXXCENSORED_XXX",
"maxTradePeriod": -1
  },
  "makerPayoutAddressString": "XXXCENSORED_XXX",
  "takerPayoutAddressString": "XXXCENSORED_XXX",
  "lockTime": 669272,
  "refundAgentNodeAddress": {
"hostName": "3z5jnirlccgxzoxc6zwkcgwj66bugvqplzf6z2iyd5oxifiaorhnanqd.onion",
"port": 9999
  }
}*@
@Conza88
Copy link

Conza88 commented Apr 6, 2021

Yep. As per chat, completely agree.
#5044 is relevant—in that when trade is escalated, mediators/arbitrators should be able to see the exchange. It saves time, and provides immediate context. Most already assume it's possible, and are surprised to find out it's not.

I'd want my trade info encrypted, and after period of time ideally removed from as well.

@ghost
Copy link

ghost commented Apr 7, 2021

The persistence subsystem in Bisq is tricky and I would like to get @chimp1984's opinion about this topic. Would be great if he could write a quick dev spec, sharing opinions on how it would best be tackled.

One thing that sprang to mind is that if the user changes his onion address, there will have to be coordination with the encryption mechanism so that user is not permanently locked out of his own data. Currently all database is clear & unencrypted on user's own drive except for Mailbox/ProtectedStorage.

When spec'd I would be interested in tackling an implementation possibly in collab with @BtcContributor.

@chimp1984
Copy link
Contributor

chimp1984 commented Apr 7, 2021

I think the benefits/costs have to be considered. No strong opinion here, but I guess there are more important and easier to achieve improvements.

Some considerations:

  • Encryption/decrytption causes delay and higher CPU usage at persistence. This might be critical as shutdown on slow harddrives/cpus
  • If user uses the wallet password we could use that but what to do if users has not set up a password?
  • If user changes password, all files need to be decrypted and re-encrypted. Any bug/mistake/race condition can lead to lost data
  • Backups with an older password which the user might have not saved will become inaccessible
  • If one gets access to your local data he might get also a keylogger set up, rendering pw protection useless

Not saying it should not be done, just that it is not trivial and comes with some risks and complexities.

@ghost
Copy link

ghost commented Apr 9, 2021

I guess there are more important and easier to achieve improvements

👍

@pazza83
Copy link
Author

pazza83 commented Apr 9, 2021

Would it be possible to keep the files unencrypted but only keep trade data / chats for a period of 30 days following trade moving to history or failed?

I appreciate the encryption aspect might come with risks and complexities, but is there another way of achieving user privacy and reducing the risks of a instance of Bisq with lots of data on it being compromised.

@chimp1984
Copy link
Contributor

Would it be possible to keep the files unencrypted but only keep trade data / chats for a period of 30 days following trade moving to history or failed?

I appreciate the encryption aspect might come with risks and complexities, but is there another way of achieving user privacy and reducing the risks of a instance of Bisq with lots of data on it being compromised.

Yes that might be a good compromise and even more effective as encryption does not prevent that the data might get revealed any time later. Should not be too hard to check at some interval for historical data and prune private data out of it.

@Xa5r
Copy link

Xa5r commented Apr 11, 2021

I would not encrypt my bisq, because I already encrypt my whole system.
Removing sensitive data after a period of time is much better.
Nobody needs to know my name and bank data after years.

And a bisq database with years of collected trading peers, will become more and more dangerous.
Sure, after a certain time period sensitive data should be deleted.
Maybe not 30 days (in case of long conflicts), but 100 or 365 days.

As power user with a large database with many trading peers, I already had the same thinking and I feel the danger if such a database becomes compromised.
With no responsible handling, old traders with a big database could one day "leak" this database to people or agencies which should not know it.

@pazza83
Copy link
Author

pazza83 commented Apr 17, 2021

Thanks @Xa5r for the helpful comments.

Seems like leaving the files unencrypted but only having data being available for a limited time is a good solution.

Maybe start at sensitive data being removed after 365 days from traders, mediators and arbitrators Bisq instances.

Would this be an acceptable solution?

@Conza88
Copy link

Conza88 commented Apr 20, 2021

Maybe start at sensitive data being removed after 365 days from traders, mediators and arbitrators Bisq instances.

Why so long? A year is a lot of sensitive exposed trading data.
Once a quarter? A month?

The shortest period of time acceptable?

How do we guarantee destruction/removal?

@pazza83
Copy link
Author

pazza83 commented Apr 20, 2021

I would be happy with keeping the trade data for the minimum amount of time possible.

On a couple of occasions I have had to access trade data over 1 month old due to payout transactions not confirming. Maybe @leo816 @huey735 and @refund-agent2 can let us know what would be the recommend amount of time needed to keep the trade data.

@chimp1984
Copy link
Contributor

We could use the confirmations of the trade payout tx to see if a trade is really completed. Once that has sufficient confirmations there is not much reason for a need to access the data.
But also keep in mind that users who do continuous backups (like Timemachine on OSX) will have backups anyway). So it can be sees just as an effort to reduce the data but not to avoid storage.
Maybe its good have the duration adjustable in settings as some nodes like mediators/arbitrators should keep data longer (e.g. when investigating scammers).

@pazza83
Copy link
Author

pazza83 commented Apr 21, 2021

Thanks @chimp1984, this all sounds good to me. I understand it is not full proof, but it would result in a lot less sensitive trade data being on users Bisq instances.

@Conza88
Copy link

Conza88 commented Apr 21, 2021 via email

@stale
Copy link

stale bot commented Jul 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 Jul 21, 2021
@ghost
Copy link

ghost commented Jul 21, 2021

Please revert, stale bot. Still relevant.

@stale stale bot removed the was:dropped label Jul 21, 2021
@pazza83
Copy link
Author

pazza83 commented Jan 27, 2022

Relevant incident in Bisq Community Forum for why this is important.

@Conza88
Copy link

Conza88 commented Jan 27, 2022 via email

@pazza83
Copy link
Author

pazza83 commented Jan 31, 2022

Commenting to say that removal of trader chats are still to be addressed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants