From 5280d285050e805daa7d8082d91277a20b64d998 Mon Sep 17 00:00:00 2001 From: Adam Ierymenko Date: Wed, 21 Aug 2019 10:44:52 -0700 Subject: [PATCH] cleanup --- node/CertificateOfMembership.hpp | 14 ++--- node/Locator.hpp | 14 ++--- node/Membership.hpp | 7 +-- node/Network.cpp | 45 ++++++++-------- node/Network.hpp | 88 +++++++++++++++++--------------- node/Packet.hpp | 3 +- 6 files changed, 89 insertions(+), 82 deletions(-) diff --git a/node/CertificateOfMembership.hpp b/node/CertificateOfMembership.hpp index dcb2a0537..4dc38ad06 100644 --- a/node/CertificateOfMembership.hpp +++ b/node/CertificateOfMembership.hpp @@ -246,16 +246,16 @@ public: { 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; - + // 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. @@ -264,17 +264,17 @@ public: if (otheridx >= other._qualifierCount) 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; } - + return true; } @@ -293,7 +293,7 @@ public: buf[ptr++] = Utils::hton(_qualifiers[i].value); buf[ptr++] = Utils::hton(_qualifiers[i].maxDelta); } - + try { _signatureLength = with.sign(buf,ptr * sizeof(uint64_t),_signature,sizeof(_signature)); _signedBy = with.address(); diff --git a/node/Locator.hpp b/node/Locator.hpp index 83ebc6d20..75ee14be2 100644 --- a/node/Locator.hpp +++ b/node/Locator.hpp @@ -45,7 +45,7 @@ namespace ZeroTier { /** * Signed information about a node's location on the network - * + * * A locator is a signed record that contains information about where a node * may be found. It can contain static physical addresses or virtual ZeroTier * addresses of nodes that can forward to the target node. Locator records @@ -87,7 +87,7 @@ public: /** * Method to be called after add() is called for each address or forwarding node - * + * * This sets timestamp and ID information and sorts and deduplicates target * lists but does not sign the locator. The sign() method should be used after * finish(). @@ -148,12 +148,12 @@ public: /** * Make DNS TXT records for this locator - * + * * DNS TXT records are signed by an entirely separate key that is added along * with DNS names to nodes to allow them to verify DNS results. It's separate * from the locator's signature so that a single DNS record can point to more * than one locator or be served by things like geo-aware DNS. - * + * * Right now only NIST P-384 is supported for signing DNS records. NIST EDDSA * is used here so that FIPS-only nodes can always use DNS to locate roots as * FIPS-only nodes may be required to disable non-FIPS algorithms. @@ -191,11 +191,11 @@ public: /** * Decode TXT records - * + * * TXT records can be provided as an iterator over std::string, Str, or char * * values, and TXT records can be provided in any order. Any oversize or empty * entries will be ignored. - * + * * This method checks the decoded locator's signature using the supplied DNS TXT * record signing public key. False is returned if the TXT records are invalid, * incomplete, or fail signature check. If true is returned this Locator object @@ -311,6 +311,8 @@ public: inline operator bool() const { return (_id); } + inline bool addressesEqual(const Locator &l) const { return ((_physical == l._physical)&&(_virtual == l._virtual)); } + inline bool operator==(const Locator &l) const { return ((_ts == l._ts)&&(_id == l._id)&&(_signedBy == l._signedBy)&&(_physical == l._physical)&&(_virtual == l._virtual)&&(_signatureLength == l._signatureLength)&&(memcmp(_signature,l._signature,_signatureLength) == 0)); } inline bool operator!=(const Locator &l) const { return (!(*this == l)); } inline bool operator<(const Locator &l) const diff --git a/node/Membership.hpp b/node/Membership.hpp index 3fc29056a..ba0bea205 100644 --- a/node/Membership.hpp +++ b/node/Membership.hpp @@ -78,12 +78,9 @@ public: void pushCredentials(const RuntimeEnvironment *RR,void *tPtr,const int64_t now,const Address &peerAddress,const NetworkConfig &nconf); /** - * @return True if we haven't pushed credentials in a long time (to cause proactive credential push) + * @return Time we last pushed credentials to this member */ - inline bool shouldPushCredentials(const int64_t now) const - { - return ((now - _lastPushedCredentials) > ZT_PEER_ACTIVITY_TIMEOUT); - } + inline int64_t lastPushedCredentials() const { return _lastPushedCredentials; } /** * Check whether we should push MULTICAST_LIKEs to this peer, and update last sent time if true diff --git a/node/Network.cpp b/node/Network.cpp index 11b3044a2..eb004f7a0 100644 --- a/node/Network.cpp +++ b/node/Network.cpp @@ -606,6 +606,11 @@ Network::Network(const RuntimeEnvironment *renv,void *tPtr,uint64_t nwid,void *u Network::~Network() { + _memberships_l.lock(); + _config_l.lock(); + _config_l.unlock(); + _memberships_l.unlock(); + ZT_VirtualNetworkConfig ctmp; _externalConfig(&ctmp); @@ -638,7 +643,8 @@ bool Network::filterOutgoingPacket( unsigned int ccLength = 0; bool ccWatch = false; - Mutex::Lock _l(_lock); + Mutex::Lock l1(_memberships_l); + Mutex::Lock l2(_config_l); Membership *const membership = (ztDest) ? _memberships.get(ztDest) : (Membership *)0; @@ -755,7 +761,8 @@ int Network::filterIncomingPacket( uint8_t qosBucket = 255; // For incoming packets this is a dummy value - Mutex::Lock _l(_lock); + Mutex::Lock l1(_memberships_l); + Mutex::Lock l2(_config_l); Membership &membership = _memberships[sourcePeer->address()]; @@ -861,7 +868,7 @@ uint64_t Network::handleConfigChunk(void *tPtr,const uint64_t packetId,const Add NetworkConfig *nc = (NetworkConfig *)0; uint64_t configUpdateId; { - Mutex::Lock _l(_lock); + Mutex::Lock l1(_config_l); _IncomingConfigChunk *c = (_IncomingConfigChunk *)0; uint64_t chunkId = 0; @@ -907,6 +914,7 @@ uint64_t Network::handleConfigChunk(void *tPtr,const uint64_t packetId,const Add // New properly verified chunks can be flooded "virally" through the network if (fastPropagate) { + Mutex::Lock l2(_memberships_l); Address *a = (Address *)0; Membership *m = (Membership *)0; Hashtable::Iterator i(_memberships); @@ -993,7 +1001,7 @@ int Network::setConfiguration(void *tPtr,const NetworkConfig &nconf,bool saveToD ZT_VirtualNetworkConfig ctmp; bool oldPortInitialized; { // do things that require lock here, but unlock before calling callbacks - Mutex::Lock _l(_lock); + Mutex::Lock _l(_config_l); _config = nconf; _lastConfigUpdate = RR->node->now(); @@ -1206,15 +1214,17 @@ void Network::requestConfiguration(void *tPtr) bool Network::gate(void *tPtr,const SharedPtr &peer) { const int64_t now = RR->node->now(); - Mutex::Lock _l(_lock); + Mutex::Lock l(_memberships_l); try { if (_config) { Membership *m = _memberships.get(peer->address()); if ( (_config.isPublic()) || ((m)&&(m->isAllowedOnNetwork(_config))) ) { if (!m) m = &(_memberships[peer->address()]); - if (m->multicastLikeGate(now)) + if (m->multicastLikeGate(now)) { + Mutex::Lock l2(_myMulticastGroups_l); _announceMulticastGroupsTo(tPtr,peer->address(),_allMulticastGroups()); + } return true; } } @@ -1224,7 +1234,7 @@ bool Network::gate(void *tPtr,const SharedPtr &peer) bool Network::recentlyAssociatedWith(const Address &addr) { - Mutex::Lock _l(_lock); + Mutex::Lock _l(_memberships_l); const Membership *m = _memberships.get(addr); return ((m)&&(m->recentlyAssociated(RR->node->now()))); } @@ -1232,7 +1242,7 @@ bool Network::recentlyAssociatedWith(const Address &addr) void Network::clean() { const int64_t now = RR->node->now(); - Mutex::Lock _l(_lock); + Mutex::Lock _l(_memberships_l); if (_destroyed) return; @@ -1260,7 +1270,7 @@ Membership::AddCredentialResult Network::addCredential(void *tPtr,const Certific { if (com.networkId() != _id) return Membership::ADD_REJECTED; - Mutex::Lock _l(_lock); + Mutex::Lock _l(_memberships_l); return _memberships[com.issuedTo()].addCredential(RR,tPtr,_config,com); } @@ -1269,7 +1279,7 @@ Membership::AddCredentialResult Network::addCredential(void *tPtr,const Address if (rev.networkId() != _id) return Membership::ADD_REJECTED; - Mutex::Lock _l(_lock); + Mutex::Lock _l(_memberships_l); Membership &m = _memberships[rev.target()]; const Membership::AddCredentialResult result = m.addCredential(RR,tPtr,_config,rev); @@ -1295,15 +1305,8 @@ Membership::AddCredentialResult Network::addCredential(void *tPtr,const Address return result; } -void Network::destroy() -{ - Mutex::Lock _l(_lock); - _destroyed = true; -} - ZT_VirtualNetworkStatus Network::_status() const { - // assumes _lock is locked if (_portError) return ZT_NETWORK_STATUS_PORT_ERROR; switch(_netconfFailure) { @@ -1320,7 +1323,7 @@ ZT_VirtualNetworkStatus Network::_status() const void Network::_externalConfig(ZT_VirtualNetworkConfig *ec) const { - // assumes _lock is locked + // assumes _config_l is locked ec->nwid = _id; ec->mac = _mac.toInt(); if (_config) @@ -1363,9 +1366,9 @@ void Network::_externalConfig(ZT_VirtualNetworkConfig *ec) const } } -void Network::_sendUpdatesToMembers(void *tPtr) +void Network::_announceMulticastGroups(void *tPtr) { - // Assumes _lock is locked + // Assumes _myMulticastGroups_l and _memberships_l are locked const std::vector groups(_allMulticastGroups()); _announceMulticastGroupsTo(tPtr,controller(),groups); { @@ -1381,7 +1384,7 @@ void Network::_sendUpdatesToMembers(void *tPtr) void Network::_announceMulticastGroupsTo(void *tPtr,const Address &peer,const std::vector &allMulticastGroups) { - // Assumes _lock is locked + // Assumes _myMulticastGroups_l and _memberships_l are locked Packet *const outp = new Packet(peer,RR->identity.address(),Packet::VERB_MULTICAST_LIKE); for(std::vector::const_iterator mg(allMulticastGroups.begin());mg!=allMulticastGroups.end();++mg) { diff --git a/node/Network.hpp b/node/Network.hpp index 40bf487f8..cf62bac83 100644 --- a/node/Network.hpp +++ b/node/Network.hpp @@ -98,7 +98,7 @@ public: inline bool multicastEnabled() const { return (_config.multicastLimit > 0); } inline bool hasConfig() const { return (_config); } inline uint64_t lastConfigUpdate() const { return _lastConfigUpdate; } - inline ZT_VirtualNetworkStatus status() const { Mutex::Lock _l(_lock); return _status(); } + inline ZT_VirtualNetworkStatus status() const { return _status(); } inline const NetworkConfig &config() const { return _config; } inline const MAC &mac() const { return _mac; } @@ -174,14 +174,14 @@ public: */ inline bool subscribedToMulticastGroup(const MulticastGroup &mg,bool includeBridgedGroups) const { - Mutex::Lock _l(_lock); + Mutex::Lock _l(_myMulticastGroups_l); if (std::binary_search(_myMulticastGroups.begin(),_myMulticastGroups.end(),mg)) return true; else if (includeBridgedGroups) return _multicastGroupsBehindMe.contains(mg); return false; } - + /** * Subscribe to a multicast group * @@ -190,10 +190,11 @@ public: */ inline void multicastSubscribe(void *tPtr,const MulticastGroup &mg) { - Mutex::Lock _l(_lock); + Mutex::Lock _l(_myMulticastGroups_l); if (!std::binary_search(_myMulticastGroups.begin(),_myMulticastGroups.end(),mg)) { _myMulticastGroups.insert(std::upper_bound(_myMulticastGroups.begin(),_myMulticastGroups.end(),mg),mg); - _sendUpdatesToMembers(tPtr); + Mutex::Lock l2(_memberships_l); + _announceMulticastGroups(tPtr); } } @@ -204,12 +205,12 @@ public: */ inline void multicastUnsubscribe(const MulticastGroup &mg) { - Mutex::Lock _l(_lock); + Mutex::Lock _l(_myMulticastGroups_l); std::vector::iterator i(std::lower_bound(_myMulticastGroups.begin(),_myMulticastGroups.end(),mg)); if ( (i != _myMulticastGroups.end()) && (*i == mg) ) _myMulticastGroups.erase(i); } - + /** * Handle an inbound network config chunk * @@ -239,20 +240,12 @@ public: /** * Set netconf failure to 'access denied' -- called in IncomingPacket when controller reports this */ - inline void setAccessDenied() - { - Mutex::Lock _l(_lock); - _netconfFailure = NETCONF_FAILURE_ACCESS_DENIED; - } + inline void setAccessDenied() { _netconfFailure = NETCONF_FAILURE_ACCESS_DENIED; } /** * Set netconf failure to 'not found' -- called by IncomingPacket when controller reports this */ - inline void setNotFound() - { - Mutex::Lock _l(_lock); - _netconfFailure = NETCONF_FAILURE_NOT_FOUND; - } + inline void setNotFound() { _netconfFailure = NETCONF_FAILURE_NOT_FOUND; } /** * Causes this network to request an updated configuration from its master node now @@ -281,7 +274,7 @@ public: * @return True if peer has recently associated */ bool recentlyAssociatedWith(const Address &addr); - + /** * Do periodic cleanup and housekeeping tasks */ @@ -294,8 +287,9 @@ public: */ inline void sendUpdatesToMembers(void *tPtr) { - Mutex::Lock _l(_lock); - _sendUpdatesToMembers(tPtr); + Mutex::Lock _l(_myMulticastGroups_l); + Mutex::Lock _l2(_memberships_l); + _announceMulticastGroups(tPtr); } /** @@ -306,7 +300,7 @@ public: */ inline Address findBridgeTo(const MAC &mac) const { - Mutex::Lock _l(_lock); + Mutex::Lock _l(_remoteBridgeRoutes_l); const Address *const br = _remoteBridgeRoutes.get(mac); return ((br) ? *br : Address()); } @@ -324,18 +318,18 @@ public: */ inline void learnBridgeRoute(const MAC &mac,const Address &addr) { - Mutex::Lock _l(_lock); + Mutex::Lock _l(_remoteBridgeRoutes_l); _remoteBridgeRoutes[mac] = addr; - + // Anti-DOS circuit breaker to prevent nodes from spamming us with absurd numbers of bridge routes while (_remoteBridgeRoutes.size() > ZT_MAX_BRIDGE_ROUTES) { Hashtable< Address,unsigned long > counts; Address maxAddr; unsigned long maxCount = 0; - + MAC *m = (MAC *)0; Address *a = (Address *)0; - + // Find the address responsible for the most entries { Hashtable::Iterator i(_remoteBridgeRoutes); @@ -347,7 +341,7 @@ public: } } } - + // Kill this address from our table, since it's most likely spamming us { Hashtable::Iterator i(_remoteBridgeRoutes); @@ -358,7 +352,7 @@ public: } } } - + /** * Learn a multicast group that is bridged to our tap device * @@ -368,7 +362,7 @@ public: */ inline void learnBridgedMulticastGroup(void *tPtr,const MulticastGroup &mg,int64_t now) { - Mutex::Lock _l(_lock); + Mutex::Lock _l(_multicastGroupsBehindMe_l); _multicastGroupsBehindMe.set(mg,now); } @@ -384,7 +378,7 @@ public: { if (cap.networkId() != _id) return Membership::ADD_REJECTED; - Mutex::Lock _l(_lock); + Mutex::Lock _l(_memberships_l); return _memberships[cap.issuedTo()].addCredential(RR,tPtr,_config,cap); } @@ -395,7 +389,7 @@ public: { if (tag.networkId() != _id) return Membership::ADD_REJECTED; - Mutex::Lock _l(_lock); + Mutex::Lock _l(_memberships_l); return _memberships[tag.issuedTo()].addCredential(RR,tPtr,_config,tag); } @@ -411,7 +405,7 @@ public: { if (coo.networkId() != _id) return Membership::ADD_REJECTED; - Mutex::Lock _l(_lock); + Mutex::Lock _l(_memberships_l); return _memberships[coo.issuedTo()].addCredential(RR,tPtr,_config,coo); } @@ -424,22 +418,23 @@ public: */ inline void pushCredentialsNow(void *tPtr,const Address &to,const int64_t now) { - Mutex::Lock _l(_lock); + Mutex::Lock _l(_memberships_l); _memberships[to].pushCredentials(RR,tPtr,now,to,_config); } /** * Push credentials if we haven't done so in a very 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 now Current time */ inline void pushCredentialsIfNeeded(void *tPtr,const Address &to,const int64_t now) { - Mutex::Lock _l(_lock); + const int64_t tout = std::min(_config.credentialTimeMaxDelta,(int64_t)ZT_PEER_ACTIVITY_TIMEOUT); + Mutex::Lock _l(_memberships_l); Membership &m = _memberships[to]; - if (m.shouldPushCredentials(now)) + if (((now - m.lastPushedCredentials()) + 5000) >= tout) m.pushCredentials(RR,tPtr,now,to,_config); } @@ -449,7 +444,14 @@ public: * This sets the network to completely remove itself on delete. This also prevents the * call of the normal port shutdown event on delete. */ - void destroy(); + inline void destroy() + { + _memberships_l.lock(); + _config_l.lock(); + _destroyed = true; + _config_l.unlock(); + _memberships_l.unlock(); + } /** * Get this network's config for export via the ZT core API @@ -458,7 +460,7 @@ public: */ inline void externalConfig(ZT_VirtualNetworkConfig *ec) const { - Mutex::Lock _l(_lock); + Mutex::Lock _l(_config_l); _externalConfig(ec); } @@ -471,7 +473,7 @@ private: ZT_VirtualNetworkStatus _status() const; void _externalConfig(ZT_VirtualNetworkConfig *ec) const; // assumes _lock is locked bool _gate(const SharedPtr &peer); - void _sendUpdatesToMembers(void *tPtr); + void _announceMulticastGroups(void *tPtr); void _announceMulticastGroupsTo(void *tPtr,const Address &peer,const std::vector &allMulticastGroups); std::vector _allMulticastGroups() const; @@ -501,9 +503,9 @@ private: }; _IncomingConfigChunk _incomingConfigChunks[ZT_NETWORK_MAX_INCOMING_UPDATES]; - bool _destroyed; + volatile bool _destroyed; - enum { + volatile enum { NETCONF_FAILURE_NONE, NETCONF_FAILURE_ACCESS_DENIED, NETCONF_FAILURE_NOT_FOUND, @@ -513,7 +515,11 @@ private: Hashtable _memberships; - Mutex _lock; + Mutex _myMulticastGroups_l; + Mutex _multicastGroupsBehindMe_l; + Mutex _remoteBridgeRoutes_l; + Mutex _config_l; + Mutex _memberships_l; AtomicCounter __refCount; }; diff --git a/node/Packet.hpp b/node/Packet.hpp index af339dec3..6c3695343 100644 --- a/node/Packet.hpp +++ b/node/Packet.hpp @@ -914,9 +914,8 @@ public: * <[2] 16-bit number of peers> * <[16] 128-bit hash of node public key> * <[2] 16-bit latency to node or 0 if unspecified> - * <[1] 8-bit number of network hops to node or 0 if unspecified> * <[4] 32-bit max bandwidth in megabits or 0 if unspecified> - * [<[...] additional hash,latency,hops,bandwidth tuples>] + * [<[...] additional hash,latency,bandwidth tuples>] * * This messages can be pushed to indicate that this peer is willing * to relay traffic to other peers. It contains a list of 128-bit