Fix bug in ECHO handling (OK was invalid!), and use ECHO on newer peers for path confirmation. Also get rid of path confirmation circuit breaker since this causes issues with some peers and should be done more intelligently anyway.

node/Constants.hpp

@@ -293,11 +293,6 @@
  */
 #define ZT_NAT_T_TACTICAL_ESCALATION_DELAY 1000
 
-/**
- * Minimum delay between attempts to confirm new paths to peers (to avoid HELLO flooding)
- */
-#define ZT_MIN_PATH_CONFIRMATION_INTERVAL 1000
-
 /**
  * How long (max) to remember network certificates of membership?
  *

node/IncomingPacket.cpp

@@ -348,7 +348,7 @@ bool IncomingPacket::_doHELLO(const RuntimeEnvironment *RR,SharedPtr<Peer> &peer
 		RR->antiRec->logOutgoingZT(outp.data(),outp.size());
 		RR->node->putPacket(_localAddress,_remoteAddress,outp.data(),outp.size());
 
-		peer->setRemoteVersion(protoVersion,vMajor,vMinor,vRevision);
+		peer->setRemoteVersion(protoVersion,vMajor,vMinor,vRevision); // important for this to go first so received() knows the version
 		peer->received(RR,_localAddress,_remoteAddress,hops(),pid,Packet::VERB_HELLO,0,Packet::VERB_NOP);
 	} catch ( ... ) {
 		TRACE("dropped HELLO from %s(%s): unexpected exception",source().toString().c_str(),_remoteAddress.toString().c_str());
@@ -426,6 +426,9 @@ bool IncomingPacket::_doOK(const RuntimeEnvironment *RR,const SharedPtr<Peer> &p
 				}
 			}	break;
 
+			//case Packet::VERB_ECHO: {
+			//}	break;
+
 			case Packet::VERB_MULTICAST_GATHER: {
 				const uint64_t nwid = at<uint64_t>(ZT_PROTO_VERB_MULTICAST_GATHER__OK__IDX_NETWORK_ID);
 				const MulticastGroup mg(MAC(field(ZT_PROTO_VERB_MULTICAST_GATHER__OK__IDX_MAC,6),6),at<uint32_t>(ZT_PROTO_VERB_MULTICAST_GATHER__OK__IDX_ADI));
@@ -638,7 +641,9 @@ bool IncomingPacket::_doECHO(const RuntimeEnvironment *RR,const SharedPtr<Peer>
 		Packet outp(peer->address(),RR->identity.address(),Packet::VERB_OK);
 		outp.append((unsigned char)Packet::VERB_ECHO);
 		outp.append((uint64_t)pid);
-		outp.append(field(ZT_PACKET_IDX_PAYLOAD,size() - ZT_PACKET_IDX_PAYLOAD),size() - ZT_PACKET_IDX_PAYLOAD);
+		if (size() > ZT_PACKET_IDX_PAYLOAD)
+			outp.append(reinterpret_cast<const unsigned char *>(data()) + ZT_PACKET_IDX_PAYLOAD,size() - ZT_PACKET_IDX_PAYLOAD);
+		outp.armor(peer->key(),true);
 		RR->antiRec->logOutgoingZT(outp.data(),outp.size());
 		RR->node->putPacket(_localAddress,_remoteAddress,outp.data(),outp.size());
 		peer->received(RR,_localAddress,_remoteAddress,hops(),pid,Packet::VERB_ECHO,0,Packet::VERB_NOP);

node/Packet.hpp

@@ -678,7 +678,14 @@ public:
 		 *   <[...] arbitrary payload to be echoed back>
 		 *
 		 * This generates OK with a copy of the transmitted payload. No ERROR
-		 * is generated. Response to ECHO requests is optional.
+		 * is generated. Response to ECHO requests is optional and ECHO may be
+		 * ignored if a node detects a possible flood.
+		 *
+		 * There is a de-facto standard for ECHO payload. No payload indicates an
+		 * ECHO used for path confirmation. Otherwise the first byte contains
+		 * flags, in which currently the only flag is 0x01 for a user-requested
+		 * echo. For user-requested echoes the result may be reported back through
+		 * the API. Otherwise the payload is for internal use.
 		 *
 		 * Support for fragmented echo packets is optional and their use is not
 		 * recommended.

node/Peer.cpp

@@ -53,7 +53,6 @@ Peer::Peer(const Identity &myIdentity,const Identity &peerIdentity)
 	_lastUnicastFrame(0),
 	_lastMulticastFrame(0),
 	_lastAnnouncedTo(0),
-	_lastPathConfirmationSent(0),
 	_lastDirectPathPushSent(0),
 	_lastDirectPathPushReceive(0),
 	_lastPathSort(0),
@@ -132,7 +131,6 @@ void Peer::received(
 
 	const uint64_t now = RR->node->now();
 	bool needMulticastGroupAnnounce = false;
-	bool pathIsConfirmed = false;
 
 	{	// begin _lock
 		Mutex::Lock _l(_lock);
@@ -149,6 +147,7 @@ void Peer::received(
 		}
 
 		if (hops == 0) {
+			bool pathIsConfirmed = false;
 			unsigned int np = _numPaths;
 			for(unsigned int p=0;p<np;++p) {
 				if ((_paths[p].address() == remoteAddr)&&(_paths[p].localAddress() == localAddr)) {
@@ -183,7 +182,6 @@ void Peer::received(
 						slot->setClusterSuboptimal(suboptimalPath);
 #endif
 						_numPaths = np;
-						pathIsConfirmed = true;
 						_sortPaths(now);
 					}
 
@@ -194,13 +192,14 @@ void Peer::received(
 
 				} else {
 
-					/* If this path is not known, send a HELLO. We don't learn
-					 * paths without confirming that a bidirectional link is in
-					 * fact present, but any packet that decodes and authenticates
-					 * correctly is considered valid. */
-					if ((now - _lastPathConfirmationSent) >= ZT_MIN_PATH_CONFIRMATION_INTERVAL) {
-						_lastPathConfirmationSent = now;
-						TRACE("got %s via unknown path %s(%s), confirming...",Packet::verbString(verb),_id.address().toString().c_str(),remoteAddr.toString().c_str());
+					TRACE("got %s via unknown path %s(%s), confirming...",Packet::verbString(verb),_id.address().toString().c_str(),remoteAddr.toString().c_str());
+
+					if ((_vMajor >= 1)&&(_vMinor >= 1)&&(_vRevision >= 1)) {
+						// 1.1.1 and newer nodes support ECHO, which is smaller
+						Packet outp(_id.address(),RR->identity.address(),Packet::VERB_ECHO);
+						outp.armor(_key,true);
+						RR->node->putPacket(localAddr,remoteAddr,outp.data(),outp.size());
+					} else {
 						sendHELLO(RR,localAddr,remoteAddr,now);
 					}
 

node/Peer.hpp

@@ -454,7 +454,7 @@ public:
 		const unsigned int recSizePos = b.size();
 		b.addSize(4); // space for uint32_t field length
 
-		b.append((uint16_t)0); // version of serialized Peer data
+		b.append((uint16_t)1); // version of serialized Peer data
 
 		_id.serialize(b,false);
 
@@ -463,7 +463,6 @@ public:
 		b.append((uint64_t)_lastUnicastFrame);
 		b.append((uint64_t)_lastMulticastFrame);
 		b.append((uint64_t)_lastAnnouncedTo);
-		b.append((uint64_t)_lastPathConfirmationSent);
 		b.append((uint64_t)_lastDirectPathPushSent);
 		b.append((uint64_t)_lastDirectPathPushReceive);
 		b.append((uint64_t)_lastPathSort);
@@ -518,7 +517,7 @@ public:
 		const unsigned int recSize = b.template at<uint32_t>(p); p += 4;
 		if ((p + recSize) > b.size())
 			return SharedPtr<Peer>(); // size invalid
-		if (b.template at<uint16_t>(p) != 0)
+		if (b.template at<uint16_t>(p) != 1)
 			return SharedPtr<Peer>(); // version mismatch
 		p += 2;
 
@@ -534,7 +533,6 @@ public:
 		np->_lastUnicastFrame = b.template at<uint64_t>(p); p += 8;
 		np->_lastMulticastFrame = b.template at<uint64_t>(p); p += 8;
 		np->_lastAnnouncedTo = b.template at<uint64_t>(p); p += 8;
-		np->_lastPathConfirmationSent = b.template at<uint64_t>(p); p += 8;
 		np->_lastDirectPathPushSent = b.template at<uint64_t>(p); p += 8;
 		np->_lastDirectPathPushReceive = b.template at<uint64_t>(p); p += 8;
 		np->_lastPathSort = b.template at<uint64_t>(p); p += 8;
@@ -585,7 +583,6 @@ private:
 	uint64_t _lastUnicastFrame;
 	uint64_t _lastMulticastFrame;
 	uint64_t _lastAnnouncedTo;
-	uint64_t _lastPathConfirmationSent;
 	uint64_t _lastDirectPathPushSent;
 	uint64_t _lastDirectPathPushReceive;
 	uint64_t _lastPathSort;

node/Switch.cpp

@@ -96,7 +96,7 @@ void Switch::onRemotePacket(const InetAddress &localAddr,const InetAddress &from
 				if ((now - _lastBeaconResponse) >= 2500) { // limit rate of responses
 					_lastBeaconResponse = now;
 					Packet outp(peer->address(),RR->identity.address(),Packet::VERB_NOP);
-					outp.armor(peer->key(),false);
+					outp.armor(peer->key(),true);
 					RR->node->putPacket(localAddr,fromAddr,outp.data(),outp.size());
 				}
 			}

version.h

@@ -41,6 +41,6 @@
 /**
  * Revision
  */
-#define ZEROTIER_ONE_VERSION_REVISION 0
+#define ZEROTIER_ONE_VERSION_REVISION 1
 
 #endif