Release 0.1.1-rc.11

Housecarl 0.1.1's eleventh release candidate. Since rc.10: a quota keeps
what it used when its rule counts it differently (per address to a total:
the sum; wider groups: added up), or starts again with a warning. make
test, the real-nft quota test and make e2e (304 checks) pass.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

CHANGELOG.md

@@ -87,6 +87,12 @@ All notable changes to Housecarl are documented here. The format follows
 
 ### Fixed
 
+- Changing how a quota counts lost or muddled what it used: per address
+  to a total started at nothing, per source to per destination kept the
+  sources' use as if it were the destinations', and other groups left
+  the old entries behind. Now a total starts at the addresses' sum,
+  wider groups add up their addresses, and what can't be carried starts
+  again with a warning (`quota_restarts`) on the change.
 - `hc stats top devices` listed each device twice (once for what it
   received, once for what it sent); it is one row with both again.
 - `hc rule add --before/--after/--first` made two changes (create at the

README.md

@@ -123,7 +123,7 @@ The web UI is on `http://127.0.0.1:8070/` (see `listen` in
 
 ## Status
 
-Housecarl 0.1.1 is in testing (0.1.1-rc.10): zones, address sets and
+Housecarl 0.1.1 is in testing (0.1.1-rc.11): zones, address sets and
 services with ordered rules, IPv4 and IPv6, port forwards with hairpin NAT
 and IPv6 pinholes, source NAT and redirects, built-ins, probation for
 network changes (confirmed by the client that made them), the ruleset

VERSION

@@ -1 +1 @@
-0.1.1-rc.10
+0.1.1-rc.11

docs/rules.md

@@ -215,6 +215,20 @@ A per-address quota counts an address: a device that gets a new one
 starts again. With Steward's reservations IPv4 addresses stay; IPv6 is
 grouped by /64 (below).
 
+Changing the amount, `over` or the resets keeps what was used. Changing
+how the quota counts carries what can be carried:
+
+| Change | What was used |
+|---|---|
+| per address to one total | the addresses' sum |
+| to wider groups (IPv6 /64 to /56) | added up per new group |
+| a total to per address | starts again: a total can't be split |
+| per source to per destination (or back) | starts again |
+| to finer groups (/64 to /128) | starts again: a group can't be split |
+
+A change that starts a quota again says so (warning `quota_restarts`),
+when it is staged for review and in `hc` (also with `-n`).
+
 **Rules with a quota or a byte rate count every packet.** The built-in
 that accepts established connections comes before your rules, so a rule
 there only sees each connection's first packet. Rules that count traffic

docs/validation.md

@@ -97,6 +97,7 @@ These apply to rules, source NAT and redirects.
 | `rule_group` | error | a grouping prefix outside 1-32 (IPv4) or 1-128 (IPv6) |
 | `rule_traffic_accept` | error | a rule with a quota or a byte rate that accepts: it is checked before established connections are accepted, so it can only drop, reject or log |
 | `rule_every_packet_accept` | error | a rule checked against every packet (`every_packet`) that accepts: it comes before established connections are accepted, so it can only drop, reject or log |
+| `quota_restarts` | warning | a change makes a rule count its quota so that what it used can't carry over (a total to per address, per source to per destination, finer groups): it starts again. Only on the change that does it |
 | `flowtable_traffic` | warning | a forward rule checked against every packet (a quota, a byte rate, `every_packet`) while the flowtable is on: it only sees each connection's first packets |
 | `reject_with` | error | `tcp-reset` on a rule that matches more than TCP, or with an action other than reject |
 | `log_prefix` | error | a log prefix over 100 characters |

internal/engine/engine.go

@@ -191,6 +191,7 @@ func (e *Engine) commit(ctx context.Context, p *auth.Principal, description stri
 	// Only report warnings the change brings; old ones are on the status
 	// page.
 	res.Warnings = newProblems(validate.Check(e.state, e.env()), problems)
+	res.Warnings = append(res.Warnings, quotaRestarts(e.state, next)...)
 	if e.Firewall != nil {
 		if err := e.Firewall.Check(ctx, next); err != nil {
 			return res, &Error{Kind: KindFirewall, Message: "nftables refused the ruleset: " + err.Error()}
@@ -230,6 +231,23 @@ func (e *Engine) commit(ctx context.Context, p *auth.Principal, description stri
 	return res, nil
 }
 
+// quotaRestarts warns of the rules whose quota starts again because they
+// now count it in a way what they used can't be carried to.
+func quotaRestarts(before, after *model.State) []validate.Problem {
+	was := map[int64]*model.Rule{}
+	for _, r := range before.Rules {
+		was[r.ID] = r
+	}
+	var out []validate.Problem
+	for _, r := range after.Rules {
+		if carry, why := model.QuotaCarry(was[r.ID], r); carry == model.QuotaRestarts {
+			out = append(out, validate.Problem{Severity: validate.Warning, Code: "quota_restarts", Object: r.Label(),
+				Message: "its quota starts again: " + why})
+		}
+	}
+	return out
+}
+
 func newProblems(before, after []validate.Problem) []validate.Problem {
 	seen := map[string]bool{}
 	for _, p := range before {

internal/engine/engine_test.go

@@ -173,3 +173,49 @@ func TestPositionsStayStableUntilCrowded(t *testing.T) {
 		t.Errorf("order %v", order)
 	}
 }
+
+// TestQuotaRestartsWarns: a change that counts a quota so what it used
+// can't carry over warns; one that can (to a total, wider groups) doesn't.
+func TestQuotaRestartsWarns(t *testing.T) {
+	e := newEngine(t)
+	ctx := context.Background()
+	var id int64
+	if _, err := e.Apply(ctx, admin, "", Options{}, func(st *model.State, newID func() int64) error {
+		id = newID()
+		st.Rules = append(st.Rules, &model.Rule{ID: id, Chain: "forward", Action: "drop", Quota: &model.Quota{Bytes: 1 << 30, Over: true, PerSource: true}})
+		return nil
+	}); err != nil {
+		t.Fatal(err)
+	}
+	change := func(f func(r *model.Rule)) []string {
+		res, err := e.Apply(ctx, admin, "", Options{DryRun: true}, func(st *model.State, _ func() int64) error {
+			f(st.ByID(model.KindRule, id).(*model.Rule))
+			return nil
+		})
+		if err != nil {
+			t.Fatal(err)
+		}
+		var codes []string
+		for _, w := range res.Warnings {
+			if w.Code == "quota_restarts" {
+				codes = append(codes, w.Message)
+			}
+		}
+		return codes
+	}
+	if w := change(func(r *model.Rule) { r.Quota.PerSource, r.Quota.PerDestination = false, true }); len(w) != 1 || !strings.Contains(w[0], "per source can't be counted per destination") {
+		t.Errorf("per source to per destination: %v", w)
+	}
+	if w := change(func(r *model.Rule) { r.Quota.PerSource = false }); len(w) != 0 {
+		t.Errorf("to a total (summed): %v", w)
+	}
+	if w := change(func(r *model.Rule) { r.GroupV6 = 56 }); len(w) != 0 {
+		t.Errorf("wider groups: %v", w)
+	}
+	if w := change(func(r *model.Rule) { r.GroupV6 = 128 }); len(w) != 1 {
+		t.Errorf("finer groups: %v", w)
+	}
+	if w := change(func(r *model.Rule) { r.Quota.Bytes = 2 << 30 }); len(w) != 0 {
+		t.Errorf("another amount: %v", w)
+	}
+}

internal/model/objects.go

@@ -274,6 +274,45 @@ func (r *Rule) Groups() (v4, v6 int) {
 	return v4, v6
 }
 
+// What becomes of what a rule's quota used when the rule changes
+// (QuotaCarry).
+const (
+	QuotaKept      = ""          // counted as before: carried as it is
+	QuotaSummed    = "summed"    // per address to a total: the addresses' sum
+	QuotaRegrouped = "regrouped" // per address in wider groups: added up per group
+	QuotaRestarts  = "restarts"  // can't be carried: starts again
+)
+
+// QuotaCarry says what becomes of what old's quota used when the rule
+// becomes new (both the same rule, with quotas), and for QuotaRestarts
+// why. The amount, over and the resets don't matter: only how it counts.
+func QuotaCarry(old, new *Rule) (carry, why string) {
+	if old == nil || new == nil || old.Quota == nil || new.Quota == nil {
+		return QuotaKept, ""
+	}
+	was, is := old.Quota.PerAddress(), new.Quota.PerAddress()
+	words := map[string]string{"saddr": "per source", "daddr": "per destination", "": "in total"}
+	switch {
+	case was == "" && is == "":
+		return QuotaKept, ""
+	case is == "":
+		return QuotaSummed, ""
+	case was == "":
+		return QuotaRestarts, "a total can't be split by address, so each address starts at nothing"
+	case was != is:
+		return QuotaRestarts, "what was counted " + words[was] + " can't be counted " + words[is]
+	}
+	o4, o6 := old.Groups()
+	n4, n6 := new.Groups()
+	switch {
+	case o4 == n4 && o6 == n6:
+		return QuotaKept, ""
+	case n4 > o4 || n6 > o6:
+		return QuotaRestarts, "addresses are grouped more finely now, and a group's use can't be split"
+	}
+	return QuotaRegrouped, ""
+}
+
 // PortForward publishes a service on an inside host: incoming connections
 // to a public address and port are sent on (DNAT) to To[:ToPort], let
 // through the forward chain, and, with Hairpin, also work from inside.

internal/model/quota_test.go

@@ -0,0 +1,26 @@
+package model
+
+import "testing"
+
+func TestQuotaCarry(t *testing.T) {
+	rule := func(per string, v4, v6 int) *Rule {
+		return &Rule{GroupV4: v4, GroupV6: v6, Quota: &Quota{Bytes: 1, PerSource: per == "src", PerDestination: per == "dst"}}
+	}
+	for _, c := range []struct {
+		old, new *Rule
+		want     string
+	}{
+		{rule("", 0, 0), rule("", 0, 0), QuotaKept},
+		{rule("src", 0, 0), rule("src", 32, 64), QuotaKept}, // the defaults, written out
+		{rule("src", 0, 0), rule("", 0, 0), QuotaSummed},
+		{rule("", 0, 0), rule("src", 0, 0), QuotaRestarts},
+		{rule("src", 0, 0), rule("dst", 0, 0), QuotaRestarts},
+		{rule("src", 0, 0), rule("src", 24, 56), QuotaRegrouped},
+		{rule("src", 0, 0), rule("src", 24, 128), QuotaRestarts},
+		{nil, rule("src", 0, 0), QuotaKept},
+	} {
+		if got, _ := QuotaCarry(c.old, c.new); got != c.want {
+			t.Errorf("%+v -> %+v: %q, want %q", c.old, c.new, got, c.want)
+		}
+	}
+}

internal/nft/nft.go

@@ -15,6 +15,7 @@ import (
 	"errors"
 	"fmt"
 	"net"
+	"net/netip"
 	"os"
 	"os/exec"
 	"path/filepath"
@@ -424,6 +425,9 @@ type Firewall struct {
 	restore *QuotaUsage
 	// adjust are quota changes the next apply makes (AdjustQuota).
 	adjust []QuotaAdjust
+	// quotaRules are the rules with a quota in the ruleset in force, by
+	// ID: what their usage was counted by (nil before the first apply).
+	quotaRules map[int64]*model.Rule
 	// flowDevs are the flowtable's devices in the ruleset in force, by
 	// name, with their interface index then (0: left out, missing).
 	flowDevs map[string]int
@@ -510,6 +514,73 @@ func (u *QuotaUsage) apply(adj []QuotaAdjust) {
 	}
 }
 
+// carry keeps what quotas used when their rules now count differently
+// (model.QuotaCarry): per address to a total, the sum; to wider groups,
+// the groups' sums; what can't be carried starts again. was are the
+// rules in force before (nil: nothing to compare with).
+func (u *QuotaUsage) carry(was map[int64]*model.Rule, st *model.State) {
+	for _, r := range st.Rules {
+		old := was[r.ID]
+		carry, _ := model.QuotaCarry(old, r)
+		if carry == model.QuotaKept {
+			continue
+		}
+		name := render.QuotaName(r.ID)
+		sets := []string{name + "_v4", name + "_v6"}
+		switch carry {
+		case model.QuotaSummed:
+			var sum int64
+			for _, set := range sets {
+				for _, e := range u.QuotaSets[set] {
+					sum += e.Used
+				}
+			}
+			if u.Quotas == nil {
+				u.Quotas = map[string]int64{}
+			}
+			u.Quotas[name] = sum
+		case model.QuotaRegrouped:
+			v4, v6 := r.Groups()
+			for _, set := range sets {
+				byKey := map[string]int64{}
+				var order []string
+				for _, e := range u.QuotaSets[set] {
+					a, err := netip.ParseAddr(e.Addr)
+					if err != nil {
+						continue
+					}
+					bits := v4
+					if a.Is6() {
+						bits = v6
+					}
+					p, _ := a.Prefix(bits)
+					k := p.Addr().String()
+					if _, ok := byKey[k]; !ok {
+						order = append(order, k)
+					}
+					byKey[k] += e.Used
+				}
+				if len(order) == 0 {
+					continue
+				}
+				var es []render.Element
+				for _, k := range order {
+					es = append(es, render.Element{Addr: k, Used: byKey[k]})
+				}
+				u.QuotaSets[set] = es
+			}
+			continue
+		}
+		// What was counted the other way goes: a total, or the entries.
+		if carry != model.QuotaSummed {
+			delete(u.Quotas, name)
+		}
+		for _, set := range sets {
+			delete(u.QuotaSets, set)
+		}
+	}
+}
+
 // ResetQuota starts a rule's quota again: the named quota, or the
 // per-address quota sets (both families).
 func (f *Firewall) ResetQuota(ctx context.Context, ruleID int64, perAddress bool) error {
@@ -579,6 +650,7 @@ func (f *Firewall) Apply(ctx context.Context, st *model.State) error {
 		usage = old.QuotaUsage
 	}
 	usage.merge(f.restore)
+	usage.carry(f.quotaRules, st)
 	usage.apply(f.adjust)
 	f.adjust = nil
 	opt.Quotas, opt.QuotaSets = usage.Quotas, usage.QuotaSets
@@ -589,6 +661,14 @@ func (f *Firewall) Apply(ctx context.Context, st *model.State) error {
 		return err
 	}
 	f.restore = nil
+	f.quotaRules = map[int64]*model.Rule{}
+	for _, r := range st.Rules {
+		if r.Quota != nil {
+			c, q := *r, *r.Quota // only the quota and the grouping are read
+			c.Quota = &q
+			f.quotaRules[r.ID] = &c
+		}
+	}
 	f.flowDevs = map[string]int{}
 	for _, d := range st.Settings.FlowtableDevices {
 		if opt.Interfaces == nil || opt.Interfaces[d] {

internal/nft/nft_test.go

@@ -197,6 +197,21 @@ func TestQuotasInContainer(t *testing.T) {
 	if t2, d2 := used(); t2 > total-1000 || t2 < total-2000 || d2 != 0 {
 		t.Errorf("after giving back: total %d (was %d, gave 1000), per destination %d (was %d, reset)", t2, total, d2, dev)
 	}
+	// Counted in total now: what the addresses used carries over.
+	ping("127.0.0.3")
+	_, dev = used()
+	st.Rules[1].Quota.PerDestination = false
+	if err := fw.Apply(ctx, st); err != nil {
+		t.Fatal(err)
+	}
+	tb, _ := fw.Table(ctx)
+	if tb.Quotas["quota_r2"] < dev || dev == 0 {
+		t.Errorf("per destination to a total: %d, was %d", tb.Quotas["quota_r2"], dev)
+	}
+	st.Rules[1].Quota.PerDestination = true
+	if err := fw.Apply(ctx, st); err != nil {
+		t.Fatal(err)
+	}
 	// Reset by hand.
 	if err := fw.ResetQuota(ctx, 1, false); err != nil {
 		t.Fatal(err)
@@ -211,7 +226,7 @@ func TestQuotasInContainer(t *testing.T) {
 	// saved usage comes back.
 	ping("127.0.0.2")
 	ping("127.0.0.3")
-	tb, _ := fw.Table(ctx)
+	tb, _ = fw.Table(ctx)
 	saved := tb.QuotaUsage
 	exec.Command("docker", "exec", name, "nft", "delete", "table", "inet", "housecarl").Run()
 	fw.RestoreQuotas(&saved)
@@ -240,6 +255,45 @@ func TestQuotaAdjust(t *testing.T) {
 	}
 }
 
+// TestQuotaCarry: usage follows a rule counting its quota differently.
+func TestQuotaCarry(t *testing.T) {
+	perSource := &model.Rule{ID: 2, Quota: &model.Quota{Bytes: 1 << 30, PerSource: true}}
+	usage := func() QuotaUsage {
+		return QuotaUsage{Quotas: map[string]int64{}, QuotaSets: map[string][]render.Element{
+			"quota_r2_v4": {{Addr: "10.0.0.1", Used: 100}, {Addr: "10.0.0.2", Used: 50}},
+			"quota_r2_v6": {{Addr: "2001:db8:0:1::", Used: 7}, {Addr: "2001:db8:0:2::", Used: 3}}}}
+	}
+	was := map[int64]*model.Rule{2: perSource}
+	with := func(f func(r *model.Rule)) *model.State {
+		r := *perSource
+		q := *perSource.Quota
+		r.Quota = &q
+		f(&r)
+		return &model.State{Rules: []*model.Rule{&r}}
+	}
+	u := usage()
+	u.carry(was, with(func(r *model.Rule) { r.Quota.PerSource = false }))
+	if u.Quotas["quota_r2"] != 160 || len(u.QuotaSets["quota_r2_v4"]) != 0 {
+		t.Errorf("to a total: %+v", u)
+	}
+	u = usage()
+	u.carry(was, with(func(r *model.Rule) { r.GroupV4, r.GroupV6 = 24, 48 }))
+	want := map[string][]render.Element{"quota_r2_v4": {{Addr: "10.0.0.0", Used: 150}}, "quota_r2_v6": {{Addr: "2001:db8::", Used: 10}}}
+	if !reflect.DeepEqual(u.QuotaSets, want) {
+		t.Errorf("wider groups: %+v", u.QuotaSets)
+	}
+	u = usage()
+	u.carry(was, with(func(r *model.Rule) { r.Quota.PerSource, r.Quota.PerDestination = false, true }))
+	if len(u.QuotaSets) != 0 {
+		t.Errorf("per destination now: %+v", u.QuotaSets)
+	}
+	u = usage()
+	u.carry(nil, with(func(r *model.Rule) { r.Quota.PerSource = false }))
+	if len(u.QuotaSets["quota_r2_v4"]) != 2 || u.Quotas["quota_r2"] != 0 {
+		t.Errorf("nothing to compare with (the first apply): %+v", u)
+	}
+}
+
 // TestDeviceCountsInContainer: the per-device counters are read back and
 // survive a reload.
 func TestDeviceCountsInContainer(t *testing.T) {

scripts/e2e.sh

@@ -342,6 +342,15 @@ run "the device is back" 0 send 1000
 run "the reset is audited" 0 H audit --action quota.
 contains "quota.reset"
 contains "quota.give"
+# Counted in total now: what the device used carries over; per
+# destination can't take it, and says so.
+run "within the quota" 0 send 300000
+run "to one total" 0 H rule update "$QR" --quota 1mbytes,over
+not_contains "quota_restarts"
+run "the total carried over" 0 H quotas
+contains "e2e quota)  *[0-9.]* KB"
+run "per destination starts again" 0 H rule update "$QR" --quota 1mbytes,over,per-destination
+contains "quota_restarts"
 run "the quota rule goes" 0 H rule delete -y "$QR"
 
 # ---- the zone matrix, of the imported ruleset