Import: keep what rulesets mean, name zones as asked

Fixes from an audit of the importer, each of which could change what an
imported ruleset does: two base chains on one hook imported their rules
twice (and hoisted the second's built-ins), now in priority order with
the stricter policy; raw rules from a jumped-to chain lost the jump's
condition, now put in front; a jump and a rule matching one field are
combined only when one's addresses lie within the other's; a second
match on a field replaced the first, and != on hop limit and TCP flags
read as ==, now kept raw; a forward accept without ct status dnat no
longer folds into the port forward, nor an interface-limited IPv6
accept into an open pinhole; raw rules' set references are renamed
(NAME_v4/_v6) or written out, so nftables takes them; DNATs with
matches a forward can't hold are left out with a note.

Sets with flags timeout become dynamic sets; a DNAT of several ports one
forward per port. Zone names: --zone IFACE=NAME (and on the Import
page), VLAN defines, more suffixes, and a note on where each name came
from. testdata/audit.nft covers the cases, checked with nft 1.1.3.

make test and make e2e (246 checks) pass.

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

internal/api/types.go

@@ -198,6 +198,9 @@ type ImportInput struct {
 	// Replace replaces Housecarl's configuration; without it, the import
 	// is refused when there is one.
 	Replace bool `json:"replace,omitempty"`
+	// Zones names the zones by interface ({"enp2s0": "lan"}); interfaces
+	// given the same name share a zone.
+	Zones map[string]string `json:"zones,omitempty"`
 }
 
 // ImportResult reports an import.

internal/importer/audit_test.go

@@ -0,0 +1,98 @@
+package importer
+
+import (
+	"context"
+	"os"
+	"os/exec"
+	"strings"
+	"testing"
+
+	"housecarl/internal/render"
+	"housecarl/internal/validate"
+)
+
+// TestImportAudit: rulesets the importer must not change the meaning of
+// (testdata/audit.nft, as nft 1.1.3 lists it).
+func TestImportAudit(t *testing.T) {
+	js, _ := os.ReadFile("testdata/audit.json")
+	txt, _ := os.ReadFile("testdata/audit.txt")
+	defs, _ := os.ReadFile("testdata/audit.nft")
+	res, err := Import(Input{JSON: js, Text: string(txt), Defines: string(defs), Zones: map[string]string{"enp2s0.20": "iot", "eth9": "x"}})
+	if err != nil {
+		t.Fatal(err)
+	}
+	st := res.State
+	notes := strings.Join(res.Notes, "\n")
+	text := render.Render(st, render.Options{}).Text
+	has := func(what, s string) {
+		t.Helper()
+		if !strings.Contains(text, s) {
+			t.Errorf("%s: no %q in\n%s", what, s, text)
+		}
+	}
+	// Two chains on the input hook: their rules once each, in priority
+	// order; established stays a rule after the first chain's rule.
+	if n := strings.Count(text, "192.0.2.66"); n != 1 {
+		t.Errorf("the early input chain's rule %d times", n)
+	}
+	if strings.Index(text, "192.0.2.66") > strings.Index(text, "ct state established,related counter accept comment \"hc:r") ||
+		!strings.Contains(notes, "both hook input") {
+		t.Errorf("chains on one hook:\n%s", text)
+	}
+	// A second match on a field, and != on the hop limit: raw, as written.
+	has("two source matches", "ip saddr 10.0.0.0/8 ip saddr != 10.0.10.0/24 tcp dport 22 drop")
+	has("hop limit", "ip6 hoplimit != 255 drop")
+	// A jump: a rule within it is the rule's own; one outside it, or raw,
+	// keeps the jump's match in front.
+	has("within the jump", `ip saddr 10.9.0.5 tcp dport 22 counter accept comment "hc:r`)
+	has("outside the jump", "ip saddr 10.9.0.0/24 ip saddr 10.8.0.1 tcp dport 23 accept")
+	has("raw under the jump", "ip saddr 10.9.0.0/24 meta mark 0x00000001 accept")
+	// Sets: a timeout set is dynamic; an interface set is written out in
+	// the raw rule that names it.
+	if s := st.Set("blocklist"); s == nil || !s.Dynamic || s.Timeout != 3600 {
+		t.Errorf("blocklist: %+v", s)
+	}
+	has("dynamic set", "ip saddr @blocklist_v4 counter drop")
+	has("interface set", `iifname { "enp2s0", "enp3s0" } accept`)
+	// An accept to a forwarded host that isn't only for forwarded
+	// connections stays a rule; a DNAT of two ports, two forwards.
+	has("not only forwarded", `ip daddr 10.0.10.37 tcp dport 443 counter accept comment "hc:r`)
+	if len(st.Forwards) != 2 || st.Forwards[0].Name != "web-80" || st.Forwards[1].Name != "web-443" {
+		t.Errorf("forwards %+v", st.Forwards)
+	}
+	// Zones: as given (a VLAN), from defines, and why.
+	if z := st.Zone("iot"); z == nil || z.Interfaces[0] != "enp2s0.20" {
+		t.Errorf("zone iot: %+v", st.Zones)
+	}
+	if st.Zone("lan") == nil || st.Zone("wan") == nil {
+		t.Errorf("zones %+v", st.Zones)
+	}
+	for _, want := range []string{"iot (enp2s0.20: as given)", "lan (enp2s0: the define lan_if)", "--zone eth9=x: no rule names"} {
+		if !strings.Contains(notes, want) {
+			t.Errorf("no note %q in\n%s", want, notes)
+		}
+	}
+	if ps := validate.Check(st, nil); validate.HasErrors(ps) {
+		t.Errorf("problems: %+v", validate.Errors(ps))
+	}
+	// And nftables takes it.
+	cmd := strings.Fields(os.Getenv("HOUSECARL_NFT"))
+	if len(cmd) == 0 {
+		return
+	}
+	c := exec.CommandContext(context.Background(), cmd[0], append(cmd[1:], "-c", "-f", "/dev/stdin")...)
+	c.Stdin = strings.NewReader(text)
+	if out, err := c.CombinedOutput(); err != nil {
+		t.Errorf("nft -c: %v\n%s\n%s", err, out, text)
+	}
+}
+
+// TestValidIface: interface names a define may hold, VLANs too.
+func TestValidIface(t *testing.T) {
+	for s, want := range map[string]bool{"enp2s0": true, "wan": true, "enp2s0.20": true, "br0.4094": true, "veth*": true,
+		"10.0.10.5": false, "example.com": false, "a.b.c": false, "enp2s0.20.30": false, "averyveryverylongname": false} {
+		if validIface(s) != want {
+			t.Errorf("validIface(%q) = %v", s, !want)
+		}
+	}
+}

internal/importer/importer.go

@@ -20,6 +20,7 @@ import (
 	"strings"
 
 	"housecarl/internal/model"
+	"housecarl/internal/render"
 )
 
 // Input is what to import.
@@ -34,6 +35,9 @@ type Input struct {
 	// Table is the table to import ("inet main"); empty: the only table
 	// that isn't Housecarl's.
 	Table string
+	// Zones names zones by interface ("enp2s0": "lan"), over the names
+	// the importer would choose; interfaces given one name share a zone.
+	Zones map[string]string
 }
 
 // Result is the imported configuration and what to know about it.
@@ -77,11 +81,13 @@ type nftRule struct {
 }
 
 type nftSet struct {
-	Family string            `json:"family"`
-	Table  string            `json:"table"`
-	Name   string            `json:"name"`
-	Type   json.RawMessage   `json:"type"`
-	Elem   []json.RawMessage `json:"elem"`
+	Family  string            `json:"family"`
+	Table   string            `json:"table"`
+	Name    string            `json:"name"`
+	Type    json.RawMessage   `json:"type"`
+	Flags   []string          `json:"flags"`
+	Timeout int               `json:"timeout"`
+	Elem    []json.RawMessage `json:"elem"`
 }
 
 // nftMap is a named map: its elements are [key, value] pairs.
@@ -121,6 +127,8 @@ type importer struct {
 	meters    map[string]bool   // sets used as meters
 	noted     map[string]bool
 	positions map[string]float64
+	// zoneFrom says where each zone's name came from, for the note.
+	zoneFrom map[string]string
 }
 
 func (im *importer) note(format string, args ...any) {
@@ -136,7 +144,7 @@ func Import(in Input) (*Result, error) {
 		return nil, fmt.Errorf("the input isn't nft's JSON (nft -j list ruleset): %v", err)
 	}
 	im := &importer{in: in, chains: map[string]nftChain{}, rules: map[string][]nftRule{}, sets: map[string]nftSet{}, maps: map[string]nftMap{},
-		text: map[int]string{}, st: model.NewState(), used: map[string]bool{}, noted: map[string]bool{}, positions: map[string]float64{}}
+		text: map[int]string{}, st: model.NewState(), used: map[string]bool{}, noted: map[string]bool{}, positions: map[string]float64{}, zoneFrom: map[string]string{}}
 	var tables []nftTable
 	for _, item := range doc.Nftables {
 		if raw, ok := item["table"]; ok {
@@ -207,6 +215,7 @@ func Import(in Input) (*Result, error) {
 	im.importSets()
 	im.run()
 	im.nameForwards()
+	im.noteZones()
 	im.st.Renumber()
 	return &Result{State: im.st, Table: im.table.Family + " " + im.table.Name, Notes: im.notes, Raw: im.raw}, nil
 }
@@ -309,10 +318,13 @@ func key(vals []string) string {
 	return strings.Join(s, ",")
 }
 
-var ifaceRe = regexp.MustCompile(`^[A-Za-z][A-Za-z0-9_.@-]{0,14}\*?$`)
+var ifaceRe = regexp.MustCompile(`^[A-Za-z][A-Za-z0-9_@-]{0,14}\*?$`)
+
+// vlanRe is a VLAN sub-interface: enp2s0.20.
+var vlanRe = regexp.MustCompile(`^[A-Za-z][A-Za-z0-9_@-]*\.[0-9]{1,4}$`)
 
 func validIface(s string) bool {
-	return ifaceRe.MatchString(s) && !strings.ContainsAny(s, ".:") || s == "lo"
+	return len(s) <= 15 && (ifaceRe.MatchString(s) || vlanRe.MatchString(s))
 }
 
 // ---- sets
@@ -360,12 +372,22 @@ func (im *importer) importSets() {
 		for _, e := range s.Elem {
 			entries = append(entries, values(e)...)
 		}
-		if existing := im.st.Set(n); existing != nil {
+		name := setName(n)
+		if contains(s.Flags, "timeout") || contains(s.Flags, "dynamic") {
+			// Filled at run time (by a program, or a rule): a dynamic set.
+			// Its entries now are passing state, not configuration.
+			im.st.Sets = append(im.st.Sets, &model.AddressSet{ID: im.newID(), Name: name, Dynamic: true, Timeout: s.Timeout})
+			im.used[name] = true
+			im.note("set %s has entries that come and go (flags %s): it became a dynamic set, without its %d entries now; whatever fills it must now fill Housecarl's (hc set entries add %s ADDRESS, the API, or nft add element inet housecarl %s_v4 / %s_v6)",
+				n, strings.Join(s.Flags, ","), len(entries), name, name, name)
+			continue
+		}
+		if existing := im.st.Set(name); existing != nil {
 			existing.Entries = append(existing.Entries, entries...)
 			continue
 		}
-		im.st.Sets = append(im.st.Sets, &model.AddressSet{ID: im.newID(), Name: setName(n), Entries: entries})
-		im.used[n] = true
+		im.st.Sets = append(im.st.Sets, &model.AddressSet{ID: im.newID(), Name: name, Entries: entries})
+		im.used[name] = true
 	}
 }
 
@@ -397,7 +419,10 @@ func (im *importer) namedSet(entries []string) string {
 	return name
 }
 
-// zone returns the zone of an interface, making it on first use.
+// zone returns the zone of an interface, making it on first use. Its
+// name: the one given for it (Input.Zones); else the name of a define
+// holding the interface ("define lan_if = enp2s0": lan); else the
+// interface's own.
 func (im *importer) zone(iface string) string {
 	for _, z := range im.st.Zones {
 		for _, i := range z.Interfaces {
@@ -406,21 +431,78 @@ func (im *importer) zone(iface string) string {
 			}
 		}
 	}
-	name := iface
+	if given, ok := im.in.Zones[iface]; ok {
+		name := setName(given)
+		// Interfaces given one name share its zone.
+		if z := im.st.Zone(name); z != nil && im.zoneFrom[name] == "given" {
+			z.Interfaces = append(z.Interfaces, iface)
+			return name
+		}
+		if im.st.Zone(name) == nil {
+			return im.newZone(name, iface, "given")
+		}
+		im.note("--zone %s=%s: the zone %s is taken (by %s), so %s is named as below", iface, given, name, strings.Join(im.st.Zone(name).Interfaces, ", "), iface)
+	}
+	name, from := iface, "its interface's name"
 	if d, ok := im.ifNames[iface]; ok {
-		name = d
-		for _, suf := range []string{"_interface", "_iface", "_int", "_if", "_dev"} {
+		name, from = d, "the define "+d
+		for _, suf := range []string{"_interface", "_ifname", "_iface", "_device", "_int", "_nic", "_if", "_dev"} {
 			name = strings.TrimSuffix(name, suf)
 		}
 	}
 	name = setName(strings.TrimSuffix(name, "*"))
 	if im.st.Zone(name) != nil {
-		name = setName(iface + "_zone")
+		name, from = setName(iface+"_zone"), "its interface's name (the other was taken)"
 	}
+	return im.newZone(name, iface, from)
+}
+
+// zone2 is the zone an interface is in, or "" (making none).
+func (im *importer) zoneOfIface(iface string) string {
+	for _, z := range im.st.Zones {
+		if contains(z.Interfaces, iface) {
+			return z.Name
+		}
+	}
+	return ""
+}
+
+func (im *importer) newZone(name, iface, from string) string {
 	im.st.Zones = append(im.st.Zones, &model.Zone{ID: im.newID(), Name: name, Interfaces: []string{iface}})
+	im.zoneFrom[name] = from
 	return name
 }
 
+// noteZones says how each zone was named, and how to change it.
+func (im *importer) noteZones() {
+	ifaces := make([]string, 0, len(im.in.Zones))
+	for i := range im.in.Zones {
+		ifaces = append(ifaces, i)
+	}
+	sort.Strings(ifaces)
+	for _, i := range ifaces {
+		if im.zoneOfIface(i) == "" {
+			im.note("--zone %s=%s: no rule names the interface %s, so there is no zone for it", i, im.in.Zones[i], i)
+		}
+	}
+	if len(im.st.Zones) == 0 {
+		return
+	}
+	var parts []string
+	for _, z := range im.st.Zones {
+		from := im.zoneFrom[z.Name]
+		if from == "given" {
+			from = "as given"
+		}
+		ext := ""
+		if z.External {
+			ext = ", external"
+		}
+		parts = append(parts, fmt.Sprintf("%s (%s%s: %s)", z.Name, strings.Join(z.Interfaces, ", "), ext, from))
+	}
+	im.note("zones: %s; name them yourself with --zone INTERFACE=NAME (lan, iot, guest: what the network is for), or later with hc zone update NAME --name NEW", strings.Join(parts, "; "))
+}
+
 func (im *importer) external(iface string) {
 	if z := im.st.Zone(im.zone(iface)); z != nil && !z.External {
 		z.External = true
@@ -522,6 +604,11 @@ type parsed struct {
 	// name: a port forward's name, when the rule's comment won't do
 	// (one of a map's elements).
 	name string
+	// inJump: the rule came from a chain jumped to; jumpText is what the
+	// jumps to it match, as nftables text (none when it isn't known), put
+	// in front of the rule's own text if it stays raw.
+	inJump   bool
+	jumpText string
 }
 
 // mapUse is a lookup in a map: "dnat ip to tcp dport map @forwards" or
@@ -550,7 +637,11 @@ func parse(r nftRule) *parsed {
 	return p
 }
 
-func (p *parsed) bad(what string) { p.unsupported = append(p.unsupported, what) }
+func (p *parsed) bad(what string) {
+	if !contains(p.unsupported, what) {
+		p.unsupported = append(p.unsupported, what)
+	}
+}
 
 func (p *parsed) expr(k string, v json.RawMessage) {
 	switch k {
@@ -839,19 +930,24 @@ func (p *parsed) match(m match) {
 	if meta, ok := left["meta"]; ok {
 		var x struct{ Key string }
 		json.Unmarshal(meta, &x)
-		switch x.Key {
-		case "iifname":
+		switch {
+		case (x.Key == "iifname" || x.Key == "oifname") && anySet(right):
+			p.bad("meta " + x.Key + " in a named set")
+		case x.Key == "iifname" && len(p.inIf) > 0, x.Key == "oifname" && len(p.outIf) > 0:
+			// Both must hold; Housecarl's rule has one list: kept raw.
+			p.bad("two matches on meta " + x.Key)
+		case x.Key == "iifname":
 			p.inIf, p.notIn = right, not
-		case "oifname":
+		case x.Key == "oifname":
 			p.outIf, p.notOut = right, not
-		case "nfproto":
-			if len(right) == 1 && !not {
+		case x.Key == "nfproto":
+			if len(right) == 1 && !not && (p.family == "" || p.family == right[0]) {
 				p.family = right[0]
 			} else {
 				p.bad("meta nfproto")
 			}
-		case "l4proto":
-			if len(right) == 1 && !not {
+		case x.Key == "l4proto":
+			if len(right) == 1 && !not && (p.proto == "" || p.proto == right[0]) {
 				p.proto = right[0]
 			} else {
 				p.bad("meta l4proto with several protocols")
@@ -864,23 +960,29 @@ func (p *parsed) match(m match) {
 	if pl, ok := left["payload"]; ok {
 		var x struct{ Protocol, Field string }
 		json.Unmarshal(pl, &x)
+		fam := map[string]string{"ip": "ipv4", "ip6": "ipv6"}[x.Protocol]
 		switch {
+		// A second match on a field Housecarl keeps one list for (both
+		// must hold): the rule stays raw, as written.
+		case x.Field == "saddr" && len(p.src) > 0, x.Field == "daddr" && len(p.dst) > 0,
+			x.Field == "dport" && len(p.dports) > 0, x.Field == "type" && len(p.icmp) > 0:
+			p.bad("two matches on " + x.Protocol + " " + x.Field)
+		case fam != "" && p.family != "" && p.family != fam && (x.Field == "saddr" || x.Field == "daddr"):
+			p.bad("both IPv4 and IPv6 addresses")
 		case (x.Protocol == "ip" || x.Protocol == "ip6") && (x.Field == "saddr" || x.Field == "daddr"):
-			if p.family == "" {
-				p.family = map[string]string{"ip": "ipv4", "ip6": "ipv6"}[x.Protocol]
-			}
+			p.family = fam
 			if x.Field == "saddr" {
 				p.src, p.notSrc = right, not
 			} else {
 				p.dst, p.notDst = right, not
 			}
-		case (x.Protocol == "tcp" || x.Protocol == "udp" || x.Protocol == "sctp") && x.Field == "dport" && !not:
+		case (x.Protocol == "tcp" || x.Protocol == "udp" || x.Protocol == "sctp") && x.Field == "dport" && !not && (p.proto == "" || p.proto == x.Protocol):
 			p.proto, p.dports = x.Protocol, right
-		case x.Protocol == "tcp" && x.Field == "flags" && len(right) == 1 && right[0] == "syn":
+		case x.Protocol == "tcp" && x.Field == "flags" && len(right) == 1 && right[0] == "syn" && !not:
 			p.tcpSyn = true
 		case (x.Protocol == "icmp" || x.Protocol == "icmpv6") && x.Field == "type" && !not:
 			p.icmpProto, p.icmp = x.Protocol, right
-		case x.Protocol == "ip6" && x.Field == "hoplimit" && len(right) == 1 && right[0] == "255":
+		case x.Protocol == "ip6" && x.Field == "hoplimit" && len(right) == 1 && right[0] == "255" && !not:
 			p.hop255 = true
 		default:
 			p.bad(x.Protocol + " " + x.Field)
@@ -891,9 +993,9 @@ func (p *parsed) match(m match) {
 		var x struct{ Key string }
 		json.Unmarshal(ct, &x)
 		switch {
-		case x.Key == "state" && !not:
+		case x.Key == "state" && !not && len(p.ctState) == 0:
 			p.ctState = right
-		case x.Key == "status" && !not:
+		case x.Key == "status" && !not && len(p.ctStatus) == 0:
 			p.ctStatus = right
 		default:
 			p.bad("ct " + x.Key)
@@ -905,40 +1007,80 @@ func (p *parsed) match(m match) {
 	}
 }
 
+// anySet reports whether a match's values name a set ("@name").
+func anySet(vals []string) bool {
+	for _, v := range vals {
+		if strings.HasPrefix(v, "@") {
+			return true
+		}
+	}
+	return false
+}
+
 // ---- converting
 
 func (im *importer) run() {
 	type base struct {
 		name, kind string
+		prio       int
 	}
 	var bases []base
-	for name, c := range im.chains {
+	names := make([]string, 0, len(im.chains))
+	for name := range im.chains {
+		names = append(names, name)
+	}
+	sort.Strings(names)
+	policies := map[string][]string{}
+	for _, name := range names {
+		c := im.chains[name]
 		switch {
 		case c.Type == "filter" && c.Hook == "prerouting":
-			bases = append(bases, base{name, model.ChainEarly})
+			bases = append(bases, base{name, model.ChainEarly, c.Prio})
+			if c.Prio > -200 {
+				im.note("chain %s hooks prerouting at priority %d, after connection tracking; Housecarl's early chain runs before it (raw), where connection states aren't known yet: check its rules", name, c.Prio)
+			}
 		case c.Type == "filter" && (c.Hook == "input" || c.Hook == "forward" || c.Hook == "output"):
-			bases = append(bases, base{name, c.Hook})
+			bases = append(bases, base{name, c.Hook, c.Prio})
+			policies[c.Hook] = append(policies[c.Hook], c.Policy)
 		case c.Type == "nat" && c.Hook == "prerouting":
-			bases = append(bases, base{name, "dstnat"})
+			bases = append(bases, base{name, "dstnat", c.Prio})
 		case c.Type == "nat" && c.Hook == "postrouting":
-			bases = append(bases, base{name, "srcnat"})
+			bases = append(bases, base{name, "srcnat", c.Prio})
 		case c.Hook != "":
 			im.note("chain %s (%s %s) isn't imported: Housecarl doesn't manage that hook", name, c.Type, c.Hook)
 		}
-		set := &im.st.Settings
-		switch {
-		case c.Hook == "input" && c.Type == "filter":
-			set.InputPolicy = c.Policy
-		case c.Hook == "forward" && c.Type == "filter":
-			set.ForwardPolicy = c.Policy
-		case c.Hook == "output" && c.Type == "filter":
-			set.OutputPolicy = c.Policy
+	}
+	// Several chains on one hook: a packet must get past each (an accept
+	// ends only its own chain), and any policy drop drops it.
+	set := &im.st.Settings
+	for hook, ps := range policies {
+		policy := model.PolicyAccept
+		if contains(ps, model.PolicyDrop) {
+			policy = model.PolicyDrop
+		}
+		switch hook {
+		case "input":
+			set.InputPolicy = policy
+		case "forward":
+			set.ForwardPolicy = policy
+		case "output":
+			set.OutputPolicy = policy
 		}
 	}
 	// NAT first: the filter chains' port-forward accepts are folded into
-	// the port forwards.
+	// the port forwards. Chains on the same hook in priority order.
 	order := map[string]int{"dstnat": 0, "srcnat": 1, model.ChainEarly: 2, model.ChainInput: 3, model.ChainForward: 4, model.ChainOutput: 5}
-	sort.SliceStable(bases, func(i, j int) bool { return order[bases[i].kind] < order[bases[j].kind] })
+	sort.SliceStable(bases, func(i, j int) bool {
+		if bases[i].kind != bases[j].kind {
+			return order[bases[i].kind] < order[bases[j].kind]
+		}
+		return bases[i].prio < bases[j].prio
+	})
+	for i := 1; i < len(bases); i++ {
+		if b, a := bases[i], bases[i-1]; b.kind == a.kind && b.kind != "dstnat" && b.kind != "srcnat" {
+			im.note("chains %s and %s both hook %s: a packet must get past each, but Housecarl has one %s chain, so their rules follow each other (by priority); that only does the same if what %s accepts, %s would accept too: check it", a.name, b.name, b.kind, b.kind, a.name, b.name)
+		}
+	}
 	im.st.Settings.Builtins = model.Builtins{}
 	found := map[string]map[string]string{} // builtin → chain → "prefix", "later" or "demoted"
 	heads := map[string][]headRule{}        // chain → the built-ins its rules start with, in order
@@ -956,8 +1098,12 @@ func (im *importer) run() {
 				im.srcnat(p)
 			}
 		default:
-			chains = append(chains, b.kind)
-			prefix := true
+			if !contains(chains, b.kind) {
+				chains = append(chains, b.kind)
+			}
+			// Built-ins come first only at the head of the hook's first
+			// chain (or of one after a chain of nothing but built-ins).
+			prefix := len(rest[b.kind]) == 0
 			for _, p := range rules {
 				if bi := im.builtinOf(b.kind, p); bi != "" {
 					if found[bi] == nil {
@@ -1016,6 +1162,7 @@ func (im *importer) flatten(chain string, prefix *parsed, depth int) []*parsed {
 			}
 			jumpMatch := *p
 			jumpMatch.verdict, jumpMatch.target = "", ""
+			jumpMatch.jumpText = joinText(p.jumpText, im.condText(p))
 			out = append(out, im.flatten(p.target, &jumpMatch, depth+1)...)
 			if p.verdict == "goto" {
 				im.note("the goto to %s became the same rules followed by the rest of %s: check the order", p.target, chain)
@@ -1147,36 +1294,153 @@ func hasMatch(p, prefix *parsed) bool {
 		len(p.inIf) != len(prefix.inIf) || len(p.outIf) != len(prefix.outIf) || len(p.ctState) != len(prefix.ctState) || len(p.icmp) != len(prefix.icmp)
 }
 
-// merge puts a jump's matches in front of a jumped-to rule's. When both
-// match the same field, the rule's own (narrower, usually) is kept and
-// noted.
+// condText is a jump rule's text without its verdict, counter and
+// comment: what it matches ("ip saddr 10.50.50.0/24"); "" when there is
+// no text for it.
+var verdictRe = regexp.MustCompile(`\s*\b(jump|goto)\s+\S+\s*$`)
+
+func (im *importer) condText(p *parsed) string {
+	text, ok := im.text[p.handle]
+	if !ok {
+		return ""
+	}
+	if i := strings.LastIndex(text, ` comment "`); i >= 0 {
+		text = text[:i]
+	}
+	text = verdictRe.ReplaceAllString(strings.TrimSpace(text), "")
+	return strings.Join(strings.Fields(strings.ReplaceAll(" "+text+" ", " counter ", " ")), " ")
+}
+
+func joinText(a, b string) string {
+	return strings.TrimSpace(a + " " + b)
+}
+
+// merge puts a jump's matches in front of a jumped-to rule's: the rule
+// applies where both match. When both match the same field, that holds as
+// one list only when one's values lie within the other's (the narrower is
+// kept); otherwise the rule is kept raw, after the jump's text.
 func (im *importer) merge(prefix, p *parsed) *parsed {
 	q := *p
+	q.inJump, q.jumpText = true, prefix.jumpText
 	q.unsupported = append(append([]string{}, prefix.unsupported...), p.unsupported...)
-	fill := func(what string, dst *[]string, not *bool, src []string, srcNot bool) {
-		if len(*dst) == 0 && len(src) > 0 {
+	both := func(what string) { q.bad("both the jump to it and the rule match " + what) }
+	addrs := func(what string, dst *[]string, not *bool, src []string, srcNot bool) {
+		switch {
+		case len(src) == 0:
+		case len(*dst) == 0:
 			*dst, *not = src, srcNot
-		} else if len(*dst) > 0 && len(src) > 0 && key(*dst) != key(src) {
-			im.note("rule handle %d: both the jump to it and the rule match %s; the rule's own (%s) are kept, the jump's (%s) dropped: check it",
-				p.handle, what, strings.Join(*dst, ", "), strings.Join(src, ", "))
+		case *not || srcNot:
+			if key(*dst) != key(src) || *not != srcNot {
+				both(what)
+			}
+		case im.within(*dst, src):
+			// The rule's own are narrower: they say it all.
+		case im.within(src, *dst):
+			*dst = src
+		default:
+			both(what)
 		}
 	}
-	fill("interfaces in", &q.inIf, &q.notIn, prefix.inIf, prefix.notIn)
-	fill("interfaces out", &q.outIf, &q.notOut, prefix.outIf, prefix.notOut)
-	fill("sources", &q.src, &q.notSrc, prefix.src, prefix.notSrc)
-	fill("destinations", &q.dst, &q.notDst, prefix.dst, prefix.notDst)
-	if q.family == "" {
+	same := func(what string, dst *[]string, not *bool, src []string, srcNot bool) {
+		switch {
+		case len(src) == 0:
+		case len(*dst) == 0:
+			*dst, *not = src, srcNot
+		case key(*dst) != key(src) || *not != srcNot:
+			both(what)
+		}
+	}
+	var noNot bool
+	same("interfaces in", &q.inIf, &q.notIn, prefix.inIf, prefix.notIn)
+	same("interfaces out", &q.outIf, &q.notOut, prefix.outIf, prefix.notOut)
+	addrs("sources", &q.src, &q.notSrc, prefix.src, prefix.notSrc)
+	addrs("destinations", &q.dst, &q.notDst, prefix.dst, prefix.notDst)
+	same("connection states", &q.ctState, &noNot, prefix.ctState, false)
+	same("connection status", &q.ctStatus, &noNot, prefix.ctStatus, false)
+	switch {
+	case prefix.family == "":
+	case q.family == "":
 		q.family = prefix.family
+	case q.family != prefix.family:
+		both("an address family")
 	}
-	if len(q.ctState) == 0 {
-		q.ctState = prefix.ctState
-	}
-	if q.proto == "" {
+	switch {
+	case prefix.proto == "":
+	case q.proto == "":
 		q.proto, q.dports = prefix.proto, prefix.dports
+	case q.proto != prefix.proto:
+		both("a protocol")
+	default:
+		same("ports", &q.dports, &noNot, prefix.dports, false)
+	}
+	switch {
+	case prefix.icmpProto == "":
+	case q.icmpProto == "":
+		q.icmpProto, q.icmp = prefix.icmpProto, prefix.icmp
+	case q.icmpProto != prefix.icmpProto || key(q.icmp) != key(prefix.icmp):
+		both("ICMP types")
+	}
+	q.hop255 = q.hop255 || prefix.hop255
+	q.tcpSyn = q.tcpSyn || prefix.tcpSyn
+	if prefix.limit != nil || prefix.quota != nil || prefix.connLimit > 0 || prefix.log {
+		// Once in the jump, these count or log each packet once; on every
+		// rule they would act once per rule.
+		q.bad("a jump with a limit, quota or log")
 	}
 	return &q
 }
 
+// within reports whether every address of inner lies within outer's
+// (literal addresses and prefixes, or sets of the table).
+func (im *importer) within(inner, outer []string) bool {
+	resolve := func(list []string) ([]netip.Prefix, bool) {
+		var out []netip.Prefix
+		for _, e := range list {
+			vals := []string{e}
+			if strings.HasPrefix(e, "@") {
+				s, ok := im.sets[e[1:]]
+				if !ok {
+					return nil, false
+				}
+				vals = nil
+				for _, el := range s.Elem {
+					vals = append(vals, values(el)...)
+				}
+			}
+			for _, v := range vals {
+				pf, err := netip.ParsePrefix(v)
+				if err != nil {
+					a, err := netip.ParseAddr(v)
+					if err != nil {
+						return nil, false // a range, or something else
+					}
+					pf = netip.PrefixFrom(a, a.BitLen())
+				}
+				out = append(out, pf.Masked())
+			}
+		}
+		return out, len(out) > 0
+	}
+	in, ok1 := resolve(inner)
+	out, ok2 := resolve(outer)
+	if !ok1 || !ok2 {
+		return false
+	}
+	for _, a := range in {
+		found := false
+		for _, b := range out {
+			if b.Bits() <= a.Bits() && b.Contains(a.Addr()) {
+				found = true
+				break
+			}
+		}
+		if !found {
+			return false
+		}
+	}
+	return true
+}
+
 // ---- built-ins
 
 var essentials6 = map[string]bool{"destination-unreachable": true, "packet-too-big": true, "time-exceeded": true, "parameter-problem": true, "echo-request": true, "echo-reply": true}
@@ -1402,8 +1666,19 @@ func (im *importer) zones(ifaces []string) []string {
 	return out
 }
 
+// checkSets marks a rule naming a set that wasn't imported (not of
+// addresses, or a meter) unsupported: it stays raw.
+func (im *importer) checkSets(p *parsed) {
+	for _, e := range append(append([]string{}, p.src...), p.dst...) {
+		if n, ok := strings.CutPrefix(e, "@"); ok && (im.meters[n] || im.st.Set(setName(n)) == nil) {
+			p.bad("the set @" + n + ", which isn't an address set Housecarl imported")
+		}
+	}
+}
+
 // toMatch builds a Match; ok is false when something can't be said.
 func (im *importer) toMatch(p *parsed) (model.Match, bool) {
+	im.checkSets(p)
 	svcs, ok := im.services(p)
 	if !ok || len(p.unsupported) > 0 {
 		return model.Match{}, false
@@ -1441,6 +1716,10 @@ func (im *importer) rule(chain string, p *parsed) {
 	default:
 		p.bad(p.verdict)
 	}
+	if p.log && !im.noted["log"] {
+		im.noted["log"] = true
+		im.note("rules that log now log to Housecarl's log (hc log, Traffic → Log), not the kernel log or the journal")
+	}
 	m, ok := im.toMatch(p)
 	if r.CountsTraffic() && r.Action == model.ActionAccept {
 		// Housecarl checks these before established connections are
@@ -1471,6 +1750,14 @@ func (im *importer) rawRule(r *model.Rule, p *parsed) {
 	if i := strings.LastIndex(text, ` comment "`); i >= 0 {
 		text = strings.TrimSpace(text[:i])
 	}
+	// From a chain jumped to: only where the jumps matched.
+	if p.inJump {
+		if p.jumpText == "" {
+			im.note("rule handle %d of chain %s couldn't be expressed (%s), and the jump to it has no text to keep it raw with: add it by hand: %s", p.handle, r.Chain, strings.Join(p.unsupported, ", "), text)
+			return
+		}
+		text = p.jumpText + " " + text
+	}
 	// A named map isn't carried over: a raw rule naming it would make
 	// nftables refuse the whole ruleset.
 	for _, ref := range mapRefRe.FindAllStringSubmatch(text, -1) {
@@ -1480,6 +1767,12 @@ func (im *importer) rawRule(r *model.Rule, p *parsed) {
 			return
 		}
 	}
+	text, bad := im.setRefs(text)
+	if bad != "" {
+		im.note("rule handle %d of chain %s uses the set @%s, which Housecarl's table doesn't have (%s): left out, add it by hand: %s",
+			p.handle, r.Chain, bad, strings.Join(p.unsupported, ", "), text)
+		return
+	}
 	if r.Action == "" {
 		r.Action = model.ActionAccept
 	}
@@ -1491,41 +1784,86 @@ func (im *importer) rawRule(r *model.Rule, p *parsed) {
 		why = "a match Housecarl has no field for"
 	}
 	im.raw = append(im.raw, fmt.Sprintf("%s: %s (%s)", r.Chain, text, why))
-	if strings.Contains(text, "@") {
-		im.note("raw rule %q names a set of the old table: Housecarl's sets are NAME_v4 and NAME_v6", text)
-	}
 }
 
+var addrSetRefRe = regexp.MustCompile(`\b(ip6?) (saddr|daddr) (!= )?@([A-Za-z0-9_]+)`)
+
+// setRefs makes a raw rule's set references work in Housecarl's table:
+// an imported address set is NAME_v4 or NAME_v6 there, by the family the
+// rule matches; another set of the old table (interface names, ports) is
+// written out as an anonymous set of its elements. It returns the name of
+// a set it couldn't resolve (a meter, an empty or unknown one).
+func (im *importer) setRefs(text string) (string, string) {
+	text = addrSetRefRe.ReplaceAllStringFunc(text, func(m string) string {
+		sm := addrSetRefRe.FindStringSubmatch(m)
+		name := setName(sm[4])
+		if im.meters[sm[4]] || im.st.Set(name) == nil {
+			return m
+		}
+		return fmt.Sprintf("%s %s %s@%s", sm[1], sm[2], sm[3], render.SetName(name, sm[1] == "ip6"))
+	})
+	bad := ""
+	text = mapRefRe.ReplaceAllStringFunc(text, func(m string) string {
+		n := m[1:]
+		if strings.HasSuffix(n, "_v4") || strings.HasSuffix(n, "_v6") {
+			if im.st.Set(strings.TrimSuffix(strings.TrimSuffix(n, "_v4"), "_v6")) != nil {
+				return m // rewritten above
+			}
+		}
+		set, ok := im.sets[n]
+		if !ok || im.meters[n] || len(set.Elem) == 0 || contains(set.Flags, "timeout") || contains(set.Flags, "dynamic") {
+			bad = n
+			return m
+		}
+		var typ string
+		json.Unmarshal(set.Type, &typ)
+		var elems []string
+		for _, e := range set.Elem {
+			for _, v := range values(e) {
+				if typ == "ifname" || typ == "string" {
+					v = quote(v)
+				}
+				elems = append(elems, v)
+			}
+		}
+		return "{ " + strings.Join(elems, ", ") + " }"
+	})
+	return text, bad
+}
+
+func quote(s string) string { return `"` + strings.ReplaceAll(s, `"`, "") + `"` }
+
 // ---- NAT
 
 func (im *importer) dstnat(p *parsed) {
+	im.checkSets(p)
+	if a, err := netip.ParseAddr(p.dnatAddr); err == nil && a.Is6() {
+		p.bad("a DNAT to an IPv6 address (Housecarl's port forwards translate IPv4; IPv6 gets a pinhole)")
+	}
+	if p.notIn || len(p.outIf) > 0 || p.limit != nil || p.quota != nil || p.connLimit > 0 || p.log || len(p.ctState) > 0 && key(p.ctState) != "new" || len(p.icmp) > 0 {
+		p.bad("a DNAT with matches a port forward has no field for")
+	}
 	switch {
-	case p.dnatAddr != "" && len(p.unsupported) == 0 && p.proto != "" && len(p.dports) == 1 && !p.notDst && !p.notSrc:
-		ports, err := model.NormalizePorts(p.dports[0])
-		if err != nil {
+	case p.dnatAddr != "" && len(p.unsupported) == 0 && p.proto != "" && len(p.dports) > 0 && !p.notDst && !p.notSrc:
+		var ports []string
+		for _, d := range p.dports {
+			n, err := model.NormalizePorts(d)
+			if err != nil {
+				p.bad("the port " + d)
+				break
+			}
+			ports = append(ports, n)
+		}
+		if len(p.unsupported) > 0 {
 			break
 		}
-		f := &model.PortForward{Protocols: []string{p.proto}, Ports: ports, To: p.dnatAddr, ToPort: p.dnatPort, Comment: p.comment, Name: forwardName(p.comment)}
-		if p.name != "" {
-			f.Name = strings.ToLower(nameRe.ReplaceAllString(p.name, "-"))
+		// "tcp dport { 80, 443 } dnat to X": a forward for each port.
+		for _, port := range ports {
+			im.portForward(p, port, len(ports) > 1)
 		}
-		if f.ToPort == ports {
-			f.ToPort = ""
-		}
-		f.PublicAddresses = im.addrRefs(p.dst)
-		f.Sources = im.addrRefs(p.src)
 		for _, i := range p.inIf {
 			im.external(i)
 		}
-		// The same forward for another protocol: one forward, both.
-		for _, o := range im.st.Forwards {
-			if o.Ports == f.Ports && o.To == f.To && o.ToPort == f.ToPort && key(o.PublicAddresses) == key(f.PublicAddresses) && key(o.Sources) == key(f.Sources) {
-				o.Protocols = append(o.Protocols, p.proto)
-				return
-			}
-		}
-		f.ID, f.Position = im.newID(), float64(len(im.st.Forwards)+1)
-		im.st.Forwards = append(im.st.Forwards, f)
 		return
 	case p.redirect != "" && p.redirect != "?" && len(p.unsupported) == 0:
 		m, ok := im.toMatch(p)
@@ -1534,7 +1872,35 @@ func (im *importer) dstnat(p *parsed) {
 			return
 		}
 	}
-	im.note("the dstnat rule handle %d couldn't be imported (%s): %s", p.handle, strings.Join(p.unsupported, ", "), im.text[p.handle])
+	im.note("the dstnat rule handle %d couldn't be imported (%s): add it by hand: %s", p.handle, strings.Join(p.unsupported, ", "), im.text[p.handle])
+}
+
+// portForward makes the port forward for one public port of a DNAT rule
+// (several: the port goes into its name).
+func (im *importer) portForward(p *parsed, ports string, several bool) {
+	f := &model.PortForward{Protocols: []string{p.proto}, Ports: ports, To: p.dnatAddr, ToPort: p.dnatPort, Comment: p.comment, Name: forwardName(p.comment)}
+	if p.name != "" {
+		f.Name = strings.ToLower(nameRe.ReplaceAllString(p.name, "-"))
+	}
+	if several && f.Name != "" {
+		f.Name += "-" + ports
+	}
+	if f.ToPort == ports {
+		f.ToPort = ""
+	}
+	f.PublicAddresses = im.addrRefs(p.dst)
+	f.Sources = im.addrRefs(p.src)
+	// The same forward for another protocol: one forward, both.
+	for _, o := range im.st.Forwards {
+		if o.Ports == f.Ports && o.To == f.To && o.ToPort == f.ToPort && key(o.PublicAddresses) == key(f.PublicAddresses) && key(o.Sources) == key(f.Sources) {
+			if !contains(o.Protocols, p.proto) {
+				o.Protocols = append(o.Protocols, p.proto)
+			}
+			return
+		}
+	}
+	f.ID, f.Position = im.newID(), float64(len(im.st.Forwards)+1)
+	im.st.Forwards = append(im.st.Forwards, f)
 }
 
 var forwardWords = regexp.MustCompile(`(?i)^\s*port\s*forward(ing)?\s*[:-]?\s*`)
@@ -1570,6 +1936,7 @@ func (im *importer) nameForwards() {
 }
 
 func (im *importer) srcnat(p *parsed) {
+	im.checkSets(p)
 	if len(p.unsupported) > 0 {
 		im.note("the srcnat rule handle %d couldn't be imported (%s): %s", p.handle, strings.Join(p.unsupported, ", "), im.text[p.handle])
 		return
@@ -1618,7 +1985,8 @@ func (im *importer) srcnat(p *parsed) {
 // absorbForward folds a port forward's own forward-chain accept (and an
 // IPv6 pinhole on its port) into the port forward.
 func (im *importer) absorbForward(p *parsed) bool {
-	if p.verdict != "accept" || len(p.unsupported) > 0 || len(p.dst) != 1 || p.notDst || len(p.dports) != 1 || p.proto == "" || len(p.src) > 0 || len(p.outIf) > 0 {
+	if p.verdict != "accept" || len(p.unsupported) > 0 || len(p.dst) != 1 || p.notDst || len(p.dports) != 1 || p.proto == "" || len(p.src) > 0 || len(p.outIf) > 0 ||
+		p.notIn || p.limit != nil || p.quota != nil || p.connLimit > 0 || p.log || len(p.ctState) > 0 && key(p.ctState) != "new" {
 		return false
 	}
 	port, err := model.NormalizePorts(p.dports[0])
@@ -1627,6 +1995,12 @@ func (im *importer) absorbForward(p *parsed) bool {
 	}
 	dst := p.dst[0]
 	if p.family == "ipv4" || !strings.Contains(dst, ":") {
+		// The forward's own accept takes only what was port forwarded: an
+		// accept of anything to that host and port stays a rule (inside
+		// clients reach the host directly through it).
+		if key(p.ctStatus) != "dnat" {
+			return false
+		}
 		for _, f := range im.st.Forwards {
 			inside := f.ToPort
 			if inside == "" {
@@ -1653,7 +2027,8 @@ func (im *importer) absorbForward(p *parsed) bool {
 			cands = append(cands, f)
 		}
 	}
-	if len(cands) == 1 && len(p.ctStatus) == 0 {
+	// The pinhole lets in from everywhere: only an accept that did too.
+	if len(cands) == 1 && len(p.ctStatus) == 0 && len(p.inIf) == 0 && len(p.ctState) == 0 {
 		cands[0].To6 = dst
 		im.note("the IPv6 rule to %s port %s became %s's IPv6 pinhole", dst, port, cands[0].Label())
 		return true

internal/importer/importer_test.go

@@ -115,9 +115,16 @@ func TestImportRouter(t *testing.T) {
 	if !wgDNS {
 		t.Errorf("wg0_input not inlined")
 	}
-	// The jump's and the rule's sources both: the rule's are kept, noted.
-	if !strings.Contains(strings.Join(res.Notes, "\n"), "the rule's own (10.50.50.3, 10.50.50.6) are kept") {
-		t.Errorf("no note on the overlapping sources: %q", res.Notes)
+	// The jump's and the rule's sources both: the rule's lie within the
+	// jump's, so they say it all, without a note.
+	var wgSSH bool
+	for _, r := range st.Rules {
+		if r.Chain == model.ChainInput && strings.Join(r.Services, ",") == "tcp/22" && strings.Join(r.Sources, ",") == "10.50.50.3,10.50.50.6" {
+			wgSSH = true
+		}
+	}
+	if !wgSSH || strings.Contains(strings.Join(res.Notes, "\n"), "jump") {
+		t.Errorf("the WireGuard SSH rule: found %v, notes %q", wgSSH, res.Notes)
 	}
 	// The imported configuration is valid, and renders.
 	if ps := validate.Check(st, nil); validate.HasErrors(ps) {

internal/importer/testdata/audit.json

@@ -0,0 +1 @@
+{"nftables": [{"metainfo": {"version": "1.1.3", "release_name": "Commodore Bullmoose #4", "json_schema_version": 1}}, {"table": {"family": "inet", "name": "audit", "handle": 1}}, {"chain": {"family": "inet", "table": "audit", "name": "early", "handle": 1, "type": "filter", "hook": "input", "prio": -10, "policy": "accept"}}, {"chain": {"family": "inet", "table": "audit", "name": "input", "handle": 2, "type": "filter", "hook": "input", "prio": 0, "policy": "drop"}}, {"chain": {"family": "inet", "table": "audit", "name": "admin", "handle": 3}}, {"chain": {"family": "inet", "table": "audit", "name": "forward", "handle": 4, "type": "filter", "hook": "forward", "prio": 0, "policy": "drop"}}, {"chain": {"family": "inet", "table": "audit", "name": "prerouting", "handle": 5, "type": "nat", "hook": "prerouting", "prio": -100, "policy": "accept"}}, {"set": {"family": "inet", "name": "blocklist", "table": "audit", "type": "ipv4_addr", "handle": 6, "flags": ["timeout"], "timeout": 3600}}, {"set": {"family": "inet", "name": "lans", "table": "audit", "type": "ifname", "handle": 7, "elem": ["enp2s0", "enp3s0"]}}, {"rule": {"family": "inet", "table": "audit", "chain": "early", "handle": 8, "comment": "an early input chain", "expr": [{"match": {"op": "==", "left": {"payload": {"protocol": "ip", "field": "saddr"}}, "right": "192.0.2.66"}}, {"drop": null}]}}, {"rule": {"family": "inet", "table": "audit", "chain": "input", "handle": 9, "expr": [{"match": {"op": "in", "left": {"ct": {"key": "state"}}, "right": ["established", "related"]}}, {"accept": null}]}}, {"rule": {"family": "inet", "table": "audit", "chain": "input", "handle": 10, "comment": "dynamic", "expr": [{"match": {"op": "==", "left": {"payload": {"protocol": "ip", "field": "saddr"}}, "right": "@blocklist"}}, {"drop": null}]}}, {"rule": {"family": "inet", "table": "audit", "chain": "input", "handle": 11, "comment": "an interface set", "expr": [{"match": {"op": "==", "left": {"meta": {"key": "iifname"}}, "right": "@lans"}}, {"accept": null}]}}, {"rule": {"family": "inet", "table": "audit", "chain": "input", "handle": 12, "comment": "two source matches", "expr": [{"match": {"op": "==", "left": {"payload": {"protocol": "ip", "field": "saddr"}}, "right": {"prefix": {"addr": "10.0.0.0", "len": 8}}}}, {"match": {"op": "!=", "left": {"payload": {"protocol": "ip", "field": "saddr"}}, "right": {"prefix": {"addr": "10.0.10.0", "len": 24}}}}, {"match": {"op": "==", "left": {"payload": {"protocol": "tcp", "field": "dport"}}, "right": 22}}, {"drop": null}]}}, {"rule": {"family": "inet", "table": "audit", "chain": "input", "handle": 13, "comment": "hop limit not 255", "expr": [{"match": {"op": "==", "left": {"payload": {"protocol": "icmpv6", "field": "type"}}, "right": "nd-neighbor-solicit"}}, {"match": {"op": "!=", "left": {"payload": {"protocol": "ip6", "field": "hoplimit"}}, "right": 255}}, {"drop": null}]}}, {"rule": {"family": "inet", "table": "audit", "chain": "input", "handle": 14, "comment": "DNS from the IoT VLAN", "expr": [{"match": {"op": "==", "left": {"meta": {"key": "iifname"}}, "right": "enp2s0.20"}}, {"match": {"op": "==", "left": {"payload": {"protocol": "udp", "field": "dport"}}, "right": 53}}, {"accept": null}]}}, {"rule": {"family": "inet", "table": "audit", "chain": "input", "handle": 15, "expr": [{"match": {"op": "==", "left": {"payload": {"protocol": "ip", "field": "saddr"}}, "right": {"prefix": {"addr": "10.9.0.0", "len": 24}}}}, {"jump": {"target": "admin"}}]}}, {"rule": {"family": "inet", "table": "audit", "chain": "admin", "handle": 16, "comment": "within the jump", "expr": [{"match": {"op": "==", "left": {"payload": {"protocol": "ip", "field": "saddr"}}, "right": "10.9.0.5"}}, {"match": {"op": "==", "left": {"payload": {"protocol": "tcp", "field": "dport"}}, "right": 22}}, {"accept": null}]}}, {"rule": {"family": "inet", "table": "audit", "chain": "admin", "handle": 17, "comment": "outside the jump", "expr": [{"match": {"op": "==", "left": {"payload": {"protocol": "ip", "field": "saddr"}}, "right": "10.8.0.1"}}, {"match": {"op": "==", "left": {"payload": {"protocol": "tcp", "field": "dport"}}, "right": 23}}, {"accept": null}]}}, {"rule": {"family": "inet", "table": "audit", "chain": "admin", "handle": 18, "comment": "raw under the jump", "expr": [{"match": {"op": "==", "left": {"meta": {"key": "mark"}}, "right": 1}}, {"accept": null}]}}, {"rule": {"family": "inet", "table": "audit", "chain": "forward", "handle": 19, "expr": [{"match": {"op": "in", "left": {"ct": {"key": "state"}}, "right": ["established", "related"]}}, {"accept": null}]}}, {"rule": {"family": "inet", "table": "audit", "chain": "forward", "handle": 20, "comment": "not only forwarded", "expr": [{"match": {"op": "==", "left": {"payload": {"protocol": "ip", "field": "daddr"}}, "right": "10.0.10.37"}}, {"match": {"op": "==", "left": {"payload": {"protocol": "tcp", "field": "dport"}}, "right": 443}}, {"accept": null}]}}, {"rule": {"family": "inet", "table": "audit", "chain": "forward", "hanLine truncated

internal/importer/testdata/audit.nft

@@ -0,0 +1,53 @@
+#! /sbin/nft -f
+# Cases the importer must not get wrong (importer_test.go TestImportAudit).
+
+define wan_if = "wan"
+define lan_if = "enp2s0"
+define iot_if = "enp2s0.20"
+define admins = { 10.9.0.0/24 }
+
+table inet audit {
+	set blocklist {
+		type ipv4_addr
+		flags timeout
+		timeout 1h
+	}
+	set lans {
+		type ifname
+		elements = { "enp2s0", "enp3s0" }
+	}
+
+	chain early {
+		type filter hook input priority -10; policy accept;
+		ip saddr 192.0.2.66 drop comment "an early input chain"
+	}
+
+	chain input {
+		type filter hook input priority filter; policy drop;
+		ct state established,related accept
+		ip saddr @blocklist drop comment "dynamic"
+		iifname @lans accept comment "an interface set"
+		ip saddr 10.0.0.0/8 ip saddr != 10.0.10.0/24 tcp dport 22 drop comment "two source matches"
+		icmpv6 type nd-neighbor-solicit ip6 hoplimit != 255 drop comment "hop limit not 255"
+		iifname $iot_if udp dport 53 accept comment "DNS from the IoT VLAN"
+		ip saddr $admins jump admin
+	}
+
+	chain admin {
+		ip saddr 10.9.0.5 tcp dport 22 accept comment "within the jump"
+		ip saddr 10.8.0.1 tcp dport 23 accept comment "outside the jump"
+		meta mark 0x00000001 accept comment "raw under the jump"
+	}
+
+	chain forward {
+		type filter hook forward priority filter; policy drop;
+		ct state established,related accept
+		ip daddr 10.0.10.37 tcp dport 443 accept comment "not only forwarded"
+		iifname $lan_if oifname $wan_if accept
+	}
+
+	chain prerouting {
+		type nat hook prerouting priority dstnat; policy accept;
+		ip daddr 203.0.113.1 tcp dport { 80, 443 } dnat ip to 10.0.10.37 comment "web"
+	}
+}

internal/importer/testdata/audit.txt

@@ -0,0 +1,46 @@
+table inet audit { # handle 1
+	set blocklist { # handle 6
+		type ipv4_addr
+		timeout 1h
+	}
+
+	set lans { # handle 7
+		type ifname
+		elements = { "enp2s0",
+			     "enp3s0" }
+	}
+
+	chain early { # handle 1
+		type filter hook input priority filter - 10; policy accept;
+		ip saddr 192.0.2.66 drop comment "an early input chain" # handle 8
+	}
+
+	chain input { # handle 2
+		type filter hook input priority filter; policy drop;
+		ct state established,related accept # handle 9
+		ip saddr @blocklist drop comment "dynamic" # handle 10
+		iifname @lans accept comment "an interface set" # handle 11
+		ip saddr 10.0.0.0/8 ip saddr != 10.0.10.0/24 tcp dport 22 drop comment "two source matches" # handle 12
+		icmpv6 type nd-neighbor-solicit ip6 hoplimit != 255 drop comment "hop limit not 255" # handle 13
+		iifname "enp2s0.20" udp dport 53 accept comment "DNS from the IoT VLAN" # handle 14
+		ip saddr 10.9.0.0/24 jump admin # handle 15
+	}
+
+	chain admin { # handle 3
+		ip saddr 10.9.0.5 tcp dport 22 accept comment "within the jump" # handle 16
+		ip saddr 10.8.0.1 tcp dport 23 accept comment "outside the jump" # handle 17
+		meta mark 0x00000001 accept comment "raw under the jump" # handle 18
+	}
+
+	chain forward { # handle 4
+		type filter hook forward priority filter; policy drop;
+		ct state established,related accept # handle 19
+		ip daddr 10.0.10.37 tcp dport 443 accept comment "not only forwarded" # handle 20
+		iifname "enp2s0" oifname "wan" accept # handle 21
+	}
+
+	chain prerouting { # handle 5
+		type nat hook prerouting priority dstnat; policy accept;
+		ip daddr 203.0.113.1 tcp dport { 80, 443 } dnat ip to 10.0.10.37 comment "web" # handle 23
+	}
+}

internal/server/h_import.go

@@ -25,7 +25,7 @@ func (s *Server) importNft(w http.ResponseWriter, r *http.Request, p *auth.Princ
 	if err := decodeJSON(r, &in); err != nil {
 		return err
 	}
-	res, err := importer.Import(importer.Input{JSON: in.JSON, Text: in.Text, Defines: in.Defines, Table: in.Table})
+	res, err := importer.Import(importer.Input{JSON: in.JSON, Text: in.Text, Defines: in.Defines, Table: in.Table, Zones: in.Zones})
 	if err != nil {
 		return badRequest("%v", err)
 	}

internal/web/static/assets/pages/import.js

@@ -8,14 +8,21 @@ export async function render(el) {
   const text = textarea("", { rows: 4, class: "mono", placeholder: "optional, the output of: sudo nft -a list ruleset (keeps what Housecarl can't express as raw rules)" });
   const defines = textarea("", { rows: 4, class: "mono", placeholder: "optional, your /etc/nftables.conf: its defines name the sets" });
   const table = input({ placeholder: "inet main (when there are several)", class: "mono" });
+  const zones = input({ placeholder: "enp2s0=lan, wg0=vpn (optional)", class: "mono" });
   const replace = checkbox("replace Housecarl's configuration", false);
   const out = h("div");
   const run = async (dry) => {
     mount(out);
+    const zoneNames = {};
+    for (const part of zones.value.split(",").map((s) => s.trim()).filter(Boolean)) {
+      const [iface, name] = part.split("=").map((s) => (s || "").trim());
+      if (!iface || !name) { mount(out, h("div", { class: "msg err" }, "Zone names: write each as interface=name (enp2s0=lan).")); return; }
+      zoneNames[iface] = name;
+    }
     let parsed;
     try { parsed = JSON.parse(json.value); } catch { mount(out, h("div", { class: "msg err" }, "The ruleset isn't JSON: paste the output of nft -j list ruleset.")); return; }
     try {
-      const r = await api("POST", "/import/nft", { body: { json: parsed, text: text.value, defines: defines.value, table: table.value.trim(), replace: replace.input.checked },
+      const r = await api("POST", "/import/nft", { body: { json: parsed, text: text.value, defines: defines.value, table: table.value.trim(), zones: zoneNames, replace: replace.input.checked },
         query: dry ? { dry_run: 1 } : undefined });
       const st = r.state;
       mount(out, h("div", { class: "msg " + (dry ? "info" : "ok") },
@@ -34,7 +41,8 @@ export async function render(el) {
     h("div", { class: "page-head" }, h("div", { class: "grow" }, h("h1", null, "Import a ruleset"),
       h("div", { class: "muted" }, "Turns an nftables ruleset into zones, sets, services, rules, port forwards and NAT. On the firewall, 'sudo hc import --defines /etc/nftables.conf -n' does the same from the command line."))),
     h("div", { class: "panel" }, h("div", { class: "form-grid" },
-      field("Ruleset (JSON)", json), field("Ruleset (text)", text), field("Ruleset file", defines), field("Table", table), h("label"), replace),
+      field("Ruleset (JSON)", json), field("Ruleset (text)", text), field("Ruleset file", defines), field("Table", table),
+      field("Zone names", zones, "Name zones by what the network is for; interfaces given one name share a zone. Otherwise a define holding the interface names it, or the interface's own name."), h("label"), replace),
       h("div", { class: "actions form-actions" },
         h("button", { onclick: () => run(true) }, "Preview"),
         h("button", { class: "primary", onclick: () => run(false) }, "Import"))),