Correctly resolve multiple inbound connections to complete-closure #42

Merged
erinn merged 1 commits from networking into master 2021-09-21 23:18:53 +00:00
1 changed files with 22 additions and 5 deletions

View File

@ -73,27 +73,44 @@ func (s *BaseOnionService) WaitForCapabilityOrClose(cid string, name tapir.Capab
// If We have 2 connections for the same hostname...
if err != nil {
log.Debugf("found duplicate connections for %v <-> %v", s.id.Hostname(), cid)
log.Debugf("found duplicate connections for %v <-> %v %v", s.id.Hostname(), cid, err)
// By convention the lowest lexicographical hostname purges all their outbounds to the higher
// hostname
// Which should only leave a single connection remaining (as we dedupe on connect too)
// This does allow people to attempt to guarantee being the outbound connection but due to the bidirectional
// authentication this should never result in an advantage in the protocol.
inboundCount := 0
if s.id.Hostname() < cid {
// Close all output connections to connection
// Close all outbound connections to connection
s.connections.Range(func(key, value interface{}) bool {
connection := value.(tapir.Connection)
if connection.Hostname() == cid {
if !connection.IsClosed() && connection.IsOutbound() {
connection.Close()
} else if !connection.IsClosed() && !connection.IsOutbound() {
inboundCount++
}
}
return true
})
// This shouldn't happen honestly, but if it does then it can cause an infinite check here
if inboundCount > 1 {
s.connections.Range(func(key, value interface{}) bool {
connection := value.(tapir.Connection)
if connection.Hostname() == cid {
if !connection.IsClosed() {
connection.Close()
}
}
return true
})
return nil, errors.New("multiple inbound connections found and closed; the only resolution to this is to close them all and try connecting again")
}
}
}
// We had a connection but it didn't have a cpability...try again
// We had a connection but it didn't have a capability...try again
conn, err = s.GetConnection(cid)
}
// There are no active connections with that hostname
@ -126,8 +143,8 @@ func (s *BaseOnionService) GetConnection(hostname string) (tapir.Connection, err
// Connect initializes a new outbound connection to the given peer, using the defined Application
func (s *BaseOnionService) Connect(hostname string, app tapir.Application) (bool, error) {
_, err := s.GetConnection(hostname)
if err == nil {
currconn, _ := s.GetConnection(hostname)
if currconn != nil {
// Note: This check is not 100% reliable. And we may end up with two connections between peers
// This can happen when a client connects to a server as the server is connecting to the client
// Because at the start of the connection the server cannot derive the true hostname of the client until it