From f8ab9f2dcf8b3e08c0ec7d377cdcc419e5c0bdb2 Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 27 Sep 2026 02:50:23 +0200 Subject: [PATCH] The mesh composes the accounts; the module owns its server MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The delivery question, decided. The alternative was a manifest field enumerating the server's ports, TLS paths and store directory so the controller could write a whole configuration file. That is wrong: those are properties of the container the module raises, they live in its image and its mounts, and the controller would have to be kept in step with a Dockerfile it never sees. So the mesh writes only what only the mesh knows — who may connect — and the module's own configuration includes it. `ComposeAccounts` is that file. A test says what must *not* be in it as plainly as what must: no port, no tls block, no store_dir. Each of those in the mesh's file is a value the controller would then own, and the module could no longer change its own image without the mesh agreeing. `bus-users` is where a module wants it written, and **asking is not enough to receive it**: the file holds every user's password hash, so a module that could ask for it could read every credential on the bus. The claim on `mesh-broker` authorises it, checked from the manifest alone. A holder with nothing composed is refused rather than given an empty file, for the reason a certificate is — a bus with no user list refuses every connection in the mesh and looks like a machine problem. Six claims checked against a running server before any of this was committed to, and two of them changed what got written: **An absolute include path is resolved relative to the including file's directory.** `include /etc/nats/accounts.conf` from /etc/nats-server/nats.conf makes the server look for /etc/nats-server/etc/nats/accounts.conf and refuse to start. So both files share one directory, and the module declares its own as a file resource beside the mesh's. **`verify: true` was refusing every connection in the mesh.** It makes the server demand a *client* certificate, and nothing in the mesh presents one: a host pins this server's exact certificate and authenticates with the password the mesh minted, and so does a module's runtime. Every connection died at the TLS handshake before any password was looked at, with an error — "client didn't provide a certificate" — that reads as a fault in the client. Removed. TLS is still required; verify only decides whether client certificates are checked. The other four: a user in an included file authenticates, an unknown user is refused so the include is the whole authority rather than an addition, a publish outside a grant is refused, and rewriting the mesh's half alone makes a new user appear — noticed by the module's own watcher, with no signal from outside, and without dropping the connection the mesh already had. That last one is task 1.2's payoff, collected. --- internal/broker/nats.go | 44 +++++++++++++- internal/broker/testdata/composed.conf | 5 +- internal/broker/users_test.go | 47 +++++++++++++++ internal/catalogue/declaration.go | 36 ++++++++++++ internal/catalogue/declaration_test.go | 80 ++++++++++++++++++++++++++ internal/catalogue/manifest.go | 31 +++++++++- 6 files changed, 239 insertions(+), 4 deletions(-) create mode 100644 internal/catalogue/declaration_test.go diff --git a/internal/broker/nats.go b/internal/broker/nats.go index 83c0e87..159fbca 100644 --- a/internal/broker/nats.go +++ b/internal/broker/nats.go @@ -365,20 +365,60 @@ func Compose(s Server, principals []Principal) (string, error) { fmt.Fprintf(&b, "port: %d\n", s.ClientPort) fmt.Fprintf(&b, "http: 127.0.0.1:%d\n\n", s.MonitoringPort) + // **No `verify`, and it said `verify: true` until this configuration was run.** That setting + // makes the server demand a *client* certificate, and nothing in the mesh presents one: a host + // pins this server's exact certificate and authenticates with the password the mesh minted + // (ADR 0004, design 25 §4), and so does a module's runtime. With it on, every connection in the + // mesh is refused at the TLS handshake before any password is looked at, and the error — + // "client didn't provide a certificate" — reads as a fault in the client. + // + // TLS is still required: the block is what requires it, and verify only decides whether client + // certificates are checked. b.WriteString("tls {\n") fmt.Fprintf(&b, " cert_file: %q\n", s.TLSCert) fmt.Fprintf(&b, " key_file: %q\n", s.TLSKey) fmt.Fprintf(&b, " ca_file: %q\n", s.TLSCA) - b.WriteString(" verify: true\n") b.WriteString("}\n\n") b.WriteString("jetstream {\n") fmt.Fprintf(&b, " store_dir: %q\n", s.StoreDir) b.WriteString("}\n\n") + accounts, err := ComposeAccounts(sorted) + if err != nil { + return "", err + } + b.WriteString(accounts) + return b.String(), nil +} + +// ComposeAccounts is the accounts block alone — every user, and nothing about the server. +// +// **This is the only part of the configuration the mesh writes, and the split is deliberate.** A +// server's ports, its TLS paths and its store directory are properties of the container the module +// raises: they live in its image and its mounts, and they change when it does. The controller has no +// business knowing them, and a controller that did would have to be kept in step with a Dockerfile +// it never sees. What only the mesh knows is *who may connect*, so that is what it writes, and the +// module's own configuration includes it. +// +// Four things checked against a running server before this shape was committed to: a user in an +// included file authenticates; an unknown user is refused, so the include is the whole authority +// rather than an addition to something; a publish outside a user's grant is refused; and rewriting +// this file alone and signalling a reload makes a new user appear **without dropping the connection +// the mesh already has** — which is what makes every later account, permission or person change cost +// nothing (task 1.2's payoff). +func ComposeAccounts(principals []Principal) (string, error) { + sorted := append([]Principal(nil), principals...) + sort.Slice(sorted, func(i, j int) bool { return sorted[i].Username() < sorted[j].Username() }) + + var b strings.Builder + b.WriteString("# The mesh's users, composed by the controller. Do not edit: the next\n") + b.WriteString("# composition overwrites it. Permissions are derived from what each module\n") + b.WriteString("# declares and nothing else (novox/hq ADR 0043, design 29 §2).\n\n") + // One account for the mesh: accounts in NATS isolate subject spaces entirely, and the mesh is // one space (design 25 §4). The cost of that — that permissions are the only isolation — is - // paid above, in the scoping of every inbox and every ack subject. + // paid in the scoping of every inbox and every ack subject. b.WriteString("accounts {\n MESH {\n users = [\n") for _, p := range sorted { perms, err := PermissionsFor(p) diff --git a/internal/broker/testdata/composed.conf b/internal/broker/testdata/composed.conf index 489435a..dd7da95 100644 --- a/internal/broker/testdata/composed.conf +++ b/internal/broker/testdata/composed.conf @@ -9,13 +9,16 @@ tls { cert_file: "/tls/tls.crt" key_file: "/tls/tls.key" ca_file: "/tls/ca.crt" - verify: true } jetstream { store_dir: "/data" } +# The mesh's users, composed by the controller. Do not edit: the next +# composition overwrites it. Permissions are derived from what each module +# declares and nothing else (novox/hq ADR 0043, design 29 §2). + accounts { MESH { users = [ diff --git a/internal/broker/users_test.go b/internal/broker/users_test.go index 5787b9e..8f40e1b 100644 --- a/internal/broker/users_test.go +++ b/internal/broker/users_test.go @@ -154,3 +154,50 @@ func TestRecordsComposeIntoAFile(t *testing.T) { } } } + +// The accounts block alone is what the mesh writes, and it holds nothing about the server. +// +// **The split is the whole design decision** (ComposeAccounts): ports, TLS paths and a store +// directory are properties of the container the module raises, and a controller that wrote them +// would have to be kept in step with a Dockerfile it never sees. So this test says what must not be +// in the file as plainly as what must. +func TestWhatTheMeshWritesIsUsersAndNothingAboutTheServer(t *testing.T) { + users, err := Users(someRecords()) + if err != nil { + t.Fatal(err) + } + hashes := map[string]string{} + for _, u := range users { + hashes[u.Username()] = "$2a$11$" + strings.Repeat("x", 22) + } + filled, missing := WithPasswords(users, hashes) + if len(missing) != 0 { + t.Fatalf("users with no password: %v", missing) + } + got, err := ComposeAccounts(filled) + if err != nil { + t.Fatal(err) + } + + for _, want := range []string{"accounts {", `user: "controller"`, `user: "one.telegram"`} { + if !strings.Contains(got, want) { + t.Errorf("the accounts file does not contain %s", want) + } + } + // None of the server's own settings. Each of these in the mesh's file is a value the controller + // would then own, and the module could no longer change its own image without the mesh agreeing. + for _, absent := range []string{"port:", "http:", "jetstream", "tls {", "store_dir", "cert_file"} { + if strings.Contains(got, absent) { + t.Errorf("the accounts file contains %q, which belongs to the module that raises the "+ + "server, not to the mesh", absent) + } + } +} + +// A user with no password is refused here too, not only by the whole-file composition: this is the +// function the controller actually calls, and a user without a password is a user anybody is. +func TestTheAccountsFileRefusesAUserWithNoPassword(t *testing.T) { + if _, err := ComposeAccounts([]Principal{{Kind: KindController}}); err == nil { + t.Fatal("a user with no password hash was written") + } +} diff --git a/internal/catalogue/declaration.go b/internal/catalogue/declaration.go index ad33e90..f78f661 100644 --- a/internal/catalogue/declaration.go +++ b/internal/catalogue/declaration.go @@ -97,6 +97,13 @@ type Rendering struct { // compose it a second time. Suffix string + // BusUsers is the mesh's composed user list, for the module holding `mesh-broker`. Empty on + // every other node, and on this one until the controller has composed it. + // + // **Only the users, never the server's own settings**: those are the module's, in its image and + // its mounts (Manifest.BusUsers). + BusUsers string + // Kept is every operator-sealed secret in the mesh, for a module that `keeps` them. Nil when // nothing on this node keeps them, or the mesh has no operator key. Kept *KeptExport @@ -359,6 +366,35 @@ func (r Resolution) compose(with Rendering, owner map[string]string) ([]map[stri }) } } + if m.BusUsers != "" { + // **The claim authorises it, not the field.** This file holds every user's password + // hash, so a module that could ask for it could read every credential on the bus. + // Checked from this manifest alone, which is the cheapest check there is: whether some + // other module also claims the seat is resolution's business elsewhere, and one holder + // mesh-wide is already guaranteed. + if !m.ClaimsSeat("mesh-broker") { + return nil, fmt.Errorf( + "%s asks for the mesh's user list and does not claim mesh-broker. That file "+ + "holds every user's password hash, so the seat is what authorises it", + m.Module) + } + if with.BusUsers == "" { + // Asked for and not composed. Refused rather than skipped, for the reason a + // certificate is: a bus with no user list refuses every connection in the mesh, and + // an empty file would look like a configuration problem on the machine. + return nil, fmt.Errorf( + "%s holds mesh-broker and the mesh composed no user list, so the bus would "+ + "refuse every connection", m.Module) + } + first = append(first, map[string]any{ + "id": BusUsersID(), "type": "file", "path": m.BusUsers, + "content": with.BusUsers, + // Readable by the server and nothing else. Hashes rather than passwords, so this is + // not a set of working credentials — but a list of every user in the mesh is worth + // keeping to the one process that needs it. + "mode": "0600", + }) + } for _, name := range sortedKeys(m.OwnSecrets) { sealed := with.Needed[m.Module][name] if sealed == "" { diff --git a/internal/catalogue/declaration_test.go b/internal/catalogue/declaration_test.go new file mode 100644 index 0000000..23b1caa --- /dev/null +++ b/internal/catalogue/declaration_test.go @@ -0,0 +1,80 @@ +package catalogue + +import "testing" + +// The mesh's user list reaches the module holding the bus, and nothing else. +// +// Three refusals and one delivery, because each of the refusals would be silent in a different way: +// a module that asked and was given it could read every credential on the bus; a bus given an empty +// file refuses every connection in the mesh and looks like a machine problem; and a bus that never +// asked gets nothing rather than a file it does not read. +func TestTheMeshsUserListGoesOnlyToTheModuleHoldingTheBus(t *testing.T) { + theBus := func() Manifest { + return Manifest{ + Module: "nats", Version: "1", + Claims: []Claim{{Name: "mesh-broker", Scope: ScopeMesh}}, + BusUsers: "/var/lib/nats-module/conf/accounts.conf", + Resources: []map[string]any{}, + } + } + + on := func(t *testing.T, m Manifest, with Rendering) ([]map[string]any, error) { + t.Helper() + return Resolution{Node: "anchor", Modules: []Manifest{m}}.Declaration(with) + } + + t.Run("the holder is given it", func(t *testing.T) { + resources, err := on(t, theBus(), Rendering{BusUsers: "accounts { MESH { users = [] } }"}) + if err != nil { + t.Fatal(err) + } + // Prefixed with the module it came from, like every resource: two modules may reasonably + // both call something "config", and without the prefix the second would silently replace + // the first. + var found map[string]any + for _, r := range resources { + if r["id"] == "nats."+BusUsersID() { + found = r + } + } + if found == nil { + t.Fatalf("the bus was given no user list: %+v", resources) + } + if found["path"] != "/var/lib/nats-module/conf/accounts.conf" { + t.Errorf("written to %v rather than where the module asked", found["path"]) + } + if found["mode"] != "0600" { + t.Errorf("mode %v: a list of every user in the mesh belongs to the one process that "+ + "needs it", found["mode"]) + } + }) + + t.Run("a module that does not claim the seat is refused", func(t *testing.T) { + m := theBus() + m.Claims = nil + if _, err := on(t, m, Rendering{BusUsers: "accounts {}"}); err == nil { + t.Fatal("a module that claims nothing was handed every user's password hash") + } + }) + + t.Run("the holder with nothing composed is refused", func(t *testing.T) { + if _, err := on(t, theBus(), Rendering{}); err == nil { + t.Fatal("the bus was given an empty user list, so it would refuse every connection in " + + "the mesh and look like a machine problem") + } + }) + + t.Run("a module that did not ask gets nothing", func(t *testing.T) { + m := theBus() + m.BusUsers = "" + resources, err := on(t, m, Rendering{BusUsers: "accounts {}"}) + if err != nil { + t.Fatal(err) + } + for _, r := range resources { + if r["id"] == "nats."+BusUsersID() { + t.Fatal("a module that asked for no user list was given one") + } + } + }) +} diff --git a/internal/catalogue/manifest.go b/internal/catalogue/manifest.go index 7115e1a..b874536 100644 --- a/internal/catalogue/manifest.go +++ b/internal/catalogue/manifest.go @@ -423,6 +423,20 @@ type Manifest struct { // A directory rather than one document for the same reason as above: each value is sealed // separately and the mesh cannot open any of them to build a list. Grants map[string]string `json:"grants,omitempty"` + + // BusUsers is where this module wants the mesh's user list written, and it is only ever + // answered for the module holding `mesh-broker`. + // + // **The mesh writes who may connect; the module owns everything else about its server** + // (novox/hq design 25 §4, task 1.7). Ports, TLS paths and a store directory live in this + // module's image and its mounts and change when it does, so the module's own configuration + // carries them and includes this file. A controller that wrote the whole configuration would + // have to be kept in step with a Dockerfile it never sees. + // + // **Asking for it is not enough to receive it.** This file holds every user's password hash, so + // a module that could ask for it could read every credential on the bus — and the claim on + // `mesh-broker` is what authorises it, checked from this manifest alone. + BusUsers string `json:"bus-users,omitempty"` } // Build says how to produce this module's artifacts from its source. @@ -641,7 +655,22 @@ type Certificate struct { // CertificateID and AuthorityID are the resource identities of what the mesh issued. func CertificateID() string { return "certificate" } -func AuthorityID() string { return "certificate-authority" } + +// ClaimsSeat says whether this manifest claims one named seat. +func (m Manifest) ClaimsSeat(seat string) bool { + for _, c := range m.Claims { + if c.Name == seat { + return true + } + } + return false +} + +// BusUsersID names the mesh's composed user list, so it is the same resource across every +// declaration and a change to it is an update rather than a second file beside the old one — which +// on a bus reading a directory would be two account lists, and the server would take both. +func BusUsersID() string { return "bus-users" } +func AuthorityID() string { return "certificate-authority" } // FilteringID names the computed rule set, so it is the same resource across every declaration // and a change to it is an update rather than an addition beside the old one.