From 7794e93762c0046b78ae28bc4715e9fb71d87eb5 Mon Sep 17 00:00:00 2001
From: Jay Wren <jay.wren@slack-corp.com>
Date: Fri, 10 Apr 2026 14:36:32 -0400
Subject: [PATCH] Address PR feedback: remove outbound rate limit, improve
 config docs

Remove rate limiting from StartHandshake (outbound) since DoS
protection only needs to limit inbound handshakes. This also avoids
returning nil from StartHandshake which historically always returned
non-nil. Update config comment to note openssl speed is single-core
and suggest scaling by routines.

Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
---
 examples/config.yml       |  8 ++++----
 handshake_manager.go      |  8 --------
 handshake_manager_test.go | 33 ++++++++-------------------------
 3 files changed, 12 insertions(+), 37 deletions(-)

diff --git a/examples/config.yml b/examples/config.yml
index 34ed7d02..1bddc393 100644
--- a/examples/config.yml
+++ b/examples/config.yml
@@ -342,12 +342,12 @@ logging:
   # after receiving the response for lighthouse queries
   #trigger_buffer: 64
 
-  # max_rate limits the number of new handshakes per second. Both incoming and outgoing new
-  # handshakes count against this limit. Once the limit is reached, new handshakes are dropped
-  # until the next second. A value of 0 means unlimited (default).
+  # max_rate limits the number of new inbound handshakes per second. Once the limit is reached,
+  # new handshakes are dropped until the next second. A value of 0 means unlimited (default).
   # This is useful for preventing DoS attacks that attempt to exhaust CPU with handshake crypto.
   # Running `openssl speed ecdhp256` on your hardware can be a good rule of thumb for choosing
-  # a max, as each handshake performs similar DH operations.
+  # a max, as each handshake performs similar DH operations. Note that this benchmarks a single
+  # core, so you may wish to scale the value by the number of `routines` configured.
   #max_rate: 0
 
 # Tunnel manager settings
diff --git a/handshake_manager.go b/handshake_manager.go
index f1ddc41f..c3e21d6b 100644
--- a/handshake_manager.go
+++ b/handshake_manager.go
@@ -496,14 +496,6 @@ func (hm *HandshakeManager) StartHandshake(vpnAddr netip.Addr, cacheCb func(*Han
 		return hh.hostinfo
 	}
 
-	// Check rate limit for new outbound handshakes
-	if !hm.handshakeRateAllow(time.Now()) {
-		hm.metricRateLimited.Inc(1)
-		hm.l.WithField("vpnAddr", vpnAddr).Debug("Handshake rate limit reached, dropping outbound handshake")
-		hm.Unlock()
-		return nil
-	}
-
 	hostinfo := &HostInfo{
 		vpnAddrs:        []netip.Addr{vpnAddr},
 		HandshakePacket: make(map[uint8][]byte, 0),
diff --git a/handshake_manager_test.go b/handshake_manager_test.go
index a735b582..99308a5f 100644
--- a/handshake_manager_test.go
+++ b/handshake_manager_test.go
@@ -74,42 +74,25 @@ func Test_HandshakeManagerRateLimit(t *testing.T) {
 
 	lh := newTestLighthouse()
 
-	cs := &CertState{
-		initiatingVersion: cert.Version1,
-		privateKey:        []byte{},
-		v1Cert:            &dummyCert{version: cert.Version1},
-		v1HandshakeBytes:  []byte{},
-	}
-
 	config := defaultHandshakeConfig
 	config.maxHandshakeRate = 2
 
 	hm := NewHandshakeManager(l, mainHM, lh, &udp.NoopConn{}, config)
 	hm.f = &Interface{handshakeManager: hm, pki: &PKI{}, l: l}
-	hm.f.pki.cs.Store(cs)
+
+	now := time.Now()
 
 	// Should allow up to maxHandshakeRate handshakes
-	ip1 := netip.MustParseAddr("172.1.1.1")
-	ip2 := netip.MustParseAddr("172.1.1.2")
-	ip3 := netip.MustParseAddr("172.1.1.3")
-
-	h1 := hm.StartHandshake(ip1, nil)
-	assert.NotNil(t, h1, "first handshake should be allowed")
-
-	h2 := hm.StartHandshake(ip2, nil)
-	assert.NotNil(t, h2, "second handshake should be allowed")
-
-	// Third should be rate limited
-	h3 := hm.StartHandshake(ip3, nil)
-	assert.Nil(t, h3, "third handshake should be rate limited")
+	hm.Lock()
+	assert.True(t, hm.handshakeRateAllow(now), "first handshake should be allowed")
+	assert.True(t, hm.handshakeRateAllow(now), "second handshake should be allowed")
+	assert.False(t, hm.handshakeRateAllow(now), "third handshake should be rate limited")
+	hm.Unlock()
 
 	// After advancing time by 1 second, tokens should refill
 	hm.Lock()
-	hm.rateLastTick = hm.rateLastTick.Add(-time.Second)
+	assert.True(t, hm.handshakeRateAllow(now.Add(time.Second)), "handshake should be allowed after token refill")
 	hm.Unlock()
-
-	h3 = hm.StartHandshake(ip3, nil)
-	assert.NotNil(t, h3, "handshake should be allowed after token refill")
 }
 
 func Test_HandshakeManagerRateLimitUnlimited(t *testing.T) {