Push credentials always if updated (client-side) and some controller-side cleanup that should be logically irrelevant but will prevent unnecessary DB lookups.

RELEASE-NOTES.md

@@ -3,8 +3,8 @@ ZeroTier Release Notes
 
 # 2022-04-15 -- Version 1.8.9
 
- * Fixed a weird bug that was causing sporadic "phantom" packet authentication failures. Not a security problem but could be behind spordaic reports of link failures under some conditions.
- * Fixed numerous issues with SSO/OIDC support.
+ * Fixed a long-standing and strange bug that was causing sporadic "phantom" packet authentication failures. Not a security problem but could be behind spordaic reports of link failures under some conditions.
+ * Fized a memory leak in SSO/OIDC support.
 
 # 2022-04-11 -- Version 1.8.8
 

controller/EmbeddedNetworkController.cpp

@@ -1344,10 +1344,6 @@ void EmbeddedNetworkController::_request(
 		authenticationExpiryTime = (int64_t)OSUtils::jsonInt(member["authenticationExpiryTime"], 0);
 		info = _db.getSSOAuthInfo(member, _ssoRedirectURL);
 		assert(info.enabled == networkSSOEnabled);
-
-		std::lock_guard<std::mutex> l(_expiringSoon_l);
-		_expiringSoon.insert(std::pair<int64_t, _MemberStatusKey>(authenticationExpiryTime, msk));
-
 		if (authenticationExpiryTime <= now) {
 			if (info.version == 0) {
 				Dictionary<4096> authInfo;
@@ -1394,6 +1390,11 @@ void EmbeddedNetworkController::_request(
 				ms.lastRequestMetaData = metaData;
 				ms.identity = identity;
 			}
+
+			if (authenticationExpiryTime > 0) {
+				std::lock_guard<std::mutex> l(_expiringSoon_l);
+				_expiringSoon.insert(std::pair<int64_t, _MemberStatusKey>(authenticationExpiryTime, msk));
+			}
 		}
 	} else {
 		// If they are not authorized, STOP!
@@ -1853,6 +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.
 							network.clear();
 							member.clear();
 							if (_db.get(s->second.networkId, network, s->second.nodeId, member)) {

node/Membership.hpp

@@ -65,11 +65,13 @@ public:
 	void pushCredentials(const RuntimeEnvironment *RR,void *tPtr,const int64_t now,const Address &peerAddress,const NetworkConfig &nconf);
 
 	/**
+	 * @param now Current time
+	 * @param lastReceivedCredentials Time we last received updated credentials from the controller
 	 * @return True if we haven't pushed credentials in a long time (to cause proactive credential push)
 	 */
-	inline bool shouldPushCredentials(const int64_t now) const
+	inline bool shouldPushCredentials(const int64_t now, const lastReceivedCredentials) const
 	{
-		return ((now - _lastPushedCredentials) > ZT_PEER_ACTIVITY_TIMEOUT);
+		return ((now - _lastPushedCredentials) > ZT_PEER_ACTIVITY_TIMEOUT) || (lastReceivedCredentials > _lastPushedCredentials);
 	}
 
 	/**

node/Network.hpp

@@ -389,7 +389,7 @@ public:
 	{
 		Mutex::Lock _l(_lock);
 		Membership &m = _membership(to);
-		if (m.shouldPushCredentials(now))
+		if (m.shouldPushCredentials(now, _lastConfigUpdate))
 			m.pushCredentials(RR,tPtr,now,to,_config);
 	}
 
@@ -439,7 +439,7 @@ private:
 	Hashtable< MAC,Address > _remoteBridgeRoutes; // remote addresses where given MACs are reachable (for tracking devices behind remote bridges)
 
 	NetworkConfig _config;
-	uint64_t _lastConfigUpdate;
+	int64_t _lastConfigUpdate;
 
 	struct _IncomingConfigChunk
 	{

node/Revocation.hpp

@@ -67,7 +67,7 @@ public:
 	 * @param tgt Target node whose credential(s) are being revoked
 	 * @param ct Credential type being revoked
 	 */
-	Revocation(const uint32_t i,const uint64_t nwid,const uint32_t cid,const uint64_t thr,const uint64_t fl,const Address &tgt,const Credential::Type ct) :
+	Revocation(const uint32_t i,const uint64_t nwid,const uint32_t cid,const int64_t thr,const uint64_t fl,const Address &tgt,const Credential::Type ct) :
 		_id(i),
 		_credentialId(cid),
 		_networkId(nwid),
@@ -155,7 +155,7 @@ public:
 		_networkId = b.template at<uint64_t>(p); p += 8;
 		p += 4; // 4 bytes, currently unused
 		_credentialId = b.template at<uint32_t>(p); p += 4;
-		_threshold = b.template at<uint64_t>(p); p += 8;
+		_threshold = (int64_t)b.template at<uint64_t>(p); p += 8;
 		_flags = b.template at<uint64_t>(p); p += 8;
 		_target.setTo(b.field(p,ZT_ADDRESS_LENGTH),ZT_ADDRESS_LENGTH); p += ZT_ADDRESS_LENGTH;
 		_signedBy.setTo(b.field(p,ZT_ADDRESS_LENGTH),ZT_ADDRESS_LENGTH); p += ZT_ADDRESS_LENGTH;