Listen on insecure transport has no peerID

[xux1217]

There are three node A, B, C and use insecure transport.

A is a listener, now B connect to A, the conn in A can't get the peerID of B, because the conn.RemotePeer() is set by handshake of secure transport.

And then C connect to A, A would recognize the conn is from B(actually A get the peerID of B and C is always nil), and set error protocols.

[xux1217]

I think maybe we should use ID Service to set the peerID(if not set before) of the conn finally.

[Stebalien]

I'd rather not add this to the identify protocol as the insecure protocol exists primarily for testing. However, we can probably change the insecure transport (or introduce a new version) to send the peer ID.

[xux1217]

You are right, change the insecure transport is the best. And I think when I use the protect network in local area, use the insecure transport would be faster.

[Stebalien]

And I think when I use the protect network in local area, use the insecure transport would be faster.

Slightly but not by much. I don't recommend it.

[xux1217]

OK, I understand.

[Stebalien]

FYI, if you want to take a stab at fixing this, you'd need to modify https://github.com/libp2p/go-conn-security/blob/master/insecure/insecure.go.

[mhchia]

The public key is sent in Identify protocol but only used to verify if it matches the peerID. Does it make sense to change the Identify, to save the corresponding peerID of the public key if peerID is not present(i.e. empty) in the first place? This way we don't need to modify the process to establish an insecure connection, which breaks the interoperability.

[raulk]

IIUC, the issue is that the receiving swarm (network implementation) will have no peer ID to store the incoming connection under. The dialing side has no problem because the libp2p APIs oblige you to have a peer ID to dial.

So indeed, it seems like the insecure channel should simply exchange peer IDs with no cryptographic validation.

[mhchia]

@raulk Thanks for the reply!

IIUC, the issue is that the receiving swarm (network implementation) will have no peer ID to store the incoming connection under. The dialing side has no problem because the libp2p APIs oblige you to have a peer ID to dial.

Yes, that is what I understand currently.

So indeed, it seems like the insecure channel should simply exchange peer IDs with no cryptographic validation.

Do you mean we just modify the process establishing the insecure channel? For instance, after negotiating the security to insecure, the dialing node passes its peer ID to the dialed node and the dialed node just receive and set the dialing nodes peer ID.

Just want to explain again what I currently understand and what I want to do, please correct me if I'm wrong:

After a connection is established(security + muxed), the dialed node opens a stream to the dialing node's "Identify protocol". The dialing node sends its public key and addresses ...etc to the dialed node. Currently, the dialed node receives and "consumes" these data, but for the public key, the dialed node only verifies if this public key corresponds to the dialing nodes' peer ID. The dialed node does not store the peer ID derived from the public key if at that moment it doesn't know the peer ID of the dialing node.

go-libp2p/p2p/protocol/identify/id.go

Lines 372 to 424 in e69d171

	func (ids *IDService) consumeReceivedPubKey(c network.Conn, kb []byte) {
	lp := c.LocalPeer()
	rp := c.RemotePeer()
	if kb == nil {
	log.Debugf("%s did not receive public key for remote peer: %s", lp, rp)
	return
	}
	newKey, err := ic.UnmarshalPublicKey(kb)
	if err != nil {
	log.Warningf("%s cannot unmarshal key from remote peer: %s, %s", lp, rp, err)
	return
	}
	// verify key matches peer.ID
	np, err := peer.IDFromPublicKey(newKey)
	if err != nil {
	log.Debugf("%s cannot get peer.ID from key of remote peer: %s, %s", lp, rp, err)
	return
	}
	if np != rp {
	// if the newKey's peer.ID does not match known peer.ID...
	if rp == "" && np != "" {
	// if local peerid is empty, then use the new, sent key.
	err := ids.Host.Peerstore().AddPubKey(rp, newKey)
	if err != nil {
	log.Debugf("%s could not add key for %s to peerstore: %s", lp, rp, err)
	}
	} else {
	// we have a local peer.ID and it does not match the sent key... error.
	log.Errorf("%s received key for remote peer %s mismatch: %s", lp, rp, np)
	}
	return
	}
	currKey := ids.Host.Peerstore().PubKey(rp)
	if currKey == nil {
	// no key? no auth transport. set this one.
	err := ids.Host.Peerstore().AddPubKey(rp, newKey)
	if err != nil {
	log.Debugf("%s could not add key for %s to peerstore: %s", lp, rp, err)
	}
	return
	}
	// ok, we have a local key, we should verify they match.
	if currKey.Equals(newKey) {
	return // ok great. we're done.
	}

I am thinking that the dialed node can just store the dialing nodes' peer ID which is derived from the public key to the Connection, and therefore the exchange is complete(dialed node knows the peer ID of the dialing node, and of course dialing node knows the one of the dialed node). This way it seems we don't need to modify the existing insecure connection establishing process.

[Stebalien]

Do you mean we just modify the process establishing the insecure channel? For instance, after negotiating the security to insecure, the dialing node passes its peer ID to the dialed node and the dialed node just receive and set the dialing nodes peer ID.

Yes. We can just make that a part of the "protocol". Really, most of libp2p breaks without a valid peer ID so we should try to make sure we have one.

[marten-seemann]

This should have been solved by the introduction of plaintext v2.