From fc6c5a92e605739a88e7969c7c11f328096911dd Mon Sep 17 00:00:00 2001 From: jochen Date: Fri, 9 Oct 2026 19:45:00 +0200 Subject: [PATCH] Believe a hand-over only signed by the mesh, and give it through no link (hq issue 356, review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two holes the review proved. The ask's subject proved nothing — a responder can reach it through a reply — so the engine now verifies the mesh's signature over a fixed context with the key it verifies declarations with, before reading anything, and refuses an ask for another node, expired, good too long, without a nonce, or replayed. And the apply followed a link the hand-over had refused: a component swapped for a link after the record carried root's chmod and chown to its target. A handed-over directory must now still resolve to itself, and is changed only through one descriptor opened from / with no link followed (fchmod, fchown). Tests for the reply guard, the run loop's hook, the fallback renewal on proof and a non-regular fallback; 27 checks each removed in turn, each failing a test. --- cmd/mesh-host/found_test.go | 31 ++-- cmd/mesh-host/witnessing_test.go | 27 ++++ internal/apply/apply.go | 45 +++++- internal/apply/found_is_said_test.go | 98 +++++++++++++ internal/apply/homes_linux.go | 28 ++++ internal/apply/homes_other.go | 7 + internal/link/handover.go | 153 +++++++++++++++++--- internal/link/handover_test.go | 207 +++++++++++++++++++++++---- internal/link/hearing_nats.go | 26 ++-- internal/upgrade/fallback.go | 6 +- internal/upgrade/fallback_test.go | 32 +++++ 11 files changed, 575 insertions(+), 85 deletions(-) diff --git a/cmd/mesh-host/found_test.go b/cmd/mesh-host/found_test.go index 1721bf6..4a13d2c 100644 --- a/cmd/mesh-host/found_test.go +++ b/cmd/mesh-host/found_test.go @@ -143,24 +143,25 @@ func TestAHandOverIsRefusedForEveryPathThatIsNotTheFoundDirectory(t *testing.T) }}); err != nil { t.Fatal(err) } - refused := map[string]string{ - "a relative path": "found", - "a path with ..": dir + "/x/../found", - "a doubled separator": dir + "//found", - "a trailing separator": found + "/", - "a directory not used as found": filepath.Join(dir, "other"), - "a link at the path": linked, - "a link in the path": through, - "a file": file, - "a directory that is gone": gone, + // Each refused by its own check, which the reason names. + refused := map[string][2]string{ + "a relative path": {"found", "not one"}, + "a path with ..": {dir + "/x/../found", "as the engine states it"}, + "a doubled separator": {dir + "//found", "as the engine states it"}, + "a trailing separator": {found + "/", "as the engine states it"}, + "a directory not used as found": {filepath.Join(dir, "other"), "not a directory this machine uses as found"}, + "a link at the path": {linked, "is a link"}, + "a link in the path": {through, "reached through a link"}, + "a file": {file, "is not a directory on this machine"}, + "a directory that is gone": {gone, "cannot be read on this machine"}, } - for name, path := range refused { - said, err := handOverFound(statePath, path, "jo") + for name, c := range refused { + said, err := handOverFound(statePath, c[0], "jo") if err == nil || said != "" { - t.Fatalf("%s (%s) was handed over: %q", name, path, said) + t.Fatalf("%s (%s) was handed over: %q", name, c[0], said) } - if !strings.Contains(err.Error(), "nothing was handed over") { - t.Fatalf("%s's refusal does not say nothing was handed over: %v", name, err) + if !strings.Contains(err.Error(), "nothing was handed over") || !strings.Contains(err.Error(), c[1]) { + t.Fatalf("%s's refusal does not say %q and that nothing was handed over: %v", name, c[1], err) } } if all, err := store.LoadHandedOver(statePath); err != nil || len(all) != 0 { diff --git a/cmd/mesh-host/witnessing_test.go b/cmd/mesh-host/witnessing_test.go index 2fc5622..13ee112 100644 --- a/cmd/mesh-host/witnessing_test.go +++ b/cmd/mesh-host/witnessing_test.go @@ -5,6 +5,7 @@ import ( "os/exec" "path/filepath" "strconv" + "strings" "testing" "time" @@ -17,6 +18,12 @@ import ( func TestOnlyAnAccountUnderThisBuildProvesIt(t *testing.T) { state := filepath.Join(t.TempDir(), "state.json") t.Setenv("MESH_HOST_LIBEXEC", t.TempDir()) + // Never the machine's own engine placed by hand: proving renews it (issue 356). + fallback := filepath.Join(t.TempDir(), "nox-mesh-host") + if err := os.WriteFile(fallback, []byte("a stale engine placed by hand"), 0o755); err != nil { + t.Fatal(err) + } + t.Setenv(upgrade.FallbackVar, fallback) var said []string p := &proving{statePath: state, version: "2.0", say: func(s string) { said = append(said, s) }} @@ -29,6 +36,9 @@ func TestOnlyAnAccountUnderThisBuildProvesIt(t *testing.T) { if !unproved(state, "2.0") { t.Fatalf("%+v proved this build", r) } + if got, _ := os.ReadFile(fallback); string(got) != "a stale engine placed by hand" { + t.Fatalf("%+v renewed the engine placed by hand", r) + } } p.heard(link.Report{Host: "2.0", Declared: "abc"}) if unproved(state, "2.0") { @@ -37,6 +47,23 @@ func TestOnlyAnAccountUnderThisBuildProvesIt(t *testing.T) { if got, _ := upgrade.ReadKnownGood(upgrade.KnownGoodPath(state)); got != "2.0" { t.Fatalf("known-good is %q", got) } + // **Proved, the engine placed by hand becomes this build** (issue 356): the command on the machine's PATH is + // the engine that runs it, and that is said. + self, err := os.Executable() + if err != nil { + t.Fatal(err) + } + want, _ := os.ReadFile(self) + if got, _ := os.ReadFile(fallback); len(want) == 0 || string(got) != string(want) { + t.Fatalf("the engine placed by hand was not renewed to this build (%d bytes, want %d)", len(got), len(want)) + } + renewed := false + for _, s := range said { + renewed = renewed || strings.Contains(s, "renewed the engine placed by hand at "+fallback) + } + if !renewed { + t.Fatalf("the renewal was not said: %q", said) + } } // Every verdict that stands is said, and a reconcile that has one says it unasked. diff --git a/internal/apply/apply.go b/internal/apply/apply.go index 02a98c0..e6c9e1e 100644 --- a/internal/apply/apply.go +++ b/internal/apply/apply.go @@ -1047,10 +1047,10 @@ func applyDirectory(r *declaration.Directory, previous store.Applied, wasBefore } if existed && wasBefore { ours := previous.Target != "" && filepath.Clean(previous.Target) == filepath.Clean(r.Path) && !previous.AsFound - // Or handed to the mesh by the operator (`nox node hand-over`, or `hand-over` at the machine): then it is given the - // declared owner and mode below, as any directory of the mesh's. + // Or handed to the mesh by the operator (`nox node hand-over`, or `hand-over` at the machine): then it is + // given the declared owner and mode — through a descriptor that followed no link (giveHandedOver). if !ours && HandedOver()[handOverKey(r.ID, r.Path)] { - ours = true + return applyHandedOver(r, out, mode) } if !ours { owned, err := ownedBy(r.Path, r.Owner) @@ -1122,6 +1122,45 @@ func applyDirectory(r *declaration.Directory, previous store.Applied, wasBefore return out, nil } +// applyHandedOver gives a directory the operator handed to the mesh its declared mode and owner (novox/hq issue +// 356). **No link, checked again at the moment of use**: the hand-over refused a link when it was recorded, and a +// component of the path may have become one since — a parent swapped for a link to /etc would carry root's chmod +// and chown there. So the path must still resolve to itself, and the change is made through one descriptor opened +// from / with no link followed on any component (giveHandedOver); a link anywhere is refused and nothing changes. +func applyHandedOver(r *declaration.Directory, out Outcome, mode os.FileMode) (Outcome, error) { + clean := filepath.Clean(r.Path) + resolved, err := filepath.EvalSymlinks(clean) + if err != nil { + return out, err + } + if resolved != clean { + return out, &PlacementRefusedError{Path: r.Path, Why: fmt.Sprintf("it was handed to the mesh, and is now "+ + "reached through a link (it is %s): the node-engine follows no link to a directory it gives an owner", resolved)} + } + uid, gid, chown := 0, 0, r.Owner != "" + if chown { + if uid, gid, err = idsOf(r.Owner); err != nil { + return out, fmt.Errorf("%s should belong to %q: %w", r.Path, r.Owner, err) + } + } + before, beforeUID, beforeGID, err := giveHandedOver(clean, mode, uid, gid, chown) + if err != nil { + var link *LinkUnderHomeError + if errors.As(err, &link) { + return out, &PlacementRefusedError{Path: r.Path, Why: fmt.Sprintf("it was handed to the mesh, and %s is "+ + "now a link: the node-engine follows no link to a directory it gives an owner", link.Link)} + } + return out, err + } + out.Action = "unchanged" + if before.Perm() != mode.Perm() || (chown && (beforeUID != uid || beforeGID != gid)) { + out.Action = "updated" + out.Detail = fmt.Sprintf("handed to the mesh: owner %d:%d, mode %o, given what was declared", beforeUID, beforeGID, + before.Perm()) + } + return out, nil +} + // applyAccess confirms an operator-owned path is present, and owns nothing about it. // // **The mirror image of applyDirectory** (novox/hq ADR 0051). A directory the host makes, chmods, diff --git a/internal/apply/found_is_said_test.go b/internal/apply/found_is_said_test.go index 3c5a530..5510466 100644 --- a/internal/apply/found_is_said_test.go +++ b/internal/apply/found_is_said_test.go @@ -136,3 +136,101 @@ func TestAHandOverIsForTheResourceThatAskedForIt(t *testing.T) { t.Fatalf("a hand-over for notes.data gave other.data the directory: mode %o", info.Mode().Perm()) } } + +// **A handed-over directory is changed through no link, at the moment of use** (review of issue 356). The +// hand-over refused a link when it was recorded; between then and the apply its parent is swapped for a link to +// a victim tree. The apply refuses, and the victim's directory keeps its mode — and so does a directory whose own +// name became a link, and one whose grandparent did. +func TestAHandedOverDirectoryIsChangedThroughNoLink(t *testing.T) { + for _, swap := range []string{"parent", "grandparent", "itself"} { + t.Run(swap, func(t *testing.T) { + dir, err := filepath.EvalSymlinks(t.TempDir()) + if err != nil { + t.Fatal(err) + } + found := filepath.Join(dir, "top", "parent", "data") + if err := os.MkdirAll(found, 0o700); err != nil { + t.Fatal(err) + } + decl := `{"id":"notes.data","type":"directory","path":"` + found + `","mode":"0755"}` + _, state, err := Apply(context.Background(), archHost(t), declare(t, decl), store.State{}, store.OriginDeclared, + noServices, nil, nil) + if err != nil { + t.Fatal(err) + } + if a, _ := state.Find("notes.data"); !a.AsFound { + t.Fatalf("not used as found: %+v", a) + } + before := HandedOver + HandedOver = func() map[string]bool { return map[string]bool{handOverKey("notes.data", found): true} } + t.Cleanup(func() { HandedOver = before }) + + // The victim: a tree of the same shape somewhere else, its data directory mode 0700. + victim := filepath.Join(dir, "victim", "top", "parent", "data") + if err := os.MkdirAll(victim, 0o700); err != nil { + t.Fatal(err) + } + var swapped, target string + switch swap { + case "parent": + swapped, target = filepath.Join(dir, "top", "parent"), filepath.Join(dir, "victim", "top", "parent") + case "grandparent": + swapped, target = filepath.Join(dir, "top"), filepath.Join(dir, "victim", "top") + case "itself": + swapped, target = found, victim + } + if err := os.Rename(swapped, swapped+".real"); err != nil { + t.Fatal(err) + } + if err := os.Symlink(target, swapped); err != nil { + t.Fatal(err) + } + _, _, err = Apply(context.Background(), archHost(t), declare(t, decl), state, store.OriginDeclared, + noServices, nil, nil) + if err == nil { + t.Fatal("a handed-over directory reached through a link was applied") + } + // Refused by the path's re-check before any descriptor is opened (a link at the name itself is refused + // earlier still, by the directory applier's own look at it). + want := "reached through a link" + if swap == "itself" { + want = "symbolic link" + } + if !strings.Contains(err.Error(), want) { + t.Fatalf("refused as %v; want %q", err, want) + } + if info, _ := os.Stat(victim); info.Mode().Perm() != 0o700 { + t.Fatalf("the victim's directory was given the declared mode through the link: %o", info.Mode().Perm()) + } + }) + } +} + +// The check at the moment of use, without the path's re-check: a component swapped for a link is refused by the +// descriptor walk itself, whatever was resolved a moment before. +func TestGiveHandedOverFollowsNoLink(t *testing.T) { + dir, err := filepath.EvalSymlinks(t.TempDir()) + if err != nil { + t.Fatal(err) + } + victim := filepath.Join(dir, "victim", "data") + if err := os.MkdirAll(victim, 0o700); err != nil { + t.Fatal(err) + } + if err := os.Symlink(filepath.Join(dir, "victim"), filepath.Join(dir, "parent")); err != nil { + t.Fatal(err) + } + if _, _, _, err := giveHandedOver(filepath.Join(dir, "parent", "data"), 0o755, 0, 0, false); err == nil { + t.Fatal("a directory reached through a link was changed") + } + if info, _ := os.Stat(victim); info.Mode().Perm() != 0o700 { + t.Fatalf("the victim was changed: %o", info.Mode().Perm()) + } + mode, _, _, err := giveHandedOver(victim, 0o750, 0, 0, false) + if err != nil || mode.Perm() != 0o700 { + t.Fatalf("the directory itself: before %o, %v", mode, err) + } + if info, _ := os.Stat(victim); info.Mode().Perm() != 0o750 { + t.Fatalf("the directory itself was not given its mode: %o", info.Mode().Perm()) + } +} diff --git a/internal/apply/homes_linux.go b/internal/apply/homes_linux.go index 58ad05a..79d2844 100644 --- a/internal/apply/homes_linux.go +++ b/internal/apply/homes_linux.go @@ -239,3 +239,31 @@ func writeUnder(home, path string, content []byte, mode os.FileMode) error { } return werr } + +// giveHandedOver gives a directory the operator handed to the mesh its declared mode and owner (novox/hq issue +// 356, issue 339) through one descriptor opened from / with no link followed on any component (openUnderAs with / +// as the home), judged a directory by fstat on that descriptor, and changed with fchmod and fchown on it. The +// hand-over refused a link when it was recorded; this refuses one at the moment of use, so a component swapped for +// a link between the record and the apply cannot carry root's chmod and chown to wherever it points. Returns the +// mode and owner the directory had before. +func giveHandedOver(path string, mode os.FileMode, uid, gid int, chown bool) (before os.FileMode, beforeUID, beforeGID int, err error) { + fd, err := openUnderAs("/", path, kindDir) + if err != nil { + return 0, 0, 0, err + } + defer unix.Close(fd) + var st unix.Stat_t + if err := unix.Fstat(fd, &st); err != nil { + return 0, 0, 0, &os.PathError{Op: "fstat", Path: path, Err: err} + } + before, beforeUID, beforeGID = os.FileMode(st.Mode&0o777), int(st.Uid), int(st.Gid) + if err := unix.Fchmod(fd, uint32(mode.Perm())); err != nil { + return 0, 0, 0, &os.PathError{Op: "chmod", Path: path, Err: err} + } + if chown && (beforeUID != uid || beforeGID != gid) { + if err := unix.Fchown(fd, uid, gid); err != nil { + return 0, 0, 0, &os.PathError{Op: "chown", Path: path, Err: err} + } + } + return before, beforeUID, beforeGID, nil +} diff --git a/internal/apply/homes_other.go b/internal/apply/homes_other.go index 64974f1..9874386 100644 --- a/internal/apply/homes_other.go +++ b/internal/apply/homes_other.go @@ -57,3 +57,10 @@ func writeUnder(home, path string, content []byte, mode os.FileMode) error { } return writeAtomicallyByPath(path, content, mode) } + +// giveHandedOver is refused where there is no openat with O_NOFOLLOW: a handed-over directory is never changed by +// path (novox/hq issue 356). The node-engine runs on Linux. +func giveHandedOver(path string, _ os.FileMode, _, _ int, _ bool) (os.FileMode, int, int, error) { + return 0, 0, 0, &PlacementRefusedError{Path: path, Why: "a handed-over directory is changed only through a " + + "descriptor that followed no link, which this system cannot open"} +} diff --git a/internal/link/handover.go b/internal/link/handover.go index 255f939..aa7cd22 100644 --- a/internal/link/handover.go +++ b/internal/link/handover.go @@ -1,32 +1,63 @@ package link import ( + "crypto/ed25519" + "encoding/hex" "encoding/json" "fmt" "strings" + "sync" + "time" ) // A hand-over asked over the bus (novox/hq issue 356, issue 339). // // A directory used as found is handed to the mesh by its operator, at the controller's terminal: // `nox node hand-over ` on the control-node (novox/hq ADR 0272). The controller asks this -// node's engine on AskHandOverSubject — a request on core NATS, its own machine's subject, which only the -// controller may publish — and the engine answers once, on the reply the request carried. The engine -// judges every value it is sent and records the hand-over, or refuses and records nothing; the next -// apply gives the directory its declared owner and mode. The controller holds the same two shapes in its -// own link code, with a test on each side holding the field names. +// node's engine on AskHandOverSubject, a request on core NATS, and the engine answers once, on the reply +// the request carried. +// +// **The subject proves nothing; the signature does** (review of issue 356). Only the controller is granted +// a publish on the subject, but the bus lets any principal allowed to answer reply to a message it received, +// on whatever reply subject that message named — a tool server asked on its own subject with this reply +// would deliver a body of its choosing here. So the controller signs the ask with the mesh's key, over +// HandOverContext and the ask's bytes, and the engine verifies it with the key it verifies declarations with +// before it reads anything out of it; then it holds the ask to this node, to its expiry, and to a nonce it +// takes once. Then it judges every value and records the hand-over, or refuses and records nothing; the next +// apply gives the directory its declared owner and mode. The controller holds the same shapes in its own link +// code, with a test on each side holding the field names. // AskHandOverSubject is where the controller asks this node's engine to take a directory used as found as // the mesh's. func AskHandOverSubject(node string) string { return "mesh.node." + node + ".ask.hand-over" } -// HandOverAsk is one hand-over asked: the directory's absolute path as the engine states it, and who asked, -// in the controller's words. +// HandOverAsk is one hand-over asked: the node it is for, the directory's absolute path as the engine states +// it, who asked in the controller's words, when it stops being good, and a nonce taken once. type HandOverAsk struct { - Path string `json:"path"` - By string `json:"by"` + Node string `json:"node"` + Path string `json:"path"` + By string `json:"by"` + Expires time.Time `json:"expires"` + Nonce string `json:"nonce"` } +// SignedHandOver is the ask as it travels: its bytes exactly as signed, and the mesh's signature. +type SignedHandOver struct { + Ask []byte `json:"ask"` + Signature []byte `json:"signature"` +} + +// HandOverContext is prefixed to an ask's bytes before signing: a hand-over's signature is never a +// declaration's, which the same key makes over a declaration's bytes alone. +const HandOverContext = "novox-mesh hand-over v1\n" + +// HandOverGood is how long the controller makes an ask good for; HandOverSkew is how far the machines' clocks +// may differ. An ask whose expiry is further ahead than both together is refused: it would be good too long. +const ( + HandOverGood = time.Minute + HandOverSkew = time.Minute +) + // HandOverAnswer is the engine's answer: what it recorded, or why it refused — never both, never neither. type HandOverAnswer struct { Said string `json:"said,omitempty"` @@ -36,34 +67,101 @@ type HandOverAnswer struct { // HandOverFunc judges and records one hand-over: the words to answer with, or the refusal. type HandOverFunc func(path, by string) (string, error) -// MaxHandOverAsk bounds a request: a path and a name are hundreds of bytes. +// MaxHandOverAsk bounds a request: a path, a name, a time, a nonce and a signature are hundreds of bytes. const MaxHandOverAsk = 8 << 10 // MaxHandOverBy bounds who asked. const MaxHandOverBy = 200 -// AnswerHandOver is the engine's judgement of one ask's bytes: every value is checked before anything is -// recorded, and a refusal records nothing. The handler is given only what passed. -func AnswerHandOver(handle HandOverFunc, body []byte) HandOverAnswer { +// HandOverCheck is what an ask is held to: the mesh's key, this node's name, the clock, and the nonces taken. +type HandOverCheck struct { + Signer ed25519.PublicKey + Node string + Now func() time.Time + Nonces *Nonces +} + +// Nonces are the nonces of asks taken, each kept until its ask expires: an ask is taken once. Kept in memory — +// an ask outlives no restart by more than its minute, and taking one again records the same hand-over again. +type Nonces struct { + mu sync.Mutex + seen map[string]time.Time +} + +// take records a nonce until its expiry, and says false when it was already taken. +func (n *Nonces) take(nonce string, expires, now time.Time) bool { + n.mu.Lock() + defer n.mu.Unlock() + if n.seen == nil { + n.seen = map[string]time.Time{} + } + for k, until := range n.seen { + if now.After(until) { + delete(n.seen, k) + } + } + if _, taken := n.seen[nonce]; taken { + return false + } + n.seen[nonce] = expires + return true +} + +// HandOverNonces are this process's: every link it opens takes from the same set, so a reconnect is no replay. +var HandOverNonces = &Nonces{} + +func refusedAsk(why string) HandOverAnswer { + return HandOverAnswer{Refused: why + "; nothing was handed over"} +} + +// AnswerHandOver is the engine's judgement of one ask's bytes: the signature first, then this node, the expiry +// and the nonce, then every value — and only then the handler, given only what passed. A refusal records nothing. +func AnswerHandOver(check HandOverCheck, handle HandOverFunc, body []byte) HandOverAnswer { if handle == nil { - return HandOverAnswer{Refused: "this engine takes no hand-over"} + return refusedAsk("this engine takes no hand-over") } if len(body) > MaxHandOverAsk { - return HandOverAnswer{Refused: fmt.Sprintf("the ask is %d bytes, more than a path and a name; nothing was handed over", - len(body))} + return refusedAsk(fmt.Sprintf("the ask is %d bytes, more than a hand-over", 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()) + } + 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") } var ask HandOverAsk - if err := json.Unmarshal(body, &ask); err != nil { - return HandOverAnswer{Refused: "the ask is not a hand-over: " + err.Error() + "; nothing was handed over"} + if err := json.Unmarshal(signed.Ask, &ask); err != nil { + return refusedAsk("the signed ask is not a hand-over: " + err.Error()) + } + now := time.Now() + if check.Now != nil { + now = check.Now() + } + nonces := check.Nonces + if nonces == nil { + nonces = HandOverNonces } 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)) + case ask.Expires.IsZero() || !now.Before(ask.Expires): + return refusedAsk("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") + case nonceErr != nil || len(nonce) < 16: + return refusedAsk("the ask carries no nonce of its own") case by == "": - return HandOverAnswer{Refused: "the ask names nobody as asking; nothing was handed over"} + return refusedAsk("the ask names nobody as asking") case len(by) > MaxHandOverBy || strings.ContainsAny(by, "\r\n\x00"): - return HandOverAnswer{Refused: "who asked is not one short line; nothing was handed over"} + return refusedAsk("who asked is not one short line") case ask.Path == "": - return HandOverAnswer{Refused: "the ask names no directory; nothing was handed over"} + return refusedAsk("the ask names no directory") + case !nonces.take(ask.Nonce, ask.Expires, now): + return refusedAsk("the ask was taken already") } said, err := handle(ask.Path, by) if err != nil { @@ -72,6 +170,19 @@ func AnswerHandOver(handle HandOverFunc, body []byte) HandOverAnswer { return HandOverAnswer{Said: said} } +// HandOverReply is what a hand-over request is answered with, and whether to answer: a request with no reply +// subject is nobody's question, so nothing is judged, recorded or said. +func HandOverReply(check HandOverCheck, handle HandOverFunc, reply string, body []byte) ([]byte, bool) { + if reply == "" { + return nil, false + } + answer, err := json.Marshal(AnswerHandOver(check, handle, body)) + if err != nil { + return nil, false + } + return answer, true +} + // HandOvers is a link that hears the controller asking this node to hand a directory over. Optional, as // Asked is. type HandOvers interface { diff --git a/internal/link/handover_test.go b/internal/link/handover_test.go index ab272c7..eaa284f 100644 --- a/internal/link/handover_test.go +++ b/internal/link/handover_test.go @@ -1,20 +1,27 @@ package link import ( + "context" + "crypto/ed25519" "encoding/json" "errors" "sort" "strings" "testing" + "time" ) // The hand-over's ask and answer hold these field names; the controller's side holds the same list // (mesh-controller internal/link, TestTheHandOverAskAndAnswerKeepTheirFieldNames). func TestTheHandOverAskAndAnswerKeepTheirFieldNames(t *testing.T) { - body, _ := json.Marshal(HandOverAsk{Path: "/srv/notes", By: "jo through mesh-cli on anchor"}) - if got := keysOf(t, body); got != "by path" { + body, _ := json.Marshal(HandOverAsk{Node: "laptop", Path: "/srv/notes", By: "jo", Expires: time.Now(), Nonce: "n"}) + if got := keysOf(t, body); got != "by expires node nonce path" { t.Fatalf("the ask's fields are %q", got) } + body, _ = json.Marshal(SignedHandOver{Ask: []byte("{}"), Signature: []byte("s")}) + if got := keysOf(t, body); got != "ask signature" { + t.Fatalf("the signed ask's fields are %q", got) + } body, _ = json.Marshal(HandOverAnswer{Said: "s", Refused: "r"}) if got := keysOf(t, body); got != "refused said" { t.Fatalf("the answer's fields are %q", got) @@ -22,6 +29,9 @@ func TestTheHandOverAskAndAnswerKeepTheirFieldNames(t *testing.T) { if AskHandOverSubject("laptop") != "mesh.node.laptop.ask.hand-over" { t.Fatalf("the subject is %q", AskHandOverSubject("laptop")) } + if HandOverContext != "novox-mesh hand-over v1\n" { + t.Fatalf("the signing context is %q; the controller holds the same words", HandOverContext) + } } func keysOf(t *testing.T, body []byte) string { @@ -38,52 +48,193 @@ func keysOf(t *testing.T, body []byte) string { return strings.Join(keys, " ") } -// Every value of an ask is judged before the handler sees it, and a refusal hands nothing to the handler: -// an ask too large, not a hand-over, naming nobody, naming somebody over more than a line, or naming no -// directory. What passes reaches the handler with its path as given and who asked trimmed. -func TestAHandOverAskIsJudgedBeforeAnythingIsRecorded(t *testing.T) { +const aNonce = "00112233445566778899aabbccddeeff" + +// signedAsk is an ask as the controller makes it, signed with key over the context and its bytes. +func signedAsk(t *testing.T, key ed25519.PrivateKey, ask HandOverAsk) []byte { + t.Helper() + raw, err := json.Marshal(ask) + if err != nil { + t.Fatal(err) + } + body, err := json.Marshal(SignedHandOver{Ask: raw, Signature: ed25519.Sign(key, append([]byte(HandOverContext), raw...))}) + if err != nil { + t.Fatal(err) + } + return body +} + +// anAsk is a good ask at now. +func anAsk(now time.Time) HandOverAsk { + return HandOverAsk{Node: "n1", Path: "/srv/notes", By: " jo through mesh-cli on anchor ", Expires: now.Add(HandOverGood), + Nonce: aNonce} +} + +// **An ask is believed only signed by the mesh, for this node, unexpired, once** (review of issue 356): a +// responder can reach the subject through a reply, so the subject proves nothing. Each refusal below hands +// the handler nothing; the good ask reaches it with its path and who asked, and the same ask again is refused. +func TestAHandOverAskIsBelievedOnlySignedForThisNodeUnexpiredAndOnce(t *testing.T) { + m, key := aMember(t) + _, stranger, _ := ed25519.GenerateKey(nil) + now := time.Date(2026, 10, 9, 18, 0, 0, 0, time.UTC) var given []string handle := func(path, by string) (string, error) { given = append(given, path+" by "+by) if path == "/refused" { - return "", errors.New("not a directory this machine uses as found") + return "", errors.New("not a directory this machine uses as found; nothing was handed over") } return path + " is handed to the mesh", nil } + check := HandOverCheck{Signer: m.Signer, Node: m.Node, Now: func() time.Time { return now }, Nonces: &Nonces{}} + with := func(change func(*HandOverAsk)) HandOverAsk { + a := anAsk(now) + change(&a) + return a + } + good, _ := json.Marshal(anAsk(now)) + unsignedBody, _ := json.Marshal(SignedHandOver{Ask: good}) + // Signed as a declaration is, over the bytes alone: not a hand-over's signature. + asDeclaration, _ := json.Marshal(SignedHandOver{Ask: good, Signature: ed25519.Sign(key, good)}) refusals := map[string][]byte{ - "too large": []byte(`{"path":"/` + strings.Repeat("a", MaxHandOverAsk) + `","by":"jo"}`), - "not a hand-over": []byte(`not json`), - "nobody": []byte(`{"path":"/srv/notes","by":" "}`), - "two lines": []byte(`{"path":"/srv/notes","by":"jo\nroot"}`), - "a name too long": []byte(`{"path":"/srv/notes","by":"` + strings.Repeat("x", MaxHandOverBy+1) + `"}`), - "no directory": []byte(`{"path":"","by":"jo"}`), - "a NUL in the name": []byte(`{"path":"/srv/notes","by":"jo\u0000"}`), + "too large": signedAsk(t, key, with(func(a *HandOverAsk) { a.Path = "/" + strings.Repeat("a", MaxHandOverAsk) })), + "not a signed hand-over": []byte(`not json`), + "unsigned": unsignedBody, + "signed by another key": signedAsk(t, stranger, anAsk(now)), + "signed as a declaration": asDeclaration, + "the bare ask": good, + "for another node": signedAsk(t, key, with(func(a *HandOverAsk) { a.Node = "n2" })), + "expired": signedAsk(t, key, with(func(a *HandOverAsk) { a.Expires = now })), + "no expiry": signedAsk(t, key, with(func(a *HandOverAsk) { a.Expires = time.Time{} })), + "good for too long": signedAsk(t, key, with(func(a *HandOverAsk) { a.Expires = now.Add(HandOverGood + HandOverSkew + time.Second) })), + "no nonce": signedAsk(t, key, with(func(a *HandOverAsk) { a.Nonce = "" })), + "a short nonce": signedAsk(t, key, with(func(a *HandOverAsk) { a.Nonce = "0011" })), + "nobody": signedAsk(t, key, with(func(a *HandOverAsk) { a.By = " " })), + "two lines": signedAsk(t, key, with(func(a *HandOverAsk) { a.By = "jo\nroot" })), + "a name too long": signedAsk(t, key, with(func(a *HandOverAsk) { a.By = strings.Repeat("x", MaxHandOverBy+1) })), + "a NUL in the name": signedAsk(t, key, with(func(a *HandOverAsk) { a.By = "jo\x00" })), + "no directory": signedAsk(t, key, with(func(a *HandOverAsk) { a.Path = "" })), } for name, body := range refusals { - a := AnswerHandOver(handle, body) - if a.Refused == "" || a.Said != "" { - t.Fatalf("%s was answered %+v; want a refusal", name, a) - } - if !strings.Contains(a.Refused, "nothing was handed over") { - t.Fatalf("%s's refusal does not say nothing was handed over: %q", name, a.Refused) + a := AnswerHandOver(check, handle, body) + if a.Refused == "" || a.Said != "" || !strings.Contains(a.Refused, "nothing was handed over") { + t.Fatalf("%s was answered %+v; want a refusal saying nothing was handed over", name, a) } } if len(given) != 0 { t.Fatalf("the handler was given %v although every ask was refused", given) } - if a := AnswerHandOver(nil, []byte(`{"path":"/srv/notes","by":"jo"}`)); a.Refused == "" { + if a := AnswerHandOver(HandOverCheck{Node: "n1", Nonces: &Nonces{}}, handle, signedAsk(t, key, anAsk(time.Now()))); a.Refused == "" { + t.Fatal("an engine with no key believed an ask") + } + if a := AnswerHandOver(check, nil, signedAsk(t, key, anAsk(now))); a.Refused == "" { t.Fatal("an engine with no handler took a hand-over") } - a := AnswerHandOver(handle, []byte(`{"path":"/refused","by":"jo"}`)) - if a.Refused != "not a directory this machine uses as found" || a.Said != "" { - t.Fatalf("the handler's refusal was answered as %+v", a) - } - a = AnswerHandOver(handle, []byte(`{"path":"/srv/notes","by":" jo through mesh-cli on anchor "}`)) + body := signedAsk(t, key, anAsk(now)) + a := AnswerHandOver(check, handle, body) if a.Said != "/srv/notes is handed to the mesh" || a.Refused != "" { - t.Fatalf("answered %+v", a) + t.Fatalf("the good ask was answered %+v", a) } - if strings.Join(given, "; ") != "/refused by jo; /srv/notes by jo through mesh-cli on anchor" { + if again := AnswerHandOver(check, handle, body); !strings.Contains(again.Refused, "taken already") { + t.Fatalf("the same ask again was answered %+v", again) + } + refused := signedAsk(t, key, with(func(a *HandOverAsk) { a.Path = "/refused"; a.Nonce = strings.Repeat("ab", 16) })) + if a := AnswerHandOver(check, handle, refused); !strings.HasPrefix(a.Refused, "not a directory") { + t.Fatalf("the handler's refusal was answered %+v", a) + } + if strings.Join(given, "; ") != "/srv/notes by jo through mesh-cli on anchor; /refused by jo through mesh-cli on anchor" { t.Fatalf("the handler was given %v", given) } + // A nonce is forgotten once its ask has expired, so the set does not grow for ever. + now = now.Add(HandOverGood + time.Second) + check.Nonces.take("ff", now.Add(time.Second), now) + if len(check.Nonces.seen) != 1 { + t.Fatalf("expired nonces are kept: %v", check.Nonces.seen) + } +} + +// **A request with no reply is nobody's question**: nothing is judged, recorded or said. One with a reply is +// answered with the judgement. +func TestAHandOverIsAnsweredOnlyWhenItCarriesAReply(t *testing.T) { + m, key := aMember(t) + now := time.Now() + called := 0 + handle := func(string, string) (string, error) { called++; return "handed", nil } + check := HandOverCheck{Signer: m.Signer, Node: m.Node, Nonces: &Nonces{}} + if body, answer := HandOverReply(check, handle, "", signedAsk(t, key, anAsk(now))); answer || body != nil || called != 0 { + t.Fatalf("a request with no reply was answered %q %v, handled %d", body, answer, called) + } + body, answer := HandOverReply(check, handle, "_INBOX.x", signedAsk(t, key, anAsk(now))) + var said HandOverAnswer + if !answer || json.Unmarshal(body, &said) != nil || said.Said != "handed" || called != 1 { + t.Fatalf("answered %q %v, handled %d", body, answer, called) + } +} + +// handingLink is a quiet link that hears hand-overs, keeping the handler it was given. +type handingLink struct { + *quietLink + given chan HandOverFunc +} + +func (l handingLink) HearHandOvers(h HandOverFunc) { l.given <- h } + +// **The link is given the queue's hand-over** when it is served: what the controller asks reaches the engine's +// judgement, and nothing else. +func TestALinkThatHearsHandOversIsGivenTheQueues(t *testing.T) { + m, _ := aMember(t) + l := handingLink{quietLink: newQuietLink(), given: make(chan HandOverFunc, 1)} + q := aQueue(m, (&applies{}).apply, &keptInMemory{}) + q.HandOver = func(path, by string) (string, error) { return "the queue's: " + path + " by " + by, nil } + ctx, stop := context.WithCancel(context.Background()) + served := make(chan error, 1) + go func() { served <- serve(ctx, l, m, q, nil, time.Second) }() + select { + case h := <-l.given: + if said, err := h("/srv/notes", "jo"); err != nil || said != "the queue's: /srv/notes by jo" { + t.Fatalf("the link was given another handler: %q %v", said, err) + } + case <-time.After(5 * time.Second): + t.Fatal("the link was never given the queue's hand-over") + } + stop() + <-served +} + +// **Against a real server**: a signed request on the node's own subject is answered; one on another node's is +// not heard; a message with no reply is answered by nobody. +func TestNatsAHandOverIsAskedOfTheNode(t *testing.T) { + conn, js := aBus(t) + m, key := aMember(t) + l := &natsLink{conn: conn, js: js, node: m.Node, signer: m.Signer} + handled := make(chan string, 4) + l.HearHandOvers(func(path, by string) (string, error) { handled <- path; return "handed " + path, nil }) + defer l.Close() + if err := conn.Flush(); err != nil { + t.Fatal(err) + } + ask := anAsk(time.Now()) + ask.Nonce = strings.Repeat("cd", 16) + reply, err := conn.Request(AskHandOverSubject(m.Node), signedAsk(t, key, ask), 5*time.Second) + if err != nil { + t.Fatal(err) + } + var said HandOverAnswer + if err := json.Unmarshal(reply.Data, &said); err != nil || said.Said != "handed /srv/notes" { + t.Fatalf("answered %s, %v", reply.Data, err) + } + ask.Nonce = strings.Repeat("ef", 16) + if err := conn.Publish(AskHandOverSubject(m.Node), signedAsk(t, key, ask)); err != nil { + t.Fatal(err) + } + if _, err := conn.Request(AskHandOverSubject("another"), signedAsk(t, key, ask), 300*time.Millisecond); err == nil { + t.Fatal("another node's ask was answered here") + } + if err := conn.Flush(); err != nil { + t.Fatal(err) + } + time.Sleep(100 * time.Millisecond) + if len(handled) != 1 { + t.Fatalf("handled %d asks; want only the one with a reply", len(handled)) + } } diff --git a/internal/link/hearing_nats.go b/internal/link/hearing_nats.go index 07708b0..1466a33 100644 --- a/internal/link/hearing_nats.go +++ b/internal/link/hearing_nats.go @@ -2,7 +2,7 @@ package link import ( "context" - "encoding/json" + "crypto/ed25519" "errors" "fmt" "strings" @@ -63,6 +63,8 @@ type natsLink struct { asked chan struct{} handing *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 lost chan error } @@ -112,7 +114,7 @@ func dialNats(ctx context.Context, m Membership, timeout time.Duration) (Link, e } l := &natsLink{ - conn: conn, js: js, node: m.Node, + conn: conn, js: js, node: m.Node, signer: m.Signer, arrived: make(chan Declaration, drainDepth), lost: make(chan error, 1), asked: make(chan struct{}, 1), @@ -195,24 +197,20 @@ func (l *natsLink) hearAsks() { } // HearHandOvers listens for the controller asking this node to hand a directory used as found to the mesh -// (novox/hq issue 356): a request on this machine's own subject, answered once on the reply it carried. -// The engine judges the ask (AnswerHandOver) and runs nothing it is sent. A bus whose user list is older -// than the grant refuses the subscription asynchronously, said by the client; the operator's line then -// says that nothing on this node answers. +// (novox/hq issue 356): a request on this machine's own subject, answered once on the reply it carried. The +// ask is verified with the mesh's key before anything is read out of it (AnswerHandOver), because a responder +// can reach this subject through a reply; the engine runs nothing it is sent. A bus whose user list is older +// than the grant refuses the subscription asynchronously, said by the client; the operator's line then says +// that nothing on this node answers. func (l *natsLink) HearHandOvers(handle HandOverFunc) { if handle == nil { return } + check := HandOverCheck{Signer: l.signer, Node: l.node} handing, err := l.conn.Subscribe(AskHandOverSubject(l.node), func(msg *nats.Msg) { - if msg.Reply == "" { - // Nobody to answer: nothing is handed over on a word nobody waits for. - return + if body, answer := HandOverReply(check, handle, msg.Reply, msg.Data); answer { + _ = msg.Respond(body) } - body, err := json.Marshal(AnswerHandOver(handle, msg.Data)) - if err != nil { - return - } - _ = msg.Respond(body) }) if err == nil { l.handing = handing diff --git a/internal/upgrade/fallback.go b/internal/upgrade/fallback.go index 73a6e0a..edd0bdd 100644 --- a/internal/upgrade/fallback.go +++ b/internal/upgrade/fallback.go @@ -41,8 +41,8 @@ func FallbackPath() string { // RenewFallback makes the file at fallback a copy of the executable at self, when it is not one already. // -// Nothing is written when the two are the same file (an engine running from the fallback itself, while -// nothing is delivered) or hold the same bytes. A fallback that is a link is refused: the mesh writes +// Nothing is written when the two hold the same bytes — which an engine running from the fallback itself, while +// nothing is delivered, always does. A fallback that is a link is refused: the mesh writes // through no link, and a link there would be somebody else's arrangement. Reports whether it was renewed. func RenewFallback(self, fallback string) (bool, error) { if self == "" || fallback == "" { @@ -66,8 +66,6 @@ func RenewFallback(self, fallback string) (bool, error) { "it is left as it is", fallback) case !placed.Mode().IsRegular(): return false, fmt.Errorf("%s is not a regular file: it is left as it is", fallback) - case os.SameFile(mine, placed): - return false, nil default: same, err := sameBytes(self, fallback) if err != nil { diff --git a/internal/upgrade/fallback_test.go b/internal/upgrade/fallback_test.go index a96a01f..a738b47 100644 --- a/internal/upgrade/fallback_test.go +++ b/internal/upgrade/fallback_test.go @@ -152,3 +152,35 @@ func TestTheFallbackPathIsTheLaunchers(t *testing.T) { t.Fatalf("the override is %q", FallbackPath()) } } + +// A fallback that is not a regular file — a directory where the engine should be — is refused and left: the mesh +// replaces a file, never something else at its name. +func TestAFallbackThatIsNotARegularFileIsRefused(t *testing.T) { + dir := t.TempDir() + self := filepath.Join(dir, "v1", BinaryName) + if err := os.MkdirAll(filepath.Dir(self), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(self, []byte("proved"), 0o755); err != nil { + t.Fatal(err) + } + fallback := filepath.Join(dir, BinaryName) + if err := os.Mkdir(fallback, 0o755); err != nil { + t.Fatal(err) + } + renewed, err := RenewFallback(self, fallback) + if err == nil || renewed || !strings.Contains(err.Error(), "not a regular file") { + t.Fatalf("a directory at the fallback was taken: %v, %v", renewed, err) + } + if info, err := os.Lstat(fallback); err != nil || !info.IsDir() { + t.Fatalf("the directory was replaced: %v %v", info, err) + } + // And a running engine that is not a regular file is refused before anything is written. + if _, err := RenewFallback(dir, filepath.Join(dir, "elsewhere")); err == nil || !strings.Contains(err.Error(), + "not a regular file") { + t.Fatalf("a directory was taken as the running engine: %v", err) + } + if _, err := os.Lstat(filepath.Join(dir, "elsewhere")); err == nil { + t.Fatal("something was placed from a directory") + } +}