From b55a38ca9fa2f4c83816ec27dc27cac58210ad5b Mon Sep 17 00:00:00 2001 From: jochen Date: Fri, 9 Oct 2026 19:44:55 +0200 Subject: [PATCH] Sign the hand-over ask, and fail the line on the engine's refusal (hq issue 356, review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The subject proved nothing: the bus lets any principal allowed to answer reply to a message it received on the reply subject that message named, so a tool server — the operator's account, every agent — could deliver a hand-over to an engine. The controller now signs the ask with the mesh's key over a fixed context (node, path, who asked, a minute's expiry, a fresh nonce), as declarations are signed, and the engine verifies it. The writers table gains the row for mesh.node.*.ask.hand-over; the subject's comment no longer claims who the engine hears. The line's known-node check and the refusal branch are tested; every check was removed in turn and a test failed. --- cmd/mesh-controller/handover_test.go | 44 +++++++++++++ cmd/mesh-controller/nodes.go | 54 +++++++++++----- internal/broker/nats.go | 11 +++- internal/broker/writers.go | 7 +++ internal/broker/writers_test.go | 20 ++++++ internal/link/handover.go | 60 ++++++++++++++++-- internal/link/handover_test.go | 92 +++++++++++++++++++++++++--- 7 files changed, 253 insertions(+), 35 deletions(-) diff --git a/cmd/mesh-controller/handover_test.go b/cmd/mesh-controller/handover_test.go index f48dd83e..740ea7e9 100644 --- a/cmd/mesh-controller/handover_test.go +++ b/cmd/mesh-controller/handover_test.go @@ -1,6 +1,8 @@ package main import ( + "bytes" + "errors" "strings" "testing" "time" @@ -88,3 +90,45 @@ func TestTheUsedAsFoundConditionNamesTheOperatorsLine(t *testing.T) { t.Fatalf("the module's health reads %v %q; want the operator's line", h, why) } } + +// **Nothing is asked of a node the mesh does not know, and the engine's refusal is the command's failure** +// (review of issue 356): a refused hand-over never exits as a success. +func TestAHandOverAsksOnlyAKnownNodeAndFailsOnARefusal(t *testing.T) { + t.Setenv(link.CallerVar, "jo through mesh-cli on anchor") + asked := 0 + ask := func(answer link.HandOverAnswer) func(node, path, by string) (link.HandOverAnswer, error) { + return func(node, path, by string) (link.HandOverAnswer, error) { + asked++ + if node != "laptop" || path != "/srv/notes" || by != "jo through mesh-cli on anchor" { + t.Fatalf("asked %q %q %q", node, path, by) + } + return answer, nil + } + } + unknown := func(string) error { return errors.New("no node called laptop") } + known := func(string) error { return nil } + var out bytes.Buffer + err := handOverAsked([]string{"laptop", "/srv/notes"}, unknown, ask(link.HandOverAnswer{Said: "x"}), &out) + if err == nil || asked != 0 || !strings.Contains(err.Error(), "nothing was asked") { + t.Fatalf("an unknown node: %v, asked %d", err, asked) + } + err = handOverAsked([]string{"laptop", "/srv/../etc"}, known, ask(link.HandOverAnswer{Said: "x"}), &out) + if err == nil || asked != 0 { + t.Fatalf("a refused line was asked: %v, asked %d", err, asked) + } + err = handOverAsked([]string{"laptop", "/srv/notes"}, known, + ask(link.HandOverAnswer{Refused: "/srv/notes is not used as found; nothing was handed over"}), &out) + if err == nil || !strings.Contains(err.Error(), "laptop refused: /srv/notes is not used as found") || out.Len() != 0 { + t.Fatalf("a refusal: %v, printed %q", err, out.String()) + } + err = handOverAsked([]string{"laptop", "/srv/notes"}, known, ask(link.HandOverAnswer{Said: "handed over"}), &out) + if err != nil || !strings.HasPrefix(out.String(), "handed over\n") || !strings.Contains(out.String(), "`nox push laptop`") { + t.Fatalf("a record: %v, printed %q", err, out.String()) + } + failing := func(string, string, string) (link.HandOverAnswer, error) { + return link.HandOverAnswer{}, errors.New("no engine") + } + if err := handOverAsked([]string{"laptop", "/srv/notes"}, known, failing, &out); err == nil { + t.Fatal("an ask that failed was a success") + } +} diff --git a/cmd/mesh-controller/nodes.go b/cmd/mesh-controller/nodes.go index 62f80756..ee878a9b 100644 --- a/cmd/mesh-controller/nodes.go +++ b/cmd/mesh-controller/nodes.go @@ -118,7 +118,7 @@ func nodeCommand(ctx context.Context, args []string) error { // A directory the node-engine uses as found, handed to the mesh (novox/hq issue 356, issue 339). Here, at // the controller's terminal, and nowhere else: at the next apply root gives the directory to the account // the module declares, and whoever may call a verb includes agents. - return nodeHandOver(ctx, inv, args[1:]) + return nodeHandOver(ctx, open, args[1:]) default: return fmt.Errorf("node has no %q; it has add, list, show, public-domain, account, agent-account and hand-over", args[0]) @@ -160,33 +160,53 @@ func handOverBy() string { // nodeHandOver asks the node's engine to take a directory it uses as found as the mesh's, and says what came of // it. The engine records the hand-over or refuses; nothing is recorded here, because the directory is the -// machine's and the engine is the one that reads it. -func nodeHandOver(ctx context.Context, inv *inventory.Inventory, args []string) error { +// machine's and the engine is the one that reads it. The ask is signed with the mesh's key (issue 356's review). +func nodeHandOver(ctx context.Context, open *stores, args []string) error { + known := func(node string) error { + _, err := open.inventory.NodeByName(ctx, node) + return err + } + ask := func(node, path, by string) (link.HandOverAnswer, error) { + ident, err := open.Identity(ctx) + if err != nil { + return link.HandOverAnswer{}, fmt.Errorf("the mesh's signing key cannot be read, so nothing was asked of %s: %w", + node, err) + } + address, err := broker.BusAddress() + if err != nil { + return link.HandOverAnswer{}, err + } + js, err := broker.Dial(address) + if err != nil { + return link.HandOverAnswer{}, fmt.Errorf("cannot reach the bus, so nothing was asked of %s: %w", node, err) + } + defer js.Close() + return link.AskHandOver(ctx, js.Conn(), ident, node, path, by, link.HandOverWithin) + } + return handOverAsked(args, known, ask, os.Stdout) +} + +// handOverAsked is the hand-over's line with its two acts given: whether the mesh knows the node, and the ask. +// Nothing is asked of a line or a node that is refused, and the engine's refusal is this command's failure — +// never a success with the refusal printed. +func handOverAsked(args []string, known func(node string) error, + ask func(node, path, by string) (link.HandOverAnswer, error), out io.Writer) error { node, path, err := handOverLine(args) if err != nil { return err } - if _, err := inv.NodeByName(ctx, node); err != nil { - return err + if err := known(node); err != nil { + return fmt.Errorf("nothing was asked: %w", err) } - address, err := broker.BusAddress() - if err != nil { - return err - } - js, err := broker.Dial(address) - if err != nil { - return fmt.Errorf("cannot reach the bus, so nothing was asked of %s: %w", node, err) - } - defer js.Close() - answer, err := link.AskHandOver(ctx, js.Conn(), node, path, handOverBy(), link.HandOverWithin) + answer, err := ask(node, path, handOverBy()) if err != nil { return err } if answer.Refused != "" { return fmt.Errorf("%s refused: %s", node, answer.Refused) } - fmt.Println(answer.Said) - fmt.Printf(" the module's condition clears once %s applies; `nox push %s` applies it now\n", node, node) + fmt.Fprintln(out, answer.Said) + fmt.Fprintf(out, " the module's condition clears once %s applies; `nox push %s` applies it now\n", node, node) return nil } diff --git a/internal/broker/nats.go b/internal/broker/nats.go index 19475706..156e9d3e 100644 --- a/internal/broker/nats.go +++ b/internal/broker/nats.go @@ -291,7 +291,10 @@ func AskReportSubject(node string) string { return "mesh.node." + node + ".ask.r // AskHandOverSubject is where the controller's terminal asks one machine's node-engine to hand a directory it // uses as found to the mesh (novox/hq issue 356, issue 339): a request on core NATS, answered once on the reply -// it carries. Only the controller publishes under `mesh.node.`, so the engine hears nobody else. +// it carries. Only the controller is granted a publish here (the writers table holds it), but **that is not who +// the engine hears**: the bus lets any principal allowed to answer reply to a message it received, on the reply +// subject that message named, so a message can arrive here from any responder. The ask is therefore signed with +// the mesh's key (link.SignedHandOver), and the engine verifies it before reading anything out of it. func AskHandOverSubject(node string) string { return "mesh.node." + node + ".ask.hand-over" } // inbox is a principal's own reply space. No user is ever granted a bare `_INBOX.>` (design 25 @@ -872,8 +875,10 @@ func PermissionsFor(p Principal) (Permissions, error) { Subscribe: sub, // A module answers what it was asked — a tool call reaches it on its own namespace, so the // authority is bounded by having been asked — and so does the controller. A node is asked one - // thing, a hand-over on its own subject (novox/hq issue 356), and answers that. A person is never - // asked anything, and is granted nothing here. + // thing, a hand-over on its own subject (novox/hq issue 356), and answers that: the node is its + // machine's engine, root there already, and it is delivered only its own subjects. What it answers is + // never trusted for being an answer — the controller reads the engine's words and records nothing. A + // person is never asked anything, and is granted nothing here. AllowResponses: p.Kind == KindModule || p.Kind == KindController || p.Kind == KindNodeTools || p.Kind == KindNode, }, nil } diff --git a/internal/broker/writers.go b/internal/broker/writers.go index a775d4ce..59382a96 100644 --- a/internal/broker/writers.go +++ b/internal/broker/writers.go @@ -84,6 +84,13 @@ func kvOf(bucket string) []string { return []string{"$KV." + bucket + ".>"} } var WritersTable = []WriterRow{ {State: "a machine's declaration", Writer: "controller (lease holder)", KeptIn: "the bus, last per subject", Others: "read", Subjects: []string{"mesh.node.*.declare"}, Writes: isController}, + // The operator's hand-over of a directory used as found, asked of the machine's engine at the controller's + // terminal (novox/hq issue 356). One publisher; and because a responder can still reach the subject through a + // reply, the ask is signed with the mesh's key and the engine verifies it — the row bounds who is granted the + // publish, the signature who is believed. + {State: "a hand-over asked of a machine", Writer: "controller, at its terminal", KeptIn: "the machine, beside its state", + Others: "the engine verifies the mesh's signature and records it, or refuses", + Subjects: []string{"mesh.node.*.ask.hand-over"}, Writes: isController}, {State: "a machine's applied state and its report", Writer: "the node-engine's apply queue", KeptIn: "the machine; the report on the bus", Others: "the reconcile and a delivery enqueue, never apply", // And its health statement between reports (novox/hq ADR 0240): the same writer stating the same diff --git a/internal/broker/writers_test.go b/internal/broker/writers_test.go index 11eeee7f..5f8c07f5 100644 --- a/internal/broker/writers_test.go +++ b/internal/broker/writers_test.go @@ -15,6 +15,7 @@ import ( // to the table; one dropped from either fails. var designRows = []string{ "a machine's declaration", + "a hand-over asked of a machine", "a machine's applied state and its report", "the controller lease", "plans and their tiers", @@ -150,3 +151,22 @@ func TestSubjectsOverlap(t *testing.T) { } } } + +// **A hand-over asked of a machine has one publisher, the controller** (novox/hq issue 356): a grant that lets any +// other principal publish it — a node, the node tools, a module — is refused at composition, naming the state. +// (Who the engine believes is the signature's; this bounds who is granted the publish.) +func TestAHandOverAskHasOnePublisher(t *testing.T) { + for _, p := range []Principal{ + {Kind: KindNode, Node: "laptop"}, + {Kind: KindNodeTools, Node: "laptop", Module: RuntimeModule}, + {Kind: KindModule, Node: "laptop", Module: "notes"}, + } { + err := CheckWriters(p, []string{"mesh.node.laptop.ask.hand-over"}) + if err == nil || !strings.Contains(err.Error(), "a hand-over asked of a machine") { + t.Errorf("%s may publish a hand-over: %v", p.Username(), err) + } + } + if err := CheckWriters(Principal{Kind: KindController}, []string{"mesh.node.>"}); err != nil { + t.Fatalf("the controller may not ask a hand-over: %v", err) + } +} diff --git a/internal/link/handover.go b/internal/link/handover.go index c249f3b0..d090119c 100644 --- a/internal/link/handover.go +++ b/internal/link/handover.go @@ -2,6 +2,8 @@ package link import ( "context" + "crypto/rand" + "encoding/hex" "encoding/json" "errors" "fmt" @@ -21,13 +23,35 @@ import ( // in its own link code (mesh-host internal/link HandOverAsk, HandOverAnswer); a test on each side holds // the field names. -// HandOverAsk is what the controller asks: the directory's absolute path as the engine states it, and who -// asked, in the controller's words. +// HandOverAsk is what the controller asks: the node it is for, the directory's absolute path as the engine states +// it, who asked in the controller's words, when the ask stops being good, and a nonce the engine takes 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 signature. +// +// **Signed, because the subject proves nothing** (review of issue 356). Only the controller may publish +// `mesh.node..ask.hand-over`, but the bus lets any principal allowed to answer reply to a message it +// received, on whatever reply subject that message named — so a tool server asked on its own subject with that +// reply could hand the engine an ask the controller never made. The engine verifies this signature, with the +// key it verifies declarations with, before it reads anything out of the ask. +type SignedHandOver struct { + Ask []byte `json:"ask"` + Signature []byte `json:"signature"` +} + +// HandOverContext is prefixed to an ask's bytes before signing, so a hand-over's signature is never a +// declaration's: the same key signs both, and a declaration is signed over its bytes alone. +const HandOverContext = "novox-mesh hand-over v1\n" + +// HandOverGood is how long a signed ask is good for: the engine refuses one past it, and one further ahead. +const HandOverGood = time.Minute + // HandOverAnswer is the engine's answer: what it recorded, or why it refused. type HandOverAnswer struct { Said string `json:"said,omitempty"` @@ -41,11 +65,12 @@ const HandOverWithin = 30 * time.Second // AskHandOver asks one machine's node-engine to hand a directory used as found to the mesh, and reads its // answer. An error is the ask not reaching an engine, or an answer that is not one; a refusal is the engine's // and comes back in the answer. -func AskHandOver(ctx context.Context, conn *nats.Conn, node, path, by string, timeout time.Duration) (HandOverAnswer, error) { +func AskHandOver(ctx context.Context, conn *nats.Conn, signer Signer, node, path, by string, + timeout time.Duration) (HandOverAnswer, error) { if conn == nil { return HandOverAnswer{}, errors.New("this controller is not on the bus") } - body, err := json.Marshal(HandOverAsk{Path: path, By: by}) + body, err := SignHandOver(ctx, signer, HandOverAsk{Node: node, Path: path, By: by}) if err != nil { return HandOverAnswer{}, err } @@ -90,3 +115,26 @@ func AskHandOver(ctx context.Context, conn *nats.Conn, node, path, by string, ti } return answer, nil } + +// SignHandOver fills the ask's expiry and nonce and signs it with the mesh's key, over HandOverContext and the +// ask's bytes exactly as they travel. +func SignHandOver(ctx context.Context, signer Signer, ask HandOverAsk) ([]byte, error) { + if signer == nil { + return nil, errors.New("no signing key, so no hand-over can be asked") + } + nonce := make([]byte, 16) + if _, err := rand.Read(nonce); err != nil { + return nil, err + } + ask.Nonce = hex.EncodeToString(nonce) + ask.Expires = time.Now().UTC().Add(HandOverGood) + raw, err := json.Marshal(ask) + if err != nil { + return nil, err + } + signature, err := signer.Sign(ctx, append([]byte(HandOverContext), raw...)) + if err != nil { + return nil, fmt.Errorf("cannot sign the hand-over: %w", err) + } + return json.Marshal(SignedHandOver{Ask: raw, Signature: signature}) +} diff --git a/internal/link/handover_test.go b/internal/link/handover_test.go index 784e9030..bf97a68f 100644 --- a/internal/link/handover_test.go +++ b/internal/link/handover_test.go @@ -2,6 +2,7 @@ package link import ( "context" + "crypto/ed25519" "encoding/json" "strings" "testing" @@ -16,10 +17,15 @@ import ( // The hand-over's ask and answer hold these field names; the engine's side holds the same list (mesh-host // internal/link, TestTheHandOverAskAndAnswerKeepTheirFieldNames). func TestTheHandOverAskAndAnswerKeepTheirFieldNames(t *testing.T) { - body, _ := json.Marshal(HandOverAsk{Path: "/srv/notes", By: "jo through mesh-cli on anchor"}) - if got := keysIn(t, body); got != "by path" { + body, _ := json.Marshal(HandOverAsk{Node: "laptop", Path: "/srv/notes", By: "jo through mesh-cli on anchor", + Expires: time.Now(), Nonce: "n"}) + if got := keysIn(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 := keysIn(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 := keysIn(t, body); got != "refused said" { t.Fatalf("the answer's fields are %q", got) @@ -27,6 +33,67 @@ func TestTheHandOverAskAndAnswerKeepTheirFieldNames(t *testing.T) { if broker.AskHandOverSubject("laptop") != "mesh.node.laptop.ask.hand-over" { t.Fatalf("the subject is %q", broker.AskHandOverSubject("laptop")) } + if HandOverContext != "novox-mesh hand-over v1\n" { + t.Fatalf("the signing context is %q; the engine holds the same words", HandOverContext) + } +} + +// keySigner signs with one key, as the controller's identity does. +type keySigner struct{ key ed25519.PrivateKey } + +func (k keySigner) Sign(_ context.Context, message []byte) ([]byte, error) { + return ed25519.Sign(k.key, message), nil +} + +func testSigner(t *testing.T) (keySigner, ed25519.PublicKey) { + t.Helper() + public, private, err := ed25519.GenerateKey(nil) + if err != nil { + t.Fatal(err) + } + return keySigner{private}, public +} + +// **The ask is signed over the context and its bytes, with a fresh nonce and a short expiry** (review of issue +// 356): the signature verifies over HandOverContext and the ask's bytes, and not over the bytes alone — so it is +// never a declaration's — and two asks never share a nonce. +func TestAHandOverIsSignedWithAContextANonceAndAnExpiry(t *testing.T) { + signer, public := testSigner(t) + body, err := SignHandOver(context.Background(), signer, HandOverAsk{Node: "laptop", Path: "/srv/notes", By: "jo"}) + if err != nil { + t.Fatal(err) + } + var signed SignedHandOver + if err := json.Unmarshal(body, &signed); err != nil { + t.Fatal(err) + } + if !ed25519.Verify(public, append([]byte(HandOverContext), signed.Ask...), signed.Signature) { + t.Fatal("the signature does not verify over the context and the ask") + } + if ed25519.Verify(public, signed.Ask, signed.Signature) { + t.Fatal("the signature verifies over the ask's bytes alone, as a declaration's would") + } + var ask HandOverAsk + if err := json.Unmarshal(signed.Ask, &ask); err != nil { + t.Fatal(err) + } + if ask.Node != "laptop" || ask.Path != "/srv/notes" || ask.By != "jo" || len(ask.Nonce) != 32 { + t.Fatalf("signed %+v", ask) + } + if left := time.Until(ask.Expires); left <= 0 || left > HandOverGood { + t.Fatalf("the ask is good for %s", left) + } + again, _ := SignHandOver(context.Background(), signer, HandOverAsk{Node: "laptop", Path: "/srv/notes", By: "jo"}) + var other SignedHandOver + _ = json.Unmarshal(again, &other) + var second HandOverAsk + _ = json.Unmarshal(other.Ask, &second) + if second.Nonce == ask.Nonce { + t.Fatal("two asks share a nonce") + } + if _, err := SignHandOver(context.Background(), nil, HandOverAsk{}); err == nil { + t.Fatal("an ask was made with no key") + } } // The ask reaches the machine's engine on its own subject and its answer comes back whole: what it recorded, or @@ -40,9 +107,16 @@ func TestAHandOverIsAskedOfTheMachineAndItsAnswerComesBack(t *testing.T) { } t.Cleanup(conn.Close) + signer, public := testSigner(t) var heard HandOverAsk engine, err := conn.Subscribe(broker.AskHandOverSubject("laptop"), func(msg *nats.Msg) { - _ = json.Unmarshal(msg.Data, &heard) + var signed SignedHandOver + _ = json.Unmarshal(msg.Data, &signed) + if !ed25519.Verify(public, append([]byte(HandOverContext), signed.Ask...), signed.Signature) { + _ = msg.Respond([]byte(`{"refused":"not the mesh's signature"}`)) + return + } + _ = json.Unmarshal(signed.Ask, &heard) switch heard.Path { case "/srv/notes": body, _ := json.Marshal(HandOverAnswer{Said: "/srv/notes (notes.data) is handed to the mesh by " + heard.By}) @@ -60,26 +134,26 @@ func TestAHandOverIsAskedOfTheMachineAndItsAnswerComesBack(t *testing.T) { t.Cleanup(func() { _ = engine.Unsubscribe() }) ctx := context.Background() - a, err := AskHandOver(ctx, conn, "laptop", "/srv/notes", "jo through mesh-cli on anchor", 5*time.Second) + a, err := AskHandOver(ctx, conn, signer, "laptop", "/srv/notes", "jo through mesh-cli on anchor", 5*time.Second) if err != nil || a.Refused != "" || !strings.Contains(a.Said, "handed to the mesh by jo through mesh-cli on anchor") { t.Fatalf("answered %+v, %v", a, err) } - if heard.Path != "/srv/notes" || heard.By != "jo through mesh-cli on anchor" { + if heard.Node != "laptop" || heard.Path != "/srv/notes" || heard.By != "jo through mesh-cli on anchor" { t.Fatalf("the engine heard %+v", heard) } - a, err = AskHandOver(ctx, conn, "laptop", "/srv/other", "jo", 5*time.Second) + a, err = AskHandOver(ctx, conn, signer, "laptop", "/srv/other", "jo", 5*time.Second) if err != nil || a.Said != "" || !strings.Contains(a.Refused, "nothing was handed over") { t.Fatalf("a refusal came back as %+v, %v", a, err) } - if _, err := AskHandOver(ctx, conn, "laptop", "/srv/empty", "jo", 5*time.Second); err == nil || + if _, err := AskHandOver(ctx, conn, signer, "laptop", "/srv/empty", "jo", 5*time.Second); err == nil || !strings.Contains(err.Error(), "neither") { t.Fatalf("an empty answer was taken: %v", err) } - if _, err := AskHandOver(ctx, conn, "anchor", "/srv/notes", "jo", 5*time.Second); err == nil || + if _, err := AskHandOver(ctx, conn, signer, "anchor", "/srv/notes", "jo", 5*time.Second); err == nil || !strings.Contains(err.Error(), "node-engine") { t.Fatalf("a machine with no engine listening: %v", err) } - if _, err := AskHandOver(ctx, nil, "anchor", "/srv/notes", "jo", time.Second); err == nil { + if _, err := AskHandOver(ctx, nil, signer, "anchor", "/srv/notes", "jo", time.Second); err == nil { t.Fatal("asked with no bus") } }