소스 검색

Fix IP limit enforcement and clarify related comments (#4699)

* fix: keep latest IP for limit enforcement

* chore: clarify IP limit comment

* chore: clarify timestamp sorting comment

---------

Co-authored-by: copilot-swe-agent[bot] <[email protected]>
ALOKY 11 시간 전
부모
커밋
16edb037e7
1개의 변경된 파일13개의 추가작업 그리고 14개의 파일을 삭제
  1. 13 14
      web/job/check_client_ip_job.go

+ 13 - 14
web/job/check_client_ip_job.go

@@ -41,8 +41,8 @@ const defaultXrayAPIPort = 62789
 //
 // Without this eviction, an IP that connected once and then went away
 // keeps sitting in the table with its old timestamp. Because the
-// excess-IP selector sorts ascending ("oldest wins, newest loses") to
-// protect the original/current connections, that stale entry keeps
+// excess-IP selector sorts ascending ("newest wins, oldest loses") to
+// protect the most recent connections, that stale entry keeps
 // occupying a slot and the IP that is *actually* currently using the
 // config gets classified as "new excess" and banned by fail2ban on
 // every single run — producing the continuous ban loop from #4077.
@@ -255,15 +255,13 @@ func mergeClientIps(old, new []IPWithTimestamp, staleCutoff int64) map[string]in
 //
 // only live ips count toward the per-client limit. historical ones stay
 // in the db so the panel keeps showing them, but they must not take a
-// protected slot. the 30min cutoff alone isn't tight enough: an ip that
-// stopped connecting a few minutes ago still looks fresh to
-// mergeClientIps, and since the over-limit picker sorts ascending and
-// keeps the oldest, those idle entries used to win the slot while the
-// ip actually connecting got classified as excess and sent to fail2ban
-// every tick. see #4077 / #4091.
+// live slot or get re-banned. the 30min cutoff alone isn't tight enough:
+// an ip that stopped connecting a few minutes ago still looks fresh to
+// mergeClientIps, and without this split it would keep triggering
+// fail2ban even though it isn't currently connected. see #4077 / #4091.
 //
-// live is sorted ascending so the "protect original, ban newcomer"
-// rule still holds when several ips are really connecting at once.
+// live is sorted ascending by timestamp (oldest → newest), so we keep
+// the most recent entries at the end of the slice (last IP wins).
 func partitionLiveIps(ipMap map[string]int64, observedThisScan map[string]bool) (live, historical []IPWithTimestamp) {
 	live = make([]IPWithTimestamp, 0, len(observedThisScan))
 	historical = make([]IPWithTimestamp, 0, len(ipMap))
@@ -408,9 +406,10 @@ func (j *CheckClientIpJob) updateInboundClientIps(inboundClientIps *model.Inboun
 	if len(liveIps) > limitIp {
 		shouldCleanLog = true
 
-		// protect the oldest live ip, ban newcomers.
-		keptLive = liveIps[:limitIp]
-		bannedLive := liveIps[limitIp:]
+		// keep the newest live ips, ban older ones.
+		cutoff := len(liveIps) - limitIp
+		keptLive = liveIps[cutoff:]
+		bannedLive := liveIps[:cutoff]
 
 		// Open log file only when a ban entry needs to be written.
 		// Use a local logger to avoid mutating the global log.* state,
@@ -456,7 +455,7 @@ func (j *CheckClientIpJob) updateInboundClientIps(inboundClientIps *model.Inboun
 	}
 
 	if len(j.disAllowedIps) > 0 {
-		logger.Infof("[LIMIT_IP] Client %s: Kept %d live IPs, queued %d new IPs for fail2ban", clientEmail, len(keptLive), len(j.disAllowedIps))
+		logger.Infof("[LIMIT_IP] Client %s: Kept %d live IPs, queued %d old IPs for fail2ban", clientEmail, len(keptLive), len(j.disAllowedIps))
 	}
 
 	return shouldCleanLog