From 83265768c1a673ff45e560756e50343f80490c64 Mon Sep 17 00:00:00 2001 From: Grant Limberg Date: Wed, 15 Sep 2021 09:45:10 -0700 Subject: [PATCH 1/9] ensure count > 0 --- controller/PostgreSQL.cpp | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/controller/PostgreSQL.cpp b/controller/PostgreSQL.cpp index 89f5cb9ba..50b128418 100644 --- a/controller/PostgreSQL.cpp +++ b/controller/PostgreSQL.cpp @@ -594,12 +594,14 @@ void PostgreSQL::initializeNetworks() auto dur = std::chrono::duration_cast(end - start);; total += dur.count(); ++count; - if (count % 10000 == 0) { + if (count > 0 && count % 10000 == 0) { fprintf(stderr, "Averaging %llu us per network\n", (total/count)); } } - fprintf(stderr, "Took %llu us per network to load\n", (total/count)); + if (count > 0) { + fprintf(stderr, "Took %llu us per network to load\n", (total/count)); + } stream.complete(); w.commit(); @@ -748,11 +750,13 @@ void PostgreSQL::initializeMembers() auto dur = std::chrono::duration_cast(end - start);; total += dur.count(); ++count; - if (count % 10000 == 0) { + if (count > 0 && count % 10000 == 0) { fprintf(stderr, "Averaging %llu us per member\n", (total/count)); } } - fprintf(stderr, "Took %llu us per member to load\n", (total/count)); + if (count > 0) { + fprintf(stderr, "Took %llu us per member to load\n", (total/count)); + } stream.complete(); From d2c3ea69017cf8699b8f0d56445b9e9f823d715a Mon Sep 17 00:00:00 2001 From: Adam Ierymenko Date: Wed, 15 Sep 2021 13:31:18 -0400 Subject: [PATCH 2/9] Another route fix on BSD. --- osdep/ManagedRoute.cpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/osdep/ManagedRoute.cpp b/osdep/ManagedRoute.cpp index fd06f9cfe..4c1214a66 100644 --- a/osdep/ManagedRoute.cpp +++ b/osdep/ManagedRoute.cpp @@ -499,23 +499,25 @@ bool ManagedRoute::sync() if (_systemVia) { _routeCmd("add",leftt,_systemVia,_systemDevice,(const char *)0); - _routeCmd("change",leftt,_systemVia,_systemDevice,(const char *)0); + //_routeCmd("change",leftt,_systemVia,_systemDevice,(const char *)0); if (rightt) { _routeCmd("add",rightt,_systemVia,_systemDevice,(const char *)0); - _routeCmd("change",rightt,_systemVia,_systemDevice,(const char *)0); + //_routeCmd("change",rightt,_systemVia,_systemDevice,(const char *)0); } } } if (!_applied.count(leftt)) { _applied[leftt] = !_via; + _routeCmd("delete",leftt,_via,(const char *)0,(_via) ? (const char *)0 : _device); _routeCmd("add",leftt,_via,(const char *)0,(_via) ? (const char *)0 : _device); - _routeCmd("change",leftt,_via,(const char *)0,(_via) ? (const char *)0 : _device); + //_routeCmd("change",leftt,_via,(const char *)0,(_via) ? (const char *)0 : _device); } if ((rightt)&&(!_applied.count(rightt))) { _applied[rightt] = !_via; + _routeCmd("delete",rightt,_via,(const char *)0,(_via) ? (const char *)0 : _device); _routeCmd("add",rightt,_via,(const char *)0,(_via) ? (const char *)0 : _device); - _routeCmd("change",rightt,_via,(const char *)0,(_via) ? (const char *)0 : _device); + //_routeCmd("change",rightt,_via,(const char *)0,(_via) ? (const char *)0 : _device); } #endif // __BSD__ ------------------------------------------------------------ From a20a29083621fbb42b5dfc0712c8839636f73555 Mon Sep 17 00:00:00 2001 From: Grant Limberg Date: Wed, 15 Sep 2021 15:27:19 -0700 Subject: [PATCH 3/9] ifdef this out --- controller/PostgreSQL.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/controller/PostgreSQL.cpp b/controller/PostgreSQL.cpp index 50b128418..5f71a3699 100644 --- a/controller/PostgreSQL.cpp +++ b/controller/PostgreSQL.cpp @@ -157,7 +157,9 @@ PostgreSQL::PostgreSQL(const Identity &myId, const char *path, int listenPort, R memset(_ssoPsk, 0, sizeof(_ssoPsk)); char *const ssoPskHex = getenv("ZT_SSO_PSK"); +#ifdef ZT_TRACE fprintf(stderr, "ZT_SSO_PSK: %s\n", ssoPskHex); +#endif if (ssoPskHex) { // SECURITY: note that ssoPskHex will always be null-terminated if libc acatually // returns something non-NULL. If the hex encodes something shorter than 48 bytes, From 39b97f91633dd8dca4a1c0834d49b7f172e9b935 Mon Sep 17 00:00:00 2001 From: Adam Ierymenko Date: Mon, 20 Sep 2021 16:15:59 -0400 Subject: [PATCH 4/9] Don't assume roots validated the identity, just in case they did not. --- node/IncomingPacket.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/node/IncomingPacket.cpp b/node/IncomingPacket.cpp index 5a2a94642..ae6c1a849 100644 --- a/node/IncomingPacket.cpp +++ b/node/IncomingPacket.cpp @@ -511,7 +511,10 @@ bool IncomingPacket::_doOK(const RuntimeEnvironment *RR,void *tPtr,const SharedP case Packet::VERB_WHOIS: if (RR->topology->isUpstream(peer->identity())) { const Identity id(*this,ZT_PROTO_VERB_WHOIS__OK__IDX_IDENTITY); - RR->sw->doAnythingWaitingForPeer(tPtr,RR->topology->addPeer(tPtr,SharedPtr(new Peer(RR,RR->identity,id)))); + // Good idea to locally validate here even if roots are doing so. In a truly distributed + // system there should not be single points of failure for global trust assertions. + if (id.locallyValidate()) + RR->sw->doAnythingWaitingForPeer(tPtr,RR->topology->addPeer(tPtr,SharedPtr(new Peer(RR,RR->identity,id)))); } break; From 7c3166e9be15e67295b12f92429e2e1a8d24ddd6 Mon Sep 17 00:00:00 2001 From: Adam Ierymenko Date: Mon, 20 Sep 2021 18:26:49 -0400 Subject: [PATCH 5/9] 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 | 2 +- node/CertificateOfMembership.cpp | 52 +++++++++++++----------- node/CertificateOfMembership.hpp | 22 ++++++++-- node/Identity.hpp | 12 ++++++ node/Membership.hpp | 7 ++-- node/Network.cpp | 9 ++-- selftest.cpp | 16 ++++---- 7 files changed, 78 insertions(+), 42 deletions(-) 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; From 3f49570f45065df875eb2b3f31e2eba73522f29b Mon Sep 17 00:00:00 2001 From: Adam Ierymenko Date: Mon, 20 Sep 2021 18:38:29 -0400 Subject: [PATCH 6/9] Remove ancient controller support. --- RELEASE-NOTES.md | 8 ++ node/CertificateOfMembership.cpp | 158 ++++++------------------------- node/CertificateOfMembership.hpp | 56 +---------- node/Constants.hpp | 5 - 4 files changed, 36 insertions(+), 191 deletions(-) diff --git a/RELEASE-NOTES.md b/RELEASE-NOTES.md index a9835cb68..4404eaf87 100644 --- a/RELEASE-NOTES.md +++ b/RELEASE-NOTES.md @@ -1,6 +1,14 @@ ZeroTier Release Notes ====== +# -- Version 1.8.1 + + * Fix UI issues on MacOS Mojave + * Fix icon not showing on Windows + * Re-eneable installation on Windows 7, 8, etc., but without any guarantee that it will work there! (7 is not supported) + * Add an extended hash verification to certificates of network membership to further harden against impersonation attacks + * Remove support for REALLY ancient 1.1.6 or earlier network controllers + # 2021-09-15 -- Version 1.8.0 * A *completely* rewritten desktop UI for Mac and Windows! diff --git a/node/CertificateOfMembership.cpp b/node/CertificateOfMembership.cpp index 2580041c8..dbda9939f 100644 --- a/node/CertificateOfMembership.cpp +++ b/node/CertificateOfMembership.cpp @@ -20,134 +20,32 @@ namespace ZeroTier { -void CertificateOfMembership::setQualifier(uint64_t id,uint64_t value,uint64_t maxDelta) +CertificateOfMembership::CertificateOfMembership(uint64_t timestamp,uint64_t timestampMaxDelta,uint64_t nwid,const Identity &issuedTo) { - _signedBy.zero(); + _qualifiers[0].id = COM_RESERVED_ID_TIMESTAMP; + _qualifiers[0].value = timestamp; + _qualifiers[0].maxDelta = timestampMaxDelta; + _qualifiers[1].id = COM_RESERVED_ID_NETWORK_ID; + _qualifiers[1].value = nwid; + _qualifiers[1].maxDelta = 0; + _qualifiers[2].id = COM_RESERVED_ID_ISSUED_TO; + _qualifiers[2].value = issuedTo.address().toInt(); + _qualifiers[2].maxDelta = 0xffffffffffffffffULL; - for(unsigned int i=0;i<_qualifierCount;++i) { - if (_qualifiers[i].id == id) { - _qualifiers[i].value = value; - _qualifiers[i].maxDelta = maxDelta; - return; - } + // 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; } - if (_qualifierCount < ZT_NETWORK_COM_MAX_QUALIFIERS) { - _qualifiers[_qualifierCount].id = id; - _qualifiers[_qualifierCount].value = value; - _qualifiers[_qualifierCount].maxDelta = maxDelta; - ++_qualifierCount; - std::sort(&(_qualifiers[0]),&(_qualifiers[_qualifierCount])); - } -} - -#ifdef ZT_SUPPORT_OLD_STYLE_NETCONF - -std::string CertificateOfMembership::toString() const -{ - char tmp[ZT_NETWORK_COM_MAX_QUALIFIERS * 32]; - std::string s; - - s.append("1:"); // COM_UINT64_ED25519 - - uint64_t *const buf = new uint64_t[_qualifierCount * 3]; - try { - unsigned int ptr = 0; - for(unsigned int i=0;i<_qualifierCount;++i) { - buf[ptr++] = Utils::hton(_qualifiers[i].id); - buf[ptr++] = Utils::hton(_qualifiers[i].value); - buf[ptr++] = Utils::hton(_qualifiers[i].maxDelta); - } - s.append(Utils::hex(buf,ptr * sizeof(uint64_t),tmp)); - delete [] buf; - } catch ( ... ) { - delete [] buf; - throw; - } - - s.push_back(':'); - - s.append(_signedBy.toString(tmp)); - - if (_signedBy) { - s.push_back(':'); - s.append(Utils::hex(_signature.data,ZT_C25519_SIGNATURE_LEN,tmp)); - } - - return s; -} - -void CertificateOfMembership::fromString(const char *s) -{ - _qualifierCount = 0; - _signedBy.zero(); + _qualifierCount = 7; memset(_signature.data,0,ZT_C25519_SIGNATURE_LEN); - - if (!*s) - return; - - unsigned int colonAt = 0; - while ((s[colonAt])&&(s[colonAt] != ':')) ++colonAt; - - if (!((colonAt == 1)&&(s[0] == '1'))) // COM_UINT64_ED25519? - return; - - s += colonAt + 1; - colonAt = 0; - while ((s[colonAt])&&(s[colonAt] != ':')) ++colonAt; - - if (colonAt) { - const unsigned int buflen = colonAt / 2; - char *const buf = new char[buflen]; - unsigned int bufactual = Utils::unhex(s,colonAt,buf,buflen); - char *bufptr = buf; - try { - while (bufactual >= 24) { - if (_qualifierCount < ZT_NETWORK_COM_MAX_QUALIFIERS) { - _qualifiers[_qualifierCount].id = Utils::ntoh(*((uint64_t *)bufptr)); bufptr += 8; - _qualifiers[_qualifierCount].value = Utils::ntoh(*((uint64_t *)bufptr)); bufptr += 8; - _qualifiers[_qualifierCount].maxDelta = Utils::ntoh(*((uint64_t *)bufptr)); bufptr += 8; - ++_qualifierCount; - } else { - bufptr += 24; - } - bufactual -= 24; - } - } catch ( ... ) {} - delete [] buf; - } - - if (s[colonAt]) { - s += colonAt + 1; - colonAt = 0; - while ((s[colonAt])&&(s[colonAt] != ':')) ++colonAt; - - if (colonAt) { - char addrbuf[ZT_ADDRESS_LENGTH]; - if (Utils::unhex(s,colonAt,addrbuf,sizeof(addrbuf)) == ZT_ADDRESS_LENGTH) - _signedBy.setTo(addrbuf,ZT_ADDRESS_LENGTH); - - if ((_signedBy)&&(s[colonAt])) { - s += colonAt + 1; - colonAt = 0; - while ((s[colonAt])&&(s[colonAt] != ':')) ++colonAt; - if (colonAt) { - if (Utils::unhex(s,colonAt,_signature.data,ZT_C25519_SIGNATURE_LEN) != ZT_C25519_SIGNATURE_LEN) - _signedBy.zero(); - } else { - _signedBy.zero(); - } - } else { - _signedBy.zero(); - } - } - } - - std::sort(&(_qualifiers[0]),&(_qualifiers[_qualifierCount])); } -#endif // ZT_SUPPORT_OLD_STYLE_NETCONF - bool CertificateOfMembership::agreesWith(const CertificateOfMembership &other, const Identity &otherIdentity) const { if ((_qualifierCount == 0)||(other._qualifierCount == 0)) @@ -160,17 +58,15 @@ bool CertificateOfMembership::agreesWith(const CertificateOfMembership &other, c bool fullIdentityVerification = false; for(unsigned int i=0;i<_qualifierCount;++i) { const uint64_t qid = _qualifiers[i].id; - if ((qid >= 3)&&(qid <= 6)) { + 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; - } + 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; } // If this COM has a full hash of its identity, assume the other must have this as well. diff --git a/node/CertificateOfMembership.hpp b/node/CertificateOfMembership.hpp index 0e4e04748..1948dd7b7 100644 --- a/node/CertificateOfMembership.hpp +++ b/node/CertificateOfMembership.hpp @@ -112,31 +112,7 @@ public: * @param nwid Network ID * @param issuedTo Certificate recipient */ - CertificateOfMembership(uint64_t timestamp,uint64_t timestampMaxDelta,uint64_t nwid,const Identity &issuedTo) - { - _qualifiers[0].id = COM_RESERVED_ID_TIMESTAMP; - _qualifiers[0].value = timestamp; - _qualifiers[0].maxDelta = timestampMaxDelta; - _qualifiers[1].id = COM_RESERVED_ID_NETWORK_ID; - _qualifiers[1].value = nwid; - _qualifiers[1].maxDelta = 0; - _qualifiers[2].id = COM_RESERVED_ID_ISSUED_TO; - _qualifiers[2].value = issuedTo.address().toInt(); - _qualifiers[2].maxDelta = 0xffffffffffffffffULL; - - // 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); - } + CertificateOfMembership(uint64_t timestamp,uint64_t timestampMaxDelta,uint64_t nwid,const Identity &issuedTo); /** * Create from binary-serialized COM in buffer @@ -196,36 +172,6 @@ public: return 0ULL; } - /** - * Add or update a qualifier in this certificate - * - * Any signature is invalidated and signedBy is set to null. - * - * @param id Qualifier ID - * @param value Qualifier value - * @param maxDelta Qualifier maximum allowed difference (absolute value of difference) - */ - void setQualifier(uint64_t id,uint64_t value,uint64_t maxDelta); - inline void setQualifier(ReservedId id,uint64_t value,uint64_t maxDelta) { setQualifier((uint64_t)id,value,maxDelta); } - -#ifdef ZT_SUPPORT_OLD_STYLE_NETCONF - /** - * @return String-serialized representation of this certificate - */ - std::string toString() const; - - /** - * Set this certificate equal to the hex-serialized string - * - * Invalid strings will result in invalid or undefined certificate - * contents. These will subsequently fail validation and comparison. - * Empty strings will result in an empty certificate. - * - * @param s String to deserialize - */ - void fromString(const char *s); -#endif // ZT_SUPPORT_OLD_STYLE_NETCONF - /** * Compare two certificates for parameter agreement * diff --git a/node/Constants.hpp b/node/Constants.hpp index 5d63f4f28..21d0754ec 100644 --- a/node/Constants.hpp +++ b/node/Constants.hpp @@ -646,11 +646,6 @@ */ #define ZT_TRUST_EXPIRATION 600000 -/** - * Enable support for older network configurations from older (pre-1.1.6) controllers - */ -#define ZT_SUPPORT_OLD_STYLE_NETCONF 1 - /** * Desired buffer size for UDP sockets (used in service and osdep but defined here) */ From b72e5e8386b73425f425b64cd81a7c53983d7fcf Mon Sep 17 00:00:00 2001 From: Adam Ierymenko Date: Mon, 20 Sep 2021 20:02:39 -0400 Subject: [PATCH 7/9] Use a faster method of fingerprinting identities. --- node/CertificateOfMembership.cpp | 6 +++--- node/Identity.hpp | 26 ++++++++++++++------------ 2 files changed, 17 insertions(+), 15 deletions(-) diff --git a/node/CertificateOfMembership.cpp b/node/CertificateOfMembership.cpp index dbda9939f..63a8efeca 100644 --- a/node/CertificateOfMembership.cpp +++ b/node/CertificateOfMembership.cpp @@ -34,8 +34,8 @@ CertificateOfMembership::CertificateOfMembership(uint64_t timestamp,uint64_t tim // 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); + uint64_t idHash[4]; + issuedTo.keyFingerprint(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]); @@ -73,7 +73,7 @@ bool CertificateOfMembership::agreesWith(const CertificateOfMembership &other, c // Otherwise we are on a controller that does not incorporate these. if (fullIdentityVerification) { uint64_t idHash[6]; - otherIdentity.publicKeyHash(idHash); + otherIdentity.keyFingerprint(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()) diff --git a/node/Identity.hpp b/node/Identity.hpp index cc8de5126..ae31e3963 100644 --- a/node/Identity.hpp +++ b/node/Identity.hpp @@ -23,6 +23,7 @@ #include "C25519.hpp" #include "Buffer.hpp" #include "SHA512.hpp" +#include "AES.hpp" #define ZT_IDENTITY_STRING_BUFFER_LENGTH 384 @@ -109,18 +110,6 @@ 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) * @@ -136,6 +125,19 @@ public: return false; } + /** + * Get a 256-bit hash of this identity's public key(s) + * + * @param buf 256-bit (32-byte) buffer + */ + inline void keyFingerprint(void *buf) const + { + // This is much faster than SHA384, which matters on heavily loaded controllers. + AES c(_publicKey.data); + c.encrypt(_publicKey.data + 32, buf); + c.encrypt(_publicKey.data + 48, reinterpret_cast(buf) + 16); + } + /** * Sign a message with this identity (private key required) * From a0239e17e9cc9a47f6e1ca4e66751c5fbde45e87 Mon Sep 17 00:00:00 2001 From: Adam Ierymenko Date: Mon, 20 Sep 2021 22:05:39 -0400 Subject: [PATCH 8/9] Revert "Use a faster method of fingerprinting identities." This reverts commit b72e5e8386b73425f425b64cd81a7c53983d7fcf. --- node/CertificateOfMembership.cpp | 6 +++--- node/Identity.hpp | 26 ++++++++++++-------------- 2 files changed, 15 insertions(+), 17 deletions(-) diff --git a/node/CertificateOfMembership.cpp b/node/CertificateOfMembership.cpp index 63a8efeca..dbda9939f 100644 --- a/node/CertificateOfMembership.cpp +++ b/node/CertificateOfMembership.cpp @@ -34,8 +34,8 @@ CertificateOfMembership::CertificateOfMembership(uint64_t timestamp,uint64_t tim // 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[4]; - issuedTo.keyFingerprint(idHash); + 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]); @@ -73,7 +73,7 @@ bool CertificateOfMembership::agreesWith(const CertificateOfMembership &other, c // Otherwise we are on a controller that does not incorporate these. if (fullIdentityVerification) { uint64_t idHash[6]; - otherIdentity.keyFingerprint(idHash); + 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()) diff --git a/node/Identity.hpp b/node/Identity.hpp index ae31e3963..cc8de5126 100644 --- a/node/Identity.hpp +++ b/node/Identity.hpp @@ -23,7 +23,6 @@ #include "C25519.hpp" #include "Buffer.hpp" #include "SHA512.hpp" -#include "AES.hpp" #define ZT_IDENTITY_STRING_BUFFER_LENGTH 384 @@ -110,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) * @@ -125,19 +136,6 @@ public: return false; } - /** - * Get a 256-bit hash of this identity's public key(s) - * - * @param buf 256-bit (32-byte) buffer - */ - inline void keyFingerprint(void *buf) const - { - // This is much faster than SHA384, which matters on heavily loaded controllers. - AES c(_publicKey.data); - c.encrypt(_publicKey.data + 32, buf); - c.encrypt(_publicKey.data + 48, reinterpret_cast(buf) + 16); - } - /** * Sign a message with this identity (private key required) * From 9bc79f94df216f5914c80380b6c97f776701ac6f Mon Sep 17 00:00:00 2001 From: Adam Ierymenko Date: Mon, 20 Sep 2021 22:05:49 -0400 Subject: [PATCH 9/9] Revert "Don't assume roots validated the identity, just in case they did not." This reverts commit 39b97f91633dd8dca4a1c0834d49b7f172e9b935. --- node/IncomingPacket.cpp | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/node/IncomingPacket.cpp b/node/IncomingPacket.cpp index ae6c1a849..5a2a94642 100644 --- a/node/IncomingPacket.cpp +++ b/node/IncomingPacket.cpp @@ -511,10 +511,7 @@ bool IncomingPacket::_doOK(const RuntimeEnvironment *RR,void *tPtr,const SharedP case Packet::VERB_WHOIS: if (RR->topology->isUpstream(peer->identity())) { const Identity id(*this,ZT_PROTO_VERB_WHOIS__OK__IDX_IDENTITY); - // Good idea to locally validate here even if roots are doing so. In a truly distributed - // system there should not be single points of failure for global trust assertions. - if (id.locallyValidate()) - RR->sw->doAnythingWaitingForPeer(tPtr,RR->topology->addPeer(tPtr,SharedPtr(new Peer(RR,RR->identity,id)))); + RR->sw->doAnythingWaitingForPeer(tPtr,RR->topology->addPeer(tPtr,SharedPtr(new Peer(RR,RR->identity,id)))); } break;