From 1469f5ff826a63ec416d19f639b32f8c9b96ac6e Mon Sep 17 00:00:00 2001 From: jochen Date: Thu, 1 Oct 2026 11:41:49 +0200 Subject: [PATCH] A module's own secret rotates when its definition says the module reads it at start (hq 180) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `secret rotate ` makes the secret anew the way the first mint did, seals it to the machine and the operator, and sends the machine, so the module starts again on the new value — said in the log with who asked and when, never the value. Only for a secret whose definition says `"taken": "at-start"`: an own secret is now a path, or {path, taken}, and a definition that says nothing of how a secret is taken is refused with the word to write, because a credential rotated under software that never reads it again is worse than one left alone (issue 179). `applied` is refused by name until the staged form ADR 0114 decided is built; a value given to the mesh is refused as ADR 0113 says. `rotate` is a verb on the controller's seat with two shapes — a pair credential by provision, an own secret by machine, module and name — so the console can ask. Registered manifests keep their bytes: a path alone is written back as a path. --- cmd/mesh-controller/modules_test.go | 4 +- cmd/mesh-controller/seatverbs.go | 14 +++ cmd/mesh-controller/seatverbs_test.go | 17 ++++ cmd/mesh-controller/secret.go | 49 +++++++++- internal/catalogue/declaration.go | 2 +- internal/catalogue/dir_into.go | 14 ++- internal/catalogue/dir_into_test.go | 8 +- internal/catalogue/filtering_test.go | 2 +- internal/catalogue/manifest.go | 93 +++++++++++++++++-- internal/catalogue/needs_test.go | 4 +- internal/catalogue/own_secret_taken_test.go | 54 +++++++++++ .../catalogue/secrets_in_environment_test.go | 2 +- internal/catalogue/secrets_into_files_test.go | 12 +-- internal/catalogue/verbs.go | 11 +++ internal/inventory/forget_test.go | 4 +- internal/inventory/operator_test.go | 2 +- internal/inventory/secrets.go | 87 ++++++++++++++++- internal/inventory/secrets_test.go | 64 ++++++++++++- 18 files changed, 409 insertions(+), 34 deletions(-) create mode 100644 internal/catalogue/own_secret_taken_test.go diff --git a/cmd/mesh-controller/modules_test.go b/cmd/mesh-controller/modules_test.go index edfc937..4de406c 100644 --- a/cmd/mesh-controller/modules_test.go +++ b/cmd/mesh-controller/modules_test.go @@ -11,7 +11,7 @@ import ( // A module that declares none is refused before the account exists, so the bus never carries an // account nothing reads (novox/hq 04-ISSUES/078). func TestAModuleWithNoBrokerSecretCannotBeIssued(t *testing.T) { - err := mayIssue(catalogue.Manifest{Module: "step-ca", OwnSecrets: map[string]string{"password": "/run/password"}}) + err := mayIssue(catalogue.Manifest{Module: "step-ca", OwnSecrets: catalogue.OwnSecrets{"password": {Path: "/run/password"}}}) if err == nil { t.Fatal("a module with no broker own secret was issued an account") } @@ -20,7 +20,7 @@ func TestAModuleWithNoBrokerSecretCannotBeIssued(t *testing.T) { t.Errorf("the refusal does not say %q: %v", want, err) } } - if err := mayIssue(catalogue.Manifest{Module: "redis", OwnSecrets: map[string]string{"broker": "/run/broker"}}); err != nil { + if err := mayIssue(catalogue.Manifest{Module: "redis", OwnSecrets: catalogue.OwnSecrets{"broker": {Path: "/run/broker"}}}); err != nil { t.Errorf("a module declaring its broker secret was refused: %v", err) } } diff --git a/cmd/mesh-controller/seatverbs.go b/cmd/mesh-controller/seatverbs.go index 55b31bd..3b3c608 100644 --- a/cmd/mesh-controller/seatverbs.go +++ b/cmd/mesh-controller/seatverbs.go @@ -87,6 +87,20 @@ func argvFor(verb string, args map[string]any) ([]string, error) { return []string{"push", n, "--wait", "0"}, nil } return []string{"push", "--behind", "--wait", "0"}, nil + case "rotate": + if p := str("provision"); p != "" { + argv := []string{"rotate", p} + if c := str("consumer"); c != "" { + argv = append(argv, "--consumer", c) + } + return argv, nil + } + if str("node") != "" && str("module") != "" && str("secret") != "" { + return []string{"secret", "rotate", str("node"), str("module"), str("secret")}, nil + } + // Half of either shape: the command says its usage, which names both shapes, and that is + // the answer the caller needs. + return []string{"rotate"}, nil case "build": if err := need("repository"); err != nil { return nil, err diff --git a/cmd/mesh-controller/seatverbs_test.go b/cmd/mesh-controller/seatverbs_test.go index d9b071f..44365e3 100644 --- a/cmd/mesh-controller/seatverbs_test.go +++ b/cmd/mesh-controller/seatverbs_test.go @@ -56,6 +56,23 @@ func TestTheBuildToolTellsAForgePathFromAURL(t *testing.T) { } } +// `rotate` is one verb with two shapes (ADR 0114, issue 180): a pair credential by provision, or a +// module's own secret by machine, module and name. +func TestRotateTakesAProvisionOrAnOwnSecret(t *testing.T) { + argv, _ := argvFor("rotate", map[string]any{"provision": "postgres-database", "consumer": "ace"}) + if strings.Join(argv, " ") != "rotate postgres-database --consumer ace" { + t.Fatalf("a pair credential: %v", argv) + } + argv, _ = argvFor("rotate", map[string]any{"node": "ace", "module": "nodered", "secret": "api-token"}) + if strings.Join(argv, " ") != "secret rotate ace nodered api-token" { + t.Fatalf("an own secret: %v", argv) + } + argv, _ = argvFor("rotate", map[string]any{"node": "ace"}) + if strings.Join(argv, " ") != "rotate" { + t.Fatalf("half an own secret falls to the command's usage: %v", argv) + } +} + // A required argument missing is refused in the verb's own words, before anything runs. func TestAVerbMissingWhatItNeedsIsRefused(t *testing.T) { if _, err := argvFor("node", map[string]any{}); err == nil || !strings.Contains(err.Error(), `node needs "node"`) { diff --git a/cmd/mesh-controller/secret.go b/cmd/mesh-controller/secret.go index eee4a37..d7f3064 100644 --- a/cmd/mesh-controller/secret.go +++ b/cmd/mesh-controller/secret.go @@ -10,6 +10,7 @@ import ( "io" "os" "strings" + "time" "github.com/novox/mesh-controller/internal/catalogue" "github.com/novox/mesh-controller/internal/inventory" @@ -38,6 +39,8 @@ func secretCommand(ctx context.Context, args []string) error { } switch args[0] { case "accept": + case "rotate": + return secretRotate(ctx, args[1:]) case "recover": return secretRecover(ctx, args[1:]) case "export": @@ -101,7 +104,8 @@ func secretCommand(ctx context.Context, args []string) error { return nil } -const secretUsage = "secret accept [--from ] [--provider [--local ]]\n" + +const secretUsage = "secret rotate \n" + + "secret accept [--from ] [--provider [--local ]]\n" + "secret recover --key [--out ] [--from-export ] [--provider ]\n" + "secret export [--out ]" @@ -359,3 +363,46 @@ func valueFor(node, module, name, from string) (string, error) { return line, nil } } + +// secretRotate makes a module's own secret anew and sends the machine, so the module starts again on +// the new value (novox/hq ADR 0114, issue 180). A pair credential rotates with `rotate `; +// this is the secret with one party. Said in the log with who asked and when, never the value. +func secretRotate(ctx context.Context, args []string) error { + rest, _ := split(args) + if len(rest) != 3 { + return errors.New(secretUsage) + } + node, module, name := rest[0], rest[1], rest[2] + open, err := openStores(ctx) + if err != nil { + return err + } + defer open.Close() + if err := open.inventory.RotateModuleSecret(ctx, node, module, name); err != nil { + var refused inventory.ErrNotRotatable + if errors.As(err, &refused) { + return fmt.Errorf("not rotated: %s", refused.Why) + } + return err + } + fmt.Printf("rotated %q of %s on %s at %s, asked by %s; the value is sealed and not shown\n", + name, module, node, time.Now().UTC().Format(time.RFC3339), whoAsked()) + fmt.Printf("sending %s, so %s starts again on the new value:\n", node, module) + if err := sendTo(ctx, open, []string{node}); err != nil { + return fmt.Errorf("%w\n\nThe new value is sealed and not yet delivered; what runs keeps the old "+ + "one until the machine next applies. Fix the cause and run `push %s`", err, node) + } + return nil +} + +// whoAsked names the caller for the log: the account the command runs as, which for a tool call +// through the console is the mesh's own. +func whoAsked() string { + if u := os.Getenv("SUDO_USER"); u != "" { + return u + } + if u := os.Getenv("USER"); u != "" { + return u + } + return "the mesh" +} diff --git a/internal/catalogue/declaration.go b/internal/catalogue/declaration.go index 8ab57f3..41e77e5 100644 --- a/internal/catalogue/declaration.go +++ b/internal/catalogue/declaration.go @@ -465,7 +465,7 @@ func (r Resolution) compose(with Rendering, owner map[string]string) ([]map[stri "%s needs a secret called %q and none was made for it", m.Module, name) } first = append(first, ownedBy(m.SecretsOwner, map[string]any{ - "id": NeedID(name), "type": "file", "path": m.OwnSecrets[name], "sealed": sealed, + "id": NeedID(name), "type": "file", "path": m.OwnSecrets[name].Path, "sealed": sealed, })) } // Operator-owned paths this module is granted use of (novox/hq ADR 0051). Written before diff --git a/internal/catalogue/dir_into.go b/internal/catalogue/dir_into.go index 3749e1a..f560d37 100644 --- a/internal/catalogue/dir_into.go +++ b/internal/catalogue/dir_into.go @@ -161,8 +161,16 @@ func placedManifest(m Manifest, with Rendering) (Manifest, error) { if m.Secrets, err = fillMap(m.Secrets); err != nil { return m, err } - if m.OwnSecrets, err = fillMap(m.OwnSecrets); err != nil { - return m, err + if len(m.OwnSecrets) > 0 { + own := make(OwnSecrets, len(m.OwnSecrets)) + for name, s := range m.OwnSecrets { + filled, err := dirFill(s.Path, dirs, m.Module) + if err != nil { + return m, err + } + own[name] = OwnSecret{Path: filled, Taken: s.Taken} + } + m.OwnSecrets = own } if m.Grants, err = fillMap(m.Grants); err != nil { return m, err @@ -335,7 +343,7 @@ func (m Manifest) unknownDirRefs() []string { } maps := map[string]map[string]string{ "receives": m.Receives, "binds": m.Binds, "secrets": m.Secrets, - "own-secrets": m.OwnSecrets, "grants": m.Grants, + "own-secrets": m.OwnSecrets.Paths(), "grants": m.Grants, } for field, entries := range maps { for _, value := range entries { diff --git a/internal/catalogue/dir_into_test.go b/internal/catalogue/dir_into_test.go index 9e13399..ee7a96e 100644 --- a/internal/catalogue/dir_into_test.go +++ b/internal/catalogue/dir_into_test.go @@ -164,7 +164,7 @@ func TestTheManifestsMapsArePlaced(t *testing.T) { }, Binds: map[string]string{"route": "${dir:state}/route.json"}, Secrets: map[string]string{"mongodb-database": "${dir:state}/database.secret"}, - OwnSecrets: map[string]string{"admin-key": "${dir:state}/admin-key.secret"}, + OwnSecrets: OwnSecrets{"admin-key": {Path: "${dir:state}/admin-key.secret"}}, Receives: map[string]string{"route": "${dir:state}/grants/mesh.json"}, } placed, err := placedManifest(m, Rendering{}) @@ -177,7 +177,7 @@ func TestTheManifestsMapsArePlaced(t *testing.T) { if placed.Secrets["mongodb-database"] != "/var/lib/photos/database.secret" { t.Fatalf("secrets are placed; got %v", placed.Secrets) } - if placed.OwnSecrets["admin-key"] != "/var/lib/photos/admin-key.secret" { + if placed.OwnSecrets["admin-key"].Path != "/var/lib/photos/admin-key.secret" { t.Fatalf("own-secrets are placed; got %v", placed.OwnSecrets) } if placed.Receives["route"] != "/var/lib/photos/grants/mesh.json" { @@ -231,7 +231,7 @@ func TestTheMeshsDirectoryForAModuleIsAPlace(t *testing.T) { {"id": "server", "type": "container", "image": "x@sha256:aa", "volumes": []any{"${dir:mesh-state}/broker:/run/secrets/broker:ro"}}, }, - OwnSecrets: map[string]string{"broker": "${dir:mesh-state}/broker"}, + OwnSecrets: OwnSecrets{"broker": {Path: "${dir:mesh-state}/broker"}}, Binds: map[string]string{"route": "${dir:state}/route.json"}, } if got := m.unknownDirRefs(); len(got) != 0 { @@ -249,7 +249,7 @@ func TestTheMeshsDirectoryForAModuleIsAPlace(t *testing.T) { if err != nil { t.Fatal(err) } - if placed.OwnSecrets["broker"] != "/var/lib/mesh/umami/broker" { + if placed.OwnSecrets["broker"].Path != "/var/lib/mesh/umami/broker" { t.Fatalf("own-secrets are placed under the mesh's directory; got %v", placed.OwnSecrets) } container := shallowCopy(m.Resources[2]) diff --git a/internal/catalogue/filtering_test.go b/internal/catalogue/filtering_test.go index 0cd3bf4..5048a98 100644 --- a/internal/catalogue/filtering_test.go +++ b/internal/catalogue/filtering_test.go @@ -419,7 +419,7 @@ func TestWhatTheMeshComputesIsAppliedBeforeWhatTheModuleDeclared(t *testing.T) { func TestAComputedModuleStillGetsWhatTheMeshMadeForIt(t *testing.T) { r := Resolution{Modules: []Manifest{{ Module: "networking", Computed: "mesh-network", - OwnSecrets: map[string]string{"key": "/var/lib/mesh/key"}, + OwnSecrets: OwnSecrets{"key": {Path: "/var/lib/mesh/key"}}, }}} out, err := r.Declaration(Rendering{ Needed: map[string]map[string]string{"networking": {"key": "sealed"}}, diff --git a/internal/catalogue/manifest.go b/internal/catalogue/manifest.go index 238a8d6..a14c453 100644 --- a/internal/catalogue/manifest.go +++ b/internal/catalogue/manifest.go @@ -390,7 +390,16 @@ type Manifest struct { // module running on three machines has three passwords and the mesh can read none of them. A // manifest carrying one instead would put the same secret on every machine that ever runs the // module, in a file anybody can read, for ever. - OwnSecrets map[string]string `json:"own-secrets,omitempty"` + // + // **And how the module takes it** (novox/hq ADR 0114, issue 180): `"admin": ""` says + // where and nothing else; `"admin": {"path": "", "taken": "at-start"}` says the module + // reads the file when it starts, so the mesh may rotate it by making a new value and starting + // the module again; `"taken": "applied"` says the module's own code applies it to a backend + // that takes it only once, so a rotation must be staged beside the current value — the form the + // mesh does not build yet, and refuses by name. A secret that says neither is not rotated by + // the mesh: the one fault worse than an unrotated credential is a rotated one the software + // never saw. + OwnSecrets OwnSecrets `json:"own-secrets,omitempty"` // SecretsOwner is who the files holding this module's secrets belong to on the machine — // `uid:gid`, or a name — when its process is not root. @@ -1419,14 +1428,20 @@ func ParseManifest(raw []byte) (Manifest, error) { } } } - for name, where := range m.OwnSecrets { - if !placedOrAbsolute(where) { + for name, own := range m.OwnSecrets { + if !placedOrAbsolute(own.Path) { problems = append(problems, fmt.Sprintf( - "%s needs %q at %q, which is neither an absolute path nor a placed one", m.Module, name, where)) + "%s needs %q at %q, which is neither an absolute path nor a placed one", m.Module, name, own.Path)) } if name == "" { problems = append(problems, m.Module+" needs a secret with no name") } + if own.Taken != "" && own.Taken != TakenAtStart && own.Taken != TakenApplied { + problems = append(problems, fmt.Sprintf( + "%s says its secret %q is taken %q; a secret is taken %q (read when the module starts) "+ + "or %q (applied by the module's own code to a backend that takes it once)", + m.Module, name, own.Taken, TakenAtStart, TakenApplied)) + } } localOf := map[string]string{} for _, to := range m.SecretRequirements() { @@ -1690,8 +1705,8 @@ func (m Manifest) undeclaredMounts() []string { claim(fmt.Sprint(r["path"])) } } - for _, where := range m.OwnSecrets { - claim(where) + for _, own := range m.OwnSecrets { + claim(own.Path) } for _, to := range m.SecretRequirements() { for _, f := range m.SecretFiles(to) { @@ -1826,3 +1841,69 @@ func invokeProblems(m Manifest) []string { } return problems } + +// How a module takes one of its own secrets (ADR 0114): read from the file when it starts, or +// applied by its own code to a backend that takes it once. +const ( + TakenAtStart = "at-start" + TakenApplied = "applied" +) + +// OwnSecret is where one of a module's own secrets lands, and how the module takes it. +type OwnSecret struct { + Path string + Taken string +} + +// OwnSecrets is a module's own secrets by name. On the wire each is a path, or an object naming +// the path and how it is taken; written back the way it was read, so a manifest the mesh holds +// keeps its bytes. +type OwnSecrets map[string]OwnSecret + +func (o *OwnSecrets) UnmarshalJSON(raw []byte) error { + var entries map[string]json.RawMessage + if err := json.Unmarshal(raw, &entries); err != nil { + return err + } + out := make(OwnSecrets, len(entries)) + for name, body := range entries { + var path string + if err := json.Unmarshal(body, &path); err == nil { + out[name] = OwnSecret{Path: path} + continue + } + var long struct { + Path string `json:"path"` + Taken string `json:"taken,omitempty"` + } + dec := json.NewDecoder(bytes.NewReader(body)) + dec.DisallowUnknownFields() + if err := dec.Decode(&long); err != nil { + return fmt.Errorf("own-secrets.%s: a path, or {\"path\", \"taken\"}: %w", name, err) + } + out[name] = OwnSecret{Path: long.Path, Taken: long.Taken} + } + *o = out + return nil +} + +func (o OwnSecrets) MarshalJSON() ([]byte, error) { + entries := make(map[string]any, len(o)) + for name, s := range o { + if s.Taken == "" { + entries[name] = s.Path + continue + } + entries[name] = map[string]string{"path": s.Path, "taken": s.Taken} + } + return json.Marshal(entries) +} + +// Paths is each own secret's path by name — the shape every placement and file walk reads. +func (o OwnSecrets) Paths() map[string]string { + out := make(map[string]string, len(o)) + for name, s := range o { + out[name] = s.Path + } + return out +} diff --git a/internal/catalogue/needs_test.go b/internal/catalogue/needs_test.go index 4fcead8..8a176a1 100644 --- a/internal/catalogue/needs_test.go +++ b/internal/catalogue/needs_test.go @@ -14,7 +14,7 @@ import ( func needy() Manifest { return Manifest{ Module: "postgres", Version: "1", - OwnSecrets: map[string]string{"superuser": "/var/lib/mesh/postgres/superuser"}, + OwnSecrets: OwnSecrets{"superuser": {Path: "/var/lib/mesh/postgres/superuser"}}, Resources: []map[string]any{ {"id": "store", "type": "container", "name": "mesh-postgres", "image": "postgres@sha256:x"}, }, @@ -74,7 +74,7 @@ func TestANeedIsAnAbsolutePath(t *testing.T) { func TestAModuleMayNeedSeveralThings(t *testing.T) { // A password and a token, say. Telling them apart is the module's business, not the mesh's. m := needy() - m.OwnSecrets["replication"] = "/var/lib/mesh/postgres/replication" + m.OwnSecrets["replication"] = OwnSecret{Path: "/var/lib/mesh/postgres/replication"} got, _ := Resolve(shelf(m), []string{"postgres"}, reachable(), World{}) out, err := got.Declaration(Rendering{Needed: map[string]map[string]string{ "postgres": {"superuser": "b25l", "replication": "dHdv"}, diff --git a/internal/catalogue/own_secret_taken_test.go b/internal/catalogue/own_secret_taken_test.go new file mode 100644 index 0000000..a9f76e5 --- /dev/null +++ b/internal/catalogue/own_secret_taken_test.go @@ -0,0 +1,54 @@ +package catalogue + +import ( + "encoding/json" + "strings" + "testing" +) + +// An own secret says how the module takes it (novox/hq ADR 0114, issue 180): a path alone says +// nothing of it, an object says `at-start` or `applied`, and the bytes the mesh holds are the bytes +// it was given either way. +func TestAnOwnSecretSaysHowItIsTaken(t *testing.T) { + m, err := ParseManifest([]byte(`{"module":"idp","version":"1","own-secrets":{ + "broker":"/var/lib/mesh/idp/broker", + "admin":{"path":"/var/lib/idp/admin.secret","taken":"applied"}, + "session":{"path":"/var/lib/idp/session.secret","taken":"at-start"}}}`)) + if err != nil { + t.Fatal(err) + } + if m.OwnSecrets["broker"] != (OwnSecret{Path: "/var/lib/mesh/idp/broker"}) { + t.Fatalf("a path alone is a path and nothing more: %+v", m.OwnSecrets["broker"]) + } + if m.OwnSecrets["admin"].Taken != TakenApplied || m.OwnSecrets["session"].Taken != TakenAtStart { + t.Fatalf("the word was not kept: %+v", m.OwnSecrets) + } + // Written back the way it was read, so a registered manifest keeps its bytes. + out, err := json.Marshal(m.OwnSecrets) + if err != nil { + t.Fatal(err) + } + var again OwnSecrets + if err := json.Unmarshal(out, &again); err != nil { + t.Fatal(err) + } + if len(again) != 3 || again["admin"].Taken != TakenApplied || again["broker"].Taken != "" { + t.Fatalf("the round trip changed the secrets: %s", out) + } + if !strings.Contains(string(out), `"broker":"/var/lib/mesh/idp/broker"`) { + t.Fatalf("a path alone is written back as a path: %s", out) + } +} + +func TestAnOwnSecretTakenSomeOtherWayIsRefused(t *testing.T) { + _, err := ParseManifest([]byte(`{"module":"idp","version":"1","own-secrets":{ + "admin":{"path":"/var/lib/idp/admin.secret","taken":"sometimes"}}}`)) + if err == nil || !strings.Contains(err.Error(), `taken "sometimes"`) { + t.Fatalf("an unknown word for how a secret is taken was accepted: %v", err) + } + _, err = ParseManifest([]byte(`{"module":"idp","version":"1","own-secrets":{ + "admin":{"path":"/var/lib/idp/admin.secret","rotate":"yes"}}}`)) + if err == nil { + t.Fatal("an unknown field on an own secret was accepted") + } +} diff --git a/internal/catalogue/secrets_in_environment_test.go b/internal/catalogue/secrets_in_environment_test.go index da911b4..39ee997 100644 --- a/internal/catalogue/secrets_in_environment_test.go +++ b/internal/catalogue/secrets_in_environment_test.go @@ -15,7 +15,7 @@ func aModuleWithAnEnvFileSecret(exception string) Manifest { } return Manifest{ Module: "app", Version: "1", - OwnSecrets: map[string]string{"token": "/var/lib/app/token.secret"}, + OwnSecrets: OwnSecrets{"token": {Path: "/var/lib/app/token.secret"}}, Resources: []map[string]any{ {"id": "env", "type": "file", "path": "/var/lib/app/server.env", "mode": "0600", "content": "APP_TOKEN=${secret:token}\n"}, diff --git a/internal/catalogue/secrets_into_files_test.go b/internal/catalogue/secrets_into_files_test.go index 99352a9..1a92a7e 100644 --- a/internal/catalogue/secrets_into_files_test.go +++ b/internal/catalogue/secrets_into_files_test.go @@ -25,7 +25,7 @@ func fileNamed(out []map[string]any, id string) map[string]any { func TestAFileGetsTheSecretItsContentAsksFor(t *testing.T) { r := Resolution{Node: "anchor", Modules: []Manifest{{ Module: "gitea", - OwnSecrets: map[string]string{"admin": "/var/lib/gitea/admin.env"}, + OwnSecrets: OwnSecrets{"admin": {Path: "/var/lib/gitea/admin.env"}}, Resources: []map[string]any{{ "id": "conf", "type": "file", "path": "/etc/gitea/app.ini", "content": "[security]\nSECRET_KEY = ${secret:admin}\n", @@ -130,7 +130,7 @@ func TestTwoConsumersOfOneProvisionEachGetTheirOwnInAPlaceholderFile(t *testing. func TestAFileNamingASecretTheModuleDoesNotHaveIsRefused(t *testing.T) { r := Resolution{Node: "anchor", Modules: []Manifest{{ Module: "gitea", - OwnSecrets: map[string]string{"admin": "/var/lib/gitea/admin.env"}, + OwnSecrets: OwnSecrets{"admin": {Path: "/var/lib/gitea/admin.env"}}, Resources: []map[string]any{{ "id": "conf", "type": "file", "path": "/etc/gitea/app.ini", "content": "SECRET_KEY = ${secret:adnim}\n", @@ -151,7 +151,7 @@ func TestAFileNamingASecretTheModuleDoesNotHaveIsRefused(t *testing.T) { // One module may not read another's credential by guessing its name. func TestAFileCannotNameAnotherModulesSecret(t *testing.T) { r := Resolution{Node: "anchor", Modules: []Manifest{ - {Module: "postgres", OwnSecrets: map[string]string{"superuser": "/var/lib/postgres/su.env"}}, + {Module: "postgres", OwnSecrets: OwnSecrets{"superuser": {Path: "/var/lib/postgres/su.env"}}}, {Module: "gitea", Resources: []map[string]any{{ "id": "conf", "type": "file", "path": "/etc/gitea/app.ini", "content": "PASSWORD=${secret:superuser}\n", @@ -174,7 +174,7 @@ func TestAFileCannotNameAnotherModulesSecret(t *testing.T) { func TestASettingThatCarriesAPlaceholderIsStillFilled(t *testing.T) { r := Resolution{Node: "workstation", Modules: []Manifest{{ Module: "chat", - OwnSecrets: map[string]string{"api-token": "/home/operator/.config/chat/token"}, + OwnSecrets: OwnSecrets{"api-token": {Path: "/home/operator/.config/chat/token"}}, Resources: []map[string]any{{ "id": "settings", "type": "file", "merge": "json", "path": "/home/operator/.config/chat/settings.json", "content": "{}", @@ -207,7 +207,7 @@ func TestANameMeaningTwoThingsIsRefused(t *testing.T) { Node: "anchor", Modules: []Manifest{{ Module: "thing", - OwnSecrets: map[string]string{"store": "/var/lib/thing/own.env"}, + OwnSecrets: OwnSecrets{"store": {Path: "/var/lib/thing/own.env"}}, Secrets: map[string]string{"store": "/var/lib/thing/granted.env"}, }}, Needs: []Needed{{Name: "store", From: "anchor", Sealed: "sealed-granted"}}, @@ -225,7 +225,7 @@ func TestANameMeaningTwoThingsIsRefused(t *testing.T) { func TestAFileWithNoPlaceholderIsLeftAlone(t *testing.T) { r := Resolution{Node: "anchor", Modules: []Manifest{{ Module: "gitea", - OwnSecrets: map[string]string{"admin": "/var/lib/gitea/admin.env"}, + OwnSecrets: OwnSecrets{"admin": {Path: "/var/lib/gitea/admin.env"}}, Resources: []map[string]any{{ "id": "conf", "type": "file", "path": "/etc/gitea/app.ini", "content": "RUN_MODE=prod\n", }}, diff --git a/internal/catalogue/verbs.go b/internal/catalogue/verbs.go index f3fe796..2ad3907 100644 --- a/internal/catalogue/verbs.go +++ b/internal/catalogue/verbs.go @@ -99,6 +99,17 @@ var ControllerVerbs = []Verb{ Input: schema(map[string]string{"node": "the machine's name", "module": "the module's name"}, []string{"node", "module"})}, {Name: "push", Description: "Send a machine everything it should be — or every machine that is behind, when no machine is named.", Input: schema(map[string]string{"node": "the machine's name; every machine behind when absent"}, nil)}, + {Name: "rotate", Description: "Replace a credential. A pair credential, by provision (and a consuming machine, " + + "else every holder): both ends are re-sent together. Or a module's own secret, by machine, module and " + + "name: made anew and the machine sent, so the module starts again on it — only for a secret its " + + "definition says it reads at start; a value given to the mesh, or one the module applies to a backend, is refused with the reason.", + Input: schema(map[string]string{ + "provision": "a pair credential: the provision whose credential to replace", + "consumer": "with provision: only the holder on this machine (optional)", + "node": "an own secret: the machine", + "module": "an own secret: the module", + "secret": "an own secret: its name in the module's definition", + }, nil)}, {Name: "build", Description: "Have the build machine build a repository. Answers at once with the build's id: " + "`builds` with that id follows it line by line, and the module is registered when the outcome comes.", Input: schema(map[string]string{ diff --git a/internal/inventory/forget_test.go b/internal/inventory/forget_test.go index 75e5a36..108296f 100644 --- a/internal/inventory/forget_test.go +++ b/internal/inventory/forget_test.go @@ -32,7 +32,7 @@ func TestRegisteringAModuleAgainKeepsWhatTheMeshHoldsForIt(t *testing.T) { t.Fatal(err) } m := catalogue.Manifest{Module: "step-ca", Version: "1", - Provides: catalogue.Offers("acme-ca"), OwnSecrets: map[string]string{"password": "/run/password"}} + Provides: catalogue.Offers("acme-ca"), OwnSecrets: catalogue.OwnSecrets{"password": {Path: "/run/password"}}} if err := inv.RegisterModule(ctx, m, Source{}); err != nil { t.Fatal(err) } @@ -228,6 +228,6 @@ func TestAModuleStillAssignedRefusesBeforeAnythingAboutWhatItHolds(t *testing.T) // withOwnSecret gives a fixture manifest an own secret, so a delivery to it is one the module // declares (novox/hq 04-ISSUES/078). func withOwnSecret(m catalogue.Manifest, name string) catalogue.Manifest { - m.OwnSecrets = map[string]string{name: "/run/" + name} + m.OwnSecrets = catalogue.OwnSecrets{name: {Path: "/run/" + name}} return m } diff --git a/internal/inventory/operator_test.go b/internal/inventory/operator_test.go index dd328ae..bfa71e7 100644 --- a/internal/inventory/operator_test.go +++ b/internal/inventory/operator_test.go @@ -14,7 +14,7 @@ import ( func TestAnOwnSecretIsSealedToTheOperatorToo(t *testing.T) { inv, ctx := twoNodesWithKeys(t) if err := inv.RegisterModule(ctx, catalogue.Manifest{Module: "postgres", Version: "1", - OwnSecrets: map[string]string{"superuser": "/run/superuser", "replication": "/run/replication"}}, Source{}); err != nil { + OwnSecrets: catalogue.OwnSecrets{"superuser": {Path: "/run/superuser"}, "replication": {Path: "/run/replication"}}}, Source{}); err != nil { t.Fatal(err) } diff --git a/internal/inventory/secrets.go b/internal/inventory/secrets.go index ae96644..56834df 100644 --- a/internal/inventory/secrets.go +++ b/internal/inventory/secrets.go @@ -505,7 +505,7 @@ func declaresOwn(m catalogue.Manifest) string { if len(m.OwnSecrets) == 0 { return "it declares no own secrets" } - return "it declares: " + strings.Join(sortedNames(m.OwnSecrets), ", ") + return "it declares: " + strings.Join(sortedNames(m.OwnSecrets.Paths()), ", ") } func sortedNames(of map[string]string) []string { @@ -523,3 +523,88 @@ func orNone(names []string) string { } return strings.Join(names, ", ") } + +// ErrNotRotatable says why the mesh will not rotate a module's own secret; the words are the caller's +// to print, and the remedy is in them. +type ErrNotRotatable struct{ Why string } + +func (e ErrNotRotatable) Error() string { return e.Why } + +// RotateModuleSecret makes a module's own secret anew, the way the first mint did (novox/hq +// ADR 0114, issue 180). The caller sends the node, so the module is started again on the new value. +// +// **Only a secret the module reads when it starts.** A secret the module's code applies to a +// backend that takes it once would, rotated this way, leave the backend on the old value and the +// module reading the new one — the fault issue 179 was. That form is staged, which the mesh does +// not build yet, and is refused by name. A secret whose manifest says neither is refused with the +// word to write; a secret given to the mesh rather than made by it is refused as 0113 says: the +// mesh will not replace what it cannot read. +func (i *Inventory) RotateModuleSecret(ctx context.Context, node, module, name string) error { + m, err := i.declared(ctx, module) + if err != nil { + return err + } + own, declared := m.OwnSecrets[name] + if !declared { + return fmt.Errorf("%s does not declare %q as an own secret; %s", module, name, declaresOwn(m)) + } + switch own.Taken { + case catalogue.TakenAtStart: + case catalogue.TakenApplied: + return ErrNotRotatable{Why: fmt.Sprintf( + "%s applies %q to a backend that takes it once, so a rotation must be staged beside the "+ + "current value until the module confirms it — the mesh does not do that yet (ADR 0114). "+ + "Changing it is a person's work: change it in %s, then `secret accept %s %s %s`", + module, name, module, node, module, name)} + default: + return ErrNotRotatable{Why: fmt.Sprintf( + "%s does not say how it takes %q, so the mesh will not rotate it: a secret rotated under "+ + "software that never reads it again is worse than one left alone. Its definition says "+ + "\"own-secrets\": {%q: {\"path\": …, \"taken\": \"at-start\"}} when the module reads it as it "+ + "starts, or \"applied\" when its own code applies it", + module, name, name)} + } + record, err := i.NodeByName(ctx, node) + if err != nil { + return err + } + key, err := i.SealingKeyOf(ctx, node) + if err != nil { + return err + } + if key == "" { + return fmt.Errorf("%s has no sealing key, so nothing can be sealed to it", node) + } + var origin string + err = i.store.Pool().QueryRow(ctx, + `select origin from module_secret where node = $1 and module = $2 and name = $3`, + record.ID, module, name).Scan(&origin) + if errors.Is(err, pgx.ErrNoRows) { + return fmt.Errorf("%s on %s holds no %q yet; the first push makes it", module, node, name) + } + if err != nil { + return err + } + if origin == OriginAccepted { + return ErrNotRotatable{Why: fmt.Sprintf( + "%s on %s holds %q as a value given to the mesh, not made by it, and the mesh will not "+ + "replace what it cannot read (ADR 0113). Change it where it lives, then `secret accept "+ + "%s %s %s` with the new value", + module, node, name, node, module, name)} + } + operator, err := i.OperatorKey(ctx) + if err != nil { + return err + } + made, blob, err := secrets.MakeWithOperator(key, key, operator) + if err != nil { + return err + } + forOperator, operatorKey := operatorColumns(operator, blob) + _, err = i.store.Pool().Exec(ctx, + `update module_secret set sealed = $4, node_key = $5, origin = 'made', made_at = now(), + operator_sealed = $6, operator_key = $7 + where node = $1 and module = $2 and name = $3`, + record.ID, module, name, made.ForConsumer, key, forOperator, operatorKey) + return err +} diff --git a/internal/inventory/secrets_test.go b/internal/inventory/secrets_test.go index e5b1610..0eb967f 100644 --- a/internal/inventory/secrets_test.go +++ b/internal/inventory/secrets_test.go @@ -5,6 +5,7 @@ import ( "crypto/ecdh" "crypto/rand" "encoding/base64" + "errors" "github.com/novox/mesh-controller/internal/catalogue" "github.com/novox/mesh-controller/internal/secrets" "strings" @@ -408,7 +409,7 @@ func TestACredentialGoesWhenEitherMachineDoes(t *testing.T) { func TestASecretTheMeshWasGivenIsNotReinventedWhenTheMachineRejoins(t *testing.T) { inv, ctx := twoNodesWithKeys(t) if err := inv.RegisterModule(ctx, catalogue.Manifest{Module: "builder", Version: "1", - OwnSecrets: map[string]string{"broker": "/run/broker"}}, Source{}); err != nil { + OwnSecrets: catalogue.OwnSecrets{"broker": {Path: "/run/broker"}}}, Source{}); err != nil { t.Fatal(err) } const url = "amqps://builder:the-password-the-broker-was-told@broker/" @@ -440,7 +441,7 @@ func TestASecretTheMeshWasGivenIsNotReinventedWhenTheMachineRejoins(t *testing.T func TestASecretTheMeshWasGivenSurvivesAnOrdinaryPush(t *testing.T) { inv, ctx := twoNodesWithKeys(t) if err := inv.RegisterModule(ctx, catalogue.Manifest{Module: "builder", Version: "1", - OwnSecrets: map[string]string{"broker": "/run/broker"}}, Source{}); err != nil { + OwnSecrets: catalogue.OwnSecrets{"broker": {Path: "/run/broker"}}}, Source{}); err != nil { t.Fatal(err) } if err := inv.AcceptSecretForModule(ctx, "consumer", "builder", "broker", @@ -725,7 +726,7 @@ func TestTheOperatorRecoversEachLocalNameApart(t *testing.T) { func TestADeliveredSecretIsRefusedUnderANameTheModuleDoesNotDeclare(t *testing.T) { inv, ctx := twoNodesWithKeys(t) if err := inv.RegisterModule(ctx, catalogue.Manifest{Module: "step-ca", Version: "1", - OwnSecrets: map[string]string{"password": "/run/password"}}, Source{}); err != nil { + OwnSecrets: catalogue.OwnSecrets{"password": {Path: "/run/password"}}}, Source{}); err != nil { t.Fatal(err) } err := inv.AcceptSecretForModule(ctx, "consumer", "step-ca", "root-key", "not-a-key") @@ -785,3 +786,60 @@ func TestADeliveredPairCredentialIsRefusedForARequirementTheModuleDoesNotHave(t t.Errorf("a delivery under a kept local was refused: %v", err) } } + +// A module's own secret rotates when its definition says the module reads it at start: made anew, +// sealed to the machine and the operator, origin made. Refused with the reason when the definition +// says nothing, says the module applies it, or when the value was given to the mesh (novox/hq +// ADR 0114, issue 180). +func TestAnOwnSecretRotatesOnlyWhenTheModuleReadsItAtStart(t *testing.T) { + inv, ctx := twoNodesWithKeys(t) + m := catalogue.Manifest{Module: "idp", Version: "1", OwnSecrets: catalogue.OwnSecrets{ + "session": {Path: "/var/lib/idp/session.secret", Taken: catalogue.TakenAtStart}, + "admin": {Path: "/var/lib/idp/admin.secret", Taken: catalogue.TakenApplied}, + "broker": {Path: "/var/lib/mesh/idp/broker"}, + }} + if err := inv.RegisterModule(ctx, m, Source{}); err != nil { + t.Fatal(err) + } + before, err := inv.SecretForModule(ctx, "consumer", "idp", "session") + if err != nil { + t.Fatal(err) + } + if err := inv.RotateModuleSecret(ctx, "consumer", "idp", "session"); err != nil { + t.Fatal(err) + } + after, err := inv.SecretForModule(ctx, "consumer", "idp", "session") + if err != nil { + t.Fatal(err) + } + if after == before { + t.Fatal("rotating made no new value") + } + + if _, err := inv.SecretForModule(ctx, "consumer", "idp", "admin"); err != nil { + t.Fatal(err) + } + var refused ErrNotRotatable + err = inv.RotateModuleSecret(ctx, "consumer", "idp", "admin") + if !errors.As(err, &refused) || !strings.Contains(err.Error(), "staged") { + t.Fatalf("an applied secret must be refused as not yet stageable: %v", err) + } + if _, err := inv.SecretForModule(ctx, "consumer", "idp", "broker"); err != nil { + t.Fatal(err) + } + err = inv.RotateModuleSecret(ctx, "consumer", "idp", "broker") + if !errors.As(err, &refused) || !strings.Contains(err.Error(), "does not say how it takes") { + t.Fatalf("a secret that says nothing of how it is taken must be refused: %v", err) + } + + if err := inv.AcceptSecretForModule(ctx, "consumer", "idp", "session", "the-real-one"); err != nil { + t.Fatal(err) + } + err = inv.RotateModuleSecret(ctx, "consumer", "idp", "session") + if !errors.As(err, &refused) || !strings.Contains(err.Error(), "given to the mesh") { + t.Fatalf("an accepted value must be refused: %v", err) + } + if err := inv.RotateModuleSecret(ctx, "consumer", "idp", "nothing"); err == nil || !strings.Contains(err.Error(), "does not declare") { + t.Fatalf("an undeclared secret: %v", err) + } +} -- 2.54.0