More tweaks to new symmetric NAT buster, and stop using old iterative method since this supersedes it.

node/Peer.cpp

@@ -240,70 +240,83 @@ bool Peer::doPingAndKeepalive(uint64_t now,int inetAddressFamily)
 	return false;
 }
 
-void Peer::pushDirectPaths(Path *path,uint64_t now,bool force)
+bool Peer::pushDirectPaths(const InetAddress &localAddr,const InetAddress &toAddress,uint64_t now,bool force)
 {
 #ifdef ZT_ENABLE_CLUSTER
 	// Cluster mode disables normal PUSH_DIRECT_PATHS in favor of cluster-based peer redirection
 	if (RR->cluster)
-		return;
+		return false;
 #endif
 
-	if (((now - _lastDirectPathPushSent) >= ZT_DIRECT_PATH_PUSH_INTERVAL)||(force)) {
+	if (!force) {
-		_lastDirectPathPushSent = now;
+		if ((now - _lastDirectPathPushSent) < ZT_DIRECT_PATH_PUSH_INTERVAL)
+			return false;
+		else _lastDirectPathPushSent = now;
+	}
 
-		std::vector<InetAddress> dps(RR->node->directPaths());
+	std::vector<InetAddress> dps(RR->node->directPaths());
-		if (dps.empty())
+	std::vector<InetAddress> sym(RR->sa->getSymmetricNatPredictions());
-			return;
+	for(unsigned long i=0,added=0;i<sym.size();++i) {
-
+		InetAddress tmp(sym[(unsigned long)RR->node->prng() % sym.size()]);
-#ifdef ZT_TRACE
+		if (std::find(dps.begin(),dps.end(),tmp) == dps.end()) {
-		{
+			dps.push_back(tmp);
-			std::string ps;
+			if (++added >= ZT_PUSH_DIRECT_PATHS_MAX_PER_SCOPE_AND_FAMILY)
-			for(std::vector<InetAddress>::const_iterator p(dps.begin());p!=dps.end();++p) {
+				break;
-				if (ps.length() > 0)
-					ps.push_back(',');
-				ps.append(p->toString());
-			}
-			TRACE("pushing %u direct paths to %s: %s",(unsigned int)dps.size(),_id.address().toString().c_str(),ps.c_str());
-		}
-#endif
-
-		std::vector<InetAddress>::const_iterator p(dps.begin());
-		while (p != dps.end()) {
-			Packet outp(_id.address(),RR->identity.address(),Packet::VERB_PUSH_DIRECT_PATHS);
-			outp.addSize(2); // leave room for count
-
-			unsigned int count = 0;
-			while ((p != dps.end())&&((outp.size() + 24) < ZT_PROTO_MAX_PACKET_LENGTH)) {
-				uint8_t addressType = 4;
-				switch(p->ss_family) {
-					case AF_INET:
-						break;
-					case AF_INET6:
-						addressType = 6;
-						break;
-					default: // we currently only push IP addresses
-						++p;
-						continue;
-				}
-
-				outp.append((uint8_t)0); // no flags
-				outp.append((uint16_t)0); // no extensions
-				outp.append(addressType);
-				outp.append((uint8_t)((addressType == 4) ? 6 : 18));
-				outp.append(p->rawIpData(),((addressType == 4) ? 4 : 16));
-				outp.append((uint16_t)p->port());
-
-				++count;
-				++p;
-			}
-
-			if (count) {
-				outp.setAt(ZT_PACKET_IDX_PAYLOAD,(uint16_t)count);
-				outp.armor(_key,true);
-				path->send(RR,outp.data(),outp.size(),now);
-			}
 		}
 	}
+	if (dps.empty())
+		return false;
+
+#ifdef ZT_TRACE
+	{
+		std::string ps;
+		for(std::vector<InetAddress>::const_iterator p(dps.begin());p!=dps.end();++p) {
+			if (ps.length() > 0)
+				ps.push_back(',');
+			ps.append(p->toString());
+		}
+		TRACE("pushing %u direct paths to %s: %s",(unsigned int)dps.size(),_id.address().toString().c_str(),ps.c_str());
+	}
+#endif
+
+	std::vector<InetAddress>::const_iterator p(dps.begin());
+	while (p != dps.end()) {
+		Packet outp(_id.address(),RR->identity.address(),Packet::VERB_PUSH_DIRECT_PATHS);
+		outp.addSize(2); // leave room for count
+
+		unsigned int count = 0;
+		while ((p != dps.end())&&((outp.size() + 24) < 1200)) {
+			uint8_t addressType = 4;
+			switch(p->ss_family) {
+				case AF_INET:
+					break;
+				case AF_INET6:
+					addressType = 6;
+					break;
+				default: // we currently only push IP addresses
+					++p;
+					continue;
+			}
+
+			outp.append((uint8_t)0); // no flags
+			outp.append((uint16_t)0); // no extensions
+			outp.append(addressType);
+			outp.append((uint8_t)((addressType == 4) ? 6 : 18));
+			outp.append(p->rawIpData(),((addressType == 4) ? 4 : 16));
+			outp.append((uint16_t)p->port());
+
+			++count;
+			++p;
+		}
+
+		if (count) {
+			outp.setAt(ZT_PACKET_IDX_PAYLOAD,(uint16_t)count);
+			outp.armor(_key,true);
+			RR->node->putPacket(localAddr,toAddress,outp.data(),outp.size(),0);
+		}
+	}
+
+	return true;
 }
 
 bool Peer::resetWithinScope(InetAddress::IpScope scope,uint64_t now)

node/Peer.hpp

@@ -170,11 +170,13 @@ public:
 	/**
 	 * Push direct paths back to self if we haven't done so in the configured timeout
 	 *
-	 * @param path Remote path to use to send the push
+	 * @param localAddr Local address
+	 * @param toAddress Remote address to send push to (usually from path)
 	 * @param now Current time
 	 * @param force If true, push regardless of rate limit
+	 * @return True if something was actually sent
 	 */
-	void pushDirectPaths(Path *path,uint64_t now,bool force);
+	bool pushDirectPaths(const InetAddress &localAddr,const InetAddress &toAddress,uint64_t now,bool force);
 
 	/**
 	 * @return All known direct paths to this peer (active or inactive)

node/SelfAwareness.cpp

@@ -20,6 +20,9 @@
 #include <stdlib.h>
 #include <string.h>
 
+#include <set>
+#include <vector>
+
 #include "Constants.hpp"
 #include "SelfAwareness.hpp"
 #include "RuntimeEnvironment.hpp"
@@ -68,30 +71,14 @@ void SelfAwareness::iam(const Address &reporter,const InetAddress &reporterPhysi
 {
 	const InetAddress::IpScope scope = myPhysicalAddress.ipScope();
 
-	// This would be weird, e.g. a public IP talking to 10.0.0.1, so just ignore it.
+	if ((scope != reporterPhysicalAddress.ipScope())||(scope == InetAddress::IP_SCOPE_NONE)||(scope == InetAddress::IP_SCOPE_LOOPBACK)||(scope == InetAddress::IP_SCOPE_MULTICAST))
-	// If your network is this weird it's probably not reliable information.
-	if (scope != reporterPhysicalAddress.ipScope())
 		return;
 
-	// Some scopes we ignore, and global scope IPs are only used for this
-	// mechanism if they come from someone we trust (e.g. a root).
-	switch(scope) {
-		case InetAddress::IP_SCOPE_NONE:
-		case InetAddress::IP_SCOPE_LOOPBACK:
-		case InetAddress::IP_SCOPE_MULTICAST:
-			return;
-		case InetAddress::IP_SCOPE_GLOBAL:
-			if (!trusted)
-				return;
-			break;
-		default:
-			break;
-	}
-
 	Mutex::Lock _l(_phy_m);
 	PhySurfaceEntry &entry = _phy[PhySurfaceKey(reporter,reporterPhysicalAddress,scope)];
 
-	if ( ((now - entry.ts) < ZT_SELFAWARENESS_ENTRY_TIMEOUT) && (!entry.mySurface.ipsEqual(myPhysicalAddress)) ) {
+	if ( (trusted) && ((now - entry.ts) < ZT_SELFAWARENESS_ENTRY_TIMEOUT) && (!entry.mySurface.ipsEqual(myPhysicalAddress)) ) {
+		// Changes to external surface reported by trusted peers causes path reset in this scope
 		entry.mySurface = myPhysicalAddress;
 		entry.ts = now;
 		TRACE("physical address %s for scope %u as seen from %s(%s) differs from %s, resetting paths in scope",myPhysicalAddress.toString().c_str(),(unsigned int)scope,reporter.toString().c_str(),reporterPhysicalAddress.toString().c_str(),entry.mySurface.toString().c_str());
@@ -123,6 +110,7 @@ void SelfAwareness::iam(const Address &reporter,const InetAddress &reporterPhysi
 			}
 		}
 	} else {
+		// Otherwise just update DB to use to determine external surface info
 		entry.mySurface = myPhysicalAddress;
 		entry.ts = now;
 	}
@@ -140,4 +128,41 @@ void SelfAwareness::clean(uint64_t now)
 	}
 }
 
+std::vector<InetAddress> SelfAwareness::getSymmetricNatPredictions()
+{
+	std::set<InetAddress> surfaces;
+
+	// Ideas based on: https://tools.ietf.org/html/draft-takeda-symmetric-nat-traversal-00
+
+	{
+		Mutex::Lock _l(_phy_m);
+		Hashtable< PhySurfaceKey,PhySurfaceEntry >::Iterator i(_phy);
+		PhySurfaceKey *k = (PhySurfaceKey *)0;
+		PhySurfaceEntry *e = (PhySurfaceEntry *)0;
+		while (i.next(k,e)) {
+			if ((e->mySurface.ss_family == AF_INET)&&(e->mySurface.ipScope() == InetAddress::IP_SCOPE_GLOBAL)) {
+				surfaces.insert(e->mySurface);
+			}
+		}
+	}
+
+	if (surfaces.size() > 1) {
+		// More than one global IPv4 surface means this is a symmetric NAT
+		std::vector<InetAddress> r;
+		for(std::set<InetAddress>::iterator i(surfaces.begin());i!=surfaces.end();++i) {
+			InetAddress nextPort(*i);
+			unsigned int p = nextPort.port();
+			if (p >= 65535)
+				p = 1025;
+			else ++p;
+			nextPort.setPort(p);
+			if (surfaces.count(nextPort) == 0)
+				r.push_back(nextPort);
+		}
+		return r;
+	}
+
+	return std::vector<InetAddress>();
+}
+
 } // namespace ZeroTier

node/SelfAwareness.hpp

@@ -56,6 +56,13 @@ public:
 	 */
 	void clean(uint64_t now);
 
+	/**
+	 * If we appear to be behind a symmetric NAT, get predictions for possible external endpoints
+	 *
+	 * @return Symmetric NAT predictions or empty vector if none
+	 */
+	std::vector<InetAddress> getSymmetricNatPredictions();
+
 private:
 	struct PhySurfaceKey
 	{

node/Switch.cpp

@@ -478,31 +478,31 @@ unsigned long Switch::doTimerTasks(uint64_t now)
 		Mutex::Lock _l(_contactQueue_m);
 		for(std::list<ContactQueueEntry>::iterator qi(_contactQueue.begin());qi!=_contactQueue.end();) {
 			if (now >= qi->fireAtTime) {
-				if (qi->peer->hasActiveDirectPath(now)) {
+				if (!qi->peer->pushDirectPaths(qi->localAddr,qi->inaddr,now,true))
-					// Cancel if connection has succeeded
+					qi->peer->sendHELLO(qi->localAddr,qi->inaddr,now);
+				_contactQueue.erase(qi++);
+				continue;
+				/* Old symmetric NAT buster code, obsoleted by port prediction alg in SelfAwareness but left around for now in case we revert
+				if (qi->strategyIteration == 0) {
+					// First strategy: send packet directly to destination
+					qi->peer->sendHELLO(qi->localAddr,qi->inaddr,now);
+				} else if (qi->strategyIteration <= 3) {
+					// Strategies 1-3: try escalating ports for symmetric NATs that remap sequentially
+					InetAddress tmpaddr(qi->inaddr);
+					int p = (int)qi->inaddr.port() + qi->strategyIteration;
+					if (p > 65535)
+						p -= 64511;
+					tmpaddr.setPort((unsigned int)p);
+					qi->peer->sendHELLO(qi->localAddr,tmpaddr,now);
+				} else {
+					// All strategies tried, expire entry
 					_contactQueue.erase(qi++);
 					continue;
-				} else {
-					if (qi->strategyIteration == 0) {
-						// First strategy: send packet directly to destination
-						qi->peer->sendHELLO(qi->localAddr,qi->inaddr,now);
-					} else if (qi->strategyIteration <= 3) {
-						// Strategies 1-3: try escalating ports for symmetric NATs that remap sequentially
-						InetAddress tmpaddr(qi->inaddr);
-						int p = (int)qi->inaddr.port() + qi->strategyIteration;
-						if (p > 65535)
-							p -= 64511;
-						tmpaddr.setPort((unsigned int)p);
-						qi->peer->sendHELLO(qi->localAddr,tmpaddr,now);
-					} else {
-						// All strategies tried, expire entry
-						_contactQueue.erase(qi++);
-						continue;
-					}
-					++qi->strategyIteration;
-					qi->fireAtTime = now + ZT_NAT_T_TACTICAL_ESCALATION_DELAY;
-					nextDelay = std::min(nextDelay,(unsigned long)ZT_NAT_T_TACTICAL_ESCALATION_DELAY);
 				}
+				++qi->strategyIteration;
+				qi->fireAtTime = now + ZT_NAT_T_TACTICAL_ESCALATION_DELAY;
+				nextDelay = std::min(nextDelay,(unsigned long)ZT_NAT_T_TACTICAL_ESCALATION_DELAY);
+				*/
 			} else {
 				nextDelay = std::min(nextDelay,(unsigned long)(qi->fireAtTime - now));
 			}
@@ -813,12 +813,13 @@ bool Switch::_trySend(const Packet &packet,bool encrypt,uint64_t nwid)
 				relay = RR->topology->getBestRoot();
 
 			if (!(relay)||(!(viaPath = relay->getBestPath(now))))
-				return false; // no paths, no root servers?
+				return false; // no paths, no root servers?, no relays? :P~~~
 		}
 
 		if ((network)&&(relay)&&(network->isAllowed(peer))) {
 			// Push hints for direct connectivity to this peer if we are relaying
-			peer->pushDirectPaths(viaPath,now,false);
+			peer->pushDirectPaths(viaPath->localAddress(),viaPath->address(),now,false);
+			viaPath->sent(now);
 		}
 
 		Packet tmp(packet);