Avoid using a hash in cipher selection process (and make fairer?)

[dominictarr]

In the secure channel implementation, I notice that a hash is used within the cipher suit selection process:

https://github.com/ipfs/go-ipfs/blob/12549ca8f9c24b03ed57830b67fa94e23ab56bb0/p2p/crypto/secio/protocol.go#L138-L140

This is used to decide the algorithms for hashing, key exchange, and encryption. That seems odd to me. that if the hash is worth negotiating then it shouldn't be hardcoded into the protocol at any point. Also, it uses a multihash (from the util lib) and then does a bytewise comparison (and remember that multihash uses the first byte to represent the algorithm)...

Also, fairness: it seems like this code is about ensuring that the cipher suite is selected randomly (fairly) but either side can force selection of the cipher suite if they know who they are expecting to talk to, by mining for a nonce such that hash(nonce || remote_pubkey) is a very high value. (I'd imagine that usually the peer who initialized the connection knows who they are intending to connect to*)

* otherwise you are vulnerable to man-in-the-middle attacks.

ciphersuite negioation protocols bother me a little (have already expressed my opinion on that in this discussion ) but if you are gonna have one you might as well make it fair. Instead of hash(nonce || remote_pubkey) you could use hash(local_nonce || remote_nonce). This would make the order unpredictable before you have received the remote nonce (oh, though actually... you could just wait to receive the remote's nonce before sending yours)... you can't include the hash of the secret, because that depends on the cipher selection ;)

I'm thinking something like choosing the nonce which is closest to xor(local_nonce, remote_nonce) (maybe by edit distance instead of lexiographic order?)

An even better idea might be to let the peer who didn't initialize the call choose first - you are exposing your self to more risk by allowing incoming calls. (prank calls are totally a thing, but prank answering isn't really... unless the caller was a telemarketer!) However, then your protocol is no longer symmetrical.

Overall, I think the most important thing is that security properties are clear. If the caller doesn't know who they are trying to call, then this is open to MITM -- so the protocol isn't really symmetrical -- so let the answerer choose...

But at least, if you removed the hash, you wouldn't be restricted from upgrading it.

[whyrusleeping]

We might be working on a couple improvements for secio in the near future. Lets consider this when doing so.

cc @Kubuxu

[Jorropo]

We do not run secio anymore.