Xud: Don't allow a peer to invalidate a peerOrder which he doesn't own

[moshababo]

And penalize him if he tries to 😎

[kilrau]

I think invalidating should require a signature and thus is easily prevented. What am I missing?

[moshababo]

I can't recall we settled on any signing protocol yet. So the current situation is that a peer can invalidate other peers orders. The simplest solution would be to verify that the invalidation came form the same socket that the order came from (since we don't persist them). Or instead just use hostId, which is already being added to the order, and controlled by the local xud.

If you're referring to signing the p2p packet, on the node pubKey, then it can still attempt to invalidate a different peer order. Are you talking about signing on the order id? or something else?

[michael1011]

The peer should also be penalized if it tries to invalidate an order that was never broadcasted.

[kilrau]

I still don't get this. As I spoke with Daniel we already have a sign & verify function prepared and will use it in various places, e.g. signing the handshake nodePubKey p2p packet (https://github.com/ExchangeUnion/xud/issues/341).

So I guess my question is:
Why don't we make anyone wanting to invalidate an order sign a this invalidation packet and anyone getting this invalidation packet verify that the order was signed by the order's nodePubKeys private key?

[sangaman]

Even without signing, we should be able to tell because we'll know the peer's pubkey and can see that it differs from the pubkey he is trying to invalidate. We should probably discuss some more about which packets actually need to be signed, it's possible that it's enough to only sign the handshake. If we encrypt p2p traffic, that will take care of this also.

[kilrau]

You are right, since we already verified the pubkey via signature on handshake, we can just make sure that the invalidation packet comes from the same socket. Since we have a direct socket connection with everyone, this works 👍

Encrypting will definitely solve that too, but is optional (whitelist mode).

[sangaman]

Encrypting traffic might be worthwhile even without whitelist mode, but it's a separate discussion. It would prevent MITM attacks and snooping.

[kilrau]

agree, I already put that discussion to a later stage: https://github.com/ExchangeUnion/xud/issues/158

[kilrau]

So if all (@moshababo @offerm @michael1011 @itsbalamurali ) agree, this task now is about:
check if an order invalidation packet is coming from the same socket connection than the order.

nodePubKey verify on handshake (https://github.com/ExchangeUnion/xud/issues/341) is a prerequisite.

[michael1011]

Yeah that sounds reasonable.

But it doesn't have to be exact same socket connection but one of the advertised addresses. @kilrau

[moshababo]

@michael1011 Since we're removing all orders on disconnection, it practically means that it's from the same socket.

315 is a prerequisite, and as Daniel said, pubkey sign/verify isn't blocking this.

[kilrau]

@moshababo is right and another (small) argument for the same socket only, is that only for this one we verified the nodePubKey on handshake. We potentially didn't connect to the other advertised addresses and even though they are advertised by the verified node they could be erroneous because we ourselves didn't directly verify them. Don't trust, verify ;)

[sangaman]

To summarize again, I think this can be considered solved simply by checking that the peerPubKey on the order matches the pub key of the peer we received the invalidation packet from. Encrypting traffic and other measures should be considered separate issues.

[erkarl]

Is this still up for grabs? If so, I'd like to work on it.

[sangaman]

Yup, this would be a good one to work on.