From 3ba54c7e3559359abd8d4734aa969829309a9dab Mon Sep 17 00:00:00 2001 From: Adam Ierymenko Date: Thu, 23 Jul 2015 09:50:10 -0700 Subject: Eliminate some poorly thought out optimizations from the netconf/controller interaction, and go ahead and bump version to 1.0.4. For a while in 1.0.3 -dev I was trying to optimize out repeated network controller requests by using a ratcheting mechanism. If the client received a network config that was indeed different from the one it had, it would respond by instantlly requesting it again. Not sure what I was thinking. It's fundamentally unsafe to respond to a message with another message of the same type -- it risks a race condition. In this case that's exactly what could happen. It just isn't worth the added complexity to avoid a tiny, tiny amount of network overhead, so I've taken this whole path out. A few extra bytes every two minutes isn't worth fretting about, but as I recall the reason for this optimization was to save CPU on the controller. This can be achieved by just caching responses in memory *there* and serving those same responses back out if they haven't changed. I think I developed that 'ratcheting' stuff before I went full time on this. It's hard to develop stuff like this without hours of sustained focus. --- node/IncomingPacket.cpp | 31 +++++++------------------------ node/Network.cpp | 13 ++++++++++++- node/NetworkConfig.cpp | 23 ++++++++--------------- node/NetworkConfig.hpp | 26 +++++++------------------- node/NetworkController.hpp | 7 +++---- 5 files changed, 37 insertions(+), 63 deletions(-) (limited to 'node') diff --git a/node/IncomingPacket.cpp b/node/IncomingPacket.cpp index ae99352e..c3d8cc6d 100644 --- a/node/IncomingPacket.cpp +++ b/node/IncomingPacket.cpp @@ -392,28 +392,7 @@ bool IncomingPacket::_doOK(const RuntimeEnvironment *RR,const SharedPtr &p const unsigned int dictlen = at(ZT_PROTO_VERB_NETWORK_CONFIG_REQUEST__OK__IDX_DICT_LEN); const std::string dict((const char *)field(ZT_PROTO_VERB_NETWORK_CONFIG_REQUEST__OK__IDX_DICT,dictlen),dictlen); if (dict.length()) { - if (nw->setConfiguration(Dictionary(dict)) == 2) { // 2 == accepted and actually new - /* If this configuration was indeed new, we do another - * controller request with its revision. We do this in - * order to (a) tell the network controller we got it (it - * won't send a duplicate if ts == current), and (b) - * get another one if the controller is changing rapidly - * until we finally have the final version. - * - * Note that we don't do this for network controllers with - * versions <= 1.0.3, since those regenerate a new controller - * with a new revision every time. In that case this double - * confirmation would create a race condition. */ - const SharedPtr nc(nw->config2()); - if ((peer->atLeastVersion(1,0,3))&&(nc)&&(nc->revision() > 0)) { - Packet outp(peer->address(),RR->identity.address(),Packet::VERB_NETWORK_CONFIG_REQUEST); - outp.append((uint64_t)nw->id()); - outp.append((uint16_t)0); // no meta-data - outp.append((uint64_t)nc->revision()); - outp.armor(peer->key(),true); - RR->node->putPacket(_remoteAddress,outp.data(),outp.size()); - } - } + nw->setConfiguration(Dictionary(dict)); TRACE("got network configuration for network %.16llx from %s",(unsigned long long)nw->id(),source().toString().c_str()); } } @@ -692,6 +671,7 @@ bool IncomingPacket::_doNETWORK_CONFIG_REQUEST(const RuntimeEnvironment *RR,cons if (RR->localNetworkController) { Dictionary netconf; switch(RR->localNetworkController->doNetworkConfigRequest((h > 0) ? InetAddress() : _remoteAddress,RR->identity,peer->identity(),nwid,metaData,haveRevision,netconf)) { + case NetworkController::NETCONF_QUERY_OK: { const std::string netconfStr(netconf.toString()); if (netconfStr.length() > 0xffff) { // sanity check since field ix 16-bit @@ -712,8 +692,7 @@ bool IncomingPacket::_doNETWORK_CONFIG_REQUEST(const RuntimeEnvironment *RR,cons } } } break; - case NetworkController::NETCONF_QUERY_OK_BUT_NOT_NEWER: // nothing to do -- netconf has not changed - break; + case NetworkController::NETCONF_QUERY_OBJECT_NOT_FOUND: { Packet outp(peer->address(),RR->identity.address(),Packet::VERB_ERROR); outp.append((unsigned char)Packet::VERB_NETWORK_CONFIG_REQUEST); @@ -723,6 +702,7 @@ bool IncomingPacket::_doNETWORK_CONFIG_REQUEST(const RuntimeEnvironment *RR,cons outp.armor(peer->key(),true); RR->node->putPacket(_remoteAddress,outp.data(),outp.size()); } break; + case NetworkController::NETCONF_QUERY_ACCESS_DENIED: { Packet outp(peer->address(),RR->identity.address(),Packet::VERB_ERROR); outp.append((unsigned char)Packet::VERB_NETWORK_CONFIG_REQUEST); @@ -732,12 +712,15 @@ bool IncomingPacket::_doNETWORK_CONFIG_REQUEST(const RuntimeEnvironment *RR,cons outp.armor(peer->key(),true); RR->node->putPacket(_remoteAddress,outp.data(),outp.size()); } break; + case NetworkController::NETCONF_QUERY_INTERNAL_SERVER_ERROR: TRACE("NETWORK_CONFIG_REQUEST failed: internal error: %s",netconf.get("error","(unknown)").c_str()); break; + default: TRACE("NETWORK_CONFIG_REQUEST failed: invalid return value from NetworkController::doNetworkConfigRequest()"); break; + } } else { Packet outp(peer->address(),RR->identity.address(),Packet::VERB_ERROR); diff --git a/node/Network.cpp b/node/Network.cpp index adc8e1b8..549219d7 100644 --- a/node/Network.cpp +++ b/node/Network.cpp @@ -38,6 +38,8 @@ #include "Buffer.hpp" #include "NetworkController.hpp" +#include "../version.h" + namespace ZeroTier { const ZeroTier::MulticastGroup Network::BROADCAST(ZeroTier::MAC(0xffffffffffffULL),0); @@ -255,9 +257,18 @@ void Network::requestConfiguration() } TRACE("requesting netconf for network %.16llx from controller %s",(unsigned long long)_id,controller().toString().c_str()); + + // TODO: in the future we will include things like join tokens here, etc. + Dictionary metaData; + metaData.setHex(ZT_NETWORKCONFIG_REQUEST_METADATA_KEY_NODE_MAJOR_VERSION,ZEROTIER_ONE_VERSION_MAJOR); + metaData.setHex(ZT_NETWORKCONFIG_REQUEST_METADATA_KEY_NODE_MINOR_VERSION,ZEROTIER_ONE_VERSION_MINOR); + metaData.setHex(ZT_NETWORKCONFIG_REQUEST_METADATA_KEY_NODE_REVISION,ZEROTIER_ONE_VERSION_REVISION); + std::string mds(metaData.toString()); + Packet outp(controller(),RR->identity.address(),Packet::VERB_NETWORK_CONFIG_REQUEST); outp.append((uint64_t)_id); - outp.append((uint16_t)0); // no meta-data + outp.append((uint16_t)mds.length()); + outp.append((const void *)mds.data(),(unsigned int)mds.length()); { Mutex::Lock _l(_lock); if (_config) diff --git a/node/NetworkConfig.cpp b/node/NetworkConfig.cpp index ba4d338b..7898646c 100644 --- a/node/NetworkConfig.cpp +++ b/node/NetworkConfig.cpp @@ -47,7 +47,6 @@ SharedPtr NetworkConfig::createTestNetworkConfig(const Address &s nc->_private = false; nc->_enableBroadcast = true; nc->_name = "ZT_TEST_NETWORK"; - nc->_description = "Built-in dummy test network"; // Make up a V4 IP from 'self' in the 10.0.0.0/8 range -- no // guarantee of uniqueness but collisions are unlikely. @@ -111,7 +110,6 @@ void NetworkConfig::_fromDictionary(const Dictionary &d) _name = d.get(ZT_NETWORKCONFIG_DICT_KEY_NAME); if (_name.length() > ZT1_MAX_NETWORK_SHORT_NAME_LENGTH) throw std::invalid_argument("network short name too long (max: 255 characters)"); - _description = d.get(ZT_NETWORKCONFIG_DICT_KEY_DESC,std::string()); // In dictionary IPs are split into V4 and V6 addresses, but we don't really // need that so merge them here. @@ -132,26 +130,22 @@ void NetworkConfig::_fromDictionary(const Dictionary &d) case AF_INET: if ((!addr.netmaskBits())||(addr.netmaskBits() > 32)) continue; - else if (addr.isNetwork()) { - // TODO: add route to network -- this is a route without an IP assignment - continue; - } break; case AF_INET6: if ((!addr.netmaskBits())||(addr.netmaskBits() > 128)) continue; - else if (addr.isNetwork()) { - // TODO: add route to network -- this is a route without an IP assignment - continue; - } break; default: // ignore unrecognized address types or junk/empty fields continue; } - _staticIps.push_back(addr); + if (addr.isNetwork()) + _localRoutes.push_back(addr); + else _staticIps.push_back(addr); } - if (_staticIps.size() > ZT1_MAX_ZT_ASSIGNED_ADDRESSES) - throw std::invalid_argument("too many ZT-assigned IP addresses or routes"); + if (_localRoutes.size() > ZT1_MAX_ZT_ASSIGNED_ADDRESSES) throw std::invalid_argument("too many ZT-assigned routes"); + if (_staticIps.size() > ZT1_MAX_ZT_ASSIGNED_ADDRESSES) throw std::invalid_argument("too many ZT-assigned IP addresses"); + std::sort(_localRoutes.begin(),_localRoutes.end()); + _localRoutes.erase(std::unique(_localRoutes.begin(),_localRoutes.end()),_localRoutes.end()); std::sort(_staticIps.begin(),_staticIps.end()); _staticIps.erase(std::unique(_staticIps.begin(),_staticIps.end()),_staticIps.end()); @@ -201,7 +195,7 @@ bool NetworkConfig::operator==(const NetworkConfig &nc) const if (_private != nc._private) return false; if (_enableBroadcast != nc._enableBroadcast) return false; if (_name != nc._name) return false; - if (_description != nc._description) return false; + if (_localRoutes != nc._localRoutes) return false; if (_staticIps != nc._staticIps) return false; if (_gateways != nc._gateways) return false; if (_activeBridges != nc._activeBridges) return false; @@ -211,4 +205,3 @@ bool NetworkConfig::operator==(const NetworkConfig &nc) const } } // namespace ZeroTier - diff --git a/node/NetworkConfig.hpp b/node/NetworkConfig.hpp index 5c7cdd7c..6111e65b 100644 --- a/node/NetworkConfig.hpp +++ b/node/NetworkConfig.hpp @@ -47,59 +47,48 @@ namespace ZeroTier { +// Fields for meta-data sent with network config requests +#define ZT_NETWORKCONFIG_REQUEST_METADATA_KEY_NODE_MAJOR_VERSION "majv" +#define ZT_NETWORKCONFIG_REQUEST_METADATA_KEY_NODE_MINOR_VERSION "minv" +#define ZT_NETWORKCONFIG_REQUEST_METADATA_KEY_NODE_REVISION "revv" + // These dictionary keys are short so they don't take up much room in // netconf response packets. // integer(hex)[,integer(hex),...] #define ZT_NETWORKCONFIG_DICT_KEY_ALLOWED_ETHERNET_TYPES "et" - // network ID #define ZT_NETWORKCONFIG_DICT_KEY_NETWORK_ID "nwid" - // integer(hex) #define ZT_NETWORKCONFIG_DICT_KEY_TIMESTAMP "ts" - // integer(hex) #define ZT_NETWORKCONFIG_DICT_KEY_REVISION "r" - // address of member #define ZT_NETWORKCONFIG_DICT_KEY_ISSUED_TO "id" - // integer(hex) #define ZT_NETWORKCONFIG_DICT_KEY_MULTICAST_LIMIT "ml" - // 0/1 #define ZT_NETWORKCONFIG_DICT_KEY_PRIVATE "p" - // text #define ZT_NETWORKCONFIG_DICT_KEY_NAME "n" - // text #define ZT_NETWORKCONFIG_DICT_KEY_DESC "d" - // IP/bits[,IP/bits,...] // Note that IPs that end in all zeroes are routes with no assignment in them. #define ZT_NETWORKCONFIG_DICT_KEY_IPV4_STATIC "v4s" - // IP/bits[,IP/bits,...] // Note that IPs that end in all zeroes are routes with no assignment in them. #define ZT_NETWORKCONFIG_DICT_KEY_IPV6_STATIC "v6s" - // serialized CertificateOfMembership #define ZT_NETWORKCONFIG_DICT_KEY_CERTIFICATE_OF_MEMBERSHIP "com" - // 0/1 #define ZT_NETWORKCONFIG_DICT_KEY_ENABLE_BROADCAST "eb" - // 0/1 #define ZT_NETWORKCONFIG_DICT_KEY_ALLOW_PASSIVE_BRIDGING "pb" - // node[,node,...] #define ZT_NETWORKCONFIG_DICT_KEY_ACTIVE_BRIDGES "ab" - // node;IP/port[,node;IP/port] #define ZT_NETWORKCONFIG_DICT_KEY_RELAYS "rl" - // IP/metric[,IP/metric,...] #define ZT_NETWORKCONFIG_DICT_KEY_GATEWAYS "gw" @@ -158,7 +147,7 @@ public: inline bool isPublic() const throw() { return (!_private); } inline bool isPrivate() const throw() { return _private; } inline const std::string &name() const throw() { return _name; } - inline const std::string &description() const throw() { return _description; } + inline const std::vector &localRoutes() const throw() { return _localRoutes; } inline const std::vector &staticIps() const throw() { return _staticIps; } inline const std::vector &gateways() const throw() { return _gateways; } inline const std::vector
&activeBridges() const throw() { return _activeBridges; } @@ -194,7 +183,7 @@ private: bool _private; bool _enableBroadcast; std::string _name; - std::string _description; + std::vector _localRoutes; std::vector _staticIps; std::vector _gateways; std::vector
_activeBridges; @@ -207,4 +196,3 @@ private: } // namespace ZeroTier #endif - diff --git a/node/NetworkController.hpp b/node/NetworkController.hpp index 265ee3d4..bef884de 100644 --- a/node/NetworkController.hpp +++ b/node/NetworkController.hpp @@ -52,10 +52,9 @@ public: enum ResultCode { NETCONF_QUERY_OK = 0, - NETCONF_QUERY_OK_BUT_NOT_NEWER = 1, - NETCONF_QUERY_OBJECT_NOT_FOUND = 2, - NETCONF_QUERY_ACCESS_DENIED = 3, - NETCONF_QUERY_INTERNAL_SERVER_ERROR = 4 + NETCONF_QUERY_OBJECT_NOT_FOUND = 1, + NETCONF_QUERY_ACCESS_DENIED = 2, + NETCONF_QUERY_INTERNAL_SERVER_ERROR = 3 }; NetworkController() {} -- cgit v1.2.3 From b3516c599bb0beb4b4827f28da472972344379c6 Mon Sep 17 00:00:00 2001 From: Adam Ierymenko Date: Thu, 23 Jul 2015 10:10:17 -0700 Subject: Add a rate limiting circuit breaker to the network controller to prevent flooding attacks and race conditions. --- controller/SqliteNetworkController.cpp | 13 +++++++++++++ controller/SqliteNetworkController.hpp | 2 ++ node/IncomingPacket.cpp | 3 +++ node/NetworkController.hpp | 3 ++- 4 files changed, 20 insertions(+), 1 deletion(-) (limited to 'node') diff --git a/controller/SqliteNetworkController.cpp b/controller/SqliteNetworkController.cpp index f6489640..bdf337ec 100644 --- a/controller/SqliteNetworkController.cpp +++ b/controller/SqliteNetworkController.cpp @@ -64,6 +64,10 @@ // API version reported via JSON control plane #define ZT_NETCONF_CONTROLLER_API_VERSION 1 +// Drop requests for a given peer and network ID that occur more frequently +// than this (ms). +#define ZT_NETCONF_MIN_REQUEST_PERIOD 5000 + namespace ZeroTier { namespace { @@ -316,6 +320,15 @@ NetworkController::ResultCode SqliteNetworkController::doNetworkConfigRequest(co return NetworkController::NETCONF_QUERY_INTERNAL_SERVER_ERROR; } + // Check rate limit + + { + uint64_t &lrt = _lastRequestTime[std::pair(identity.address(),nwid)]; + uint64_t lrt2 = lrt; + if (((lrt = OSUtils::now()) - lrt2) <= ZT_NETCONF_MIN_REQUEST_PERIOD) + return NetworkController::NETCONF_QUERY_IGNORE; + } + NetworkRecord network; memset(&network,0,sizeof(network)); Utils::snprintf(network.id,sizeof(network.id),"%.16llx",(unsigned long long)nwid); diff --git a/controller/SqliteNetworkController.hpp b/controller/SqliteNetworkController.hpp index bae11519..002493ec 100644 --- a/controller/SqliteNetworkController.hpp +++ b/controller/SqliteNetworkController.hpp @@ -98,6 +98,8 @@ private: std::string _dbPath; std::string _instanceId; + std::map< std::pair,uint64_t > _lastRequestTime; + sqlite3 *_db; sqlite3_stmt *_sGetNetworkById; diff --git a/node/IncomingPacket.cpp b/node/IncomingPacket.cpp index c3d8cc6d..76c47933 100644 --- a/node/IncomingPacket.cpp +++ b/node/IncomingPacket.cpp @@ -717,6 +717,9 @@ bool IncomingPacket::_doNETWORK_CONFIG_REQUEST(const RuntimeEnvironment *RR,cons TRACE("NETWORK_CONFIG_REQUEST failed: internal error: %s",netconf.get("error","(unknown)").c_str()); break; + case NetworkController::NETCONF_QUERY_IGNORE: + break; + default: TRACE("NETWORK_CONFIG_REQUEST failed: invalid return value from NetworkController::doNetworkConfigRequest()"); break; diff --git a/node/NetworkController.hpp b/node/NetworkController.hpp index bef884de..ee481a62 100644 --- a/node/NetworkController.hpp +++ b/node/NetworkController.hpp @@ -54,7 +54,8 @@ public: NETCONF_QUERY_OK = 0, NETCONF_QUERY_OBJECT_NOT_FOUND = 1, NETCONF_QUERY_ACCESS_DENIED = 2, - NETCONF_QUERY_INTERNAL_SERVER_ERROR = 3 + NETCONF_QUERY_INTERNAL_SERVER_ERROR = 3, + NETCONF_QUERY_IGNORE = 4 }; NetworkController() {} -- cgit v1.2.3 From d647a587a1c920cdf58ce77872280e4e4ec9cca9 Mon Sep 17 00:00:00 2001 From: Adam Ierymenko Date: Thu, 23 Jul 2015 17:18:20 -0700 Subject: (1) Fix updating of network revision counter on member change. (2) Go back to timestamp as certificate revision number. This is simpler and more robust than using the network revision number for this and forcing network revision fast-forward, which could cause some peers to fall off the horizon when you don't want them to. --- controller/SqliteNetworkController.cpp | 17 +++++++++++++++-- include/ZeroTierOne.h | 11 ----------- node/Constants.hpp | 4 ++-- 3 files changed, 17 insertions(+), 15 deletions(-) (limited to 'node') diff --git a/controller/SqliteNetworkController.cpp b/controller/SqliteNetworkController.cpp index bdf337ec..b41c7ef5 100644 --- a/controller/SqliteNetworkController.cpp +++ b/controller/SqliteNetworkController.cpp @@ -66,7 +66,7 @@ // Drop requests for a given peer and network ID that occur more frequently // than this (ms). -#define ZT_NETCONF_MIN_REQUEST_PERIOD 5000 +#define ZT_NETCONF_MIN_REQUEST_PERIOD 1000 namespace ZeroTier { @@ -689,7 +689,7 @@ NetworkController::ResultCode SqliteNetworkController::doNetworkConfigRequest(co // TODO: IPv6 auto-assign once it's supported in UI if (network.isPrivate) { - CertificateOfMembership com(network.revision,ZT1_CERTIFICATE_OF_MEMBERSHIP_REVISION_MAX_DELTA,nwid,identity.address()); + CertificateOfMembership com(OSUtils::now(),ZT_NETWORK_AUTOCONF_DELAY + (ZT_NETWORK_AUTOCONF_DELAY / 2),nwid,identity.address()); if (com.sign(signingId)) // basically can't fail unless our identity is invalid netconf[ZT_NETWORKCONFIG_DICT_KEY_CERTIFICATE_OF_MEMBERSHIP] = com.toString(); else { @@ -757,6 +757,8 @@ unsigned int SqliteNetworkController::handleControlPlaneHttpPOST( char addrs[24]; Utils::snprintf(addrs,sizeof(addrs),"%.10llx",address); + int64_t addToNetworkRevision = 0; + int64_t memberRowId = 0; sqlite3_reset(_sGetMember); sqlite3_bind_text(_sGetMember,1,nwids,16,SQLITE_STATIC); @@ -780,6 +782,7 @@ unsigned int SqliteNetworkController::handleControlPlaneHttpPOST( sqlite3_reset(_sIncrementMemberRevisionCounter); sqlite3_bind_text(_sIncrementMemberRevisionCounter,1,nwids,16,SQLITE_STATIC); sqlite3_step(_sIncrementMemberRevisionCounter); + addToNetworkRevision = 1; } json_value *j = json_parse(body.c_str(),body.length()); @@ -799,6 +802,7 @@ unsigned int SqliteNetworkController::handleControlPlaneHttpPOST( sqlite3_reset(_sIncrementMemberRevisionCounter); sqlite3_bind_text(_sIncrementMemberRevisionCounter,1,nwids,16,SQLITE_STATIC); sqlite3_step(_sIncrementMemberRevisionCounter); + addToNetworkRevision = 1; } } else if (!strcmp(j->u.object.values[k].name,"activeBridge")) { if (j->u.object.values[k].value->type == json_boolean) { @@ -812,6 +816,7 @@ unsigned int SqliteNetworkController::handleControlPlaneHttpPOST( sqlite3_reset(_sIncrementMemberRevisionCounter); sqlite3_bind_text(_sIncrementMemberRevisionCounter,1,nwids,16,SQLITE_STATIC); sqlite3_step(_sIncrementMemberRevisionCounter); + addToNetworkRevision = 1; } } else if (!strcmp(j->u.object.values[k].name,"ipAssignments")) { if (j->u.object.values[k].value->type == json_array) { @@ -855,6 +860,7 @@ unsigned int SqliteNetworkController::handleControlPlaneHttpPOST( } } } + addToNetworkRevision = 1; } } @@ -863,6 +869,13 @@ unsigned int SqliteNetworkController::handleControlPlaneHttpPOST( json_value_free(j); } + if ((addToNetworkRevision > 0)&&(revision > 0)) { + sqlite3_reset(_sSetNetworkRevision); + sqlite3_bind_int64(_sSetNetworkRevision,1,revision + addToNetworkRevision); + sqlite3_bind_text(_sSetNetworkRevision,2,nwids,16,SQLITE_STATIC); + sqlite3_step(_sSetNetworkRevision); + } + return _doCPGet(path,urlArgs,headers,body,responseBody,responseContentType); } // else 404 diff --git a/include/ZeroTierOne.h b/include/ZeroTierOne.h index 7ae524a8..dc2243f2 100644 --- a/include/ZeroTierOne.h +++ b/include/ZeroTierOne.h @@ -105,17 +105,6 @@ extern "C" { */ #define ZT1_MAX_PEER_NETWORK_PATHS 4 -/** - * Maximum number of revisions over which a network COM can differ and still be in-horizon (agree) - * - * This is the default max delta for the revision field in COMs issued - * by network controllers, and is defined here for documentation purposes. - * When a network is changed so as to de-authorize a member, its revision - * should be incremented by this number. Otherwise all other changes that - * materially affect the network should result in increment by one. - */ -#define ZT1_CERTIFICATE_OF_MEMBERSHIP_REVISION_MAX_DELTA 16 - /** * Feature flag: ZeroTier One was built to be thread-safe -- concurrent processXXX() calls are okay */ diff --git a/node/Constants.hpp b/node/Constants.hpp index d15fef13..c192381c 100644 --- a/node/Constants.hpp +++ b/node/Constants.hpp @@ -158,7 +158,7 @@ /** * Maximum number of packet fragments we'll support - * + * * The actual spec allows 16, but this is the most we'll support right * now. Packets with more than this many fragments are dropped. */ @@ -216,7 +216,7 @@ /** * Maximum number of ZT hops allowed (this is not IP hops/TTL) - * + * * The protocol allows up to 7, but we limit it to something smaller. */ #define ZT_RELAY_MAX_HOPS 3 -- cgit v1.2.3 From 7a15d8a7e3fb4934200887666afdf17afb1178e5 Mon Sep 17 00:00:00 2001 From: Adam Ierymenko Date: Fri, 24 Jul 2015 14:50:44 -0700 Subject: Fix leaving of networks to actually call Network::destroy(). --- node/Node.cpp | 1 + osdep/OSUtils.hpp | 5 ++--- 2 files changed, 3 insertions(+), 3 deletions(-) (limited to 'node') diff --git a/node/Node.cpp b/node/Node.cpp index ebe0527e..d40ceab9 100644 --- a/node/Node.cpp +++ b/node/Node.cpp @@ -316,6 +316,7 @@ ZT1_ResultCode Node::leave(uint64_t nwid) for(std::vector< std::pair< uint64_t,SharedPtr > >::const_iterator n(_networks.begin());n!=_networks.end();++n) { if (n->first != nwid) newn.push_back(*n); + else n->second->destroy(); } _networks.swap(newn); return ZT1_RESULT_OK; diff --git a/osdep/OSUtils.hpp b/osdep/OSUtils.hpp index bfe9b68a..5de35eba 100644 --- a/osdep/OSUtils.hpp +++ b/osdep/OSUtils.hpp @@ -121,10 +121,10 @@ public: /** * Set modes on a file to something secure - * + * * This locks a file so that only the owner can access it. What it actually * does varies by platform. - * + * * @param path Path to lock * @param isDir True if this is a directory */ @@ -252,4 +252,3 @@ private: } // namespace ZeroTier #endif - -- cgit v1.2.3 From e30ba3e1382b50aa8f393132204f8f27ccfb03f9 Mon Sep 17 00:00:00 2001 From: Adam Ierymenko Date: Mon, 27 Jul 2015 16:43:05 -0700 Subject: Eliminate some aggressive port scanning NAT-t behavior that has proven ineffective. --- node/Switch.cpp | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) (limited to 'node') diff --git a/node/Switch.cpp b/node/Switch.cpp index cf4fe249..3d9ef5b1 100644 --- a/node/Switch.cpp +++ b/node/Switch.cpp @@ -451,7 +451,7 @@ unsigned long Switch::doTimerTasks(uint64_t now) { unsigned long nextDelay = 0xffffffff; // ceiling delay, caller will cap to minimum - { // Aggressive NAT traversal time! + { Mutex::Lock _l(_contactQueue_m); for(std::list::iterator qi(_contactQueue.begin());qi!=_contactQueue.end();) { if (now >= qi->fireAtTime) { @@ -460,26 +460,17 @@ unsigned long Switch::doTimerTasks(uint64_t now) _contactQueue.erase(qi++); continue; } else { - // Nope, nothing yet. Time to kill some kittens. if (qi->strategyIteration == 0) { // First strategy: send packet directly (we already tried this but try again) qi->peer->attemptToContactAt(RR,qi->inaddr,now); - } else if (qi->strategyIteration <= 9) { - // Strategies 1-9: try escalating ports + } else if (qi->strategyIteration <= 4) { + // Strategies 1-4: try escalating ports InetAddress tmpaddr(qi->inaddr); int p = (int)qi->inaddr.port() + qi->strategyIteration; if (p < 0xffff) { tmpaddr.setPort((unsigned int)p); qi->peer->attemptToContactAt(RR,tmpaddr,now); } else qi->strategyIteration = 9; - } else if (qi->strategyIteration <= 18) { - // Strategies 10-18: try ports below - InetAddress tmpaddr(qi->inaddr); - int p = (int)qi->inaddr.port() - (qi->strategyIteration - 9); - if (p >= 1024) { - tmpaddr.setPort((unsigned int)p); - qi->peer->attemptToContactAt(RR,tmpaddr,now); - } else qi->strategyIteration = 18; } else { // All strategies tried, expire entry _contactQueue.erase(qi++); -- cgit v1.2.3 From f0003ea92277039f70dd6f16920dd2db9fb2fb1e Mon Sep 17 00:00:00 2001 From: Adam Ierymenko Date: Mon, 27 Jul 2015 17:02:43 -0700 Subject: Push remote surface as reported by peers along with known interface direct paths to assist with (some) NAT traversal. (trying this, may back out if not effective) --- node/Peer.cpp | 19 +++++++++++++++++++ node/SelfAwareness.hpp | 14 ++++++++++++++ node/Topology.hpp | 8 ++++---- 3 files changed, 37 insertions(+), 4 deletions(-) (limited to 'node') diff --git a/node/Peer.cpp b/node/Peer.cpp index 73c20228..804e6e39 100644 --- a/node/Peer.cpp +++ b/node/Peer.cpp @@ -33,6 +33,7 @@ #include "Switch.hpp" #include "Network.hpp" #include "AntiRecursion.hpp" +#include "SelfAwareness.hpp" #include @@ -229,6 +230,24 @@ void Peer::pushDirectPaths(const RuntimeEnvironment *RR,RemotePath *path,uint64_ _lastDirectPathPush = now; std::vector dps(RR->node->directPaths()); + + /* Also push paths reported to us by non-root-server peers. This assists + * with NAT traversal behind NATs that engage in strange or randomized + * port assignment behavior. */ + std::vector
rootAddresses(RR->topology->rootAddresses()); + std::vector< std::pair > surface(RR->sa->getReportedSurface()); + for(std::vector< std::pair >::const_iterator s(surface.begin());s!=surface.end();++s) { + bool alreadyHave = false; + for(std::vector::const_iterator p(dps.begin());p!=dps.end();++p) { + if (p->address() == s->second) { + alreadyHave = true; + break; + } + } + if ((!alreadyHave)&&(std::find(rootAddresses.begin(),rootAddresses.end(),s->first) == rootAddresses.end())) + dps.push_back(Path(s->second,0,Path::TRUST_NORMAL)); + } + #ifdef ZT_TRACE { std::string ps; diff --git a/node/SelfAwareness.hpp b/node/SelfAwareness.hpp index 4780fa5b..9fcefa62 100644 --- a/node/SelfAwareness.hpp +++ b/node/SelfAwareness.hpp @@ -29,6 +29,7 @@ #define ZT_SELFAWARENESS_HPP #include +#include #include "InetAddress.hpp" #include "Address.hpp" @@ -65,6 +66,19 @@ public: */ void clean(uint64_t now); + /** + * @return List of external surface addresses as reported by peers + */ + inline std::vector< std::pair > getReportedSurface() const + { + std::vector< std::pair > r; + Mutex::Lock _l(_phy_m); + r.reserve(_phy.size()); + for(std::map< PhySurfaceKey,PhySurfaceEntry >::const_iterator p(_phy.begin());p!=_phy.end();) + r.push_back(std::pair(p->first.reporter,p->second.mySurface)); + return r; + } + private: struct PhySurfaceKey { diff --git a/node/Topology.hpp b/node/Topology.hpp index c878bcc6..1c5cca00 100644 --- a/node/Topology.hpp +++ b/node/Topology.hpp @@ -86,7 +86,7 @@ public: /** * Get a peer from its address - * + * * @param zta ZeroTier address of peer * @return Peer or NULL if not found */ @@ -103,7 +103,7 @@ public: /** * Get the current favorite root server - * + * * @return Root server with lowest latency or NULL if none */ inline SharedPtr getBestRoot() @@ -113,11 +113,11 @@ public: /** * Get the best root server, avoiding root servers listed in an array - * + * * This will get the best root server (lowest latency, etc.) but will * try to avoid the listed root servers, only using them if no others * are available. - * + * * @param avoid Nodes to avoid * @param avoidCount Number of nodes to avoid * @param strictAvoid If false, consider avoided root servers anyway if no non-avoid root servers are available -- cgit v1.2.3 From fadb2919625f72b7f0fbff17478ac467a86e657b Mon Sep 17 00:00:00 2001 From: Adam Ierymenko Date: Mon, 27 Jul 2015 17:14:49 -0700 Subject: Fix infinite loop typo. --- node/SelfAwareness.hpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) (limited to 'node') diff --git a/node/SelfAwareness.hpp b/node/SelfAwareness.hpp index 9fcefa62..1b160a3f 100644 --- a/node/SelfAwareness.hpp +++ b/node/SelfAwareness.hpp @@ -72,10 +72,12 @@ public: inline std::vector< std::pair > getReportedSurface() const { std::vector< std::pair > r; - Mutex::Lock _l(_phy_m); - r.reserve(_phy.size()); - for(std::map< PhySurfaceKey,PhySurfaceEntry >::const_iterator p(_phy.begin());p!=_phy.end();) - r.push_back(std::pair(p->first.reporter,p->second.mySurface)); + { + Mutex::Lock _l(_phy_m); + r.reserve(_phy.size()); + for(std::map< PhySurfaceKey,PhySurfaceEntry >::const_iterator p(_phy.begin());p!=_phy.end();++p) + r.push_back(std::pair(p->first.reporter,p->second.mySurface)); + } return r; } -- cgit v1.2.3 From e99eda4a4a178bbdbb419791587b581431061439 Mon Sep 17 00:00:00 2001 From: Adam Ierymenko Date: Mon, 27 Jul 2015 17:28:13 -0700 Subject: Fix IP scoping bug, and disable remotely reported surface push... not helping. :( --- node/InetAddress.cpp | 2 +- node/Peer.cpp | 11 +++++------ 2 files changed, 6 insertions(+), 7 deletions(-) (limited to 'node') diff --git a/node/InetAddress.cpp b/node/InetAddress.cpp index 91bfbed6..1942c4cd 100644 --- a/node/InetAddress.cpp +++ b/node/InetAddress.cpp @@ -74,7 +74,7 @@ InetAddress::IpScope InetAddress::ipScope() const if ((ip & 0xfff00000) == 0xac100000) return IP_SCOPE_PRIVATE; // 172.16.0.0/12 break; case 0xc0: - if ((ip & 0xffff0000) == 0xc9a80000) return IP_SCOPE_PRIVATE; // 192.168.0.0/16 + if ((ip & 0xffff0000) == 0xc0a80000) return IP_SCOPE_PRIVATE; // 192.168.0.0/16 break; case 0xff: return IP_SCOPE_NONE; // 255.0.0.0/8 (broadcast, or unused/unusable) default: diff --git a/node/Peer.cpp b/node/Peer.cpp index 804e6e39..ab3d61a6 100644 --- a/node/Peer.cpp +++ b/node/Peer.cpp @@ -226,15 +226,13 @@ void Peer::doPingAndKeepalive(const RuntimeEnvironment *RR,uint64_t now) void Peer::pushDirectPaths(const RuntimeEnvironment *RR,RemotePath *path,uint64_t now,bool force) { - if ((((now - _lastDirectPathPush) >= ZT_DIRECT_PATH_PUSH_INTERVAL)||(force))) { + if ((true)||(((now - _lastDirectPathPush) >= ZT_DIRECT_PATH_PUSH_INTERVAL)||(force))) { _lastDirectPathPush = now; std::vector dps(RR->node->directPaths()); - /* Also push paths reported to us by non-root-server peers. This assists - * with NAT traversal behind NATs that engage in strange or randomized - * port assignment behavior. */ - std::vector
rootAddresses(RR->topology->rootAddresses()); + // Push peer-reported surface -- tried this and it didn't help much with difficult NATs so commenting out. + /* std::vector< std::pair > surface(RR->sa->getReportedSurface()); for(std::vector< std::pair >::const_iterator s(surface.begin());s!=surface.end();++s) { bool alreadyHave = false; @@ -244,9 +242,10 @@ void Peer::pushDirectPaths(const RuntimeEnvironment *RR,RemotePath *path,uint64_ break; } } - if ((!alreadyHave)&&(std::find(rootAddresses.begin(),rootAddresses.end(),s->first) == rootAddresses.end())) + if (!alreadyHave) dps.push_back(Path(s->second,0,Path::TRUST_NORMAL)); } + */ #ifdef ZT_TRACE { -- cgit v1.2.3 From 821f1f366e468d1ff2f45a9e871ccec04b80fd3f Mon Sep 17 00:00:00 2001 From: Adam Ierymenko Date: Mon, 27 Jul 2015 17:34:58 -0700 Subject: Fix to NAT escalation sequence. --- node/Switch.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'node') diff --git a/node/Switch.cpp b/node/Switch.cpp index 3d9ef5b1..18935ce5 100644 --- a/node/Switch.cpp +++ b/node/Switch.cpp @@ -470,7 +470,7 @@ unsigned long Switch::doTimerTasks(uint64_t now) if (p < 0xffff) { tmpaddr.setPort((unsigned int)p); qi->peer->attemptToContactAt(RR,tmpaddr,now); - } else qi->strategyIteration = 9; + } else qi->strategyIteration = 5; } else { // All strategies tried, expire entry _contactQueue.erase(qi++); -- cgit v1.2.3 From dda376c9eb0800b824f423db30399d93e89cb162 Mon Sep 17 00:00:00 2001 From: Adam Ierymenko Date: Tue, 28 Jul 2015 11:16:43 -0700 Subject: Nuke some abandoned code. --- node/Peer.cpp | 16 ---------------- node/SelfAwareness.hpp | 15 --------------- 2 files changed, 31 deletions(-) (limited to 'node') diff --git a/node/Peer.cpp b/node/Peer.cpp index ab3d61a6..2bfd421f 100644 --- a/node/Peer.cpp +++ b/node/Peer.cpp @@ -231,22 +231,6 @@ void Peer::pushDirectPaths(const RuntimeEnvironment *RR,RemotePath *path,uint64_ std::vector dps(RR->node->directPaths()); - // Push peer-reported surface -- tried this and it didn't help much with difficult NATs so commenting out. - /* - std::vector< std::pair > surface(RR->sa->getReportedSurface()); - for(std::vector< std::pair >::const_iterator s(surface.begin());s!=surface.end();++s) { - bool alreadyHave = false; - for(std::vector::const_iterator p(dps.begin());p!=dps.end();++p) { - if (p->address() == s->second) { - alreadyHave = true; - break; - } - } - if (!alreadyHave) - dps.push_back(Path(s->second,0,Path::TRUST_NORMAL)); - } - */ - #ifdef ZT_TRACE { std::string ps; diff --git a/node/SelfAwareness.hpp b/node/SelfAwareness.hpp index 1b160a3f..4eede592 100644 --- a/node/SelfAwareness.hpp +++ b/node/SelfAwareness.hpp @@ -66,21 +66,6 @@ public: */ void clean(uint64_t now); - /** - * @return List of external surface addresses as reported by peers - */ - inline std::vector< std::pair > getReportedSurface() const - { - std::vector< std::pair > r; - { - Mutex::Lock _l(_phy_m); - r.reserve(_phy.size()); - for(std::map< PhySurfaceKey,PhySurfaceEntry >::const_iterator p(_phy.begin());p!=_phy.end();++p) - r.push_back(std::pair(p->first.reporter,p->second.mySurface)); - } - return r; - } - private: struct PhySurfaceKey { -- cgit v1.2.3 From b31071463cafda54afbf6f01d37aa7451b217b12 Mon Sep 17 00:00:00 2001 From: Adam Ierymenko Date: Tue, 28 Jul 2015 11:28:47 -0700 Subject: Try another NAT traversal improvement. --- node/Constants.hpp | 3 +++ node/IncomingPacket.cpp | 2 +- node/SelfAwareness.cpp | 15 +++++++++++++++ node/SelfAwareness.hpp | 5 +++++ node/Switch.cpp | 19 ++++++++++++++----- node/Switch.hpp | 4 ++-- 6 files changed, 40 insertions(+), 8 deletions(-) (limited to 'node') diff --git a/node/Constants.hpp b/node/Constants.hpp index c192381c..31a4c313 100644 --- a/node/Constants.hpp +++ b/node/Constants.hpp @@ -296,6 +296,9 @@ /** * Delay between initial direct NAT-t packet and more aggressive techniques + * + * This may also be a delay before sending the first packet if we determine + * that we should wait for the remote to initiate rendezvous first. */ #define ZT_NAT_T_TACTICAL_ESCALATION_DELAY 1000 diff --git a/node/IncomingPacket.cpp b/node/IncomingPacket.cpp index 76c47933..b1fda8ef 100644 --- a/node/IncomingPacket.cpp +++ b/node/IncomingPacket.cpp @@ -488,7 +488,7 @@ bool IncomingPacket::_doRENDEZVOUS(const RuntimeEnvironment *RR,const SharedPtr< InetAddress atAddr(field(ZT_PROTO_VERB_RENDEZVOUS_IDX_ADDRESS,addrlen),addrlen,port); TRACE("RENDEZVOUS from %s says %s might be at %s, starting NAT-t",peer->address().toString().c_str(),with.toString().c_str(),atAddr.toString().c_str()); peer->received(RR,_remoteAddress,hops(),packetId(),Packet::VERB_RENDEZVOUS,0,Packet::VERB_NOP); - RR->sw->contact(withPeer,atAddr); + RR->sw->rendezvous(withPeer,atAddr); } else { TRACE("dropped corrupt RENDEZVOUS from %s(%s) (bad address or port)",peer->address().toString().c_str(),_remoteAddress.toString().c_str()); } diff --git a/node/SelfAwareness.cpp b/node/SelfAwareness.cpp index 00015788..716cf7f3 100644 --- a/node/SelfAwareness.cpp +++ b/node/SelfAwareness.cpp @@ -147,4 +147,19 @@ void SelfAwareness::clean(uint64_t now) } } +bool SelfAwareness::areGlobalIPv4PortsRandomized() const +{ + int port = 0; + Mutex::Lock _l(_phy_m); + for(std::map< PhySurfaceKey,PhySurfaceEntry >::const_iterator p(_phy.begin());p!=_phy.end();++p) { + if ((p->first.scope == InetAddress::IP_SCOPE_GLOBAL)&&(p->second.mySurface.ss_family == AF_INET)) { + const int tmp = (int)p->second.mySurface.port(); + if ((port)&&(tmp != port)) + return true; + else port = tmp; + } + } + return false; +} + } // namespace ZeroTier diff --git a/node/SelfAwareness.hpp b/node/SelfAwareness.hpp index 4eede592..d3b79d18 100644 --- a/node/SelfAwareness.hpp +++ b/node/SelfAwareness.hpp @@ -66,6 +66,11 @@ public: */ void clean(uint64_t now); + /** + * @return True if our external (global scope) IPv4 ports appear to be randomized by a NAT device + */ + bool areGlobalIPv4PortsRandomized() const; + private: struct PhySurfaceKey { diff --git a/node/Switch.cpp b/node/Switch.cpp index 18935ce5..a580078e 100644 --- a/node/Switch.cpp +++ b/node/Switch.cpp @@ -43,6 +43,7 @@ #include "Topology.hpp" #include "Peer.hpp" #include "AntiRecursion.hpp" +#include "SelfAwareness.hpp" #include "Packet.hpp" namespace ZeroTier { @@ -385,15 +386,23 @@ bool Switch::unite(const Address &p1,const Address &p2,bool force) return true; } -void Switch::contact(const SharedPtr &peer,const InetAddress &atAddr) +void Switch::rendezvous(const SharedPtr &peer,const InetAddress &atAddr) { TRACE("sending NAT-t message to %s(%s)",peer->address().toString().c_str(),atAddr.toString().c_str()); const uint64_t now = RR->node->now(); - // Attempt to contact directly - peer->attemptToContactAt(RR,atAddr,now); - - // If we have not punched through after this timeout, open refreshing can of whupass + /* Attempt direct contact now unless we are IPv4 and our external ports + * appear to be randomized by a NAT device. In that case, we should let + * the other side send a message first. Why? If the other side is also + * randomized and symmetric, we are probably going to fail. But if the + * other side is "port restricted" but otherwise sane, us sending a + * packet first may actually close the remote's outgoing port to us! + * This assists with NAT-t in cases where one side is symmetric and the + * other is full cone but port restricted. */ + if ((atAddr.ss_family != AF_INET)||(!RR->sa->areGlobalIPv4PortsRandomized())) + peer->attemptToContactAt(RR,atAddr,now); + + // After 1s, try again and perhaps try more NAT-t strategies { Mutex::Lock _l(_contactQueue_m); _contactQueue.push_back(ContactQueueEntry(peer,now + ZT_NAT_T_TACTICAL_ESCALATION_DELAY,atAddr)); diff --git a/node/Switch.hpp b/node/Switch.hpp index e7f1523a..ac85606e 100644 --- a/node/Switch.hpp +++ b/node/Switch.hpp @@ -136,12 +136,12 @@ public: bool unite(const Address &p1,const Address &p2,bool force); /** - * Send NAT traversal messages to peer at the given candidate address + * Attempt NAT traversal to peer at a given physical address * * @param peer Peer to contact * @param atAddr Address of peer */ - void contact(const SharedPtr &peer,const InetAddress &atAddr); + void rendezvous(const SharedPtr &peer,const InetAddress &atAddr); /** * Request WHOIS on a given address -- cgit v1.2.3 From 17bfd4d55e96390147e2804b81c08985816ac4cd Mon Sep 17 00:00:00 2001 From: Adam Ierymenko Date: Tue, 28 Jul 2015 11:32:34 -0700 Subject: Add TRACE for NAT-t debugging. --- node/Switch.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'node') diff --git a/node/Switch.cpp b/node/Switch.cpp index a580078e..6f4659d5 100644 --- a/node/Switch.cpp +++ b/node/Switch.cpp @@ -399,8 +399,11 @@ void Switch::rendezvous(const SharedPtr &peer,const InetAddress &atAddr) * packet first may actually close the remote's outgoing port to us! * This assists with NAT-t in cases where one side is symmetric and the * other is full cone but port restricted. */ - if ((atAddr.ss_family != AF_INET)||(!RR->sa->areGlobalIPv4PortsRandomized())) + if ((atAddr.ss_family != AF_INET)||(!RR->sa->areGlobalIPv4PortsRandomized())) { peer->attemptToContactAt(RR,atAddr,now); + } else { + TRACE("behind randomizing symmetric NAT -- delaying initial message to %s(%s)",peer->address().toString().c_str(),atAddr.toString().c_str()); + } // After 1s, try again and perhaps try more NAT-t strategies { -- cgit v1.2.3 From 708aac1ea73a01fd81997a7215824dab832ba3d3 Mon Sep 17 00:00:00 2001 From: Adam Ierymenko Date: Tue, 28 Jul 2015 11:43:09 -0700 Subject: Remove some left over debug code, and fix attempt to send to self if we are an active bridge. --- node/Multicaster.cpp | 16 ++++++++++------ node/Peer.cpp | 2 +- 2 files changed, 11 insertions(+), 7 deletions(-) (limited to 'node') diff --git a/node/Multicaster.cpp b/node/Multicaster.cpp index 33424e4a..489c170b 100644 --- a/node/Multicaster.cpp +++ b/node/Multicaster.cpp @@ -211,9 +211,11 @@ void Multicaster::send( unsigned int count = 0; for(std::vector
::const_iterator ast(alwaysSendTo.begin());ast!=alwaysSendTo.end();++ast) { - out.sendOnly(RR,*ast); - if (++count >= limit) - break; + if (*ast != RR->identity.address()) { + out.sendOnly(RR,*ast); + if (++count >= limit) + break; + } } unsigned long idx = 0; @@ -264,9 +266,11 @@ void Multicaster::send( unsigned int count = 0; for(std::vector
::const_iterator ast(alwaysSendTo.begin());ast!=alwaysSendTo.end();++ast) { - out.sendAndLog(RR,*ast); - if (++count >= limit) - break; + if (*ast != RR->identity.address()) { + out.sendAndLog(RR,*ast); + if (++count >= limit) + break; + } } unsigned long idx = 0; diff --git a/node/Peer.cpp b/node/Peer.cpp index 2bfd421f..3cf0ec4e 100644 --- a/node/Peer.cpp +++ b/node/Peer.cpp @@ -226,7 +226,7 @@ void Peer::doPingAndKeepalive(const RuntimeEnvironment *RR,uint64_t now) void Peer::pushDirectPaths(const RuntimeEnvironment *RR,RemotePath *path,uint64_t now,bool force) { - if ((true)||(((now - _lastDirectPathPush) >= ZT_DIRECT_PATH_PUSH_INTERVAL)||(force))) { + if (((now - _lastDirectPathPush) >= ZT_DIRECT_PATH_PUSH_INTERVAL)||(force)) { _lastDirectPathPush = now; std::vector dps(RR->node->directPaths()); -- cgit v1.2.3 From b69afa010ee04924b36cd9b605e8f37ed917f0b2 Mon Sep 17 00:00:00 2001 From: Adam Ierymenko Date: Tue, 28 Jul 2015 11:50:01 -0700 Subject: Disable type punning on ARM by ifdef. --- node/Constants.hpp | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'node') diff --git a/node/Constants.hpp b/node/Constants.hpp index 31a4c313..901237ce 100644 --- a/node/Constants.hpp +++ b/node/Constants.hpp @@ -60,6 +60,13 @@ #include #endif +// Disable type punning on ARM architecture -- some ARM chips throw SIGBUS on unaligned access +#if defined(__arm__) || defined(__ARMEL__) +#ifndef ZT_NO_TYPE_PUNNING +#define ZT_NO_TYPE_PUNNING +#endif +#endif + #if defined(__FreeBSD__) || defined(__OpenBSD__) #ifndef __UNIX_LIKE__ #define __UNIX_LIKE__ -- cgit v1.2.3 From d2bfdfa6e79e54ba1d5127a75a56f7ec57415cf9 Mon Sep 17 00:00:00 2001 From: Adam Ierymenko Date: Tue, 28 Jul 2015 11:57:18 -0700 Subject: Play with NAT-t tweaks some more. --- node/Switch.cpp | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) (limited to 'node') diff --git a/node/Switch.cpp b/node/Switch.cpp index 6f4659d5..247b2d18 100644 --- a/node/Switch.cpp +++ b/node/Switch.cpp @@ -391,24 +391,15 @@ void Switch::rendezvous(const SharedPtr &peer,const InetAddress &atAddr) TRACE("sending NAT-t message to %s(%s)",peer->address().toString().c_str(),atAddr.toString().c_str()); const uint64_t now = RR->node->now(); - /* Attempt direct contact now unless we are IPv4 and our external ports - * appear to be randomized by a NAT device. In that case, we should let - * the other side send a message first. Why? If the other side is also - * randomized and symmetric, we are probably going to fail. But if the - * other side is "port restricted" but otherwise sane, us sending a - * packet first may actually close the remote's outgoing port to us! - * This assists with NAT-t in cases where one side is symmetric and the - * other is full cone but port restricted. */ - if ((atAddr.ss_family != AF_INET)||(!RR->sa->areGlobalIPv4PortsRandomized())) { + if ((atAddr.ss_family == AF_INET)&&(RR->sa->areGlobalIPv4PortsRandomized())) { peer->attemptToContactAt(RR,atAddr,now); } else { TRACE("behind randomizing symmetric NAT -- delaying initial message to %s(%s)",peer->address().toString().c_str(),atAddr.toString().c_str()); } - // After 1s, try again and perhaps try more NAT-t strategies { Mutex::Lock _l(_contactQueue_m); - _contactQueue.push_back(ContactQueueEntry(peer,now + ZT_NAT_T_TACTICAL_ESCALATION_DELAY,atAddr)); + _contactQueue.push_back(ContactQueueEntry(peer,now + (ZT_NAT_T_TACTICAL_ESCALATION_DELAY / 2),atAddr)); } } @@ -473,10 +464,10 @@ unsigned long Switch::doTimerTasks(uint64_t now) continue; } else { if (qi->strategyIteration == 0) { - // First strategy: send packet directly (we already tried this but try again) + // First strategy: send packet directly to destination qi->peer->attemptToContactAt(RR,qi->inaddr,now); } else if (qi->strategyIteration <= 4) { - // Strategies 1-4: try escalating ports + // Strategies 1-4: try escalating ports for symmetric NATs that remap sequentially InetAddress tmpaddr(qi->inaddr); int p = (int)qi->inaddr.port() + qi->strategyIteration; if (p < 0xffff) { -- cgit v1.2.3 From 4564dd95ffa287b3752a59378137c59773d51ef9 Mon Sep 17 00:00:00 2001 From: Adam Ierymenko Date: Tue, 28 Jul 2015 12:00:50 -0700 Subject: Revert... no luck with any of that. --- node/Switch.cpp | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) (limited to 'node') diff --git a/node/Switch.cpp b/node/Switch.cpp index 247b2d18..bf9308b0 100644 --- a/node/Switch.cpp +++ b/node/Switch.cpp @@ -390,16 +390,10 @@ void Switch::rendezvous(const SharedPtr &peer,const InetAddress &atAddr) { TRACE("sending NAT-t message to %s(%s)",peer->address().toString().c_str(),atAddr.toString().c_str()); const uint64_t now = RR->node->now(); - - if ((atAddr.ss_family == AF_INET)&&(RR->sa->areGlobalIPv4PortsRandomized())) { - peer->attemptToContactAt(RR,atAddr,now); - } else { - TRACE("behind randomizing symmetric NAT -- delaying initial message to %s(%s)",peer->address().toString().c_str(),atAddr.toString().c_str()); - } - + peer->attemptToContactAt(RR,atAddr,now); { Mutex::Lock _l(_contactQueue_m); - _contactQueue.push_back(ContactQueueEntry(peer,now + (ZT_NAT_T_TACTICAL_ESCALATION_DELAY / 2),atAddr)); + _contactQueue.push_back(ContactQueueEntry(peer,now + ZT_NAT_T_TACTICAL_ESCALATION_DELAY,atAddr)); } } -- cgit v1.2.3 From 5986d837384d5794286ee6954bc433eb0d7cc668 Mon Sep 17 00:00:00 2001 From: Adam Ierymenko Date: Tue, 28 Jul 2015 12:04:14 -0700 Subject: Kill more kittens. --- node/Constants.hpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'node') diff --git a/node/Constants.hpp b/node/Constants.hpp index 901237ce..b7aa9817 100644 --- a/node/Constants.hpp +++ b/node/Constants.hpp @@ -253,10 +253,10 @@ /** * How frequently to send a zero-byte UDP keepalive packet * - * There are NATs with timeouts as short as 30 seconds, so this turns out + * There are NATs with timeouts as short as 20 seconds, so this turns out * to be needed. */ -#define ZT_NAT_KEEPALIVE_DELAY 25000 +#define ZT_NAT_KEEPALIVE_DELAY 19000 /** * Delay between scans of the topology active peer DB for peers that need ping -- cgit v1.2.3 From 21e6850722539d4ff72b6c5841da47356233bb67 Mon Sep 17 00:00:00 2001 From: Adam Ierymenko Date: Tue, 28 Jul 2015 12:18:59 -0700 Subject: Cancel NAT-t attempts if peer is no longer "alive" --- node/Switch.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'node') diff --git a/node/Switch.cpp b/node/Switch.cpp index bf9308b0..02881331 100644 --- a/node/Switch.cpp +++ b/node/Switch.cpp @@ -452,8 +452,8 @@ unsigned long Switch::doTimerTasks(uint64_t now) Mutex::Lock _l(_contactQueue_m); for(std::list::iterator qi(_contactQueue.begin());qi!=_contactQueue.end();) { if (now >= qi->fireAtTime) { - if (qi->peer->hasActiveDirectPath(now)) { - // We've successfully NAT-t'd, so cancel attempt + if ((!qi->peer->alive(now))||(qi->peer->hasActiveDirectPath(now))) { + // Cancel attempt if we've already connected or peer is no longer "alive" _contactQueue.erase(qi++); continue; } else { -- cgit v1.2.3 From eea8d58afa6ceb0fd04d6d82c551ff9f81a0ffe7 Mon Sep 17 00:00:00 2001 From: Adam Ierymenko Date: Tue, 28 Jul 2015 12:39:03 -0700 Subject: docs,cleanup --- node/Switch.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'node') diff --git a/node/Switch.cpp b/node/Switch.cpp index 02881331..989f497a 100644 --- a/node/Switch.cpp +++ b/node/Switch.cpp @@ -448,7 +448,7 @@ unsigned long Switch::doTimerTasks(uint64_t now) { unsigned long nextDelay = 0xffffffff; // ceiling delay, caller will cap to minimum - { + { // Iterate through NAT traversal strategies for entries in contact queue Mutex::Lock _l(_contactQueue_m); for(std::list::iterator qi(_contactQueue.begin());qi!=_contactQueue.end();) { if (now >= qi->fireAtTime) { -- cgit v1.2.3