Yet MORE refactoring, including moving expected-reply-to out of Node and into its own class for cleanliness.

node/AES.cpp

@@ -362,10 +362,12 @@ void AES::GMAC::finish(uint8_t tag[16]) noexcept
 		__m128i t4 = _mm_clmulepi64_si128(h,y,0x11);
 
 		encIV = _mm_aesenc_si128(encIV,k[1]);
-		encIV = _mm_aesenc_si128(encIV,k[2]);
 
 		t2 = _mm_xor_si128(t2,t3);
 		t3 = _mm_slli_si128(t2,8);
+
+		encIV = _mm_aesenc_si128(encIV,k[2]);
+
 		t2 = _mm_srli_si128(t2,8);
 		t1 = _mm_xor_si128(t1,t3);
 
@@ -380,47 +382,57 @@ void AES::GMAC::finish(uint8_t tag[16]) noexcept
 
 		t4 = _mm_slli_epi32(t4,1);
 		t3 = _mm_srli_si128(t5,12);
+
+		encIV = _mm_aesenc_si128(encIV,k[5]);
+
 		t6 = _mm_slli_si128(t6,4);
 		t5 = _mm_slli_si128(t5,4);
 
-		encIV = _mm_aesenc_si128(encIV,k[5]);
+		encIV = _mm_aesenc_si128(encIV,k[6]);
 
 		t1 = _mm_or_si128(t1,t5);
 		t4 = _mm_or_si128(t4,t6);
 
-		encIV = _mm_aesenc_si128(encIV,k[6]);
 		encIV = _mm_aesenc_si128(encIV,k[7]);
 
 		t4 = _mm_or_si128(t4,t3);
 		t5 = _mm_slli_epi32(t1,31);
+
+		encIV = _mm_aesenc_si128(encIV,k[8]);
+
 		t6 = _mm_slli_epi32(t1,30);
 		t3 = _mm_slli_epi32(t1,25);
 
-		encIV = _mm_aesenc_si128(encIV,k[8]);
 		encIV = _mm_aesenc_si128(encIV,k[9]);
 
 		t5 = _mm_xor_si128(t5,t6);
 		t5 = _mm_xor_si128(t5,t3);
 
 		encIV = _mm_aesenc_si128(encIV,k[10]);
-		encIV = _mm_aesenc_si128(encIV,k[11]);
 
 		t6 = _mm_srli_si128(t5,4);
 		t4 = _mm_xor_si128(t4,t6);
+
+		encIV = _mm_aesenc_si128(encIV,k[11]);
+
 		t5 = _mm_slli_si128(t5,12);
 		t1 = _mm_xor_si128(t1,t5);
 
 		t4 = _mm_xor_si128(t4,t1);
 		t5 = _mm_srli_epi32(t1,1);
+
+		encIV = _mm_aesenc_si128(encIV,k[12]);
+
 		t2 = _mm_srli_epi32(t1,2);
 		t3 = _mm_srli_epi32(t1,7);
 
-		encIV = _mm_aesenc_si128(encIV,k[12]);
 		encIV = _mm_aesenc_si128(encIV,k[13]);
-		encIV = _mm_aesenclast_si128(encIV,k[14]);
 
 		t4 = _mm_xor_si128(t4,t2);
 		t4 = _mm_xor_si128(t4,t3);
+
+		encIV = _mm_aesenclast_si128(encIV,k[14]);
+
 		t4 = _mm_xor_si128(t4,t5);
 
 		_mm_storeu_si128(reinterpret_cast<__m128i *>(tag),_mm_xor_si128(_mm_shuffle_epi8(t4,s_shuf),encIV));

node/CMakeLists.txt

@@ -14,6 +14,7 @@ set(core_headers
 	Defragmenter.hpp
 	Dictionary.hpp
 	ECC384.hpp
+	Expect.hpp
 	FCV.hpp
 	Hashtable.hpp
 	Identity.hpp

node/Expect.hpp

@@ -0,0 +1,86 @@
+/*
+ * Copyright (c)2013-2020 ZeroTier, Inc.
+ *
+ * Use of this software is governed by the Business Source License included
+ * in the LICENSE.TXT file in the project's root directory.
+ *
+ * Change Date: 2024-01-01
+ *
+ * On the date above, in accordance with the Business Source License, use
+ * of this software will be governed by version 2.0 of the Apache License.
+ */
+/****/
+
+#ifndef ZT_EXPECT_HPP
+#define ZT_EXPECT_HPP
+
+#include "Constants.hpp"
+#include "Utils.hpp"
+
+/**
+ * Number of buckets to use to maintain a list of expected replies.
+ *
+ * More buckets means less chance of two packets tagging the same
+ * bucket. This doesn't actually hurt anything since this class
+ * behaves like a bloom filter: you can have false positives but
+ * not false negatives.
+ *
+ * OKs are also cryptographically authenticated, so this is not a
+ * huge problem, but this helps harden the system against replay
+ * attacks for e.g. denial of service.
+ */
+#define ZT_EXPECT_BUCKETS 131072
+
+/**
+ * 1/2 the TTL for expected replies in milliseconds
+ *
+ * Making this a power of two improves efficiency a little by allowing bit
+ * shift division.
+ */
+#define ZT_EXPECT_TTL 4096
+
+namespace ZeroTier {
+
+/**
+ * Tracker for expected OK replies to packet IDs of sent packets
+ */
+class Expect
+{
+public:
+	ZT_ALWAYS_INLINE Expect() : _salt(Utils::getSecureRandomU64()) {}
+
+	/**
+	 * Called by other code when something is sending a packet that may receive an OK response
+	 *
+	 * @param packetId Packet ID of packet being sent (be sure it's post-armor())
+	 * @param now Current time
+	 */
+	ZT_ALWAYS_INLINE void sending(const uint64_t packetId,const int64_t now) noexcept
+	{
+		_packetIdSent[Utils::hash64(packetId ^ _salt) % ZT_EXPECT_BUCKETS] = (int32_t)(now / ZT_EXPECT_TTL);
+	}
+
+	/**
+	 * Check whether an OK is expected for this packet
+	 *
+	 * @param inRePacketId
+	 * @param now
+	 * @return
+	 */
+	ZT_ALWAYS_INLINE bool expecting(const uint64_t inRePacketId,const int64_t now) const noexcept
+	{
+		return ((((int32_t)(now / ZT_EXPECT_TTL)) - _packetIdSent[Utils::hash64(inRePacketId ^ _salt) % ZT_EXPECT_BUCKETS]) <= 1);
+	}
+
+private:
+	// This is a static per-object salt that's XORed and mixed with the packet ID
+	// to make it difficult for a third party to predict expected-reply buckets.
+	const uint64_t _salt;
+
+	// Each bucket contains a timestamp in units of the expect duration.
+	std::atomic<int32_t> _packetIdSent[ZT_EXPECT_TTL];
+};
+
+} // namespace ZeroTier
+
+#endif

node/InetAddress.cpp

@@ -374,27 +374,6 @@ bool InetAddress::isNetwork() const noexcept
 	return false;
 }
 
-unsigned long InetAddress::rateGateHash() const noexcept
-{
-	unsigned long h = 0;
-	switch(ss_family) {
-		case AF_INET:
-			h = (Utils::ntoh((uint32_t)reinterpret_cast<const struct sockaddr_in *>(this)->sin_addr.s_addr) & 0xffffff00U) >> 8U;
-			h ^= (h >> 14U);
-			break;
-		case AF_INET6: {
-			const uint8_t *ip = reinterpret_cast<const uint8_t *>(reinterpret_cast<const struct sockaddr_in6 *>(this)->sin6_addr.s6_addr);
-			h = ((unsigned long)ip[0]); h <<= 1U;
-			h += ((unsigned long)ip[1]); h <<= 1U;
-			h += ((unsigned long)ip[2]); h <<= 1U;
-			h += ((unsigned long)ip[3]); h <<= 1U;
-			h += ((unsigned long)ip[4]); h <<= 1U;
-			h += ((unsigned long)ip[5]);
-		}	break;
-	}
-	return (h & 0x3fffU);
-}
-
 int InetAddress::marshal(uint8_t data[ZT_INETADDRESS_MARSHAL_SIZE_MAX]) const noexcept
 {
 	unsigned int port;

node/InetAddress.hpp

@@ -397,11 +397,6 @@ public:
 	 */
 	bool isNetwork() const noexcept;
 
-	/**
-	 * @return 14-bit (0-16383) hash of this IP's first 24 or 48 bits (for V4 or V6) for rate limiting code, or 0 if non-IP
-	 */
-	unsigned long rateGateHash() const noexcept;
-
 	/**
 	 * @return True if address family is non-zero
 	 */

node/Meter.hpp

@@ -54,8 +54,7 @@ public:
 		// the log size and then if it's a new bucket setting it or otherwise adding
 		// to it.
 		const unsigned long bucket = ((unsigned int)((uint64_t)(now / TUNIT))) % LSIZE;
-		const unsigned long prevBucket = _bucket.exchange(bucket);
+		if (_bucket.exchange(bucket) != bucket)
-		if (prevBucket != bucket)
 			_counts[bucket].store((uint64_t)count);
 		else _counts[bucket].fetch_add((uint64_t)count);
 	}

node/Node.cpp

@@ -48,10 +48,6 @@ Node::Node(void *uPtr,void *tPtr,const struct ZT_Node_Callbacks *callbacks,int64
 {
 	_networks.resize(64); // _networksMask + 1, must be power of two
 
-	memset((void *)_expectingRepliesToBucketPtr,0,sizeof(_expectingRepliesToBucketPtr));
-	memset((void *)_expectingRepliesTo,0,sizeof(_expectingRepliesTo));
-	memset((void *)_lastIdentityVerification,0,sizeof(_lastIdentityVerification));
-
 	uint64_t idtmp[2]; idtmp[0] = 0; idtmp[1] = 0;
 	std::vector<uint8_t> data(stateObjectGet(tPtr,ZT_STATE_OBJECT_IDENTITY_SECRET,idtmp));
 	bool haveIdentity = false;

node/Node.hpp

@@ -290,65 +290,11 @@ public:
 	 */
 	ZT_ALWAYS_INLINE const Identity &identity() const noexcept { return _RR.identity; }
 
-	/**
-	 * Register that we are expecting a reply to a packet ID
-	 *
-	 * This only uses the most significant bits of the packet ID, both to save space
-	 * and to avoid using the higher bits that can be modified during armor() to
-	 * mask against the packet send counter used for QoS detection.
-	 *
-	 * @param packetId Packet ID to expect reply to
-	 */
-	ZT_ALWAYS_INLINE void expectReplyTo(const uint64_t packetId) noexcept
-	{
-		const unsigned long pid2 = (unsigned long)(packetId >> 32U);
-		const unsigned long bucket = (unsigned long)(pid2 & ZT_EXPECTING_REPLIES_BUCKET_MASK1);
-		_expectingRepliesTo[bucket][_expectingRepliesToBucketPtr[bucket]++ & ZT_EXPECTING_REPLIES_BUCKET_MASK2] = (uint32_t)pid2;
-	}
-
-	/**
-	 * Check whether a given packet ID is something we are expecting a reply to
-	 *
-	 * This only uses the most significant bits of the packet ID, both to save space
-	 * and to avoid using the higher bits that can be modified during armor() to
-	 * mask against the packet send counter used for QoS detection.
-	 *
-	 * @param packetId Packet ID to check
-	 * @return True if we're expecting a reply
-	 */
-	ZT_ALWAYS_INLINE bool expectingReplyTo(const uint64_t packetId) const noexcept
-	{
-		const uint32_t pid2 = (uint32_t)(packetId >> 32);
-		const unsigned long bucket = (unsigned long)(pid2 & ZT_EXPECTING_REPLIES_BUCKET_MASK1);
-		for(unsigned long i=0;i<=ZT_EXPECTING_REPLIES_BUCKET_MASK2;++i) {
-			if (_expectingRepliesTo[bucket][i] == pid2)
-				return true;
-		}
-		return false;
-	}
-
 	/**
 	 * @return True if aggressive NAT-traversal mechanisms like scanning of <1024 ports are enabled
 	 */
 	ZT_ALWAYS_INLINE bool natMustDie() const noexcept { return _natMustDie; }
 
-	/**
-	 * Check whether we should do potentially expensive identity verification (rate limit)
-	 *
-	 * @param now Current time
-	 * @param from Source address of packet
-	 * @return True if within rate limits
-	 */
-	ZT_ALWAYS_INLINE bool rateGateIdentityVerification(const int64_t now,const InetAddress &from) noexcept
-	{
-		unsigned long iph = from.rateGateHash();
-		if ((now - _lastIdentityVerification[iph]) >= ZT_IDENTITY_VALIDATION_SOURCE_RATE_LIMIT) {
-			_lastIdentityVerification[iph] = now;
-			return true;
-		}
-		return false;
-	}
-
 	/**
 	 * Wake any peers with the given address by calling their alarm() methods at or after the specified time
 	 *
@@ -389,13 +335,6 @@ private:
 	ZT_Node_Callbacks _cb;
 	void *_uPtr; // _uptr (lower case) is reserved in Visual Studio :P
 
-	// For tracking packet IDs to filter out OK/ERROR replies to packets we did not send.
-	volatile uint8_t _expectingRepliesToBucketPtr[ZT_EXPECTING_REPLIES_BUCKET_MASK1 + 1];
-	volatile uint32_t _expectingRepliesTo[ZT_EXPECTING_REPLIES_BUCKET_MASK1 + 1][ZT_EXPECTING_REPLIES_BUCKET_MASK2 + 1];
-
-	// Time of last identity verification indexed by InetAddress.rateGateHash() -- used in IncomingPacket::_doHELLO() via rateGateIdentityVerification()
-	volatile int64_t _lastIdentityVerification[16384];
-
 	// Addresses of peers that want to have their alarm() function called at some point in the future.
 	// These behave like weak references in that the node looks them up in Topology and calls alarm()
 	// in each peer if that peer object is still held in memory. Calling alarm() unnecessarily on a peer

node/RuntimeEnvironment.hpp

@@ -27,6 +27,7 @@ class Node;
 class NetworkController;
 class SelfAwareness;
 class Trace;
+class Expect;
 
 /**
  * Holds global state for an instance of ZeroTier::Node
@@ -39,6 +40,7 @@ public:
 		localNetworkController(nullptr),
 		rtmem(nullptr),
 		t(nullptr),
+		expect(nullptr),
 		vl2(nullptr),
 		vl1(nullptr),
 		topology(nullptr),
@@ -69,6 +71,7 @@ public:
 	 * These are constant and never null after startup unless indicated. */
 
 	Trace *t;
+	Expect *expect;
 	VL2 *vl2;
 	VL1 *vl1;
 	Topology *topology;

node/VL1.cpp

@@ -598,10 +598,6 @@ bool VL1::_HELLO(void *tPtr,const SharedPtr<Path> &path,SharedPtr<Peer> &peer,Bu
 	const int64_t now = RR->node->now();
 
 	if (!peer) {
-		if (!RR->node->rateGateIdentityVerification(now,path->address())) {
-			RR->t->incomingPacketDropped(tPtr,0xaffa9ff7,p.h.packetId,0,id,path->address(),hops,Protocol::VERB_HELLO,ZT_TRACE_PACKET_DROP_REASON_RATE_LIMIT_EXCEEDED);
-			return false;
-		}
 		if (!id.locallyValidate()) {
 			RR->t->incomingPacketDropped(tPtr,0x2ff7a909,p.h.packetId,0,id,path->address(),hops,Protocol::VERB_HELLO,ZT_TRACE_PACKET_DROP_REASON_INVALID_OBJECT);
 			return false;