Add a bit of hardening in the network certificate of membership by incorporating a full hash of the identity to which it is issued. This means the recipient need not depend entirely on the root verifying identities properly to make sure impersonation is not occurring.

controller/EmbeddedNetworkController.cpp

@@ -1801,7 +1801,7 @@ void EmbeddedNetworkController::_request(
 		nc->certificateOfOwnershipCount = 1;
 	}
 
-	CertificateOfMembership com(now,credentialtmd,nwid,identity.address());
+	CertificateOfMembership com(now,credentialtmd,nwid,identity);
 	if (com.sign(_signingId)) {
 		nc->com = com;
 	} else {

node/CertificateOfMembership.cpp

@@ -148,37 +148,43 @@ void CertificateOfMembership::fromString(const char *s)
 
 #endif // ZT_SUPPORT_OLD_STYLE_NETCONF
 
-bool CertificateOfMembership::agreesWith(const CertificateOfMembership &other) const
+bool CertificateOfMembership::agreesWith(const CertificateOfMembership &other, const Identity &otherIdentity) const
 {
-	unsigned int myidx = 0;
-	unsigned int otheridx = 0;
-
 	if ((_qualifierCount == 0)||(other._qualifierCount == 0))
 		return false;
 
-	while (myidx < _qualifierCount) {
-		// Fail if we're at the end of other, since this means the field is
-		// missing.
-		if (otheridx >= other._qualifierCount)
-			return false;
+	std::map< uint64_t, uint64_t > otherFields;
+	for(unsigned int i=0;i<other._qualifierCount;++i)
+		otherFields[other._qualifiers[i].id] = other._qualifiers[i].value;
 
-		// Seek to corresponding tuple in other, ignoring tuples that
-		// we may not have. If we run off the end of other, the tuple is
-		// missing. This works because tuples are sorted by ID.
-		while (other._qualifiers[otheridx].id != _qualifiers[myidx].id) {
-			++otheridx;
-			if (otheridx >= other._qualifierCount)
+	bool fullIdentityVerification = false;
+	for(unsigned int i=0;i<_qualifierCount;++i) {
+		const uint64_t qid = _qualifiers[i].id;
+		if ((qid >= 3)&&(qid <= 6)) {
+			fullIdentityVerification = true;
+		} else {
+			std::map< uint64_t, uint64_t >::iterator otherQ(otherFields.find(qid));
+			if (otherQ == otherFields.end())
+				return false;
+			const uint64_t a = _qualifiers[i].value;
+			const uint64_t b = otherQ->second;
+			if (((a >= b) ? (a - b) : (b - a)) > _qualifiers[i].maxDelta)
 				return false;
 		}
+	}
 
-		// Compare to determine if the absolute value of the difference
-		// between these two parameters is within our maxDelta.
-		const uint64_t a = _qualifiers[myidx].value;
-		const uint64_t b = other._qualifiers[myidx].value;
-		if (((a >= b) ? (a - b) : (b - a)) > _qualifiers[myidx].maxDelta)
-			return false;
-
-		++myidx;
+	// If this COM has a full hash of its identity, assume the other must have this as well.
+	// Otherwise we are on a controller that does not incorporate these.
+	if (fullIdentityVerification) {
+		uint64_t idHash[6];
+		otherIdentity.publicKeyHash(idHash);
+		for(unsigned long i=0;i<4;++i) {
+			std::map< uint64_t, uint64_t >::iterator otherQ(otherFields.find((uint64_t)(i + 3)));
+			if (otherQ == otherFields.end())
+				return false;
+			if (otherQ->second != Utils::ntoh(idHash[i]))
+				return false;
+		}
 	}
 
 	return true;

node/CertificateOfMembership.hpp

@@ -94,6 +94,8 @@ public:
 		 * ZeroTier address to whom certificate was issued
 		 */
 		COM_RESERVED_ID_ISSUED_TO = 2
+
+		// IDs 3-6 reserved for full hash of identity to which this COM was issued.
 	};
 
 	/**
@@ -110,7 +112,7 @@ public:
 	 * @param nwid Network ID
 	 * @param issuedTo Certificate recipient
 	 */
-	CertificateOfMembership(uint64_t timestamp,uint64_t timestampMaxDelta,uint64_t nwid,const Address &issuedTo)
+	CertificateOfMembership(uint64_t timestamp,uint64_t timestampMaxDelta,uint64_t nwid,const Identity &issuedTo)
 	{
 		_qualifiers[0].id = COM_RESERVED_ID_TIMESTAMP;
 		_qualifiers[0].value = timestamp;
@@ -119,9 +121,20 @@ public:
 		_qualifiers[1].value = nwid;
 		_qualifiers[1].maxDelta = 0;
 		_qualifiers[2].id = COM_RESERVED_ID_ISSUED_TO;
-		_qualifiers[2].value = issuedTo.toInt();
+		_qualifiers[2].value = issuedTo.address().toInt();
 		_qualifiers[2].maxDelta = 0xffffffffffffffffULL;
-		_qualifierCount = 3;
+
+		// Include hash of full identity public key in COM for hardening purposes. Pack it in
+		// using the original COM format. Format may be revised in the future to make this cleaner.
+		uint64_t idHash[6];
+		issuedTo.publicKeyHash(idHash);
+		for(unsigned long i=0;i<4;++i) {
+			_qualifiers[i + 3].id = (uint64_t)(i + 3);
+			_qualifiers[i + 3].value = Utils::ntoh(idHash[i]);
+			_qualifiers[i + 3].maxDelta = 0xffffffffffffffffULL;
+		}
+
+		_qualifierCount = 7;
 		memset(_signature.data,0,ZT_C25519_SIGNATURE_LEN);
 	}
 
@@ -224,9 +237,10 @@ public:
 	 * tuples present in this cert but not in other result in 'false'.
 	 *
 	 * @param other Cert to compare with
+	 * @param otherIdentity Identity of other node
 	 * @return True if certs agree and 'other' may be communicated with
 	 */
-	bool agreesWith(const CertificateOfMembership &other) const;
+	bool agreesWith(const CertificateOfMembership &other, const Identity &otherIdentity) const;
 
 	/**
 	 * Sign this certificate

node/Identity.hpp

@@ -109,6 +109,18 @@ public:
 	 */
 	inline bool hasPrivate() const { return (_privateKey != (C25519::Private *)0); }
 
+	/**
+	 * Compute a SHA384 hash of this identity's address and public key(s).
+	 * 
+	 * @param sha384buf Buffer with 48 bytes of space to receive hash
+	 */
+	inline void publicKeyHash(void *sha384buf) const
+	{
+		uint8_t address[ZT_ADDRESS_LENGTH];
+		_address.copyTo(address, ZT_ADDRESS_LENGTH);
+		SHA384(sha384buf, address, ZT_ADDRESS_LENGTH, _publicKey.data, ZT_C25519_PUBLIC_KEY_LEN);
+	}
+
 	/**
 	 * Compute the SHA512 hash of our private key (if we have one)
 	 *

node/Membership.hpp

@@ -91,13 +91,14 @@ public:
 	 * Check whether the peer represented by this Membership should be allowed on this network at all
 	 *
 	 * @param nconf Our network config
+	 * @param otherNodeIdentity Identity of remote node
 	 * @return True if this peer is allowed on this network at all
 	 */
-	inline bool isAllowedOnNetwork(const NetworkConfig &nconf) const
+	inline bool isAllowedOnNetwork(const NetworkConfig &thisNodeNetworkConfig, const Identity &otherNodeIdentity) const
 	{
-		if (nconf.isPublic()) return true;
+		if (thisNodeNetworkConfig.isPublic()) return true;
 		if (_com.timestamp() <= _comRevocationThreshold) return false;
-		return nconf.com.agreesWith(_com);
+		return thisNodeNetworkConfig.com.agreesWith(_com, otherNodeIdentity);
 	}
 
 	inline bool recentlyAssociated(const int64_t now) const

node/Network.cpp

@@ -1227,7 +1227,7 @@ bool Network::gate(void *tPtr,const SharedPtr<Peer> &peer)
 	try {
 		if (_config) {
 			Membership *m = _memberships.get(peer->address());
-			if ( (_config.isPublic()) || ((m)&&(m->isAllowedOnNetwork(_config))) ) {
+			if ( (_config.isPublic()) || ((m)&&(m->isAllowedOnNetwork(_config, peer->identity()))) ) {
 				if (!m)
 					m = &(_membership(peer->address()));
 				if (m->multicastLikeGate(now)) {
@@ -1487,8 +1487,11 @@ void Network::_sendUpdatesToMembers(void *tPtr,const MulticastGroup *const newMu
 		Membership *m = (Membership *)0;
 		Hashtable<Address,Membership>::Iterator i(_memberships);
 		while (i.next(a,m)) {
-			if ( ( m->multicastLikeGate(now) || (newMulticastGroup) ) && (m->isAllowedOnNetwork(_config)) && (!std::binary_search(alwaysAnnounceTo.begin(),alwaysAnnounceTo.end(),*a)) )
-				_announceMulticastGroupsTo(tPtr,*a,groups);
+			const Identity remoteIdentity(RR->topology->getIdentity(tPtr, *a));
+			if (remoteIdentity) {
+				if ( ( m->multicastLikeGate(now) || (newMulticastGroup) ) && (m->isAllowedOnNetwork(_config, remoteIdentity)) && (!std::binary_search(alwaysAnnounceTo.begin(),alwaysAnnounceTo.end(),*a)) )
+					_announceMulticastGroupsTo(tPtr,*a,groups);
+			}
 		}
 	}
 }

selftest.cpp

@@ -561,8 +561,8 @@ static int testCertificate()
 	std::cout << idA.address().toString(buf) << ", " << idB.address().toString(buf) << std::endl;
 
 	std::cout << "[certificate] Generating certificates A and B...";
-	CertificateOfMembership cA(10000,100,1,idA.address());
-	CertificateOfMembership cB(10099,100,1,idB.address());
+	CertificateOfMembership cA(10000,100,1,idA);
+	CertificateOfMembership cB(10099,100,1,idB);
 	std::cout << std::endl;
 
 	std::cout << "[certificate] Signing certificates A and B with authority...";
@@ -574,13 +574,13 @@ static int testCertificate()
 	//std::cout << "[certificate] B: " << cB.toString() << std::endl;
 
 	std::cout << "[certificate] A agrees with B and B with A... ";
-	if (cA.agreesWith(cB))
+	if (cA.agreesWith(cB, idB))
 		std::cout << "yes, ";
 	else {
 		std::cout << "FAIL" << std::endl;
 		return -1;
 	}
-	if (cB.agreesWith(cA))
+	if (cB.agreesWith(cA, idA))
 		std::cout << "yes." << std::endl;
 	else {
 		std::cout << "FAIL" << std::endl;
@@ -588,18 +588,18 @@ static int testCertificate()
 	}
 
 	std::cout << "[certificate] Generating two certificates that should not agree...";
-	cA = CertificateOfMembership(10000,100,1,idA.address());
-	cB = CertificateOfMembership(10101,100,1,idB.address());
+	cA = CertificateOfMembership(10000,100,1,idA);
+	cB = CertificateOfMembership(10101,100,1,idB);
 	std::cout << std::endl;
 
 	std::cout << "[certificate] A agrees with B and B with A... ";
-	if (!cA.agreesWith(cB))
+	if (!cA.agreesWith(cB, idB))
 		std::cout << "no, ";
 	else {
 		std::cout << "FAIL" << std::endl;
 		return -1;
 	}
-	if (!cB.agreesWith(cA))
+	if (!cB.agreesWith(cA, idA))
 		std::cout << "no." << std::endl;
 	else {
 		std::cout << "FAIL" << std::endl;