Staged undo, built-ins window, machine warnings; raw and dynamic set fixes

- Undo is staged like any edit: in review mode the web UI's Undo, and hc
  history undo with -C or HOUSECARL_CHANGESET, add a revert operation
  that checks, when replayed, that nothing it touches changed since;
  applied, History links the entries. Imports and takeovers still can't
  be staged.
- The rules page sums up a chain's built-ins in one line; a window lists
  them with what they do and their hits, and switches them.
- Warnings about the machine (dashboard, hc status): nftables.service
  enabled (it loads /etc/nftables.conf at boot and flushes every table
  when stopped), and the imported table still in force beside
  Housecarl's. The docs say to disable nftables.service but not stop it.
- A raw rule with other fields set is refused (raw_conflict) instead of
  silently ignoring them; the rule dialog's nftables section hides what
  it replaces, and the importer's raw rules carry only their text.
- A dynamic set with configured entries is refused
  (set_dynamic_entries): they were silently ignored. The set dialog
  shows entries or the entry timeout, as fits.
- The rule dialog's chain choices are just the chains' names.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
abrown38andClaude Opus 5.5 committed 2026-10-19 14:52:55 +01:00
1 parent d25e12995e
commit 9a165cf70b
26 files changed
+408 -58

No files matched your search

+7 -2
View File
@@ -35,7 +35,8 @@ the way Jarl and Steward are.
review shows the problems, nftables' check and the diff of the ruleset.
The web UI collects edits as pending changes by default (pending rows,
a bar with Review and Apply now, a Changes page), with a switch for live
edits; hc stages with `-C ID` or `HOUSECARL_CHANGESET`, and has `hc
edits, and Undo in History is staged like any edit; hc stages with
`-C ID` or `HOUSECARL_CHANGESET`, and has `hc
changeset create/list/show/diff/apply/discard/remove`. An applied
changeset is on probation like any change. `--dry-run` shows the
ruleset's diff too (docs/design.md, docs/cli.md).
@@ -66,6 +67,9 @@ the way Jarl and Steward are.
- **At boot:** the confirmed ruleset is loaded by
`housecarl-ruleset.service` before the network comes up, without
housecarld. An empty configuration applies nothing.
- **Warnings about the machine:** while `nftables.service` is enabled
(it loads `/etc/nftables.conf` at boot, and flushes every table when
stopped), and while the table an import came from still applies.
- **Hits and the log:** packet and byte counters for every rule, NAT rule,
built-in and policy; packets logged by rules and policies, read from
netlink log group 4171 (`hc log -f`, the Log page).
@@ -81,7 +85,8 @@ the way Jarl and Steward are.
chain with hits and reordering, NAT, zones, sets, services, the log,
settings, import, history with undo; the rule dialog keeps the less
used parts, such as rate limits, quotas and time windows, behind "+"
buttons), with users, roles, scoped API
buttons; each chain's built-ins are listed, with their hits, and
switched in a window), with users, roles, scoped API
tokens, sessions, the audit log, and the web UI's certificate from
Let's Encrypt through Jarl, as in Steward.
- **Packaging:** Debian packages `housecarl` and `housecarl-cli`, a
+8 -1
View File
@@ -285,6 +285,9 @@ func ruleCmd() *cobra.Command {
}
extra := ruleExtras(r)
action := r.Action
if r.Raw != "" {
action = "-" // the text's own
}
if r.Log {
action += "+log"
}
@@ -298,7 +301,11 @@ func ruleCmd() *cobra.Command {
}
cmd.AddCommand(ls)
cmd.AddCommand(showCmd("rules", "rule", func(w io.Writer, r model.Rule) {
fmt.Fprintf(w, "ID:\t%d\nChain:\t%s\nMatch:\t%s%s\nAction:\t%s\n", r.ID, r.Chain, describe(r.Match), ruleExtras(r), r.Action)
if r.Raw == "" {
fmt.Fprintf(w, "ID:\t%d\nChain:\t%s\nMatch:\t%s%s\nAction:\t%s\n", r.ID, r.Chain, describe(r.Match), ruleExtras(r), r.Action)
} else {
fmt.Fprintf(w, "ID:\t%d\nChain:\t%s\n", r.ID, r.Chain)
}
if r.Raw != "" {
fmt.Fprintf(w, "Raw:\t%s\n", r.Raw)
}
+4 -1
View File
@@ -157,7 +157,10 @@ Changes staged to be reviewed and applied together ([design.md](design.md#review
Staging (`?changeset=ID` on any change to objects or settings) checks
the changeset with the new operation, and nftables checks its ruleset;
an error refuses the operation. Only open changesets take operations, and
only their owner's (or an administrator's). Undo, `/import/nft` and
only their owner's (or an administrator's). An undo
(`POST /history/{id}/revert?changeset=ID`) is staged as one operation,
which checks when it is replayed that nothing it touches changed since;
applied, History links it to the entry it undoes. `/import/nft` and
`/takeover` can't be staged: with a changeset named they answer 400.
Reads of objects, settings and `/ruleset` with `?changeset=ID` show the
+3 -3
View File
@@ -117,9 +117,9 @@ it, because an exported ID easily outlives its changeset:
`-C ID` and an ID argument are used as given (housecarld still refuses a
changeset that isn't open, or another user's unless you are an
administrator). Only configuration changes are staged: dynamic set
entries, `confirm` and `rollback` are made at once, and `history undo`,
`import` and `takeover` can't be staged, so they are refused while a
changeset is named. Changesets staged in the web UI show up here too,
entries, `confirm` and `rollback` are made at once; `history undo` is
staged like any change; `import` and `takeover` can't be staged, so they
are refused while a changeset is named. Changesets staged in the web UI show up here too,
and the other way round.
## Commands
+3 -2
View File
@@ -75,8 +75,9 @@ to be reviewed and applied together with others:
| CLI | Immediate: each command is a change of its own. Stage with `-C ID` or `HOUSECARL_CHANGESET`. | Scripts want a clear result per step. |
| API | Immediate unless a changeset is named (`?changeset=ID`, `X-Housecarl-Changeset`). | As the CLI. |
Undo, imports and takeovers can't be staged; with a changeset named, they
are refused rather than applied.
An undo is staged like any edit (in review mode, the web UI's Undo
collects it for review). Imports and takeovers can't be staged; with a
changeset named, they are refused rather than applied.
## Its own table only
+9 -1
View File
@@ -107,7 +107,10 @@ the old table:
sudo hc takeover "inet main"
```
From now on only Housecarl's rules apply. Over the network, this is on
From now on only Housecarl's rules apply. Until you do, the dashboard
and `hc status` warn that the imported table still applies
(`takeover_pending`); they also warn while `nftables.service` is enabled
(`nftables_service`). Over the network, this is on
probation too: unless confirmed, the old table comes back.
## 4. The boot
@@ -122,6 +125,11 @@ sudo systemctl disable nftables.service
sudo mv /etc/nftables.conf /etc/nftables.conf.before-housecarl
```
Disable it, but **don't stop it** (no `systemctl stop nftables`, no
`disable --now`): Debian's unit stops by running `nft flush ruleset`,
which deletes every table, Housecarl's and Docker's included, at once.
Disabled, it simply doesn't run at the next boot.
`housecarl-ruleset.service` runs after `nftables.service` in any case,
so a leftover `flush ruleset` can't undo it. Docker, Incus and libvirt
keep adding their own tables as before.
+15 -4
View File
@@ -80,10 +80,21 @@ sudo nft -c -f /var/lib/housecarl/ruleset.nft # check it
Dynamic sets start empty after a reboot.
Keep Debian's `nftables.service` disabled, or `/etc/nftables.conf`
without `flush ruleset`: otherwise it deletes Housecarl's table at
boot. `housecarl-ruleset.service` runs after it, which covers the
ordinary case.
Keep Debian's `nftables.service` disabled (`systemctl disable
nftables`, without `--now`): at boot it loads `/etc/nftables.conf`, and
stopping or restarting it runs `nft flush ruleset`, which deletes every
table, Housecarl's included. `housecarl-ruleset.service` runs after it
at boot, which covers the ordinary case, but not a restart of
`nftables.service` later. Housecarl never reads or writes
`/etc/nftables.conf`; its ruleset is saved in
`/var/lib/housecarl/ruleset.nft`. The dashboard and `hc status` warn
while `nftables.service` is enabled.
Other programs' tables are left alone: fail2ban's (`inet f2b-table`),
Docker's, Incus's. Housecarl replaces only its own table, so they keep
their rules through every Housecarl change and restart, and nothing
needs restarting in a particular order. A packet must get past every
table on its hook, so a drop in any of them wins.
## Hits
+9 -3
View File
@@ -38,7 +38,10 @@ directly (`--from 192.168.1.3`); a set is better when several rules share
them.
A **dynamic** set's entries are added at run time, each with a timeout,
and nftables removes them when it runs out, even with housecarld stopped:
and nftables removes them when it runs out, even with housecarld stopped.
It has no configured entries (validation refuses them); its timeout is
the default for entries added without one, and without a timeout they
stay until removed:
```sh
hc set add guests --dynamic --timeout 24h
@@ -193,8 +196,11 @@ hand, placed like any other:
hc rule add forward --raw 'iifname != "wan" ct state invalid ip daddr @trusted_v4 accept'
```
It is checked by nftables with the rest of the ruleset, and can use the
sets' nftables names (`@trusted_v4`). The importer makes raw rules from
It is used exactly as written: its text replaces the rule's zones,
addresses, services, action, limits and log, so a raw rule with any of
those set is refused (`raw_conflict`); write them in the text. It is
checked by nftables with the rest of the ruleset, and can use the sets'
nftables names (`@trusted_v4`). The importer makes raw rules from
what it can't translate. It goes into the ruleset as written: add
`counter` to it to count its packets.
+12
View File
@@ -51,6 +51,7 @@ are unique per kind, ignoring case.
|---|---|---|
| `set_entry` | error | an entry that isn't an address, prefix (without host bits) or range |
| `set_empty` | warning | a set (not dynamic) with no entries: rules using it match nothing |
| `set_dynamic_entries` | error | configured entries on a dynamic set: its entries come at run time |
| `set_timeout` | error | a timeout on a set that isn't dynamic, or under a second |
| `service_empty` | error | a service with no entries |
| `service_entry` | error | an entry that isn't `PROTO/PORTS` (`tcp`, `udp`, `sctp`; a port or range) or `icmp/TYPE`, `icmpv6/TYPE` with a type nftables knows |
@@ -77,6 +78,7 @@ These apply to rules, source NAT and redirects.
| `rule_chain` | error | a chain that isn't `input`, `forward`, `output` or `early` |
| `rule_action` | error | an action that isn't `accept`, `drop`, `reject` or `log` |
| `raw_rule` | error | a raw rule over several lines, or with `;` |
| `raw_conflict` | error | a raw rule with other fields set (zones, addresses, services, limits, log…): it is used as written, so they would be ignored |
| `early_conntrack` | error | connection states, limits or a connection limit in the early chain, which runs before connection tracking |
| `early_reject` | error | reject in the early chain: drop instead |
| `ct_state`, `ct_status` | error | an unknown connection state or status |
@@ -115,3 +117,13 @@ These apply to rules, source NAT and redirects.
| `snat_everything` | warning | no out zones and no sources: everything leaving the firewall is translated |
| `redirect_service` | error | no service, or one without TCP, UDP or SCTP ports |
| `redirect_to_port` | error | the local port isn't one port |
## The machine
These come from housecarld looking at the machine, not at the
configuration: the dashboard and `hc status` show them.
| Code | | When |
|---|---|---|
| `nftables_service` | warning, *host* | Debian's `nftables.service` is enabled: at boot it loads `/etc/nftables.conf`, and stopping or restarting it runs `nft flush ruleset`, deleting Housecarl's table too. Disable it with `systemctl disable nftables` (without `--now`) |
| `takeover_pending` | warning, *host* | the table you imported from is still in force beside Housecarl's, so a packet must get past both; `hc takeover TABLE` deletes it once Housecarl's rules are right |
+3 -3
View File
@@ -39,9 +39,9 @@ type Client struct {
}
// Staged reports whether a request is a configuration change that a
// changeset takes: objects and settings, and the changes that can't be
// staged (undo, import, takeover), which the server then refuses rather
// than apply them. Changesets' own calls and dynamic set entries aren't.
// changeset takes: objects, settings and undo, and the changes that can't
// be staged (import, takeover), which the server then refuses rather than
// apply them. Changesets' own calls and dynamic set entries aren't.
func Staged(method, path string) bool {
if method == http.MethodGet || strings.Contains(path, "/elements") {
return false
+23 -6
View File
@@ -137,6 +137,9 @@ type Edit func(st *model.State, newID func() int64) error
type Options struct {
DryRun bool
Via string // web, cli, api, import
// RevertOf is the history entry the change undoes (a changeset with a
// staged undo), so History links the two.
RevertOf int64
}
// Apply makes a change: edit runs on a copy of the configuration, the
@@ -161,7 +164,7 @@ func (e *Engine) Apply(ctx context.Context, p *auth.Principal, description strin
if idErr != nil {
return nil, idErr
}
return e.commit(ctx, p, description, opts, next, 0)
return e.commit(ctx, p, description, opts, next, opts.RevertOf)
}
// commit validates next and saves it as the new state.
@@ -264,8 +267,22 @@ func (e *Engine) Revert(ctx context.Context, p *auth.Principal, id int64, opts O
}
e.mu.Lock()
defer e.mu.Unlock()
cur := objects(e.state)
next := e.state.Clone()
if err := Undo(next, h); err != nil {
return nil, err
}
return e.commit(ctx, p, UndoDescription(h), opts, next, h.ID)
}
// UndoDescription is an undo's description: "undo 12: create rule …".
func UndoDescription(h *model.HistoryEntry) string {
return fmt.Sprintf("undo %d: %s", h.ID, h.Description)
}
// Undo applies a history entry backwards to st, if every object it
// changed is still as it left it (a change made since would be lost).
func Undo(st *model.State, h *model.HistoryEntry) error {
cur := objects(st)
for i := len(h.Changes) - 1; i >= 0; i-- {
c := h.Changes[i]
now := cur[key{c.Kind, c.ID}]
@@ -274,13 +291,13 @@ func (e *Engine) Revert(ctx context.Context, p *auth.Principal, id int64, opts O
if what == "" {
what = fmt.Sprintf("%s %d", c.Kind, c.ID)
}
return nil, Errorf(KindConflict, "%s %s has changed since entry %d; undo the later changes first, or change it by hand", c.Kind, what, id)
return Errorf(KindConflict, "%s %s has changed since entry %d; undo the later changes first, or change it by hand", c.Kind, what, h.ID)
}
if err := put(next, c.Kind, c.ID, c.Before); err != nil {
return nil, err
if err := put(st, c.Kind, c.ID, c.Before); err != nil {
return err
}
}
return e.commit(ctx, p, fmt.Sprintf("undo %d: %s", h.ID, h.Description), opts, next, h.ID)
return nil
}
// Replace makes the configuration next as a whole (imports).
+5 -2
View File
@@ -39,6 +39,8 @@ type Input struct {
// Result is the imported configuration and what to know about it.
type Result struct {
State *model.State
// Table is the table imported ("inet main").
Table string
Notes []string
// Raw lists the rules that became raw rules.
Raw []string
@@ -206,7 +208,7 @@ func Import(in Input) (*Result, error) {
im.run()
im.nameForwards()
im.st.Renumber()
return &Result{State: im.st, Notes: im.notes, Raw: im.raw}, nil
return &Result{State: im.st, Table: im.table.Family + " " + im.table.Name, Notes: im.notes, Raw: im.raw}, nil
}
func tableList(ts []nftTable) string {
@@ -1464,7 +1466,8 @@ func (im *importer) rawRule(r *model.Rule, p *parsed) {
if r.Action == "" {
r.Action = model.ActionAccept
}
r.Match, r.Raw = model.Match{}, text
// Only the text counts: what was parsed of it goes.
*r = model.Rule{ID: r.ID, Chain: r.Chain, Position: r.Position, Comment: r.Comment, Action: r.Action, Raw: text}
im.st.Rules = append(im.st.Rules, r)
why := strings.Join(p.unsupported, ", ")
if why == "" {
+4 -1
View File
@@ -35,13 +35,16 @@ const (
OpDelete = "delete"
OpMove = "move"
OpSettings = "settings"
// OpRevert undoes a history entry: Data is the entry (its changes),
// ID its number.
OpRevert = "revert"
)
// Op is one change, as the API received it, so it can be stored and
// replayed on whatever the configuration is when it is applied.
type Op struct {
Seq int `json:"seq"`
Op string `json:"op"` // create, update, delete, move, settings
Op string `json:"op"` // create, update, delete, move, settings, revert
Kind string `json:"kind,omitempty"`
// Ref names the object (ID, or name) for update, delete and move.
Ref string `json:"ref,omitempty"`
+18 -3
View File
@@ -36,8 +36,8 @@ func changesetParam(r *http.Request) (int64, error) {
return id, nil
}
// notStaged refuses a changeset on a change that can't be staged (undo,
// import, takeover): it would otherwise be applied at once, unexpectedly.
// notStaged refuses a changeset on a change that can't be staged (an
// import, a takeover): it would otherwise be applied at once, unexpectedly.
func notStaged(r *http.Request, what string) error {
if id, err := changesetParam(r); err != nil || id == 0 {
return err
@@ -102,6 +102,11 @@ func (s *Server) stage(w http.ResponseWriter, r *http.Request, p *auth.Principal
return nil
}
op.Summary = engine.Describe(changes)
if op.Op == model.OpRevert {
var h model.HistoryEntry
json.Unmarshal(op.Data, &h)
op.Summary = engine.UndoDescription(&h)
}
op.AddedAt = s.nowFn()
for _, o := range cs.Ops {
if o.Seq >= op.Seq {
@@ -353,7 +358,17 @@ func (s *Server) applyChangeset(w http.ResponseWriter, r *http.Request, p *auth.
if len(cs.Ops) == 0 {
return badRequest("changeset %d has no changes to apply", id)
}
res, err := s.eng.Apply(r.Context(), p, cs.Description, engine.Options{Via: via(r, p)}, func(st *model.State, _ func() int64) error {
opts := engine.Options{Via: via(r, p)}
desc := cs.Description
for _, o := range cs.Ops {
if o.Op == model.OpRevert {
opts.RevertOf = o.ID // History links the undo
if desc == "" && len(cs.Ops) == 1 {
desc = o.Summary
}
}
}
res, err := s.eng.Apply(r.Context(), p, desc, opts, func(st *model.State, _ func() int64) error {
return replay(st, cs.Ops)
})
if err != nil {
+17 -3
View File
@@ -469,13 +469,27 @@ func (s *Server) revertHistory(w http.ResponseWriter, r *http.Request, p *auth.P
if err := require(p, auth.ResConfig, true); err != nil {
return err
}
if err := notStaged(r, "an undo"); err != nil {
return err
}
id, err := pathID(r, "id")
if err != nil {
return err
}
// Staged, an undo is an operation of its own, reviewed like any.
if csID, err := changesetParam(r); err != nil {
return err
} else if csID != 0 {
h, err := s.store.GetHistory(r.Context(), id)
if errors.Is(err, store.ErrNotFound) {
return notFound("history entry %d not found", id)
}
if err != nil {
return err
}
if h.RevertedBy != 0 {
return conflict("history entry %d was already undone by entry %d", id, h.RevertedBy)
}
data, _ := json.Marshal(h)
return s.stage(w, r, p, csID, model.Op{Op: model.OpRevert, ID: id, Data: data})
}
res, err := s.eng.Revert(r.Context(), p, id, engine.Options{DryRun: queryBool(r, "dry_run"), Via: via(r, p)})
if err != nil {
s.writeErr(w, r, err, res)
+5
View File
@@ -51,6 +51,11 @@ func (s *Server) importNft(w http.ResponseWriter, r *http.Request, p *auth.Princ
return nil
}
s.afterChange(r.Context(), p, er)
if er.Applied {
// Until it is taken over, the old table still applies too: the
// status warns (takeover_pending).
s.store.SetMeta(r.Context(), importedTableKey, res.Table)
}
out.MutationResponse = api.MutationResponse{Applied: er.Applied, DryRun: er.DryRun, HistoryID: er.HistoryID, Changes: er.Changes, Warnings: er.Warnings}
if out.Changes == nil {
out.Changes = []model.Change{}
+36
View File
@@ -1,7 +1,10 @@
package server
import (
"context"
"fmt"
"net/http"
"os"
"sort"
"strings"
"sync"
@@ -44,10 +47,43 @@ func (s *Server) status(w http.ResponseWriter, r *http.Request, p *auth.Principa
if data, err := s.fw.ListRuleset(r.Context()); err == nil {
out.OtherTables = otherTables(data)
}
out.Problems = append(out.Problems, s.hostProblems(r.Context(), out.OtherTables)...)
writeJSON(w, http.StatusOK, out)
return nil
}
const importedTableKey = "imported_table"
// nftablesUnitLinks are where an enabled nftables.service shows (Debian
// enables it for sysinit.target).
var nftablesUnitLinks = []string{
"/etc/systemd/system/sysinit.target.wants/nftables.service",
"/etc/systemd/system/multi-user.target.wants/nftables.service",
}
// hostProblems are warnings about the machine rather than the
// configuration: another ruleset that still loads at boot, or the table
// an import came from, still in force beside Housecarl's.
func (s *Server) hostProblems(ctx context.Context, others []string) []validate.Problem {
var out []validate.Problem
for _, link := range s.unitLinks {
if _, err := os.Lstat(link); err == nil {
out = append(out, validate.Problem{Severity: validate.Warning, Code: "nftables_service", Object: "nftables.service",
Message: "nftables.service is enabled: at boot it loads /etc/nftables.conf, and stopping or restarting it flushes every table, Housecarl's included; disable it (systemctl disable nftables, without --now)"})
break
}
}
if t, err := s.store.GetMeta(ctx, importedTableKey); err == nil && t != "" {
for _, o := range others {
if o == t {
out = append(out, validate.Problem{Severity: validate.Warning, Code: "takeover_pending", Object: "table " + t,
Message: fmt.Sprintf("%s, the table you imported, still applies beside Housecarl's: a packet must get past both; once Housecarl's rules are right, delete it with 'hc takeover %q'", t, t)})
}
}
}
return out
}
// hostInterfaces lists the machine's interfaces with their zones.
func (s *Server) hostInterfaces(st *model.State) []api.HostInterface {
out := []api.HostInterface{}
+6
View File
@@ -31,6 +31,12 @@ func applyOp(st *model.State, op *model.Op) (int64, error) {
return moveOp(st, op)
case model.OpSettings:
return 0, settingsOp(st, op)
case model.OpRevert:
var h model.HistoryEntry
if err := json.Unmarshal(op.Data, &h); err != nil {
return 0, badRequest("revert: %v", err)
}
return 0, engine.Undo(st, &h)
}
return 0, badRequest("unknown operation %q", op.Op)
}
+4 -1
View File
@@ -88,6 +88,9 @@ type Server struct {
probation probation
logs *logRing
// unitLinks are where an enabled nftables.service shows (tests change
// them).
unitLinks []string
// csMu orders changes to changesets (staging, applying, discarding).
csMu sync.Mutex
nowFn func() time.Time
@@ -113,7 +116,7 @@ func New(cfg Config, eng *engine.Engine, fw Firewall, ifaces Interfaces, log *sl
log = slog.Default()
}
s := &Server{cfg: cfg, eng: eng, fw: fw, ifaces: ifaces, store: eng.Store(), log: log, mux: http.NewServeMux(), limiter: newRateLimiter(),
web: web, started: time.Now(), logs: newLogRing(2000), nowFn: time.Now}
web: web, started: time.Now(), logs: newLogRing(2000), nowFn: time.Now, unitLinks: nftablesUnitLinks}
s.routes()
return s
}
+70
View File
@@ -795,3 +795,73 @@ func TestAutoConfirm(t *testing.T) {
t.Errorf("manual confirmation: %+v", mr)
}
}
// TestStagedUndo: an undo staged in a changeset changes nothing until the
// changeset is applied; then History links the two entries.
func TestStagedUndo(t *testing.T) {
e := newEnv(t)
e.srv.probationFor = 5 * time.Second
var mr api.MutationResponse
e.call(e.token, "POST", "/zones", model.Zone{Name: "lan", Interfaces: []string{"eth1"}}, &mr)
e.call(e.token, "POST", "/confirm", nil, nil)
made := mr.HistoryID
var cs model.Changeset
e.call(e.token, "POST", "/changesets", nil, &cs)
q := "?changeset=" + itoa(cs.ID)
if code := e.call(e.token, "POST", "/history/"+itoa(made)+"/revert"+q, nil, &mr); code != 200 || !mr.Staged {
t.Fatalf("staging the undo: %d %+v", code, mr)
}
if op := mr.Changeset.Ops[0]; op.Op != model.OpRevert || !strings.HasPrefix(op.Summary, "undo "+itoa(made)+":") {
t.Errorf("the operation: %+v", op)
}
if len(e.eng.State().Zones) != 1 {
t.Fatal("a staged undo was applied")
}
// Staging it again can't work: the changeset already undoes it.
var eb api.ErrorBody
if code := e.call(e.token, "POST", "/history/"+itoa(made)+"/revert"+q, nil, &eb); code != 409 {
t.Errorf("a second undo: %d %+v", code, eb)
}
if code := e.call(e.token, "POST", "/changesets/"+itoa(cs.ID)+"/apply", nil, &mr); code != 200 || !mr.Applied {
t.Fatalf("apply: %d %+v", code, mr)
}
if len(e.eng.State().Zones) != 0 {
t.Error("the undo wasn't applied")
}
h, _ := e.srv.store.GetHistory(context.Background(), mr.HistoryID)
orig, _ := e.srv.store.GetHistory(context.Background(), made)
if h.RevertOf != made || orig.RevertedBy != h.ID || !strings.HasPrefix(h.Description, "undo ") {
t.Errorf("history: %+v, original %+v", h, orig)
}
}
// TestHostProblems: an enabled nftables.service, and an imported table
// still in force, are warned about.
func TestHostProblems(t *testing.T) {
e := newEnv(t)
dir := t.TempDir()
link := dir + "/nftables.service"
e.srv.unitLinks = []string{link}
codes := func() string {
var st api.Status
e.call(e.token, "GET", "/status", nil, &st)
var out []string
for _, p := range st.Problems {
out = append(out, p.Code)
}
return strings.Join(out, " ")
}
if c := codes(); strings.Contains(c, "nftables_service") || strings.Contains(c, "takeover_pending") {
t.Errorf("warned without cause: %s", c)
}
os.Symlink("/usr/lib/systemd/system/nftables.service", link) // dangling is fine: Lstat
// The fake ruleset lists inet main: imported from it, it still applies.
e.srv.store.SetMeta(context.Background(), importedTableKey, "inet main")
if c := codes(); !strings.Contains(c, "nftables_service") || !strings.Contains(c, "takeover_pending") {
t.Errorf("not warned: %s", c)
}
e.srv.store.SetMeta(context.Background(), importedTableKey, "inet gone")
if c := codes(); strings.Contains(c, "takeover_pending") {
t.Errorf("warned about a table that is gone: %s", c)
}
}
+35
View File
@@ -85,6 +85,35 @@ func Check(st *model.State, env *Env) []Problem {
return c.out
}
// rawIgnores names what a raw rule has besides its text, which the
// ruleset wouldn't use (the action too is the text's; it isn't checked).
func rawIgnores(r *model.Rule) []string {
m := r.Match
var out []string
for _, x := range []struct {
name string
set bool
}{
{"zones", len(m.InZones) > 0 || len(m.OutZones) > 0},
{"addresses", len(m.Sources) > 0 || len(m.Destinations) > 0},
{"services", len(m.Services) > 0},
{"family", m.Family != ""},
{"connection state", len(r.CtState) > 0 || len(r.CtStatus) > 0},
{"time window", len(r.Days) > 0 || r.HourFrom != "" || r.HourTo != ""},
{"rate limit", r.Limit != nil},
{"connection limit", r.ConnLimit != 0},
{"quota", r.Quota != nil},
{"address grouping", r.GroupV4 != 0 || r.GroupV6 != 0},
{"log", r.Log || r.LogPrefix != ""},
{"reject with", r.RejectWith != ""},
} {
if x.set {
out = append(out, x.name)
}
}
return out
}
// maxByteRate is the largest byte rate per period nftables (1.1.3) can
// hold: the kernel's token bucket multiplies the rate by the period.
var maxByteRate = map[string]int64{"second": 4294967294, "minute": 307445734, "hour": 5124095, "day": 213503}
@@ -202,6 +231,9 @@ func (c *checker) sets() {
c.add(Error, "set_entry", obj, "%v", err)
}
}
if len(s.Entries) > 0 && s.Dynamic {
c.add(Error, "set_dynamic_entries", obj, "a dynamic set gets its entries at run time (hc set entries add, the Entries button), each with its timeout; leave out the configured ones, or make it a set that isn't dynamic")
}
if len(s.Entries) == 0 && !s.Dynamic {
c.add(Warning, "set_empty", obj, "the set is empty: rules using it match nothing")
}
@@ -331,6 +363,9 @@ func (c *checker) rule(r *model.Rule) {
if strings.ContainsAny(r.Raw, ";\n\r") {
c.add(Error, "raw_rule", obj, "a raw rule is one nftables rule on one line, without ;")
}
if ignored := rawIgnores(r); len(ignored) > 0 {
c.add(Error, "raw_conflict", obj, "a raw rule is used as written, so its %s would be ignored: write them in the nftables rule, or leave them out", strings.Join(ignored, ", "))
}
return
}
in := r.Chain != model.ChainOutput
+2
View File
@@ -66,6 +66,7 @@ func TestCodes(t *testing.T) {
{"set_entry", func(st *model.State) { st.Sets[0].Entries = []string{"10.0.10.5/24"} }},
{"set_empty", func(st *model.State) { st.Sets[0].Entries = nil }},
{"set_timeout", func(st *model.State) { st.Sets[0].Timeout = 60 }},
{"set_dynamic_entries", func(st *model.State) { st.Sets[0].Dynamic = true }},
{"service_empty", func(st *model.State) { st.Services[0].Entries = nil }},
{"service_entry", func(st *model.State) { st.Services[0].Entries = []string{"tcp/99999"} }},
{"unknown_zone", func(st *model.State) { st.Rules[1].InZones = []string{"dmz"} }},
@@ -83,6 +84,7 @@ func TestCodes(t *testing.T) {
{"rule_chain", func(st *model.State) { rule(st).Chain = "prerouting" }},
{"rule_action", func(st *model.State) { rule(st).Action = "allow" }},
{"raw_rule", func(st *model.State) { rule(st).Raw = "tcp dport 22 accept; udp dport 53 accept" }},
{"raw_conflict", func(st *model.State) { rule(st).Raw = "tcp dport 22 accept" }}, // it still has sources and services
{"early_conntrack", func(st *model.State) { rule(st).Chain = model.ChainEarly; rule(st).CtState = []string{"new"} }},
{"early_reject", func(st *model.State) { rule(st).Chain = model.ChainEarly; rule(st).Action = model.ActionReject }},
{"ct_state", func(st *model.State) { rule(st).CtState = []string{"fresh"} }},
+4 -4
View File
@@ -1,6 +1,6 @@
// History: every configuration change, with undo.
import { h, mount, table, time, confirmDialog, modal, errorBox, badge } from "../ui.js";
import { get, change, can } from "../api.js";
import { get, change, can, state } from "../api.js";
import { rerender } from "../app.js";
export async function render(el, params) {
@@ -20,10 +20,10 @@ export async function render(el, params) {
}
async function undo(e) {
if (!await confirmDialog("Undo change #" + e.id, "Undo “" + e.description + "”? This is recorded as a new change, which can be undone too.", "Undo")) return;
if (!await confirmDialog("Undo change #" + e.id, "Undo “" + e.description + "”? This is recorded as a new change, which can be undone too." + (!state.immediate ? " It is added to your pending changes, to review and apply." : ""), "Undo")) return;
try {
// An undo can't be staged: it is applied at once.
await change("POST", "/history/" + e.id + "/revert", undefined, { immediate: true });
// Like any edit: staged for review, or applied at once in live mode.
await change("POST", "/history/" + e.id + "/revert");
rerender();
} catch (err) { modal("Can't undo", errorBox(err)); }
}
+87 -14
View File
@@ -31,17 +31,14 @@ export async function render(el, params, query) {
const write = can("config", true);
const info = chains.find((c) => c[0] === chain) || chains[0];
const policy = settings[chain + "_policy"];
const bi = (builtins[chain] || []).filter(([flag]) => flag === "" || settings.builtins[flag]);
const actionBadge = (r) => badge(r.action + (r.log ? " + log" : ""), { accept: "ok", drop: "err", reject: "warn", log: "" }[r.action]);
const actionBadge = (r) => r.raw ? badge("as written", "") : badge(r.action + (r.log ? " + log" : ""), { accept: "ok", drop: "err", reject: "warn", log: "" }[r.action]);
mount(el,
h("div", { class: "page-head" },
h("div", { class: "grow" }, h("h1", null, "Rules"), h("div", { class: "muted" }, "Checked top to bottom; the first rule that decides wins. ", info[2])),
write ? h("div", { class: "actions" }, h("button", { class: "primary", onclick: () => ruleDialog(null, chain) }, "Add rule")) : null),
h("div", { class: "tabs" }, chains.map(([k, label]) => h("a", { href: "#/rules?chain=" + k, class: k === chain ? "active" : "" }, label))),
h("div", { class: "panel" },
bi.length ? h("div", { class: "builtins muted small" }, h("strong", null, "Added by Housecarl first: "),
bi.map(([, mark, text], i) => [i ? " · " : "", text, mark ? [" (", hits(cs, "b-" + mark), ")"] : ""]), " — ",
h("a", { href: "#/settings" }, "settings")) : null,
builtinsRow(chain, settings, cs),
table([
{ label: "#", render: (r) => r.pending === "deleted" ? "" : String(live(rules).indexOf(r) + 1) },
{ label: "Match", render: (r) => r.raw ? [badge("raw", ""), " ", h("span", { class: "mono small" }, r.raw)]
@@ -73,6 +70,63 @@ function countsTraffic(r) {
return !r.raw && (!!r.quota || !!(r.limit && ["bytes", "kbytes", "mbytes"].includes(r.limit.unit)));
}
// What each built-in does (a switch covers all its rules).
const builtinText = {
loopback: "Accept everything on lo (the firewall talking to itself)",
established: "Accept established and related connections: replies, and the rest of connections a rule let in",
drop_invalid: "Drop invalid packets: what connection tracking can't place",
icmpv6: "Accept the ICMPv6 that IPv6 needs: errors, echo, neighbour discovery, MLD (RFC 4890)",
ping_internal: "Accept ping to the firewall from internal zones",
dhcpv6_client: "Accept DHCPv6 replies on external zones (your ISP's prefix delegation)",
antispoof: "Drop sources that can't come from outside, on external zones (Settings: the lists and your prefixes)",
mss_clamp: "Clamp TCP's MSS to the route's MTU on external zones (needed with PPPoE)",
};
// builtinsRow sums up the chain's built-ins in one line, with a button to
// see and switch them.
function builtinsRow(chain, settings, cs) {
const list = builtins[chain] || [];
const on = list.filter(([flag]) => flag === "" || settings.builtins[flag]);
if (!list.length) return null;
return h("div", { class: "builtins muted small" },
on.length ? (on.length === 1 ? "Housecarl adds 1 rule before yours. " : "Housecarl adds " + on.length + " rules before yours. ") : "Housecarl adds no rules before yours here. ",
h("button", { class: "small", onclick: () => builtinsDialog(chain, settings, cs) }, "Built-ins…"));
}
// builtinsDialog lists the chain's built-ins: what each does, its hits,
// and a switch (a setting, so it covers every chain the built-in is in).
async function builtinsDialog(chain, settings, cs) {
const list = builtins[chain] || [];
const flags = [...new Set(list.map(([flag]) => flag))];
const write = can("config", true);
const boxes = {};
const rows = flags.map((flag) => {
const marks = list.filter(([f]) => f === flag).map(([, mark]) => mark).filter(Boolean);
const box = flag ? h("input", { type: "checkbox", checked: settings.builtins[flag], disabled: !write }) : null;
if (box) boxes[flag] = box;
return { flag, marks, box, text: flag ? builtinText[flag] : list.find(([f]) => f === flag)[2] };
});
const err = h("div");
const body = h("div", null,
h("p", { class: "muted" }, "Checked in this order, before your rules (rules with a quota or a byte rate come before the established built-in, so they see every packet). Each is a setting: switching one changes every chain it is in."),
table([
{ label: "On", render: (x) => x.box || h("span", { class: "muted small" }, "always") },
{ label: "Built-in", render: (x) => x.flag ? x.text : [x.text, h("div", { class: "muted small" }, "Part of each port forward.")] },
{ label: "Hits", render: (x) => x.marks.length ? hits(cs, ...x.marks.map((m) => "b-" + m)) : h("span", { class: "muted" }, "–") },
], rows),
err);
await modal("Built-ins of the " + chain + " chain", body, write ? [
{ label: "Cancel", value: null },
{ label: "Save", kind: "primary", onclick: async () => {
mount(err);
const b = {};
for (const [flag, box] of Object.entries(boxes)) if (box.checked !== settings.builtins[flag]) b[flag] = box.checked;
if (!Object.keys(b).length) return true;
try { await change("PATCH", "/settings", { builtins: b }); rerender(); return true; } catch (e) { mount(err, errorBox(e)); return false; }
} },
] : [{ label: "Close", value: null }], { wide: true });
}
function extras(r, q) {
const x = [];
if ((r.ct_state || []).length) x.push("state " + r.ct_state.join(","));
@@ -149,7 +203,7 @@ async function del(r) {
export async function ruleDialog(r, chain) {
const n = await names();
r = r || { chain, action: "accept" };
const ch = select([["input", "input (to the firewall)"], ["forward", "forward (through it)"], ["output", "output (from it)"], ["early", "early (before conntrack)"]], r.chain || chain);
const ch = select(["input", "forward", "output", "early"], r.chain || chain);
const match = matchEditor(r, n);
const action = select([["accept", "accept"], ["drop", "drop"], ["reject", "reject"], ["log", "log only (and go on)"]], r.action);
const rejectWith = select([["", "with an ICMP error"], ["tcp-reset", "with a TCP reset (TCP rules only)"]], r.reject_with || "");
@@ -205,6 +259,7 @@ export async function ruleDialog(r, chain) {
sec.open = on;
for (const n of nodes) n.classList.toggle("hidden", !on);
add.classList.toggle("hidden", on);
if (sec.onShow) sec.onShow(on);
if (on) { const f = nodes.find((n) => n.querySelector && n.querySelector("input, select, textarea")); if (f) f.querySelector("input, select, textarea").focus(); }
updateGroup();
};
@@ -234,7 +289,7 @@ export async function ruleDialog(r, chain) {
field("Connection limit", conn, "Match a source that already has more than this many connections."),
]);
const rawFields = section("raw", "Write it in nftables", !!r.raw, [
field("Raw nftables", raw, "Instead of the matches above: one nftables rule, for what Housecarl can't say. Checked by nftables."),
field("Raw nftables", raw, "One nftables rule, for what Housecarl can't say, used as written: it replaces the zones, addresses, services, action, limits and log (write them in it). Checked by nftables."),
]);
// Grouping only means something with a per-address limit, quota or
// connection limit.
@@ -248,14 +303,30 @@ export async function ruleDialog(r, chain) {
limitKey.addEventListener("change", updateGroup);
quotaKey.addEventListener("change", updateGroup);
// Written in nftables, a rule is its text: the fields it replaces go
// (and the other parts close).
const matchNodes = match.flat().filter(Boolean);
const actionField = field("Action", h("div", null, action, " ", rejectWith)).filter(Boolean);
const logField = field("Log", h("div", null, log, logPrefix)).filter(Boolean);
const addersLabel = h("label");
sections.raw.onShow = (on) => {
for (const n of [...matchNodes, ...actionField, ...logField]) n.classList.toggle("hidden", on);
for (const [k, sec] of Object.entries(sections)) {
if (k === "raw") continue;
if (on && sec.open) for (const n of sec.nodes) n.classList.add("hidden");
if (on) sec.open = false;
}
for (const b of adders.children) if (!b.textContent.includes("nftables")) b.classList.toggle("hidden", on);
updateGroup();
};
const body = h("div", null,
h("div", { class: "form-grid" },
field("Chain", ch),
...match.flat(),
field("Action", h("div", null, action, " ", rejectWith)),
...matchNodes,
...actionField,
...stateFields, ...timeFields, ...rateFields, ...quotaFields, ...connFields, ...groupFields, ...rawFields,
h("label"), adders,
field("Log", h("div", null, log, logPrefix)),
addersLabel, adders,
...logField,
field("Comment", comment),
r.id ? [h("label"), disabled] : null),
err);
@@ -270,7 +341,9 @@ export async function ruleDialog(r, chain) {
async function save(dry) {
mount(err);
const on = (k) => sections[k].open;
const v = { chain: ch.value, ...match.value(), action: action.value, reject_with: action.value === "reject" ? rejectWith.value : "",
const isRaw = on("raw");
const m = isRaw ? { in_zones: [], not_in: false, out_zones: [], not_out: false, sources: [], not_sources: false, destinations: [], not_destinations: false, services: [], family: "" } : match.value();
const v = { chain: ch.value, ...m, action: isRaw ? "accept" : action.value, reject_with: !isRaw && action.value === "reject" ? rejectWith.value : "",
ct_state: on("state") ? states.value() : [], ct_status: on("state") ? statuses.value() : [],
days: on("time") ? days.value() : [], hour_from: on("time") ? hourFrom.value.trim() : "", hour_to: on("time") ? hourTo.value.trim() : "",
limit: on("rate") && rate.value ? { rate: parseInt(rate.value, 10), unit: unit.value, per: per.value, burst: parseInt(burst.value || "0", 10), over: over.input.checked,
@@ -278,8 +351,8 @@ export async function ruleDialog(r, chain) {
quota: on("quota") && quotaAmount.value ? { bytes: parseInt(quotaAmount.value, 10) * (quotaUnit.value === "gb" ? 1 << 30 : 1 << 20), over: quotaOver.input.checked,
per_source: quotaKey.value === "source", per_destination: quotaKey.value === "destination",
reset: quotaReset.value, day: quotaReset.value === "monthly" ? parseInt(quotaDay.value || "1", 10) : 0 } : null,
group_v4: parseInt(group4.value || "0", 10), group_v6: parseInt(group6.value || "0", 10),
conn_limit: on("conn") ? parseInt(conn.value || "0", 10) : 0, log: log.input.checked, log_prefix: logPrefix.value.trim(),
group_v4: isRaw ? 0 : parseInt(group4.value || "0", 10), group_v6: isRaw ? 0 : parseInt(group6.value || "0", 10),
conn_limit: on("conn") ? parseInt(conn.value || "0", 10) : 0, log: !isRaw && log.input.checked, log_prefix: isRaw ? "" : logPrefix.value.trim(),
raw: on("raw") ? raw.value.trim() : "", comment: comment.value.trim() };
if (r.id) v.disabled = disabled.input.checked;
try {
+13 -3
View File
@@ -31,19 +31,29 @@ async function setDialog(s) {
s = s || { entries: [] };
const name = input({ value: s.name || "", placeholder: "trusted" });
const list = listEditor(s.entries, { placeholder: "192.168.1.0/24, 2001:db8:1::/64, 192.168.1.100-192.168.1.199", addLabel: "Add entry" });
const dynamic = checkbox("dynamic: entries are added at run time", s.dynamic);
const dynamic = checkbox("dynamic: entries are added at run time (the Entries button, hc set entries, a program with a token), each for a time", s.dynamic);
// A dynamic set has no configured entries; only it has a timeout.
const entryFields = field("Entries", list).filter(Boolean);
const timeoutFields = field("Entry timeout", timeout, "How long an entry stays unless it is added with its own. Empty: until removed (or a reboot).").filter(Boolean);
const showKind = () => {
for (const n of entryFields) n.classList.toggle("hidden", dynamic.input.checked);
for (const n of timeoutFields) n.classList.toggle("hidden", !dynamic.input.checked);
};
dynamic.input.addEventListener("change", showKind);
showKind();
const timeout = input({ value: s.timeout ? fmtSeconds(s.timeout) : "", placeholder: "1h, 1d (none: until removed)", class: "short" });
const comment = input({ value: s.comment || "" });
const err = h("div");
await modal(s.id ? "Edit set " + s.name : "Add set", h("div", null, h("div", { class: "form-grid" },
field("Name", name), field("Entries", list), h("label"), dynamic, field("Entry timeout", timeout, "A dynamic set's default."), field("Comment", comment)), err), [
field("Name", name), ...entryFields, h("label"), dynamic, ...timeoutFields, field("Comment", comment)), err), [
{ label: "Cancel", value: null },
{ label: s.id ? "Save" : "Add set", kind: "primary", onclick: async () => {
mount(err);
const t = parseSeconds(timeout.value);
if (t === null) { mount(err, h("div", { class: "msg err" }, "Write the timeout as 1h, 30m or 1d.")); return false; }
try {
await change(s.id ? "PATCH" : "POST", s.id ? "/sets/" + s.id : "/sets", { name: name.value.trim(), entries: list.value(), dynamic: dynamic.input.checked, timeout: t, comment: comment.value.trim() });
await change(s.id ? "PATCH" : "POST", s.id ? "/sets/" + s.id : "/sets", { name: name.value.trim(), entries: dynamic.input.checked ? [] : list.value(), dynamic: dynamic.input.checked,
timeout: dynamic.input.checked ? t : 0, comment: comment.value.trim() });
rerender();
return true;
} catch (e) { mount(err, errorBox(e)); return false; }
+6 -1
View File
@@ -240,9 +240,14 @@ contains "^jellyfin$"
run "a stale HOUSECARL_CHANGESET is refused" 1 CE zone add stale -i stale0
contains "is applied"
CS2=$(H changeset create -q)
run "an undo can't be staged" 6 H -C "$CS2" history undo 1
UNDO=$(H zone add undome -i undo0 | sed -n 's/.*history \([0-9]*\);.*/\1/p')
run "an undo is staged" 0 H -C "$CS2" history undo "$UNDO"
contains "staged in changeset $CS2"
run "not applied yet: the zone is there" 0 H zone show undome
run "an import can't be staged" 6 sh -c "echo '{\"nftables\":[]}' | docker exec -i ${P}-router /w/hc -C $CS2 import -"
contains "can't be staged"
run "discarded" 0 H changeset discard "$CS2"
run "the zone stays" 0 H zone show undome
run "a dry run shows the ruleset's diff" 0 H -n set add e2e_dry 10.1.2.3
contains "^+.*set e2e_dry_v4"