From 2aa7138373b3d61bf5d6ec8026192ef82a3d6ec3 Mon Sep 17 00:00:00 2001 From: Adam Ierymenko Date: Mon, 22 Feb 2016 09:47:50 -0800 Subject: [PATCH] Reduce direct ping delay back to 1m and make SelfAwareness aware of local received-on address to eliminate false symmetric classification. --- node/Constants.hpp | 2 +- node/IncomingPacket.cpp | 4 +-- node/SelfAwareness.cpp | 63 +++++++++++++++++++++++------------------ node/SelfAwareness.hpp | 8 ++++-- 4 files changed, 44 insertions(+), 33 deletions(-) diff --git a/node/Constants.hpp b/node/Constants.hpp index 356260e64..19ac3619d 100644 --- a/node/Constants.hpp +++ b/node/Constants.hpp @@ -255,7 +255,7 @@ /** * Delay between ordinary case pings of direct links */ -#define ZT_PEER_DIRECT_PING_DELAY 90000 +#define ZT_PEER_DIRECT_PING_DELAY 60000 /** * Timeout for overall peer activity (measured from last receive) diff --git a/node/IncomingPacket.cpp b/node/IncomingPacket.cpp index efb43d23f..66c956b30 100644 --- a/node/IncomingPacket.cpp +++ b/node/IncomingPacket.cpp @@ -282,7 +282,7 @@ bool IncomingPacket::_doHELLO(const RuntimeEnvironment *RR,SharedPtr &peer } if (externalSurfaceAddress) - RR->sa->iam(id.address(),_remoteAddress,externalSurfaceAddress,RR->topology->isRoot(id),RR->node->now()); + RR->sa->iam(id.address(),_localAddress,_remoteAddress,externalSurfaceAddress,RR->topology->isRoot(id),RR->node->now()); Packet outp(id.address(),RR->identity.address(),Packet::VERB_OK); outp.append((unsigned char)Packet::VERB_HELLO); @@ -388,7 +388,7 @@ bool IncomingPacket::_doOK(const RuntimeEnvironment *RR,const SharedPtr &p peer->setRemoteVersion(vProto,vMajor,vMinor,vRevision); if (externalSurfaceAddress) - RR->sa->iam(peer->address(),_remoteAddress,externalSurfaceAddress,trusted,RR->node->now()); + RR->sa->iam(peer->address(),_localAddress,_remoteAddress,externalSurfaceAddress,trusted,RR->node->now()); } break; case Packet::VERB_WHOIS: { diff --git a/node/SelfAwareness.cpp b/node/SelfAwareness.cpp index dde77ee57..c05028154 100644 --- a/node/SelfAwareness.cpp +++ b/node/SelfAwareness.cpp @@ -67,7 +67,7 @@ SelfAwareness::~SelfAwareness() { } -void SelfAwareness::iam(const Address &reporter,const InetAddress &reporterPhysicalAddress,const InetAddress &myPhysicalAddress,bool trusted,uint64_t now) +void SelfAwareness::iam(const Address &reporter,const InetAddress &receivedOnLocalAddress,const InetAddress &reporterPhysicalAddress,const InetAddress &myPhysicalAddress,bool trusted,uint64_t now) { const InetAddress::IpScope scope = myPhysicalAddress.ipScope(); @@ -75,7 +75,7 @@ void SelfAwareness::iam(const Address &reporter,const InetAddress &reporterPhysi return; Mutex::Lock _l(_phy_m); - PhySurfaceEntry &entry = _phy[PhySurfaceKey(reporter,reporterPhysicalAddress,scope)]; + PhySurfaceEntry &entry = _phy[PhySurfaceKey(reporter,receivedOnLocalAddress,reporterPhysicalAddress,scope)]; 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 @@ -130,10 +130,22 @@ void SelfAwareness::clean(uint64_t now) std::vector SelfAwareness::getSymmetricNatPredictions() { - std::set surfaces; - - // Ideas based on: https://tools.ietf.org/html/draft-takeda-symmetric-nat-traversal-00 + /* This is based on ideas and strategies found here: + * https://tools.ietf.org/html/draft-takeda-symmetric-nat-traversal-00 + * + * In short: a great many symmetric NATs allocate ports sequentially. + * This is common on enterprise and carrier grade NATs as well as consumer + * devices. This code generates a list of "you might try this" addresses by + * extrapolating likely port assignments from currently known external + * global IPv4 surfaces. These can then be included in a PUSH_DIRECT_PATHS + * message to another peer, causing it to possibly try these addresses and + * bust our local symmetric NAT. It works often enough to be worth the + * extra bit of code and does no harm in cases where it fails. */ + // Gather unique surfaces indexed by local received-on address and flag + // us as behind a symmetric NAT if there is more than one. + std::map< InetAddress,std::set > surfaces; + bool symmetric = false; { Mutex::Lock _l(_phy_m); Hashtable< PhySurfaceKey,PhySurfaceEntry >::Iterator i(_phy); @@ -141,33 +153,30 @@ std::vector SelfAwareness::getSymmetricNatPredictions() 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); + std::set &s = surfaces[k->receivedOnLocalAddress]; + s.insert(e->mySurface); + symmetric = symmetric||(s.size() > 1); } } } - if (surfaces.size() > 1) { - // More than one global IPv4 surface means this is a symmetric NAT + // If we appear to be symmetrically NATed, generate and return extrapolations + // of those surfaces. Since PUSH_DIRECT_PATHS is sent multiple times, we + // probabilistically generate extrapolations of anywhere from +1 to +5 to + // increase the odds that it will work "eventually". + if (symmetric) { std::vector r; - for(std::set::iterator i(surfaces.begin());i!=surfaces.end();++i) { - InetAddress ipp(*i); - unsigned int p = ipp.port(); - - // Try 1+ surface ports - if (p >= 0xffff) - p = 1025; - else ++p; - ipp.setPort(p); - if ((surfaces.count(ipp) == 0)&&(std::find(r.begin(),r.end(),ipp) == r.end())) - r.push_back(ipp); - - // Try 2+ surface ports - if (p >= 0xffff) - p = 1025; - else ++p; - ipp.setPort(p); - if ((surfaces.count(ipp) == 0)&&(std::find(r.begin(),r.end(),ipp) == r.end())) - r.push_back(ipp); + for(std::map< InetAddress,std::set >::iterator si(surfaces.begin());si!=surfaces.end();++si) { + for(std::set::iterator i(si->second.begin());i!=si->second.end();++i) { + InetAddress ipp(*i); + unsigned int p = ipp.port() + 1 + ((unsigned int)RR->node->prng() % 5); + if (p >= 65535) + p -= 64510; // NATs seldom use ports <=1024 so wrap to 1025 + ipp.setPort(p); + if ((si->second.count(ipp) == 0)&&(std::find(r.begin(),r.end(),ipp) == r.end())) { + r.push_back(ipp); + } + } } return r; } diff --git a/node/SelfAwareness.hpp b/node/SelfAwareness.hpp index 73950a53e..06c264a91 100644 --- a/node/SelfAwareness.hpp +++ b/node/SelfAwareness.hpp @@ -42,12 +42,13 @@ public: * Called when a trusted remote peer informs us of our external network address * * @param reporter ZeroTier address of reporting peer + * @param receivedOnLocalAddress Local address on which report was received * @param reporterPhysicalAddress Physical address that reporting peer seems to have * @param myPhysicalAddress Physical address that peer says we have * @param trusted True if this peer is trusted as an authority to inform us of external address changes * @param now Current time */ - void iam(const Address &reporter,const InetAddress &reporterPhysicalAddress,const InetAddress &myPhysicalAddress,bool trusted,uint64_t now); + void iam(const Address &reporter,const InetAddress &receivedOnLocalAddress,const InetAddress &reporterPhysicalAddress,const InetAddress &myPhysicalAddress,bool trusted,uint64_t now); /** * Clean up database periodically @@ -67,14 +68,15 @@ private: struct PhySurfaceKey { Address reporter; + InetAddress receivedOnLocalAddress; InetAddress reporterPhysicalAddress; InetAddress::IpScope scope; PhySurfaceKey() : reporter(),scope(InetAddress::IP_SCOPE_NONE) {} - PhySurfaceKey(const Address &r,const InetAddress &ra,InetAddress::IpScope s) : reporter(r),reporterPhysicalAddress(ra),scope(s) {} + PhySurfaceKey(const Address &r,const InetAddress &rol,const InetAddress &ra,InetAddress::IpScope s) : reporter(r),receivedOnLocalAddress(rol),reporterPhysicalAddress(ra),scope(s) {} inline unsigned long hashCode() const throw() { return ((unsigned long)reporter.toInt() + (unsigned long)scope); } - inline bool operator==(const PhySurfaceKey &k) const throw() { return ((reporter == k.reporter)&&(reporterPhysicalAddress == k.reporterPhysicalAddress)&&(scope == k.scope)); } + inline bool operator==(const PhySurfaceKey &k) const throw() { return ((reporter == k.reporter)&&(receivedOnLocalAddress == k.receivedOnLocalAddress)&&(reporterPhysicalAddress == k.reporterPhysicalAddress)&&(scope == k.scope)); } }; struct PhySurfaceEntry {