From d65c37caadca94a6ebc83b9a8e136c930fb982a4 Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 27 Sep 2026 14:13:52 +0200 Subject: [PATCH] The controller could not answer an enrolment, and a probe on an open server said it could MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Found while reasoning about issue 127's replay question, in code committed earlier today. The controller's permissions granted no inbox at all, so the answer to every enrolment on the mesh would have been refused — "Permissions Violation for Publish to _INBOX.enrol.anchor…" — while the controller logged that it had enrolled the node. **`allow_responses` does not cover it, and that is the trap.** It permits one reply to the reply subject of a message the user received, and a message a JetStream consumer delivers has had that field claimed for the consumer's own ack address (design 25 §2). The address the controller actually answers is the one the request carried in its *payload*, which the server does not recognise as a reply subject at all. The two mechanisms look interchangeable and are not. **My earlier verification could not have caught this.** The live enrolment tests run against a server with no accounts and no permissions, so they exercise the subjects and the round trip and nothing about authority. Composing the real configuration and running a server on it is what found it. Granted the enrolment inbox space and nothing wider: nothing but an enrolling node ever subscribes under that prefix, each scoped to its own token's, so the controller publishing there is the mesh answering enrolments and reaches nothing else. Confirmed against the permissioned server both ways — the answer arrives, and a node's own inbox is still refused. Pinned as a rule that needs no server: whatever an enrolling node subscribes, the controller must be able to publish to, and a node's, a module's and a person's inbox must stay out of reach. That check is a subject-pattern match rather than a string compare, so a grant that widened by a wildcard would not slip past it. It also bears on 127's open question about who replays a build announcement: an answer to a *module's* inbox would need `_INBOX.>`, which is exactly the blanket grant design 25 §4 refuses. So the catch-up cannot become an inbox reply. --- internal/broker/nats.go | 22 ++++++++- internal/broker/nats_test.go | 68 ++++++++++++++++++++++++++ internal/broker/testdata/composed.conf | 2 +- 3 files changed, 90 insertions(+), 2 deletions(-) diff --git a/internal/broker/nats.go b/internal/broker/nats.go index 159fbca..579079e 100644 --- a/internal/broker/nats.go +++ b/internal/broker/nats.go @@ -76,6 +76,10 @@ type Principal struct { PasswordHash string } +// enrolmentPrefix is the space every enrolling node's user and inbox live under, so the one place the +// controller may answer an enrolment is derived from the same constant the user is named from. +const enrolmentPrefix = "enrol" + // safeSubject refuses anything that would change the meaning of a subject rather than sit inside // one. A name carrying a dot would silently widen a permission by adding a token; a name carrying // `>` or `*` would widen it to a wildcard, which is the whole authority model gone. @@ -104,7 +108,7 @@ func (p Principal) Username() string { // the mesh holds one live claim per record, and the node's name is the one identifier both // sides already have before anything else is agreed. It is also exactly what the other // transport does, where the account is named after the node and the secret is its password. - return "enrol." + p.Node + return enrolmentPrefix + "." + p.Node } return "" } @@ -163,6 +167,22 @@ func PermissionsFor(p Principal) (Permissions, error) { sub = append(sub, ControllerFollows...) pub = append(pub, "$JS.ACK.EVENTS."+ControllerName+".>") + // **Where an enrolment's answer goes**, and `allow_responses` does not cover it. That + // permits one reply to the reply subject of a message the user received — and a message a + // JetStream consumer delivers has had that field claimed for the consumer's own ack address + // (design 25 §2), so the address the controller actually answers is the one the request + // carried in its payload, which is not a reply subject as the server understands it. + // + // Verified against a real server before this line existed: the answer was refused with + // "Permissions Violation for Publish to _INBOX.enrol.anchor…", and every enrolment on the + // mesh would have timed out while the controller logged success. + // + // **The enrolment inbox space, not a blanket `_INBOX.>`.** Design 25 §4 refuses that, and + // this is not it: nothing but an enrolling node ever subscribes under this prefix, each + // scoped to its own token's, so the controller publishing here is the mesh answering + // enrolments and can reach nothing else. + pub = append(pub, "_INBOX."+enrolmentPrefix+".>") + case KindPerson: // Tools, and nothing else. Every subject a person may publish is a tool call; a person // who could publish an event would be able to claim a module said something. diff --git a/internal/broker/nats_test.go b/internal/broker/nats_test.go index f5c2500..6ba56b1 100644 --- a/internal/broker/nats_test.go +++ b/internal/broker/nats_test.go @@ -256,3 +256,71 @@ func TestAToolGrantThatNamesNoToolIsRefused(t *testing.T) { t.Fatal("a grant naming a module but no tool was accepted") } } + +// The controller can answer an enrolment, and reach no other inbox. +// +// **`allow_responses` does not cover this and that is the trap.** It permits one reply to the reply +// subject of a message the user received — and a message a JetStream consumer delivers has had that +// field claimed for the consumer's own ack address, so the address the controller actually answers is +// the one the request carried in its payload, which the server does not recognise as a reply subject +// at all. +// +// Found against a real server, after a live test on an *unpermissioned* one had passed: every +// enrolment on the mesh would have timed out while the controller logged success. +func TestTheControllerCanAnswerAnEnrolmentAndReachNoOtherInbox(t *testing.T) { + ctl, err := PermissionsFor(Principal{Kind: KindController, PasswordHash: "x"}) + if err != nil { + t.Fatal(err) + } + enrolling, err := PermissionsFor(Principal{Kind: KindEnrolment, Node: "anchor", PasswordHash: "x"}) + if err != nil { + t.Fatal(err) + } + + // Whatever the enrolling node waits on, the controller must be able to publish to. + if len(enrolling.Subscribe) != 1 { + t.Fatalf("an enrolling node subscribes %v, and this test knows only how to check one", + enrolling.Subscribe) + } + waitsOn := enrolling.Subscribe[0] + if !covers(ctl.Publish, waitsOn) { + t.Fatalf("the controller may publish %v, none of which reaches %s — so every enrolment on "+ + "the mesh times out while the controller logs success", ctl.Publish, waitsOn) + } + + // And nothing wider. A node's own inbox and a module's are not the controller's to write into: + // that is the blanket grant design 25 §4 refuses. + for _, other := range []string{"_INBOX.node.anchor.x", "_INBOX.one.shop.x", "_INBOX.person.ada.x"} { + if covers(ctl.Publish, other) { + t.Errorf("the controller can publish to %s, which is an inbox privacy the permission "+ + "list is the only thing protecting", other) + } + } +} + +// covers says whether any granted subject pattern admits one concrete subject, with NATS's own +// wildcard meanings: `*` is one token, `>` is the rest. +func covers(granted []string, subject string) bool { + want := strings.Split(subject, ".") + for _, pattern := range granted { + if admits(strings.Split(pattern, "."), want) { + return true + } + } + return false +} + +func admits(pattern, subject []string) bool { + for i, token := range pattern { + if token == ">" { + return i < len(subject) + } + if i >= len(subject) { + return false + } + if token != "*" && token != subject[i] { + return false + } + } + return len(pattern) == len(subject) +} diff --git a/internal/broker/testdata/composed.conf b/internal/broker/testdata/composed.conf index dd7da95..6bcd013 100644 --- a/internal/broker/testdata/composed.conf +++ b/internal/broker/testdata/composed.conf @@ -23,7 +23,7 @@ accounts { MESH { users = [ { user: "controller", password: "$2a$11$cccccccccccccccccccccc", permissions: { - publish: { allow: ["$JS.ACK.CONTROL.controller.>", "$JS.ACK.EVENTS.controller.>", "$JS.API.>", "mesh.build.>", "mesh.control.>", "mesh.node.>"] } + publish: { allow: ["$JS.ACK.CONTROL.controller.>", "$JS.ACK.EVENTS.controller.>", "$JS.API.>", "_INBOX.enrol.>", "mesh.build.>", "mesh.control.>", "mesh.node.>"] } subscribe: { allow: ["$JS.API.>", "_INBOX.controller.>", "mesh.build.>", "mesh.control.>", "mesh.mod.mesh-catalog.event.module.mesh-catalog.catching-up", "mesh.mod.mesh-catalog.event.module.mesh-catalog.upgraded"] } allow_responses: { max: 1, ttl: "1m" } } }