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

Add Bisq IP 2: Payment account age based trade amount limits #2

Merged

Conversation

ManfredKarrer
Copy link
Contributor

No description provided.

@ManfredKarrer ManfredKarrer self-assigned this Sep 19, 2017
Copy link

@mrosseel mrosseel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some remarks, but general concept now OK I think


== Abstract

This proposal describes a verification model for the age of the first trade with a certain payment account as protection against a fraud scheme in which a criminal has obtained illegal access of a foreign bank account and tries to buy Bitcoin with the stolen funds.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bank accounts don't need to be foreign to be abused, I would drop 'foreign'

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was meaning anyone else's but is obvious anyway, will remove "foreign".


With our *trade amount limit* we have already a partial protection against that fraud scheme but we would like to increase the security by having additionally a verification scheme for the *age of the bank account*.

To protect users privacy we use a hashing scheme and only the other trading peer who will receive anyway the payment details during the trade process is able to verify that the provided hash in the offer matches the real account data.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and only the other trading peer - who will receive the payment details anyway - is able to verify ...


== Overview

The basic idea for that scheme was already discussed on the link:https://forum.bisq.io/t/new-requirement-for-payment-accounts-with-chargeback-risk/2376/65[Bisq Forum [1\]].

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for this scheme

When a user makes his *first trade* with a certain Fiat payment account (e.g. SEPA, Zelle,...) he publishes a new object (we call it AgeWitness) to the P2P network.

_Side note:
The reason why we separate that into a new data structure and don't use the already existing trade statistics data structure is because in future, trade statistic data might get pruned once the historical data gets too large. We don't want to be exposed to the complexity introduced in future pruning solutions nor do we want to add constraints to that pruning. We can assume that the AgeWitness data will never become that large that pruning will become necessary._

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there will be 1 AgeWitness object per user of Bisq. If the ambition is to gain localbitcoins/coinbase userbases, will this scale to such user bases? File is shipped with bisq so maybe not an issue, what's the size of 1M AgeWitness objects, etc. After you thought about it for 1 min just write something on what we could do if it doesn't scale anymore :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There might be multiple per user if he has several fiat accounts. The data only contains a few fields. Have checked the size it's 529 raw bytes (PB size might be smaller) that would be about 2000 entries per MB. If we ship 50 MB it should not be a big issue, so that would support 100 000 entries. We could optimize by using only the hash of the pubKey, the pubKey has 443 bytes so that is the big chunk, which a hash that would be only 32 bytes, so in total: 118 bytes, thus giving us 500 000 entries with 50 MB data. A alternative storage solution could be considered as well once that data gets too big to get shipped with the binary. E.g. loading old signed snapshots from a provider node...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will check i the hash of the pubKey is enough and use that if ok. Otherwise we might reach earlier the limits.


=== AgeWitness data object

The AgeWitness object contains a hash, a signature, the pubKey and the tradeDate. The hash gets created with Sha256. Input is the ageWitnessInputData concatenated with a 256 bytes salt. The ageWitnessInputData is the smallest set of uniquely identifying payment account data (e.g. concatenated IBAN and BIC). We don't use the complete payment account data because we don't want to break an existing ageWitness by minor changes like changing the holder name (e.g. adding middle name).

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

256 bit salt

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

upps...


The AgeWitness object contains a hash, a signature, the pubKey and the tradeDate. The hash gets created with Sha256. Input is the ageWitnessInputData concatenated with a 256 bytes salt. The ageWitnessInputData is the smallest set of uniquely identifying payment account data (e.g. concatenated IBAN and BIC). We don't use the complete payment account data because we don't want to break an existing ageWitness by minor changes like changing the holder name (e.g. adding middle name).

We add also a signature of the hash and the public key for assuring that the AgeWitness data cannot be used by anyone else (see: <<hijacking>>). The application wide 1024 bit DSA signing key is used. Signature algorith is "SHA256withDSA".

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

algorithm

// class PaymentAccountAgeWitnessService
public PaymentAccountAgeWitness getPaymentAccountWitness(PaymentAccount paymentAccount, Trade trade) throws CryptoException {
byte[] ageWitnessInputData = paymentAccount.getAgeWitnessInputData();
byte[] salt = paymentAccount.getSalt();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getSalt(32) or don't define the getSalt with a parameter

Copy link
Contributor Author

@ManfredKarrer ManfredKarrer Sep 23, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its created in the init method of the PaymentAccount with that param. As it is only used once in a Payment account and persisted there we store it there.


The salt value will be locally persisted with the payment account object. The trader will *only publish the AgeWitness object at the first trade*!

The AgeWitness data will be distributed in the P2P network and stored locally at each user. At each new release we will ship the actual data set as resource file (e.g. `EntryMap_BTC_MANINET`) to the application to avoid that new users need to download the complete history. This follows the same model as it is used for the trade statistic data.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTC_MAINNET


=== AgeWitness propagation

The salt value will be locally persisted with the payment account object. The trader will *only publish the AgeWitness object at the first trade*!

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

another random thought - in this design there is only one salt per user.
Wouldn't it be possible to derive the salt from his seed words? Suppose he reinstalls bisq and loses the salt, we can recreate it by taking a hash of his private key + account data? Just a thought and one less thing to store.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. But I think we should even make it accessible for backup/export, so if the user starts over again with a new app /wallet he can keep his ageWitness. Changing the wallet increases privacy (bloom filter) so binding it to the seed words might be not so good. But exporting the payment account is already supported and there we can add the salt. But it need to be set-able as well if the user re-creates an account with same iban/bic he wants to use the same salt. So that need to be readable and set-able at account creation.
Will add that to the doc.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added:

=== Salt management

If the user changes his payment account or start over with a new application we need to support that he can re-use the salt he used with a certain bank account. One option would be to add an extra field in the payment account setup screen where the user can add a past salt (otherwise the app generates a random salt).


=== Self trade

Any user could make a self trade with using a second application. This does not cause any risk because he proves that he is in possession of the payment account data (IBAN, BIC) and that is all we want to proof. If the trade was done with another user or not is not relevant here.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't understand this, suppose I do a self trade with myself and never do an actual fiat payment but just click the 'payment received' button. No actual payment has happened, the scammer can wait 2 months and transact the full amount. The only protection we have in such a case is the transaction limit + the fact that the scammer has to wait 2 months before doing something.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The scheme assumes that the scammer does not want to wait 2 months when he has a stolen account. He does not know the IBAN/BIC before so he can only make a self-trade once he has the account. But to wait 2 months is a risk that it gets discovered in the meantime. Sure a real trade (bank transfer) gives even stronger protection.
Will add a comment to make that more clear.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New text:

=== Self trade

Any user could make a self trade with using a second application. This does not cause any risk because he proves that he is in possession of the payment account data (IBAN, BIC) and that is all we want to proof. If the trade was done with another user or not is not relevant here. Our basic assumption is that a fraudster does not want to wait a few months until he can use the stolen account for cashing out as he risks that the fraud get detected in the meantime.

@ManfredKarrer
Copy link
Contributor Author

@mrosseel @cbeams

May i ask you for feedback about the below numbers and transition plan?

Once released that cannot be changed (without much headache) so it is very important to get that right.

I opted in to not implement it in a more flexible way (putting those reduction factors and dates in the offer) because it would overload the offer and it is a one time temporary event. I don't expect that after it is applied fully we need to change the basic factors and duration.

Here is the relevant section from the doc:

To fade in that feature we use a date based approach:

  • Before December, 15, 2017 (about 1.5 months after release) we don’t apply the lower limit based on the account age.
  • After that date and before January, 15, 2018 we only apply a factor of 0.75 to those which are less then 30 days old. Accounts which are 30-60 days old are not affected (no reduction).
  • After that date and before February, 15 2018 we apply a factor of 0.75 to the default limit for accounts which are 30-60 days old and 0.5 to those which are less then 30 days old.
  • After February, 15, 2018 we apply the target factor of 0.5 to the default limit for accounts which are 30-60 days old and 0.25 to those which are less then 30 days old.

I assume we release early November, at least before 15th of November (otherwise I need to adjust the dates).
So it is a fade in over about 3.5 months until we apply the full rules giving existing users the chance to not get affected by the limits.
E.g. if a user updates and opens Bisq after the release, the witness data for all his accounts will get published with that date - lest's assume it's the 10th of November. The first phase we apply on 15th of December so by then the user has > 30 days account age leading to no reduction of the trade limit. Only users who have updated later and their account age is < 30 days will get a 25% reduction (factor 0.75). At 15th of January our user has reached > 60 days so no limit reduction applies again. For users with 30-60 days we apply a 25% reduction, for users with < 30 days 50%. After 15th of February the target limits are applied and offers created without age witness will get rejected (> 3.5 months old offers never taken so far are likely not taken ever).

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 this pull request may close these issues.

2 participants