Sanity checks on array sizes and fix a bug in IPv4 auto-assign.

4 files changed, 82 insertions, 48 deletions

diff --git a/controller/DB.cpp b/controller/DB.cpp
index 7b792eb4..70472e57 100644
--- a/controller/DB.cpp
+++ b/controller/DB.cpp
@@ -373,8 +373,10 @@ void DB::_fillSummaryInfo(const std::shared_ptr<_Network> &nw,NetworkSummaryInfo
 {
 	for(auto ab=nw->activeBridgeMembers.begin();ab!=nw->activeBridgeMembers.end();++ab)
 		info.activeBridges.push_back(Address(*ab));
+	std::sort(info.activeBridges.begin(),info.activeBridges.end());
 	for(auto ip=nw->allocatedIps.begin();ip!=nw->allocatedIps.end();++ip)
 		info.allocatedIps.push_back(*ip);
+	std::sort(info.allocatedIps.begin(),info.allocatedIps.end());
 	info.authorizedMemberCount = (unsigned long)nw->authorizedMembers.size();
 	info.totalMemberCount = (unsigned long)nw->members.size();
 	info.mostRecentDeauthTime = nw->mostRecentDeauthTime;
diff --git a/controller/EmbeddedNetworkController.cpp b/controller/EmbeddedNetworkController.cpp
index e9cb41fc..3945c8c7 100644
--- a/controller/EmbeddedNetworkController.cpp
+++ b/controller/EmbeddedNetworkController.cpp
@@ -53,6 +53,9 @@ using json = nlohmann::json;
 // Min duration between requests for an address/nwid combo to prevent floods
 #define ZT_NETCONF_MIN_REQUEST_PERIOD 1000
 
+// Global maximum size of arrays in JSON objects
+#define ZT_CONTROLLER_MAX_ARRAY_SIZE 16384
+
 namespace ZeroTier {
 
 namespace {
@@ -686,6 +689,8 @@ unsigned int EmbeddedNetworkController::handleControlPlaneHttpPOST(
 									if ((ip.ss_family == AF_INET)||(ip.ss_family == AF_INET6)) {
 										char tmpip[64];
 										mipa.push_back(ip.toIpString(tmpip));
+										if (mipa.size() >= ZT_CONTROLLER_MAX_ARRAY_SIZE)
+											break;
 									}
 								}
 								member["ipAssignments"] = mipa;
@@ -707,6 +712,8 @@ unsigned int EmbeddedNetworkController::handleControlPlaneHttpPOST(
 									ta.push_back(t->first);
 									ta.push_back(t->second);
 									mtagsa.push_back(ta);
+									if (mtagsa.size() >= ZT_CONTROLLER_MAX_ARRAY_SIZE)
+										break;
 								}
 								member["tags"] = mtagsa;
 							}
@@ -718,6 +725,8 @@ unsigned int EmbeddedNetworkController::handleControlPlaneHttpPOST(
 								json mcaps = json::array();
 								for(unsigned long i=0;i<capabilities.size();++i) {
 									mcaps.push_back(OSUtils::jsonInt(capabilities[i],0ULL));
+									if (mcaps.size() >= ZT_CONTROLLER_MAX_ARRAY_SIZE)
+										break;
 								}
 								std::sort(mcaps.begin(),mcaps.end());
 								mcaps.erase(std::unique(mcaps.begin(),mcaps.end()),mcaps.end());
@@ -850,6 +859,8 @@ unsigned int EmbeddedNetworkController::handleControlPlaneHttpPOST(
 												tmp["via"] = v.toIpString(tmp2);
 											else tmp["via"] = json();
 											nrts.push_back(tmp);
+											if (nrts.size() >= ZT_CONTROLLER_MAX_ARRAY_SIZE)
+												break;
 										}
 									}
 								}
@@ -873,6 +884,8 @@ unsigned int EmbeddedNetworkController::handleControlPlaneHttpPOST(
 										tmp["ipRangeStart"] = f.toIpString(tmp2);
 										tmp["ipRangeEnd"] = t.toIpString(tmp2);
 										nipp.push_back(tmp);
+										if (nipp.size() >= ZT_CONTROLLER_MAX_ARRAY_SIZE)
+											break;
 									}
 								}
 							}
@@ -888,8 +901,11 @@ unsigned int EmbeddedNetworkController::handleControlPlaneHttpPOST(
 								json &rule = rules[i];
 								if (rule.is_object()) {
 									ZT_VirtualNetworkRule ztr;
-									if (_parseRule(rule,ztr))
+									if (_parseRule(rule,ztr)) {
 										nrules.push_back(_renderRule(ztr));
+										if (nrules.size() >= ZT_CONTROLLER_MAX_ARRAY_SIZE)
+											break;
+									}
 								}
 							}
 							network["rules"] = nrules;
@@ -929,8 +945,11 @@ unsigned int EmbeddedNetworkController::handleControlPlaneHttpPOST(
 											json &rule = rules[i];
 											if (rule.is_object()) {
 												ZT_VirtualNetworkRule ztr;
-												if (_parseRule(rule,ztr))
+												if (_parseRule(rule,ztr)) {
 													nrules.push_back(_renderRule(ztr));
+													if (nrules.size() >= ZT_CONTROLLER_MAX_ARRAY_SIZE)
+														break;
+												}
 											}
 										}
 									}
@@ -941,8 +960,11 @@ unsigned int EmbeddedNetworkController::handleControlPlaneHttpPOST(
 							}
 
 							json ncapsa = json::array();
-							for(std::map< uint64_t,json >::iterator c(ncaps.begin());c!=ncaps.end();++c)
+							for(std::map< uint64_t,json >::iterator c(ncaps.begin());c!=ncaps.end();++c) {
 								ncapsa.push_back(c->second);
+								if (ncapsa.size() >= ZT_CONTROLLER_MAX_ARRAY_SIZE)
+									break;
+							}
 							network["capabilities"] = ncapsa;
 						}
 					}
@@ -966,8 +988,11 @@ unsigned int EmbeddedNetworkController::handleControlPlaneHttpPOST(
 							}
 
 							json ntagsa = json::array();
-							for(std::map< uint64_t,json >::iterator t(ntags.begin());t!=ntags.end();++t)
+							for(std::map< uint64_t,json >::iterator t(ntags.begin());t!=ntags.end();++t) {
 								ntagsa.push_back(t->second);
+								if (ntagsa.size() >= ZT_CONTROLLER_MAX_ARRAY_SIZE)
+									break;
+							}
 							network["tags"] = ntagsa;
 						}
 					}
@@ -1304,7 +1329,7 @@ void EmbeddedNetworkController::_request(
 		}
 	}
 
-	std::auto_ptr<NetworkConfig> nc(new NetworkConfig());
+	std::unique_ptr<NetworkConfig> nc(new NetworkConfig());
 
 	nc->networkId = nwid;
 	nc->type = OSUtils::jsonBool(network["private"],true) ? ZT_NETWORK_TYPE_PRIVATE : ZT_NETWORK_TYPE_PUBLIC;
@@ -1483,29 +1508,29 @@ void EmbeddedNetworkController::_request(
 	json ipAssignments = member["ipAssignments"]; // we want to make a copy
 	if (ipAssignments.is_array()) {
 		for(unsigned long i=0;i<ipAssignments.size();++i) {
-			if (!ipAssignments[i].is_string())
-				continue;
-			std::string ips = ipAssignments[i];
-			InetAddress ip(ips.c_str());
-
-			// IP assignments are only pushed if there is a corresponding local route. We also now get the netmask bits from
-			// this route, ignoring the netmask bits field of the assigned IP itself. Using that was worthless and a source
-			// of user error / poor UX.
-			int routedNetmaskBits = 0;
-			for(unsigned int rk=0;rk<nc->routeCount;++rk) {
-				if ( (!nc->routes[rk].via.ss_family) && (reinterpret_cast<const InetAddress *>(&(nc->routes[rk].target))->containsAddress(ip)) )
-					routedNetmaskBits = reinterpret_cast<const InetAddress *>(&(nc->routes[rk].target))->netmaskBits();
-			}
+			if (ipAssignments[i].is_string()) {
+				const std::string ips = ipAssignments[i];
+				InetAddress ip(ips.c_str());
+
+				// IP assignments are only pushed if there is a corresponding local route. We also now get the netmask bits from
+				// this route, ignoring the netmask bits field of the assigned IP itself. Using that was worthless and a source
+				// of user error / poor UX.
+				int routedNetmaskBits = -1;
+				for(unsigned int rk=0;rk<nc->routeCount;++rk) {
+					if ( (!nc->routes[rk].via.ss_family) && (reinterpret_cast<const InetAddress *>(&(nc->routes[rk].target))->containsAddress(ip)) )
+						routedNetmaskBits = reinterpret_cast<const InetAddress *>(&(nc->routes[rk].target))->netmaskBits();
+				}
 
-			if (routedNetmaskBits > 0) {
-				if (nc->staticIpCount < ZT_MAX_ZT_ASSIGNED_ADDRESSES) {
-					ip.setPort(routedNetmaskBits);
-					nc->staticIps[nc->staticIpCount++] = ip;
+				if (routedNetmaskBits >= 0) {
+					if (nc->staticIpCount < ZT_MAX_ZT_ASSIGNED_ADDRESSES) {
+						ip.setPort(routedNetmaskBits);
+						nc->staticIps[nc->staticIpCount++] = ip;
+					}
+					if (ip.ss_family == AF_INET)
+						haveManagedIpv4AutoAssignment = true;
+					else if (ip.ss_family == AF_INET6)
+						haveManagedIpv6AutoAssignment = true;
 				}
-				if (ip.ss_family == AF_INET)
-					haveManagedIpv4AutoAssignment = true;
-				else if (ip.ss_family == AF_INET6)
-					haveManagedIpv6AutoAssignment = true;
 			}
 		}
 	} else {
@@ -1559,13 +1584,16 @@ void EmbeddedNetworkController::_request(
 						// If it's routed, then try to claim and assign it and if successful end loop
 						if ( (routedNetmaskBits > 0) && (!std::binary_search(ns.allocatedIps.begin(),ns.allocatedIps.end(),ip6)) ) {
 							char tmpip[64];
-							ipAssignments.push_back(ip6.toIpString(tmpip));
-							member["ipAssignments"] = ipAssignments;
-							ip6.setPort((unsigned int)routedNetmaskBits);
-							if (nc->staticIpCount < ZT_MAX_ZT_ASSIGNED_ADDRESSES)
-								nc->staticIps[nc->staticIpCount++] = ip6;
-							haveManagedIpv6AutoAssignment = true;
-							break;
+							const std::string ipStr(ip6.toIpString(tmpip));
+							if (std::find(ipAssignments.begin(),ipAssignments.end(),ipStr) == ipAssignments.end()) {
+								ipAssignments.push_back(ipStr);
+								member["ipAssignments"] = ipAssignments;
+								ip6.setPort((unsigned int)routedNetmaskBits);
+								if (nc->staticIpCount < ZT_MAX_ZT_ASSIGNED_ADDRESSES)
+									nc->staticIps[nc->staticIpCount++] = ip6;
+								haveManagedIpv6AutoAssignment = true;
+								break;
+							}
 						}
 					}
 				}
@@ -1612,16 +1640,19 @@ void EmbeddedNetworkController::_request(
 						const InetAddress ip4(Utils::hton(ip),0);
 						if ( (routedNetmaskBits > 0) && (!std::binary_search(ns.allocatedIps.begin(),ns.allocatedIps.end(),ip4)) ) {
 							char tmpip[64];
-							ipAssignments.push_back(ip4.toIpString(tmpip));
-							member["ipAssignments"] = ipAssignments;
-							if (nc->staticIpCount < ZT_MAX_ZT_ASSIGNED_ADDRESSES) {
-								struct sockaddr_in *const v4ip = reinterpret_cast<struct sockaddr_in *>(&(nc->staticIps[nc->staticIpCount++]));
-								v4ip->sin_family = AF_INET;
-								v4ip->sin_port = Utils::hton((uint16_t)routedNetmaskBits);
-								v4ip->sin_addr.s_addr = Utils::hton(ip);
+							const std::string ipStr(ip4.toIpString(tmpip));
+							if (std::find(ipAssignments.begin(),ipAssignments.end(),ipStr) == ipAssignments.end()) {
+								ipAssignments.push_back(ipStr);
+								member["ipAssignments"] = ipAssignments;
+								if (nc->staticIpCount < ZT_MAX_ZT_ASSIGNED_ADDRESSES) {
+									struct sockaddr_in *const v4ip = reinterpret_cast<struct sockaddr_in *>(&(nc->staticIps[nc->staticIpCount++]));
+									v4ip->sin_family = AF_INET;
+									v4ip->sin_port = Utils::hton((uint16_t)routedNetmaskBits);
+									v4ip->sin_addr.s_addr = Utils::hton(ip);
+								}
+								haveManagedIpv4AutoAssignment = true;
+								break;
 							}
-							haveManagedIpv4AutoAssignment = true;
-							break;
 						}
 					}
 				}
diff --git a/controller/RethinkDB.cpp b/controller/RethinkDB.cpp
index 3f730efc..a3bdfbc9 100644
--- a/controller/RethinkDB.cpp
+++ b/controller/RethinkDB.cpp
@@ -212,6 +212,7 @@ RethinkDB::RethinkDB(EmbeddedNetworkController *const nc,const Identity &myId,co
 					delete config;
 					if (!table)
 						continue;
+					const std::string jdump(OSUtils::jsonDump(record,-1));
 
 					while (_run == 1) {
 						try {
@@ -223,7 +224,7 @@ RethinkDB::RethinkDB(EmbeddedNetworkController *const nc,const Identity &myId,co
 									R::db(this->_db).table(table).get(deleteId).delete_().run(*rdb);
 								} else {
 									//printf("UPSERT: %s" ZT_EOL_S,record.dump().c_str());
-									R::db(this->_db).table(table).insert(R::Datum::from_json(OSUtils::jsonDump(record,-1)),R::optargs("conflict","update","return_changes",false)).run(*rdb);
+									R::db(this->_db).table(table).insert(R::Datum::from_json(jdump),R::optargs("conflict","update","return_changes",false)).run(*rdb);
 								}
 								break;
 							} else {
@@ -231,13 +232,13 @@ RethinkDB::RethinkDB(EmbeddedNetworkController *const nc,const Identity &myId,co
 								rdb.reset();
 							}
 						} catch (std::exception &e) {
-							fprintf(stderr,"[%s] ERROR: %.10llx controller RethinkDB (insert/update): %s" ZT_EOL_S,_timestr(),(unsigned long long)_myAddress.toInt(),e.what());
+							fprintf(stderr,"[%s] ERROR: %.10llx controller RethinkDB (insert/update): %s [%s]" ZT_EOL_S,_timestr(),(unsigned long long)_myAddress.toInt(),e.what(),jdump.c_str());
 							rdb.reset();
 						} catch (R::Error &e) {
-							fprintf(stderr,"[%s] ERROR: %.10llx controller RethinkDB (insert/update): %s" ZT_EOL_S,_timestr(),(unsigned long long)_myAddress.toInt(),e.message.c_str());
+							fprintf(stderr,"[%s] ERROR: %.10llx controller RethinkDB (insert/update): %s [%s]" ZT_EOL_S,_timestr(),(unsigned long long)_myAddress.toInt(),e.message.c_str(),jdump.c_str());
 							rdb.reset();
 						} catch ( ... ) {
-							fprintf(stderr,"[%s] ERROR: %.10llx controller RethinkDB (insert/update): unknown exception" ZT_EOL_S,_timestr(),(unsigned long long)_myAddress.toInt());
+							fprintf(stderr,"[%s] ERROR: %.10llx controller RethinkDB (insert/update): unknown exception [%s]" ZT_EOL_S,_timestr(),(unsigned long long)_myAddress.toInt(),jdump.c_str());
 							rdb.reset();
 						}
 						std::this_thread::sleep_for(std::chrono::milliseconds(250));
@@ -280,7 +281,7 @@ RethinkDB::RethinkDB(EmbeddedNetworkController *const nc,const Identity &myId,co
 							tmpobj["ts"] = i->second.first;
 							tmpobj["phy"] = i->second.second.toIpString(tmp2);
 							batch.emplace_back(tmpobj);
-							if (batch.size() >= 256) {
+							if (batch.size() >= 1024) {
 								R::db(this->_db).table("MemberStatus",R::optargs("read_mode","outdated")).insert(batch,R::optargs("conflict","update")).run(*rdb);
 								batch.clear();
 							}
diff --git a/controller/RethinkDB.hpp b/controller/RethinkDB.hpp
index ab2611c0..01b46a47 100644
--- a/controller/RethinkDB.hpp
+++ b/controller/RethinkDB.hpp
@@ -23,7 +23,7 @@
 
 #include "DB.hpp"
 
-#define ZT_CONTROLLER_RETHINKDB_COMMIT_THREADS 8
+#define ZT_CONTROLLER_RETHINKDB_COMMIT_THREADS 4
 
 namespace ZeroTier
 {