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.

7 files changed, 28 insertions, 25 deletions

diff --git a/node/Constants.hpp b/node/Constants.hpp
index 2b4b1dee..7368a634 100644
--- a/node/Constants.hpp
+++ b/node/Constants.hpp
@@ -294,11 +294,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?
  *
  * This only applies to networks we don't belong to.
diff --git a/node/IncomingPacket.cpp b/node/IncomingPacket.cpp
index cffa0b9a..69680544 100644
--- a/node/IncomingPacket.cpp
+++ b/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);
diff --git a/node/Packet.hpp b/node/Packet.hpp
index e0c702ce..6c1b2984 100644
--- a/node/Packet.hpp
+++ b/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.
diff --git a/node/Peer.cpp b/node/Peer.cpp
index f0f43399..e93a27b8 100644
--- a/node/Peer.cpp
+++ b/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);
 					}
 
diff --git a/node/Peer.hpp b/node/Peer.hpp
index 5816db69..069d44c0 100644
--- a/node/Peer.hpp
+++ b/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;
diff --git a/node/Switch.cpp b/node/Switch.cpp
index bf0d1aff..a06de17e 100644
--- a/node/Switch.cpp
+++ b/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());
 				}
 			}
diff --git a/version.h b/version.h
index e2773877..eb95635c 100644
--- a/version.h
+++ b/version.h
@@ -41,6 +41,6 @@
 /**
  * Revision
  */
-#define ZEROTIER_ONE_VERSION_REVISION 0
+#define ZEROTIER_ONE_VERSION_REVISION 1
 
 #endif