Security issues

[PlasmaPower]

Creating this issue to hopefully unify my posts in discord:

• Key stealing attacks
  • R commitment isn't validated (documented in Signature contributions should not be requested/provided until R commitments have been validated. #1)
  • R value isn't randomly generated each time, so if an attacker retries a mix they can anticipate the R value and effectively bypass the commitment

    nanofusion/client/src/model/Cryptography/BlockSigner.js

    Line 173 in 9102e40

    let zValue = this.cryptoUtils.ByteArrayToHex(blakejs.blake2b(this.cryptoUtils.HexToByteArray(secret)));

  • R value can be requested before the commitments are released

    nanofusion/client/src/model/Client/JointAccountClient.js

    Line 30 in e4adbd1

    this.sessionClient.SubscribeToEvent(EventTypes.ProvideRPoint, this.onPeerProvidesRPoint.bind(this));

• DoS attacks
  • The exit path Receive a -> send back to a, in case b doesn't send, should have at the end of it receive b -> send back to b (the full path being recv a, send a, recv b, send b). This is in case a publishes the exit path even though b did send the funds https://github.com/unyieldinggrace/nanofusion#solving-the-refund-problem
• Identification attacks
  • The server knows everybody right now, and even if the immediate problem was fixed there would be other issues like linking through IPs, MITMing keys, and so forth. Ideally this would use something like a Monero transaction with a ring sig over commitments, and send it via a separate Tor route.
• Maybe others, I haven't read through even most of the code yet

[unyieldinggrace]

@PlasmaPower thanks for the code-review and feedback points. I'm taking a couple of days off from coding to give myself rest, but will certainly get on them. There are some bits that I consider as belonging in the "won't fix" pile (or at least "not my problem" pile). Masking IP addresses is a great idea, but I think it's something that realistically needs to be left to the end user, not baked into NanoFusion. I'm prepared to be persuaded otherwise, but that's my thinking for now.

I hadn't quite worked out the point of the zValue. The scheme "works" if zValues are all empty, which is why I thought it was fine to calculate deterministically (also helped me with debugging, because output was consistent each time I ran it). I see now that it's meant to be a random variable to prevent key re-use attacks. Thanks for pointing that out.

[unyieldinggrace]

The issues with R commitments not being validated should now be fixed in the mixing context. The PhaseTracker should prevent moving on to signing until all commitments are checked.

[PlasmaPower]

Thanks, I'm liking the new phase architecture a lot better :)