Do not bifurcate if not replacing an existing route. (Still need to tie up Linux and Windows.)

osdep/ManagedRoute.cpp

@@ -69,23 +69,28 @@ static void _forkTarget(const InetAddress &t,InetAddress &left,InetAddress &righ
 {
 	const unsigned int bits = t.netmaskBits() + 1;
 	left = t;
-	if ((t.ss_family == AF_INET)&&(bits <= 32)) {
-		left.setPort(bits);
-		right = t;
-		reinterpret_cast<struct sockaddr_in *>(&right)->sin_addr.s_addr ^= Utils::hton((uint32_t)(1 << (32 - bits)));
-		right.setPort(bits);
-	} else if ((t.ss_family == AF_INET6)&&(bits <= 128)) {
-		left.setPort(bits);
-		right = t;
-		uint8_t *b = reinterpret_cast<uint8_t *>(reinterpret_cast<struct sockaddr_in6 *>(&right)->sin6_addr.s6_addr);
-		b[bits / 8] ^= 1 << (8 - (bits % 8));
-		right.setPort(bits);
+	if (t.ss_family == AF_INET) {
+		if (bits <= 32) {
+			left.setPort(bits);
+			right = t;
+			reinterpret_cast<struct sockaddr_in *>(&right)->sin_addr.s_addr ^= Utils::hton((uint32_t)(1 << (32 - bits)));
+			right.setPort(bits);
+		} else {
+			right.zero();
+		}
+	} else if (t.ss_family == AF_INET6) {
+		if (bits <= 128) {
+			left.setPort(bits);
+			right = t;
+			uint8_t *b = reinterpret_cast<uint8_t *>(reinterpret_cast<struct sockaddr_in6 *>(&right)->sin6_addr.s6_addr);
+			b[bits / 8] ^= 1 << (8 - (bits % 8));
+			right.setPort(bits);
+		} else {
+			right.zero();
+		}
 	}
 }
 
-#ifdef __BSD__ // ------------------------------------------------------------
-#define ZT_ROUTING_SUPPORT_FOUND 1
-
 struct _RTE
 {
 	InetAddress target;
@@ -95,6 +100,9 @@ struct _RTE
 	bool ifscope;
 };
 
+#ifdef __BSD__ // ------------------------------------------------------------
+#define ZT_ROUTING_SUPPORT_FOUND 1
+
 static std::vector<_RTE> _getRTEs(const InetAddress &target,bool contains)
 {
 	std::vector<_RTE> rtes;
@@ -369,145 +377,161 @@ bool ManagedRoute::sync()
 		return false;
 #endif
 
-	if ((_target.isDefaultRoute())||((_target.ss_family == AF_INET)&&(_target.netmaskBits() < 32))) {
-		/* In ZeroTier we create two more specific routes for every one route. We
-		 * do this for default routes and IPv4 routes other than /32s. If there
-		 * is a pre-existing system route that this route will override, we create
-		 * two more specific interface-bound shadow routes for it.
-		 *
-		 * This means that ZeroTier can *itself* continue communicating over
-		 * whatever physical routes might be present while simultaneously
-		 * overriding them for general system traffic. This is mostly for
-		 * "full tunnel" VPN modes of operation, but might be useful for
-		 * virtualizing physical networks in a hybrid design as well. */
-
-		// Generate two more specific routes than target with one extra bit
-		InetAddress leftt,rightt;
-		_forkTarget(_target,leftt,rightt);
+	// Generate two more specific routes than target with one extra bit
+	InetAddress leftt,rightt;
+	_forkTarget(_target,leftt,rightt);
 
 #ifdef __BSD__ // ------------------------------------------------------------
 
-		// Find lowest metric system route that this route should override (if any)
-		InetAddress newSystemVia;
-		char newSystemDevice[128];
-		newSystemDevice[0] = (char)0;
-		int systemMetric = 9999999;
-		std::vector<_RTE> rtes(_getRTEs(_target,false));
-		for(std::vector<_RTE>::iterator r(rtes.begin());r!=rtes.end();++r) {
-			if (r->via) {
-				if ((!newSystemVia)||(r->metric < systemMetric)) {
-					newSystemVia = r->via;
-					Utils::scopy(newSystemDevice,sizeof(newSystemDevice),r->device);
-					systemMetric = r->metric;
-				}
+	// Find lowest metric system route that this route should override (if any)
+	InetAddress newSystemVia;
+	char newSystemDevice[128];
+	newSystemDevice[0] = (char)0;
+	int systemMetric = 9999999;
+	std::vector<_RTE> rtes(_getRTEs(_target,false));
+	for(std::vector<_RTE>::iterator r(rtes.begin());r!=rtes.end();++r) {
+		if (r->via) {
+			if ( ((!newSystemVia)||(r->metric < systemMetric)) && (strcmp(r->device,_device) != 0) ) {
+				newSystemVia = r->via;
+				Utils::scopy(newSystemDevice,sizeof(newSystemDevice),r->device);
+				systemMetric = r->metric;
 			}
 		}
-		if ((newSystemVia)&&(!newSystemDevice[0])) {
-			rtes = _getRTEs(newSystemVia,true);
-			for(std::vector<_RTE>::iterator r(rtes.begin());r!=rtes.end();++r) {
-				if (r->device[0]) {
-					Utils::scopy(newSystemDevice,sizeof(newSystemDevice),r->device);
-					break;
-				}
+	}
+
+	// Get device corresponding to route if we don't have that already
+	if ((newSystemVia)&&(!newSystemDevice[0])) {
+		rtes = _getRTEs(newSystemVia,true);
+		for(std::vector<_RTE>::iterator r(rtes.begin());r!=rtes.end();++r) {
+			if ( (r->device[0]) && (strcmp(r->device,_device) != 0) ) {
+				Utils::scopy(newSystemDevice,sizeof(newSystemDevice),r->device);
+				break;
 			}
 		}
-
-		// Shadow system route if it exists, also delete any obsolete shadows
-		// and replace them with the new state. sync() is called periodically to
-		// allow us to do that if underlying connectivity changes.
-		if ( ((_systemVia != newSystemVia)||(strcmp(_systemDevice,newSystemDevice))) && (strcmp(_device,newSystemDevice)) ) {
-			if ((_systemVia)&&(_systemDevice[0])) {
-				_routeCmd("delete",leftt,_systemVia,_systemDevice,(const char *)0);
+	}
+	if (!newSystemDevice[0])
+		newSystemVia.zero();
+
+	// Shadow system route if it exists, also delete any obsolete shadows
+	// and replace them with the new state. sync() is called periodically to
+	// allow us to do that if underlying connectivity changes.
+	if ((_systemVia != newSystemVia)||(strcmp(_systemDevice,newSystemDevice) != 0)) {
+		if (_systemVia) {
+			_routeCmd("delete",leftt,_systemVia,_systemDevice,(const char *)0);
+			if (rightt)
 				_routeCmd("delete",rightt,_systemVia,_systemDevice,(const char *)0);
-			}
+		}
 
-			_systemVia = newSystemVia;
-			Utils::scopy(_systemDevice,sizeof(_systemDevice),newSystemDevice);
+		_systemVia = newSystemVia;
+		Utils::scopy(_systemDevice,sizeof(_systemDevice),newSystemDevice);
 
-			if ((_systemVia)&&(_systemDevice[0])) {
-				_routeCmd("add",leftt,_systemVia,_systemDevice,(const char *)0);
-				_routeCmd("change",leftt,_systemVia,_systemDevice,(const char *)0);
+		if (_systemVia) {
+			_routeCmd("add",leftt,_systemVia,_systemDevice,(const char *)0);
+			_routeCmd("change",leftt,_systemVia,_systemDevice,(const char *)0);
+			if (rightt) {
 				_routeCmd("add",rightt,_systemVia,_systemDevice,(const char *)0);
 				_routeCmd("change",rightt,_systemVia,_systemDevice,(const char *)0);
 			}
 		}
+	}
 
-		// Apply overriding non-device-scoped routes
-		if (!_applied) {
-			if (_via) {
-				_routeCmd("add",leftt,_via,(const char *)0,(const char *)0);
-				_routeCmd("change",leftt,_via,(const char *)0,(const char *)0);
-				_routeCmd("add",rightt,_via,(const char *)0,(const char *)0);
-				_routeCmd("change",rightt,_via,(const char *)0,(const char *)0);
-			} else if (_device[0]) {
-				_routeCmd("add",leftt,_via,(const char *)0,_device);
-				_routeCmd("change",leftt,_via,(const char *)0,_device);
-				_routeCmd("add",rightt,_via,(const char *)0,_device);
-				_routeCmd("change",rightt,_via,(const char *)0,_device);
-			}
-
-			_applied = true;
+	if (_systemVia) {
+		if (!_applied.count(leftt)) {
+			_applied.insert(leftt);
+			_routeCmd("add",leftt,_via,(const char *)0,(_via) ? (const char *)0 : _device);
+			_routeCmd("change",leftt,_via,(const char *)0,(_via) ? (const char *)0 : _device);
 		}
+		if ((rightt)&&(!_applied.count(rightt))) {
+			_applied.insert(rightt);
+			_routeCmd("add",rightt,_via,(const char *)0,(_via) ? (const char *)0 : _device);
+			_routeCmd("change",rightt,_via,(const char *)0,(_via) ? (const char *)0 : _device);
+		}
+		if (_applied.count(_target)) {
+			_applied.erase(_target);
+			_routeCmd("delete",_target,_via,(const char *)0,(_via) ? (const char *)0 : _device);
+		}
+	} else {
+		if (_applied.count(leftt)) {
+			_applied.erase(leftt);
+			_routeCmd("delete",leftt,_via,(const char *)0,(_via) ? (const char *)0 : _device);
+		}
+		if ((rightt)&&(_applied.count(rightt))) {
+			_applied.erase(rightt);
+			_routeCmd("delete",rightt,_via,(const char *)0,(_via) ? (const char *)0 : _device);
+		}
+		if (!_applied.count(_target)) {
+			_applied.insert(_target);
+			_routeCmd("add",_target,_via,(const char *)0,(_via) ? (const char *)0 : _device);
+			_routeCmd("change",_target,_via,(const char *)0,(_via) ? (const char *)0 : _device);
+		}
+	}
 
 #endif // __BSD__ ------------------------------------------------------------
 
 #ifdef __LINUX__ // ----------------------------------------------------------
 
-		if (!_applied) {
-			_routeCmd("replace",leftt,_via,(_via) ? _device : (const char *)0);
-			_routeCmd("replace",rightt,_via,(_via) ? _device : (const char *)0);
-			_applied = true;
+	if (needBifurcation) {
+		if (!_applied.count(leftt)) {
+			_applied.insert(leftt);
+			_routeCmd("replace",leftt,_via,(_via) ? (const char *)0 : _device);
+		}
+		if ((rightt)&&(!_applied.count(rightt))) {
+			_applied.insert(rightt);
+			_routeCmd("replace",rightt,_via,(_via) ? (const char *)0 : _device);
+		}
+		if (_applied.count(_target)) {
+			_applied.erase(_target);
+			_routeCmd("del",_target,_via,(_via) ? (const char *)0 : _device);
+		}
+	} else {
+		if (_applied.count(leftt)) {
+			_applied.erase(leftt);
+			_routeCmd("del",leftt,_via,(_via) ? (const char *)0 : _device);
+		}
+		if ((rightt)&&(_applied.count(rightt))) {
+			_applied.erase(rightt);
+			_routeCmd("del",rightt,_via,(_via) ? (const char *)0 : _device);
+		}
+		if (!_applied.count(_target)) {
+			_applied.insert(_target);
+			_routeCmd("replace",_target,_via,(_via) ? (const char *)0 : _device);
 		}
+	}
 
 #endif // __LINUX__ ----------------------------------------------------------
 
 #ifdef __WINDOWS__ // --------------------------------------------------------
 
-		if (!_applied) {
+	if (needBifurcation) {
+		if (!_applied.count(leftt)) {
+			_applied.insert(leftt);
 			_winRoute(false,interfaceLuid,interfaceIndex,leftt,_via);
+		}
+		if ((rightt)&&(!_applied.count(rightt))) {
+			_applied.insert(rightt);
 			_winRoute(false,interfaceLuid,interfaceIndex,rightt,_via);
-			_applied = true;
 		}
-
-#endif // __WINDOWS__ --------------------------------------------------------
-
+		if (_applied.count(_target)) {
+			_applied.erase(_target);
+			_winRoute(true,interfaceLuid,interfaceIndex,_target,_via);
+		}
 	} else {
-
-#ifdef __BSD__ // ------------------------------------------------------------
-
-		if (!_applied) {
-			if (_via) {
-				_routeCmd("add",_target,_via,(const char *)0,(const char *)0);
-				_routeCmd("change",_target,_via,(const char *)0,(const char *)0);
-			} else if (_device[0]) {
-				_routeCmd("add",_target,_via,(const char *)0,_device);
-				_routeCmd("change",_target,_via,(const char *)0,_device);
-			}
-			_applied = true;
+		if (_applied.count(leftt)) {
+			_applied.erase(leftt);
+			_winRoute(true,interfaceLuid,interfaceIndex,leftt,_via);
 		}
-
-#endif // __BSD__ ------------------------------------------------------------
-
-#ifdef __LINUX__ // ----------------------------------------------------------
-
-		if (!_applied) {
-			_routeCmd("replace",_target,_via,(_via) ? _device : (const char *)0);
-			_applied = true;
+		if ((rightt)&&(_applied.count(rightt))) {
+			_applied.erase(rightt);
+			_winRoute(true,interfaceLuid,interfaceIndex,rightt,_via);
 		}
-
-#endif // __LINUX__ ----------------------------------------------------------
-
-#ifdef __WINDOWS__ // --------------------------------------------------------
-
-		if (!_applied) {
+		if (!_applied.count(_target)) {
+			_applied.insert(_target);
 			_winRoute(false,interfaceLuid,interfaceIndex,_target,_via);
-			_applied = true;
 		}
+	}
 
 #endif // __WINDOWS__ --------------------------------------------------------
 
-	}
-
 	return true;
 }
 
@@ -521,66 +545,28 @@ void ManagedRoute::remove()
 		return;
 #endif
 
-	if (_applied) {
-		if ((_target.isDefaultRoute())||((_target.ss_family == AF_INET)&&(_target.netmaskBits() < 32))) {
-			InetAddress leftt,rightt;
-			_forkTarget(_target,leftt,rightt);
-
-#ifdef __BSD__ // ------------------------------------------------------------
-
-			if ((_systemVia)&&(_systemDevice[0])) {
-				_routeCmd("delete",leftt,_systemVia,_systemDevice,(const char *)0);
-				_routeCmd("delete",rightt,_systemVia,_systemDevice,(const char *)0);
-			}
-			if (_via) {
-				_routeCmd("delete",leftt,_via,(const char *)0,(const char *)0);
-				_routeCmd("delete",rightt,_via,(const char *)0,(const char *)0);
-			} else if (_device[0]) {
-				_routeCmd("delete",leftt,_via,(const char *)0,_device);
-				_routeCmd("delete",rightt,_via,(const char *)0,_device);
-			}
-
+#ifdef __BSD__
+	if (_systemVia) {
+		InetAddress leftt,rightt;
+		_forkTarget(_target,leftt,rightt);
+		_routeCmd("delete",leftt,_systemVia,_systemDevice,(const char *)0);
+		if (rightt)
+			_routeCmd("delete",rightt,_systemVia,_systemDevice,(const char *)0);
+	}
 #endif // __BSD__ ------------------------------------------------------------
 
-#ifdef __LINUX__ // ----------------------------------------------------------
-
-			_routeCmd("del",leftt,_via,(_via) ? _device : (const char *)0);
-			_routeCmd("del",rightt,_via,(_via) ? _device : (const char *)0);
-
-#endif // __LINUX__ ----------------------------------------------------------
-
-#ifdef __WINDOWS__ // --------------------------------------------------------
-
-			_winRoute(true,interfaceLuid,interfaceIndex,leftt,_via);
-			_winRoute(true,interfaceLuid,interfaceIndex,rightt,_via);
-
-#endif // __WINDOWS__ --------------------------------------------------------
-
-		} else {
-
+	for(std::set<InetAddress>::iterator r(_applied.begin());r!=_applied.end();++r) {
 #ifdef __BSD__ // ------------------------------------------------------------
-
-		if (_via) {
-			_routeCmd("delete",_target,_via,(const char *)0,(const char *)0);
-		} else if (_device[0]) {
-			_routeCmd("delete",_target,_via,(const char *)0,_device);
-		}
-
+		_routeCmd("delete",*r,_via,(const char *)0,(_via) ? (const char *)0 : _device);
 #endif // __BSD__ ------------------------------------------------------------
 
 #ifdef __LINUX__ // ----------------------------------------------------------
-
-			_routeCmd("del",_target,_via,(_via) ? _device : (const char *)0);
-
+		_routeCmd("del",*r,_via,(_via) ? (const char *)0 : _device);
 #endif // __LINUX__ ----------------------------------------------------------
 
 #ifdef __WINDOWS__ // --------------------------------------------------------
-
-			_winRoute(true,interfaceLuid,interfaceIndex,_target,_via);
-
+		_winRoute(true,interfaceLuid,interfaceIndex,*r,_via);
 #endif // __WINDOWS__ --------------------------------------------------------
-
-		}
 	}
 
 	_target.zero();
@@ -588,7 +574,7 @@ void ManagedRoute::remove()
 	_systemVia.zero();
 	_device[0] = (char)0;
 	_systemDevice[0] = (char)0;
-	_applied = false;
+	_applied.clear();
 }
 
 } // namespace ZeroTier

osdep/ManagedRoute.hpp

@@ -9,6 +9,7 @@
 
 #include <stdexcept>
 #include <vector>
+#include <set>
 
 namespace ZeroTier {
 
@@ -22,7 +23,6 @@ public:
 	{
 		_device[0] = (char)0;
 		_systemDevice[0] = (char)0;
-		_applied = false;
 	}
 
 	~ManagedRoute()
@@ -32,15 +32,20 @@ public:
 
 	ManagedRoute(const ManagedRoute &r)
 	{
-		_applied = false;
 		*this = r;
 	}
 
 	inline ManagedRoute &operator=(const ManagedRoute &r)
 	{
-		if ((!_applied)&&(!r._applied)) {
-			memcpy(this,&r,sizeof(ManagedRoute)); // InetAddress is memcpy'able
+		if (_applied.size() == 0) {
+			_target = r._target;
+			_via = r._via;
+			_systemVia = r._systemVia;
+			_applied = r._applied;
+			Utils::scopy(_device,sizeof(_device),r._device);
+			Utils::scopy(_systemDevice,sizeof(_systemDevice),r._systemDevice);
 		} else {
+			// Sanity check -- this is an 'assert' to detect a bug
 			fprintf(stderr,"Applied ManagedRoute isn't copyable!\n");
 			abort();
 		}
@@ -65,6 +70,10 @@ public:
 		this->remove();
 		_target = target;
 		_via = via;
+		if (via.ss_family == AF_INET)
+			_via.setPort(32);
+		else if (via.ss_family == AF_INET6)
+			_via.setPort(128);
 		Utils::scopy(_device,sizeof(_device),device);
 		return this->sync();
 	}
@@ -93,13 +102,12 @@ public:
 	inline const char *device() const { return _device; }
 
 private:
-
 	InetAddress _target;
 	InetAddress _via;
 	InetAddress _systemVia; // for route overrides
+	std::set<InetAddress> _applied; // routes currently applied
 	char _device[128];
 	char _systemDevice[128]; // for route overrides
-	bool _applied;
 };
 
 } // namespace ZeroTier