perf(policy): decide the bare-domain FQDN match by index, not by concatenation

[KidCarmi]

סיכום / Summary

hostutil.MatchFQDNNorm is called once per rule, per request: evalAccessRules walks the enabled rulebase and every rule carrying a DestFQDN reaches it (policy.go matchDestNorm), and the SSL-bypass matcher walks its own pattern list the same way per CONNECT. Its cost is multiplied by the rule count on every proxied request, and an allowed request pays the MISS on every rule ahead of the one that matches.

A CPU profile of the end-to-end proxy benchmark (BenchmarkPerfQual_ProxyHTTPForward, rules=100) put matchFQDNNorm at 150 ms of evalAccessRules' 270 ms — the hottest leaf in the policy scan, and the largest single Culvert-owned line on the allowed path after the upstream round trip.

The bare-domain branch decided its suffix test as strings.HasSuffix(host, "."+pattern). That concatenation exists only to be compared and thrown away, and Go hands a non-escaping concatenation a 32-byte stack buffer — so a pattern of ≤31 bytes cost a copy, and anything longer cost a heap allocation. Long is the ordinary shape for bare-domain rules operators actually write (assets.cdn.example-corporation.com is 34 bytes), which put the allocation on the wrong side of the common case: one per rule, per request. A 100-rule policy of such patterns burned ~4.8 KB of garbage on every proxied request, scaling with the rulebase.

Deciding the same test by index costs neither. With n = len(host)-len(pattern), strings.HasSuffix(host, "."+pattern) is true exactly when n > 0 && host[n-1] == '.' && host[n:] == pattern.

The "*." branch is deliberately untouched — pattern[1:] is a substring, which never allocated. A benchmark pins it so a future unification of the two branches cannot regress it silently.

סוג שינוי / Change type:

• refactor — שיפור קוד ללא שינוי פונקציונלי / refactor (performance; no behaviour change)

---

Measurements

MatchFQDNNorm, 100 rules that all miss (hostutil_matchfqdn_bench_test.go, median of 5). The baseline benchmark runs the verbatim pre-fix body in the same binary, so the before/after is reproducible from one file without checking out the parent commit:

pattern	before	after
26 bytes	2160 ns/op, 0 B, 0 allocs	660 ns/op, 0 B, 0 allocs
37 bytes	5171 ns/op, 4800 B, 100 allocs	492 ns/op, 0 B, 0 allocs
54 bytes	5363 ns/op, 6400 B, 100 allocs	422 ns/op, 0 B, 0 allocs

The 26-byte row never allocated and still gets 3.3× faster, so the gain does not depend on a policy being written with long patterns.

Whole-engine, through the real PolicyStore.Evaluate entry point (BenchmarkPolicyEvaluate_NoMatch, 26-byte patterns — the conservative half, the case that never allocated; median of 5, measured by stashing this change):

rules	before	after	delta
10	382 ns/op	238 ns/op	−37.7%
100	3001 ns/op	1493 ns/op	−50.2%
1000	30166 ns/op	14526 ns/op	−51.8%
10000	348764 ns/op	242599 ns/op	−30.4%

Policy evaluation is the largest Culvert-owned cost on the request path; at realistic rule counts it is now roughly 2× faster.

Not claimed: BenchmarkPerfQual_ProxyHTTPForward is dominated by loopback syscalls and the in-process client/backend (~374 µs cpu-ns/op, of which handleRequest is ~13%), so it cannot resolve a ~10 µs CPU delta and is reported here as unchanged-within-noise rather than as a win.

---

בדיקות שבוצעו / Testing Done

go test ./internal/...                       # all pass
go test -count=1 -timeout=45m .              # root suite: ok 137s
go test -race ./internal/hostutil/ ./internal/sslbypass/
go test -race -run 'FQDN|Policy|Bypass' .    # ok
go vet ./... && go vet -tags benchgate ./... # clean
gofmt -l internal/hostutil/                  # clean
go build -o culvert .                        # ok

Correctness is the load-bearing part of this PR, because a behaviour difference here would be a rule that stops matching, or one that starts matching a host it should not. Three gates, all in internal/hostutil/hostutil_matchfqdn_bench_test.go:

• TestMatchFQDNNorm_MatchesPreOptimizationBehaviour — differential against the verbatim pre-fix body over hand-picked boundary shapes: empty pattern, lone/double dots, pattern longer than host, equal lengths that are not equal values, trailing dots, and the label-boundary confusion this rewrite could plausibly introduce (notexample.com / xexample.com vs example.com).
• FuzzMatchFQDNNorm — the open-ended half. 6.0M executions, zero divergence (-fuzztime=45s, 255 corpus entries).
• TestMatchFQDNNorm_AllocRegression — the hardware-independent gate: zero allocations at every pattern length, including the lengths where the old body allocated per rule. Reintroducing "."+pattern fails this immediately.

Plus TestMatchFQDNNorm_BaselineAllocatedAtLongPatterns, which measures the old body in the same binary so the justification above fails loudly rather than quietly becoming folklore if a toolchain change ever invalidates it, and TestMatchFQDNNorm_SubdomainSemanticsUnchanged, which restates the product rule in its own words so the intent survives if the reference body is ever removed.

⚠️ golangci-lint run could not be run in this environment: the preinstalled binary is built with Go 1.25 and panics with package requires newer Go version go1.26 on any package, including untouched ones. This is environmental, not caused by this change; CI's pinned version applies. go vet and gofmt are clean.

---

רשימת בדיקות לפני Merge / Pre-Merge Checklist

קוד / Code Quality

• הרצתי go vet ./... ו-go build ./... בהצלחה
• הרצתי golangci-lint run מקומית — see the note above (toolchain mismatch in this environment; go vet + gofmt clean)
• אין קוד מת, TODO-ים שנשכחו, או log.Printf debug
• פונקציות ופרמטרים חדשים תועדו ב-comments
• לא הכנסתי ספריות חדשות (go.mod לא השתנה)

אבטחה / Security

• אין secrets, passwords, tokens, או keys בקוד (גם לא ב-tests)
• קלט מהמשתמש מאומת לפני שימוש — matching semantics are byte-for-byte unchanged; inputs are already IDNA-normalized by the caller, as before
• אין נתיבי קבצים חדשים
• אין InsecureSkipVerify: true חדש
• שינויים ב-proxy.go/auth*.go/ca.go — none; the diff touches only internal/hostutil

אם נגעת ב-Policy Engine

• בדקתי שכל הרולות הקיימות עדיין עובדות (אין regression) — full root suite green, incl. policy_bypass_security_test.go, which pins the policy engine and the SSL-bypass matcher to the same matcher
• בדקתי התנהגות default-deny / default-allow — BenchmarkPolicyEvaluate_NoMatch asserts default-deny on every iteration; PolicyEvaluate/Bypass suites green

API / Frontend / Documentation

• N/A — no route, endpoint, config field, or UI change. go.mod, api/, static/ untouched.

---

סיכון ו-Rollback / Risk & Rollback

רמת סיכון / Risk level: 🟢 Low

Six lines of straight-line code with no new state, no concurrency, no allocation, and no new dependency. The risk is concentrated entirely in one question — does the index form decide exactly what the concatenating form decided? — which is why the equivalence is argued from the definition of HasSuffix in the source comment and then pinned two independent ways (hand-picked boundary differential + 6M-execution fuzz) against the old body, which is kept in-tree as the oracle.

Residual risk: the differential's oracle is a copy of the old body, so it protects against a rewrite mistake, not against the original semantics having been wrong. That is deliberate — this PR is a cost change and takes the existing FQDN-matching semantics as given.

תוכנית Rollback: git revert this single commit. It is self-contained (one function body plus its tests), touches no persisted state, no wire format, and no config surface, so a revert is safe at any point and needs no coordination with a DP fleet or a config migration.

---

Generated by Claude Code

[KidCarmi]

✅ Snyk checks have passed. No issues have been found so far.

Status	Scan Engine	Critical	High	Medium	Low	Total (0)
✅	Open Source Security	0	0	0	0	0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

[aegisdiff]

❌ AegisDiff Security Triage — ERROR

Field	Value
Verdict	ERROR
Severity	✅ N/A
CWE	N/A
Confidence	0%
Analyzed by	unknown
Sanitizer Found	❌ None detected

All LLM providers exhausted. Last error: Client error '404 Not Found' for url 'h

All LLM providers exhausted. Last error: Client error '404 Not Found' for url 'https://api.groq.com/openai/v1/chat/completions'
For more information check: https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/404

_{Commit 769b42e · PR #1221 · Powered by AegisDiff}