diff --git a/cmd/syncthing/connections.go b/cmd/syncthing/connections.go index 24bc90b5..eb7c402e 100644 --- a/cmd/syncthing/connections.go +++ b/cmd/syncthing/connections.go @@ -42,12 +42,19 @@ func listenConnect(myID protocol.DeviceID, m *model.Model, tlsCfg *tls.Config) { next: for conn := range conns { cs := conn.ConnectionState() + + // We should have negotiated the next level protocol "bep/1.0" as part + // of the TLS handshake. If we didn't, we're not speaking to another + // BEP-speaker so drop the connection. if !cs.NegotiatedProtocolIsMutual || cs.NegotiatedProtocol != bepProtocolName { l.Infof("Peer %s did not negotiate bep/1.0", conn.RemoteAddr()) conn.Close() continue } + // We should have received exactly one certificate from the other + // side. If we didn't, they don't have a device ID and we drop the + // connection. certs := cs.PeerCertificates if cl := len(certs); cl != 1 { l.Infof("Got peer certificate list of length %d != 1 from %s; protocol error", cl, conn.RemoteAddr()) @@ -57,12 +64,21 @@ next: remoteCert := certs[0] remoteID := protocol.NewDeviceID(remoteCert.Raw) + // The device ID should not be that of ourselves. It can happen + // though, especially in the presense of NAT hairpinning, multiple + // clients between the same NAT gateway, and global discovery. if remoteID == myID { l.Infof("Connected to myself (%s) - should not happen", remoteID) conn.Close() continue } + // We should not already be connected to the other party. TODO: This + // could use some better handling. If the old connection is dead but + // hasn't timed out yet we may want to drop *that* connection and keep + // this one. But in case we are two devices connecting to each other + // in parallell we don't want to do that or we end up with no + // connections still established... if m.ConnectedTo(remoteID) { l.Infof("Connected to already connected device (%s)", remoteID) conn.Close()