From ef08346a74342b0d5ba32fb190064ffd6acd3c96 Mon Sep 17 00:00:00 2001 From: Adam Ierymenko Date: Tue, 19 Apr 2022 19:59:54 -0400 Subject: [PATCH] Fix a possible excessive memory use issue in controller and clean up a bunch of COM handling and other code in the normal node. --- controller/EmbeddedNetworkController.cpp | 2 +- controller/EmbeddedNetworkController.hpp | 3 ++- node/Constants.hpp | 5 +++++ node/IncomingPacket.cpp | 14 +------------- node/Membership.cpp | 3 ++- node/Membership.hpp | 2 +- node/Network.cpp | 1 + node/Network.hpp | 5 ++++- 8 files changed, 17 insertions(+), 18 deletions(-) diff --git a/controller/EmbeddedNetworkController.cpp b/controller/EmbeddedNetworkController.cpp index 6b2b53da7..d625da829 100644 --- a/controller/EmbeddedNetworkController.cpp +++ b/controller/EmbeddedNetworkController.cpp @@ -1854,7 +1854,7 @@ void EmbeddedNetworkController::_startThreads() for(auto s=_expiringSoon.begin();s!=_expiringSoon.end();) { const int64_t when = s->first; if (when <= now) { - // The user MAY have re-authorized, so we must actually look it up and check. + // The user may have re-authorized, so we must actually look it up and check. network.clear(); member.clear(); if (_db.get(s->second.networkId, network, s->second.nodeId, member)) { diff --git a/controller/EmbeddedNetworkController.hpp b/controller/EmbeddedNetworkController.hpp index 6a6c919cd..49c1c36eb 100644 --- a/controller/EmbeddedNetworkController.hpp +++ b/controller/EmbeddedNetworkController.hpp @@ -117,6 +117,7 @@ private: uint64_t networkId; uint64_t nodeId; inline bool operator==(const _MemberStatusKey &k) const { return ((k.networkId == networkId)&&(k.nodeId == nodeId)); } + inline bool operator<(const _MemberStatusKey &k) const { return (k.networkId < networkId) || ((k.networkId == networkId)&&(k.nodeId < nodeId)); } }; struct _MemberStatus { @@ -154,7 +155,7 @@ private: std::unordered_map< _MemberStatusKey,_MemberStatus,_MemberStatusHash > _memberStatus; std::mutex _memberStatus_l; - std::multimap< int64_t, _MemberStatusKey > _expiringSoon; + std::set< std::pair > _expiringSoon; std::mutex _expiringSoon_l; RedisConfig *_rc; diff --git a/node/Constants.hpp b/node/Constants.hpp index e000563cf..52a2f0fa2 100644 --- a/node/Constants.hpp +++ b/node/Constants.hpp @@ -539,6 +539,11 @@ */ #define ZT_PEER_CREDENTIALS_CUTOFF_LIMIT 15 +/** + * Rate limit for responding to peer credential requests + */ +#define ZT_PEER_CREDENTIALS_REQUEST_RATE_LIMIT 1000 + /** * WHOIS rate limit (we allow these to be pretty fast) */ diff --git a/node/IncomingPacket.cpp b/node/IncomingPacket.cpp index f749c8bdf..78c1a5568 100644 --- a/node/IncomingPacket.cpp +++ b/node/IncomingPacket.cpp @@ -1098,16 +1098,7 @@ bool IncomingPacket::_doMULTICAST_GATHER(const RuntimeEnvironment *RR,void *tPtr } catch ( ... ) {} // discard invalid COMs } - bool trustEstablished = false; - if (network) { - if (network->gate(tPtr,peer)) { - trustEstablished = true; - } else { - _sendErrorNeedCredentials(RR,tPtr,peer,nwid); - return false; - } - } - + const bool trustEstablished = (network) ? network->gate(tPtr,peer) : false; const int64_t now = RR->node->now(); if ((gatherLimit > 0)&&((trustEstablished)||(RR->topology->amUpstream())||(RR->node->localControllerHasAuthorized(now,nwid,peer->address())))) { Packet outp(peer->address(),RR->identity.address(),Packet::VERB_OK); @@ -1224,9 +1215,6 @@ bool IncomingPacket::_doMULTICAST_FRAME(const RuntimeEnvironment *RR,void *tPtr, } peer->received(tPtr,_path,hops(),packetId(),payloadLength(),Packet::VERB_MULTICAST_FRAME,0,Packet::VERB_NOP,true,nwid,ZT_QOS_NO_FLOW); - } else { - _sendErrorNeedCredentials(RR,tPtr,peer,nwid); - return false; } return true; diff --git a/node/Membership.cpp b/node/Membership.cpp index 4f829ecb5..8ddd61f09 100644 --- a/node/Membership.cpp +++ b/node/Membership.cpp @@ -115,7 +115,7 @@ Membership::AddCredentialResult Membership::addCredential(const RuntimeEnvironme RR->t->credentialRejected(tPtr,com,"old"); return ADD_REJECTED; } - if ((newts == oldts)&&(_com == com)) + if (_com == com) return ADD_ACCEPTED_REDUNDANT; switch(com.verify(RR,tPtr)) { @@ -123,6 +123,7 @@ Membership::AddCredentialResult Membership::addCredential(const RuntimeEnvironme RR->t->credentialRejected(tPtr,com,"invalid"); return ADD_REJECTED; case 0: + //printf("%.16llx %.10llx replacing COM %lld with %lld\n", com.networkId(), com.issuedTo().toInt(), _com.timestamp(), com.timestamp()); fflush(stdout); _com = com; return ADD_ACCEPTED_NEW; case 1: diff --git a/node/Membership.hpp b/node/Membership.hpp index 63f0b7e22..c21281cc3 100644 --- a/node/Membership.hpp +++ b/node/Membership.hpp @@ -90,7 +90,7 @@ public: */ inline bool isAllowedOnNetwork(const NetworkConfig &thisNodeNetworkConfig, const Identity &otherNodeIdentity) const { - return (thisNodeNetworkConfig.isPublic() || (((_com.timestamp() > _comRevocationThreshold) && (thisNodeNetworkConfig.com.agreesWith(_com, otherNodeIdentity))))); + return thisNodeNetworkConfig.isPublic() || (((_com.timestamp() > _comRevocationThreshold) && (thisNodeNetworkConfig.com.agreesWith(_com, otherNodeIdentity)))); } inline bool recentlyAssociated(const int64_t now) const diff --git a/node/Network.cpp b/node/Network.cpp index c3165b14b..f222b0d92 100644 --- a/node/Network.cpp +++ b/node/Network.cpp @@ -1237,6 +1237,7 @@ bool Network::gate(void *tPtr,const SharedPtr &peer) } } } catch ( ... ) {} + //printf("%.16llx %.10llx not allowed\n", _id, peer->address().toInt()); fflush(stdout); return false; } diff --git a/node/Network.hpp b/node/Network.hpp index 6fa6f9795..b427a83d6 100644 --- a/node/Network.hpp +++ b/node/Network.hpp @@ -375,7 +375,10 @@ public: inline void peerRequestedCredentials(void *tPtr,const Address &to,const int64_t now) { Mutex::Lock _l(_lock); - _membership(to).pushCredentials(RR,tPtr,now,to,_config); + Membership &m = _membership(to); + const int64_t lastPushed = m.lastPushedCredentials(); + if ((lastPushed < _lastConfigUpdate)||((now - lastPushed) > ZT_PEER_CREDENTIALS_REQUEST_RATE_LIMIT)) + m.pushCredentials(RR,tPtr,now,to,_config); } /**