diff --git a/DESIGN.md b/DESIGN.md new file mode 100644 index 0000000..b514a5d --- /dev/null +++ b/DESIGN.md @@ -0,0 +1,23 @@ +# Design notes + +## Per-rule enable/disable (not implemented) + +The manage feature set (Save, edit in place, move) deliberately leaves +per-rule disable out. nftables has no disabled state: a rule either is in the +ruleset or it is not, so "disable" means deleting the rule while remembering +enough to put it back exactly where it was. + +Doing that honestly needs a second persistence format — call it v2 of the +saved file — that stores the tool's own view of the ruleset rather than nft's +listing: a list of rule *specs* (the same `RuleSpec` the add and edit forms +produce), each with an enabled flag and its position. Disabling would then be +"delete the rule, keep its spec"; enabling, "insert the spec at its recorded +position"; and Save would write both the active ruleset (as the `.nft` file it +writes today, still loadable by plain `nft -f` on boot) and the spec sidecar +that carries the disabled entries. + +That sidecar is a real format with compatibility duties, and it only pays for +itself once the tool also reconciles it against a ruleset that changed behind +its back. Until that reconciliation exists, a fake disable would silently drop +rules on the floor, so the feature waits for persistence v2 instead of +shipping as a trap. diff --git a/README.md b/README.md index 3990a14..25b03e0 100644 --- a/README.md +++ b/README.md @@ -307,6 +307,8 @@ firewall nor `sudo` is available, it says so and points at `--demo`. | `pgup` / `pgdn` | Scroll a page | | `/` | Filter rules (matches any column; `esc` clears) | | `a` | Add a rule | +| `E` | Edit the selected rule in place — same handle, same position (nftables) | +| `K` / `J` | Move the selected rule up / down, as one atomic transaction (nftables) | | `d` | Delete the selected rule | | `e` | Enable or disable the firewall (ufw only) | | `r` | Reload the firewall | @@ -314,6 +316,7 @@ firewall nor `sudo` is available, it says so and points at `--demo`. | `L` | Change the logging level (ufw) or the log-denied value (firewalld) | | `l` | Toggle logging on the selected rule (nftables) | | `w` | Watch the live firewall log of the logged rules (nftables) | +| `W` | Save the tool's own table to a file loaded on boot, with a diff preview (nftables) | | `x` | Actions this backend offers beyond these keys | | `[` / `]` | Previous / next group — the firewalld zones and policy objects | | `R` | Re-read the firewall | @@ -464,6 +467,44 @@ is restored automatically when the timer expires, and you are back on the rulese that was working. It is the standard `iptables-apply` idea, done as one nft transaction. +### Manage, not view: edit, move and save + +The nftables backend does not stop at adding and deleting. + +**Edit in place.** Press `E` on a rule and the add-rule form opens pre-filled +with the rule as it stands — interfaces, connection state, alias source, +comment, all of it read back from the modelled match. Submitting builds one +`nft replace rule … handle H`, the same replace-by-handle the log toggle uses, +so the rule keeps its handle and its position. A rule the tool cannot hold in +full — one with a match shown only as text, a NAT translation, a source-port +match, a log statement with a level or an nflog group — is refused before the +form opens, with the reason, rather than silently rewritten with half of +itself missing. + +**Move.** `K` and `J` move a rule one position up or down. nftables has no +move command, so a move is a copy written at the new position plus a delete of +the old handle — two statements that only make sense together, which is why +they always run as **one `nft -f` transaction**: wrapped as one when staging +is off, or staged into the atomic batch when it is on. The rule is never +duplicated or missing in between; the moved copy gets a fresh handle and a +reset counter, and the preview says so. + +**Save.** Everything above changes the *running* ruleset, which a reboot +throws away. Press `W` and the tool serialises its own table — `nft list +table inet tui`, nft's own text, which is a valid nft script — and installs it +with `install -m 644 /dev/stdin `. The confirm dialog shows a **unified +diff** against the file as it currently stands, so "what does saving change" +is answered before a byte is written. On an Omarchy router the file is +`/etc/omarchy/router/tui-firewall.nft`, which the router profile includes on +boot; on any other machine it is `/etc/nftables.d/tui-firewall.nft`, loaded by +the usual include from `/etc/nftables.conf` (the dialog spells this out), and +`TUI_FIREWALL_SAVE_PATH` overrides the path for tests and labs. Saving writes +the running table: staged changes that were not applied yet are not in the +file, and the dialog says so. + +Per-rule enable/disable is deliberately absent for now; `DESIGN.md` describes +the persisted-spec format it is waiting for. + ### Per-rule logging and the live log Press `l` on a rule to mark it for logging. nft rules are immutable, so the rule @@ -535,6 +576,17 @@ are capped, so a firewall under a scan cannot grow it without bound. table, and say why when it will not. Delete by handle. - Stage changes and apply them as one atomic `nft -f` transaction, with an automatic connectivity-safe rollback if the apply is not kept in time. +- Edit a rule in place with `E`: the form opens pre-filled from the rule's own + modelled match and the submit is one `nft replace rule … handle H`, keeping + the handle and the position; a rule the model does not hold in full is + refused, never half-rewritten. +- Move a rule up and down with `K` and `J`, as one atomic `nft -f` transaction + (a copy at the new position plus a delete of the old handle), staged into + the batch when staging is on. +- Save the tool's own table to disk with `W` — `nft list table inet tui` + installed to `/etc/omarchy/router/tui-firewall.nft` on an Omarchy router, + `/etc/nftables.d/tui-firewall.nft` elsewhere — with a unified diff against + the current file in the confirm dialog. - Mark a rule to be logged with `l`: the rule is replaced in place, keeping its handle and position, with a stable `log prefix "tui: "` the live view greps for. Toggle it off the same way. @@ -545,7 +597,8 @@ are capped, so a firewall under a scan cannot grow it without bound. ## What v0.1 cannot do -- **No rule editing.** Change a rule by deleting it and adding the new one. +- **No rule editing on ufw or firewalld.** Change a rule by deleting it and + adding the new one; only the nftables backend edits in place with `E`. - **No `ufw` interface qualifiers** (`ufw allow in on eth0 …`): they are parsed and displayed, but the form cannot create them. - **No `ufw` application profile management** (only using existing profiles). @@ -556,10 +609,13 @@ are capped, so a firewall under a scan cannot grow it without bound. editable here. - **No live log tail on ufw or firewalld** — it is an nftables feature, since it reads the kernel log the nftables `log` statement writes. -- **No nftables rule editing or reordering**, and no set-element editing: a rule - is changed by deleting it and adding the new one, and a named set is created or - used, not edited member by member. -- Rules follow the backend's own order; there is no reordering key. +- **No nftables set-element editing**: a named set is created or used, and its + members are added and removed one at a time from the actions menu, not + edited in bulk. +- **No per-rule enable/disable**: nftables has no disabled state, and faking + one honestly needs the persisted-spec format `DESIGN.md` describes. +- On ufw and firewalld, rules follow the backend's own order; the move keys + are an nftables feature. ## Compatibility diff --git a/cmd/tui-firewall/app.go b/cmd/tui-firewall/app.go index 8c84e2a..87d7205 100644 --- a/cmd/tui-firewall/app.go +++ b/cmd/tui-firewall/app.go @@ -99,6 +99,11 @@ type app struct { // the applied batch is on screen before its keep window begins. keepTickPending bool + // editing marks the open rule form as an edit: submitting replaces + // editTarget in place rather than adding a rule. + editing bool + editTarget firewall.Rule + // live holds the live-log view: the open stream, the events retained, and // whether the feed is paused. It is only reachable on the nftables backend // and its demo, the ones that implement logStreamer. @@ -273,6 +278,14 @@ func (a *app) Update(msg tea.Msg) (tea.Model, tea.Cmd) { a.openApplyConfirm(msg.change) return a, nil + case saveReadyMsg: + if msg.err != nil { + a.setStatus(ui.StatusError, msg.err.Error()) + return a, nil + } + a.openSaveConfirm(msg) + return a, nil + case logEventMsg: return a, a.handleLogEvent(msg) @@ -452,6 +465,7 @@ func (a *app) handleForm(msg tea.KeyMsg) (tea.Model, tea.Cmd) { switch msg.String() { case "esc": a.mode = modeTable + a.editing = false a.setStatus(ui.StatusInfo, "cancelled") return a, nil case "tab", "down": @@ -484,18 +498,32 @@ func (a *app) handleForm(msg tea.KeyMsg) (tea.Model, tea.Cmd) { return a, a.form.updateActive(msg) } -// submitForm builds the add command from the form and opens the confirm dialog. +// submitForm builds the add — or, in edit mode, the replace-in-place — command +// from the form and opens the confirm dialog. func (a *app) submitForm() tea.Cmd { spec, err := a.form.spec() if err != nil { a.setStatus(ui.StatusError, err.Error()) return nil } - change, err := a.backend.BuildAddRule(a.group, spec) + var change firewall.Change + if a.editing { + editor, ok := a.backend.(ruleEditor) + if !ok { + // Unreachable: editing is only set by startEdit, which required the + // interface. Refusing beats replacing a rule with an add. + a.setStatus(ui.StatusError, "this backend cannot edit a rule in place") + return nil + } + change, err = editor.BuildEditRule(a.group, a.editTarget, spec) + } else { + change, err = a.backend.BuildAddRule(a.group, spec) + } if err != nil { a.setStatus(ui.StatusError, err.Error()) return nil } + a.editing = false a.stageOrConfirm(change.Description, "The firewall will be changed as follows.", change) return nil } @@ -534,6 +562,14 @@ func (a *app) handleTableKey(msg tea.KeyMsg) (tea.Model, tea.Cmd) { return a, a.load() case "a": return a, a.startAdd() + case "E": + return a, a.startEdit() + case "K": + return a, a.confirmMove(-1) + case "J": + return a, a.confirmMove(1) + case "W": + return a, a.beginSave() case "s": a.toggleStaging() case "S": @@ -575,6 +611,7 @@ func (a *app) startAdd() tea.Cmd { return a.openExtrasMenu() default: a.form = newRuleForm(a.caps, a.model.Services) + a.editing = false a.mode = modeForm return nil } diff --git a/cmd/tui-firewall/edit.go b/cmd/tui-firewall/edit.go new file mode 100644 index 0000000..b0d553a --- /dev/null +++ b/cmd/tui-firewall/edit.go @@ -0,0 +1,101 @@ +package main + +import ( + "fmt" + "strings" + + tea "github.com/charmbracelet/bubbletea" + "github.com/tui-tools/tui-firewall/internal/firewall" + "github.com/tui-tools/tui-firewall/internal/nftables/staging" + "github.com/tui-tools/tui-kit/ui" +) + +// ruleEditor is the part of a backend the edit-in-place flow needs: the spec a +// rule decomposes into, to pre-fill the form, and the replace-by-handle +// command the submitted form builds. Only the nftables backend and its demo +// implement it. +type ruleEditor interface { + RuleSpecFor(group string, rule firewall.Rule) (firewall.RuleSpec, error) + BuildEditRule(group string, rule firewall.Rule, + spec firewall.RuleSpec) (firewall.Change, error) +} + +// ruleMover is the part of a backend the move keys need. The Change it builds +// is a copy plus a delete that only make sense as one transaction, which the +// caller guarantees: staged into the atomic batch, or wrapped in a single +// `nft -f` when staging is off. +type ruleMover interface { + BuildMoveRule(group string, rule firewall.Rule, + delta int) (firewall.Change, error) +} + +// startEdit opens the add-rule form pre-filled with the selected rule, in edit +// mode: submitting replaces the rule in place, keeping its handle and +// position. A rule the backend cannot decompose into the form is refused here, +// before the form opens, with the backend's own reason. +func (a *app) startEdit() tea.Cmd { + editor, ok := a.backend.(ruleEditor) + if !ok { + a.setStatus(ui.StatusWarn, + "this backend does not edit a rule in place: delete it with d and "+ + "add the new one with a") + return nil + } + rule, ok := a.selectedRule() + if !ok { + a.setStatus(ui.StatusWarn, "no rule selected") + return nil + } + spec, err := editor.RuleSpecFor(a.group, rule) + if err != nil { + a.setStatus(ui.StatusError, err.Error()) + return nil + } + a.form = newRuleForm(a.caps, a.model.Services) + a.form.title = fmt.Sprintf("Edit rule (replaces handle %s in place)", rule.ID) + a.form.prefill(spec) + a.editing = true + a.editTarget = rule + a.mode = modeForm + return nil +} + +// confirmMove builds the up-or-down move for the selected rule. With staging +// on it joins the pending batch, where the whole batch is already one nft +// transaction; with staging off it is wrapped as a single `nft -f` so the copy +// and the delete still apply together or not at all. +func (a *app) confirmMove(delta int) tea.Cmd { + mover, ok := a.backend.(ruleMover) + if !ok { + a.setStatus(ui.StatusWarn, + "this backend does not reorder rules; rule order is only "+ + "meaningful on nftables") + return nil + } + rule, ok := a.selectedRule() + if !ok { + a.setStatus(ui.StatusWarn, "no rule selected") + return nil + } + change, err := mover.BuildMoveRule(a.group, rule, delta) + if err != nil { + a.setStatus(ui.StatusError, err.Error()) + return nil + } + if a.stagingOn && a.staging != nil { + a.stageOrConfirm(change.Description, "", change) + return nil + } + + atomic := staging.AtomicCommand(change) + a.mode = modeConfirm + a.confirm = ui.Confirm{ + Title: change.Description, + Body: "The statements below apply as one nft -f transaction: both of " + + "them, or neither.\n" + change.Note + ".", + Command: strings.TrimRight(atomic.Stdin, "\n"), + Danger: true, + Payload: firewall.One(atomic), + } + return nil +} diff --git a/cmd/tui-firewall/form.go b/cmd/tui-firewall/form.go index 471f1bd..420b551 100644 --- a/cmd/tui-firewall/form.go +++ b/cmd/tui-firewall/form.go @@ -69,6 +69,8 @@ type ruleForm struct { fields []formField active int caps firewall.Capabilities + // title heads the dialog: "Add rule", or the edit flow's own heading. + title string } // newRuleForm builds the form for a backend and its service list. Which @@ -166,11 +168,76 @@ func newRuleForm(caps firewall.Capabilities, services []string) ruleForm { kind: fieldText, input: text("empty appends to the end")}) } - f := ruleForm{fields: fields, caps: caps} + f := ruleForm{fields: fields, caps: caps, title: "Add rule"} f.focusActive() return f } +// prefill writes a spec back into the form, which is how the edit flow opens +// the form showing the rule as it stands. Field keys the form does not carry +// (a backend without interfaces, say) are silently skipped: the spec came from +// the same backend the form was built for, so nothing can actually be lost. +func (f *ruleForm) prefill(spec firewall.RuleSpec) { + f.setChoice("action", string(spec.Action)) + f.setChoice("service", spec.Service) + f.setText("ports", spec.Ports) + f.setChoice("proto", spec.Proto) + f.setText("from", spec.From) + f.setText("to", spec.To) + f.setText("icmptype", spec.ICMPType) + f.setText("iif", spec.InIface) + f.setText("oif", spec.OutIface) + f.setChoice("ctstate", strings.Join(spec.CTStates, ",")) + f.setChoice("family", string(spec.Family)) + if spec.Log { + f.setChoice("log", "yes") + } else { + f.setChoice("log", "no") + } + f.setText("comment", spec.Comment) + f.active = 0 + f.focusActive() +} + +// setText fills a text field by key. +func (f *ruleForm) setText(key, value string) { + for i := range f.fields { + if f.fields[i].key == key && f.fields[i].kind == fieldText { + f.fields[i].input.SetValue(value) + return + } + } +} + +// setChoice selects an option by key. An empty value selects the "(none)" +// entry where the field has one. A value the option list does not carry — a +// rule whose conntrack combination is not among the presets, say — is added to +// the list and selected, so pre-filling never silently changes the rule. +func (f *ruleForm) setChoice(key, value string) { + for i := range f.fields { + field := &f.fields[i] + if field.key != key || field.kind != fieldChoice { + continue + } + want := value + if want == "" { + want = noneOption + } + for j, option := range field.options { + if option == want { + field.choice = j + return + } + } + if value == "" { + return + } + field.options = append(field.options, value) + field.choice = len(field.options) - 1 + return + } +} + // focusActive moves the text cursor to the active field. func (f *ruleForm) focusActive() { for i := range f.fields { @@ -295,7 +362,7 @@ func (f ruleForm) view(t theme.Theme, width, height int) string { inner := min(max(width-8, 30), 72) valueWidth := max(inner-labelWidth-6, 10) - lines := []string{t.Title.Render("Add rule"), ""} + lines := []string{t.Title.Render(f.title), ""} for i, field := range f.fields { label := t.Muted.Render(ui.Pad(field.label, labelWidth)) var value string diff --git a/cmd/tui-firewall/manage_ui_test.go b/cmd/tui-firewall/manage_ui_test.go new file mode 100644 index 0000000..374bafd --- /dev/null +++ b/cmd/tui-firewall/manage_ui_test.go @@ -0,0 +1,209 @@ +package main + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/tui-tools/tui-firewall/internal/firewall" + "github.com/tui-tools/tui-firewall/internal/nftables" +) + +const inputView = "inet tui / input" + +func TestSaveKeyPreviewsTheDiffAndInstalls(t *testing.T) { + path := filepath.Join(t.TempDir(), "tui-firewall.nft") + t.Setenv(nftables.SavePathEnv, path) + a, fake := newNftablesApp(t, 120, 30) + + send(t, a, "W") + if a.mode != modeConfirm { + t.Fatalf("W should open the save confirm, mode = %v", a.mode) + } + for _, want := range []string{ + "install -m 644 /dev/stdin " + path, + "+table inet tui {", + } { + if !strings.Contains(a.confirm.Command, want) { + t.Errorf("the save preview should carry %q:\n%s", want, a.confirm.Command) + } + } + if !strings.Contains(a.confirm.Body, nftables.SavePathEnv) { + t.Errorf("the body should carry the boot note:\n%s", a.confirm.Body) + } + + send(t, a, "y") + if fake.SavedPath != path { + t.Errorf("confirming should have run the install, SavedPath = %q", fake.SavedPath) + } + if !strings.Contains(fake.Saved, "table inet tui {") { + t.Errorf("the demo should have recorded the capture:\n%s", fake.Saved) + } + if _, err := os.Stat(path); err == nil { + t.Error("the demo must not write the file") + } +} + +func TestSaveReportsAnUnchangedFile(t *testing.T) { + path := filepath.Join(t.TempDir(), "tui-firewall.nft") + t.Setenv(nftables.SavePathEnv, path) + a, fake := newNftablesApp(t, 120, 30) + + // Pretend a previous save already wrote exactly the current capture. + listing, err := fake.SnapshotOwnTable(t.Context()) + if err != nil { + t.Fatalf("SnapshotOwnTable: %v", err) + } + content := strings.TrimSpace(listing) + "\n" + if err := os.WriteFile(path, []byte(content), 0o600); err != nil { + t.Fatalf("seeding the save file: %v", err) + } + + send(t, a, "W") + if a.mode != modeTable { + t.Fatalf("an up-to-date file needs no dialog, mode = %v", a.mode) + } + if !strings.Contains(a.status, "already matches") { + t.Errorf("status = %q", a.status) + } +} + +func TestEditKeyOpensThePrefilledFormAndReplaces(t *testing.T) { + a, _ := newNftablesApp(t, 130, 30) + gotoView(t, a, inputView) + a.cursor = 4 // handle 14, the WAN ssh drop + + send(t, a, "E") + if a.mode != modeForm { + t.Fatalf("E should open the form, mode = %v", a.mode) + } + if !strings.Contains(a.form.title, "handle 14") { + t.Errorf("the form should say what it replaces: %q", a.form.title) + } + for key, want := range map[string]string{ + "action": "DENY", "iif": "wan0", "proto": "tcp", "ports": "22", + "comment": "no ssh from the wan", + } { + if got := a.form.get(key); got != want { + t.Errorf("prefilled %s = %q, want %q", key, got, want) + } + } + + a.form.setFieldForTest("ports", "2222") + a.form.setFieldForTest("comment", "no ssh from the wan") + send(t, a, "enter") + if a.mode != modeConfirm { + t.Fatalf("submitting the edit should open a confirm, mode = %v", a.mode) + } + for _, want := range []string{ + "replace rule inet tui input handle 14", "tcp dport 2222", + } { + if !strings.Contains(a.confirm.Command, want) { + t.Errorf("the edit preview should carry %q:\n%s", want, a.confirm.Command) + } + } + if a.editing { + t.Error("submitting must clear the editing state") + } +} + +func TestEditEscClearsTheEditingState(t *testing.T) { + a, _ := newNftablesApp(t, 130, 30) + gotoView(t, a, inputView) + a.cursor = 4 + send(t, a, "E") + send(t, a, "esc") + if a.editing { + t.Error("esc must clear the editing state") + } + // The next a is a plain add again. + send(t, a, "a") + if a.form.title != "Add rule" { + t.Errorf("after an aborted edit, a opens a plain add form: %q", a.form.title) + } +} + +func TestEditRefusesARuleItCannotRebuild(t *testing.T) { + a, _ := newNftablesApp(t, 130, 30) + gotoView(t, a, inputView) + a.cursor = 6 // handle 16, the log-only tail rule with a level + send(t, a, "E") + if a.mode != modeTable { + t.Fatalf("the refusal happens before the form opens, mode = %v", a.mode) + } + if !strings.Contains(a.status, "handle 16") { + t.Errorf("the refusal should name the rule: %q", a.status) + } +} + +func TestMoveKeysBuildOneAtomicTransaction(t *testing.T) { + a, fake := newNftablesApp(t, 130, 30) + gotoView(t, a, inputView) + a.cursor = 1 // handle 11, "keep state" + + send(t, a, "J") + if a.mode != modeConfirm { + t.Fatalf("J should open the move confirm, mode = %v", a.mode) + } + for _, want := range []string{ + "add rule inet tui input index 2", + "delete rule inet tui input handle 11", + } { + if !strings.Contains(a.confirm.Command, want) { + t.Errorf("the move preview should carry %q:\n%s", want, a.confirm.Command) + } + } + change, ok := a.confirm.Payload.(firewall.Change) + if !ok || len(change.Commands) != 1 || + strings.Join(change.Commands[0].Argv, " ") != "nft -f -" { + t.Fatalf("the move must run as one nft -f, got %+v", change) + } + + send(t, a, "y") + chain, _ := fake.Ruleset().Chain(nftables.OwnTable, "input") + if chain.Rules[2].Comment != "keep state" { + t.Errorf("the rule should now sit one position lower, position 3 is %q", + chain.Rules[2].Comment) + } +} + +func TestMoveRefusesTheFirstRuleUp(t *testing.T) { + a, _ := newNftablesApp(t, 130, 30) + gotoView(t, a, inputView) + a.cursor = 0 + send(t, a, "K") + if a.mode != modeTable { + t.Fatalf("a refused move opens nothing, mode = %v", a.mode) + } + if !strings.Contains(a.status, "already first") { + t.Errorf("status = %q", a.status) + } +} + +func TestMoveJoinsTheStagedBatch(t *testing.T) { + a, _ := newNftablesApp(t, 130, 30) + gotoView(t, a, inputView) + a.cursor = 1 + send(t, a, "s") // staging on + send(t, a, "J") + if a.staging.Len() != 1 { + t.Fatalf("with staging on a move is staged, pending = %d", a.staging.Len()) + } + if a.mode != modeTable { + t.Errorf("staging a move opens no dialog, mode = %v", a.mode) + } +} + +func TestManageKeysRefuseOnBackendsWithoutThem(t *testing.T) { + a, _ := newTestApp(t, 100, 30) // the ufw demo + for _, key := range []string{"E", "W", "K", "J"} { + send(t, a, key) + if a.mode != modeTable { + t.Fatalf("%s on ufw should only set a status, mode = %v", key, a.mode) + } + if a.status == "" { + t.Errorf("%s on ufw should explain itself", key) + } + } +} diff --git a/cmd/tui-firewall/save.go b/cmd/tui-firewall/save.go new file mode 100644 index 0000000..2bb8671 --- /dev/null +++ b/cmd/tui-firewall/save.go @@ -0,0 +1,107 @@ +package main + +import ( + "context" + "fmt" + "os" + "strings" + "time" + + tea "github.com/charmbracelet/bubbletea" + "github.com/tui-tools/tui-firewall/internal/firewall" + "github.com/tui-tools/tui-firewall/internal/nftables" + "github.com/tui-tools/tui-kit/ui" +) + +// tableSaver is the part of a backend the Save action needs: a serialisation +// of the table this tool owns, as text `nft -f` can load back. The nftables +// backend and its demo implement it; ufw and firewalld persist through their +// own daemons and need no file from us. +type tableSaver interface { + SnapshotOwnTable(ctx context.Context) (string, error) +} + +// saveReadyMsg carries the capture once it has been read off the machine: the +// install command to confirm, and the diff against the file as it stands. +type saveReadyMsg struct { + change firewall.Change + diff string + path string + err error +} + +// beginSave captures the owned table and prepares the install command. The +// capture is a privileged read that can block, so it runs off the update loop +// and comes back through saveReadyMsg. +func (a *app) beginSave() tea.Cmd { + saver, ok := a.backend.(tableSaver) + if !ok { + a.setStatus(ui.StatusWarn, + "saving the ruleset to a file is an nftables feature; ufw and "+ + "firewalld persist their own configuration") + return nil + } + if a.awaitingKeep { + a.setStatus(ui.StatusWarn, + "an applied batch is waiting: press k to keep it before saving") + return nil + } + return func() tea.Msg { + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + listing, err := saver.SnapshotOwnTable(ctx) + if err != nil { + return saveReadyMsg{err: fmt.Errorf( + "could not capture table %s to save it: %w", nftables.OwnTable, err)} + } + path, bootNote := nftables.SavePath() + change, err := nftables.BuildSave(listing, path, bootNote) + if err != nil { + return saveReadyMsg{err: err} + } + diff := nftables.UnifiedDiff(currentSaveFile(path), + strings.TrimSpace(listing)+"\n", path, "the capture to save") + return saveReadyMsg{change: change, diff: diff, path: path} + } +} + +// currentSaveFile reads what the save path holds now, for the diff the preview +// shows. A file that is missing or unreadable diffs as empty, which renders +// every captured line as an addition — exactly what installing it would do. +func currentSaveFile(path string) string { + data, err := os.ReadFile(path) // #nosec G304 -- path is the vetted save location (constant or the TUI_FIREWALL_SAVE_PATH override), read only to render the preview diff + if err != nil { + return "" + } + return string(data) +} + +// openSaveConfirm previews the save: the install command, and under it the +// unified diff between the file as it stands and the capture about to replace +// it, so "what does saving change" is answered before anything is written. +// +// Save never joins a staged batch: the batch is an nft transaction and this is +// a file write, and mixing the two would make the atomic apply a lie. What it +// writes is the running table — a staged change that was not applied yet is +// not in the file, and the body says so while staging is collecting. +func (a *app) openSaveConfirm(msg saveReadyMsg) { + if msg.diff == "" { + a.setStatusf(ui.StatusOK, "%s already matches the running table %s", + msg.path, nftables.OwnTable) + return + } + body := fmt.Sprintf( + "The running table %s is written to %s, replacing the file shown in "+ + "the diff below.\n%s.", nftables.OwnTable, msg.path, msg.change.Note) + if a.stagingOn && a.staging != nil && a.staging.Len() > 0 { + body += "\nStaged changes that were not applied yet are not part of " + + "this file." + } + a.mode = modeConfirm + a.confirm = ui.Confirm{ + Title: msg.change.Description, + Body: body, + Command: a.backend.Preview(msg.change) + "\n\n" + msg.diff, + Payload: msg.change, + } +} diff --git a/cmd/tui-firewall/view.go b/cmd/tui-firewall/view.go index 49e5a42..487557c 100644 --- a/cmd/tui-firewall/view.go +++ b/cmd/tui-firewall/view.go @@ -397,6 +397,14 @@ func (a *app) shortHelpKeys() []ui.KeyHint { if _, ok := a.backend.(logStreamer); ok { hints = append(hints, ui.KeyHint{Key: "w", Desc: "live log"}) } + // Edit in place, move and save are offered only where the backend can do + // them: the nftables backend and its demo. + if _, ok := a.backend.(ruleEditor); ok { + hints = append(hints, ui.KeyHint{Key: "E", Desc: "edit"}) + } + if _, ok := a.backend.(tableSaver); ok { + hints = append(hints, ui.KeyHint{Key: "W", Desc: "save"}) + } if len(a.backend.Extras(a.model, a.group)) > 0 { hints = append(hints, ui.KeyHint{Key: "x", Desc: "actions"}) } @@ -426,6 +434,8 @@ func helpKeys() []ui.KeyHint { {Key: "pgup/pgdn", Desc: "scroll a page"}, {Key: "/", Desc: "filter the rules (esc clears)"}, {Key: "a", Desc: "add a rule"}, + {Key: "E", Desc: "edit the selected rule in place, keeping its handle and position (nftables)"}, + {Key: "K / J", Desc: "move the selected rule up / down, as one atomic transaction (nftables)"}, {Key: "d", Desc: "delete the selected rule"}, {Key: "e", Desc: "enable or disable the firewall (ufw)"}, {Key: "r", Desc: "reload the firewall"}, @@ -440,6 +450,7 @@ func helpKeys() []ui.KeyHint { {Key: "s", Desc: "toggle staging: collect changes instead of applying them (nftables)"}, {Key: "S", Desc: "review and apply the staged changes as one atomic transaction"}, {Key: "k", Desc: "keep an applied batch before its rollback timer fires"}, + {Key: "W", Desc: "save the tool's own table to a file loaded on boot, with a diff preview (nftables)"}, {Key: "?", Desc: "this help"}, {Key: "q", Desc: "quit"}, {Key: "", Desc: ""}, diff --git a/internal/nftables/backend.go b/internal/nftables/backend.go index 40367ea..dd261ed 100644 --- a/internal/nftables/backend.go +++ b/internal/nftables/backend.go @@ -35,9 +35,13 @@ type Real struct { // mu guards ruleset, which is the state the command builders decide // against: which chains exist, which of them have a policy, how many - // rules use each alias. It is replaced wholesale by Load. + // rules use each alias. It is replaced wholesale by Load. It also guards + // the lazily-built install runner below. mu sync.Mutex ruleset Ruleset + // install runs install(1) for the Save action; built on first use. + install *runner.Runner + installErr error } // Available reports whether nft is installed on this host. @@ -104,9 +108,82 @@ func (r *Real) Ruleset() Ruleset { return r.ruleset } -// Run executes a previewed change. +// Run executes a previewed change. Almost every command is an nft invocation; +// the one exception is the Save action's `install`, which the dispatching +// runner below routes to its own binary. func (r *Real) Run(ctx context.Context, change firewall.Change) (string, error) { - return firewall.RunChange(ctx, r.run, change) + return firewall.RunChange(ctx, dispatchRunner{r}, change) +} + +// dispatchRunner routes each command of a change to the runner that owns its +// binary. The nft runner resolves argv[0] to the nft path, so a command whose +// argv[0] is another binary must not go through it; today that is only the +// Save action's install(1). +type dispatchRunner struct{ r *Real } + +// Preview renders the command the way the nft runner would; the privilege +// prefix is the same for both binaries, so the text is too. +func (d dispatchRunner) Preview(cmd firewall.Command) string { + return d.r.run.Preview(cmd) +} + +// Run routes one command to its binary's runner. +func (d dispatchRunner) Run(ctx context.Context, cmd firewall.Command) (string, error) { + if len(cmd.Argv) > 0 && cmd.Argv[0] == "install" { + ir, err := d.r.installRunner() + if err != nil { + return "", err + } + return ir.Run(ctx, cmd) + } + return d.r.run.Run(ctx, cmd) +} + +// installRunner lazily builds the runner for install(1), which the Save action +// uses to write the serialized table with the right owner and mode in one +// step. It is built on first use rather than at startup so a machine without +// it — none in practice; install ships with coreutils — still gets every other +// feature. +func (r *Real) installRunner() (*runner.Runner, error) { + r.mu.Lock() + defer r.mu.Unlock() + if r.install != nil || r.installErr != nil { + return r.install, r.installErr + } + r.install, r.installErr = runner.New(runner.Options{ + Bin: "install", + SearchPaths: []string{"/usr/bin/install", "/bin/install"}, + SudoPrefix: r.sudoPrefix, + InstallHint: "install(1) ships with coreutils", + }) + return r.install, r.installErr +} + +// SnapshotOwnTable serialises the table this tool owns as nft's own text, +// which is what the Save action writes to disk. Like the staging snapshot it +// is the human listing, not the JSON: the text is a valid nft script, so the +// saved file loads straight back with `nft -f`. +func (r *Real) SnapshotOwnTable(ctx context.Context) (string, error) { + argv := append([]string{"nft", "list", "table"}, OwnTable.Args()...) + return r.run.Read(ctx, argv...) +} + +// RuleSpecFor reads the selected row back into the spec the edit form opens +// pre-filled with. +func (r *Real) RuleSpecFor(group string, rule firewall.Rule) (firewall.RuleSpec, error) { + return r.Ruleset().SpecFor(group, rule) +} + +// BuildEditRule replaces the selected row in place with the edited spec. +func (r *Real) BuildEditRule(group string, rule firewall.Rule, + spec firewall.RuleSpec) (firewall.Change, error) { + return r.Ruleset().EditRule(group, rule, spec) +} + +// BuildMoveRule moves the selected row one position up or down its chain. +func (r *Real) BuildMoveRule(group string, rule firewall.Rule, + delta int) (firewall.Change, error) { + return r.Ruleset().MoveRule(group, rule, delta) } // SnapshotRuleset reads the whole ruleset as nft's own text, which is what the diff --git a/internal/nftables/command.go b/internal/nftables/command.go index 78ed5cc..91e4aa1 100644 --- a/internal/nftables/command.go +++ b/internal/nftables/command.go @@ -127,7 +127,7 @@ func (r Ruleset) BuildAddRule(chain Chain, spec firewall.RuleSpec) (firewall.Cha if err := r.checkMutable(chain); err != nil { return firewall.Change{}, err } - expr, err := r.ruleExpression(chain, spec) + expr, err := r.ruleExpression(chain, spec, "") if err != nil { return firewall.Change{}, err } @@ -151,8 +151,11 @@ func (r Ruleset) BuildAddRule(chain Chain, spec firewall.RuleSpec) (firewall.Cha // ruleExpression assembles the match and the statements of a new rule, in the // order nft prints them, so the preview reads like the rule the list will -// show afterwards. -func (r Ruleset) ruleExpression(chain Chain, spec firewall.RuleSpec) ([]string, error) { +// show afterwards. logPrefix, when not empty, is the prefix a logging rule +// carries; the add-rule path passes none and logs bare, the edit path passes +// the stable tui: prefix so an edited rule feeds the live log view. +func (r Ruleset) ruleExpression(chain Chain, spec firewall.RuleSpec, + logPrefix string) ([]string, error) { verdict, err := verdictFor(spec.Action) if err != nil { return nil, err @@ -226,6 +229,9 @@ func (r Ruleset) ruleExpression(chain Chain, spec firewall.RuleSpec) ([]string, if spec.Log { expr = append(expr, "log") + if logPrefix != "" { + expr = append(expr, "prefix", quote(logPrefix)) + } } // Every rule this backend writes carries a counter. It costs a few // instructions per packet and it is the only way the rule list can answer @@ -305,25 +311,63 @@ func (r Ruleset) BuildToggleLog(chain Chain, target Rule) (firewall.Change, erro // statements come out in the order nft prints them, the same order the add-rule // builder uses, so the replacement reads like the rule the list will show. func logToggleExpr(chain Chain, rule Rule) ([]string, error) { + m := rule.Match + if err := checkRebuildable(rule, "toggle its log"); err != nil { + return nil, err + } + return rebuildRuleExpr(chain, rule, !m.Log, logPrefix(chain.Name, m.Verdict)) +} + +// checkRebuildable is the guard every replace-by-handle rebuild shares: a rule +// this package does not hold in full is refused rather than rewritten from a +// rendering that might drop half of it. The action names what the caller was +// about to do, so the refusal says which key was pressed. +func checkRebuildable(rule Rule, action string) error { m := rule.Match if m.NAT != nil { - return nil, errorf( - "rule handle %d translates addresses; logging is toggled from the "+ - "filter views, not the NAT one", rule.Handle) + return errorf( + "rule handle %d translates addresses; a NAT rule is deleted and "+ + "re-created from the actions menu, not rebuilt in place", + rule.Handle) } if len(m.Unmodeled) > 0 { - return nil, errorf( + return errorf( "rule handle %d carries a match this tool shows only as text (%s), so "+ - "it cannot be rebuilt safely to toggle logging; edit it with nft "+ - "directly", rule.Handle, strings.Join(m.Unmodeled, "; ")) + "it cannot be rebuilt safely to %s; edit it with nft "+ + "directly", rule.Handle, strings.Join(m.Unmodeled, "; "), action) } switch m.Verdict { case "accept", "drop", "reject", "": default: - return nil, errorf( - "per-rule logging is toggled on accept, drop and reject rules; rule "+ + return errorf( + "only accept, drop and reject rules are rebuilt in place; rule "+ "handle %d is a %q rule", rule.Handle, m.Verdict) } + return nil +} + +// checkLogRoundTrip refuses a rule whose log statement carries arguments the +// rebuild would drop: a level, or an nflog group. The log toggle does not need +// it — toggling off removes the whole statement anyway — but an edit or a move +// promises to keep the rule as it stands, and silently downgrading `nflog +// group 5` to a plain kernel log would break whatever reads that group. +func checkLogRoundTrip(rule Rule) error { + if rule.Match.LogGroup != "" || rule.Match.LogLevel != "" { + return errorf( + "rule handle %d logs with arguments this tool does not rebuild "+ + "(a level or an nflog group), so it is not rewritten in place; "+ + "edit it with nft directly", rule.Handle) + } + return nil +} + +// rebuildRuleExpr renders a modelled rule's whole expression again, in the +// order nft prints statements, with its log statement under the caller's +// control: log says whether the rebuilt rule logs at all, and prefix is the +// log prefix it carries (empty logs bare). The caller has already run +// checkRebuildable. +func rebuildRuleExpr(chain Chain, rule Rule, log bool, prefix string) ([]string, error) { + m := rule.Match var expr []string ifaces, err := interfaceSelectors(m.IIF, m.OIF) @@ -347,10 +391,19 @@ func logToggleExpr(chain Chain, rule Rule) ([]string, error) { } expr = append(expr, l4...) - // The toggle: add the log statement when the rule does not log, drop it when - // it does. A rule that logs and nothing else keeps its bare counter. - if !m.Log { - expr = append(expr, "log", "prefix", quote(logPrefix(chain.Name, m.Verdict))) + // The log statement, under the caller's control: the toggle flips it, an + // edit or a move carries it over. A prefix rides in one quoted argv word, + // the way a comment does, so it must not smuggle a quote or a line break. + if log { + if strings.ContainsAny(prefix, "\n\r\"") { + return nil, errorf("rule handle %d has a log prefix nft would not "+ + "round-trip", rule.Handle) + } + if prefix != "" { + expr = append(expr, "log", "prefix", quote(prefix)) + } else { + expr = append(expr, "log") + } } expr = append(expr, "counter") @@ -369,7 +422,7 @@ func logToggleExpr(chain Chain, rule Rule) ([]string, error) { } if len(expr) == 0 { return nil, errorf( - "rule handle %d has no match this tool models, so toggling its log "+ + "rule handle %d has no match this tool models, so rebuilding it "+ "would rewrite it as a match-everything rule", rule.Handle) } return expr, nil diff --git a/internal/nftables/diff.go b/internal/nftables/diff.go new file mode 100644 index 0000000..ef674c6 --- /dev/null +++ b/internal/nftables/diff.go @@ -0,0 +1,152 @@ +package nftables + +import ( + "fmt" + "strings" +) + +// UnifiedDiff renders the change between two texts as a unified diff: the +// familiar ---/+++ header and one hunk per run of changes, three lines of +// context around each. It is what the Save preview shows — the file as it is +// against the file the confirm would install — so "what does saving change" +// is answered before anything is written. +// +// It is a plain longest-common-subsequence diff over lines. The files it +// compares are one table's listing; quadratic is fine at that size. +func UnifiedDiff(oldText, newText, oldLabel, newLabel string) string { + oldLines := splitLines(oldText) + newLines := splitLines(newText) + ops := diffOps(oldLines, newLines) + hunks := groupHunks(ops, 3) + if len(hunks) == 0 { + return "" + } + + var b strings.Builder + fmt.Fprintf(&b, "--- %s\n+++ %s\n", oldLabel, newLabel) + for _, hunk := range hunks { + writeHunk(&b, hunk, oldLines, newLines) + } + return strings.TrimRight(b.String(), "\n") +} + +// diffOp is one line of the diff: kept, deleted or added. +type diffOp struct { + // kind is ' ', '-' or '+'. + kind byte + // oldIndex and newIndex are the 0-based line positions; -1 where the op + // has no line on that side. + oldIndex, newIndex int +} + +// splitLines cuts a text into lines without a trailing phantom. +func splitLines(text string) []string { + if text == "" { + return nil + } + return strings.Split(strings.TrimRight(text, "\n"), "\n") +} + +// diffOps walks the LCS table into the op list. +func diffOps(oldLines, newLines []string) []diffOp { + n, m := len(oldLines), len(newLines) + // lcs[i][j] is the length of the longest common subsequence of + // oldLines[i:] and newLines[j:]. + lcs := make([][]int, n+1) + for i := range lcs { + lcs[i] = make([]int, m+1) + } + for i := n - 1; i >= 0; i-- { + for j := m - 1; j >= 0; j-- { + if oldLines[i] == newLines[j] { + lcs[i][j] = lcs[i+1][j+1] + 1 + continue + } + lcs[i][j] = max(lcs[i+1][j], lcs[i][j+1]) + } + } + + var ops []diffOp + i, j := 0, 0 + for i < n && j < m { + switch { + case oldLines[i] == newLines[j]: + ops = append(ops, diffOp{' ', i, j}) + i++ + j++ + case lcs[i+1][j] >= lcs[i][j+1]: + ops = append(ops, diffOp{'-', i, -1}) + i++ + default: + ops = append(ops, diffOp{'+', -1, j}) + j++ + } + } + for ; i < n; i++ { + ops = append(ops, diffOp{'-', i, -1}) + } + for ; j < m; j++ { + ops = append(ops, diffOp{'+', -1, j}) + } + return ops +} + +// groupHunks slices the op list into hunks: runs of changes with up to +// context kept lines around each, merged when their contexts touch. +func groupHunks(ops []diffOp, context int) [][]diffOp { + var hunks [][]diffOp + start := -1 // index into ops where the current hunk began + lastChange := -1 + for i, op := range ops { + if op.kind == ' ' { + if start >= 0 && i-lastChange > context*2 { + hunks = append(hunks, ops[start:lastChange+context+1]) + start = -1 + } + continue + } + if start < 0 { + start = max(i-context, 0) + } + lastChange = i + } + if start >= 0 { + end := min(lastChange+context+1, len(ops)) + hunks = append(hunks, ops[start:end]) + } + return hunks +} + +// writeHunk renders one hunk with its @@ header. +func writeHunk(b *strings.Builder, hunk []diffOp, oldLines, newLines []string) { + oldStart, newStart := 0, 0 + oldCount, newCount := 0, 0 + for i, op := range hunk { + if op.oldIndex >= 0 { + if oldCount == 0 { + oldStart = op.oldIndex + 1 + } + oldCount++ + } + if op.newIndex >= 0 { + if newCount == 0 { + newStart = op.newIndex + 1 + } + newCount++ + } + _ = i + } + // A side with no lines at all points just before where the change lands, + // the way diff(1) spells an empty range. + fmt.Fprintf(b, "@@ -%d,%d +%d,%d @@\n", oldStart, oldCount, newStart, newCount) + for _, op := range hunk { + switch op.kind { + case ' ': + b.WriteString(" " + oldLines[op.oldIndex] + "\n") + case '-': + b.WriteString("-" + oldLines[op.oldIndex] + "\n") + case '+': + b.WriteString("+" + newLines[op.newIndex] + "\n") + } + } +} diff --git a/internal/nftables/edit.go b/internal/nftables/edit.go new file mode 100644 index 0000000..0c8f1c6 --- /dev/null +++ b/internal/nftables/edit.go @@ -0,0 +1,140 @@ +package nftables + +import ( + "fmt" + "strconv" + "strings" + + "github.com/tui-tools/tui-firewall/internal/firewall" +) + +// BuildEditRule replaces a rule in place with the rule the spec describes. +// +// nftables rules are immutable, so an edit is the same machinery the log +// toggle uses: `nft replace rule … handle H …`, which keeps the handle and the +// position while swapping the whole expression. The rule being replaced must +// be one this package holds in full — the same refusal BuildToggleLog makes — +// because an edit form was pre-filled from the modelled match, and a rule the +// model shows only half of would silently lose the other half on submit. +func (r Ruleset) BuildEditRule(chain Chain, target Rule, + spec firewall.RuleSpec) (firewall.Change, error) { + if err := r.checkMutable(chain); err != nil { + return firewall.Change{}, err + } + if target.Handle <= 0 { + return firewall.Change{}, errorf( + "this rule has no handle, so there is no safe way to name it to nft; " + + "re-read the ruleset with R") + } + if err := checkRebuildable(target, "edit it"); err != nil { + return firewall.Change{}, err + } + if err := checkLogRoundTrip(target); err != nil { + return firewall.Change{}, err + } + if spec.Position > 0 { + return firewall.Change{}, errorf( + "an edited rule keeps its position; move it with K and J instead " + + "of setting an insert position") + } + // The edited rule logs with the stable tui: prefix, the one the live log + // view greps for, so an edit that keeps logging on keeps feeding it. + prefix := "" + if spec.Log { + verdict, err := verdictFor(spec.Action) + if err != nil { + return firewall.Change{}, err + } + prefix = logPrefix(chain.Name, verdict) + } + expr, err := r.ruleExpression(chain, spec, prefix) + if err != nil { + return firewall.Change{}, err + } + + argv := []string{"nft", "replace", "rule"} + argv = append(argv, chain.Table.Args()...) + argv = append(argv, chain.Name, "handle", strconv.Itoa(target.Handle)) + argv = append(argv, expr...) + + return firewall.One(firewall.Command{ + Argv: argv, + Description: fmt.Sprintf("Edit rule handle %d of %s in place", + target.Handle, chain.Name), + // Replacing a rule resets its counter, and an edit can open or close + // traffic: both reasons for the danger colour. + Destructive: true, + }), nil +} + +// SpecForRule reads a modelled rule back into the RuleSpec the add-rule form +// is built from, so the edit form opens pre-filled with the rule as it stands. +// A rule the form could not have expressed — a source-port match, say — is +// refused here, before the form opens, rather than silently dropped on submit. +func (r Ruleset) SpecForRule(chain Chain, target Rule) (firewall.RuleSpec, error) { + m := target.Match + if err := checkRebuildable(target, "edit it"); err != nil { + return firewall.RuleSpec{}, err + } + if err := checkLogRoundTrip(target); err != nil { + return firewall.RuleSpec{}, err + } + if m.SPort != "" { + return firewall.RuleSpec{}, errorf( + "rule handle %d matches a source port, which the form cannot "+ + "express; edit it with nft directly", target.Handle) + } + if m.Verdict == "" { + return firewall.RuleSpec{}, errorf( + "rule handle %d has no verdict of its own (it only logs and falls "+ + "through), so the form cannot express it; toggle its log with l "+ + "or edit it with nft directly", target.Handle) + } + if m.RejectWith != "" { + return firewall.RuleSpec{}, errorf( + "rule handle %d rejects with %q, an answer the form cannot "+ + "express; edit it with nft directly", target.Handle, m.RejectWith) + } + + spec := firewall.RuleSpec{ + Action: actionOf(m.Verdict), + Proto: m.Proto, + Ports: formPorts(m.DPort), + To: m.Daddr, + InIface: m.IIF, + OutIface: m.OIF, + ICMPType: m.ICMPType, + Comment: target.Comment, + Family: firewall.Family(m.Family()), + Log: m.Log, + } + // An alias source goes in the form's alias field, a literal in From: the + // same split the form's own validation enforces on the way back. + if strings.HasPrefix(m.Saddr, "@") { + spec.Service = m.Saddr + } else { + spec.From = m.Saddr + } + // An ICMP match came out of the parser as a protocol plus a port-less + // selector; the form spells icmp with an empty port, so nothing to map. + if m.CTState != "" { + spec.CTStates = strings.Split(m.CTState, ",") + } + return spec, nil +} + +// formPorts renders a modelled port match the way the form's port field spells +// it: a braced set the reader rendered as "{ 80, 443 }" becomes "80,443", and +// anything else passes through. +func formPorts(port string) string { + inner, ok := strings.CutPrefix(strings.TrimSpace(port), "{") + if !ok { + return port + } + inner = strings.TrimSuffix(strings.TrimSpace(inner), "}") + items := strings.Split(inner, ",") + for i := range items { + items[i] = strings.TrimSpace(items[i]) + } + return strings.Join(items, ",") +} diff --git a/internal/nftables/edit_test.go b/internal/nftables/edit_test.go new file mode 100644 index 0000000..c6dcf11 --- /dev/null +++ b/internal/nftables/edit_test.go @@ -0,0 +1,176 @@ +package nftables + +import ( + "strings" + "testing" + + "github.com/tui-tools/tui-firewall/internal/firewall" +) + +// inputGroup is the group name of the fixture's input chain, the same string +// the UI hands back to the backend. +func inputGroup(t *testing.T, r Ruleset) string { + t.Helper() + chain, ok := r.Chain(OwnTable, "input") + if !ok { + t.Fatal("the fixture has no input chain") + } + return GroupName(chain) +} + +func TestSpecForReadsARuleBackIntoTheForm(t *testing.T) { + ruleset := parseFixture(t, "router") + // Handle 14 is the WAN ssh drop: iifname wan0, tcp dport 22, drop. + spec, err := ruleset.SpecFor(inputGroup(t, ruleset), firewall.Rule{ID: "14"}) + if err != nil { + t.Fatalf("SpecFor: %v", err) + } + if spec.Action != firewall.ActionDeny { + t.Errorf("Action = %q, want %q", spec.Action, firewall.ActionDeny) + } + if spec.InIface != "wan0" { + t.Errorf("InIface = %q, want wan0", spec.InIface) + } + if spec.Proto != "tcp" || spec.Ports != "22" { + t.Errorf("Proto/Ports = %q/%q, want tcp/22", spec.Proto, spec.Ports) + } + if spec.Comment != "no ssh from the wan" { + t.Errorf("Comment = %q", spec.Comment) + } + if spec.Log { + t.Error("handle 14 does not log") + } +} + +func TestSpecForKeepsAnAliasSource(t *testing.T) { + ruleset := parseFixture(t, "router") + // Handle 12 matches @lan_hosts and @admin_ports. + spec, err := ruleset.SpecFor(inputGroup(t, ruleset), firewall.Rule{ID: "12"}) + if err != nil { + t.Fatalf("SpecFor: %v", err) + } + if spec.Service != "@lan_hosts" { + t.Errorf("Service = %q, want @lan_hosts", spec.Service) + } + if spec.From != "" { + t.Errorf("an alias source must not also fill From, got %q", spec.From) + } + if spec.Ports != "@admin_ports" { + t.Errorf("Ports = %q, want @admin_ports", spec.Ports) + } +} + +func TestBuildEditRuleReplacesInPlace(t *testing.T) { + ruleset := parseFixture(t, "router") + chain, _ := ruleset.Chain(OwnTable, "input") + group := inputGroup(t, ruleset) + spec, err := ruleset.SpecFor(group, firewall.Rule{ID: "14"}) + if err != nil { + t.Fatalf("SpecFor: %v", err) + } + // The edit: a new port, and logging turned on. + spec.Ports = "2222" + spec.Log = true + + change, err := ruleset.EditRule(group, firewall.Rule{ID: "14"}, spec) + if err != nil { + t.Fatalf("EditRule: %v", err) + } + got := argvString(t, change) + want := `nft replace rule inet tui input handle 14 iifname "wan0" ` + + `tcp dport 2222 log prefix "tui:input drop " counter drop ` + + `comment "no ssh from the wan"` + if got != want { + t.Errorf("argv =\n %s\nwant\n %s", got, want) + } + if !change.Destructive { + t.Error("an edit resets the counter and can change traffic: destructive") + } + _ = chain +} + +func TestBuildEditRuleRefusesAPosition(t *testing.T) { + ruleset := parseFixture(t, "router") + group := inputGroup(t, ruleset) + spec, err := ruleset.SpecFor(group, firewall.Rule{ID: "14"}) + if err != nil { + t.Fatalf("SpecFor: %v", err) + } + spec.Position = 2 + if _, err := ruleset.EditRule(group, firewall.Rule{ID: "14"}, spec); err == nil || + !strings.Contains(err.Error(), "keeps its position") { + t.Errorf("a position on an edit should be refused, got %v", err) + } +} + +func TestBuildEditRuleRefusesUnmodeledRules(t *testing.T) { + ruleset := parseFixture(t, "router") + chain, _ := ruleset.Chain(OwnTable, "input") + target := Rule{ + Handle: 99, + Match: Match{ + Verdict: "accept", + Unmodeled: []string{"meta mark 0x1"}, + }, + } + _, err := ruleset.BuildEditRule(chain, target, firewall.RuleSpec{ + Action: firewall.ActionAllow, Ports: "22", Proto: "tcp"}) + if err == nil || !strings.Contains(err.Error(), "only as text") { + t.Errorf("an unmodeled rule must be refused like the log toggle does, got %v", err) + } +} + +func TestSpecForRefusesASourcePortMatch(t *testing.T) { + ruleset := parseFixture(t, "router") + chain, _ := ruleset.Chain(OwnTable, "input") + target := Rule{Handle: 99, Match: Match{Verdict: "accept", SPort: "1000"}} + if _, err := ruleset.SpecForRule(chain, target); err == nil || + !strings.Contains(err.Error(), "source port") { + t.Errorf("a source-port rule cannot round-trip the form, got %v", err) + } +} + +func TestSpecForRefusesLogArguments(t *testing.T) { + ruleset := parseFixture(t, "router") + // Handle 16 logs with a level, which an edit could not carry over. + _, err := ruleset.SpecFor(inputGroup(t, ruleset), firewall.Rule{ID: "16"}) + if err == nil || !strings.Contains(err.Error(), "logs with arguments") { + t.Errorf("a log level must refuse the edit before the form opens, got %v", err) + } +} + +func TestEditRefusesTheNonChainGroups(t *testing.T) { + ruleset := parseFixture(t, "router") + for _, group := range []string{GroupNAT, GroupAliases} { + if _, err := ruleset.EditRule(group, firewall.Rule{ID: "23"}, + firewall.RuleSpec{}); err == nil { + t.Errorf("editing in %s should be refused", group) + } + } +} + +func TestBuildEditRuleGuardsInjectionInEveryOperand(t *testing.T) { + ruleset := parseFixture(t, "router") + group := inputGroup(t, ruleset) + base, err := ruleset.SpecFor(group, firewall.Rule{ID: "14"}) + if err != nil { + t.Fatalf("SpecFor: %v", err) + } + poison := func(mutate func(*firewall.RuleSpec)) error { + spec := base + mutate(&spec) + _, err := ruleset.EditRule(group, firewall.Rule{ID: "14"}, spec) + return err + } + cases := map[string]func(*firewall.RuleSpec){ + "ports": func(s *firewall.RuleSpec) { s.Ports = "22; flush ruleset" }, + "iface": func(s *firewall.RuleSpec) { s.InIface = `wan0" accept` }, + "from": func(s *firewall.RuleSpec) { s.From = "10.0.0.1\ndrop" }, + "comment": func(s *firewall.RuleSpec) { s.Comment = "a \"quoted\" word" }, + } + for name, mutate := range cases { + if err := poison(mutate); err == nil { + t.Errorf("%s: an operand carrying nft syntax must be refused", name) + } + } +} diff --git a/internal/nftables/fake.go b/internal/nftables/fake.go index a095a0f..bf3ea63 100644 --- a/internal/nftables/fake.go +++ b/internal/nftables/fake.go @@ -32,6 +32,10 @@ type Fake struct { Log []firewall.Command // FailWith, when set, makes the next Run fail with this error. FailWith error + // Saved and SavedPath record what the demo pretended to install with the + // Save action, so the tests can assert the file content round-tripped. + Saved string + SavedPath string } // NewFake returns a Fake preloaded with the sample router ruleset. @@ -125,6 +129,15 @@ func (f *Fake) runOne(cmd firewall.Command) (string, error) { return "", errorf("empty command") } + // The Save action's install(1) is the one command that is not nft. The + // demo records what would have been written and touches no file. + if args[0] == "install" { + f.SavedPath = args[len(args)-1] + f.Saved = cmd.Stdin + return "demo: pretended to install " + f.SavedPath + + "; nothing on disk was written", nil + } + // `nft -f -` is the one batch form: the whole staged transaction, or a // snapshot restore, arrives on standard input. The demo applies it so // staging behaves in --demo exactly as it does against a real nft. @@ -398,9 +411,18 @@ func (f *Fake) addRule(operands []string, insert bool) (string, error) { expr := rest[1:] position := len(chain.Rules) - if insert && len(expr) >= 2 && expr[0] == "index" { - if n, err := strconv.Atoi(expr[1]); err == nil && n >= 0 && n <= position { - position = n + if len(expr) >= 2 && expr[0] == "index" { + // `insert … index N` writes before the rule now at N, and + // `add … index N` writes after it: the two forms the move builder + // uses, applied here the way nft applies them. + if n, err := strconv.Atoi(expr[1]); err == nil { + at := n + if !insert { + at = n + 1 + } + if at >= 0 && at <= position { + position = at + } } expr = expr[2:] } @@ -829,6 +851,67 @@ func (f *Fake) BuildSetLogging(level string) (firewall.Change, error) { return (&Real{}).BuildSetLogging(level) } +// SnapshotOwnTable renders the in-memory table this tool owns as nft-style +// text, which is what the Save action captures. The demo has no nft to print +// the canonical form, so this is a faithful-enough rendering built from the +// same parsed state the views show: the diff the Save preview draws in --demo +// behaves like the real one without a byte leaving memory. +func (f *Fake) SnapshotOwnTable(_ context.Context) (string, error) { + rs := f.Ruleset() + table, ok := rs.Table(OwnTable) + if !ok { + return "", errorf("there is no table %s to save; the actions menu "+ + "offers to create it", OwnTable) + } + var b strings.Builder + b.WriteString("table " + OwnTable.String() + " {\n") + for _, set := range table.Sets { + b.WriteString("\tset " + set.Name + " {\n") + b.WriteString("\t\ttype " + set.Type + "\n") + if len(set.Flags) > 0 { + b.WriteString("\t\tflags " + strings.Join(set.Flags, ",") + "\n") + } + if set.Comment != "" { + b.WriteString("\t\tcomment \"" + set.Comment + "\"\n") + } + if len(set.Elements) > 0 { + b.WriteString("\t\telements = { " + + strings.Join(set.Elements, ", ") + " }\n") + } + b.WriteString("\t}\n") + } + for _, chain := range table.Chains { + b.WriteString("\tchain " + chain.Name + " {\n") + if chain.Base() { + b.WriteString("\t\t" + chain.Describe() + ";\n") + } + for _, rule := range chain.Rules { + b.WriteString("\t\t" + rule.Raw + "\n") + } + b.WriteString("\t}\n") + } + b.WriteString("}\n") + return b.String(), nil +} + +// RuleSpecFor reads the selected row back into the spec the edit form opens +// pre-filled with. +func (f *Fake) RuleSpecFor(group string, rule firewall.Rule) (firewall.RuleSpec, error) { + return f.Ruleset().SpecFor(group, rule) +} + +// BuildEditRule replaces the selected row in place with the edited spec. +func (f *Fake) BuildEditRule(group string, rule firewall.Rule, + spec firewall.RuleSpec) (firewall.Change, error) { + return f.Ruleset().EditRule(group, rule, spec) +} + +// BuildMoveRule moves the selected row one position up or down its chain. +func (f *Fake) BuildMoveRule(group string, rule firewall.Rule, + delta int) (firewall.Change, error) { + return f.Ruleset().MoveRule(group, rule, delta) +} + // Extras lists the nftables-specific actions. func (f *Fake) Extras(_ firewall.Model, _ string) []firewall.Extra { return f.Ruleset().Extras() diff --git a/internal/nftables/move.go b/internal/nftables/move.go new file mode 100644 index 0000000..5ea4d3c --- /dev/null +++ b/internal/nftables/move.go @@ -0,0 +1,119 @@ +package nftables + +import ( + "fmt" + "strconv" + + "github.com/tui-tools/tui-firewall/internal/firewall" +) + +// BuildMoveRule moves a rule one position up or down its chain. +// +// nftables has no move command, so a move is two statements: a copy of the +// rule written at the new position, and a delete of the old handle. The two +// only make sense together — run apart, a failure in between leaves the rule +// duplicated or gone — which is why the Change carries a note saying it must +// apply as one `nft -f` transaction, and why the callers route it through the +// staging engine (or wrap it with staging.AtomicCommand) rather than running +// the commands one at a time. +// +// The copy is rebuilt from the rule's own modelled match, exactly like the log +// toggle: a rule the model does not hold in full is refused rather than +// re-rendered with half its matches missing. +func (r Ruleset) BuildMoveRule(chain Chain, target Rule, delta int) (firewall.Change, error) { + if err := r.checkMutable(chain); err != nil { + return firewall.Change{}, err + } + if delta != 1 && delta != -1 { + return firewall.Change{}, errorf("a rule moves one position at a time") + } + if target.Handle <= 0 { + return firewall.Change{}, errorf( + "this rule has no handle, so there is no safe way to name it to nft; " + + "re-read the ruleset with R") + } + if err := checkRebuildable(target, "move it"); err != nil { + return firewall.Change{}, err + } + if err := checkLogRoundTrip(target); err != nil { + return firewall.Change{}, err + } + + // The position comes from where the handle sits in the chain right now, + // not from the row's display index: the two agree unless the ruleset + // changed underneath, and then the handle is the one that is true. + position := -1 + for i, rule := range chain.Rules { + if rule.Handle == target.Handle { + position = i + break + } + } + if position < 0 { + return firewall.Change{}, errorf( + "rule handle %d is no longer in chain %s; press R to re-read the "+ + "ruleset", target.Handle, chain.Name) + } + + // The rule keeps its expression exactly as it stands, its own log prefix + // included: a move changes where the rule sits, nothing about what it does. + expr, err := rebuildRuleExpr(chain, target, target.Match.Log, + target.Match.LogPrefix) + if err != nil { + return firewall.Change{}, err + } + // nft counts rule indexes from zero. `insert … index N` writes the copy + // before the rule now at N; `add … index N` writes it after. Moving up + // inserts before the previous rule, moving down appends after the next + // one, and in both cases the original is still in place when the copy + // lands, so the indexes name the pre-move chain. + var verb string + var index int + var where string + if delta < 0 { + if position == 0 { + return firewall.Change{}, errorf( + "rule handle %d is already first in %s", target.Handle, chain.Name) + } + verb, index, where = "insert", position-1, "up" + } else { + if position == len(chain.Rules)-1 { + return firewall.Change{}, errorf( + "rule handle %d is already last in %s", target.Handle, chain.Name) + } + verb, index, where = "add", position+1, "down" + } + + place := []string{"nft", verb, "rule"} + place = append(place, chain.Table.Args()...) + place = append(place, chain.Name, "index", strconv.Itoa(index)) + place = append(place, expr...) + + del := []string{"nft", "delete", "rule"} + del = append(del, chain.Table.Args()...) + del = append(del, chain.Name, "handle", strconv.Itoa(target.Handle)) + + description := fmt.Sprintf("Move rule handle %d %s in %s", + target.Handle, where, chain.Name) + return firewall.Change{ + Description: description, + Destructive: true, + Note: "the copy and the delete apply as one atomic nft transaction, " + + "so the rule is never duplicated or missing in between; the new " + + "copy gets a fresh handle and a reset counter", + Commands: []firewall.Command{ + { + Argv: place, + Description: fmt.Sprintf("Write the rule at its new position "+ + "in %s", chain.Name), + Destructive: true, + }, + { + Argv: del, + Description: fmt.Sprintf("Delete the old copy, handle %d", + target.Handle), + Destructive: true, + }, + }, + }, nil +} diff --git a/internal/nftables/move_test.go b/internal/nftables/move_test.go new file mode 100644 index 0000000..6b59417 --- /dev/null +++ b/internal/nftables/move_test.go @@ -0,0 +1,147 @@ +package nftables + +import ( + "context" + "strconv" + "strings" + "testing" + + "github.com/tui-tools/tui-firewall/internal/firewall" + "github.com/tui-tools/tui-firewall/internal/nftables/staging" +) + +// moveCommands joins each command of a move for assertions. +func moveCommands(t *testing.T, change firewall.Change) []string { + t.Helper() + if len(change.Commands) != 2 { + t.Fatalf("a move is a copy plus a delete, got %d commands", len(change.Commands)) + } + out := make([]string, 0, 2) + for _, cmd := range change.Commands { + out = append(out, strings.Join(cmd.Argv, " ")) + } + return out +} + +func TestBuildMoveRuleUp(t *testing.T) { + ruleset := parseFixture(t, "router") + // Handle 14 sits at 0-based position 4 of input (handles 10..16). + change, err := ruleset.MoveRule(inputGroup(t, ruleset), firewall.Rule{ID: "14"}, -1) + if err != nil { + t.Fatalf("MoveRule: %v", err) + } + cmds := moveCommands(t, change) + wantPlace := `nft insert rule inet tui input index 3 iifname "wan0" ` + + `tcp dport 22 counter drop comment "no ssh from the wan"` + if cmds[0] != wantPlace { + t.Errorf("place =\n %s\nwant\n %s", cmds[0], wantPlace) + } + if cmds[1] != "nft delete rule inet tui input handle 14" { + t.Errorf("delete = %q", cmds[1]) + } + if !change.Destructive { + t.Error("a move rewrites the rule and should be destructive") + } + if !strings.Contains(change.Note, "atomic") { + t.Errorf("the note must say the two commands are one transaction: %q", change.Note) + } +} + +func TestBuildMoveRuleDown(t *testing.T) { + ruleset := parseFixture(t, "router") + change, err := ruleset.MoveRule(inputGroup(t, ruleset), firewall.Rule{ID: "14"}, 1) + if err != nil { + t.Fatalf("MoveRule: %v", err) + } + cmds := moveCommands(t, change) + if !strings.HasPrefix(cmds[0], "nft add rule inet tui input index 5 ") { + t.Errorf("moving down appends after the next rule: %q", cmds[0]) + } +} + +func TestBuildMoveRuleRefusesTheEnds(t *testing.T) { + ruleset := parseFixture(t, "router") + group := inputGroup(t, ruleset) + if _, err := ruleset.MoveRule(group, firewall.Rule{ID: "10"}, -1); err == nil || + !strings.Contains(err.Error(), "already first") { + t.Errorf("moving the first rule up should be refused, got %v", err) + } + chain, _ := ruleset.Chain(OwnTable, "output") + outGroup := GroupName(chain) + last := chain.Rules[len(chain.Rules)-1] + if _, err := ruleset.MoveRule(outGroup, + firewall.Rule{ID: strconv.Itoa(last.Handle)}, 1); err == nil || + !strings.Contains(err.Error(), "already last") { + t.Errorf("moving the last rule down should be refused, got %v", err) + } +} + +func TestBuildMoveRuleKeepsTheLogStatement(t *testing.T) { + ruleset := parseFixture(t, "router") + chain, _ := ruleset.Chain(OwnTable, "input") + target, ok := findRuleByHandle(chain, 14) + if !ok { + t.Fatal("handle 14 is missing") + } + target.Match.Log = true + target.Match.LogPrefix = "tui:input drop " + change, err := ruleset.BuildMoveRule(chain, target, -1) + if err != nil { + t.Fatalf("BuildMoveRule: %v", err) + } + place := strings.Join(change.Commands[0].Argv, " ") + if !strings.Contains(place, `log prefix "tui:input drop "`) { + t.Errorf("the moved copy must keep its own log statement: %s", place) + } +} + +func TestBuildMoveRuleRefusesUnmodeledAndNAT(t *testing.T) { + ruleset := parseFixture(t, "router") + chain, _ := ruleset.Chain(OwnTable, "input") + unmodeled := Rule{Handle: 99, Match: Match{ + Verdict: "accept", Unmodeled: []string{"limit rate 5/second"}}} + if _, err := ruleset.BuildMoveRule(chain, unmodeled, -1); err == nil { + t.Error("a rule the model does not hold in full must not be moved") + } + if _, err := ruleset.MoveRule(GroupNAT, firewall.Rule{ID: "23"}, -1); err == nil { + t.Error("the NAT view has no move") + } + if _, err := ruleset.MoveRule(GroupAliases, firewall.Rule{ID: "x"}, -1); err == nil { + t.Error("the alias view has no move") + } +} + +// TestMoveAppliesAtomicallyThroughTheFake proves the whole path: the move's +// two commands wrapped as one `nft -f` transaction, applied by the demo the +// way nft would, leaving the rule at its new position and the old handle gone. +func TestMoveAppliesAtomicallyThroughTheFake(t *testing.T) { + fake := NewFake() + group := inputGroup(t, fake.Ruleset()) + change, err := fake.BuildMoveRule(group, firewall.Rule{ID: "14"}, -1) + if err != nil { + t.Fatalf("BuildMoveRule: %v", err) + } + atomic := staging.AtomicCommand(change) + if strings.Join(atomic.Argv, " ") != "nft -f -" { + t.Fatalf("the wrapper must be one nft -f, got %v", atomic.Argv) + } + if _, err := fake.Run(context.Background(), firewall.One(atomic)); err != nil { + t.Fatalf("applying the move: %v", err) + } + + chain, _ := fake.Ruleset().Chain(OwnTable, "input") + if len(chain.Rules) != 7 { + t.Fatalf("the chain should still hold 7 rules, has %d", len(chain.Rules)) + } + if chain.Rules[3].Comment != "no ssh from the wan" { + t.Errorf("position 4 should now be the moved rule, is %q (handle %d)", + chain.Rules[3].Comment, chain.Rules[3].Handle) + } + if chain.Rules[4].Comment != "admin from lan v6" { + t.Errorf("the displaced rule should follow, position 5 is %q", + chain.Rules[4].Comment) + } + if _, ok := findRuleByHandle(chain, 14); ok { + t.Error("the old handle must be gone after the move") + } +} diff --git a/internal/nftables/ops.go b/internal/nftables/ops.go index 79cb14d..08de67e 100644 --- a/internal/nftables/ops.go +++ b/internal/nftables/ops.go @@ -123,6 +123,71 @@ func (r Ruleset) ToggleLog(group string, rule firewall.Rule) (firewall.Change, e return r.BuildToggleLog(chain, target) } +// chainRule resolves a group and a UI row down to the chain and the modelled +// rule the row stands for: the shared front half of every action that rebuilds +// a rule by its handle. what names the action for the refusal messages. +func (r Ruleset) chainRule(group string, row firewall.Rule, + what string) (Chain, Rule, error) { + switch group { + case GroupAliases: + return Chain{}, Rule{}, errorf( + "an alias is not a rule; %s applies to a rule in one of the chain "+ + "views", what) + case GroupNAT: + return Chain{}, Rule{}, errorf( + "%s applies to filter rules; a NAT rule is deleted and re-created "+ + "from the actions menu", what) + } + chain, err := r.ChainForGroup(group) + if err != nil { + return Chain{}, Rule{}, err + } + handle, err := strconv.Atoi(row.ID) + if err != nil || handle <= 0 { + return Chain{}, Rule{}, errorf( + "this rule has no handle (%q), so there is no safe way to name it to "+ + "nft; re-read the ruleset with R", row.ID) + } + target, ok := findRuleByHandle(chain, handle) + if !ok { + return Chain{}, Rule{}, errorf( + "rule handle %d is no longer in chain %s; press R to re-read the "+ + "ruleset", handle, chain.Name) + } + return chain, target, nil +} + +// EditRule builds the in-place replacement for the selected row from the spec +// the edit form collected. +func (r Ruleset) EditRule(group string, row firewall.Rule, + spec firewall.RuleSpec) (firewall.Change, error) { + chain, target, err := r.chainRule(group, row, "editing") + if err != nil { + return firewall.Change{}, err + } + return r.BuildEditRule(chain, target, spec) +} + +// SpecFor reads the selected row back into the RuleSpec the edit form opens +// pre-filled with. +func (r Ruleset) SpecFor(group string, row firewall.Rule) (firewall.RuleSpec, error) { + chain, target, err := r.chainRule(group, row, "editing") + if err != nil { + return firewall.RuleSpec{}, err + } + return r.SpecForRule(chain, target) +} + +// MoveRule builds the atomic up-or-down move for the selected row. +func (r Ruleset) MoveRule(group string, row firewall.Rule, + delta int) (firewall.Change, error) { + chain, target, err := r.chainRule(group, row, "moving") + if err != nil { + return firewall.Change{}, err + } + return r.BuildMoveRule(chain, target, delta) +} + // findRuleByHandle returns the rule a chain holds under a handle. func findRuleByHandle(chain Chain, handle int) (Rule, bool) { for _, rule := range chain.Rules { diff --git a/internal/nftables/save.go b/internal/nftables/save.go new file mode 100644 index 0000000..aa78242 --- /dev/null +++ b/internal/nftables/save.go @@ -0,0 +1,88 @@ +package nftables + +import ( + "os" + "path/filepath" + "strings" + + "github.com/tui-tools/tui-firewall/internal/firewall" +) + +// SavePathEnv overrides where the Save action writes the serialized table. +// It exists for the tests and the lab, which must not touch /etc. +const SavePathEnv = "TUI_FIREWALL_SAVE_PATH" + +// The two places the saved table lands, by kind of machine. On an Omarchy +// router the profile's own boot script includes the file; on any other host it +// goes where nftables include snippets conventionally live, and loading it on +// boot is one include line the note below spells out. +const ( + savePathRouter = "/etc/omarchy/router/tui-firewall.nft" + savePathPlain = "/etc/nftables.d/tui-firewall.nft" +) + +// SavePath decides where the Save action writes, and what happens to the file +// on boot. The environment wins, then the Omarchy router layout when the +// machine has one, then the plain nftables.d convention. +func SavePath() (path, bootNote string) { + if p := os.Getenv(SavePathEnv); p != "" { + return p, "the path comes from " + SavePathEnv + + "; arrange for your boot to load it with nft -f" + } + if info, err := os.Stat("/etc/omarchy"); err == nil && info.IsDir() { + return savePathRouter, "the Omarchy router profile includes this file " + + "when it renders the firewall on boot" + } + return savePathPlain, "load it on boot by including it from " + + "/etc/nftables.conf (include \"/etc/nftables.d/*.nft\") and enabling " + + "the nftables service" +} + +// BuildSave turns a captured `nft list table` listing into the install command +// that writes it to disk. The listing travels on standard input rather than in +// an argv word: a whole ruleset does not belong on a command line, and +// `install -m 644 /dev/stdin ` writes it root-owned with the mode in one +// step, no temp file left behind on failure. +// +// The listing is nft's own text, which is a valid nft script — the same +// property the staged rollback relies on — so the file `nft -f` loads on boot +// is byte-for-byte what nft printed here. +func BuildSave(listing, path, bootNote string) (firewall.Change, error) { + if err := checkSavePath(path); err != nil { + return firewall.Change{}, err + } + trimmed := strings.TrimSpace(listing) + if trimmed == "" { + return firewall.Change{}, errorf( + "there is nothing to save: table %s does not exist yet; the "+ + "actions menu offers to create it", OwnTable) + } + if !strings.Contains(trimmed, "table "+OwnTable.String()) { + return firewall.Change{}, errorf( + "the capture does not look like table %s, so it is not written "+ + "anywhere; re-read the ruleset with R and try again", OwnTable) + } + + return firewall.Change{ + Description: "Save table " + OwnTable.String() + " to " + path, + Note: bootNote, + Commands: []firewall.Command{{ + Argv: []string{"install", "-m", "644", "/dev/stdin", path}, + Description: "Install the serialized table as " + path, + Stdin: trimmed + "\n", + }}, + }, nil +} + +// checkSavePath refuses a destination that could not have come from this +// tool's own constants or a sane override: relative, empty, or carrying a +// character that has no business in a system path. +func checkSavePath(path string) error { + if !filepath.IsAbs(path) { + return errorf("the save path must be absolute, not %q", path) + } + if strings.ContainsAny(path, " \t\n\r\"';") { + return errorf("%q contains a character a save path cannot carry", path) + } + return nil +} diff --git a/internal/nftables/save_test.go b/internal/nftables/save_test.go new file mode 100644 index 0000000..2bb1362 --- /dev/null +++ b/internal/nftables/save_test.go @@ -0,0 +1,146 @@ +package nftables + +import ( + "context" + "os" + "strings" + "testing" + + "github.com/tui-tools/tui-firewall/internal/firewall" +) + +func TestSavePathEnvOverride(t *testing.T) { + t.Setenv(SavePathEnv, "/tmp/somewhere/save.nft") + path, note := SavePath() + if path != "/tmp/somewhere/save.nft" { + t.Errorf("path = %q, want the override", path) + } + if !strings.Contains(note, SavePathEnv) { + t.Errorf("the note should say where the path came from: %q", note) + } +} + +func TestSavePathByMachineKind(t *testing.T) { + t.Setenv(SavePathEnv, "") + path, note := SavePath() + if info, err := os.Stat("/etc/omarchy"); err == nil && info.IsDir() { + if path != savePathRouter || !strings.Contains(note, "Omarchy") { + t.Errorf("on an Omarchy machine: path %q, note %q", path, note) + } + return + } + if path != savePathPlain { + t.Errorf("path = %q, want %q", path, savePathPlain) + } + if !strings.Contains(note, "nftables") { + t.Errorf("the note should say how the file loads on boot: %q", note) + } +} + +func TestBuildSave(t *testing.T) { + listing := "table inet tui {\n\tchain input {\n\t}\n}\n" + change, err := BuildSave(listing, savePathPlain, "loaded on boot") + if err != nil { + t.Fatalf("BuildSave: %v", err) + } + got := argvString(t, change) + want := "install -m 644 /dev/stdin " + savePathPlain + if got != want { + t.Errorf("argv = %q, want %q", got, want) + } + if change.Commands[0].Stdin != listing { + t.Errorf("the stdin must be the capture byte for byte:\n%q", change.Commands[0].Stdin) + } + if change.Note != "loaded on boot" { + t.Errorf("the boot note must reach the dialog: %q", change.Note) + } + if change.Destructive { + t.Error("saving the running table to disk closes no traffic") + } +} + +func TestBuildSaveRefusals(t *testing.T) { + good := "table inet tui {\n}\n" + cases := []struct { + name string + listing string + path string + }{ + {"empty capture", "", savePathPlain}, + {"whitespace capture", " \n", savePathPlain}, + {"not the owned table", "table inet firewalld {\n}\n", savePathPlain}, + {"relative path", good, "etc/save.nft"}, + {"path with a space", good, "/etc/save file.nft"}, + {"path with a newline", good, "/etc/save\n.nft"}, + {"path with a semicolon", good, "/etc/save;rm.nft"}, + {"path with a quote", good, `/etc/"save".nft`}, + } + for _, tc := range cases { + if _, err := BuildSave(tc.listing, tc.path, ""); err == nil { + t.Errorf("%s: should have been refused", tc.name) + } + } +} + +func TestUnifiedDiff(t *testing.T) { + if diff := UnifiedDiff("a\nb\n", "a\nb\n", "old", "new"); diff != "" { + t.Errorf("equal texts diff to nothing, got %q", diff) + } + diff := UnifiedDiff("a\nb\nc\n", "a\nB\nc\nd\n", "old", "new") + for _, want := range []string{"--- old", "+++ new", "-b", "+B", "+d", " a"} { + if !strings.Contains(diff, want) { + t.Errorf("diff missing %q:\n%s", want, diff) + } + } + // A brand-new file is all additions. + fresh := UnifiedDiff("", "table inet tui {\n}\n", "old", "new") + if !strings.Contains(fresh, "+table inet tui {") { + t.Errorf("a first save shows the whole file as additions:\n%s", fresh) + } +} + +// TestFakeSaveRoundTrip proves the --demo parity: the fake serialises its own +// table, the builder wraps it, and the fake's Run records the install instead +// of touching the machine. +func TestFakeSaveRoundTrip(t *testing.T) { + fake := NewFake() + listing, err := fake.SnapshotOwnTable(context.Background()) + if err != nil { + t.Fatalf("SnapshotOwnTable: %v", err) + } + for _, want := range []string{"table inet tui {", "chain input {", "set lan_hosts {"} { + if !strings.Contains(listing, want) { + t.Errorf("the demo capture should carry %q:\n%s", want, listing) + } + } + change, err := BuildSave(listing, "/tmp/tui-firewall-test.nft", "note") + if err != nil { + t.Fatalf("BuildSave: %v", err) + } + if _, err := fake.Run(context.Background(), change); err != nil { + t.Fatalf("Run: %v", err) + } + if fake.SavedPath != "/tmp/tui-firewall-test.nft" { + t.Errorf("SavedPath = %q", fake.SavedPath) + } + if fake.Saved != change.Commands[0].Stdin { + t.Error("the demo must record exactly the bytes the install would write") + } + if _, err := os.Stat("/tmp/tui-firewall-test.nft"); err == nil { + t.Error("the demo must not create the file") + } +} + +// TestSaveNeverJoinsAnNftBatch documents why the Save change is one install +// command: a change that mixed nft statements with a file write could not be +// one atomic transaction, and the builder never produces such a mix. +func TestSaveNeverJoinsAnNftBatch(t *testing.T) { + change, err := BuildSave("table inet tui {\n}\n", savePathPlain, "") + if err != nil { + t.Fatalf("BuildSave: %v", err) + } + if len(change.Commands) != 1 || change.Commands[0].Argv[0] != "install" { + t.Errorf("the save is exactly one install command, got %v", change.Commands) + } + var _ = firewall.Change{} // keep the import honest if assertions change +} diff --git a/internal/nftables/staging/atomic_test.go b/internal/nftables/staging/atomic_test.go new file mode 100644 index 0000000..7a43cd6 --- /dev/null +++ b/internal/nftables/staging/atomic_test.go @@ -0,0 +1,36 @@ +package staging + +import ( + "strings" + "testing" + + "github.com/tui-tools/tui-firewall/internal/firewall" +) + +func TestAtomicCommandWrapsAMultiCommandChange(t *testing.T) { + change := firewall.Change{ + Description: "Move rule handle 14 up in input", + Destructive: true, + Commands: []firewall.Command{ + {Argv: []string{"nft", "insert", "rule", "inet", "tui", "input", + "index", "3", "tcp", "dport", "22", "counter", "drop"}}, + {Argv: []string{"nft", "delete", "rule", "inet", "tui", "input", + "handle", "14"}}, + }, + } + cmd := AtomicCommand(change) + if strings.Join(cmd.Argv, " ") != "nft -f -" { + t.Errorf("argv = %v, want nft -f -", cmd.Argv) + } + want := "insert rule inet tui input index 3 tcp dport 22 counter drop\n" + + "delete rule inet tui input handle 14\n" + if cmd.Stdin != want { + t.Errorf("stdin =\n%q\nwant\n%q", cmd.Stdin, want) + } + if cmd.Description != change.Description { + t.Errorf("description = %q", cmd.Description) + } + if !cmd.Destructive { + t.Error("the wrapper keeps the change's danger flag") + } +} diff --git a/internal/nftables/staging/staging.go b/internal/nftables/staging/staging.go index 806319c..6aea531 100644 --- a/internal/nftables/staging/staging.go +++ b/internal/nftables/staging/staging.go @@ -322,6 +322,21 @@ func restoreCommand(snapshot string) firewall.Command { } } +// AtomicCommand renders one multi-command Change as a single `nft -f` +// transaction: every command becomes a script line on standard input, and nft +// applies all of them or none. It is how a change whose commands only make +// sense together — a rule move is a copy plus a delete — runs atomically even +// when staging mode is off. The script rides in Stdin, so a confirm dialog +// shows it the way the staged apply shows its batch. +func AtomicCommand(change firewall.Change) firewall.Command { + return firewall.Command{ + Argv: []string{"nft", "-f", "-"}, + Description: change.Description, + Destructive: change.Destructive, + Stdin: batchScript([]firewall.Change{change}), + } +} + // batchScript renders the staged changes as an nft script: one line per // command, the leading "nft" dropped because a script carries statements, not // invocations. The quoting the builders put in — around a comment, an interface