diff --git a/controller/EmbeddedNetworkController.cpp b/controller/EmbeddedNetworkController.cpp index 99d59aeef..ea70cb3ae 100644 --- a/controller/EmbeddedNetworkController.cpp +++ b/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 { diff --git a/node/CertificateOfMembership.cpp b/node/CertificateOfMembership.cpp index 10cb0863a..2580041c8 100644 --- a/node/CertificateOfMembership.cpp +++ b/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) + 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; diff --git a/node/CertificateOfMembership.hpp b/node/CertificateOfMembership.hpp index f8500628d..0e4e04748 100644 --- a/node/CertificateOfMembership.hpp +++ b/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 diff --git a/node/Identity.hpp b/node/Identity.hpp index e6f658dc3..cc8de5126 100644 --- a/node/Identity.hpp +++ b/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) * diff --git a/node/Membership.hpp b/node/Membership.hpp index 476987714..63a7c10f5 100644 --- a/node/Membership.hpp +++ b/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 diff --git a/node/Network.cpp b/node/Network.cpp index a9007258f..f3138f3ac 100644 --- a/node/Network.cpp +++ b/node/Network.cpp @@ -1227,7 +1227,7 @@ bool Network::gate(void *tPtr,const SharedPtr &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::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); + } } } } diff --git a/selftest.cpp b/selftest.cpp index 357e9a026..42e9bc232 100644 --- a/selftest.cpp +++ b/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;