Banned coin check feature #12967
Banned coin check feature #12967
Conversation
If the client has a banned coin then it does not finish the coin connection confirmation. It won't participate in the next phase, but checks the coins whether they are banned or not. Fixes WalletWasabi#12790
|
I used throw new CoinJoinClientException(CoinjoinError.BannedCoinCheckFinished to leave the phase. |
|
Tested with feature/unfairban (my repo). |
There was a problem hiding this comment.
I have no strong opinion about this concept because it introduces a privacy risk that I can't fully assess.
If this PR is implemented, the coordinator can selectively lie to a coin about its banning status, so it detects which inputs are not confirmed along with it. If it does it for the same input in successive rounds, it can draw common input ownership conclusions.
Alternatively we can add a change that every client adds in 1-2 coins that will leave before the end of the input registration to increase the noise. |
| if (coinBanCheckMode.IsCancellationRequested) | ||
| { | ||
| var aliceWouldBeRemovedByBackendTime = DateTime.UtcNow + roundState.CoinjoinState.Parameters.ConnectionConfirmationTimeout; | ||
| if (aliceWouldBeRemovedByBackendTime > roundState.InputRegistrationEnd - TimeSpan.FromMinutes(1) || SecureRandom.Instance.GetInt(0, 100) < 50) |
There was a problem hiding this comment.
ACK for the implementation.
About the privacy-risk: I agree there is an attack vector there. As @turbolay pointed out there are two ways: one-by-one or tricky bail out. As far as I can see one-by-one has the same issue if it is not the first coin, let me know if I missed something. If we consider it severe it is a dealbreaker for these type of solutions.
My personal opinion is: I do not see a practical risk with this. We have a lot of timeouts in rounds, the noise will cover this. The concerns beaten up by the fact of suboptimal coin selection - that is the tradeoff.
|
The practical risk is not for 1 round but for a succession of rounds. You try to register, coordinator sees the input and wants to target attack it, so it tells it that it's banned 1 minute. Then notes the inputs that drop from confirmation. New round he does the same on the same input, note again inputs that dropped focusing on overlapping ones with last round etc etc... After few rounds, coordinator will be sure of common inputs ownership. And it's a new risk because there is currently no mechanism where the coordinator can tell a specific client to don't confirm all its inputs. As I mentioned, this doesn't seem enough for me to cNACK, but it's enough to withdraw myself from reviewing the PR out of doubts and uncertainty of adding risk at this point of the development |
|
Should I set a minimum (random) ban time on the coins? |
It mitigates. The attack will become much harder with that. |
If the ban time is too short the client will hold back the coin for 30-60 minutes instead. By the time this coin came into the server again the other inputs will be already coinjoined.
|
Added a change that will hold back the banned coin for 30-60 minutes if the ban time is too short and completely disregard server's ban time if its too short. |
Accidentally set on backend, revert
If the ban time is too short the client will hold back the coin for 30-60 minutes instead. By the time this coin came into the server again the other inputs will be already coinjoined.
|
Now its on the client side... :/ |
If the client has a banned coin then it does not finish the coin connection confirmation. It won't participate in the next phase, but checks the coins whether they are banned or not.
Fixes #12790