pazza83 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

I guess there are more important and easier to achieve improvements

:+1:

pazza83 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

Adjustable scale if there's a broad range of acceptable times by use case.

But choice as well. If I want it a week, or all acceptable trades after they are done, if I get a notification/warning I won't be able to seek escalation, accept that, then so be it.

From: pazza @.> Sent: Thursday, April 22, 2021 5:58:42 AM To: bisq-network/bisq @.> Cc: Conza @.>; Comment @.> Subject: Re: [bisq-network/bisq] Encrypt or remove saved trader chats and trade data on local Bisq instances (#5396)

Thanks @chimp1984https://github.com/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.

— You are receiving this because you commented. Reply to this email directly, view it on GitHubhttps://github.com/bisq-network/bisq/issues/5396#issuecomment-824319868, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AMMLWYWUBCWJMV5J7PAMFT3TJ4U7FANCNFSM42NNRC6A.

stale[bot] commented 3 years ago

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.

ghost commented 3 years ago

Please revert, stale bot. Still relevant.

pazza83 commented 2 years ago

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

Conza88 commented 2 years ago

Must rectify

From: pazza @.> Sent: Friday, January 28, 2022 6:34:14 AM To: bisq-network/bisq @.> Cc: Conza @.>; Comment @.> Subject: Re: [bisq-network/bisq] Encrypt or remove saved trader chats and trade data on local Bisq instances (#5396)

Relevant incident in Bisq Community Forumhttps://bisq.community/t/had-recently-a-raid-and-the-police-got-infos-from-many-bisq-traders/11353/5 for why this is important.

— Reply to this email directly, view it on GitHubhttps://github.com/bisq-network/bisq/issues/5396#issuecomment-1023570796, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AMMLWYXJZY52U76URQYE5ELUYGM3NANCNFSM42NNRC6A. You are receiving this because you commented.Message ID: @.***>

pazza83 commented 2 years ago

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

bisq-network / bisq

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

Description

Version

Steps to reproduce

Expected behaviour

Actual behaviour

Screenshots

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.