A bunch of comments and cleanup, including some to yesterday's direct path pushing changes. Move path viability check to one place, and stop trying to use link-local addresses since they are not reliable.

5 files changed, 100 insertions, 32 deletions

diff --git a/node/IncomingPacket.cpp b/node/IncomingPacket.cpp
index 80159194..7e883221 100644
--- a/node/IncomingPacket.cpp
+++ b/node/IncomingPacket.cpp
@@ -134,6 +134,9 @@ bool IncomingPacket::_doERROR(const RuntimeEnvironment *RR,const SharedPtr<Peer>
 				break;
 
 			case Packet::ERROR_NEED_MEMBERSHIP_CERTIFICATE: {
+				/* Note: certificates are public so it's safe to push them to anyone
+				 * who asks. We won't communicate unless we also get a certificate
+				 * from the remote that agrees. */
 				SharedPtr<Network> network(RR->node->network(at<uint64_t>(ZT_PROTO_VERB_ERROR_IDX_PAYLOAD)));
 				if (network) {
 					SharedPtr<NetworkConfig> nconf(network->config2());
@@ -170,8 +173,20 @@ bool IncomingPacket::_doERROR(const RuntimeEnvironment *RR,const SharedPtr<Peer>
 
 bool IncomingPacket::_doHELLO(const RuntimeEnvironment *RR)
 {
+	/* Note: this is the only packet ever sent in the clear, and it's also
+	 * the only packet that we authenticate via a different path. Authentication
+	 * occurs here and is based on the validity of the identity and the
+	 * integrity of the packet's MAC, but it must be done after we check
+	 * the identity since HELLO is a mechanism for learning new identities
+	 * in the first place. */
+
 	try {
 		const unsigned int protoVersion = (*this)[ZT_PROTO_VERB_HELLO_IDX_PROTOCOL_VERSION];
+		if (protoVersion < ZT_PROTO_VERSION_MIN) {
+			TRACE("dropped HELLO from %s(%s): protocol version too old",id.address().toString().c_str(),_remoteAddress.toString().c_str());
+			return true;
+		}
+
 		const unsigned int vMajor = (*this)[ZT_PROTO_VERB_HELLO_IDX_MAJOR_VERSION];
 		const unsigned int vMinor = (*this)[ZT_PROTO_VERB_HELLO_IDX_MINOR_VERSION];
 		const unsigned int vRevision = at<uint16_t>(ZT_PROTO_VERB_HELLO_IDX_REVISION);
@@ -179,6 +194,10 @@ bool IncomingPacket::_doHELLO(const RuntimeEnvironment *RR)
 
 		Identity id;
 		unsigned int destAddrPtr = id.deserialize(*this,ZT_PROTO_VERB_HELLO_IDX_IDENTITY) + ZT_PROTO_VERB_HELLO_IDX_IDENTITY;
+		if (source() != id.address()) {
+			TRACE("dropped HELLO from %s(%s): identity not for sending address",source().toString().c_str(),_remoteAddress.toString().c_str());
+			return true;
+		}
 
 		InetAddress destAddr;
 		if (destAddrPtr < size()) { // ZeroTier One < 1.0.3 did not include this field
@@ -193,16 +212,6 @@ bool IncomingPacket::_doHELLO(const RuntimeEnvironment *RR)
 			}
 		}
 
-		if (source() != id.address()) {
-			TRACE("dropped HELLO from %s(%s): identity not for sending address",source().toString().c_str(),_remoteAddress.toString().c_str());
-			return true;
-		}
-
-		if (protoVersion < ZT_PROTO_VERSION_MIN) {
-			TRACE("dropped HELLO from %s(%s): protocol version too old",id.address().toString().c_str(),_remoteAddress.toString().c_str());
-			return true;
-		}
-
 		SharedPtr<Peer> peer(RR->topology->getPeer(id.address()));
 		if (peer) {
 			// We already have an identity with this address -- check for collisions
@@ -245,12 +254,14 @@ bool IncomingPacket::_doHELLO(const RuntimeEnvironment *RR)
 		} else {
 			// We don't already have an identity with this address -- validate and learn it
 
+			// Check identity proof of work
 			if (!id.locallyValidate()) {
 				RR->node->postEvent(ZT1_EVENT_AUTHENTICATION_FAILURE,(const void *)&_remoteAddress);
 				TRACE("dropped HELLO from %s(%s): identity invalid",id.address().toString().c_str(),_remoteAddress.toString().c_str());
 				return true;
 			}
 
+			// Check packet integrity and authentication
 			SharedPtr<Peer> newPeer(new Peer(RR->identity,id));
 			if (!dearmor(newPeer->key())) {
 				RR->node->postEvent(ZT1_EVENT_AUTHENTICATION_FAILURE,(const void *)&_remoteAddress);
@@ -554,14 +565,17 @@ bool IncomingPacket::_doEXT_FRAME(const RuntimeEnvironment *RR,const SharedPtr<P
 				const unsigned int flags = (*this)[ZT_PROTO_VERB_EXT_FRAME_IDX_FLAGS];
 
 				unsigned int comLen = 0;
+				bool comFailed = false;
 				if ((flags & 0x01) != 0) {
 					CertificateOfMembership com;
 					comLen = com.deserialize(*this,ZT_PROTO_VERB_EXT_FRAME_IDX_COM);
-					if (com.hasRequiredFields())
-						network->validateAndAddMembershipCertificate(com);
+					if (com.hasRequiredFields()) {
+						if (!network->validateAndAddMembershipCertificate(com))
+							comFailed = true; // technically this check is redundant to isAllowed(), but do it anyway for thoroughness
+					}
 				}
 
-				if (!network->isAllowed(peer->address())) {
+				if ((comFailed)||(!network->isAllowed(peer->address()))) {
 					TRACE("dropped EXT_FRAME from %s(%s): not a member of private network %.16llx",peer->address().toString().c_str(),_remoteAddress.toString().c_str(),network->id());
 					_sendErrorNeedCertificate(RR,peer,network->id());
 					return true;
@@ -647,8 +661,21 @@ bool IncomingPacket::_doNETWORK_MEMBERSHIP_CERTIFICATE(const RuntimeEnvironment
 			ptr += com.deserialize(*this,ptr);
 			if (com.hasRequiredFields()) {
 				SharedPtr<Network> network(RR->node->network(com.networkId()));
-				if (network)
-					network->validateAndAddMembershipCertificate(com);
+				if (network) {
+					if (network->validateAndAddMembershipCertificate(com)) {
+						if ((network->isAllowed(peer->address()))&&(network->peerNeedsOurMembershipCertificate(peer->address(),RR->node->now()))) {
+							// If peer passed our check and we haven't sent it our cert yet, respond
+							// and push our cert as well for instant authorization setup.
+							SharedPtr<NetworkConfig> nconf(network->config2());
+							if ((nconf)&&(nconf->com())) {
+								Packet outp(peer->address(),RR->identity.address(),Packet::VERB_NETWORK_MEMBERSHIP_CERTIFICATE);
+								nconf->com().serialize(outp);
+								outp.armor(peer->key(),true);
+								RR->node->putPacket(_remoteAddress,outp.data(),outp.size());
+							}
+						}
+					}
+				}
 			}
 		}
 
@@ -890,24 +917,25 @@ bool IncomingPacket::_doPUSH_DIRECT_PATHS(const RuntimeEnvironment *RR,const Sha
 		unsigned int ptr = ZT_PACKET_IDX_PAYLOAD + 2;
 
 		while (count) { // if ptr overflows Buffer will throw
+			// TODO: properly handle blacklisting, support other features... see Packet.hpp.
+
 			unsigned int flags = (*this)[ptr++];
 			/*int metric = (*this)[ptr++];*/ ++ptr;
 			unsigned int extLen = at<uint16_t>(ptr); ptr += 2;
 			ptr += extLen; // unused right now
 			unsigned int addrType = (*this)[ptr++];
+
 			unsigned int addrLen = (*this)[ptr++];
 			switch(addrType) {
 				case 4: {
 					InetAddress a(field(ptr,4),4,at<uint16_t>(ptr + 4));
-					if ((flags & (0x01 | 0x02)) == 0) {
+					if ( ((flags & (0x01 | 0x02)) == 0) && (Path::isAddressValidForPath(a)) )
 						peer->attemptToContactAt(RR,a,RR->node->now());
-					}
 				}	break;
 				case 6: {
 					InetAddress a(field(ptr,16),16,at<uint16_t>(ptr + 16));
-					if ((flags & (0x01 | 0x02)) == 0) {
+					if ( ((flags & (0x01 | 0x02)) == 0) && (Path::isAddressValidForPath(a)) )
 						peer->attemptToContactAt(RR,a,RR->node->now());
-					}
 				}	break;
 			}
 			ptr += addrLen;
diff --git a/node/Node.cpp b/node/Node.cpp
index 057dc285..d8bd8910 100644
--- a/node/Node.cpp
+++ b/node/Node.cpp
@@ -426,12 +426,16 @@ void Node::freeQueryResult(void *qr)
 		::free(qr);
 }
 
-void Node::addLocalInterfaceAddress(const struct sockaddr_storage *addr,int metric,ZT1_LocalInterfaceAddressTrust trust,int reliable)
-{
-	Mutex::Lock _l(_directPaths_m);
-	_directPaths.push_back(Path(*(reinterpret_cast<const InetAddress *>(addr)),metric,(Path::Trust)trust,reliable != 0));
-	std::sort(_directPaths.begin(),_directPaths.end());
-	_directPaths.erase(std::unique(_directPaths.begin(),_directPaths.end()),_directPaths.end());
+int Node::addLocalInterfaceAddress(const struct sockaddr_storage *addr,int metric,ZT1_LocalInterfaceAddressTrust trust,int reliable)
+{
+	if (Path::isAddressValidForPath(*(reinterpret_cast<const InetAddress *>(addr)))) {
+		Mutex::Lock _l(_directPaths_m);
+		_directPaths.push_back(Path(*(reinterpret_cast<const InetAddress *>(addr)),metric,(Path::Trust)trust,reliable != 0));
+		std::sort(_directPaths.begin(),_directPaths.end());
+		_directPaths.erase(std::unique(_directPaths.begin(),_directPaths.end()),_directPaths.end());
+		return 1;
+	}
+	return 0;
 }
 
 void Node::clearLocalInterfaceAddresses()
@@ -693,11 +697,13 @@ void ZT1_Node_setNetconfMaster(ZT1_Node *node,void *networkControllerInstance)
 	} catch ( ... ) {}
 }
 
-void ZT1_Node_addLocalInterfaceAddress(ZT1_Node *node,const struct sockaddr_storage *addr,int metric,ZT1_LocalInterfaceAddressTrust trust,int reliable)
+int ZT1_Node_addLocalInterfaceAddress(ZT1_Node *node,const struct sockaddr_storage *addr,int metric,ZT1_LocalInterfaceAddressTrust trust,int reliable)
 {
 	try {
-		reinterpret_cast<ZeroTier::Node *>(node)->addLocalInterfaceAddress(addr,metric,trust,reliable);
-	} catch ( ... ) {}
+		return reinterpret_cast<ZeroTier::Node *>(node)->addLocalInterfaceAddress(addr,metric,trust,reliable);
+	} catch ( ... ) {
+		return 0;
+	}
 }
 
 void ZT1_Node_clearLocalInterfaceAddresses(ZT1_Node *node)
diff --git a/node/Node.hpp b/node/Node.hpp
index 1c260545..fe31576c 100644
--- a/node/Node.hpp
+++ b/node/Node.hpp
@@ -104,7 +104,7 @@ public:
 	ZT1_VirtualNetworkConfig *networkConfig(uint64_t nwid) const;
 	ZT1_VirtualNetworkList *networks() const;
 	void freeQueryResult(void *qr);
-	void addLocalInterfaceAddress(const struct sockaddr_storage *addr,int metric,ZT1_LocalInterfaceAddressTrust trust,int reliable);
+	int addLocalInterfaceAddress(const struct sockaddr_storage *addr,int metric,ZT1_LocalInterfaceAddressTrust trust,int reliable);
 	void clearLocalInterfaceAddresses();
 	void setNetconfMaster(void *networkControllerInstance);
 
diff --git a/node/Packet.hpp b/node/Packet.hpp
index 51a241ba..9787edb7 100644
--- a/node/Packet.hpp
+++ b/node/Packet.hpp
@@ -678,9 +678,10 @@ public:
 		 *   <[...] serialized certificate of membership>
 		 *   [ ... additional certificates may follow ...]
 		 *
-		 * Certificate contains network ID, peer it was issued for, etc.
-		 *
-		 * OK/ERROR are not generated.
+		 * OK/ERROR are not generated, but the recipient should push its network
+		 * membership certificate if the certificate the peer pushed is valid
+		 * and agrees and if it hasn't done so in too long. This ensures instant
+		 * network authentication setup between valid and authorized peers.
 		 */
 		VERB_NETWORK_MEMBERSHIP_CERTIFICATE = 10,
 
diff --git a/node/Path.hpp b/node/Path.hpp
index 80b9a3c0..cd21444b 100644
--- a/node/Path.hpp
+++ b/node/Path.hpp
@@ -94,6 +94,39 @@ public:
 	inline bool operator<=(const Path &p) const throw() { return (_addr <= p._addr); }
 	inline bool operator>=(const Path &p) const throw() { return (_addr >= p._addr); }
 
+	/**
+	 * Check whether this address is valid for a ZeroTier path
+	 *
+	 * This checks the address type and scope against address types and scopes
+	 * that we currently support for ZeroTier communication.
+	 *
+	 * @param a Address to check
+	 * @return True if address is good for ZeroTier path use
+	 */
+	static inline bool isAddressValidForPath(const InetAddress &a)
+		throw()
+	{
+		if ((a.ss_family == AF_INET)||(a.ss_family == AF_INET6)) {
+			switch(a.ipScope()) {
+				/* Note: we don't do link-local at the moment. Unfortunately these
+				 * cause several issues. The first is that they usually require a
+				 * device qualifier, which we don't handle yet and can't portably
+				 * push in PUSH_DIRECT_PATHS. The second is that some OSes assign
+				 * these very ephemerally or otherwise strangely. So we'll use
+				 * private, pseudo-private, shared (e.g. carrier grade NAT), or
+				 * global IP addresses. */
+				case InetAddress::IP_SCOPE_PRIVATE:
+				case InetAddress::IP_SCOPE_PSEUDOPRIVATE:
+				case InetAddress::IP_SCOPE_SHARED:
+				case InetAddress::IP_SCOPE_GLOBAL:
+					return true;
+				default:
+					return false;
+			}
+		}
+		return false;
+	}
+
 protected:
 	InetAddress _addr;
 	int _metric; // negative == blacklisted