From d865ed6425a8154405751644948f7034ef629c63 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 12 Oct 2026 14:41:28 +0200 Subject: [PATCH] Make no value for an own secret issued outside the mesh that nobody gave (issue 401) The send minted a random value for a plain own secret marked issued-by outside and placed it, so a module could not tell "not given" from "given and wrong" (ADR 0228). It now places only a value a person gave: not given is no file, an empty value where a module's file names it, and nothing to restart on, as ADR 0283 decided for a family's member. A value the mesh made for one earlier is not read as given. --- cmd/mesh-controller/outside_not_given_test.go | 120 ++++++++++++++++++ cmd/mesh-controller/plan.go | 38 +++++- internal/catalogue/declaration.go | 10 +- internal/catalogue/outside_not_given_test.go | 106 ++++++++++++++++ internal/catalogue/secrets_into_files.go | 27 +++- internal/inventory/secrets.go | 31 +++++ 6 files changed, 324 insertions(+), 8 deletions(-) create mode 100644 cmd/mesh-controller/outside_not_given_test.go create mode 100644 internal/catalogue/outside_not_given_test.go diff --git a/cmd/mesh-controller/outside_not_given_test.go b/cmd/mesh-controller/outside_not_given_test.go new file mode 100644 index 00000000..11a096c0 --- /dev/null +++ b/cmd/mesh-controller/outside_not_given_test.go @@ -0,0 +1,120 @@ +package main + +import ( + "testing" + + "github.com/novox/mesh-controller/internal/catalogue" +) + +// An own secret issued outside the mesh that nobody gave is never made (novox/hq issue 401, ADR 0228). Seen on +// 2026-10-10: the send made a random value for the mounts module's smb-credentials and placed it, and the module read +// it as a credential given and wrong. The send now makes none, places none, and places a value only once a person +// gave one — and a value the mesh made for it before is not placed either. + +// anAssistant is a module with a secret the mesh makes and one only an outside party issues. +func anAssistant() catalogue.Manifest { + return catalogue.Manifest{Module: "assistant", Version: "1", + OwnSecrets: catalogue.OwnSecrets{ + "password": {Path: "/var/lib/mesh/assistant/password", Taken: catalogue.TakenAtStart}, + "vendor-key": {Path: "/var/lib/mesh/assistant/vendor-key", Taken: catalogue.TakenAtStart, IssuedBy: catalogue.IssuedOutside}, + }, + Resources: []map[string]any{ + {"id": "state", "type": "directory", "path": "/var/lib/mesh/assistant", "mode": "0700"}, + }} +} + +// sentTo composes what a push sends to a machine, on the send path. +func sentTo(t *testing.T, open *stores, node string) []map[string]any { + t.Helper() + plan, settings, err := planFor(t.Context(), open, node) + if err != nil { + t.Fatal(err) + } + declared, err := declarationFor(t.Context(), open, node, plan, settings) + if err != nil { + t.Fatalf("the send to %s did not compose: %v", node, err) + } + return declared.Resources +} + +func placedAt(resources []map[string]any, path string) map[string]any { + for _, r := range resources { + if r["path"] == path && r["sealed"] != nil { + return r + } + } + return nil +} + +func TestTheSendMakesNoValueForAnOutsideSecretNobodyGave(t *testing.T) { + open := aMesh(t) + ctx := t.Context() + register(t, open, anAssistant()) + if _, err := assign(ctx, open, "laptop", "assistant"); err != nil { + t.Fatal(err) + } + + sent := sentTo(t, open, "laptop") + if placedAt(sent, "/var/lib/mesh/assistant/password") == nil { + t.Fatal("the secret the mesh makes was not placed") + } + if got := placedAt(sent, "/var/lib/mesh/assistant/vendor-key"); got != nil { + t.Fatalf("a secret nobody gave was placed: %v", got) + } + if _, held, err := open.inventory.ModuleSecretIfIssued(ctx, "laptop", "assistant", "vendor-key"); err != nil || held { + t.Fatalf("the send made a value for a secret issued outside the mesh (held %v, %v)", held, err) + } + + // A value the mesh made before this rule is not one a person gave: it is not placed. + if _, err := open.inventory.SecretForModule(ctx, "laptop", "assistant", "vendor-key"); err != nil { + t.Fatal(err) + } + if got := placedAt(sentTo(t, open, "laptop"), "/var/lib/mesh/assistant/vendor-key"); got != nil { + t.Fatalf("a value the mesh made for a secret issued outside was placed: %v", got) + } + + // Given, it is placed. + if err := open.inventory.AcceptSecretForModule(ctx, "laptop", "assistant", "vendor-key", "the vendor's key"); err != nil { + t.Fatal(err) + } + given, ok, err := open.inventory.GivenOwnSecret(ctx, "laptop", "assistant", "vendor-key") + if err != nil || !ok || !given.Current { + t.Fatalf("the value given is not read as given: %+v %v %v", given, ok, err) + } + placed := placedAt(sentTo(t, open, "laptop"), "/var/lib/mesh/assistant/vendor-key") + if placed == nil || placed["sealed"] != given.Sealed { + t.Fatalf("the value given is placed as %v", placed) + } +} + +// The read paths answer as the send does: a question about the machine composes without the secret nobody gave, and +// asking writes nothing. +func TestAQuestionComposesWithoutAnOutsideSecretNobodyGave(t *testing.T) { + open := aMesh(t) + ctx := t.Context() + register(t, open, anAssistant()) + if _, err := assign(ctx, open, "laptop", "assistant"); err != nil { + t.Fatal(err) + } + pushedBy(t, open, "laptop") // the mesh's own secret is made by the send + plan, settings, err := planFor(ctx, open, "laptop") + if err != nil { + t.Fatal(err) + } + gens, err := generators(ctx, open) + if err != nil { + t.Fatal(err) + } + for _, choosing := range []Choosing{Reading, Foreseeing} { + got, err := declarationWith(ctx, open, "laptop", plan, settings, gens, choosing) + if err != nil { + t.Fatalf("%v: %v", choosing, err) + } + if placedAt(got.Resources, "/var/lib/mesh/assistant/vendor-key") != nil || len(got.foreseen) != 0 { + t.Fatalf("%v: a secret nobody gave is composed (foreseen %v)", choosing, got.foreseen) + } + } + if _, held, err := open.inventory.ModuleSecretIfIssued(ctx, "laptop", "assistant", "vendor-key"); err != nil || held { + t.Fatalf("a question made a value (held %v, %v)", held, err) + } +} diff --git a/cmd/mesh-controller/plan.go b/cmd/mesh-controller/plan.go index b4811f4e..11d6c316 100644 --- a/cmd/mesh-controller/plan.go +++ b/cmd/mesh-controller/plan.go @@ -561,6 +561,13 @@ func busCredentialIssued(ctx context.Context, inv *inventory.Inventory, node, mo return nil } +// givenUnderAnOldKey is the refusal for a value a person gave — a family's member, or an own secret issued outside +// the mesh — sealed to a key the machine no longer holds: the mesh cannot make another, so it is given again. +func givenUnderAnOldKey(module, node, name string) error { + return fmt.Errorf("%s on %s holds %q, which was given to the mesh rather than made by it, and %s has "+ + "since generated a new sealing key. The mesh cannot make another; give it again", module, node, name, node) +} + // renderingFor is everything a node's declaration is composed with, and the node's record. func renderingFor(ctx context.Context, open *stores, node string, plan catalogue.Resolution, settings catalogue.SettingsBy, @@ -679,10 +686,7 @@ func renderingFor(ctx context.Context, open *stores, node string, } if !g.Current { if choosing == Allocating { - return catalogue.Rendering{}, inventory.Node{}, fmt.Errorf( - "%s on %s holds %q, which was given to the mesh rather than made by it, and %s has "+ - "since generated a new sealing key. The mesh cannot make another; give it again", - m.Module, node, g.Name, node) + return catalogue.Rendering{}, inventory.Node{}, givenUnderAnOldKey(m.Module, node, g.Name) } continue } @@ -692,7 +696,31 @@ func renderingFor(ctx context.Context, open *stores, node string, needed[m.Module][g.Name] = g.Sealed } } - for name := range m.OwnSecrets.Plain() { + for name, own := range m.OwnSecrets.Plain() { + // **An own secret issued outside the mesh is never made** (novox/hq issue 401, ADR 0228; ADR 0283 for + // a family): only a value a person gave is placed. One not given is nothing — no file, and an empty + // value where a file of the module names it — so the module can tell it was not given, where a value + // the mesh made looked given and wrong. A value the mesh made for it before is not one either. + if own.IssuedBy == catalogue.IssuedOutside { + g, given, err := inv.GivenOwnSecret(ctx, node, m.Module, name) + if err != nil { + return catalogue.Rendering{}, inventory.Node{}, err + } + if !given { + continue + } + if !g.Current { + if choosing == Allocating { + return catalogue.Rendering{}, inventory.Node{}, givenUnderAnOldKey(m.Module, node, name) + } + continue + } + if needed[m.Module] == nil { + needed[m.Module] = map[string]string{} + } + needed[m.Module][name] = g.Sealed + continue + } // Minted on the send path and only read on every other. Making one is an insert, and // a question that writes is a question that can block against the machine it is about. var sealed string diff --git a/internal/catalogue/declaration.go b/internal/catalogue/declaration.go index a74ca6dc..4cbdde6b 100644 --- a/internal/catalogue/declaration.go +++ b/internal/catalogue/declaration.go @@ -645,6 +645,11 @@ func (r Resolution) compose(with Rendering, owner map[string]string, sealed = ForeseenSealed *foreseen = append(*foreseen, m.Module+"/"+name) } + if sealed == "" && m.OwnSecrets[name].IssuedBy == IssuedOutside { + // **Not given, and the mesh never makes it** (novox/hq issue 401, ADR 0228): no file, as for a + // family's member (ADR 0283), so the module reads its absence as not given. + continue + } if sealed == "" { // Declared and not made. Refused rather than skipped: a module whose own // credential is silently absent starts, fails to authenticate, and the reason is @@ -1002,7 +1007,7 @@ func (r Resolution) compose(with Rendering, owner map[string]string, // content is where a setting lands, so a placeholder may only exist once the setting // has been put in — filling secrets first would look at content that is not yet what // the machine receives. - if err := intoFile(copied, sealed, m.Module); err != nil { + if err := intoFile(copied, sealed, outsideNotGiven(m, with.Needed[m.Module]), m.Module); err != nil { return nil, err } // **Substituted here, not on the machine.** A bound value is not secret — the mesh @@ -2431,6 +2436,9 @@ func secretsReadBy(resource map[string]any, m Manifest, given map[string]string) } paths := map[string]string{} for name, own := range m.OwnSecrets.Plain() { + if own.IssuedBy == IssuedOutside && given[name] == "" { + continue // not given, so not placed: nothing to restart on (novox/hq issue 401) + } paths[name] = own.Path } for name, sealed := range given { diff --git a/internal/catalogue/outside_not_given_test.go b/internal/catalogue/outside_not_given_test.go new file mode 100644 index 00000000..a63e3d8a --- /dev/null +++ b/internal/catalogue/outside_not_given_test.go @@ -0,0 +1,106 @@ +package catalogue + +import ( + "strings" + "testing" +) + +// An own secret issued outside the mesh that nobody gave (novox/hq issue 401, ADR 0228): the mesh makes no value for it, +// so the composition places no file, a file of the module that names it says empty there, and nothing restarts on it. +// Given, it is placed and filled like any own secret. + +func outsideManifest(t *testing.T) Manifest { + t.Helper() + m, err := ParseManifest([]byte(`{"module":"assistant","version":"1", + "own-secrets":{ + "password":{"path":"/var/lib/assistant/password.secret","taken":"at-start"}, + "vendor-key":{"path":"/var/lib/assistant/vendor-key.secret","taken":"at-start","issued-by":"outside"}}, + "resources":[ + {"id":"env","type":"file","path":"/var/lib/assistant/env","mode":"0600", + "content":"PASSWORD=${secret:password}\nVENDOR_KEY=${secret:vendor-key}\n"}, + {"id":"server","type":"container","name":"assistant","image":"assistant:1", + "env-file":["/var/lib/assistant/env"], + "volumes":["/var/lib/assistant/vendor-key.secret:/run/vendor-key:ro"], + "secrets-in-environment":"reads its settings from the environment only"}]}`)) + if err != nil { + t.Fatal(err) + } + return m +} + +func resourceAt(out []map[string]any, path string) map[string]any { + for _, r := range out { + if r["path"] == path { + return r + } + } + return nil +} + +func resourceNamed(out []map[string]any, name string) map[string]any { + for _, r := range out { + if r["name"] == name { + return r + } + } + return nil +} + +func TestAnOutsideSecretNotGivenIsNoFileAndAFileNamingItSaysEmpty(t *testing.T) { + r := Resolution{Node: "server", Modules: []Manifest{outsideManifest(t)}} + out, err := r.Declaration(Rendering{Needed: map[string]map[string]string{"assistant": {"password": "sealed-password"}}}) + if err != nil { + t.Fatalf("a module whose outside secret nobody gave did not compose: %v", err) + } + if got := resourceAt(out, "/var/lib/assistant/vendor-key.secret"); got != nil { + t.Fatalf("a secret nobody gave was placed: %v", got) + } + env := resourceAt(out, "/var/lib/assistant/env") + if env == nil { + t.Fatal("the module's own file is not composed") + } + if content := env["content"].(string); content != "PASSWORD=${secret:password}\nVENDOR_KEY=\n" { + t.Fatalf("the file names the secret nobody gave as %q", content) + } + secrets, _ := env["secrets"].(map[string]any) + if len(secrets) != 1 || secrets["password"] != "sealed-password" { + t.Fatalf("the file is filled from %v", secrets) + } + server := resourceNamed(out, "assistant") + if on := strings.Join(stringsIn(server["restart-on"]), " "); strings.Contains(on, "vendor-key") { + t.Fatalf("the container restarts on a secret nobody gave: %q", on) + } +} + +func TestAnOutsideSecretGivenIsPlacedAndFilled(t *testing.T) { + r := Resolution{Node: "server", Modules: []Manifest{outsideManifest(t)}} + out, err := r.Declaration(Rendering{Needed: map[string]map[string]string{"assistant": { + "password": "sealed-password", "vendor-key": "sealed-vendor-key"}}}) + if err != nil { + t.Fatal(err) + } + placed := resourceAt(out, "/var/lib/assistant/vendor-key.secret") + if placed == nil || placed["sealed"] != "sealed-vendor-key" { + t.Fatalf("the secret given is placed as %v", placed) + } + env := resourceAt(out, "/var/lib/assistant/env") + if content := env["content"].(string); content != "PASSWORD=${secret:password}\nVENDOR_KEY=${secret:vendor-key}\n" { + t.Fatalf("the file was changed: %q", content) + } + if secrets, _ := env["secrets"].(map[string]any); secrets["vendor-key"] != "sealed-vendor-key" { + t.Fatalf("the file is filled from %v", secrets) + } + server := resourceNamed(out, "assistant") + if on := strings.Join(stringsIn(server["restart-on"]), " "); !strings.Contains(on, "assistant."+NeedID("vendor-key")) { + t.Fatalf("the container does not restart on the secret given: %q", on) + } +} + +// A secret the mesh makes is still refused when it was not made: only an outside one is left out. +func TestAnOwnSecretTheMeshMakesIsStillRefusedWhenNotMade(t *testing.T) { + r := Resolution{Node: "server", Modules: []Manifest{outsideManifest(t)}} + _, err := r.Declaration(Rendering{}) + if nm, ok := err.(*NotMadeError); !ok || nm.Name != "password" { + t.Fatalf("a secret the mesh makes, not made: %v", err) + } +} diff --git a/internal/catalogue/secrets_into_files.go b/internal/catalogue/secrets_into_files.go index 106d7898..0ed07c37 100644 --- a/internal/catalogue/secrets_into_files.go +++ b/internal/catalogue/secrets_into_files.go @@ -156,7 +156,11 @@ func secretFilesOf(resources []map[string]any) map[string]bool { // A name the module never declared is refused here rather than on the machine. The host would // refuse it too — but it would do so having already been handed a declaration, which reads as the // mesh sending something broken, and the name it could not find is a manifest's typo. -func intoFile(resource map[string]any, sealed map[string]string, module string) error { +// +// **An own secret issued outside the mesh that nobody gave is filled with nothing** (novox/hq issue 401): the mesh +// never makes one (ADR 0228), and a file is where the module reads it, so the place says empty — the module sees it +// was not given, where a value the mesh made looked given and wrong. notGiven names those secrets. +func intoFile(resource map[string]any, sealed map[string]string, notGiven map[string]bool, module string) error { if fmt.Sprint(resource["type"]) != "file" { return nil } @@ -171,6 +175,10 @@ func intoFile(resource map[string]any, sealed map[string]string, module string) into := map[string]any{} for _, name := range used { value := sealed[name] + if value == "" && notGiven[name] { + content = strings.ReplaceAll(content, "${secret:"+name+"}", "") + continue + } if value == "" { return fmt.Errorf( "%s has a file that says ${secret:%s}, and %s has no secret of its own by that "+ @@ -179,10 +187,25 @@ func intoFile(resource map[string]any, sealed map[string]string, module string) } into[name] = value } - resource["secrets"] = into + resource["content"] = content + if len(into) > 0 { + resource["secrets"] = into + } return nil } +// outsideNotGiven is each own secret of a module issued outside the mesh that this composition holds no value for +// (novox/hq issue 401): what intoFile fills with nothing, and what nothing restarts on. +func outsideNotGiven(m Manifest, given map[string]string) map[string]bool { + out := map[string]bool{} + for name, own := range m.OwnSecrets.Plain() { + if own.IssuedBy == IssuedOutside && given[name] == "" { + out[name] = true + } + } + return out +} + // namesOr says what a module could have written, because the answer to "that name is wrong" is // almost always one of two or three right ones. func namesOr(sealed map[string]string) string { diff --git a/internal/inventory/secrets.go b/internal/inventory/secrets.go index 34ee9cb1..c51480a1 100644 --- a/internal/inventory/secrets.go +++ b/internal/inventory/secrets.go @@ -325,6 +325,8 @@ func (i *Inventory) ModuleSecretIfIssued( return sealed, true, nil } +// SecretForModule makes what is not held, so it is never asked for an own secret issued outside the mesh: the +// composition reads only a value a person gave for one (GivenOwnSecret; novox/hq issue 401, ADR 0228). func (i *Inventory) SecretForModule(ctx context.Context, node, module, name string) (string, error) { key, err := i.SealingKeyOf(ctx, node) if err != nil { @@ -655,6 +657,35 @@ func (i *Inventory) GivenMembers(ctx context.Context, node, module, family strin return out, rows.Err() } +// GivenOwnSecret is an own secret issued outside the mesh as a person gave it on a machine (novox/hq issue 401, ADR +// 0228): its value sealed to the machine and whether that is the key the machine holds now. Not given is false: a +// value the mesh made for it is not one a person gave, and is never read as one; a machine with no sealing key +// holds none either. +func (i *Inventory) GivenOwnSecret(ctx context.Context, node, module, name string) (GivenMember, bool, error) { + key, err := i.SealingKeyOf(ctx, node) + if err != nil || key == "" { + return GivenMember{}, false, err + } + record, err := i.NodeByName(ctx, node) + if err != nil { + return GivenMember{}, false, err + } + g := GivenMember{Name: name} + var against string + err = i.store.Pool().QueryRow(ctx, + `select sealed, node_key from module_secret + where node = $1 and module = $2 and name = $3 and origin = 'accepted'`, + record.ID, module, name).Scan(&g.Sealed, &against) + if errors.Is(err, pgx.ErrNoRows) { + return GivenMember{}, false, nil + } + if err != nil { + return GivenMember{}, false, err + } + g.Current = against == key + return g, true, nil +} + // GivenOwnSecrets is every own secret of a module a person gave on a machine, by name, with when (novox/hq ADR // 0283): what the controller checks a module's wait for a secret against. A value the mesh made is not one. func (i *Inventory) GivenOwnSecrets(ctx context.Context, node, module string) (map[string]time.Time, error) {