diff --git a/CHANGELOG.md b/CHANGELOG.md index a0a16aa..edb1d93 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,45 @@ All notable changes to Housecarl are documented here. The format follows ## [Unreleased] +### Added + +- **Zone names at import:** `hc import --zone enp2s0=lan` (repeat; + interfaces given one name share a zone; Zone names on the Import + page), and a note listing each zone and where its name came from. + Defines holding a VLAN sub-interface (`enp2s0.20`) name their zone, + and `_ifname`, `_device` and `_nic` are taken off define names too + (docs/import.md, "How things are named"). +- Import: sets with `flags timeout` become dynamic sets; a DNAT of + several ports becomes one port forward per port. + +### Fixed + +- Import: two base chains on one hook had their rules imported twice, + and the second chain's leading built-ins were moved above the first + chain's rules; they now follow each other by priority (read in a + fixed order), with the stricter policy, and a note. +- Import: a rule inside a jumped-to chain that stays raw kept only its + own text and so applied to everything; the jump's condition now goes + in front. A jump and a rule matching the same field are combined only + when one's addresses lie within the other's; else the rule stays raw. + A jump's ports, ICMP types, connection status and hop limit were + dropped; a limit or log on the jump is no longer silently lost. +- Import: a second match on the same field (`ip saddr A ip saddr != B`) + replaced the first; `!=` on the hop limit and TCP flags was read as + `==`. Such rules now stay raw, as written. +- Import: a forward-chain accept to a forwarded host and port without + `ct status dnat` was folded into the port forward, which only lets + forwarded connections through; it stays a rule now. An IPv6 accept + limited to an interface no longer becomes a pinhole open to all. +- Import: raw rules naming a set of the old table made nftables refuse + the ruleset; address sets are now renamed `NAME_v4`/`NAME_v6`, other + sets written out with their elements, and what can't be is left out + with a note. Rules naming a set that isn't imported stay raw instead + of failing validation. +- Import: a DNAT to an IPv6 address, or with matches a port forward + can't hold (an interface excluded, a limit, a log), is left out with a + note instead of being imported without them. + ## [0.1.0] - 2026-10-19 The first version of Housecarl, a firewall manager for nftables, built diff --git a/cmd/hc/firewall.go b/cmd/hc/firewall.go index 52f4452..260444c 100644 --- a/cmd/hc/firewall.go +++ b/cmd/hc/firewall.go @@ -434,6 +434,7 @@ func rollbackCmd() *cobra.Command { func importCmd() *cobra.Command { var replace bool var table, defines, textFile string + var zones []string cmd := &cobra.Command{ Use: "import [FILE|-]", Short: "Import an nftables ruleset (nft -j JSON) as Housecarl's configuration", @@ -445,7 +446,9 @@ forwards and NAT. The input is nft's JSON; on the firewall: Without a FILE, hc runs 'nft -j list ruleset' and 'nft -a list ruleset' itself (root on the firewall); from elsewhere, give the JSON as FILE and the text with --text. --defines reads the ruleset file's define lines, to -name the sets as you did (admins, lan_net, …). -n shows the result without +name the sets as you did (admins, lan_net, …), and the zones; --zone +enp2s0=lan names one yourself (repeat; interfaces given one name share a +zone). -n shows the result without saving; what can't be expressed becomes raw rules, listed with notes. Once it looks right, import without -n, check it with 'hc ruleset', and 'hc takeover inet main' deletes the old table.`, @@ -473,7 +476,14 @@ Once it looks right, import without -n, check it with 'hc ruleset', and return err } } - in := api.ImportInput{JSON: data, Text: string(text), Table: table, Replace: replace} + in := api.ImportInput{JSON: data, Text: string(text), Table: table, Replace: replace, Zones: map[string]string{}} + for _, z := range zones { + iface, name, ok := strings.Cut(z, "=") + if !ok || iface == "" || name == "" { + return fmt.Errorf("--zone %q: write it as INTERFACE=NAME (enp2s0=lan)", z) + } + in.Zones[iface] = name + } if defines != "" { b, err := os.ReadFile(defines) if err != nil { @@ -508,6 +518,7 @@ Once it looks right, import without -n, check it with 'hc ruleset', and cmd.Flags().StringVar(&table, "table", "", "import only this table (\"inet main\"; default: all but Housecarl's)") cmd.Flags().StringVar(&defines, "defines", "", "the ruleset file, whose defines name the sets") cmd.Flags().StringVar(&textFile, "text", "", "'nft -a list ruleset' output, to keep what can't be expressed as raw rules") + cmd.Flags().StringArrayVar(&zones, "zone", nil, "name an interface's zone: INTERFACE=NAME (repeat)") return cmd } diff --git a/docs/api.md b/docs/api.md index 0dc52f0..b2b887e 100644 --- a/docs/api.md +++ b/docs/api.md @@ -210,7 +210,7 @@ history, but are audited. | `GET /history[?limit=&before=]` | changes, newest first | | `GET /history/{id}` | one, with each object before and after | | `POST /history/{id}/revert` | undo it; 409 if an object it touched has changed since, or it was undone already | -| `POST /import/nft` | `{"json": NFT_JSON, "text": "nft -a list ruleset output", "defines": "ruleset file", "table": "inet main", "replace": false}` ([import.md](import.md)); answers with `state` (the imported configuration), `notes` and `raw` too | +| `POST /import/nft` | `{"json": NFT_JSON, "text": "nft -a list ruleset output", "defines": "ruleset file", "table": "inet main", "zones": {"enp2s0": "lan"}, "replace": false}` ([import.md](import.md)); answers with `state` (the imported configuration), `notes` and `raw` too | | `POST /takeover` | `{"tables": ["inet main"]}`: delete these tables (administrators); on probation over the network | ### Certificate diff --git a/docs/cli.md b/docs/cli.md index 6b02443..cbb240e 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -219,7 +219,7 @@ See [nat.md](nat.md). | `history list [--limit N]` | changes, newest first | | `history show ID` | each object before and after | | `history undo ID` | apply the change backwards (refused if something it touched has changed since) | -| `import [FILE\|-] [--text FILE] [--defines FILE] [--table T] [--replace]` | [import.md](import.md) | +| `import [FILE\|-] [--text FILE] [--defines FILE] [--table T] [--zone IFACE=NAME]... [--replace]` | [import.md](import.md) | | `takeover TABLE... [-y]` | delete tables Housecarl's ruleset replaces (administrators) | ### cert diff --git a/docs/import.md b/docs/import.md index dde2772..19e8d50 100644 --- a/docs/import.md +++ b/docs/import.md @@ -18,11 +18,9 @@ shows what it would make, without saving anything: zones, sets, services, rules, port forwards and NAT, then **notes** on every guess it made, and the rules it **kept raw**. -`--defines` reads your ruleset file's `define` lines, so sets get your -names (`define admins = { ... }` becomes the set `admins`). -Without it, sets are named after what they hold. Interfaces named by a -define name their zone too (`define wan_interface = "ppp0"` makes the -zone `wan`); otherwise a zone is named after its interface. +`--defines` reads your ruleset file's `define` lines, so sets and zones +get your names ([How things are named](#how-things-are-named)). +`--zone enp2s0=lan` names a zone yourself. From another machine, give the JSON as a file, and the text with `--text`: @@ -37,6 +35,46 @@ With several tables, `--table "inet main"` picks one; by default every table but Housecarl's is read. The web UI has the same under Server → Import a ruleset. +## How things are named + +**Zones** are named in this order: + +1. **As you say**: `--zone enp2s0=lan` (repeat it; Zone names on the + web UI's Import page). Interfaces given the same name share one zone + (`--zone enp2s0=lan --zone enp3s0=lan`). +2. **After a define holding the interface**, with a trailing `_int`, + `_if`, `_iface`, `_ifname`, `_interface`, `_dev`, `_device` or `_nic` + taken off: `define wan_int = "ppp0"` makes the zone `wan`, `define + iot_if = enp2s0.20` (a VLAN) the zone `iot`. +3. **After the interface itself**: `enp2s0`. + +If the name is taken, the zone is called `INTERFACE_zone`. A define +names what it was *for* in your old ruleset, which isn't always what the +network is (`define dhcp_int = enp2s0` makes `dhcp`, for your LAN). A +note lists each zone and where its name came from; rename one with +`--zone` and import again, or afterwards with `hc zone update dhcp +--name lan` (its rules follow). Name zones for what the network is for: +`lan`, `iot`, `guest`, `servers`, `vpn`, `wan`. + +**Address sets**: a named set of the table keeps its name; a define +holding a list of addresses, or one prefix, gives its name to a set +(`define trusted = { 10.0.10.0/24, 10.1.1.0/24 }` makes `trusted`, +wherever those addresses appear together). A single address stays an +address. When two defines hold the same addresses, the first by name +wins. + +**Services**: a define holding several ports names a service (`define +web_ports = { 80, 443 }` makes `web_ports`); one port stays a port +(`tcp/22`). + +**Port forwards** are named after their comment, without a leading +"Port forward" (`comment "Port Forward Jellyfin"` makes `jellyfin`); +several ports in one DNAT become one forward each (`web-80`, `web-443`), +and a DNAT map's elements are named after the map (`forwards-443`). + +Names that aren't valid are made so: other characters become `_` (`-` +in forward names), and a name must start with a letter. + ## What becomes what * **Interfaces** become zones. Zones are marked external when the rules @@ -45,7 +83,11 @@ Import a ruleset. * **Sets and defines** of addresses become address sets; port lists become services. * **Jumps and gotos** to your own chains are flattened: the jump's - condition is added to each rule of the chain, in place. + condition is added to each rule of the chain, in place. When the jump + and a rule both match sources (or destinations), the narrower is kept + if one lies within the other; otherwise, and for any rule there that + stays raw, the jump's condition is written in front of the raw rule's + text, so it still only applies where the jump did. * **Rules** keep their order. A rule that is a built-in (loopback, established, invalid, the ICMPv6 rules, MSS clamping, the DHCPv6 client rule, antispoofing) becomes that built-in only when it sits where the @@ -56,10 +98,15 @@ Import a ruleset. stays a rule keeps the ICMPv6 accepts after it behind it, as they were. Built-ins your ruleset doesn't have are turned off. * **DNAT** rules become port forwards, TCP and UDP of the same port - merged into one. The forward-chain accepts that let them through - become part of the port forward, and a masquerade for inside sources - to the same host becomes its hairpin. An IPv6 accept to the same port - becomes the forward's IPv6 pinhole when there is only one candidate. + merged into one, one per port when a rule has several. The + forward-chain accepts that let them through (`ct status dnat`) become + part of the port forward; an accept to the same host and port without + `ct status dnat` lets in more than forwarded connections, and stays a + rule. A masquerade for inside sources to the same host becomes its + hairpin. An IPv6 accept to the same port, from everywhere, becomes the + forward's IPv6 pinhole when there is only one candidate. A DNAT to an + IPv6 address, or with matches a port forward has no field for, is left + out with a note. * **Maps are expanded**, one element at a time. A DNAT map (`dnat ip to tcp dport map @forwards`, named or written in the rule) becomes one port forward per element; forwards from a named map are @@ -78,13 +125,27 @@ Import a ruleset. Byte rates and quotas are imported too; a quota imported resets only by hand until you give it a period. A rule with a quota or a byte rate that accepts stays a raw rule ([rules.md](rules.md#bandwidth-and-quotas)). +* **Sets with `flags timeout`** (filled by a program or a rule) become + dynamic sets, without their entries of the moment: whatever fills them + must fill Housecarl's from then on, as a note says. * **Policies** become the chains' policies. -* **Anything else** becomes a raw rule with its original text, in place. - A raw rule naming a set of the old table needs the new set name: - Housecarl's are `NAME_v4` and `NAME_v6`, and a note says so. +* **Anything else** becomes a raw rule with its original text, in place: + a match Housecarl has no field for, two matches on the same field + (`ip saddr 10.0.0.0/8 ip saddr != 10.0.10.0/24`), `!=` where Housecarl + only has `==`, a set of interface names. Its set references are made + to work in Housecarl's table: an imported address set becomes + `@NAME_v4` or `@NAME_v6`, any other set (interface names, ports) is + written out with its elements. A raw rule naming a set that can't be + (a meter, an empty set) is left out with a note. +* **Logging rules** log to Housecarl's log (`hc log`), not the kernel's. -Chains on hooks Housecarl doesn't manage (ingress, a bridge table) are -left out, with a note. +Several base chains on one hook (`input` at priority -10 and at 0) are +put one after the other, by priority, into Housecarl's one chain, with +the stricter policy; a note says to check it, since a packet had to get +past each, and an accept in the first no longer ends there. A prerouting +filter chain after connection tracking becomes the early chain, which +runs before it: a note says so. Chains on hooks Housecarl doesn't manage +(ingress, a bridge table) are left out, with a note. ## 2. Import diff --git a/internal/api/types.go b/internal/api/types.go index a2b1f3d..a11cf98 100644 --- a/internal/api/types.go +++ b/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. diff --git a/internal/importer/audit_test.go b/internal/importer/audit_test.go new file mode 100644 index 0000000..f40c523 --- /dev/null +++ b/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) + } + } +} diff --git a/internal/importer/importer.go b/internal/importer/importer.go index c18918e..599202f 100644 --- a/internal/importer/importer.go +++ b/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 diff --git a/internal/importer/importer_test.go b/internal/importer/importer_test.go index 815393d..6a804d9 100644 --- a/internal/importer/importer_test.go +++ b/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) { diff --git a/internal/importer/testdata/audit.json b/internal/importer/testdata/audit.json new file mode 100644 index 0000000..607cfa2 --- /dev/null +++ b/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", "handle": 21, "expr": [{"match": {"op": "==", "left": {"meta": {"key": "iifname"}}, "right": "enp2s0"}}, {"match": {"op": "==", "left": {"meta": {"key": "oifname"}}, "right": "wan"}}, {"accept": null}]}}, {"rule": {"family": "inet", "table": "audit", "chain": "prerouting", "handle": 23, "comment": "web", "expr": [{"match": {"op": "==", "left": {"payload": {"protocol": "ip", "field": "daddr"}}, "right": "203.0.113.1"}}, {"match": {"op": "==", "left": {"payload": {"protocol": "tcp", "field": "dport"}}, "right": {"set": [80, 443]}}}, {"dnat": {"family": "ip", "addr": "10.0.10.37"}}]}}]} diff --git a/internal/importer/testdata/audit.nft b/internal/importer/testdata/audit.nft new file mode 100644 index 0000000..147d491 --- /dev/null +++ b/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" + } +} diff --git a/internal/importer/testdata/audit.txt b/internal/importer/testdata/audit.txt new file mode 100644 index 0000000..20e37a9 --- /dev/null +++ b/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 + } +} diff --git a/internal/server/h_import.go b/internal/server/h_import.go index 620d10b..f7facee 100644 --- a/internal/server/h_import.go +++ b/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) } diff --git a/internal/web/static/assets/pages/import.js b/internal/web/static/assets/pages/import.js index 7a5b935..101db51 100644 --- a/internal/web/static/assets/pages/import.js +++ b/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"))), diff --git a/scripts/e2e.sh b/scripts/e2e.sh index 2d28fc1..875ce00 100755 --- a/scripts/e2e.sh +++ b/scripts/e2e.sh @@ -124,6 +124,10 @@ contains "several tables" run "dry run of the import" 0 H import --table "inet main" --defines /w/router.nft -n contains "5 port forwards, 2 source NAT rules" contains "dry run: nothing was changed" +contains "dhcp (enp2s0: the define dhcp_int)" +run "zone names given at import" 0 H import --table "inet main" --defines /w/router.nft --zone enp2s0=lan --zone wg0=vpn -n +contains "lan (enp2s0: as given)" +contains "vpn (wg0: as given)" run "import" 0 H import --table "inet main" --defines /w/router.nft contains "applied and saved" run "the ruleset is in force" 0 R nft list table inet housecarl