From 8e79403c2163e0a3078229708b5c5b7cf14387df Mon Sep 17 00:00:00 2001 From: jochen Date: Sat, 10 Oct 2026 01:55:58 +0200 Subject: [PATCH 1/2] Show how far the setuid search got, and start a fresh one when the terminal asks (hq issue 361) The walk kept its progress every minute and nobody could read it: root-free said only that a search ran. The account verdict now carries the progress (counts, times, mount points; never a path) as fields and in its reason, outside what makes a verdict changed. A signed ask from the controller's terminal throws the last search away and starts a full one, after the operator removed by hand what it found. --- cmd/mesh-host/main.go | 36 +++- cmd/mesh-host/units_test.go | 22 +++ internal/accounts/accounts.go | 36 ++-- internal/accounts/setuid_cache.go | 180 +++++++++++++++++-- internal/accounts/setuid_progress_test.go | 205 ++++++++++++++++++++++ internal/accounts/setuid_walk.go | 23 +++ internal/link/handover.go | 118 ++++++++++--- internal/link/hearing_nats.go | 26 ++- internal/link/messages.go | 25 +++ internal/link/queue.go | 7 +- internal/link/run.go | 4 + internal/link/setuid_search_test.go | 60 +++++++ 12 files changed, 687 insertions(+), 55 deletions(-) create mode 100644 internal/accounts/setuid_progress_test.go create mode 100644 internal/link/setuid_search_test.go diff --git a/cmd/mesh-host/main.go b/cmd/mesh-host/main.go index d675fb0..98ff07d 100644 --- a/cmd/mesh-host/main.go +++ b/cmd/mesh-host/main.go @@ -1199,8 +1199,15 @@ func runLink(ctx context.Context, opts options) error { // And the operator's hand-over of a directory used as found, carried by the controller from the // terminal (novox/hq issue 356): judged and recorded here, applied by the next apply. HandOver: handOverFrom(opts.state), - Say: say, - Timeout: opts.timeout, + // A fresh search for setuid programs, asked at the controller's terminal (novox/hq issue 361). + SetuidSearch: func(by string) (string, error) { + if setuidSearch == nil { + return "", errors.New("this engine judges no account for root, so it keeps no search for setuid programs") + } + return setuidSearch.SearchNow(by), nil + }, + Say: say, + Timeout: opts.timeout, } worker := make(chan struct{}) go func() { @@ -1668,11 +1675,36 @@ func withAccounts(h *link.Health, as *accounts.Statement) *link.Health { if v.Root { rh.Root = declaration.RootNever } + // How far the setuid search got, while the verdict waits for it (novox/hq issue 361): in the reason the + // root-free verb repeats, and as fields. Never in the journal's line, which says a change alone. + if v.Search != nil { + rh.Reason += "; " + v.Search.Words() + rh.Search = searchAsReported(v.Search) + } h.Resources = append(h.Resources, rh) } return h } +// searchAsReported is a search's progress as the statement carries it. +func searchAsReported(sp *accounts.SearchProgress) *link.SetuidSearch { + out := &link.SetuidSearch{Started: sp.Started.UTC(), Filesystems: sp.Filesystems, Done: sp.Done, + Walking: sp.Walking, Visited: sp.Visited, Found: sp.Found} + if !sp.Kept.IsZero() { + kept := sp.Kept.UTC() + out.Kept = &kept + } + for _, f := range sp.Finished { + sf := link.SearchedFilesystem{Mount: f.Mount} + if f.Took >= 0 { + secs := int64(f.Took.Round(time.Second) / time.Second) + sf.TookSeconds = &secs + } + out.Finished = append(out.Finished, sf) + } + return out +} + // userOf is the account whose own service manager a unit is in (novox/hq ADR 0254), empty for the // machine's own manager. func userOf(scope, user string) string { diff --git a/cmd/mesh-host/units_test.go b/cmd/mesh-host/units_test.go index d679d22..b7c5e37 100644 --- a/cmd/mesh-host/units_test.go +++ b/cmd/mesh-host/units_test.go @@ -98,3 +98,25 @@ func TestTheStatementSaysAnAccountJudgedForRoot(t *testing.T) { t.Fatalf("root said: %v", got) } } + +// A verdict waiting for the setuid search says how far it got, in the reason the root-free verb repeats and as +// fields (novox/hq issue 361); a mount point, never a path the walk visited or found. +func TestTheStatementSaysHowFarTheSetuidSearchGot(t *testing.T) { + at := time.Date(2026, 10, 10, 1, 40, 0, 0, time.UTC) + sp := &accounts.SearchProgress{Started: at, Filesystems: 3, Done: 1, Walking: "/home", Visited: 42, Found: 2, + Kept: at, Finished: []accounts.SearchedFilesystem{{Mount: "/", Took: 90 * time.Second}}} + as := &accounts.Statement{At: at, Accounts: []accounts.Verdict{{Account: accounts.Account{Module: "claude-code", + ID: "claude-code.agent", Name: "agent", Root: true}, State: accounts.Unknown, + Reason: accounts.ReasonPending + ": the search for setuid programs, started 2026-10-10 01:27, has not finished yet", + Since: at, Search: sp}}} + h := withAccounts(healthAsReported(liveness.Statement{At: at}, nil), as) + r := h.Resources[0] + if !strings.HasPrefix(r.Reason, accounts.ReasonPending) || !strings.Contains(r.Reason, "1 of 3 filesystems walked, walking /home, 42 entries visited") { + t.Fatalf("the reason: %s", r.Reason) + } + s := r.Search + if s == nil || s.Filesystems != 3 || s.Done != 1 || s.Walking != "/home" || s.Visited != 42 || s.Found != 2 || + s.Kept == nil || len(s.Finished) != 1 || s.Finished[0].Mount != "/" || *s.Finished[0].TookSeconds != 90 { + t.Fatalf("the fields: %+v", s) + } +} diff --git a/internal/accounts/accounts.go b/internal/accounts/accounts.go index 5e9cf4f..f72169e 100644 --- a/internal/accounts/accounts.go +++ b/internal/accounts/accounts.go @@ -153,6 +153,10 @@ type Verdict struct { Reason string Since time.Time Streak int + // Search is how far the setuid search got, while the verdict waits for it (novox/hq issue 361); nil + // otherwise. Never part of Reason: it moves every minute, and a verdict that changed every minute would + // be said in the journal every minute. + Search *SearchProgress } // Statement is one look at every account. @@ -219,14 +223,14 @@ func (j *Judge) Look(ctx context.Context) (Statement, bool) { st := Statement{At: now} changed := len(j.last.Accounts) != len(j.accounts) for _, a := range j.accounts { - state, reason := j.judge(ctx, a) + state, reason, search := j.judge(ctx, a) k := j.kept[a.ID] if k == nil || k.State != state || k.Reason != reason { changed = true k = &Verdict{Account: a, State: state, Reason: reason, Since: now} j.kept[a.ID] = k } - k.Account = a + k.Account, k.Search = a, search if state == Unhealthy { k.Streak++ } else { @@ -238,8 +242,8 @@ func (j *Judge) Look(ctx context.Context) (Statement, bool) { return st, changed } -// judge is one account's verdict on one look. -func (j *Judge) judge(ctx context.Context, a Account) (string, string) { +// judge is one account's verdict on one look, and how far the setuid search got while it waits for one. +func (j *Judge) judge(ctx context.Context, a Account) (string, string, *SearchProgress) { if a.Root { ways, err := j.reader.Escalation(ctx, a.Name, a.Secrets) var notJudged *SetuidNotJudged @@ -250,22 +254,26 @@ func (j *Judge) judge(ctx context.Context, a Account) (string, string) { if errors.As(err, ¬Judged) { reason += "; " + firstLine(err.Error()) } - return Unhealthy, reason + var progress *SearchProgress + if notJudged != nil { + progress = notJudged.Progress + } + return Unhealthy, reason, progress case errors.As(err, ¬Judged): // The setuid search has no result yet: not judged, said as such — the same words on every look // until it has one, never a search that stalls the look (novox/hq ADR 0266). - return Unknown, firstLine(err.Error()) + return Unknown, firstLine(err.Error()), notJudged.Progress case err != nil: - return Unknown, "whether the account can become root could not be read: " + firstLine(err.Error()) + return Unknown, "whether the account can become root could not be read: " + firstLine(err.Error()), nil } if len(a.Groups) == 0 { // Declared for root alone: nothing of a session to read. - return Healthy, "" + return Healthy, "", nil } } in, err := j.reader.InDatabase(ctx, a.Name) if err != nil { - return Unknown, "the user database could not be read: " + firstLine(err.Error()) + return Unknown, "the user database could not be read: " + firstLine(err.Error()), nil } listed := map[string]bool{} for _, g := range in { @@ -281,15 +289,15 @@ func (j *Judge) judge(ctx context.Context, a Account) (string, string) { } if len(missing) > 0 { return Unhealthy, fmt.Sprintf("not in the group %s: the apply has not put %s there; its outcome says why", - strings.Join(missing, ", "), a.Name) + strings.Join(missing, ", "), a.Name), nil } s, err := j.reader.Session(ctx, a.Name, inDB) if err != nil { - return Unknown, "the account's own service manager could not be read: " + firstLine(err.Error()) + return Unknown, "the account's own service manager could not be read: " + firstLine(err.Error()), nil } if !s.Running { // Nobody is logged in, and the account does not linger: the next login takes every group. - return Healthy, "" + return Healthy, "", nil } var lacking []string for _, g := range inDB { @@ -299,9 +307,9 @@ func (j *Judge) judge(ctx context.Context, a Account) (string, string) { } if len(lacking) > 0 { return Unhealthy, fmt.Sprintf("%s: %s is in the group %s, and its running session began before it was; "+ - "log out of every session and in again, or reboot", ReasonRelogin, a.Name, strings.Join(lacking, ", ")) + "log out of every session and in again, or reboot", ReasonRelogin, a.Name, strings.Join(lacking, ", ")), nil } - return Healthy, "" + return Healthy, "", nil } func firstLine(s string) string { diff --git a/internal/accounts/setuid_cache.go b/internal/accounts/setuid_cache.go index 3e189a0..7125a3d 100644 --- a/internal/accounts/setuid_cache.go +++ b/internal/accounts/setuid_cache.go @@ -10,6 +10,7 @@ import ( "os" "path/filepath" "slices" + "strings" "sync" "syscall" "time" @@ -55,11 +56,14 @@ type SetuidCache struct { loaded bool last *setuidResult // the last complete search unfinished *SetuidProgress // a walk that did not finish, to resume + progress *SearchProgress // how far the running search got, as of its last checkpoint running bool - startedAt time.Time // when the search running, or the last one, was started (a resumed one: its start) - attemptAt time.Time // when the running or last search was started or resumed - endedAt time.Time // when the last search ended, on the same clock - again bool // asked to search again since the running or last one started + gen int // counts SearchNow: a search started under an older one is thrown away + cancel context.CancelFunc // ends the running search's walk + startedAt time.Time // when the search running, or the last one, was started (a resumed one: its start) + attemptAt time.Time // when the running or last search was started or resumed + endedAt time.Time // when the last search ended, on the same clock + again bool // asked to search again since the running or last one started failedAt time.Time failed error backoff time.Duration @@ -101,6 +105,84 @@ type SetuidNotJudged struct { Err error // Last is when the last complete search started, when there is one too old to judge. Last time.Time + // Progress is how far the running search got, as of its last checkpoint: nil when none runs. + Progress *SearchProgress +} + +// SearchProgress is how far a running search got, as of its last checkpoint, for anybody to read (novox/hq +// issue 361): counts, times and mount points only. **Never a path the walk visited or found**: the cursor and +// the files found are under accounts' homes as often as not, and stay in the engine's state and journal. +type SearchProgress struct { + // Started is when the search began, on the cache's clock. + Started time.Time + // Filesystems is how many it walks; Done how many it has walked to their end; Walking the mount point of + // the one it walks now, empty when every one is walked. + Filesystems, Done int + Walking string + // Visited is how many entries it has gone past; Found how many setuid- or setgid-root files it found so + // far, before the package manager is asked about them. + Visited int64 + Found int + // Kept is when its progress was last kept, zero before the first time. + Kept time.Time + // Finished is every filesystem walked to its end, in order, and how long its walk took. + Finished []SearchedFilesystem +} + +// SearchedFilesystem is one filesystem a search walked to its end. Took is negative when not known: walked by +// an engine before it counted. +type SearchedFilesystem struct { + Mount string + Took time.Duration +} + +// progressOf is p as anybody may read it. Counts and mount points, no I/O. +func progressOf(p *SetuidProgress, kept time.Time) *SearchProgress { + sp := &SearchProgress{Started: p.Started, Filesystems: len(p.Mounts), Done: len(p.Done), Visited: p.Visited, + Found: len(p.Found), Kept: kept} + for i, m := range p.Done { + took := time.Duration(-1) + if i < len(p.Took) { + took = p.Took[i] + } + sp.Finished = append(sp.Finished, SearchedFilesystem{Mount: m, Took: took}) + } + for _, m := range p.Mounts { + if !slices.Contains(p.Done, m) { + sp.Walking = m + break + } + } + return sp +} + +// Words are the progress in one line, as the verdict says it beside "not judged yet". +func (sp *SearchProgress) Words() string { + if sp == nil { + return "" + } + s := fmt.Sprintf("progress: %d of %d filesystems walked", sp.Done, sp.Filesystems) + if sp.Walking != "" { + s += ", walking " + sp.Walking + } + s += fmt.Sprintf(", %d entries visited, %d setuid- or setgid-root files found so far", sp.Visited, sp.Found) + if sp.Kept.IsZero() { + s += ", not kept yet" + } else { + s += ", kept " + sp.Kept.Local().Format("2006-01-02 15:04:05") + } + var took []string + for _, f := range sp.Finished { + if f.Took < 0 { + took = append(took, f.Mount+" in a time not known") + } else { + took = append(took, f.Mount+" in "+f.Took.Round(time.Second).String()) + } + } + if len(took) > 0 { + s += "; walked " + strings.Join(took, ", ") + } + return s } func (e *SetuidNotJudged) Error() string { @@ -152,6 +234,44 @@ func (c *SetuidCache) SearchAgain() { c.forgetProgress() } +// SearchNow throws away the last complete search, kept or in memory, and any walk under way or kept, and has +// the next look start a full search at once, past AgainAfter and any back-off (novox/hq issue 361): the +// operator changed by hand what the last one found, which no apply says. Until the new one completes the +// verdict is "not judged yet". Asked at the controller's terminal alone (`nox node setuid-search `): a +// search can never make a machine free wrongly, but asked again and again it would keep a machine unjudged +// and its disks busy. by is who asked, for the journal. Nil c has nothing to throw away. +func (c *SetuidCache) SearchNow(by string) string { + if c == nil { + return "this engine keeps no search for setuid programs" + } + c.mu.Lock() + defer c.mu.Unlock() + c.loaded = true // nothing kept is read after this: it is thrown away + what := "no complete search was kept" + if c.last != nil { + what = "the complete search started " + c.last.Started.Local().Format("2006-01-02 15:04") + " is thrown away" + } + if c.running { + what += ", and the search running since " + c.startedAt.Local().Format("2006-01-02 15:04") + " is ended" + if c.cancel != nil { + c.cancel() + } + } + c.gen++ + c.last, c.unfinished, c.progress = nil, nil, nil + c.failed, c.failedAt, c.backoff, c.again = nil, time.Time{}, 0, false + c.forgetProgress() + if c.Path != "" { + if err := os.Remove(c.Path); err != nil && !errors.Is(err, fs.ErrNotExist) { + c.say("the search for setuid programs could not forget the complete search it kept: " + err.Error()) + } + } + said := "a new search for setuid programs starts at the next look, asked by " + by + ": " + what + + "; until it completes, whether an account can become root through one is not judged" + c.say(said) + return said +} + // SetuidSearch is one search: it carries p on to its end — walking what p has not walked, calling checkpoint // as it goes so its progress can be kept — and answers the setuid-root programs no package owns. type SetuidSearch func(ctx context.Context, p *SetuidProgress, checkpoint func()) ([]string, error) @@ -199,7 +319,8 @@ func (c *SetuidCache) Look(now time.Time, mounts []string, search SetuidSearch) case fresh: return slices.Clone(c.last.Found), c.last.Started, nil case c.running: - return nil, time.Time{}, &SetuidNotJudged{Pending: true, Since: c.startedAt, Last: c.lastStarted()} + return nil, time.Time{}, &SetuidNotJudged{Pending: true, Since: c.startedAt, Last: c.lastStarted(), + Progress: c.progress} case c.failed != nil: return nil, time.Time{}, &SetuidNotJudged{Since: c.failedAt, Err: c.failed, Last: c.lastStarted()} default: @@ -243,11 +364,17 @@ func (c *SetuidCache) start(now time.Time, mounts []string, search SetuidSearch) } c.running, c.startedAt, c.attemptAt, c.again = true, p.Started, now, false c.starts++ + // What a look says until the first checkpoint: where the kept walk was, or nothing walked. + kept := p.Kept + c.progress = progressOf(p, kept) if resumed { c.say(fmt.Sprintf("the search for setuid programs started %s carries on where it was (%d of %d "+ "filesystems walked)", p.Started.Local().Format("2006-01-02 15:04"), len(p.Done), len(p.Mounts))) } began := time.Now() + gen := c.gen + ctx, cancel := context.WithCancel(context.Background()) + c.cancel = cancel c.wg.Add(1) go func() { defer c.wg.Done() @@ -261,25 +388,50 @@ func (c *SetuidCache) start(now time.Time, mounts []string, search SetuidSearch) } if err == nil { unkept := false - found, err = search(context.Background(), p, func() { + found, err = search(ctx, p, func() { c.mu.Lock() - superseded := c.again + superseded := c.again || c.gen != gen c.mu.Unlock() - if superseded { - return // root acted since this walk started: its progress is not to be carried on + if !superseded { // root acted since this walk started: its progress is not to be carried on + at := time.Now() + p.Kept = at + if kerr := c.keepProgress(p); kerr != nil { + p.Kept = kept // not kept: the file still says the time before + if !unkept { + unkept = true + c.say("the search for setuid programs could not keep how far it got, and starts over " + + "if the engine stops: " + kerr.Error()) + } + } else { + kept = at + } } - if kerr := c.keepProgress(p); kerr != nil && !unkept { - unkept = true - c.say("the search for setuid programs could not keep how far it got, and starts over if " + - "the engine stops: " + kerr.Error()) + // What a look says from now: computed from p here, on the walk's own goroutine, never read + // by a look while the walk changes it. + sp := progressOf(p, kept) + c.mu.Lock() + if c.gen == gen { + c.progress = sp } + c.mu.Unlock() }) } took := time.Since(began) + cancel() c.mu.Lock() defer c.mu.Unlock() - c.running = false + c.running, c.progress, c.cancel = false, nil, nil c.endedAt = now.Add(took) + if c.gen != gen { + // Thrown away by SearchNow while it ran: neither a result nor a failure, and nothing to carry on. The + // next look starts the search asked for. + c.endedAt = time.Time{} + // A checkpoint that read its generation just before SearchNow may have kept this walk again. + c.forgetProgress() + c.say(fmt.Sprintf("the search for setuid programs started %s was ended, as asked (%s this time)", + p.Started.Local().Format("2006-01-02 15:04"), took.Round(time.Second))) + return + } if err != nil { // The back-off counts from when it failed. c.failed, c.failedAt = err, c.endedAt diff --git a/internal/accounts/setuid_progress_test.go b/internal/accounts/setuid_progress_test.go new file mode 100644 index 0000000..2fa03e2 --- /dev/null +++ b/internal/accounts/setuid_progress_test.go @@ -0,0 +1,205 @@ +package accounts + +import ( + "context" + "errors" + "os" + "path/filepath" + "strings" + "testing" + "time" +) + +// How far a running search got is readable through the verdict (novox/hq issue 361): counts, times and mount +// points, counted once across a resume, and never a path the walk visited or found. + +// A walk stopped midway and resumed counts every entry once, as the walk that was never stopped does, and says +// how long each filesystem took. +func TestTheProgressCountsEachEntryOnceAcrossAResume(t *testing.T) { + top := tree(t, "a/1!", "a/2", "a-b/3!", "b/c/4!", "b/c/5", "b/d!", "c/6!", "d/7!") + other := tree(t, "x/1", "y!") + whole := &SetuidProgress{Mounts: []string{top, other}} + if err := mine().walk(t.Context(), whole, func() {}); err != nil { + t.Fatal(err) + } + // top and the 14 entries below it, and other and its 3. + if whole.Visited != 15+4 || len(whole.Took) != 2 || whole.Took[0] < 0 || whole.Walking != 0 { + t.Fatalf("the whole walk: visited %d, took %v, walking %v", whole.Visited, whole.Took, whole.Walking) + } + + w := mine() + w.every = 0 + ctx, stop := context.WithCancel(t.Context()) + p := &SetuidProgress{Mounts: []string{top, other}} + var kept []byte + err := w.walk(ctx, p, func() { + kept = mustJSON(t, p) + if strings.HasSuffix(p.Cursor[top], "/b/c/4") { + stop() + } + }) + if !errors.Is(err, context.Canceled) { + t.Fatalf("the walk was stopped: %v", err) + } + var resumed SetuidProgress + fromJSON(t, kept, &resumed) + if resumed.Visited == 0 || resumed.Visited >= whole.Visited || resumed.Walking < 0 { + t.Fatalf("the kept walk counted %d of %d", resumed.Visited, whole.Visited) + } + if err := mine().walk(t.Context(), &resumed, func() {}); err != nil { + t.Fatal(err) + } + if resumed.Visited != whole.Visited || len(resumed.Took) != 2 { + t.Fatalf("the resumed walk counted %d entries (the whole one %d), took %v", resumed.Visited, + whole.Visited, resumed.Took) + } +} + +// A walk kept before the times were counted has them not known, never shifted onto the next filesystem. +func TestAWalkKeptWithoutTimesSaysThemNotKnown(t *testing.T) { + top, other := tree(t, "a"), tree(t, "b") + p := &SetuidProgress{Mounts: []string{top, other}, Done: Paths{top}} + if err := mine().walk(t.Context(), p, func() {}); err != nil { + t.Fatal(err) + } + if len(p.Took) != 2 || p.Took[0] >= 0 || p.Took[1] < 0 { + t.Fatalf("took %v", p.Took) + } + if w := progressOf(p, time.Time{}).Words(); !strings.Contains(w, top+" in a time not known") { + t.Fatalf("the words: %s", w) + } +} + +// A look while the search runs says how far it got at its last checkpoint, with no path it visited or found. +func TestALookSaysHowFarTheRunningSearchGotAndNoPath(t *testing.T) { + dir := t.TempDir() + c := &SetuidCache{ProgressPath: filepath.Join(dir, SetuidProgressFile)} + secret := "/home/someone/private/rootshell" + checkpointed, release := make(chan struct{}), make(chan struct{}) + search := func(ctx context.Context, p *SetuidProgress, checkpoint func()) ([]string, error) { + p.Mounts = Paths{"/", "/home", "/boot"} + p.Done, p.Took = Paths{"/"}, []time.Duration{90 * time.Minute} + p.Cursor["/home"] = "/home/someone/private" + p.Found = Paths{"/usr/bin/odd", secret} + p.Visited = 1234567 + checkpoint() + close(checkpointed) + <-release + return nil, nil + } + _, _, err := c.Look(at, root, search) + nj := notJudged(t, err, true) + if nj.Progress == nil || nj.Progress.Visited != 0 || nj.Progress.Walking != "/" { + t.Fatalf("before the first checkpoint: %+v", nj.Progress) + } + <-checkpointed + _, _, err = c.Look(at.Add(time.Minute), root, search) + nj = notJudged(t, err, true) + sp := nj.Progress + if sp == nil || sp.Filesystems != 3 || sp.Done != 1 || sp.Walking != "/home" || sp.Visited != 1234567 || + sp.Found != 2 || sp.Kept.IsZero() || len(sp.Finished) != 1 || sp.Finished[0].Took != 90*time.Minute { + t.Fatalf("the progress: %+v", sp) + } + words := sp.Words() + for _, want := range []string{"1 of 3 filesystems walked", "walking /home", "1234567 entries visited", + "2 setuid- or setgid-root files found so far", "kept " + sp.Kept.Local().Format("2006-01-02 15:04:05"), + "walked / in 1h30m0s"} { + if !strings.Contains(words, want) { + t.Errorf("the words lack %q: %s", want, words) + } + } + for _, leak := range []string{"someone", "private", "odd", "rootshell"} { + if strings.Contains(words, leak) || strings.Contains(err.Error(), leak) { + t.Errorf("a path leaks (%q): %s / %v", leak, words, err) + } + } + close(release) + c.Wait() + if _, _, err := c.Look(at.Add(2*time.Minute), root, search); err != nil { + t.Fatalf("the complete search judges: %v", err) + } +} + +// The progress moves every minute; the verdict does not change for it, so the journal says it once. +type waiting struct{ progress *SearchProgress } + +func (w *waiting) InDatabase(context.Context, string) ([]string, error) { return nil, nil } +func (w *waiting) Session(context.Context, string, []string) (Session, error) { + return Session{}, nil +} +func (w *waiting) Escalation(context.Context, string, []string) ([]string, error) { + return nil, &SetuidNotJudged{Pending: true, Since: at, Progress: w.progress} +} + +func TestTheProgressMovesWithoutChangingTheVerdict(t *testing.T) { + r := &waiting{progress: &SearchProgress{Filesystems: 2, Visited: 10}} + j := New(r) + j.Set([]Account{{Module: "claude-code", ID: "claude-code.agent", Name: "agent", Root: true}}) + st, changed := j.Look(t.Context()) + if !changed || st.Accounts[0].State != Unknown || st.Accounts[0].Search.Visited != 10 { + t.Fatalf("first look: changed %v, %+v", changed, st.Accounts[0]) + } + r.progress = &SearchProgress{Filesystems: 2, Visited: 20} + st, changed = j.Look(t.Context()) + if changed || st.Accounts[0].Search.Visited != 20 || strings.Contains(st.Accounts[0].Reason, "progress") { + t.Fatalf("second look: changed %v, %+v", changed, st.Accounts[0]) + } +} + +// Asked at the terminal, a search starts afresh at the next look: the last complete one is thrown away, kept or +// in memory, and a running one is ended and neither judges nor is carried on. +func TestSearchNowThrowsAwayTheLastSearchAndStartsAFullOne(t *testing.T) { + dir := t.TempDir() + c := &SetuidCache{Path: filepath.Join(dir, SetuidSearchFile), ProgressPath: filepath.Join(dir, SetuidProgressFile)} + g := newGate() + g.found = []string{"/home/someone/rootshell"} + c.Look(at, root, g.search) + <-g.started + g.finish(c) + if found, _, err := c.Look(at.Add(time.Hour), root, g.search); err != nil || len(found) != 1 { + t.Fatalf("the complete search judges: %v %v", found, err) + } + said := c.SearchNow("jo at the controller's terminal") + if !strings.Contains(said, "thrown away") || strings.Contains(said, "someone") { + t.Fatalf("said: %s", said) + } + if _, err := os.Stat(c.Path); !errors.Is(err, os.ErrNotExist) { + t.Fatalf("the kept search is still there: %v", err) + } + // The next look starts a full search at once, though the last ended only an hour ago. + ended := make(chan error, 1) + running := func(ctx context.Context, p *SetuidProgress, checkpoint func()) ([]string, error) { + if len(p.Done) != 0 || len(p.Cursor) != 0 { + t.Errorf("not a full search: %+v", p) + } + checkpoint() + <-ctx.Done() + ended <- ctx.Err() + return nil, ctx.Err() + } + _, _, err := c.Look(at.Add(time.Hour+time.Minute), root, running) + notJudged(t, err, true) + if starts(c) != 2 { + t.Fatalf("searches started: %d", starts(c)) + } + // Asked again while it runs: it is ended, judges nothing, fails nothing, and is not carried on. + c.SearchNow("jo") + if err := <-ended; !errors.Is(err, context.Canceled) { + t.Fatalf("the running search was not ended: %v", err) + } + c.Wait() + if _, err := os.Stat(c.ProgressPath); !errors.Is(err, os.ErrNotExist) { + t.Fatalf("the ended walk is still kept: %v", err) + } + g2 := newGate() + _, _, err = c.Look(at.Add(time.Hour+2*time.Minute), root, g2.search) + notJudged(t, err, true) + <-g2.started + g2.finish(c) + if starts(c) != 3 { + t.Fatalf("searches started: %d", starts(c)) + } + if _, _, err := c.Look(at.Add(time.Hour+3*time.Minute), root, g2.search); err != nil { + t.Fatalf("the new search judges: %v", err) + } +} diff --git a/internal/accounts/setuid_walk.go b/internal/accounts/setuid_walk.go index 1df3031..58f3e89 100644 --- a/internal/accounts/setuid_walk.go +++ b/internal/accounts/setuid_walk.go @@ -41,6 +41,16 @@ type SetuidProgress struct { Cursor Cursors `json:"cursor"` // Found is every setuid- or setgid-root regular file found so far, before the package manager is asked. Found Paths `json:"found"` + // Visited is how many entries the walk has gone past: each once, across a resume, as it is kept with the + // cursor it matches. + Visited int64 `json:"visited"` + // Took is how long the walk spent in each of Done, in its order (negative: not known, a walk kept by an + // engine before it counted); Walking how long it has spent in the filesystem it walks now. Time the + // engine did not run is in neither. + Took []time.Duration `json:"took"` + Walking time.Duration `json:"walking"` + // Kept is when this progress was kept, as the cache wrote it; zero before the first time. + Kept time.Time `json:"kept"` } // Paths are paths kept as their bytes. **Never as JSON strings**: encoding/json replaces bytes that are not @@ -120,6 +130,11 @@ func (w walker) walk(ctx context.Context, p *SetuidProgress, checkpoint func()) if p.Cursor == nil { p.Cursor = Cursors{} } + // One Took for each of Done: a walk kept before they were counted has them not known. + for len(p.Took) < len(p.Done) { + p.Took = append(p.Took, -1) + } + p.Took = p.Took[:len(p.Done)] last := time.Now() for _, m := range p.Mounts { if slices.Contains(p.Done, m) { @@ -129,6 +144,8 @@ func (w walker) walk(ctx context.Context, p *SetuidProgress, checkpoint func()) return err } p.Done = append(p.Done, m) + p.Took = append(p.Took, p.Walking) + p.Walking = 0 delete(p.Cursor, m) checkpoint() last = time.Now() @@ -166,6 +183,9 @@ func (w walker) walkMount(ctx context.Context, p *SetuidProgress, mount string, if c := p.Cursor[mount]; c != "" { cursor = components(c) } + // How long it walks this filesystem, carried on from what a resumed walk had spent in it. + began, before := time.Now(), p.Walking + defer func() { p.Walking = before + time.Since(began) }() return filepath.WalkDir(mount, func(path string, d fs.DirEntry, err error) error { if ctxErr := ctx.Err(); ctxErr != nil { return ctxErr @@ -212,10 +232,13 @@ func (w walker) walkMount(ctx context.Context, p *SetuidProgress, mount string, } } // The cursor only moves on: the directories on the way back to it are visited again, and are before it. + // Counted here alone, so an entry visited again on the way back to the cursor is counted once. if cursor == nil || walkOrder(at, cursor) > 0 { p.Cursor[mount], cursor = path, at + p.Visited++ } if time.Since(*last) >= w.every { + p.Walking = before + time.Since(began) checkpoint() *last = time.Now() } diff --git a/internal/link/handover.go b/internal/link/handover.go index aa7cd22..1fc7f60 100644 --- a/internal/link/handover.go +++ b/internal/link/handover.go @@ -120,20 +120,36 @@ func AnswerHandOver(check HandOverCheck, handle HandOverFunc, body []byte) HandO if handle == nil { return refusedAsk("this engine takes no hand-over") } + ask, refused := believed(check, HandOverContext, true, body) + if refused != "" { + return refusedAsk(refused) + } + said, err := handle(ask.Path, ask.By) + if err != nil { + return HandOverAnswer{Refused: err.Error()} + } + return HandOverAnswer{Said: said} +} + +// believed is an ask's bytes held to what every signed ask is: signed by the mesh over context and its bytes, +// for this node, unexpired and good no longer than an ask may be, with a nonce of its own taken once here, and +// one short line naming who asked, and a path when withPath and none otherwise. The nonce is taken last, after +// every other check passed. It answers the ask with who asked trimmed, or why it is refused. +func believed(check HandOverCheck, context string, withPath bool, body []byte) (HandOverAsk, string) { if len(body) > MaxHandOverAsk { - return refusedAsk(fmt.Sprintf("the ask is %d bytes, more than a hand-over", len(body))) + return HandOverAsk{}, fmt.Sprintf("the ask is %d bytes, more than an ask", len(body)) } var signed SignedHandOver if err := json.Unmarshal(body, &signed); err != nil { - return refusedAsk("the ask is not a signed hand-over: " + err.Error()) + return HandOverAsk{}, "the ask is not a signed ask: " + err.Error() } if len(check.Signer) != ed25519.PublicKeySize || - !ed25519.Verify(check.Signer, append([]byte(HandOverContext), signed.Ask...), signed.Signature) { - return refusedAsk("the ask is not signed by the mesh") + !ed25519.Verify(check.Signer, append([]byte(context), signed.Ask...), signed.Signature) { + return HandOverAsk{}, "the ask is not signed by the mesh" } var ask HandOverAsk if err := json.Unmarshal(signed.Ask, &ask); err != nil { - return refusedAsk("the signed ask is not a hand-over: " + err.Error()) + return HandOverAsk{}, "the signed ask is not an ask: " + err.Error() } now := time.Now() if check.Now != nil { @@ -143,31 +159,29 @@ func AnswerHandOver(check HandOverCheck, handle HandOverFunc, body []byte) HandO if nonces == nil { nonces = HandOverNonces } - by := strings.TrimSpace(ask.By) + ask.By = strings.TrimSpace(ask.By) nonce, nonceErr := hex.DecodeString(ask.Nonce) switch { case check.Node == "" || ask.Node != check.Node: - return refusedAsk(fmt.Sprintf("the ask is for %q, and this is %q", ask.Node, check.Node)) + return HandOverAsk{}, fmt.Sprintf("the ask is for %q, and this is %q", ask.Node, check.Node) case ask.Expires.IsZero() || !now.Before(ask.Expires): - return refusedAsk("the ask has expired") + return HandOverAsk{}, "the ask has expired" case ask.Expires.After(now.Add(HandOverGood + HandOverSkew)): - return refusedAsk("the ask is good for longer than an ask may be") + return HandOverAsk{}, "the ask is good for longer than an ask may be" case nonceErr != nil || len(nonce) < 16: - return refusedAsk("the ask carries no nonce of its own") - case by == "": - return refusedAsk("the ask names nobody as asking") - case len(by) > MaxHandOverBy || strings.ContainsAny(by, "\r\n\x00"): - return refusedAsk("who asked is not one short line") - case ask.Path == "": - return refusedAsk("the ask names no directory") + return HandOverAsk{}, "the ask carries no nonce of its own" + case ask.By == "": + return HandOverAsk{}, "the ask names nobody as asking" + case len(ask.By) > MaxHandOverBy || strings.ContainsAny(ask.By, "\r\n\x00"): + return HandOverAsk{}, "who asked is not one short line" + case withPath && ask.Path == "": + return HandOverAsk{}, "the ask names no directory" + case !withPath && ask.Path != "": + return HandOverAsk{}, "the ask names a path, and this ask takes none" case !nonces.take(ask.Nonce, ask.Expires, now): - return refusedAsk("the ask was taken already") + return HandOverAsk{}, "the ask was taken already" } - said, err := handle(ask.Path, by) - if err != nil { - return HandOverAnswer{Refused: err.Error()} - } - return HandOverAnswer{Said: said} + return ask, "" } // HandOverReply is what a hand-over request is answered with, and whether to answer: a request with no reply @@ -188,3 +202,63 @@ func HandOverReply(check HandOverCheck, handle HandOverFunc, reply string, body type HandOvers interface { HearHandOvers(HandOverFunc) } + +// A search for setuid programs asked over the bus (novox/hq issue 361). +// +// The operator who changed by hand what the last search found — removed a setuid-root copy no package owns — +// asks for a fresh one at the controller's terminal: `nox node setuid-search ` on the control-node. The +// controller asks this node's engine on AskSetuidSearchSubject, signed as a hand-over is and held to the same +// checks (believed), under a signing context of its own so neither ask's signature is the other's. The engine +// throws away the last search and starts a full one; it never names a path in its answer. +// +// **At the terminal alone, never a verb** (decided in issue 361): a search can never make a machine free +// wrongly — it judges only once complete — but one asked again and again would keep the machine unjudged and +// its disks busy, and whoever may call a verb includes agents. + +// AskSetuidSearchSubject is where the controller asks this node's engine for a fresh search. +func AskSetuidSearchSubject(node string) string { return "mesh.node." + node + ".ask.setuid-search" } + +// SetuidSearchContext is prefixed to a search ask's bytes before signing: never a hand-over's, nor a +// declaration's. +const SetuidSearchContext = "novox-mesh setuid-search v1\n" + +// SetuidSearchFunc throws away the last search and starts a full one, asked by by: the words to answer with. +type SetuidSearchFunc func(by string) (string, error) + +func refusedSearch(why string) HandOverAnswer { + return HandOverAnswer{Refused: why + "; no search was started"} +} + +// AnswerSetuidSearch is the engine's judgement of one search ask's bytes: believed as a hand-over is, a path +// refused, and only then the handler. +func AnswerSetuidSearch(check HandOverCheck, handle SetuidSearchFunc, body []byte) HandOverAnswer { + if handle == nil { + return refusedSearch("this engine searches for setuid programs on no ask") + } + ask, refused := believed(check, SetuidSearchContext, false, body) + if refused != "" { + return refusedSearch(refused) + } + said, err := handle(ask.By) + if err != nil { + return refusedSearch(err.Error()) + } + return HandOverAnswer{Said: said} +} + +// SetuidSearchReply is what a search request is answered with, and whether to answer, as HandOverReply. +func SetuidSearchReply(check HandOverCheck, handle SetuidSearchFunc, reply string, body []byte) ([]byte, bool) { + if reply == "" { + return nil, false + } + answer, err := json.Marshal(AnswerSetuidSearch(check, handle, body)) + if err != nil { + return nil, false + } + return answer, true +} + +// SetuidSearches is a link that hears the controller asking this node for a fresh search. Optional. +type SetuidSearches interface { + HearSetuidSearches(SetuidSearchFunc) +} diff --git a/internal/link/hearing_nats.go b/internal/link/hearing_nats.go index 1466a33..72d16f9 100644 --- a/internal/link/hearing_nats.go +++ b/internal/link/hearing_nats.go @@ -62,7 +62,9 @@ type natsLink struct { asking *nats.Subscription asked chan struct{} handing *nats.Subscription - node string + // searching hears the controller's terminal asking for a fresh setuid search (novox/hq issue 361). + searching *nats.Subscription + node string // signer is the mesh's key, which a hand-over ask is verified with as a declaration is (issue 356). signer ed25519.PublicKey arrived chan Declaration @@ -217,6 +219,25 @@ func (l *natsLink) HearHandOvers(handle HandOverFunc) { } } +// HearSetuidSearches listens for the controller asking this node for a fresh search for setuid programs +// (novox/hq issue 361), on this machine's own subject, verified as a hand-over is (AnswerSetuidSearch). A bus +// whose user list is older than the grant refuses the subscription, said by the client; the operator's line +// then says that nothing on this node answers. +func (l *natsLink) HearSetuidSearches(handle SetuidSearchFunc) { + if handle == nil { + return + } + check := HandOverCheck{Signer: l.signer, Node: l.node} + searching, err := l.conn.Subscribe(AskSetuidSearchSubject(l.node), func(msg *nats.Msg) { + if body, answer := SetuidSearchReply(check, handle, msg.Reply, msg.Data); answer { + _ = msg.Respond(body) + } + }) + if err == nil { + l.searching = searching + } +} + func (l *natsLink) Declarations() <-chan Declaration { return l.arrived } func (l *natsLink) AskedToReport() <-chan struct{} { return l.asked } func (l *natsLink) Lost() <-chan error { return l.lost } @@ -231,6 +252,9 @@ func (l *natsLink) Close() { if l.handing != nil { _ = l.handing.Unsubscribe() } + if l.searching != nil { + _ = l.searching.Unsubscribe() + } if l.conn != nil { l.conn.Close() } diff --git a/internal/link/messages.go b/internal/link/messages.go index 7fe19e4..b36a7e1 100644 --- a/internal/link/messages.go +++ b/internal/link/messages.go @@ -373,6 +373,31 @@ type ResourceHealth struct { // a person (novox/hq ADR 0266): healthy then also means the engine found no way for it to. Empty on // every other verdict, and on an account judged for its groups alone. Root string `json:"root,omitempty"` + // Search is how far the setuid search got, on an account's verdict that waits for it (novox/hq issue + // 361): Reason says the same in words. Absent otherwise. + Search *SetuidSearch `json:"search,omitempty"` +} + +// SetuidSearch is how far a running search for setuid programs got, as of its last checkpoint. Counts, times +// and mount points only: never a path the walk visited or found. +type SetuidSearch struct { + Started time.Time `json:"started"` + Filesystems int `json:"filesystems"` + Done int `json:"done"` + // Walking is the mount point of the filesystem walked now; empty when every one is walked. + Walking string `json:"walking,omitempty"` + Visited int64 `json:"visited"` + // Found is how many setuid- or setgid-root files it found so far, before the package manager is asked. + Found int `json:"found"` + // Kept is when its progress was last kept; absent before the first time. + Kept *time.Time `json:"kept,omitempty"` + Finished []SearchedFilesystem `json:"finished,omitempty"` +} + +// SearchedFilesystem is one filesystem a search walked to its end; TookSeconds is absent when not known. +type SearchedFilesystem struct { + Mount string `json:"mount"` + TookSeconds *int64 `json:"took_seconds,omitempty"` } // HealthSaid is the health event: a machine's statement between its reports, on HealthSubject. diff --git a/internal/link/queue.go b/internal/link/queue.go index af58f5e..38a777d 100644 --- a/internal/link/queue.go +++ b/internal/link/queue.go @@ -54,8 +54,11 @@ type Queue struct { // HandOver judges and records a directory used as found handed to the mesh by the operator, asked by // the controller over the link (novox/hq issue 356). Nil answers every such ask with a refusal. HandOver HandOverFunc - Say Announce - Timeout time.Duration + // SetuidSearch throws away the last search for setuid programs and starts a full one, asked by the + // controller's terminal over the link (novox/hq issue 361). Nil answers every such ask with a refusal. + SetuidSearch SetuidSearchFunc + Say Announce + Timeout time.Duration once sync.Once wake chan struct{} diff --git a/internal/link/run.go b/internal/link/run.go index 039d9a9..e13135e 100644 --- a/internal/link/run.go +++ b/internal/link/run.go @@ -244,6 +244,10 @@ func serve(ctx context.Context, link Link, m Membership, queue *Queue, say Annou if h, ok := link.(HandOvers); ok { h.HearHandOvers(queue.HandOver) } + // And asking for a fresh search for setuid programs (novox/hq issue 361). + if s, ok := link.(SetuidSearches); ok { + s.HearSetuidSearches(queue.SetuidSearch) + } for { // Asked to stop — by standing aside, say — nothing further is taken in. diff --git a/internal/link/setuid_search_test.go b/internal/link/setuid_search_test.go new file mode 100644 index 0000000..5e90229 --- /dev/null +++ b/internal/link/setuid_search_test.go @@ -0,0 +1,60 @@ +package link + +import ( + "crypto/ed25519" + "encoding/json" + "strings" + "testing" + "time" +) + +// The subject and the signing context the controller holds the same words for (mesh-controller internal/link). +func TestTheSetuidSearchAskKeepsItsSubjectAndContext(t *testing.T) { + if AskSetuidSearchSubject("laptop") != "mesh.node.laptop.ask.setuid-search" { + t.Fatalf("the subject is %q", AskSetuidSearchSubject("laptop")) + } + if SetuidSearchContext != "novox-mesh setuid-search v1\n" || SetuidSearchContext == HandOverContext { + t.Fatalf("the signing context is %q", SetuidSearchContext) + } +} + +func signedSearch(t *testing.T, key ed25519.PrivateKey, context string, ask HandOverAsk) []byte { + t.Helper() + raw, _ := json.Marshal(ask) + body, _ := json.Marshal(SignedHandOver{Ask: raw, Signature: ed25519.Sign(key, append([]byte(context), raw...))}) + return body +} + +// A search ask is believed as a hand-over is: signed by the mesh under its own context (a hand-over's signature +// starts no search), for this node, unexpired, once, and naming no path. +func TestASetuidSearchAskIsBelievedOnlySignedForThisNodeAndOnce(t *testing.T) { + m, key := aMember(t) + now := time.Date(2026, 10, 10, 2, 0, 0, 0, time.UTC) + var asked []string + handle := func(by string) (string, error) { asked = append(asked, by); return "a new search starts", nil } + check := HandOverCheck{Signer: m.Signer, Node: "n1", Now: func() time.Time { return now }, Nonces: &Nonces{}} + good := HandOverAsk{Node: "n1", By: " jo ", Expires: now.Add(HandOverGood), Nonce: aNonce} + for name, body := range map[string][]byte{ + "a hand-over's signature": signedSearch(t, key, HandOverContext, good), + "another node": signedSearch(t, key, SetuidSearchContext, HandOverAsk{Node: "n2", By: "jo", Expires: good.Expires, Nonce: aNonce}), + "a path": signedSearch(t, key, SetuidSearchContext, HandOverAsk{Node: "n1", Path: "/x", By: "jo", Expires: good.Expires, Nonce: aNonce}), + "expired": signedSearch(t, key, SetuidSearchContext, HandOverAsk{Node: "n1", By: "jo", Expires: now, Nonce: aNonce}), + } { + if a := AnswerSetuidSearch(check, handle, body); !strings.HasSuffix(a.Refused, "no search was started") || a.Said != "" { + t.Errorf("%s: %+v", name, a) + } + } + if len(asked) != 0 { + t.Fatalf("a refused ask reached the handler: %v", asked) + } + body := signedSearch(t, key, SetuidSearchContext, good) + if a := AnswerSetuidSearch(check, handle, body); a.Said != "a new search starts" || len(asked) != 1 || asked[0] != "jo" { + t.Fatalf("the good ask: %+v, %v", a, asked) + } + if a := AnswerSetuidSearch(check, handle, body); !strings.Contains(a.Refused, "taken already") { + t.Fatalf("the same ask again: %+v", a) + } + if a := AnswerSetuidSearch(check, nil, body); a.Refused == "" { + t.Fatalf("no handler: %+v", a) + } +} From 0b4c86e3bed4be02f9302fc047fa712b39c10775 Mon Sep 17 00:00:00 2001 From: jochen Date: Sat, 10 Oct 2026 02:00:10 +0200 Subject: [PATCH 2/2] Count a setuid find only where the account can execute it, read at every judgement (hq issue 361) The two finds on the control-node sit in the operator's home of mode 0700, which the agent account cannot search. The search keeps every find; each judgement reads the modes on the way again, since an account can open its own home without root. A find in a home is said by count, its path in the journal alone. Also from review: a checkpoint racing SearchNow forgets the walk it just kept, and a walk kept by an older engine says its counts as lower bounds. --- internal/accounts/exec.go | 17 ++++ internal/accounts/root_test.go | 15 ++- internal/accounts/setuid_cache.go | 32 ++++-- internal/accounts/setuid_progress_test.go | 21 +++- internal/accounts/setuid_reach_test.go | 115 ++++++++++++++++++++++ internal/accounts/setuid_walk.go | 17 +++- internal/accounts/ways.go | 82 ++++++++++++++- internal/accounts/ways_test.go | 6 +- 8 files changed, 287 insertions(+), 18 deletions(-) create mode 100644 internal/accounts/setuid_reach_test.go diff --git a/internal/accounts/exec.go b/internal/accounts/exec.go index b7d8889..55a3eb4 100644 --- a/internal/accounts/exec.go +++ b/internal/accounts/exec.go @@ -28,6 +28,10 @@ type Exec struct { Proc string // Stat is a file's owner and mode; nil is the machine's own (os.Stat). A test gives files of its own. Stat func(path string) (FileMode, error) + // Lstat is a path's own owner and mode, never through a link, with its type bits in Perm (fs.ModeSymlink, + // fs.ModeDir); nil is the machine's own (os.Lstat). Whether an account can reach a setuid program is read + // with it at every judgement (novox/hq issue 361). + Lstat func(path string) (FileMode, error) // ReadFile, ReadDir and ACL read doas's and polkit's rules and a file's POSIX ACL; nil is the machine's. ReadFile func(path string) ([]byte, error) ReadDir func(path string) ([]fs.DirEntry, error) @@ -187,6 +191,19 @@ func statOf(path string) (FileMode, error) { return FileMode{UID: int(st.Uid), GID: int(st.Gid), Perm: info.Mode().Perm()}, nil } +// lstatOf is a path's own owner and mode, its type bits kept. +func lstatOf(path string) (FileMode, error) { + info, err := os.Lstat(path) + if err != nil { + return FileMode{}, err + } + st, ok := info.Sys().(*syscall.Stat_t) + if !ok { + return FileMode{}, fmt.Errorf("%s has no owner this platform reports", path) + } + return FileMode{UID: int(st.Uid), GID: int(st.Gid), Perm: info.Mode()}, nil +} + // InDatabase is `id -nG`: every group the user database lists the account in. func (e Exec) InDatabase(ctx context.Context, account string) ([]string, error) { out, err := e.Run(ctx, "id", "-nG", account) diff --git a/internal/accounts/root_test.go b/internal/accounts/root_test.go index 72ae4ad..387a90c 100644 --- a/internal/accounts/root_test.go +++ b/internal/accounts/root_test.go @@ -40,6 +40,19 @@ type agentMachine struct { findArgs func([]string) // pacmanCalls counts the package manager's calls: one for every path found. pacmanCalls int + // nodes are paths' own owners and modes (Lstat); any other is root's, a directory searchable by all, or a + // file the walk found, setuid and executable by all. + nodes map[string]FileMode +} + +func (m *agentMachine) lstat(path string) (FileMode, error) { + if n, ok := m.nodes[path]; ok { + return n, nil + } + if strings.Contains(m.setuid, path+"\n") { + return FileMode{Perm: fs.ModeSetuid | 0o755}, nil + } + return FileMode{Perm: fs.ModeDir | 0o755}, nil } func (m *agentMachine) readFile(path string) ([]byte, error) { @@ -157,7 +170,7 @@ func clean() *agentMachine { func lookAgent(t *testing.T, m *agentMachine) Verdict { t.Helper() - j := New(Exec{Run: m.run, Walk: m.walk, Stat: m.stat, ReadFile: m.readFile, ReadDir: m.readDir, ACL: m.acl}) + j := New(Exec{Run: m.run, Walk: m.walk, Stat: m.stat, ReadFile: m.readFile, ReadDir: m.readDir, ACL: m.acl, Lstat: m.lstat}) j.Set([]Account{agent}) st, _ := j.Look(t.Context()) if len(st.Accounts) != 1 { diff --git a/internal/accounts/setuid_cache.go b/internal/accounts/setuid_cache.go index 7125a3d..777bbf5 100644 --- a/internal/accounts/setuid_cache.go +++ b/internal/accounts/setuid_cache.go @@ -123,6 +123,8 @@ type SearchProgress struct { // far, before the package manager is asked about them. Visited int64 Found int + // AtLeast says Visited is a lower bound: the walk was begun by an engine that did not count. + AtLeast bool // Kept is when its progress was last kept, zero before the first time. Kept time.Time // Finished is every filesystem walked to its end, in order, and how long its walk took. @@ -139,7 +141,7 @@ type SearchedFilesystem struct { // progressOf is p as anybody may read it. Counts and mount points, no I/O. func progressOf(p *SetuidProgress, kept time.Time) *SearchProgress { sp := &SearchProgress{Started: p.Started, Filesystems: len(p.Mounts), Done: len(p.Done), Visited: p.Visited, - Found: len(p.Found), Kept: kept} + Found: len(p.Found), Kept: kept, AtLeast: !p.Counted} for i, m := range p.Done { took := time.Duration(-1) if i < len(p.Took) { @@ -165,7 +167,11 @@ func (sp *SearchProgress) Words() string { if sp.Walking != "" { s += ", walking " + sp.Walking } - s += fmt.Sprintf(", %d entries visited, %d setuid- or setgid-root files found so far", sp.Visited, sp.Found) + atLeast := "" + if sp.AtLeast { + atLeast = "at least " + } + s += fmt.Sprintf(", %s%d entries visited, %d setuid- or setgid-root files found so far", atLeast, sp.Visited, sp.Found) if sp.Kept.IsZero() { s += ", not kept yet" } else { @@ -286,7 +292,7 @@ type SetuidSearch func(ctx context.Context, p *SetuidProgress, checkpoint func() // start, a result reads at most one search older than it is, which errs towards not judging. func (c *SetuidCache) Look(now time.Time, mounts []string, search SetuidSearch) ([]string, time.Time, error) { if c == nil { - p := &SetuidProgress{Started: now, Mounts: slices.Clone(mounts)} + p := &SetuidProgress{Started: now, Mounts: slices.Clone(mounts), Counted: true} found, err := search(context.Background(), p, func() {}) return found, now, err } @@ -360,7 +366,7 @@ func (c *SetuidCache) start(now time.Time, mounts []string, search SetuidSearch) resumed := p != nil if p == nil { p = &SetuidProgress{ID: fmt.Sprintf("%x", now.UnixNano()), Started: now, Mounts: slices.Clone(mounts), - Cursor: map[string]string{}} + Cursor: map[string]string{}, Counted: true} } c.running, c.startedAt, c.attemptAt, c.again = true, p.Started, now, false c.starts++ @@ -395,7 +401,16 @@ func (c *SetuidCache) start(now time.Time, mounts []string, search SetuidSearch) if !superseded { // root acted since this walk started: its progress is not to be carried on at := time.Now() p.Kept = at - if kerr := c.keepProgress(p); kerr != nil { + kerr := c.keepProgress(p) + // Superseded while it wrote (SearchAgain, SearchNow), after they forgot the kept walk: forgotten + // again here, so no engine restarted before this walk ends carries it on (the review of + // 2026-10-10). Written outside the lock, so a look never waits on the disk. + c.mu.Lock() + if c.again || c.gen != gen { + c.forgetProgress() + } + c.mu.Unlock() + if kerr != nil { p.Kept = kept // not kept: the file still says the time before if !unkept { unkept = true @@ -450,8 +465,13 @@ func (c *SetuidCache) start(now time.Time, mounts []string, search SetuidSearch) "restarts: " + err.Error()) } c.forgetProgress() + // The paths in the engine's journal alone: the verdict says one in a home by count. + named := "" + if len(found) > 0 { + named = ": " + strings.Join(found, ", ") + } c.say(fmt.Sprintf("the search for setuid programs started %s completed (%s this time): %d no package "+ - "owns", p.Started.Local().Format("2006-01-02 15:04"), took.Round(time.Second), len(found))) + "owns%s", p.Started.Local().Format("2006-01-02 15:04"), took.Round(time.Second), len(found), named)) }() } diff --git a/internal/accounts/setuid_progress_test.go b/internal/accounts/setuid_progress_test.go index 2fa03e2..3bcfe11 100644 --- a/internal/accounts/setuid_progress_test.go +++ b/internal/accounts/setuid_progress_test.go @@ -30,7 +30,7 @@ func TestTheProgressCountsEachEntryOnceAcrossAResume(t *testing.T) { w := mine() w.every = 0 ctx, stop := context.WithCancel(t.Context()) - p := &SetuidProgress{Mounts: []string{top, other}} + p := &SetuidProgress{Mounts: []string{top, other}, Counted: true} var kept []byte err := w.walk(ctx, p, func() { kept = mustJSON(t, p) @@ -49,13 +49,14 @@ func TestTheProgressCountsEachEntryOnceAcrossAResume(t *testing.T) { if err := mine().walk(t.Context(), &resumed, func() {}); err != nil { t.Fatal(err) } - if resumed.Visited != whole.Visited || len(resumed.Took) != 2 { + if resumed.Visited != whole.Visited || len(resumed.Took) != 2 || resumed.Took[0] < 0 { t.Fatalf("the resumed walk counted %d entries (the whole one %d), took %v", resumed.Visited, whole.Visited, resumed.Took) } } -// A walk kept before the times were counted has them not known, never shifted onto the next filesystem. +// A walk kept before the times were counted has them not known, never shifted onto the next filesystem, and +// its entries visited are a lower bound; the filesystem it was part-way through took a time not known. func TestAWalkKeptWithoutTimesSaysThemNotKnown(t *testing.T) { top, other := tree(t, "a"), tree(t, "b") p := &SetuidProgress{Mounts: []string{top, other}, Done: Paths{top}} @@ -65,7 +66,8 @@ func TestAWalkKeptWithoutTimesSaysThemNotKnown(t *testing.T) { if len(p.Took) != 2 || p.Took[0] >= 0 || p.Took[1] < 0 { t.Fatalf("took %v", p.Took) } - if w := progressOf(p, time.Time{}).Words(); !strings.Contains(w, top+" in a time not known") { + if w := progressOf(p, time.Time{}).Words(); !strings.Contains(w, top+" in a time not known") || + !strings.Contains(w, "at least ") { t.Fatalf("the words: %s", w) } } @@ -203,3 +205,14 @@ func TestSearchNowThrowsAwayTheLastSearchAndStartsAFullOne(t *testing.T) { t.Fatalf("the new search judges: %v", err) } } + +func TestAFilesystemPartWalkedByAnEngineThatDidNotCountTookATimeNotKnown(t *testing.T) { + top := tree(t, "a", "b") + p := &SetuidProgress{Mounts: []string{top}, Cursor: Cursors{top: filepath.Join(top, "a")}} + if err := mine().walk(t.Context(), p, func() {}); err != nil { + t.Fatal(err) + } + if len(p.Took) != 1 || p.Took[0] >= 0 { + t.Fatalf("took %v", p.Took) + } +} diff --git a/internal/accounts/setuid_reach_test.go b/internal/accounts/setuid_reach_test.go new file mode 100644 index 0000000..940f49a --- /dev/null +++ b/internal/accounts/setuid_reach_test.go @@ -0,0 +1,115 @@ +package accounts + +import ( + "io/fs" + "os" + "path/filepath" + "strings" + "testing" +) + +// A setuid program no package owns counts against an account only where it can execute it, read at every +// judgement from the modes on the way (novox/hq issue 361): the operator's copies in a home of mode 0700 are no +// way for the agent account, and become one the moment the home is opened, without a new search. + +const homeFind = "/home/op/usr/bin/mount" + +func homeMachine(home FileMode) *agentMachine { + m := clean() + m.setuid = homeFind + "\n" + m.nodes = map[string]FileMode{"/home/op": home} + return m +} + +func TestReachabilityDecidesWhetherAFindCounts(t *testing.T) { + op := func(perm fs.FileMode) FileMode { return FileMode{UID: 1500, GID: 1500, Perm: fs.ModeDir | perm} } + for _, c := range []struct { + name string + set func(*agentMachine) + counted bool + }{ + {"a home of 0700", func(m *agentMachine) {}, false}, + {"a home of 0755", func(m *agentMachine) { m.nodes["/home/op"] = op(0o755) }, true}, + {"a home its group may search, the agent in it", func(m *agentMachine) { + m.nodes["/home/op"], m.gids = op(0o710), "1600 1500" + }, true}, + {"a home its group may search, the agent not in it", func(m *agentMachine) { m.nodes["/home/op"] = op(0o710) }, false}, + {"an ACL on the home", func(m *agentMachine) { + m.files["/home/op"] = FileMode{UID: 1500, GID: 1500, Perm: 0o700} + m.acls = map[string][]ACLEntry{"/home/op": {{}}} + }, true}, + {"a link on the way", func(m *agentMachine) { + m.nodes["/home/op"] = FileMode{UID: 1500, GID: 1500, Perm: fs.ModeSymlink | 0o777} + }, true}, + {"a file the agent may not execute", func(m *agentMachine) { + m.nodes["/home/op"] = op(0o755) + m.nodes[homeFind] = FileMode{Perm: fs.ModeSetuid | 0o4750} + }, false}, + } { + t.Run(c.name, func(t *testing.T) { + m := homeMachine(FileMode{UID: 1500, GID: 1500, Perm: fs.ModeDir | 0o700}) + c.set(m) + v := lookAgent(t, m) + if c.counted != (v.State == Unhealthy) { + t.Fatalf("counted %v: %+v", c.counted, v) + } + if strings.Contains(v.Reason, "/home/op") { + t.Fatalf("a path in a home was said: %s", v.Reason) + } + if c.counted && !strings.Contains(v.Reason, "a setuid-root program no package owns in an account's home, which agent can execute") { + t.Fatalf("the reason: %s", v.Reason) + } + }) + } +} + +// The search keeps every find; whether one counts is read again at every look. +func TestAHomeOpenedAfterTheSearchCountsWithoutANewSearch(t *testing.T) { + m := homeMachine(FileMode{UID: 1500, GID: 1500, Perm: fs.ModeDir | 0o700}) + c := &SetuidCache{} + j := New(Exec{Run: m.run, Walk: m.walk, Stat: m.stat, ReadFile: m.readFile, ReadDir: m.readDir, ACL: m.acl, + Lstat: m.lstat, Cache: c}) + j.Set([]Account{agent}) + j.Look(t.Context()) + c.Wait() + if st, _ := j.Look(t.Context()); st.Accounts[0].State != Healthy { + t.Fatalf("a find in a closed home: %+v", st.Accounts[0]) + } + m.nodes["/home/op"] = FileMode{UID: 1500, GID: 1500, Perm: fs.ModeDir | 0o755} + st, _ := j.Look(t.Context()) + if st.Accounts[0].State != Unhealthy || starts(c) != 1 { + t.Fatalf("the home opened: %+v, %d searches", st.Accounts[0], starts(c)) + } + m.nodes["/home/op"] = FileMode{UID: 1500, GID: 1500, Perm: fs.ModeDir | 0o700} + if st, _ := j.Look(t.Context()); st.Accounts[0].State != Healthy || starts(c) != 1 { + t.Fatalf("the home closed again: %+v", st.Accounts[0]) + } +} + +// On the machine's own modes, as their owner (the directories above a test's are its own): a directory without +// its owner's search bit hides what is in it. +func TestExecutableReadsTheMachinesModes(t *testing.T) { + if os.Getuid() == 0 { + t.Skip("root searches every directory") + } + top := tree(t, "home/bin/x!") + home := filepath.Join(top, "home") + me := os.Getuid() + noACL := func(string) ([]ACLEntry, error) { return nil, nil } + if err := os.Chmod(home, 0o600); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = os.Chmod(home, 0o700) }) + if Executable(filepath.Join(home, "bin/x"), me, map[int]bool{}, lstatOf, noACL) { + t.Fatal("a file in a directory its account may not search was executable") + } + if err := os.Chmod(home, 0o700); err != nil { + t.Fatal(err) + } + if !Executable(filepath.Join(home, "bin/x"), me, map[int]bool{}, lstatOf, noACL) { + t.Fatal("a file every directory opens was not executable") + } + if Executable(filepath.Join(home, "bin/gone"), me, map[int]bool{}, lstatOf, noACL) { + t.Fatal("a file gone was executable") + } +} diff --git a/internal/accounts/setuid_walk.go b/internal/accounts/setuid_walk.go index 58f3e89..30c66f2 100644 --- a/internal/accounts/setuid_walk.go +++ b/internal/accounts/setuid_walk.go @@ -51,6 +51,10 @@ type SetuidProgress struct { Walking time.Duration `json:"walking"` // Kept is when this progress was kept, as the cache wrote it; zero before the first time. Kept time.Time `json:"kept"` + // Counted is a walk counted from its start: false for one kept by an engine before it counted, whose + // Visited is then a lower bound and whose filesystem under way took a time not known (the review of + // 2026-10-10). + Counted bool `json:"counted"` } // Paths are paths kept as their bytes. **Never as JSON strings**: encoding/json replaces bytes that are not @@ -135,6 +139,9 @@ func (w walker) walk(ctx context.Context, p *SetuidProgress, checkpoint func()) p.Took = append(p.Took, -1) } p.Took = p.Took[:len(p.Done)] + if !p.Counted && len(p.Cursor) > 0 { + p.Walking = -1 // walked partly by an engine that did not count its time + } last := time.Now() for _, m := range p.Mounts { if slices.Contains(p.Done, m) { @@ -185,7 +192,13 @@ func (w walker) walkMount(ctx context.Context, p *SetuidProgress, mount string, } // How long it walks this filesystem, carried on from what a resumed walk had spent in it. began, before := time.Now(), p.Walking - defer func() { p.Walking = before + time.Since(began) }() + spent := func() time.Duration { + if before < 0 { + return -1 + } + return before + time.Since(began) + } + defer func() { p.Walking = spent() }() return filepath.WalkDir(mount, func(path string, d fs.DirEntry, err error) error { if ctxErr := ctx.Err(); ctxErr != nil { return ctxErr @@ -238,7 +251,7 @@ func (w walker) walkMount(ctx context.Context, p *SetuidProgress, mount string, p.Visited++ } if time.Since(*last) >= w.every { - p.Walking = before + time.Since(began) + p.Walking = spent() checkpoint() *last = time.Now() } diff --git a/internal/accounts/ways.go b/internal/accounts/ways.go index 100770a..90d68e9 100644 --- a/internal/accounts/ways.go +++ b/internal/accounts/ways.go @@ -640,9 +640,31 @@ func (e Exec) moreWays(ctx context.Context, account string, uid int, groups []st if searchErr != nil && !errors.As(searchErr, ¬Judged) { return nil, searchErr } + // Counted only where the account can execute it, read now from the paths' modes, never kept with the + // search: an account may open its own home to others later without root, and no search would see it. + lstat := e.Lstat + if lstat == nil { + lstat = lstatOf + } + started := " (search started " + searchedAt.Local().Format("2006-01-02 15:04") + ")" + inHomes := 0 for _, p := range unowned { - ways = append(ways, "a setuid-root program no package owns: "+p+" (search started "+ - searchedAt.Local().Format("2006-01-02 15:04")+")") + if !Executable(p, uid, gids, lstat, acl) { + continue + } + if inAHome(p) { + inHomes++ // its path stays in the engine's journal: a home's names are its owner's + continue + } + ways = append(ways, "a setuid-root program no package owns, which "+account+" can execute: "+p+started) + } + switch { + case inHomes == 1: + ways = append(ways, "a setuid-root program no package owns in an account's home, which "+account+ + " can execute"+started) + case inHomes > 1: + ways = append(ways, fmt.Sprintf("%d setuid-root programs no package owns in accounts' homes, which %s can "+ + "execute%s", inHomes, account, started)) } // The kernel's protection of links: off, any account may link another's file where root then acts on it. @@ -682,6 +704,62 @@ func (e Exec) moreWays(ctx context.Context, account string, uid int, groups []st return ways, nil } +// Executable says whether an account of uid, in the groups gids, can execute path as the kernel decides it: +// every directory from the root down searchable for it, and the file executable for it, each by the owner's +// bits for its owner, the group's for a member, the others' for anybody else (novox/hq issue 361). It errs +// towards a way: a link on the way, an ACL on any of them, or a mode that could not be read is executable. +// A path gone is not: there is nothing to execute. +func Executable(path string, uid int, gids map[int]bool, lstat func(string) (FileMode, error), + acl func(string) ([]ACLEntry, error)) bool { + if uid == 0 { + return true + } + at := "/" + steps := []string{at} + for _, name := range components(path) { + at = filepath.Join(at, name) + steps = append(steps, at) + } + for i, p := range steps { + m, err := lstat(p) + if errors.Is(err, fs.ErrNotExist) { + return false + } + if err != nil || m.Perm&fs.ModeSymlink != 0 { + return true + } + if a, err := acl(p); (err != nil && !errors.Is(err, fs.ErrNotExist)) || len(a) > 0 { + return true + } + bit := fs.FileMode(0o001) + switch { + case m.UID == uid: + bit = 0o100 + case gids[m.GID]: + bit = 0o010 + } + if m.Perm&bit == 0 { + return false + } + if i < len(steps)-1 && !m.Perm.IsDir() { + return false // a file on the way: the path names nothing + } + } + return true +} + +// homes are where accounts' homes are: a path under one is said by count alone, its name its owner's. +var homes = []string{"/home/", "/root/", "/var/home/"} + +func inAHome(path string) bool { + for _, h := range homes { + if strings.HasPrefix(path, h) { + return true + } + } + return false +} + // walkFiles calls fn for every regular file below dir, through readDir. func walkFiles(readDir func(string) ([]fs.DirEntry, error), dir string, fn func(string) error) error { entries, err := readDir(dir) diff --git a/internal/accounts/ways_test.go b/internal/accounts/ways_test.go index c8be804..51451a3 100644 --- a/internal/accounts/ways_test.go +++ b/internal/accounts/ways_test.go @@ -71,11 +71,11 @@ func TestEachFurtherWayToRootIsSaid(t *testing.T) { panic(fmt.Sprintf("searched %v", args)) } } - }, "a setuid-root program no package owns: /srv/bin/rootshell"}, + }, "a setuid-root program no package owns, which agent can execute: /srv/bin/rootshell"}, {"setuid no package owns", func(m *agentMachine) { m.setuid = "/usr/bin/sudo\n/usr/local/bin/rootshell\n" m.packaged = map[string]bool{"/usr/bin/sudo": true} - }, "a setuid-root program no package owns: /usr/local/bin/rootshell"}, + }, "a setuid-root program no package owns, which agent can execute: /usr/local/bin/rootshell"}, } { t.Run(c.name, func(t *testing.T) { m := clean() @@ -124,7 +124,7 @@ func TestAFurtherQuestionUnansweredIsUnknown(t *testing.T) { return "", fmt.Errorf("%s: %w", name, exec.ErrNotFound) } return m.run(ctx, name, args...) - }, Walk: m.walk, Stat: m.stat, ReadFile: m.readFile, ReadDir: m.readDir, ACL: m.acl}) + }, Walk: m.walk, Stat: m.stat, ReadFile: m.readFile, ReadDir: m.readDir, ACL: m.acl, Lstat: m.lstat}) j.Set([]Account{agent}) if st, _ := j.Look(t.Context()); st.Accounts[0].State != Unknown { t.Fatalf("no package manager to ask: %+v", st.Accounts[0])