Correlate dial back responses with actual inbound dials #48
Conversation
aarshkshah1992
changed the title
aarshkshah1992
changed the title
| func (d *dialBackTracker) ClosedStream(net network.Network, s network.Stream) {} | ||
|
|
||
| func (d *dialBackTracker) Connected(net network.Network, c network.Conn) { | ||
| dialer := c.RemotePeer() |
There was a problem hiding this comment.
@Stebalien Since this & the dial back response happen on separate go-routines, we could see the dial back response before this code has had a chance to execute. However, the possibility of that seems low to me given the RTT times. Should we handle this race or just document it ?
There was a problem hiding this comment.
This'll be a problem for QUIC because the initiator will learn that the connection has finished before the receiver. That is, the initiator will receive the second message in the three-way handshake, send the final message, then start sending data.
Assuming the initiator (the AutoNAT server) acts fast enough, they can send the dial back response at pretty much the same time as the final handshake packet. At that point, it's a matter of scheduling and/or lost packets.
There was a problem hiding this comment.
But let's not put too much work into this before we discuss it a bit.
There was a problem hiding this comment.
Only design note is that the inboundDials cache could be specific to the in-flight autonat requests. It might be a bit more resilient to DoS requests if instead of recording all recent inbound dials in one cache structure, we only record the ones where we're expecting to see an inbound dial from an autonat service.
| msg := &pb.Message_DialerIdentityCertificate{} | ||
| msg.PublicKey = v.dialerPk |
There was a problem hiding this comment.
| msg := &pb.Message_DialerIdentityCertificate{} | |
| msg.PublicKey = v.dialerPk | |
| msg := &pb.Message_DialerIdentityCertificate{PublicKey: v.dialerPk} |
| n := v.readNonce(s) | ||
|
|
||
| // dial | ||
| require.NoError(v.t, v.dialer.Connect(v.ctx, peer.AddrInfo{s.Conn().RemotePeer(), []ma.Multiaddr{s.Conn().RemoteMultiaddr()}}), |
There was a problem hiding this comment.
does it need to do this in the private case?
| c := &client{ | ||
| h: h, | ||
| getAddrs: getAddrs, | ||
| inboundDials: cache.New(10*time.Minute, 10*time.Minute), |
There was a problem hiding this comment.
move the cache time to a const at the top of the file
This is hard to do. Remember, there is no way for us to know which inbound dials we are expecting because we get the dialer peerID only when we get the dialback response. One way to solve this would be to change the message flow as follows:
But, this comes with additional costs. |
…ibp2p/go-libp2p-core-0.4.0 Bump github.com/libp2p/go-libp2p-core from 0.3.1 to 0.4.0
@Stebalien @willscott
Please take a look.
For libp2p/go-libp2p#1480