From adfb61c053a1f55515bed018bc4e00e0ebabaaab Mon Sep 17 00:00:00 2001 From: jochen Date: Tue, 20 Oct 2026 21:21:15 +0200 Subject: [PATCH] brother: the admin account, its owned secrets, the process and the printer-asks stream declared as mesh-mail's sender is; an integration test and the README (issue 1453) --- modules/brother/README.md | 21 ++- .../cmd/brother/admin_integration_test.go | 130 ++++++++++++++++++ modules/brother/cmd/brother/adminproc.go | 32 +++-- modules/brother/cmd/brother/manifest_test.go | 121 ++++++++++++++-- modules/brother/go.mod | 12 +- modules/brother/go.sum | 1 + modules/brother/module.json | 69 +++++++++- 7 files changed, 351 insertions(+), 35 deletions(-) create mode 100644 modules/brother/cmd/brother/admin_integration_test.go diff --git a/modules/brother/README.md b/modules/brother/README.md index 2b2e661c..379c893e 100644 --- a/modules/brother/README.md +++ b/modules/brother/README.md @@ -59,7 +59,7 @@ makes it (mode 0755) when it files the first scan. | verb (tool) | does | |---|---| | `printer.status` | without a login: over IPP (`Get-Printer-Attributes`, encoded by hand, on port 631) the state (idle, processing, stopped), the state reasons (such as `toner-low-warning`) and the markers' names, levels (percent; below 0 is unknown) and colours; over eSCL the scanner's state and its feeder's. A part that does not answer is `answers: false` with why. Needs `printer` and `scanner` | -| `printer.settings` | logs in, reads one admin page, logs out. Argument `page`: a path on the admin site, starting with `/` and ending in `.html` (default `/general/status.html`). Answers the page's title, links (href and text) and forms with their fields (name, type, value, label); a password field's value is never given. Read-only: it sends the login and the logout and nothing else | +| `printer.settings` | asks the admin process (below) to log in, read one admin page and log out, and waits up to 60 s for its answer. Argument `page`: a path on the admin site, starting with `/` and ending in `.html` (default `/general/status.html`). Answers the page's title, links (href and text) and forms with their fields (name, type, value, label); a password field's value is never given. Read-only: it sends the login and the logout and nothing else | ### Trust and the password @@ -69,9 +69,22 @@ to it; the error names both fingerprints. Get the fingerprint from the device's upper-case hex pairs joined by colons. The admin password is the module's own secret `admin-password` (ADR 0467 clause 4; ADR 0283): the operator gives it -at their desk, the mesh never generates or rotates it, and it reaches the code as the file `BROTHER_ADMIN_PASSWORD` -names (`${dir:state}/admin-password.secret`). While it is not given, `printer.settings` says that the secret is not -given and contacts nothing. +at their desk, and the mesh never generates or rotates it. It is owned by the account `brother-admin`, which the +module makes, because the tools run as the operator account and must never read it (ADR 0436 §1, §12; the shape of +mesh-mail's sender). The file is `${dir:state}/admin-password.secret`, mode 0600, and `BROTHER_ADMIN_PASSWORD` names +it for the admin process alone; the tools' bundle is given no such name, and `admin_boundary_test.go` checks that +no tool file imports the package that reads it. While it is not given, `printer.settings` answers that the secret +is not given, and the printer is not contacted. + +### The admin process + +`brother admin` runs as `brother-admin` with a bus account of its own (the own secret `broker-admin`), never a +seat. It consumes the work-queue stream `printer-asks` (event `printer.asked`, body `id`, `page`, `asked-at`) and +writes each answer, once, into the state `printer-answers` under the ask's id (ten minutes' life): the page's +title, links and form fields, or `error`. It alone reads the password, pins the certificate, logs in, reads the +page and logs out. An ask older than two minutes is dropped without contacting the printer; at most three +deliveries. It reads `settings.json` (mode 0644, no secret in it) and the state directory is 0711 so that it can +reach its own files; both are the choices that let its account read what it needs. Its health is its unit. The login reads the form each time: the device generates the password field's name, so the code takes the name from the form, posts it with the form's hidden fields, and keeps the session cookie. A login is failed when the answer is diff --git a/modules/brother/cmd/brother/admin_integration_test.go b/modules/brother/cmd/brother/admin_integration_test.go new file mode 100644 index 00000000..a9706a0a --- /dev/null +++ b/modules/brother/cmd/brother/admin_integration_test.go @@ -0,0 +1,130 @@ +//go:build integration + +package main + +// The admin process over a real bus (novox/hq ADR 0467 clause 4; the shape of mesh-mail's +// sender_integration_test.go, ADR 0436 §7): a JetStream server of the test's own, the work-queue stream +// printer-asks and its consumer, the state bucket printer-answers, and a fake printer. An ask published on the +// stream is answered in the state under its id and acknowledged. Compiled by the merge gate's +// `go vet -tags integration`; never run on this machine. + +import ( + "context" + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" + "time" + + "git.novox.be/novox/mesh-sdk/go/process" + "github.com/nats-io/nats-server/v2/server" + "github.com/nats-io/nats.go" + "github.com/nats-io/nats.go/jetstream" + + "brother/cmd/brother/internal/secret" +) + +// printerAskedSubject is the wire subject the controller composes for the stream's one event: +// "mesh.stream." + module + ".event." + event (mesh-controller internal/broker/modstreams.go). +const printerAskedSubject = "mesh.stream.brother.event." + EventAsked + +func newTestBus(t *testing.T) string { + t.Helper() + opts := &server.Options{Host: "127.0.0.1", Port: server.RANDOM_PORT, JetStream: true, StoreDir: t.TempDir(), + NoLog: true, NoSigs: true} + s, err := server.NewServer(opts) + if err != nil { + t.Fatalf("a bus for this test could not be made: %v", err) + } + go s.Start() + if !s.ReadyForConnections(30 * time.Second) { + s.Shutdown() + t.Fatal("a bus for this test did not come up within 30s") + } + t.Cleanup(func() { s.Shutdown(); s.WaitForShutdown() }) + return s.ClientURL() +} + +// 1453: an ask on printer-asks is answered under its id in printer-answers by the admin process, over the +// printer's pinned HTTPS and the login, and acknowledged; stopping it leaves nothing in hand. +func TestTheAdminProcessAnswersAnAskOverARealBus(t *testing.T) { + url := newTestBus(t) + ctx := context.Background() + nc, err := nats.Connect(url) + if err != nil { + t.Fatal(err) + } + defer nc.Close() + js, err := jetstream.New(nc) + if err != nil { + t.Fatal(err) + } + if _, err := js.CreateStream(ctx, jetstream.StreamConfig{Name: AsksStream, Subjects: []string{printerAskedSubject}, + Retention: jetstream.WorkQueuePolicy, Discard: jetstream.DiscardNew, MaxAge: 24 * time.Hour, MaxBytes: 8 << 20}); err != nil { + t.Fatal(err) + } + account := process.Account{URL: url, Node: "g14", Module: "brother", User: "g14.brother." + AdminProcessID, Password: "pw"} + if _, err := js.CreateConsumer(ctx, AsksStream, jetstream.ConsumerConfig{Durable: account.QueueConsumerName(), + AckPolicy: jetstream.AckExplicitPolicy, AckWait: 30 * time.Second, MaxDeliver: 3}); err != nil { + t.Fatal(err) + } + kv, err := js.CreateKeyValue(ctx, jetstream.KeyValueConfig{Bucket: account.Module + "_" + AnswersState, TTL: 10 * time.Minute}) + if err != nil { + t.Fatal(err) + } + + f := newFakePrinter(t, "B1891", "pw") + dir := t.TempDir() + settings := filepath.Join(dir, "settings.json") + os.WriteFile(settings, []byte(`{"printer":"`+f.host()+`","printer-certificate":"`+f.print()+`"}`), 0o600) + pwFile := filepath.Join(dir, "admin-password.secret") + os.WriteFile(pwFile, []byte("pw\n"), 0o600) + w := &adminWorker{ + Settings: func() (Settings, error) { return LoadSettings(settings) }, + Password: func() (string, error) { return secret.Read(pwFile) }, + Now: time.Now, + } + runCtx, cancel := context.WithCancel(ctx) + done := make(chan error, 1) + go func() { done <- runAdmin(runCtx, account, w) }() + + body, _ := json.Marshal(Ask{ID: "ask-1", Page: "/general/other.html", At: time.Now()}) + if _, err := js.Publish(ctx, printerAskedSubject, body, jetstream.WithMsgID("ask-1")); err != nil { + t.Fatal(err) + } + var answer string + for deadline := time.Now().Add(15 * time.Second); time.Now().Before(deadline); time.Sleep(50 * time.Millisecond) { + if e, err := kv.Get(ctx, "ask-1"); err == nil { + answer = string(e.Value()) + break + } + } + if !strings.Contains(answer, "Synthetic settings") { + t.Fatalf("the answer under ask-1 is %q", answer) + } + if strings.Contains(answer, "shouldnotshow") { + t.Errorf("a password field's value is in the answer") + } + time.Sleep(500 * time.Millisecond) + cancel() + select { + case err := <-done: + if err == nil { + t.Fatal("runAdmin returned nil, want ctx.Err() once its context was cancelled") + } + case <-time.After(10 * time.Second): + t.Fatal("runAdmin did not stop once its context was cancelled") + } + cons, err := js.Consumer(ctx, AsksStream, account.QueueConsumerName()) + if err != nil { + t.Fatal(err) + } + info, err := cons.Info(ctx) + if err != nil { + t.Fatal(err) + } + if info.NumPending != 0 || info.NumAckPending != 0 { + t.Errorf("the ask was not acknowledged: %d pending, %d awaiting an acknowledgement", info.NumPending, info.NumAckPending) + } +} diff --git a/modules/brother/cmd/brother/adminproc.go b/modules/brother/cmd/brother/adminproc.go index 7e0d6178..e758e9bc 100644 --- a/modules/brother/cmd/brother/adminproc.go +++ b/modules/brother/cmd/brother/adminproc.go @@ -158,21 +158,32 @@ func admin() int { } ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM) defer stop() - conn, err := process.Connect(account) - if err != nil { - fmt.Fprintf(os.Stderr, "[brother] admin: the bus cannot be reached: %v\n", err) - return 1 - } - defer conn.Close() w := &adminWorker{ Settings: func() (Settings, error) { return LoadSettings(settings) }, Password: func() (string, error) { return secret.Read(passwordFile) }, - Answers: conn.State(AnswersState), Now: time.Now, Logf: func(f string, a ...any) { fmt.Fprintf(os.Stderr, f+"\n", a...) }, } fmt.Fprintf(os.Stderr, "[brother] admin is consuming %s as %s\n", AsksStream, account.User) - err = conn.Consume(ctx, AsksStream, func(msg *process.Message) { + err = runAdmin(ctx, account, w) + if err != nil && ctx.Err() == nil { + fmt.Fprintf(os.Stderr, "[brother] admin stopped: %v\n", err) + return 1 + } + return 0 +} + +// runAdmin connects as the admin process's own account and consumes printer-asks until ctx ends, answering each +// ask in the state printer-answers on that connection and then settling the message as Handle says. Apart from +// admin so a test can drive it over a bus of its own. +func runAdmin(ctx context.Context, account process.Account, w *adminWorker) error { + conn, err := process.Connect(account) + if err != nil { + return fmt.Errorf("the bus cannot be reached: %w", err) + } + defer conn.Close() + w.Answers = conn.State(AnswersState) + return conn.Consume(ctx, AsksStream, func(msg *process.Message) { act, delay := w.Handle(msg.Body) var aerr error switch act { @@ -187,9 +198,4 @@ func admin() int { w.logf("[brother] admin: settling %s: %v", msg.Subject, aerr) } }) - if err != nil && ctx.Err() == nil { - fmt.Fprintf(os.Stderr, "[brother] admin stopped: %v\n", err) - return 1 - } - return 0 } diff --git a/modules/brother/cmd/brother/manifest_test.go b/modules/brother/cmd/brother/manifest_test.go index 9e22a14c..32f631e2 100644 --- a/modules/brother/cmd/brother/manifest_test.go +++ b/modules/brother/cmd/brother/manifest_test.go @@ -177,9 +177,23 @@ func TestTheScanModuleIsGone(t *testing.T) { } } -// 1453/AC4 (ADR 0467 clause 4): the admin password is the module's own secret, issued by the operator (never -// generated), and reaches the code as the file BROTHER_ADMIN_PASSWORD names. -func TestTheAdminPasswordIsTheOperatorsOwnSecret(t *testing.T) { +// resource is the manifest's resource of this id. +func resource(t *testing.T, m manifest, id string) map[string]any { + t.Helper() + for _, r := range m.Resources { + if r["id"] == id { + return r + } + } + t.Fatalf("no resource %s", id) + return nil +} + +// 1453/AC4 (ADR 0467 clause 4, ADR 0436 §1): the admin password is the module's own secret, issued by the +// operator (never generated), and owned by an account the module makes, brother-admin: the tools' runner +// (the operator account) cannot read it, the admin process (that account) can. The process's bus account is +// the own secret broker-admin, owned by the same account. +func TestTheAdminPasswordIsTheOperatorsOwnSecretOwnedByTheAdminAccount(t *testing.T) { m := readManifest(t) s, ok := m.OwnSecrets["admin-password"] if !ok { @@ -188,14 +202,99 @@ func TestTheAdminPasswordIsTheOperatorsOwnSecret(t *testing.T) { if s["issued-by"] != "outside" { t.Errorf("issued-by = %v, want outside: the mesh never generates the device's password", s["issued-by"]) } - path, _ := s["path"].(string) - var env string - for _, a := range m.Build.Artifacts { - if a["kind"] == "bundle" { - env, _ = a["env"].(map[string]any)["BROTHER_ADMIN_PASSWORD"].(string) - } + if s["owner"] != "brother-admin" { + t.Errorf("owner = %v, want brother-admin (never root or ${machine:account}: ADR 0436 §1)", s["owner"]) } - if path == "" || env != path { - t.Errorf("the secret's path %q and BROTHER_ADMIN_PASSWORD %q differ", path, env) + if p, _ := s["prompt"].(string); p != "the admin password of the Brother printer" { + t.Errorf("prompt = %q", p) + } + if b := m.OwnSecrets["broker-admin"]; b["owner"] != "brother-admin" || b["path"] != "${dir:state}/broker-admin.secret" { + t.Errorf("broker-admin = %v", b) + } + acct := resource(t, m, "account") + if acct["type"] != "user" || acct["name"] != "brother-admin" || acct["shell"] != "/usr/bin/nologin" { + t.Errorf("account = %v", acct) + } +} + +// 1453 (ADR 0467 clause 4): the admin process runs as brother-admin with the bus access of ADR 0436 §7: it +// consumes the work queue printer-asks and writes the state printer-answers, and is judged by its unit. The +// tools' bundle is given no password file. +func TestTheAdminProcessIsDeclaredAsMeshMailsSenderIs(t *testing.T) { + m := readManifest(t) + p := resource(t, m, "admin") + if p["type"] != "process" || p["name"] != "brother-admin" || p["user"] != "brother-admin" || p["artifact"] != "tools" { + t.Errorf("process = %v", p) + } + if !reflect.DeepEqual(p["run"], []any{"./brother", "admin"}) { + t.Errorf("run = %v", p["run"]) + } + bus, _ := p["bus"].(map[string]any) + if !reflect.DeepEqual(bus["queues"], []any{"printer-asks"}) || !reflect.DeepEqual(bus["state"], []any{"printer-answers"}) { + t.Errorf("bus = %v", bus) + } + if h, _ := p["health"].(map[string]any); h["kind"] != "unit" { + t.Errorf("health = %v", p["health"]) + } + env, _ := p["env"].(map[string]any) + want := map[string]any{ + "BROTHER_SETTINGS": "${dir:state}/settings.json", + "BROTHER_ADMIN_PASSWORD": m.OwnSecrets["admin-password"]["path"], + "BROTHER_BROKER": m.OwnSecrets["broker-admin"]["path"], + } + if !reflect.DeepEqual(env, want) { + t.Errorf("env = %v, want %v", env, want) + } + for _, a := range m.Build.Artifacts { + if a["kind"] == "bundle" { + if _, has := a["env"].(map[string]any)["BROTHER_ADMIN_PASSWORD"]; has { + t.Errorf("the tools' bundle is given the password file") + } + } + } + // The admin account reads the settings file and walks the state directory; neither holds a secret. + if r := resource(t, m, "settings"); r["mode"] != "0644" { + t.Errorf("settings mode = %v: the admin account must read it", r["mode"]) + } + if r := resource(t, m, "state"); r["mode"] != "0711" { + t.Errorf("state mode = %v: the admin account must reach its files in it", r["mode"]) + } + for _, r := range m.Resources { + if r["id"] != "admin" { + if r["takes-config"] != nil { + for _, k := range r["takes-config"].([]any) { + if k == "admin-password" { + t.Errorf("the password is a config value") + } + } + } + } + } +} + +// 1453 (ADR 0467 clause 4): the stream printer-asks is a work queue like mesh-mail's mail-sends, and the +// answers are a state that expires. +func TestThePrinterAsksStreamIsAWorkQueue(t *testing.T) { + raw, _ := os.ReadFile(filepath.Join("..", "..", "module.json")) + var m struct { + Streams []map[string]any `json:"streams"` + State []any `json:"state"` + } + if err := json.Unmarshal(raw, &m); err != nil { + t.Fatal(err) + } + if len(m.Streams) != 1 { + t.Fatalf("streams = %v", m.Streams) + } + st := m.Streams[0] + if st["name"] != "printer-asks" || st["retention"] != "work-queue" || st["max-deliveries"] != float64(3) || + !reflect.DeepEqual(st["events"], []any{EventAsked}) { + t.Errorf("stream = %v", st) + } + if st["max-age-days"] == nil || st["max-mib"] == nil { + t.Errorf("the stream is bounded: %v", st) + } + if len(m.State) != 1 || m.State[0].(map[string]any)["name"] != AnswersState || m.State[0].(map[string]any)["ttl-seconds"] == nil { + t.Errorf("state = %v", m.State) } } diff --git a/modules/brother/go.mod b/modules/brother/go.mod index 34ac1eb4..4c4a0e28 100644 --- a/modules/brother/go.mod +++ b/modules/brother/go.mod @@ -2,13 +2,21 @@ module brother go 1.26.0 -require git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261020060201-88581d0acaef +require ( + git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261020060201-88581d0acaef + github.com/nats-io/nats-server/v2 v2.11.17 + github.com/nats-io/nats.go v1.54.0 +) require ( + github.com/antithesishq/antithesis-sdk-go v0.7.0-default-no-op // indirect + github.com/google/go-tpm v0.9.8 // indirect github.com/klauspost/compress v1.20.0 // indirect - github.com/nats-io/nats.go v1.54.0 // indirect + github.com/minio/highwayhash v1.0.4 // indirect + github.com/nats-io/jwt/v2 v2.8.1 // indirect github.com/nats-io/nkeys v0.4.16 // indirect github.com/nats-io/nuid v1.0.1 // indirect golang.org/x/crypto v0.57.0 // indirect golang.org/x/sys v0.48.0 // indirect + golang.org/x/time v0.15.0 // indirect ) diff --git a/modules/brother/go.sum b/modules/brother/go.sum index a53f2835..628b153b 100644 --- a/modules/brother/go.sum +++ b/modules/brother/go.sum @@ -20,6 +20,7 @@ github.com/nats-io/nuid v1.0.1 h1:5iA8DT8V7q8WK2EScv2padNa/rTESc1KdnPw4TC2paw= github.com/nats-io/nuid v1.0.1/go.mod h1:19wcPz3Ph3q0Jbyiqsd0kePYG7A95tJPxeL+1OSON2c= golang.org/x/crypto v0.57.0 h1:3ZVCjf8Ggz7zneR/EHRVx68Ctf+2pmIMP2UFhh9cC6M= golang.org/x/crypto v0.57.0/go.mod h1:Fdz0i5U6CoizGwLda9DttjSk6qlZo25zYNtR+ycvuZA= +golang.org/x/sys v0.21.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/sys v0.48.0 h1:bbX/i/6MgT9BVLM9RT1thmxL04yeTAhbEz4SyadbXoo= golang.org/x/sys v0.48.0/go.mod h1:hNLxWAXmnKAxqDtdwIYC4bM9oQPEecfsnNMuSxOs3og= golang.org/x/time v0.15.0 h1:bbrp8t3bGUeFOx08pvsMYRTCVSMk89u4tKbNOZbp88U= diff --git a/modules/brother/module.json b/modules/brother/module.json index cf5dfb2d..8b35310b 100644 --- a/modules/brother/module.json +++ b/modules/brother/module.json @@ -65,9 +65,33 @@ "own-secrets": { "admin-password": { "path": "${dir:state}/admin-password.secret", - "issued-by": "outside" + "issued-by": "outside", + "prompt": "the admin password of the Brother printer", + "owner": "brother-admin" + }, + "broker-admin": { + "path": "${dir:state}/broker-admin.secret", + "owner": "brother-admin" } }, + "streams": [ + { + "name": "printer-asks", + "events": [ + "printer.asked" + ], + "max-age-days": 1, + "max-mib": 8, + "retention": "work-queue", + "max-deliveries": 3 + } + ], + "state": [ + { + "name": "printer-answers", + "ttl-seconds": 600 + } + ], "seats": [ { "name": "scanner", @@ -114,10 +138,16 @@ "scanner.list" ], "resources": [ + { + "id": "account", + "type": "user", + "name": "brother-admin", + "shell": "/usr/bin/nologin" + }, { "id": "state", "type": "directory", - "mode": "0700", + "mode": "0711", "owner": "${machine:account}", "place": ".", "unknown": "program" @@ -127,7 +157,7 @@ "type": "file", "path": "${dir:state}/settings.json", "owner": "${machine:account}", - "mode": "0600", + "mode": "0644", "takes-config": [ "scanner", "scan-folder", @@ -139,6 +169,36 @@ ], "merge": "json", "template": "files/settings.json" + }, + { + "id": "admin", + "type": "process", + "name": "brother-admin", + "artifact": "tools", + "run": [ + "./brother", + "admin" + ], + "user": "brother-admin", + "env": { + "BROTHER_SETTINGS": "${dir:state}/settings.json", + "BROTHER_ADMIN_PASSWORD": "${dir:state}/admin-password.secret", + "BROTHER_BROKER": "${dir:state}/broker-admin.secret" + }, + "restart-on": [ + "settings" + ], + "bus": { + "queues": [ + "printer-asks" + ], + "state": [ + "printer-answers" + ] + }, + "health": { + "kind": "unit" + } } ], "build": { @@ -154,8 +214,7 @@ "brother" ], "env": { - "BROTHER_SETTINGS": "${dir:state}/settings.json", - "BROTHER_ADMIN_PASSWORD": "${dir:state}/admin-password.secret" + "BROTHER_SETTINGS": "${dir:state}/settings.json" } }, {