diff --git a/Bootstrap.go b/Bootstrap.go index 7f1fc5f..907b69b 100644 --- a/Bootstrap.go +++ b/Bootstrap.go @@ -226,6 +226,10 @@ func (peer *PeerInfo) cmdResponseBootstrapFindSelf(msg *MessageResponse, closest } for _, closePeer := range closest { + if closePeer.IsBadQuality() { + continue + } + // Use the self-reported external port if available. port := closePeer.Port if closePeer.PortReportedExternal > 0 { @@ -239,9 +243,10 @@ func (peer *PeerInfo) cmdResponseBootstrapFindSelf(msg *MessageResponse, closest } // If NAT is detected and the port is not forwarded, send a Traverse message. - // NAT detection is the same algorithm as peer.IsBehindNAT. + // NAT detection is the same algorithm as connection.IsBehindNAT. if closePeer.PortReportedExternal == 0 && closePeer.Port != closePeer.PortReportedInternal { // TODO send traverse message + //fmt.Printf("FIND_SELF Traverse message needed for target %s target port %d internal %d\n", closePeer.IP.String(), closePeer.Port, closePeer.PortReportedInternal) } } @@ -252,3 +257,20 @@ func ShouldSendFindSelf() bool { // TODO return true } + +// IsBadQuality checks if the returned peer record is bad quality and should be discarded +func (record *PeerRecord) IsBadQuality() bool { + // Internal port must be provided. Otherwise the external port is likely not provided either, and checking the NAT and port forwarded status is not possible. + if record.PortReportedInternal == 0 { + //fmt.Printf("IsBadQuality port internal not available for target %s port %d, peer %s\n", record.IP.String(), record.Port, hex.EncodeToString(record.PublicKey.SerializeCompressed())) + return true + } + + // Must not be self. There is no point that a remote peer would return self + if record.PublicKey.IsEqual(peerPublicKey) { + //fmt.Printf("IsBadQuality received self peer\n") + return true + } + + return false +} diff --git a/Commands.go b/Commands.go index c0b577d..a8cae8a 100644 --- a/Commands.go +++ b/Commands.go @@ -59,7 +59,8 @@ func (peer *PeerInfo) cmdAnouncement(msg *MessageAnnouncement) { for _, findPeer := range msg.FindPeerKeys { details := Hash2Peer{ID: findPeer} - for _, node := range nodesDHT.GetClosestContacts(respondClosesContactsCount, findPeer.Hash, filterFunc(msg.connection.IsLocal(), allowIPv4, allowIPv6)) { + // Same as before, put self as ignoredNodes. + for _, node := range nodesDHT.GetClosestContacts(respondClosesContactsCount, findPeer.Hash, filterFunc(msg.connection.IsLocal(), allowIPv4, allowIPv6), peer.NodeID) { if info := node.Info.(*PeerInfo).peer2Record(msg.connection.IsLocal(), allowIPv4, allowIPv6); info != nil { details.Closest = append(details.Closest, *info) } @@ -150,7 +151,7 @@ func (peer *PeerInfo) cmdResponse(msg *MessageResponse) { } for _, hash2Peer := range msg.Hash2Peers { - info.QueueResult(&dht.NodeMessage{SenderID: peer.NodeID, Closest: records2Nodes(hash2Peer.Closest, msg.connection.Network), Storing: records2Nodes(hash2Peer.Storing, msg.connection.Network)}) + info.QueueResult(&dht.NodeMessage{SenderID: peer.NodeID, Closest: records2Nodes(hash2Peer.Closest, msg.connection.Network, peer), Storing: records2Nodes(hash2Peer.Storing, msg.connection.Network, peer)}) if hash2Peer.IsLast { info.Done() diff --git a/Connection.go b/Connection.go index dd83916..8cb6f2d 100644 --- a/Connection.go +++ b/Connection.go @@ -29,6 +29,7 @@ type Connection struct { Expires time.Time // Inactive connections only: Expiry date. If it does not become active by that date, it will be considered expired and removed. Status int // 0 = Active established connection, 1 = Inactive, 2 = Removed, 3 = Redundant RoundTripTime time.Duration // Full round-trip time of last reply. + traversePeer *PeerInfo // Temporary peer that may act as proxy for a Traverse message used for the first packet. This is used to establish this Connection to a peer that is behind a NAT. } // Connection status @@ -59,6 +60,11 @@ func (c *Connection) IsIPv6() bool { return IsIPv6(c.Address.IP) } +// IsBehindNAT checks if the remote peer on the connection is likely behind a NAT +func (c *Connection) IsBehindNAT() bool { + return c.PortInternal > 0 && c.Address.Port != int(c.PortInternal) +} + // GetConnections returns the list of connections func (peer *PeerInfo) GetConnections(active bool) (connections []*Connection) { peer.RLock() @@ -77,6 +83,13 @@ func (peer *PeerInfo) IsConnectable(allowLocal, allowIPv4, allowIPv6 bool) bool // Only 1 active connection must be allowed for being connectable. for _, connection := range peer.connectionActive { + // If the internal port is not known, which happens if no Announcement or Response was returned, do not share the peer details. + // This can happen if only other messages such as Ping/Pong were received, or the protocol implementation is not compatible. The external port is also likely not available. + // In this case sharing the peer would be bad, since the receiving peer could not use internal/external port to detemine the NAT status and port forwarding. + if connection.PortInternal == 0 { + continue + } + if IsIPv4(connection.Address.IP) && allowIPv4 || IsIPv6(connection.Address.IP) && allowIPv6 { if !(!allowLocal && connection.IsLocal()) { return true @@ -94,12 +107,12 @@ func (peer *PeerInfo) GetConnection2Share(allowLocal, allowIPv4, allowIPv6 bool) defer peer.RUnlock() if peer.connectionLatest != nil && !(!allowLocal && peer.connectionLatest.IsLocal()) && - (IsIPv4(peer.connectionLatest.Address.IP) && allowIPv4 || IsIPv6(peer.connectionLatest.Address.IP) && allowIPv6) { + (IsIPv4(peer.connectionLatest.Address.IP) && allowIPv4 || IsIPv6(peer.connectionLatest.Address.IP) && allowIPv6) && peer.connectionLatest.PortInternal > 0 { return peer.connectionLatest } for _, connection := range peer.connectionActive { - if (IsIPv4(connection.Address.IP) && allowIPv4 || IsIPv6(connection.Address.IP) && allowIPv6) && !(!allowLocal && connection.IsLocal()) { + if (IsIPv4(connection.Address.IP) && allowIPv4 || IsIPv6(connection.Address.IP) && allowIPv6) && !(!allowLocal && connection.IsLocal()) && connection.PortInternal > 0 { return connection } } @@ -254,13 +267,13 @@ func (peer *PeerInfo) IsBehindNAT() (result bool) { // PortInternal is 0 if no Announcement or Response message was received. for _, connection := range peer.connectionActive { - if connection.PortInternal > 0 && connection.Address.Port != int(connection.PortInternal) { + if connection.IsBehindNAT() { return true } } for _, connection := range peer.connectionInactive { - if connection.PortInternal > 0 && connection.Address.Port != int(connection.PortInternal) { + if connection.IsBehindNAT() { return true } } @@ -304,6 +317,10 @@ func (peer *PeerInfo) send(packet *PacketRaw) (err error) { packet.Protocol = ProtocolVersion + // For Traverse: check if no packet has been sent, and none received (i.e. initial contact). + // If a packet was already received directly (note: not via incoming traversed message), a valid connection is already established. + isFirstPacketOut := atomic.LoadUint64(&peer.StatsPacketSent) == 0 && atomic.LoadUint64(&peer.StatsPacketReceived) == 0 + // always count as one sent packet even if sent via broadcast atomic.AddUint64(&peer.StatsPacketSent, 1) @@ -322,6 +339,11 @@ func (peer *PeerInfo) send(packet *PacketRaw) (err error) { c.LastPacketOut = time.Now() if err = c.Network.send(c.Address.IP, c.Address.Port, raw); err == nil { + // Send Traverse message if the peer is behind NAT and this is the first message. + if isFirstPacketOut && c.IsBehindNAT() { + //fmt.Printf("Traverse message needed for target %s\n", c.Address.String()) + } + return nil } @@ -345,7 +367,14 @@ func (peer *PeerInfo) send(packet *PacketRaw) (err error) { } c.LastPacketOut = time.Now() - c.Network.send(c.Address.IP, c.Address.Port, raw) + + if err = c.Network.send(c.Address.IP, c.Address.Port, raw); err == nil { + // Send Traverse message if the peer is behind NAT and this is the first message. + if isFirstPacketOut && c.IsBehindNAT() { + //fmt.Printf("Traverse message needed for target %s\n", c.Address.String()) + + } + } } return nil // on broadcast no error is known and returned diff --git a/Peer ID.go b/Peer ID.go index e8ff223..e8cf66b 100644 --- a/Peer ID.go +++ b/Peer ID.go @@ -188,30 +188,29 @@ func publicKey2NodeID(publicKey *btcec.PublicKey) (nodeID []byte) { return hashData(publicKey.SerializeCompressed()) } -// record2Peer translate a peer record (from a message) into an actual usable PeerInfo structure -// It requires the network parameter which must be the same as caller/supplier. This ensures that peer details do not "jump" between physical network adapters. -func record2Peer(record PeerRecord, network *Network) (peerN *PeerInfo) { - if peerN = PeerlistLookup(record.PublicKey); peerN != nil { - return peerN - } - - // Create temporary peer which is not added to the global list and not added to Kademlia. - port := record.Port - if record.PortReportedExternal > 0 { // Use the external port if available - port = record.PortReportedExternal - } - - // TODO: Traverse message needs to be considered here - - connection := &Connection{Network: network, Address: &net.UDPAddr{IP: record.IP, Port: int(port)}, Status: ConnectionActive, PortInternal: record.PortReportedInternal, PortExternal: record.PortReportedExternal} - return &PeerInfo{PublicKey: record.PublicKey, connectionActive: []*Connection{connection}, connectionLatest: connection, NodeID: publicKey2NodeID(record.PublicKey), messageSequence: rand.Uint32()} -} - -// records2Nodes translates infoPeer structures to nodes +// records2Nodes translates infoPeer structures to nodes. If the reported nodes are not in the peer table, it will create temporary PeerInfo structures. // LastContact is passed on in the Node.LastSeen field. -func records2Nodes(records []PeerRecord, network *Network) (nodes []*dht.Node) { +// It requires the network parameter which must be the same as caller/supplier. This ensures that peer details do not "jump" between physical network adapters. +func records2Nodes(records []PeerRecord, network *Network, peerSource *PeerInfo) (nodes []*dht.Node) { for _, record := range records { - peer := record2Peer(record, network) + if record.IsBadQuality() { + continue + } + + var peer *PeerInfo + if peer = PeerlistLookup(record.PublicKey); peer == nil { + // Create temporary peer which is not added to the global list and not added to Kademlia. + port := record.Port + if record.PortReportedExternal > 0 { // Use the external port if available + port = record.PortReportedExternal + } + + // traversePeer is set to the peer who provided the node information. + + connection := &Connection{Network: network, Address: &net.UDPAddr{IP: record.IP, Port: int(port)}, Status: ConnectionActive, PortInternal: record.PortReportedInternal, PortExternal: record.PortReportedExternal, traversePeer: peerSource} + peer = &PeerInfo{PublicKey: record.PublicKey, connectionActive: []*Connection{connection}, connectionLatest: connection, NodeID: publicKey2NodeID(record.PublicKey), messageSequence: rand.Uint32()} + } + nodes = append(nodes, &dht.Node{ID: peer.NodeID, LastSeen: record.LastContactT, Info: peer}) }