Skip to content

Implement IP health monitoring and management features - #294

Merged
DanielLavrushin merged 9 commits into
mainfrom
ip-block-heal
Aug 8, 2026
Merged

Implement IP health monitoring and management features#294
DanielLavrushin merged 9 commits into
mainfrom
ip-block-heal

Conversation

@DanielLavrushin

Copy link
Copy Markdown
Owner
  • Add goodIPStore for managing and storing good IP addresses with TTL.
  • Implement methods for storing, looking up, and cleaning up good IPs.
  • Create tests for goodIPStore functionality including case insensitivity and IP capping.
  • Introduce ipHealthStore for monitoring the health of IP addresses with SYN detection.
  • Implement probing logic to determine if IPs are alive or dead, with burst detection to suspend monitoring.
  • Add tests for ipHealthStore to verify correct behavior under various conditions.
  • Implement DNS response healing to filter out unreachable IPs based on health status.
  • Create tests for DNS healing functionality to ensure proper address handling.
  • Add RST packet building functions for both IPv4 and IPv6, with tests to validate behavior.

- Add goodIPStore for managing and storing good IP addresses with TTL.
- Implement methods for storing, looking up, and cleaning up good IPs.
- Create tests for goodIPStore functionality including case insensitivity and IP capping.
- Introduce ipHealthStore for monitoring the health of IP addresses with SYN detection.
- Implement probing logic to determine if IPs are alive or dead, with burst detection to suspend monitoring.
- Add tests for ipHealthStore to verify correct behavior under various conditions.
- Implement DNS response healing to filter out unreachable IPs based on health status.
- Create tests for DNS healing functionality to ensure proper address handling.
- Add RST packet building functions for both IPv4 and IPv6, with tests to validate behavior.
…ker and KnownGood

- Removed goodIPStore and its associated tests, replacing it with iphealth.Tracker and KnownGood for better IP health tracking.
- Updated runtimeState to use iphealth.Tracker and iphealth.KnownGood.
- Modified recordDestAlive to use the new Remember method from KnownGood.
- Refactored tests to accommodate the new IP health management structure.
- Introduced new leak tests for iphealth package.
Copilot AI lite review requested due to automatic review settings August 8, 2026 14:37
@DanielLavrushin DanielLavrushin self-assigned this Aug 8, 2026
@codacy-production

codacy-production Bot commented Aug 8, 2026

Copy link
Copy Markdown

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 561 complexity

Metric Results
Complexity 561

View in Codacy

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds an IP health/“known good” subsystem and integrates it into NFQ TCP/DNS handling to detect unreachable destinations earlier (including unanswered SYNs), optionally “heal” DNS answers by removing dead IPs, and support proxy-routing fallback actions.

Changes:

  • Introduces iphealth.Tracker (SYN-based reachability probing + dead/burst suspension) and iphealth.KnownGood (per-host remembered IPs with TTL/caps).
  • Integrates IP health into NFQ TCP handling (SYN detection + SYN-specific RST/ACK generation) and DNS handling (answer filtering + fallback to last-known-good IP).
  • Extends config + UI + i18n and adds comprehensive tests for the new DNS/IP health behaviors.

Reviewed changes

Copilot reviewed 29 out of 29 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/nfq/types.go Wires iphealth tracker/known-good into worker state.
src/nfq/rst_client.go Adds SYN-specific RST/ACK builders for IPv4/IPv6.
src/nfq/rst_client_test.go Tests SYN RST/ACK construction for v4/v6.
src/nfq/pool.go Hooks tracker/known-good cleanup and adds dynamic blocked TTL selection.
src/nfq/ipblock.go Implements SYN health detection actions + DNS healing and known-good fallback.
src/nfq/ipblock_test.go Tests DNS healing behaviors, proxy diversion, and synDetect gating.
src/nfq/inc.go Marks destinations alive on inbound traffic (incl. SYN-ACK).
src/nfq/handler.go Invokes SYN health handling on outbound SYNs before SYN injection logic.
src/nfq/dnstcp.go Applies DNS healing for DNS-over-TCP responses.
src/nfq/dns.go Applies DNS healing for UDP DNS responses and redirect responses.
src/nfq/connstate.go Stops refreshing blocked-cache timestamps on reads; adds ipHealth/goodIPs to runtime state.
src/nfq/connstate_test.go Tests blocked-cache expiration despite repeated lookups.
src/iphealth/tracker.go New reachability tracker with probing, dead TTL re-probe, and burst suspension.
src/iphealth/tracker_test.go Tests probing thresholds, dead/live transitions, expiry, burst suspension, Stop idempotence.
src/iphealth/leak_test.go Adds leaktest harness for iphealth package tests.
src/iphealth/knowngood.go New known-good IP store keyed by host with TTL and caps.
src/iphealth/knowngood_test.go Tests ordering, family separation, normalization, caps, expiry, and input validation.
src/http/ui/src/models/config.ts Extends UI config model for new IP block detection fields.
src/http/ui/src/i18n/ru.json Updates RU copy for new IP block detection actions/fields.
src/http/ui/src/i18n/en.json Updates EN copy for new IP block detection actions/fields.
src/http/ui/src/components/sets/tcp/TcpGeneral.tsx Adds UI controls for action selection, SYN detect/threshold, TTL sliders, proxy warning.
src/dns/answer.go Adds DNS answer filtering + answer rebuilding from remembered IPs.
src/dns/answer_test.go Tests DNS answer filtering/rebuild logic incl. CNAME chain and OPT handling.
src/config/validation.go Normalizes/fills defaults for new IP block detect fields and validates proxy action prerequisites.
src/config/types.go Adds new IP block action constants/defaults and config helpers (resolved TTL/action/threshold).
src/config/ipblock_test.go Tests action normalization, defaults, syn_detect default behavior, validation fill-ins, proxy prerequisites.
src/config/config.go Adds defaults for new IP block detect fields in DefaultSetConfig.
changelog.md Documents new SYN detection, probing, cache expiry fix, DNS heal action, and proxy routing action.
changelog_ru.md Russian changelog entries for the same feature set.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/dns/answer.go Outdated
Comment thread src/nfq/pool.go Outdated
Copilot AI review requested due to automatic review settings August 8, 2026 15:13

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 29 out of 29 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

src/nfq/dns.go:270

  • healSet is populated from the pending DNS route set even if that set is disabled. Because healDNSResponse only checks IPBlockDetect.Enabled/Action (not set.Enabled), this can cause DNS answers to be rewritten for a set that is currently disabled (e.g., after a config reload between request and response).
					if set := cfg.GetSetById(setID); set != nil {
						if healSet == nil {
							healSet = set
						}
						w.storeHostHints(clientIP, set, domain, ips)

Comment thread src/nfq/ipblock.go
Copilot AI review requested due to automatic review settings August 8, 2026 15:32

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 29 out of 29 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/iphealth/tracker.go:330

  • In Cleanup(), expired dead entries are temporarily moved to stateSuspect while a re-probe runs. Because IsDead() only returns true for stateDead, this means callers will treat the destination as reachable during the re-test window (e.g., SYN/RST handling and DNS healing stop applying), which can reintroduce client hangs until the probe finishes. Keeping the entry in stateDead while probing preserves the "blocked until retested" behavior and avoids letting traffic hit a still-unreachable IP before the probe result is known.
				e.state = stateSuspect
				e.probing = true
				e.changedAt = now
				reprobe = append(reprobe, probeRequest{ip: ip, port: e.port, mark: e.mark})

Copilot AI review requested due to automatic review settings August 8, 2026 15:51

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 31 out of 31 changed files in this pull request and generated no new comments.

Suppressed comments (4)

src/http/ui/src/i18n/ru.json:373

  • Грамматика: фраза "Сколько адрес…" звучит неестественно; лучше "Как долго адрес…".
      "retestIntervalHelper": "Сколько адрес числится недоступным до следующей проверки роутером. Блокировки снимают, а адреса переиспользуют, поэтому бессрочный вердикт оставил бы устройства на устаревшем адресе. Столько же занимает и восстановление: соединение к отбракованному адресу сбрасывается раньше, чем успевает пройти рукопожатие."

src/http/ui/src/i18n/ru.json:369

  • Грамматика: не хватает слова "времени"/"как долго" в описании секции.

This issue also appears on line 373 of the same file.

      "description": "Сколько адрес числится недоступным",

src/nfq/pool.go:116

  • Pool now uses the configurable IP health retest interval as the TTL passed into destState.Cleanup(), but destStateTracker.AddBlocked still prunes entries using a hard-coded 300s when the cache hits max size (nfq/connstate.go:193-199). If the user sets a different retest interval, the blocked-IP cache can evict too early or retain too long under pressure. Consider threading the same TTL into AddBlocked (or centralizing the TTL constant) so eviction is consistent with Cleanup().
				retest := pool.retestInterval()
				pool.state.connState.Cleanup()
				pool.state.tlsCache.Cleanup()
				pool.state.destState.Cleanup(retest)
				pool.state.hostHints.Cleanup()

src/dns/answer.go:91

  • encodeName() silently drops invalid labels (empty or >63 bytes). When rewriting a DNS message, this can change the effective owner name / CNAME target if the upstream response is malformed or adversarial, producing a rewritten payload that no longer matches the original semantics. It would be safer to treat such names as un-rewriteable and fall back to FilterUnchanged instead of attempting to "sanitize" them during rewrite.
	for _, label := range strings.Split(name, ".") {
		if label == "" || len(label) > 63 {
			continue
		}

Copilot AI review requested due to automatic review settings August 8, 2026 16:06
@sonarqubecloud

sonarqubecloud Bot commented Aug 8, 2026

Copy link
Copy Markdown

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 31 out of 31 changed files in this pull request and generated no new comments.

Suppressed comments (3)

src/nfq/ipblock.go:46

  • This call into sendSynRSTToClientV6 does not pass pkt.ihl, but the IPv6 SYN may include extension headers (parseIPHeaders sets pkt.ihl accordingly). If sendSynRSTToClientV6/buildSynRSTV6 are updated to use the correct TCP offset, update this call to pass pkt.ihl so the RST/ACK is built from the real TCP header.
	if pkt.ver == IPv4 {
		w.sendSynRSTToClientV4(pkt.raw, pkt.ihl, pkt.src, pkt.dst)
	} else {
		w.sendSynRSTToClientV6(pkt.raw, pkt.src, pkt.dst)
	}

src/nfq/rst_client.go:103

  • buildSynRSTV6 assumes the TCP header starts at a fixed 40-byte IPv6 header (ipv6HdrLen = 40). However, parseIPHeaders tracks IPv6 extension headers via pkt.ihl; if a SYN includes extension headers, this function will read ports/seq from the wrong offset and build an invalid RST/ACK, so the client may not reset promptly.
	const ipv6HdrLen = 40
	if len(raw) < ipv6HdrLen+20 {
		return nil
	}
	tcp := raw[ipv6HdrLen:]

src/nfq/rst_client_test.go:79

  • buildSynRSTV6 currently assumes a fixed 40-byte IPv6 header; if the implementation is updated to accept an ihl/tcpOffset (to handle extension headers), update this test to pass the offset (typically 40 for packets without extensions), and consider adding a test case with an extension header to prevent regressions.
	rst := buildSynRSTV6(syn, client, server)

@DanielLavrushin
DanielLavrushin merged commit c15b254 into main Aug 8, 2026
3 checks passed
@DanielLavrushin
DanielLavrushin deleted the ip-block-heal branch August 8, 2026 17:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants