From ca5ac971e5296371b37d34563250147355a0826c Mon Sep 17 00:00:00 2001 From: abrown38 Date: Sun, 25 Oct 2026 00:02:12 +0100 Subject: [PATCH] Keep built-ins and policies out of the busiest rules The established built-in passes most of the traffic, so it topped the statistics' busiest rules every time and made every rule's bar look empty. The summary now lists rules and NAT objects in rules, and built-ins and policies in builtins; the Statistics page and hc stats show them as two lists. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 5 +++++ cmd/hc/stats.go | 2 ++ docs/stats.md | 6 ++++-- internal/server/h_stats.go | 6 ++++-- internal/stats/collector_test.go | 9 +++++++++ internal/stats/query.go | 19 ++++++++++++++----- internal/web/static/assets/pages/stats.js | 3 ++- 7 files changed, 40 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 55bf90d..c9b3c67 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -124,6 +124,11 @@ All notable changes to Housecarl are documented here. The format follows ### Changed +- Statistics' "Busiest rules" (the Statistics page, `hc stats`, + `GET /stats/summary`) lists only rules and NAT objects. Built-ins and + policies have their own list ("Built-ins and policies", `builtins` in + the API): the established built-in passes most of the traffic, so it + topped the list every time and made every rule's bar look empty. - `hc user delete` asks first, as every other delete does, and takes `-y`/`--yes` not to; it deleted the user and their tokens at once. - The ruleset's diff (Changes, `hc changeset diff`, `--dry-run`) names diff --git a/cmd/hc/stats.go b/cmd/hc/stats.go index ceddad4..250d2de 100644 --- a/cmd/hc/stats.go +++ b/cmd/hc/stats.go @@ -33,6 +33,7 @@ type statSummary struct { Drops int64 `json:"drops"` DropsByChain []statEntry `json:"drops_by_chain"` Rules []statEntry `json:"rules"` + BuiltIns []statEntry `json:"builtins"` PeakConnections int64 `json:"peak_connections"` ConntrackMax int64 `json:"conntrack_max"` BlockedSources []statEntry `json:"blocked_sources"` @@ -136,6 +137,7 @@ func printSummary(w io.Writer, s statSummary) { } packets := func(n int64) string { return humanCount(uint64(n)) + " packets" } list("Busiest rules:", s.Rules, packets) + list("Built-ins and policies:", s.BuiltIns, packets) list("Blocked most (logged drops):", s.BlockedSources, packets) list("What they tried:", s.BlockedPorts, packets) list("Devices, sent:", s.DevicesUp, humanBytes) diff --git a/docs/stats.md b/docs/stats.md index 8bce8c4..17f944d 100644 --- a/docs/stats.md +++ b/docs/stats.md @@ -5,7 +5,9 @@ Housecarl keeps what the firewall did over time: - traffic per interface and zone, received and sent; - packets dropped, by chain, and when drops happen (a weekday × hour heatmap); -- hits per rule, built-in and policy, and the rules that matched nothing; +- hits per rule, and per built-in and policy in a list of their own (the + established built-in counts most packets), and the rules that matched + nothing; - the sources blocked most and what they tried (from the log); - traffic per inside device (a setting, below); - connections tracked, against the kernel's limit; @@ -127,7 +129,7 @@ answer. | Endpoint | | |---|---| | `GET /stats/status` | `enabled`, `database`, `size`, `first_data`, `last_sample`, `sample_error`, `per_device`, `retention`, `latest` (the newest gauges) | -| `GET /stats/summary?since=24h&limit=10` | `zones` and `interfaces` (`name`, `received`, `sent`), `drops`, `drops_by_chain`, `rules` (busiest, named), `peak_connections`, `conntrack_max`, `blocked_sources`, `blocked_ports`, `devices_up`, `devices_down` | +| `GET /stats/summary?since=24h&limit=10` | `zones` and `interfaces` (`name`, `received`, `sent`), `drops`, `drops_by_chain`, `rules` (busiest rules and NAT objects, named), `builtins` (busiest built-ins and policies, named), `peak_connections`, `conntrack_max`, `blocked_sources`, `blocked_ports`, `devices_up`, `devices_down` | | `GET /stats/timeline?keys=zrx:*,drops&since=24h&step=auto` | `step`, `times` (bucket starts), `series` (`key`, `values`); `GROUP:*` means every key of a group; `step` is `auto`, `1m`, `5m`, `15m`, `1h`, `6h` or `1d`; a gauge without a sample is -1 | | `GET /stats/top?kind=device_down&since=7d&limit=20` | a top list (`key`, `count`) | | `GET /stats/hits?since=30d` | packets per rule, built-in and policy (`key`, `count`, `name`), with the enabled rules that counted none | diff --git a/internal/server/h_stats.go b/internal/server/h_stats.go index 408348a..f5165a8 100644 --- a/internal/server/h_stats.go +++ b/internal/server/h_stats.go @@ -150,8 +150,10 @@ func (s *Server) statsSummary(w http.ResponseWriter, r *http.Request, p *auth.Pr return err } st := s.eng.State() - for i := range sum.Rules { - sum.Rules[i].Name = markName(st, sum.Rules[i].Key) + for _, es := range [][]stats.Entry{sum.Rules, sum.BuiltIns} { + for i := range es { + es[i].Name = markName(st, es[i].Key) + } } writeJSON(w, http.StatusOK, sum) return nil diff --git a/internal/stats/collector_test.go b/internal/stats/collector_test.go index 2a5a140..e9502a2 100644 --- a/internal/stats/collector_test.go +++ b/internal/stats/collector_test.go @@ -7,6 +7,7 @@ import ( "log/slog" "os" "path/filepath" + "slices" "testing" "time" @@ -154,6 +155,14 @@ func TestCollector(t *testing.T) { t.Fatal(err) } check("drops", s.Drops, 18) + for _, e := range s.Rules { + if e.Key[0] == 'b' || e.Key[0] == 'p' { + t.Errorf("built-in or policy %s among the busiest rules %+v", e.Key, s.Rules) + } + } + if !slices.ContainsFunc(s.Rules, func(e Entry) bool { return e.Key == "r1" }) || !slices.ContainsFunc(s.BuiltIns, func(e Entry) bool { return e.Key == "p-input" }) { + t.Errorf("rules %+v, built-ins %+v", s.Rules, s.BuiltIns) + } if len(s.Zones) == 0 || s.Zones[0].Name != "wan" || s.Zones[0].Received != 4000 { t.Errorf("zones %+v", s.Zones) } diff --git a/internal/stats/query.go b/internal/stats/query.go index 97e00ba..b2d223b 100644 --- a/internal/stats/query.go +++ b/internal/stats/query.go @@ -191,8 +191,11 @@ type Summary struct { // chain. Drops int64 `json:"drops"` DropsByChain []Entry `json:"drops_by_chain"` - // Rules are the busiest rules, built-ins and policies, by packets. - Rules []Entry `json:"rules"` + // Rules are the busiest rules and NAT objects, by packets; BuiltIns + // the busiest built-ins and policies, kept apart because the + // established built-in counts most of the traffic. + Rules []Entry `json:"rules"` + BuiltIns []Entry `json:"builtins"` // Connections: the most tracked at once (per-bucket averages), and // the table's size. PeakConnections int64 `json:"peak_connections"` @@ -251,9 +254,15 @@ func (q *Querier) Summary(ctx context.Context, since time.Duration, topLimit int s.Zones, s.Interfaces = traffic("zrx:", "ztx:"), traffic("rx:", "tx:") s.Drops = all["drops"] s.DropsByChain = sorted(all, "drop:") - s.Rules = sorted(all, "hits:") - if len(s.Rules) > topLimit { - s.Rules = s.Rules[:topLimit] + s.Rules, s.BuiltIns = []Entry{}, []Entry{} + for _, e := range sorted(all, "hits:") { + if strings.HasPrefix(e.Key, "b-") || strings.HasPrefix(e.Key, "p-") { + if len(s.BuiltIns) < topLimit { + s.BuiltIns = append(s.BuiltIns, e) + } + } else if len(s.Rules) < topLimit { + s.Rules = append(s.Rules, e) + } } if tl, err := q.Timeline(ctx, since, "auto", []string{"g:" + GaugeConntrack, "g:" + GaugeConntrackMax}); err == nil { for _, v := range tl.Series[0].Values { diff --git a/internal/web/static/assets/pages/stats.js b/internal/web/static/assets/pages/stats.js index c240907..e400313 100644 --- a/internal/web/static/assets/pages/stats.js +++ b/internal/web/static/assets/pages/stats.js @@ -98,7 +98,8 @@ export async function render(el, params, query) { ? "No traffic per device " + rangeLabel + "." : ["Traffic per device is off. Turn it on under ", h("a", { href: "#/settings" }, "Settings"), " (it adds a counter for each inside address)."]); parts.push(h("div", { class: "chart-grid" }, - listCard("Busiest rules", "packets each counted, " + rangeLabel, barList(s.rules, { label: (k) => k, href: (k) => ruleHref(k) })), + listCard("Busiest rules", "packets each counted, " + rangeLabel, barList(s.rules, { label: (k) => k, href: (k) => ruleHref(k), empty: "No rule counted anything " + rangeLabel + "." })), + listCard("Built-ins and policies", "packets each counted; 'established' passes the replies and the rest of every allowed connection", barList(s.builtins || [], { label: (k) => k, href: (k) => ruleHref(k) })), listCard("Dropped, by chain", "early: antispoofing · invalid: packets conntrack can't place", barList(s.drops_by_chain, { empty: "Nothing dropped " + rangeLabel + "." })), listCard("Blocked most", "sources of logged drops (the policies' with 'log drops' on, and rules that log)", barList(s.blocked_sources, { empty: "No logged drops " + rangeLabel + ".", href: (k) => "#/trace?address=" + encodeURIComponent(k) })), listCard("What they tried", "the logged drops' protocol and port", barList(s.blocked_ports, { empty: "No logged drops " + rangeLabel + "." })),