From e4efdf9c78cff7506022ad180820665775230269 Mon Sep 17 00:00:00 2001 From: abrown38 Date: Mon, 19 Oct 2026 13:23:28 +0100 Subject: [PATCH] Confirm changes automatically from the client that made them A change made over the network answers with a one-time confirm_token and closes its connection (HTTP/2: a GOAWAY). The web UI and hc send the token straight back to /confirm, which accepts it only for the newest change, from the same user and API token, the same address, and over a new connection: proof that the client can still reach the API through the new ruleset (an established connection would get through anyway). A change that cuts its client off can't be confirmed so, and rolls back by itself. The setting manual_confirm (hc settings set --auto-confirm off) and hc --no-confirm leave confirming to a person. Changes through the local socket still need no confirmation. e2e: 136 checks, among them a lockout change rolled back without anyone confirming. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 10 ++- cmd/hc/firewall.go | 22 ++++- cmd/hc/main.go | 49 ++++++++--- docs/api.md | 8 +- docs/cli.md | 11 +-- docs/design.md | 13 +++ docs/operations.md | 20 ++++- internal/api/types.go | 12 +++ internal/model/settings.go | 5 ++ internal/server/h_changesets.go | 1 + internal/server/h_config.go | 24 ++++-- internal/server/h_import.go | 9 +- internal/server/probation.go | 87 +++++++++++++++++++- internal/server/server_test.go | 73 ++++++++++++++++ internal/web/static/assets/api.js | 19 +++-- internal/web/static/assets/app.js | 31 ++++++- internal/web/static/assets/pages/import.js | 7 +- internal/web/static/assets/pages/settings.js | 6 +- scripts/e2e.sh | 27 +++++- 19 files changed, 384 insertions(+), 50 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8c287af..f3033f6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -55,7 +55,11 @@ the way Jarl and Steward are. own table, `inet housecarl`, and nothing else. Dynamic sets' entries and the counters carry across. - **Probation:** a change made over the network rolls itself back unless - confirmed within two minutes (`hc confirm`, a banner in the web UI); + confirmed within two minutes. The web UI and `hc` confirm their own + changes once they reach Housecarl again over a new connection, from the + same address (a change that cuts its client off can't, and rolls back); + `--no-confirm` and the "a person confirms each change" setting leave it + to `hc confirm` or the banner; changes through the local socket are kept at once; a restart rolls back what was pending. No rule is forced to keep SSH or the web UI open. @@ -75,7 +79,9 @@ the way Jarl and Steward are. verdict maps are expanded into one port forward or rule per element. - **The `hc` CLI, the REST API and the web UI** (dashboard, rules per chain with hits and reordering, NAT, zones, sets, services, the log, - settings, import, history with undo), with users, roles, scoped API + 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 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 diff --git a/cmd/hc/firewall.go b/cmd/hc/firewall.go index e7a75a1..7982141 100644 --- a/cmd/hc/firewall.go +++ b/cmd/hc/firewall.go @@ -125,10 +125,15 @@ func settingsCmd() *cobra.Command { fmt.Fprintf(w, "Antispoof IPv6:\t%s\n", list(s.AntispoofV6)) fmt.Fprintf(w, "Flowtable:\t%s\n", list(s.FlowtableDevices)) fmt.Fprintf(w, "Probation:\t%s\n", fmtSeconds(s.Probation())) + if s.ManualConfirm { + fmt.Fprintln(w, "Confirmation:\tby a person ('hc confirm', the web UI's banner)") + } else { + fmt.Fprintln(w, "Confirmation:\tautomatic, by the client that made the change, once it reaches Housecarl again over a new connection") + } }) }, }) - var input, forward, output, logDrops, probation string + var input, forward, output, logDrops, probation, autoConf string var builtins, own, flowtable, spoof4, spoof6 []string set := &cobra.Command{ Use: "set [flags]", Short: "Change settings (only the flags given)", Args: cobra.NoArgs, @@ -172,6 +177,13 @@ dhcpv6_client, antispoof, mss_clamp. p[map[string]string{"own-prefix": "own_prefixes", "flowtable": "flowtable_devices", "antispoof-v4": "antispoof_v4", "antispoof-v6": "antispoof_v6"}[flag]] = v } } + if fl.Changed("auto-confirm") { + on, err := onOff("auto-confirm", autoConf) + if err != nil { + return err + } + p["manual_confirm"] = !on + } if fl.Changed("probation") { n, err := parseSeconds(probation) if err != nil { @@ -193,6 +205,7 @@ dhcpv6_client, antispoof, mss_clamp. f.StringArrayVar(&spoof4, "antispoof-v4", nil, "IPv4 sources dropped from the internet (replaces)") f.StringArrayVar(&spoof6, "antispoof-v6", nil, "IPv6 sources dropped from the internet (replaces)") f.StringVar(&probation, "probation", "", "how long a network change has to be confirmed (2m)") + f.StringVar(&autoConf, "auto-confirm", "", "on: the client that made a change confirms it once it reaches Housecarl again over a new connection; off: a person confirms") cmd.AddCommand(set) return cmd } @@ -510,6 +523,13 @@ come back. Don't forget the boot: disable nftables.service, or empty return err } fmt.Println(out["message"]) + if tok, _ := out["confirm_token"].(string); tok != "" && !flagNoConfirm { + if err := autoConfirm(tok); err != nil { + fmt.Printf("couldn't confirm it (%v): confirm with 'hc confirm', or the tables come back\n", err) + } else { + fmt.Println("confirmed: this client reached Housecarl again over a new connection") + } + } return nil }, } diff --git a/cmd/hc/main.go b/cmd/hc/main.go index 4d58623..6b8b906 100644 --- a/cmd/hc/main.go +++ b/cmd/hc/main.go @@ -37,14 +37,18 @@ func main() { // Global flags. var ( - flagServer string - flagToken string - flagConfig string - flagDryRun bool - flagOutput string - flagInsecure bool - flagCACert string - flagQuiet bool + flagServer string + flagToken string + flagConfig string + flagDryRun bool + flagOutput string + flagInsecure bool + flagCACert string + flagQuiet bool + flagNoConfirm bool + // lastClient is the client the command made (to confirm a change by + // itself after it). + lastClient *client.Client ) // DefaultSocket is housecarld's local socket. @@ -135,6 +139,7 @@ func newClient() (*client.Client, error) { if err := setupChangeset(c); err != nil { return nil, err } + lastClient = c return c, nil } @@ -151,9 +156,10 @@ Changes are made at once; to review several and apply them together, stage them in a changeset ('hc changeset --help', -C ID or HOUSECARL_CHANGESET). -A change made over the network is on probation: confirm it with 'hc -confirm' (within 2 minutes by default) or it is rolled back by itself, so a -change that cuts you off undoes itself. On the firewall itself, root uses +A change made over the network is on probation: hc confirms it by itself +once it reaches Housecarl again over a new connection, and a change that +cuts you off can't be confirmed, so it is rolled back by itself (within 2 +minutes by default). --no-confirm leaves it to 'hc confirm'. On the firewall itself, root uses housecarld's local socket: no token, no probation. From elsewhere, set the server and an API token once: @@ -175,6 +181,7 @@ Exit codes: 0 ok, 1 error, 3 not authenticated or not allowed, 4 not found, pf.BoolVar(&flagInsecure, "insecure", false, "don't verify the server's TLS certificate") pf.StringVar(&flagCACert, "ca-cert", "", "PEM file with a CA to trust for the server's certificate") pf.BoolVarP(&flagQuiet, "quiet", "q", false, "print only errors (and IDs of new objects)") + pf.BoolVar(&flagNoConfirm, "no-confirm", false, "leave a change on probation instead of confirming it once this client reaches Housecarl again") pf.Int64VarP(&flagChangeset, "changeset", "C", 0, "stage configuration changes in this changeset instead of applying them (env HOUSECARL_CHANGESET)") root.AddGroup(&cobra.Group{ID: "fw", Title: "Firewall:"}, &cobra.Group{ID: "changes", Title: "Changes:"}, &cobra.Group{ID: "admin", Title: "Administration:"}) add := func(group string, cmds ...*cobra.Command) { @@ -299,6 +306,19 @@ func exitCode(err error) int { return 1 } +// autoConfirm confirms a change with its token: the request goes over a +// new connection (the server closed the change's), so it gets through +// only if the new ruleset still lets this client reach Housecarl. +func autoConfirm(token string) error { + if lastClient == nil { + return errors.New("no client") + } + c, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + var out map[string]int + return lastClient.Do(c, "POST", "/confirm", nil, api.ConfirmInput{Token: token}, &out) +} + // report prints the outcome of a change. func report(resp *api.MutationResponse) error { switch flagOutput { @@ -343,6 +363,13 @@ func report(resp *api.MutationResponse) error { fmt.Fprintln(w) } fmt.Fprintln(w, "dry run: nothing was changed") + case resp.Applied && resp.ConfirmBy != nil && resp.ConfirmToken != "" && !flagNoConfirm: + if err := autoConfirm(resp.ConfirmToken); err != nil { + fmt.Fprintf(w, "applied on probation (history %d), but couldn't confirm it: %v\nIt is rolled back within %s unless confirmed ('hc confirm')\n", + resp.HistoryID, err, time.Until(*resp.ConfirmBy).Round(time.Second)) + } else if !flagQuiet { + fmt.Fprintf(w, "applied and confirmed (history %d): this client reached Housecarl again over a new connection\n", resp.HistoryID) + } case resp.Applied && resp.ConfirmBy != nil: fmt.Fprintf(w, "applied on probation (history %d): confirm with 'hc confirm' within %s, or it is rolled back\n", resp.HistoryID, time.Until(*resp.ConfirmBy).Round(time.Second)) diff --git a/docs/api.md b/docs/api.md index c56a89f..97d8881 100644 --- a/docs/api.md +++ b/docs/api.md @@ -51,6 +51,10 @@ Calls that change the configuration answer: * `confirm_by`: the change is on probation, and is rolled back at that time unless `POST /confirm` comes first. Missing for changes made through the socket. +* `confirm_token`: the client that made the change can confirm it by + itself with `POST /confirm {"token"}`, from the same address, as the + same user and token, over a new connection (the answer closes this + one). Missing when a person must confirm (`manual_confirm`). * `?dry_run=1` checks the change, with nftables too, and returns what it would do, with `"applied": false, "dry_run": true`, and `diff`: the ruleset's unified diff. @@ -89,7 +93,7 @@ Creating answers 201, other changes 200. "builtins": {"loopback": true, "established": true, "drop_invalid": true, "icmpv6": true, "ping_internal": true, "dhcpv6_client": true, "antispoof": true, "mss_clamp": false}, "own_prefixes": ["2001:db8::/48"], "antispoof_v4": ["10.0.0.0/8", "..."], "antispoof_v6": ["fc00::/7", "..."], - "flowtable_devices": ["wan", "eth1"], "probation_seconds": 120} + "flowtable_devices": ["wan", "eth1"], "probation_seconds": 120, "manual_confirm": false} ``` Lists replace the old ones. `probation_seconds` 0 is the default (120). @@ -180,7 +184,7 @@ history, but are audited. | | | |---|---| | `GET /probation` | `{"pending": [history IDs], "takeovers": N, "deadline", "seconds_left"}` | -| `POST /confirm` | keep them: `{"confirmed": N}`; the ruleset is saved for the boot | +| `POST /confirm` | keep them: `{"confirmed": N}`; the ruleset is saved for the boot. With `{"token"}`, the client confirming its own change (see `confirm_token`): 409 if a newer change is pending, or over the change's own connection; 403 from another user or address | | `POST /rollback` | undo them now | ### History, import, takeover diff --git a/docs/cli.md b/docs/cli.md index 8dbe118..ec1c982 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -28,6 +28,7 @@ command and its flags. | | `HOUSECARL_SOCKET` | the local socket's path | | `--insecure`, `--ca-cert FILE` | | TLS: skip the check, or trust another CA | | `-n, --dry-run` | | check and show a change without making it, with the ruleset's diff | +| `--no-confirm` | | leave a change on probation instead of confirming it once `hc` reaches Housecarl again | | `-C, --changeset ID` | `HOUSECARL_CHANGESET` (checked, see [Changesets](#changesets)) | stage configuration changes in this changeset instead of making them | | `-o, --output FORMAT` | | `table` (default), `json`, `yaml` or `id` | | `-q, --quiet` | | print only errors (and the IDs of new objects) | @@ -41,14 +42,14 @@ server error. Every change is checked as a whole, then by nftables, before it is put in force. A refused change says why, with the problem's code ([validation.md](validation.md)). An applied one says its history entry -and any new warnings. Over the network, it is on probation: +and any new warnings. Over the network, it is on probation, and `hc` +confirms it by reaching Housecarl again over a new connection +([operations.md](operations.md#changes-on-probation)): ``` $ hc rule add input --from trusted --service ssh --comment "SSH from home" created rule input #31 (from trusted · ssh) -applied on probation (history 12): confirm with 'hc confirm' within 2m0s, or it is rolled back -$ hc confirm -confirmed 1 change; it is kept and loaded at boot +applied and confirmed (history 12): this client reached Housecarl again over a new connection ``` Through the socket the change is kept at once: @@ -203,7 +204,7 @@ See [nat.md](nat.md). | Command | | |---|---| | `settings show` | | -| `settings set [--input-policy\|--forward-policy\|--output-policy accept\|drop] [--log-drops on\|off] [--builtin NAME=on\|off]... [--own-prefix P]... [--antispoof-v4 P]... [--antispoof-v6 P]... [--flowtable DEV]... [--probation 2m]` | lists given replace the old ones | +| `settings set [--input-policy\|--forward-policy\|--output-policy accept\|drop] [--log-drops on\|off] [--builtin NAME=on\|off]... [--own-prefix P]... [--antispoof-v4 P]... [--antispoof-v6 P]... [--flowtable DEV]... [--probation 2m] [--auto-confirm on\|off]` | lists given replace the old ones | ### probation, history, import diff --git a/docs/design.md b/docs/design.md index deb1101..79ba092 100644 --- a/docs/design.md +++ b/docs/design.md @@ -107,6 +107,19 @@ confirmed within two minutes (Settings; `hc confirm`, the banner in the web UI, `POST /confirm`), it is rolled back by itself, and the rollback is recorded in the history. A change that cuts you off undoes itself. +* **The client that made a change confirms it by itself** once it + reaches the API again: the change's answer carries a one-time token and + closes its connection (HTTP/2: a GOAWAY), and the web UI and `hc` send + the token straight back. That proves something only on a new + connection, since the built-in that accepts established connections + would let the old one through whatever the change did. So the token is + accepted only from the same user (and API token), the same address, a + different connection, and for the newest change. A change that cuts its + client off can't be confirmed by it, and rolls back. This proves the + API is still reachable, not that SSH or the LAN still work; with the + API, you can fix those. Settings → "a person confirms each change" + (`hc settings set --auto-confirm off`) goes back to confirming by hand; + `hc --no-confirm` leaves one change waiting. * Changes made through the local socket (`sudo hc …` on the firewall) need no confirmation: whoever makes them is already on the machine. One made while others are pending joins them. diff --git a/docs/operations.md b/docs/operations.md index 5442c9b..2530c93 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -28,7 +28,25 @@ edits) applies each edit at once instead. With hc, see ## Changes on probation Changes made over the network roll back unless confirmed -([design.md](design.md#not-locking-yourself-out)): +([design.md](design.md#not-locking-yourself-out)). The web UI and `hc` +confirm their own changes once they reach Housecarl again over a new +connection, so normally nothing waits: + +``` +$ hc rule add input --from admins --service ssh +created rule input #31 +applied and confirmed (history 12): this client reached Housecarl again over a new connection +``` + +A change that cuts the client off can't be confirmed that way: + +``` +applied on probation (history 13), but couldn't confirm it: … context deadline exceeded +It is rolled back within 1m50s unless confirmed ('hc confirm') +``` + +By hand (with `--no-confirm`, or with Settings → "a person confirms each +change"): ```sh hc status # Probation: 2 changes roll back in 97s unless confirmed diff --git a/internal/api/types.go b/internal/api/types.go index 5e1da25..d52a146 100644 --- a/internal/api/types.go +++ b/internal/api/types.go @@ -66,6 +66,11 @@ type MutationResponse struct { // ConfirmBy is set when the change is on probation: unless confirmed // (POST /confirm) by then, it is rolled back. ConfirmBy *time.Time `json:"confirm_by,omitempty"` + // ConfirmToken lets the client that made the change confirm it by + // itself (POST /confirm {"token"}): from the same address, over a new + // connection, which proves it can still reach the API. The answer + // closes its connection, so the next request opens a new one. + ConfirmToken string `json:"confirm_token,omitempty"` // Diff is the ruleset's unified diff, for a dry run. Diff string `json:"diff,omitempty"` // Staged: the change was added to a changeset (Changeset), not @@ -318,3 +323,10 @@ type QuotaAddress struct { Address string `json:"address"` Used int64 `json:"used"` } + +// ConfirmInput confirms the changes on probation. With Token (a change's +// confirm_token), it is the client that made the change confirming it by +// itself; without, a person's confirmation. +type ConfirmInput struct { + Token string `json:"token,omitempty"` +} diff --git a/internal/model/settings.go b/internal/model/settings.go index 3f0aedb..6eca9d8 100644 --- a/internal/model/settings.go +++ b/internal/model/settings.go @@ -31,6 +31,11 @@ type Settings struct { // ProbationSeconds is how long a change made over the network has to // be confirmed before it is rolled back (0: the default). ProbationSeconds int `json:"probation_seconds,omitempty"` + // ManualConfirm: changes made over the network wait for a person to + // confirm them. Otherwise the client that made a change confirms it by + // itself once it reaches the API again over a new connection, from the + // same address: proof that it can still manage the firewall. + ManualConfirm bool `json:"manual_confirm,omitempty"` } // Builtins are rules Housecarl adds itself, shown as "added by Housecarl". diff --git a/internal/server/h_changesets.go b/internal/server/h_changesets.go index 01ed1f3..7c12c39 100644 --- a/internal/server/h_changesets.go +++ b/internal/server/h_changesets.go @@ -368,6 +368,7 @@ func (s *Server) applyChangeset(w http.ResponseWriter, r *http.Request, p *auth. s.eng.Audit(r.Context(), p, "changeset.apply", fmt.Sprintf("changeset %d", id), "success", fmt.Sprintf("history %d", res.HistoryID)) s.afterChange(r.Context(), p, res) out := s.mutation(res, nil) + out.ConfirmToken = s.issueCheck(w, r, p, res) out.Changeset = cs writeJSON(w, http.StatusOK, out) return nil diff --git a/internal/server/h_config.go b/internal/server/h_config.go index 12ea0bf..5462e97 100644 --- a/internal/server/h_config.go +++ b/internal/server/h_config.go @@ -48,7 +48,9 @@ func (s *Server) change(w http.ResponseWriter, r *http.Request, p *auth.Principa return nil } s.afterChange(r.Context(), p, res) - writeJSON(w, s.mutationStatus(res, op.Op == model.OpCreate), s.mutation(res, objectOf(&op, id))) + out := s.mutation(res, objectOf(&op, id)) + out.ConfirmToken = s.issueCheck(w, r, p, res) + writeJSON(w, s.mutationStatus(res, op.Op == model.OpCreate), out) return nil } @@ -83,8 +85,10 @@ func (s *Server) mutationStatus(res *engine.Result, creates bool) int { } // respondChange answers a change made without an operation (undo). -func (s *Server) respondChange(w http.ResponseWriter, r *http.Request, res *engine.Result, object func(st *model.State) any, creates bool) { - writeJSON(w, s.mutationStatus(res, creates), s.mutation(res, object)) +func (s *Server) respondChange(w http.ResponseWriter, r *http.Request, p *auth.Principal, res *engine.Result, object func(st *model.State) any, creates bool) { + out := s.mutation(res, object) + out.ConfirmToken = s.issueCheck(w, r, p, res) + writeJSON(w, s.mutationStatus(res, creates), out) } // rulesetDiff is the unified diff of the rulesets two configurations @@ -478,7 +482,7 @@ func (s *Server) revertHistory(w http.ResponseWriter, r *http.Request, p *auth.P return nil } s.afterChange(r.Context(), p, res) - s.respondChange(w, r, res, nil, false) + s.respondChange(w, r, p, res, nil, false) return nil } @@ -496,7 +500,17 @@ func (s *Server) confirmChanges(w http.ResponseWriter, r *http.Request, p *auth. if err := require(p, auth.ResConfig, true); err != nil { return err } - n, err := s.confirm(r.Context(), p) + var in api.ConfirmInput + if r.ContentLength != 0 { + if err := decodeJSON(r, &in); err != nil { + return err + } + } + confirm := func() (int, error) { return s.confirm(r.Context(), p) } + if in.Token != "" { + confirm = func() (int, error) { return s.confirmToken(r, p, in.Token) } + } + n, err := confirm() if err != nil { return err } diff --git a/internal/server/h_import.go b/internal/server/h_import.go index b1043b0..30a0ba6 100644 --- a/internal/server/h_import.go +++ b/internal/server/h_import.go @@ -58,6 +58,7 @@ func (s *Server) importNft(w http.ResponseWriter, r *http.Request, p *auth.Princ if pr := s.probationStatus(); er.Applied && len(pr.Pending) > 0 { out.ConfirmBy = &pr.Deadline } + out.ConfirmToken = s.issueCheck(w, r, p, er) writeJSON(w, http.StatusOK, out) return nil } @@ -148,6 +149,12 @@ func (s *Server) takeover(w http.ResponseWriter, r *http.Request, p *auth.Princi } else if pr := s.probationStatus(); pr.Takeovers > 0 { msg += fmt.Sprintf("; confirm with 'hc confirm' within %ds, or they come back", pr.SecondsLeft) } - writeJSON(w, http.StatusOK, map[string]any{"deleted": done, "message": msg}) + answer := map[string]any{"deleted": done, "message": msg} + if !queryBool(r, "dry_run") && len(done) > 0 { + if tok := s.issueCheck(w, r, p, nil); tok != "" { + answer["confirm_token"] = tok + } + } + writeJSON(w, http.StatusOK, answer) return nil } diff --git a/internal/server/probation.go b/internal/server/probation.go index a0b768f..b0ed816 100644 --- a/internal/server/probation.go +++ b/internal/server/probation.go @@ -2,8 +2,10 @@ package server import ( "context" + "crypto/subtle" "encoding/json" "fmt" + "net/http" "sync" "time" @@ -31,6 +33,21 @@ type probation struct { timer *time.Timer // restores are nft scripts that bring back tables a takeover deleted. restores []string + // check is the latest change's confirmation token (reachCheck): only + // it can confirm, since the ruleset in force is the one it tests. + check *reachCheck +} + +// reachCheck is what a client must show to confirm its change by itself: +// the token, from the same user (and API token), the same address, and a +// new connection (another remote address and port than the change's). +type reachCheck struct { + token string + userID int64 + username string + tokenID int64 + ip string + remote string } // system is the principal of rollbacks. @@ -88,6 +105,60 @@ func (s *Server) startProbation(ctx context.Context, p *auth.Principal, historyI s.saveProbation(ctx) } +// issueCheck gives the client that made a network change on probation a +// token to confirm it by itself, unless the settings want a person to +// confirm. The answer then closes its connection (HTTP/2: a GOAWAY), so +// the confirmation has to open a new one through the new ruleset; an +// established connection would get through whatever the change did. +func (s *Server) issueCheck(w http.ResponseWriter, r *http.Request, p *auth.Principal, res *engine.Result) string { + if p == nil || p.Method == "socket" || (res != nil && !res.Applied) || s.eng.State().Settings.ManualConfirm { + return "" + } + pr := &s.probation + pr.mu.Lock() + defer pr.mu.Unlock() + if len(pr.pending) == 0 && len(pr.restores) == 0 { + return "" + } + c := &reachCheck{token: auth.RandomString(24), username: p.Name(), ip: s.clientIP(r), remote: r.RemoteAddr} + if p.User != nil { + c.userID = p.User.ID + } + if p.Token != nil { + c.tokenID = p.Token.ID + } + pr.check = c + w.Header().Set("Connection", "close") + return c.token +} + +// confirmToken confirms with a change's token: the proof that the client +// that made it still reaches the API, from where it made it. +func (s *Server) confirmToken(r *http.Request, p *auth.Principal, token string) (int, error) { + pr := &s.probation + pr.mu.Lock() + c := pr.check + pending := len(pr.pending) + len(pr.restores) + pr.mu.Unlock() + var tokenID int64 + if p.Token != nil { + tokenID = p.Token.ID + } + switch { + case pending == 0: + return 0, nil + case c == nil || subtle.ConstantTimeCompare([]byte(c.token), []byte(token)) != 1: + return 0, conflict("this confirmation is for an earlier change: a newer one is on probation, and only its own client (or a person) can confirm it") + case p.User == nil || p.User.ID != c.userID || p.Name() != c.username || tokenID != c.tokenID: + return 0, forbidden("only the client that made the change can confirm it this way") + case s.clientIP(r) != c.ip: + return 0, forbidden("the confirmation comes from %s, the change from %s: it must come from the same place", s.clientIP(r), c.ip) + case r.RemoteAddr == c.remote: + return 0, conflict("the confirmation came over the change's own connection, which proves nothing; it must open a new one") + } + return s.confirmWith(r.Context(), p, fmt.Sprintf("confirmed automatically: %s reached the API again over a new connection", c.ip)) +} + func (s *Server) saveBoot() { if err := s.fw.SaveBoot(s.eng.State()); err != nil { s.log.Error("saving the ruleset for the next boot failed", "err", err) @@ -110,8 +181,12 @@ func (s *Server) probationStatus() api.Probation { return out } -// confirm makes the pending changes permanent. +// confirm makes the pending changes permanent (a person's confirmation). func (s *Server) confirm(ctx context.Context, p *auth.Principal) (int, error) { + return s.confirmWith(ctx, p, "") +} + +func (s *Server) confirmWith(ctx context.Context, p *auth.Principal, how string) (int, error) { pr := &s.probation pr.mu.Lock() defer pr.mu.Unlock() @@ -122,10 +197,14 @@ func (s *Server) confirm(ctx context.Context, p *auth.Principal) (int, error) { if pr.timer != nil { pr.timer.Stop() } - pr.pending, pr.restores, pr.deadline, pr.timer = nil, nil, time.Time{}, nil + pr.pending, pr.restores, pr.deadline, pr.timer, pr.check = nil, nil, time.Time{}, nil, nil s.saveProbation(ctx) s.saveBoot() - s.eng.Audit(ctx, p, "config.confirm", "", "success", fmt.Sprintf("%d changes confirmed", n)) + detail := fmt.Sprintf("%d changes confirmed", n) + if how != "" { + detail += "; " + how + } + s.eng.Audit(ctx, p, "config.confirm", "", "success", detail) return n, nil } @@ -136,7 +215,7 @@ func (s *Server) rollback(why string) { pr.mu.Lock() defer pr.mu.Unlock() pending, restores := pr.pending, pr.restores - pr.pending, pr.restores, pr.deadline, pr.timer = nil, nil, time.Time{}, nil + pr.pending, pr.restores, pr.deadline, pr.timer, pr.check = nil, nil, time.Time{}, nil, nil for i := len(restores) - 1; i >= 0; i-- { if err := s.fw.ApplyScript(ctx, restores[i]); err != nil { s.log.Error("bringing back a table a takeover deleted failed", "err", err) diff --git a/internal/server/server_test.go b/internal/server/server_test.go index 5f5d6e7..e769804 100644 --- a/internal/server/server_test.go +++ b/internal/server/server_test.go @@ -722,3 +722,76 @@ func TestQuotaResets(t *testing.T) { t.Errorf("reset by hand: %d %v", code, e.fw.resets) } } + +// TestAutoConfirm: the client that made a change confirms it by itself: +// with its token, from the same address, over a new connection. Nobody +// else can, an older token can't, and with manual_confirm there is none. +func TestAutoConfirm(t *testing.T) { + e := newEnv(t) + e.srv.probationFor = 5 * time.Second + post := func(token, path string, body any) (*http.Response, []byte) { + b, _ := json.Marshal(body) + req, _ := http.NewRequest("POST", e.ts.URL+api.Prefix+path, bytes.NewReader(b)) + req.Header.Set("Authorization", "Bearer "+token) + req.Header.Set("Content-Type", "application/json") + resp, err := http.DefaultClient.Do(req) + if err != nil { + t.Fatal(err) + } + defer resp.Body.Close() + out, _ := io.ReadAll(resp.Body) + return resp, out + } + resp, body := post(e.token, "/zones", model.Zone{Name: "lan", Interfaces: []string{"eth1"}}) + var mr api.MutationResponse + json.Unmarshal(body, &mr) + if mr.ConfirmToken == "" || !resp.Close { + t.Fatalf("no token, or the connection stays open: %+v close=%v", mr, resp.Close) + } + first := mr.ConfirmToken + // A newer change: only its token confirms now. + _, body = post(e.token, "/zones", model.Zone{Name: "dmz", Interfaces: []string{"eth2"}}) + json.Unmarshal(body, &mr) + if resp, _ := post(e.token, "/confirm", api.ConfirmInput{Token: first}); resp.StatusCode != 409 { + t.Errorf("an older token: %d", resp.StatusCode) + } + // Another user can't use it. + ctx := context.Background() + other := &model.User{Username: "sam", Grants: auth.Roles["operator"]} + e.srv.store.CreateUser(ctx, other) + otok, pid, sh := auth.NewToken() + e.srv.store.CreateToken(ctx, &model.Token{UserID: other.ID, Name: "t", PublicID: pid, SecretHash: sh}) + if resp, _ := post(otok, "/confirm", api.ConfirmInput{Token: mr.ConfirmToken}); resp.StatusCode != 403 { + t.Errorf("another user's confirmation: %d", resp.StatusCode) + } + // Not over the change's own connection, nor from elsewhere. + c := e.srv.probation.check + p := &auth.Principal{User: &model.User{ID: c.userID, Username: c.username}, Method: "token", Token: &model.Token{ID: c.tokenID}} + same := httptest.NewRequest("POST", "/", nil) + same.RemoteAddr = c.remote + if _, err := e.srv.confirmToken(same, p, c.token); err == nil || !strings.Contains(err.Error(), "new one") { + t.Errorf("over the same connection: %v", err) + } + elsewhere := httptest.NewRequest("POST", "/", nil) + elsewhere.RemoteAddr = "192.0.2.9:4444" + if _, err := e.srv.confirmToken(elsewhere, p, c.token); err == nil || !strings.Contains(err.Error(), "same place") { + t.Errorf("from elsewhere: %v", err) + } + // The client itself, over a new connection: confirmed. + resp, body = post(e.token, "/confirm", api.ConfirmInput{Token: mr.ConfirmToken}) + if resp.StatusCode != 200 || !strings.Contains(string(body), `"confirmed":2`) { + t.Fatalf("auto-confirm: %d %s", resp.StatusCode, body) + } + if pr := e.srv.probationStatus(); len(pr.Pending) != 0 { + t.Errorf("still pending: %+v", pr) + } + // manual_confirm: no token; a person confirms. + e.call(e.token, "PATCH", "/settings", map[string]any{"manual_confirm": true}, nil) + e.call(e.token, "POST", "/confirm", nil, nil) + _, body = post(e.token, "/zones", model.Zone{Name: "guest", Interfaces: []string{"eth3"}}) + mr = api.MutationResponse{} + json.Unmarshal(body, &mr) + if mr.ConfirmToken != "" || mr.ConfirmBy == nil { + t.Errorf("manual confirmation: %+v", mr) + } +} diff --git a/internal/web/static/assets/api.js b/internal/web/static/assets/api.js index e4d302b..427b5fa 100644 --- a/internal/web/static/assets/api.js +++ b/internal/web/static/assets/api.js @@ -22,11 +22,13 @@ export class APIError extends Error { let onUnauthorized = () => {}; export function setUnauthorizedHandler(fn) { onUnauthorized = fn; } -// onChange is told about every applied change (the probation banner). +// onChange is told about every applied change (the probation banner), with +// the answer (its confirm_token, to confirm it by itself). let onChange = () => {}; export function setChangeHandler(fn) { onChange = fn; } -// api calls the REST API. opts: {body, query, stage (a changeset ID)}. +// api calls the REST API. opts: {body, query, stage (a changeset ID), +// timeout (ms)}. export async function api(method, path, opts = {}) { const headers = { "Accept": "application/json" }; if (opts.stage) headers["X-Housecarl-Changeset"] = String(opts.stage); @@ -43,7 +45,13 @@ export async function api(method, path, opts = {}) { const s = q.toString(); if (s) url += (url.includes("?") ? "&" : "?") + s; } - const resp = await fetch(url, { method, headers, body, credentials: "same-origin" }); + let signal; + if (opts.timeout) { + const ac = new AbortController(); + setTimeout(() => ac.abort(), opts.timeout); + signal = ac.signal; + } + const resp = await fetch(url, { method, headers, body, credentials: "same-origin", signal }); if (resp.status === 204) return null; const ct = resp.headers.get("Content-Type") || ""; const data = ct.includes("application/json") ? await resp.json() : await resp.text(); @@ -173,11 +181,12 @@ export function report(r) { const n = ((r.changeset || {}).ops || []).length; toast("Staged: " + ((r.changeset.ops || [])[n - 1] || {}).summary + " — " + (n === 1 ? "1 pending change" : n + " pending changes") + " to review and apply"); } else if (r.dry_run) toast("Checked by Housecarl and nftables: nothing was changed"); - else if (r.applied && r.confirm_by) toast("Applied on probation (history #" + r.history_id + "): confirm it in the banner", "warn", 6000); + else if (r.applied && r.confirm_by && !r.confirm_token) toast("Applied on probation (history #" + r.history_id + "): confirm it in the banner", "warn", 6000); + else if (r.applied && r.confirm_by) toast("Applied (history #" + r.history_id + ")"); else if (r.applied) toast("Applied and saved (history #" + r.history_id + ")"); else if (!(r.changes || []).length) toast("Nothing to change"); for (const w of r.warnings || []) toast(w.message + (w.object ? " (" + w.object + ")" : ""), "warn", 10000); - if (r.applied) onChange(); + if (r.applied) onChange(r); } // can reports whether the user may read (or write) a resource. diff --git a/internal/web/static/assets/app.js b/internal/web/static/assets/app.js index ab466c4..7b1bfbf 100644 --- a/internal/web/static/assets/app.js +++ b/internal/web/static/assets/app.js @@ -158,10 +158,11 @@ export async function applyDraft(btn, d, description) { try { const r = await api("POST", "/changesets/" + d.id + "/apply", { body: { description: description || "" } }); useDraft(null); - if (r.confirm_by) toast("Applied changeset #" + d.id + " on probation (history #" + r.history_id + "): confirm it in the banner", "warn", 8000); + if (r.confirm_by && !r.confirm_token) toast("Applied changeset #" + d.id + " on probation (history #" + r.history_id + "): confirm it in the banner", "warn", 8000); else toast("Applied changeset #" + d.id + " (history #" + r.history_id + ")"); for (const w of r.warnings || []) toast(w.message + (w.object ? " (" + w.object + ")" : ""), "warn", 10000); - await refreshProbation(); + if (r.confirm_token) await autoConfirm(r.confirm_token); + else await refreshProbation(); return r; } catch (e) { btn.disabled = false; @@ -176,7 +177,25 @@ export async function applyDraft(btn, d, description) { // ---- probation: changes made here roll back unless confirmed -let probationTimer = null, deadline = 0, pending = 0; +let probationTimer = null, deadline = 0, pending = 0, checking = false; + +// autoConfirm confirms a change with its token. The server closed the +// change's connection, so this request opens a new one through the new +// ruleset: it only gets through if this browser can still reach +// Housecarl. If it can't, the change rolls back by itself. +export async function autoConfirm(token) { + checking = true; + await refreshProbation(); + try { + await api("POST", "/confirm", { body: { token }, timeout: 10000 }); + checking = false; + toast("Confirmed automatically: this browser reached Housecarl again over a new connection"); + } catch (e) { + checking = false; + toast("Couldn't confirm the change (" + (e.name === "AbortError" ? "no answer over a new connection" : e.message) + "): unless it is confirmed, it rolls back by itself", "warn", 10000); + } + await refreshProbation(); +} // refreshProbation asks whether changes wait for confirmation, and shows // the banner with a countdown while they do. @@ -204,6 +223,10 @@ function drawBanner() { } bannerEl.classList.remove("hidden"); const write = can("config", true); + if (checking) { + mount(bannerEl, h("strong", null, "Checking you can still reach Housecarl…"), " The change is confirmed once this browser gets through over a new connection."); + return; + } mount(bannerEl, h("strong", null, pending === 1 ? "1 change is on probation" : pending + " changes are on probation"), " — rolled back in ", h("span", { class: "mono" }, left + "s"), " unless you confirm them. If this page stops answering, wait: the firewall puts itself back.", @@ -303,7 +326,7 @@ export function rerender() { if (current.page) route(); } -setChangeHandler(() => refreshProbation()); +setChangeHandler((r) => r && r.confirm_token ? autoConfirm(r.confirm_token) : refreshProbation()); setUnauthorizedHandler(() => { if (state.me) { diff --git a/internal/web/static/assets/pages/import.js b/internal/web/static/assets/pages/import.js index f58a04e..7a5b935 100644 --- a/internal/web/static/assets/pages/import.js +++ b/internal/web/static/assets/pages/import.js @@ -1,7 +1,7 @@ // Import: an existing nftables ruleset as Housecarl's configuration. import { h, mount, field, textarea, input, checkbox, errorBox, copyBox } from "../ui.js"; import { api } from "../api.js"; -import { refreshProbation } from "../app.js"; +import { refreshProbation, autoConfirm } from "../app.js"; export async function render(el) { const json = textarea("", { rows: 6, class: "mono", placeholder: "the output of: sudo nft -j list ruleset" }); @@ -21,12 +21,13 @@ export async function render(el) { mount(out, h("div", { class: "msg " + (dry ? "info" : "ok") }, dry ? "Would import: " : "Imported: ", `${st.zones.length} zones, ${st.sets.length} sets, ${st.services.length} services, ${st.rules.length} rules, ${st.forwards.length} port forwards, ${st.snats.length} source NAT rules, ${st.redirects.length} redirects.`, - r.confirm_by ? " It is on probation: confirm it in the banner." : ""), + r.confirm_by && !r.confirm_token ? " It is on probation: confirm it in the banner." : ""), r.notes.length ? h("div", { class: "panel" }, h("h2", null, "Notes"), h("ul", null, r.notes.map((n) => h("li", null, n)))) : null, r.raw.length ? h("div", { class: "panel" }, h("h2", null, "Kept as raw rules"), h("ul", { class: "mono small" }, r.raw.map((n) => h("li", null, n)))) : null, !dry ? h("p", null, "Check the ", h("a", { href: "#/rules" }, "rules"), " and ", h("a", { href: "#/nat" }, "NAT"), "; both rulesets apply until the old table goes: ", copyBox("sudo hc takeover \"inet main\"")) : null); - if (!dry) refreshProbation(); + if (!dry && r.confirm_token) autoConfirm(r.confirm_token); + else if (!dry) refreshProbation(); } catch (e) { mount(out, errorBox(e)); } }; mount(el, diff --git a/internal/web/static/assets/pages/settings.js b/internal/web/static/assets/pages/settings.js index 5439bbf..37fe453 100644 --- a/internal/web/static/assets/pages/settings.js +++ b/internal/web/static/assets/pages/settings.js @@ -27,6 +27,7 @@ export async function render(el) { const v6 = listEditor(s.antispoof_v6, { addLabel: "Add" }); const flow = listEditor(s.flowtable_devices, { suggestions: ifs.map((i) => i.name), placeholder: "wan", addLabel: "Add device" }); const probation = input({ value: fmtSeconds(s.probation_seconds || 120), class: "short" }); + const manual = checkbox("a person confirms each change (otherwise the browser or hc that made it confirms it by itself, once it reaches Housecarl again over a new connection)", s.manual_confirm); const err = h("div"); const save = async (dry) => { mount(err); @@ -37,7 +38,7 @@ export async function render(el) { try { const r = await change("PATCH", "/settings", { input_policy: input_.value, forward_policy: forward.value, output_policy: output.value, log_policy_drops: logDrops.input.checked, builtins, own_prefixes: own.value(), antispoof_v4: v4.value(), antispoof_v6: v6.value(), - flowtable_devices: flow.value(), probation_seconds: p === 120 ? 0 : p }, { query: dry ? { dry_run: 1 } : undefined }); + flowtable_devices: flow.value(), probation_seconds: p === 120 ? 0 : p, manual_confirm: manual.input.checked }, { query: dry ? { dry_run: 1 } : undefined }); if (!dry && (r.applied || r.staged)) rerender(); } catch (e) { mount(err, errorBox(e)); } }; @@ -54,7 +55,8 @@ export async function render(el) { field("Antispoof IPv4", v4), field("Antispoof IPv6", v6), field("Flowtable", flow, "Forwarded TCP and UDP between these devices skip the rules once established (fast path). Exact names; none: off."), - field("Probation", probation, "How long a change made here has to be confirmed before it is rolled back.")), + field("Probation", probation, "How long a change made over the network has to be confirmed before it is rolled back."), + h("label"), manual), err, write ? h("div", { class: "actions form-actions" }, h("button", { class: "primary", onclick: () => save(false) }, "Save"), diff --git a/scripts/e2e.sh b/scripts/e2e.sh index e484286..11b80a3 100755 --- a/scripts/e2e.sh +++ b/scripts/e2e.sh @@ -172,11 +172,11 @@ run "probation of 10 seconds" 0 H settings set --probation 10s TOKEN=$(R /w/housecarld -c /w/housecarl.yaml user token admin e2e 2>/dev/null | tail -1) C() { docker exec ${P}-client /w/hc --config /w/cli.yaml --server http://$LAN4:8070 --token "$TOKEN" "$@"; } run "the API from the LAN" 0 C whoami -run "a change over the network is on probation" 0 C zone add guests --interface guest0 +run "a change over the network is on probation" 0 C --no-confirm zone add guests --interface guest0 contains "applied on probation" run "confirmed" 0 C confirm contains "confirmed 1 change;" -run "a change that cuts the client off" 0 C rule disable "$APIRULE" +run "a change that cuts the client off" 0 C --no-confirm rule disable "$APIRULE" contains "on probation" run "the client is cut off (a timeout)" 124 docker exec ${P}-client timeout 3 /w/hc --config /w/cli.yaml --server http://$LAN4:8070 --token "$TOKEN" whoami sleep 12 @@ -185,6 +185,25 @@ run "the rule is enabled again" 0 H rule show "$APIRULE" not_contains "disabled" run "the rollback is in the history" 0 H history list contains "rollback" +# Confirmed by the client itself: it reached the API over a new connection. +run "a change confirmed by its client" 0 C zone add guests2 --interface guest2 +contains "applied and confirmed" +run "nothing waits" 0 C status +not_contains "Probation:" +run "the confirmation is audited" 0 H audit --action config.confirm +contains "confirmed automatically" +# A change that cuts its client off can't be confirmed by it: rolled back. +run "a lockout change, not confirmed" 0 C rule disable "$APIRULE" +contains "couldn't confirm it" +sleep 3 +run "rolled back: the client is back" 0 C whoami +run "the rule is enabled again" 0 H rule show "$APIRULE" +not_contains "disabled" +run "a person must confirm, by setting" 0 H settings set --auto-confirm off +run "then a change waits" 0 C zone add guests3 --interface guest3 +contains "applied on probation" +run "confirmed by a person" 0 C confirm +run "back to automatic" 0 H settings set --auto-confirm on # ---- changesets: staged, reviewed, applied together @@ -209,8 +228,8 @@ run "remove the forward operation" 0 CE changeset remove 3 contains "has 2 operations" run "the diff without it" 0 CE changeset diff not_contains "dnat ip to 10.88.10.57" -run "applied together" 0 CE changeset apply -contains "applied on probation" +run "applied together, and confirmed by this client" 0 CE changeset apply +contains "applied and confirmed" contains "HOUSECARL_CHANGESET still names changeset $CS" run "one history entry" 0 H history list contains "e2e batch"