diff --git a/node/CertificateOfMembership.cpp b/node/CertificateOfMembership.cpp index d897ce699..9dcc634f7 100644 --- a/node/CertificateOfMembership.cpp +++ b/node/CertificateOfMembership.cpp @@ -28,10 +28,10 @@ bool CertificateOfMembership::agreesWith(const CertificateOfMembership &other) c // conditions that could introduce a vulnerability. if (other._timestamp > _timestamp) { - if ((other._timestamp - _timestamp) > _timestampMaxDelta) + if ((other._timestamp - _timestamp) > std::min(_timestampMaxDelta,other._timestampMaxDelta)) return false; } else { - if ((_timestamp - other._timestamp) > _timestampMaxDelta) + if ((_timestamp - other._timestamp) > std::min(_timestampMaxDelta,other._timestampMaxDelta)) return false; } @@ -81,7 +81,7 @@ bool CertificateOfMembership::agreesWith(const CertificateOfMembership &other) c // SECURITY: check for issued-to inequality is a sanity check. This should be impossible since elsewhere // in the code COMs are checked to ensure that they do in fact belong to their issued-to identities. - return (other._networkId != _networkId) && (other._issuedTo.address() != _issuedTo.address()); + return (other._networkId == _networkId) && (_networkId != 0) && (other._issuedTo.address() != _issuedTo.address()); } bool CertificateOfMembership::sign(const Identity &with) noexcept @@ -214,7 +214,7 @@ int CertificateOfMembership::unmarshal(const uint8_t *data,int len) noexcept if ((p + 2) > len) return -1; _signatureLength = Utils::loadBigEndian(data + p); - if ((_signatureLength > sizeof(_signature))||((p + _signatureLength) > len)) + if ((_signatureLength > (unsigned int)sizeof(_signature))||((p + (int)_signatureLength) > len)) return -1; memcpy(_signature,data + p,_signatureLength); return p + (int)_signatureLength; diff --git a/node/CertificateOfMembership.hpp b/node/CertificateOfMembership.hpp index 73cc55a96..0317c7f24 100644 --- a/node/CertificateOfMembership.hpp +++ b/node/CertificateOfMembership.hpp @@ -179,8 +179,10 @@ public: */ ZT_INLINE Credential::VerifyResult verify(const RuntimeEnvironment *RR,void *tPtr) const { return _verify(RR,tPtr,*this); } + // NOTE: right now we use v1 serialization format which works with both ZeroTier 1.x and 2.x. V2 format + // will be switched on once 1.x is pretty much dead and out of support. static constexpr int marshalSizeMax() noexcept { return ZT_CERTIFICATEOFMEMBERSHIP_MARSHAL_SIZE_MAX; } - int marshal(uint8_t data[ZT_CERTIFICATEOFMEMBERSHIP_MARSHAL_SIZE_MAX],bool v2) const noexcept; + int marshal(uint8_t data[ZT_CERTIFICATEOFMEMBERSHIP_MARSHAL_SIZE_MAX],bool v2 = false) const noexcept; int unmarshal(const uint8_t *data,int len) noexcept; private: diff --git a/node/Membership.cpp b/node/Membership.cpp index 1490c87cf..1d6ea87b6 100644 --- a/node/Membership.cpp +++ b/node/Membership.cpp @@ -24,6 +24,8 @@ namespace ZeroTier { Membership::Membership() : _comRevocationThreshold(0), _lastPushedCredentials(0), + _comAgreementLocalTimestamp(0), + _comAgreementRemoteTimestamp(0), _revocations(4), _remoteTags(4), _remoteCaps(4), @@ -35,7 +37,7 @@ Membership::~Membership() { } -void Membership::pushCredentials(const RuntimeEnvironment *RR,void *tPtr,const int64_t now,const Address &peerAddress,const NetworkConfig &nconf) +void Membership::pushCredentials(const RuntimeEnvironment *RR,void *tPtr,const int64_t now,const Identity &to,const NetworkConfig &nconf) { if (!nconf.com) // sanity check return; @@ -48,7 +50,7 @@ void Membership::pushCredentials(const RuntimeEnvironment *RR,void *tPtr,const i bool complete = false; while (!complete) { ph.packetId = Protocol::getPacketId(); - peerAddress.copyTo(ph.destination); + to.address().copyTo(ph.destination); RR->identity.address().copyTo(ph.source); ph.flags = 0; ph.verb = Protocol::VERB_NETWORK_CREDENTIALS; diff --git a/node/Membership.hpp b/node/Membership.hpp index 193dc09fb..4be3bd208 100644 --- a/node/Membership.hpp +++ b/node/Membership.hpp @@ -57,29 +57,16 @@ public: * @param RR Runtime environment * @param tPtr Thread pointer to be handed through to any callbacks called as a result of this call * @param now Current time - * @param peerAddress Address of member peer (the one that this Membership describes) + * @param to Peer identity * @param nconf My network config */ - void pushCredentials(const RuntimeEnvironment *RR,void *tPtr,int64_t now,const Address &peerAddress,const NetworkConfig &nconf); + void pushCredentials(const RuntimeEnvironment *RR,void *tPtr,int64_t now,const Identity &to,const NetworkConfig &nconf); /** * @return Time we last pushed credentials to this member */ ZT_INLINE int64_t lastPushedCredentials() const noexcept { return _lastPushedCredentials; } - /** - * Check whether the peer represented by this Membership should be allowed on this network at all - * - * @param nconf Our network config - * @return True if this peer is allowed on this network at all - */ - ZT_INLINE bool isAllowedOnNetwork(const NetworkConfig &nconf) const noexcept - { - if (nconf.isPublic()) return true; // public network - if (_com.timestamp() <= _comRevocationThreshold) return false; // COM has been revoked - return nconf.com.agreesWith(_com); // check timestamp agreement window - } - /** * Check whether the peer represented by this Membership owns a given address * @@ -129,6 +116,42 @@ public: */ static ZT_INLINE uint64_t credentialKey(const ZT_CredentialType &t,const uint32_t i) noexcept { return (((uint64_t)t << 32U) | (uint64_t)i); } + /** + * Check if our local COM agrees with theirs, with possible memo-ization. + * + * @param localCom + */ + ZT_INLINE bool certificateOfMembershipAgress(const CertificateOfMembership &localCom,const Identity &remoteIdentity) + { + if ((_comAgreementLocalTimestamp == localCom.timestamp())&&(_comAgreementRemoteTimestamp == _com.timestamp())) + return true; + if (_com.agreesWith(localCom)) { + // SECURITY: newer network controllers embed the full fingerprint into the COM. If we are + // joined to a network managed by one of these, our COM will contain one. If it's present + // we compare vs the other and require them to match. If our COM does not contain a full + // identity fingerprint we compare by address only, which is a legacy mode supported for + // old network controllers. Note that this is safe because the controller issues us our COM + // and in so doing indicates if it's new or old. However this will go away after a while + // once we can be pretty sure there are no ancient controllers around. + if (localCom.issuedTo().haveHash()) { + if (localCom.issuedTo() != _com.issuedTo()) + return false; + } else { + // LEGACY: support networks run by old controllers. + if (localCom.issuedTo().address() != _com.issuedTo().address()) + return false; + } + + // Remember that these two COMs agreed. If any are updated this is invalidated and a full + // agreement check will be done again. + _comAgreementLocalTimestamp = localCom.timestamp(); + _comAgreementRemoteTimestamp = _com.timestamp(); + + return true; + } + return false; + } + AddCredentialResult addCredential(const RuntimeEnvironment *RR,void *tPtr,const Identity &sourcePeerIdentity,const NetworkConfig &nconf,const CertificateOfMembership &com); AddCredentialResult addCredential(const RuntimeEnvironment *RR,void *tPtr,const Identity &sourcePeerIdentity,const NetworkConfig &nconf,const Tag &tag); AddCredentialResult addCredential(const RuntimeEnvironment *RR,void *tPtr,const Identity &sourcePeerIdentity,const NetworkConfig &nconf,const Capability &cap); @@ -173,6 +196,9 @@ private: // Time we last pushed credentials int64_t _lastPushedCredentials; + // COM timestamps at which we last agreed-- used to memo-ize agreement and avoid having to recompute constantly. + int64_t _comAgreementLocalTimestamp,_comAgreementRemoteTimestamp; + // Remote member's latest network COM CertificateOfMembership _com; diff --git a/node/Network.cpp b/node/Network.cpp index 1820d9661..b0bbeaf2f 100644 --- a/node/Network.cpp +++ b/node/Network.cpp @@ -1051,19 +1051,28 @@ int Network::setConfiguration(void *tPtr,const NetworkConfig &nconf,bool saveToD return 0; } -bool Network::gate(void *tPtr,const SharedPtr &peer) +bool Network::gate(void *tPtr,const SharedPtr &peer) noexcept { - Mutex::Lock l(_memberships_l); + Mutex::Lock lc(_config_l); + + if (!_config) + return false; + if (_config.isPublic()) + return true; + try { - if (_config) { - Membership *m = _memberships.get(peer->address()); - if ( (_config.isPublic()) || ((m)&&(m->isAllowedOnNetwork(_config))) ) { - if (!m) - m = &(_memberships[peer->address()]); - return true; - } + Mutex::Lock l(_memberships_l); + Membership *m = _memberships.get(peer->address()); + if (m) { + // SECURITY: this method in CertificateOfMembership does a full fingerprint check as well as + // checking certificate agreement. See Membership.hpp. + return m->certificateOfMembershipAgress(_config.com,peer->identity()); + } else { + m = &(_memberships[peer->address()]); + return false; } } catch ( ... ) {} + return false; } @@ -1147,7 +1156,7 @@ Membership::AddCredentialResult Network::addCredential(void *tPtr,const Identity if (com.networkId() != _id) return Membership::ADD_REJECTED; Mutex::Lock _l(_memberships_l); - return _memberships[com.issuedTo()].addCredential(RR,tPtr,sourcePeerIdentity,_config,com); + return _memberships[com.issuedTo().address()].addCredential(RR,tPtr,sourcePeerIdentity,_config,com); } Membership::AddCredentialResult Network::addCredential(void *tPtr,const Identity &sourcePeerIdentity,const Capability &cap) @@ -1208,12 +1217,6 @@ Membership::AddCredentialResult Network::addCredential(void *tPtr,const Identity return _memberships[coo.issuedTo()].addCredential(RR,tPtr,sourcePeerIdentity,_config,coo); } -void Network::pushCredentialsNow(void *tPtr,const Address &to,const int64_t now) -{ - Mutex::Lock _l(_memberships_l); - _memberships[to].pushCredentials(RR,tPtr,now,to,_config); -} - void Network::destroy() { _memberships_l.lock(); diff --git a/node/Network.hpp b/node/Network.hpp index 13109b912..35389f0ec 100644 --- a/node/Network.hpp +++ b/node/Network.hpp @@ -226,7 +226,7 @@ public: * @param tPtr Thread pointer to be handed through to any callbacks called as a result of this call * @param peer Peer to check */ - bool gate(void *tPtr,const SharedPtr &peer); + bool gate(void *tPtr,const SharedPtr &peer) noexcept; /** * Do periodic cleanup and housekeeping tasks @@ -292,27 +292,18 @@ public: */ Membership::AddCredentialResult addCredential(void *tPtr,const Identity &sourcePeerIdentity,const CertificateOfOwnership &coo); - /** - * Force push credentials (COM, etc.) to a peer now - * - * @param tPtr Thread pointer to be handed through to any callbacks called as a result of this call - * @param to Destination peer address - * @param now Current time - */ - void pushCredentialsNow(void *tPtr,const Address &to,int64_t now); - /** * Push credentials if we haven't done so in a long time * * @param tPtr Thread pointer to be handed through to any callbacks called as a result of this call - * @param to Destination peer address + * @param to Destination peer * @param now Current time */ - ZT_INLINE void pushCredentialsIfNeeded(void *tPtr,const Address &to,const int64_t now) + ZT_INLINE void pushCredentialsIfNeeded(void *tPtr,const Identity &to,const int64_t now) { const int64_t tout = std::min(_config.credentialTimeMaxDelta,(int64_t)ZT_PEER_ACTIVITY_TIMEOUT); Mutex::Lock _l(_memberships_l); - Membership &m = _memberships[to]; + Membership &m = _memberships[to.address()]; if (((now - m.lastPushedCredentials()) + 5000) >= tout) m.pushCredentials(RR,tPtr,now,to,_config); } diff --git a/node/NetworkConfig.cpp b/node/NetworkConfig.cpp index 9d2ae89f2..fbb5833e9 100644 --- a/node/NetworkConfig.cpp +++ b/node/NetworkConfig.cpp @@ -39,8 +39,9 @@ bool NetworkConfig::toDictionary(Dictionary &d,bool includeLegacy) const d.add(ZT_NETWORKCONFIG_DICT_KEY_NAME,this->name); d.add(ZT_NETWORKCONFIG_DICT_KEY_MTU,this->mtu); - if (this->com) + if (this->com) { d.add(ZT_NETWORKCONFIG_DICT_KEY_COM,tmp,this->com.marshal(tmp)); + } std::vector *blob = &(d[ZT_NETWORKCONFIG_DICT_KEY_CAPABILITIES]); for (unsigned int i = 0; i < this->capabilityCount; ++i) { diff --git a/node/Trace.cpp b/node/Trace.cpp index 9862f954d..9430ad45d 100644 --- a/node/Trace.cpp +++ b/node/Trace.cpp @@ -116,12 +116,12 @@ void Trace::_tryingNewPath( ev.evSize = ZT_CONST_TO_BE_UINT16(sizeof(ev)); ev.evType = ZT_CONST_TO_BE_UINT16(ZT_TRACE_VL1_TRYING_NEW_PATH); ev.codeLocation = Utils::hton(codeLocation); - trying.fingerprint().getAPIFingerprint(&ev.peer); + memcpy(&ev.peer,trying.fingerprint().apiFingerprint(),sizeof(ev.peer)); physicalAddress.forTrace(ev.physicalAddress); triggerAddress.forTrace(ev.triggerAddress); ev.triggeringPacketId = triggeringPacketId; ev.triggeringPacketVerb = triggeringPacketVerb; - triggeringPeer.fingerprint().getAPIFingerprint(&ev.triggeringPeer); + memcpy(&ev.triggeringPeer,triggeringPeer.fingerprint().apiFingerprint(),sizeof(ev.triggeringPeer)); ev.reason = (uint8_t)reason; RR->node->postEvent(tPtr,ZT_EVENT_TRACE,&ev); } @@ -139,7 +139,7 @@ void Trace::_learnedNewPath( ev.evType = ZT_CONST_TO_BE_UINT16(ZT_TRACE_VL1_LEARNED_NEW_PATH); ev.codeLocation = Utils::hton(codeLocation); ev.packetId = packetId; // packet IDs are kept in big-endian - peerIdentity.fingerprint().getAPIFingerprint(&ev.peer); + memcpy(&ev.peer,peerIdentity.fingerprint().apiFingerprint(),sizeof(ev.peer)); physicalAddress.forTrace(ev.physicalAddress); replaced.forTrace(ev.replaced); @@ -163,7 +163,7 @@ void Trace::_incomingPacketDropped( ev.codeLocation = Utils::hton(codeLocation); ev.packetId = packetId; // packet IDs are kept in big-endian ev.networkId = Utils::hton(networkId); - peerIdentity.fingerprint().getAPIFingerprint(&ev.peer); + memcpy(&ev.peer,peerIdentity.fingerprint().apiFingerprint(),sizeof(ev.peer)); physicalAddress.forTrace(ev.physicalAddress); ev.hops = hops; ev.verb = verb; @@ -226,7 +226,7 @@ void Trace::_incomingNetworkFrameDropped( ev.networkId = Utils::hton(networkId); ev.sourceMac = Utils::hton(sourceMac.toInt()); ev.destMac = Utils::hton(destMac.toInt()); - peerIdentity.fingerprint().getAPIFingerprint(&ev.sender); + memcpy(&ev.sender,peerIdentity.fingerprint().apiFingerprint(),sizeof(ev.sender)); physicalAddress.forTrace(ev.physicalAddress); ev.hops = hops; ev.frameLength = Utils::hton(frameLength); @@ -325,7 +325,7 @@ void Trace::_credentialRejected( ev.codeLocation = Utils::hton(codeLocation); ev.networkId = Utils::hton(networkId); if (identity) { - identity.fingerprint().getAPIFingerprint(&ev.peer); + memcpy(&ev.peer,identity.fingerprint().apiFingerprint(),sizeof(ev.peer)); } else { ev.peer.address = address.toInt(); memset(ev.peer.hash,0,sizeof(ev.peer.hash));