Rule parse fix.

3 files changed, 78 insertions, 68 deletions

diff --git a/node/Capability.hpp b/node/Capability.hpp
index fd6ae091..c129485d 100644
--- a/node/Capability.hpp
+++ b/node/Capability.hpp
@@ -176,7 +176,6 @@ public:
 	template<unsigned int C>
 	static inline void serializeRules(Buffer<C> &b,const ZT_VirtualNetworkRule *rules,unsigned int ruleCount)
 	{
-		b.append((uint16_t)ruleCount);
 		for(unsigned int i=0;i<ruleCount;++i) {
 			// Each rule consists of its 8-bit type followed by the size of that type's
 			// field followed by field data. The inclusion of the size will allow non-supported
@@ -267,75 +266,73 @@ public:
 	template<unsigned int C>
 	static inline void deserializeRules(const Buffer<C> &b,unsigned int &p,ZT_VirtualNetworkRule *rules,unsigned int &ruleCount,const unsigned int maxRuleCount)
 	{
-		ruleCount = b.template at<uint16_t>(p); p += 2;
-		if (ruleCount > maxRuleCount)
-			throw std::runtime_error("rule count overflow");
-		for(unsigned int i=0;i<ruleCount;++i) {
-			rules[i].t = (uint8_t)b[p++];
+		while ((ruleCount < maxRuleCount)&&(p < b.size())) {
+			rules[ruleCount].t = (uint8_t)b[p++];
 			const unsigned int fieldLen = (unsigned int)b[p++];
-			switch((ZT_VirtualNetworkRuleType)(rules[i].t & 0x7f)) {
+			switch((ZT_VirtualNetworkRuleType)(rules[ruleCount].t & 0x7f)) {
 				default:
 					break;
 				case ZT_NETWORK_RULE_ACTION_TEE:
 				case ZT_NETWORK_RULE_ACTION_REDIRECT:
 				case ZT_NETWORK_RULE_MATCH_SOURCE_ZEROTIER_ADDRESS:
 				case ZT_NETWORK_RULE_MATCH_DEST_ZEROTIER_ADDRESS:
-					rules[i].v.zt = Address(b.field(p,ZT_ADDRESS_LENGTH),ZT_ADDRESS_LENGTH).toInt();
+					rules[ruleCount].v.zt = Address(b.field(p,ZT_ADDRESS_LENGTH),ZT_ADDRESS_LENGTH).toInt();
 					break;
 				case ZT_NETWORK_RULE_MATCH_VLAN_ID:
-					rules[i].v.vlanId = b.template at<uint16_t>(p);
+					rules[ruleCount].v.vlanId = b.template at<uint16_t>(p);
 					break;
 				case ZT_NETWORK_RULE_MATCH_VLAN_PCP:
-					rules[i].v.vlanPcp = (uint8_t)b[p];
+					rules[ruleCount].v.vlanPcp = (uint8_t)b[p];
 					break;
 				case ZT_NETWORK_RULE_MATCH_VLAN_DEI:
-					rules[i].v.vlanDei = (uint8_t)b[p];
+					rules[ruleCount].v.vlanDei = (uint8_t)b[p];
 					break;
 				case ZT_NETWORK_RULE_MATCH_ETHERTYPE:
-					rules[i].v.etherType = b.template at<uint16_t>(p);
+					rules[ruleCount].v.etherType = b.template at<uint16_t>(p);
 					break;
 				case ZT_NETWORK_RULE_MATCH_MAC_SOURCE:
 				case ZT_NETWORK_RULE_MATCH_MAC_DEST:
-					memcpy(rules[i].v.mac,b.field(p,6),6);
+					memcpy(rules[ruleCount].v.mac,b.field(p,6),6);
 					break;
 				case ZT_NETWORK_RULE_MATCH_IPV4_SOURCE:
 				case ZT_NETWORK_RULE_MATCH_IPV4_DEST:
-					memcpy(&(rules[i].v.ipv4.ip),b.field(p,4),4);
-					rules[i].v.ipv4.mask = (uint8_t)b[p + 4];
+					memcpy(&(rules[ruleCount].v.ipv4.ip),b.field(p,4),4);
+					rules[ruleCount].v.ipv4.mask = (uint8_t)b[p + 4];
 					break;
 				case ZT_NETWORK_RULE_MATCH_IPV6_SOURCE:
 				case ZT_NETWORK_RULE_MATCH_IPV6_DEST:
-					memcpy(rules[i].v.ipv6.ip,b.field(p,16),16);
-					rules[i].v.ipv6.mask = (uint8_t)b[p + 16];
+					memcpy(rules[ruleCount].v.ipv6.ip,b.field(p,16),16);
+					rules[ruleCount].v.ipv6.mask = (uint8_t)b[p + 16];
 					break;
 				case ZT_NETWORK_RULE_MATCH_IP_TOS:
-					rules[i].v.ipTos = (uint8_t)b[p];
+					rules[ruleCount].v.ipTos = (uint8_t)b[p];
 					break;
 				case ZT_NETWORK_RULE_MATCH_IP_PROTOCOL:
-					rules[i].v.ipProtocol = (uint8_t)b[p];
+					rules[ruleCount].v.ipProtocol = (uint8_t)b[p];
 					break;
 				case ZT_NETWORK_RULE_MATCH_IP_SOURCE_PORT_RANGE:
 				case ZT_NETWORK_RULE_MATCH_IP_DEST_PORT_RANGE:
-					rules[i].v.port[0] = b.template at<uint16_t>(p);
-					rules[i].v.port[1] = b.template at<uint16_t>(p + 2);
+					rules[ruleCount].v.port[0] = b.template at<uint16_t>(p);
+					rules[ruleCount].v.port[1] = b.template at<uint16_t>(p + 2);
 					break;
 				case ZT_NETWORK_RULE_MATCH_CHARACTERISTICS:
-					rules[i].v.characteristics[0] = b.template at<uint64_t>(p);
-					rules[i].v.characteristics[1] = b.template at<uint64_t>(p + 8);
+					rules[ruleCount].v.characteristics[0] = b.template at<uint64_t>(p);
+					rules[ruleCount].v.characteristics[1] = b.template at<uint64_t>(p + 8);
 					break;
 				case ZT_NETWORK_RULE_MATCH_FRAME_SIZE_RANGE:
-					rules[i].v.frameSize[0] = b.template at<uint16_t>(p);
-					rules[i].v.frameSize[0] = b.template at<uint16_t>(p + 2);
+					rules[ruleCount].v.frameSize[0] = b.template at<uint16_t>(p);
+					rules[ruleCount].v.frameSize[0] = b.template at<uint16_t>(p + 2);
 					break;
 				case ZT_NETWORK_RULE_MATCH_TAGS_SAMENESS:
 				case ZT_NETWORK_RULE_MATCH_TAGS_BITWISE_AND:
 				case ZT_NETWORK_RULE_MATCH_TAGS_BITWISE_OR:
 				case ZT_NETWORK_RULE_MATCH_TAGS_BITWISE_XOR:
-					rules[i].v.tag.id = b.template at<uint32_t>(p);
-					rules[i].v.tag.value = b.template at<uint32_t>(p + 4);
+					rules[ruleCount].v.tag.id = b.template at<uint32_t>(p);
+					rules[ruleCount].v.tag.value = b.template at<uint32_t>(p + 4);
 					break;
 			}
 			p += fieldLen;
+			++ruleCount;
 		}
 	}
 
@@ -350,6 +347,7 @@ public:
 		b.append(_expiration);
 		b.append(_id);
 
+		b.append((uint16_t)_ruleCount);
 		serializeRules(b,_rules,_ruleCount);
 		b.append((uint8_t)_maxCustodyChainLength);
 
@@ -387,7 +385,10 @@ public:
 		_expiration = b.template at<uint64_t>(p); p += 8;
 		_id = b.template at<uint32_t>(p); p += 4;
 
-		deserializeRules(b,p,_rules,_ruleCount,ZT_MAX_CAPABILITY_RULES);
+		const unsigned int rc = b.template at<uint16_t>(p); p += 2;
+		if (rc > ZT_MAX_CAPABILITY_RULES)
+			throw std::runtime_error("rule overflow");
+		deserializeRules(b,p,_rules,_ruleCount,rc);
 
 		_maxCustodyChainLength = (unsigned int)b[p++];
 		if ((_maxCustodyChainLength < 1)||(_maxCustodyChainLength > ZT_MAX_CAPABILITY_CUSTODY_CHAIN_LENGTH))
diff --git a/node/IncomingPacket.cpp b/node/IncomingPacket.cpp
index 53f6b88a..5c9e80f8 100644
--- a/node/IncomingPacket.cpp
+++ b/node/IncomingPacket.cpp
@@ -84,7 +84,7 @@ bool IncomingPacket::tryDecode(const RuntimeEnvironment *RR)
 			}
 
 			const Packet::Verb v = verb();
-			TRACE("<< %s from %s(%s)",Packet::verbString(v),sourceAddress.toString().c_str(),_remoteAddress.toString().c_str());
+			//TRACE("<< %s from %s(%s)",Packet::verbString(v),sourceAddress.toString().c_str(),_remoteAddress.toString().c_str());
 			switch(v) {
 				//case Packet::VERB_NOP:
 				default: // ignore unknown verbs, but if they pass auth check they are "received"
@@ -401,8 +401,9 @@ bool IncomingPacket::_doOK(const RuntimeEnvironment *RR,const SharedPtr<Peer> &p
 			} break;
 
 			case Packet::VERB_NETWORK_CONFIG_REQUEST: {
-				const SharedPtr<Network> nw(RR->node->network(at<uint64_t>(ZT_PROTO_VERB_NETWORK_CONFIG_REQUEST__OK__IDX_NETWORK_ID)));
-				if ((nw)&&(nw->controller() == peer->address())) {
+				const uint64_t nwid = at<uint64_t>(ZT_PROTO_VERB_NETWORK_CONFIG_REQUEST__OK__IDX_NETWORK_ID);
+				const SharedPtr<Network> network(RR->node->network(nwid));
+				if ((network)&&(network->controller() == peer->address())) {
 					const unsigned int chunkLen = at<uint16_t>(ZT_PROTO_VERB_NETWORK_CONFIG_REQUEST__OK__IDX_DICT_LEN);
 					const void *chunkData = field(ZT_PROTO_VERB_NETWORK_CONFIG_REQUEST__OK__IDX_DICT,chunkLen);
 					unsigned int chunkIndex = 0;
@@ -411,7 +412,8 @@ bool IncomingPacket::_doOK(const RuntimeEnvironment *RR,const SharedPtr<Peer> &p
 						totalSize = at<uint32_t>(ZT_PROTO_VERB_NETWORK_CONFIG_REQUEST__OK__IDX_DICT + chunkLen);
 						chunkIndex = at<uint32_t>(ZT_PROTO_VERB_NETWORK_CONFIG_REQUEST__OK__IDX_DICT + chunkLen + 4);
 					}
-					nw->handleInboundConfigChunk(inRePacketId,chunkData,chunkLen,chunkIndex,totalSize);
+					TRACE("%s(%s): OK(NETWORK_CONFIG_REQUEST) chunkLen==%u chunkIndex==%u totalSize==%u",source().toString().c_str(),_remoteAddress.toString().c_str(),chunkLen,chunkIndex,totalSize);
+					network->handleInboundConfigChunk(inRePacketId,chunkData,chunkLen,chunkIndex,totalSize);
 				}
 			}	break;
 
@@ -500,33 +502,32 @@ bool IncomingPacket::_doWHOIS(const RuntimeEnvironment *RR,const SharedPtr<Peer>
 bool IncomingPacket::_doRENDEZVOUS(const RuntimeEnvironment *RR,const SharedPtr<Peer> &peer)
 {
 	try {
-		if (RR->topology->isUpstream(peer->identity())) { // only upstream peers can tell us to rendezvous, otherwise this opens a potential amplification attack vector
-			const Address with(field(ZT_PROTO_VERB_RENDEZVOUS_IDX_ZTADDRESS,ZT_ADDRESS_LENGTH),ZT_ADDRESS_LENGTH);
-			const SharedPtr<Peer> withPeer(RR->topology->getPeer(with));
-			if (withPeer) {
-				const unsigned int port = at<uint16_t>(ZT_PROTO_VERB_RENDEZVOUS_IDX_PORT);
-				const unsigned int addrlen = (*this)[ZT_PROTO_VERB_RENDEZVOUS_IDX_ADDRLEN];
-				if ((port > 0)&&((addrlen == 4)||(addrlen == 16))) {
-					peer->received(_localAddress,_remoteAddress,hops(),packetId(),Packet::VERB_RENDEZVOUS,0,Packet::VERB_NOP);
-
-					const InetAddress atAddr(field(ZT_PROTO_VERB_RENDEZVOUS_IDX_ADDRESS,addrlen),addrlen,port);
-					TRACE("RENDEZVOUS from %s says %s might be at %s, attempting to contact",peer->address().toString().c_str(),with.toString().c_str(),atAddr.toString().c_str());
-					if (RR->node->shouldUsePathForZeroTierTraffic(_localAddress,atAddr)) {
-						const uint64_t now = RR->node->now();
-						peer->sendHELLO(_localAddress,atAddr,now,2); // send low-TTL packet to 'open' local NAT(s)
-						if (!peer->pushDirectPaths(_localAddress,atAddr,now,true))
-							peer->sendHELLO(_localAddress,atAddr,now);
-					}
+		const Address with(field(ZT_PROTO_VERB_RENDEZVOUS_IDX_ZTADDRESS,ZT_ADDRESS_LENGTH),ZT_ADDRESS_LENGTH);
+		const SharedPtr<Peer> withPeer(RR->topology->getPeer(with));
+		if (withPeer) {
+			const unsigned int port = at<uint16_t>(ZT_PROTO_VERB_RENDEZVOUS_IDX_PORT);
+			const unsigned int addrlen = (*this)[ZT_PROTO_VERB_RENDEZVOUS_IDX_ADDRLEN];
+			if ((port > 0)&&((addrlen == 4)||(addrlen == 16))) {
+				const InetAddress atAddr(field(ZT_PROTO_VERB_RENDEZVOUS_IDX_ADDRESS,addrlen),addrlen,port);
+				if (!RR->topology->isUpstream(peer->identity())) {
+					TRACE("RENDEZVOUS from %s says %s might be at %s, ignoring since peer is not upstream",peer->address().toString().c_str(),with.toString().c_str(),atAddr.toString().c_str());
+				} else if (RR->node->shouldUsePathForZeroTierTraffic(_localAddress,atAddr)) {
+					const uint64_t now = RR->node->now();
+					peer->sendHELLO(_localAddress,atAddr,now,2); // send low-TTL packet to 'open' local NAT(s)
+					if (!peer->pushDirectPaths(_localAddress,atAddr,now,true))
+						peer->sendHELLO(_localAddress,atAddr,now);
+					TRACE("RENDEZVOUS from %s says %s might be at %s, sent verification attempt",peer->address().toString().c_str(),with.toString().c_str(),atAddr.toString().c_str());
 				} else {
-					TRACE("dropped corrupt RENDEZVOUS from %s(%s) (bad address or port)",peer->address().toString().c_str(),_remoteAddress.toString().c_str());
+					TRACE("RENDEZVOUS from %s says %s might be at %s, ignoring since path is not suitable",peer->address().toString().c_str(),with.toString().c_str(),atAddr.toString().c_str());
 				}
 			} else {
-				RR->sw->requestWhois(with);
-				TRACE("ignored RENDEZVOUS from %s(%s) to meet unknown peer %s",peer->address().toString().c_str(),_remoteAddress.toString().c_str(),with.toString().c_str());
+				TRACE("dropped corrupt RENDEZVOUS from %s(%s) (bad address or port)",peer->address().toString().c_str(),_remoteAddress.toString().c_str());
 			}
 		} else {
-			TRACE("ignored RENDEZVOUS from %s(%s): not a root server or a network relay",peer->address().toString().c_str(),_remoteAddress.toString().c_str());
+			TRACE("ignored RENDEZVOUS from %s(%s) to meet unknown peer %s",peer->address().toString().c_str(),_remoteAddress.toString().c_str(),with.toString().c_str());
 		}
+
+		peer->received(_localAddress,_remoteAddress,hops(),packetId(),Packet::VERB_RENDEZVOUS,0,Packet::VERB_NOP);
 	} catch ( ... ) {
 		TRACE("dropped RENDEZVOUS from %s(%s): unexpected exception",peer->address().toString().c_str(),_remoteAddress.toString().c_str());
 	}
diff --git a/node/Network.cpp b/node/Network.cpp
index b9a2ca1d..4d588a30 100644
--- a/node/Network.cpp
+++ b/node/Network.cpp
@@ -593,13 +593,15 @@ int Network::setConfiguration(const NetworkConfig &nconf,bool saveToDisk)
 void Network::handleInboundConfigChunk(const uint64_t inRePacketId,const void *data,unsigned int chunkSize,unsigned int chunkIndex,unsigned int totalSize)
 {
 	std::string newConfig;
-	if ((_inboundConfigPacketId == inRePacketId)&&(totalSize < ZT_NETWORKCONFIG_DICT_CAPACITY)&&((chunkIndex + chunkSize) < totalSize)) {
+	if ((_inboundConfigPacketId == inRePacketId)&&(totalSize < ZT_NETWORKCONFIG_DICT_CAPACITY)&&((chunkIndex + chunkSize) <= totalSize)) {
 		Mutex::Lock _l(_lock);
-		TRACE("got %u bytes at position %u of network config request %.16llx, total expected length %u",chunkSize,chunkIndex,inRePacketId,totalSize);
+
 		_inboundConfigChunks[chunkIndex].append((const char *)data,chunkSize);
+
 		unsigned int totalWeHave = 0;
 		for(std::map<unsigned int,std::string>::iterator c(_inboundConfigChunks.begin());c!=_inboundConfigChunks.end();++c)
 			totalWeHave += (unsigned int)c->second.length();
+
 		if (totalWeHave == totalSize) {
 			TRACE("have all chunks for network config request %.16llx, assembling...",inRePacketId);
 			for(std::map<unsigned int,std::string>::iterator c(_inboundConfigChunks.begin());c!=_inboundConfigChunks.end();++c)
@@ -610,23 +612,29 @@ void Network::handleInboundConfigChunk(const uint64_t inRePacketId,const void *d
 			_inboundConfigPacketId = 0;
 			_inboundConfigChunks.clear();
 		}
+	} else {
+		return;
 	}
 
-	if (newConfig.length() > 0) {
-		if (newConfig.length() < ZT_NETWORKCONFIG_DICT_CAPACITY) {
-			Dictionary<ZT_NETWORKCONFIG_DICT_CAPACITY> *dict = new Dictionary<ZT_NETWORKCONFIG_DICT_CAPACITY>(newConfig.c_str());
-			try {
-				Identity controllerId(RR->topology->getIdentity(this->controller()));
-				if (controllerId) {
-					NetworkConfig nc;
-					if (nc.fromDictionary(controllerId,*dict))
-						this->setConfiguration(nc,true);
+	if ((newConfig.length() > 0)&&(newConfig.length() < ZT_NETWORKCONFIG_DICT_CAPACITY)) {
+		Dictionary<ZT_NETWORKCONFIG_DICT_CAPACITY> *dict = new Dictionary<ZT_NETWORKCONFIG_DICT_CAPACITY>(newConfig.c_str());
+		NetworkConfig *nc = new NetworkConfig();
+		try {
+			Identity controllerId(RR->topology->getIdentity(this->controller()));
+			if (controllerId) {
+				if (nc->fromDictionary(controllerId,*dict)) {
+					this->setConfiguration(*nc,true);
+				} else {
+					TRACE("error parsing new config with length %u: deserialization of NetworkConfig failed (certificate error?)",(unsigned int)newConfig.length());
 				}
-				delete dict;
-			} catch ( ... ) {
-				delete dict;
-				throw;
 			}
+			delete nc;
+			delete dict;
+		} catch ( ... ) {
+			TRACE("error parsing new config with length %u: unexpected exception",(unsigned int)newConfig.length());
+			delete nc;
+			delete dict;
+			throw;
 		}
 	}
 }