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 <noreply@anthropic.com>
This commit is contained in:
1 parent
fc390eb57c
commit
e4efdf9c78
19 files changed
+384
-50
No files matched your search
@@ -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
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
@@ -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
|
||||
}
|
||||
@@ -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)
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user