diff --git a/internal/catalogue/declaration.go b/internal/catalogue/declaration.go index 6926dbea..73ca1288 100644 --- a/internal/catalogue/declaration.go +++ b/internal/catalogue/declaration.go @@ -680,7 +680,13 @@ func (r Resolution) compose(with Rendering, owner map[string]string, if !ok || family == "" || with.Needed[m.Module][name] == "" { continue } - first = append(first, ownedBy(m.SecretsOwner, map[string]any{ + // A per-secret owner (novox/hq ADR 0436 §1) stands in for the module's secrets-owner, when + // the manifest named one for this secret (or the family it belongs to). + owner := m.SecretsOwner + if own.Owner != "" { + owner = own.Owner + } + first = append(first, ownedBy(owner, map[string]any{ "id": NeedID(name), "type": "file", "path": own.Path, "sealed": with.Needed[m.Module][name], })) } @@ -694,6 +700,9 @@ func (r Resolution) compose(with Rendering, owner map[string]string, if m.Module == RuntimeModule && r.Account != "" { owner = r.Account } + if o := m.OwnSecrets[name].Owner; o != "" { + owner = o + } first = append(first, ownedBy(owner, map[string]any{ "id": NeedID(name), "type": "file", "path": m.OwnSecrets[name].Path, HandedSecretField: name, })) @@ -709,6 +718,9 @@ func (r Resolution) compose(with Rendering, owner map[string]string, if m.Module == RuntimeModule && r.Account != "" { owner = r.Account } + if o := m.OwnSecrets[name].Owner; o != "" { + owner = o + } first = append(first, ownedBy(owner, map[string]any{ "id": NeedID(name), "type": "file", "path": m.OwnSecrets[name].Path, VaultSealedField: box, })) @@ -742,6 +754,9 @@ func (r Resolution) compose(with Rendering, owner map[string]string, if m.Module == RuntimeModule && r.Account != "" { owner = r.Account } + if o := m.OwnSecrets[name].Owner; o != "" { + owner = o + } first = append(first, ownedBy(owner, map[string]any{ "id": NeedID(name), "type": "file", "path": m.OwnSecrets[name].Path, "sealed": sealed, })) diff --git a/internal/catalogue/dir_into.go b/internal/catalogue/dir_into.go index 585358e3..dd3be2f5 100644 --- a/internal/catalogue/dir_into.go +++ b/internal/catalogue/dir_into.go @@ -168,7 +168,7 @@ func placedManifest(m Manifest, with Rendering) (Manifest, error) { if err != nil { return m, err } - own[name] = OwnSecret{Path: filled, Taken: s.Taken, IssuedBy: s.IssuedBy} + own[name] = OwnSecret{Path: filled, Taken: s.Taken, IssuedBy: s.IssuedBy, Prompt: s.Prompt, Owner: s.Owner} } m.OwnSecrets = own } diff --git a/internal/catalogue/manifest.go b/internal/catalogue/manifest.go index 3acc5cbd..3d983484 100644 --- a/internal/catalogue/manifest.go +++ b/internal/catalogue/manifest.go @@ -2348,6 +2348,7 @@ func parseManifest(raw []byte, predefined PredefinedList) (Manifest, error) { m.Module, name, own.IssuedBy, IssuedOutside)) } problems = append(problems, ownSecretPromptProblems(m, name, own.Prompt, IsFamily(name))...) + problems = append(problems, ownSecretOwnerProblems(m, name, own.Owner)...) } localOf := map[string]string{} for _, to := range m.SecretRequirements() { @@ -2930,6 +2931,11 @@ type OwnSecret struct { // family's member suffix, filled where the secret is asked for. Optional; reviewed in the // module's pull request like the rest of the manifest. Prompt string + // Owner is the account this secret's files belong to on the machine, in place of the module's + // secrets-owner (novox/hq ADR 0436 §1): an account the module makes (a `user` resource of that + // name), never root and never the operator account. Empty means the module's secrets-owner, or + // root where that is absent too. + Owner string } // MeshMayMake says the mesh may put a value it makes in place of the one the secret holds: the @@ -2984,6 +2990,34 @@ func ownSecretPromptProblems(m Manifest, name, prompt string, family bool) []str return problems } +// ownSecretOwnerProblems is what is wrong with an own secret's declared owner (novox/hq ADR 0436 §1): naming +// an account the module makes — a `user` resource of that name — never root, which a secret file owned by it +// would make readable by anything, and never the operator account's placeholder (`${machine:account}`), which +// the mesh fills per machine and a manifest cannot say safely (novox/hq ADR 0436, settled by the lead). +func ownSecretOwnerProblems(m Manifest, name, owner string) []string { + if owner == "" { + return nil + } + if owner == "root" { + return []string{fmt.Sprintf( + "%s names %q as the owner of its secret %q; an own secret's owner is an account the module "+ + "makes, never root (novox/hq ADR 0436 §1)", m.Module, owner, name)} + } + if owner == "${machine:account}" { + return []string{fmt.Sprintf( + "%s names %q as the owner of its secret %q; an own secret's owner is an account the module "+ + "makes, never the operator account (novox/hq ADR 0436 §1)", m.Module, owner, name)} + } + for _, r := range m.Resources { + if fmt.Sprint(r["type"]) == "user" && fmt.Sprint(r["name"]) == owner { + return nil + } + } + return []string{fmt.Sprintf( + "%s names %q as the owner of its secret %q, which it does not make: a user resource named %q "+ + "(novox/hq ADR 0436 §1)", m.Module, owner, name, owner)} +} + // 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. @@ -3006,13 +3040,14 @@ func (o *OwnSecrets) UnmarshalJSON(raw []byte) error { Taken string `json:"taken,omitempty"` IssuedBy string `json:"issued-by,omitempty"` Prompt string `json:"prompt,omitempty"` + Owner string `json:"owner,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\", \"issued-by\", \"prompt\"}: %w", name, typedUnknown(err)) + return fmt.Errorf("own-secrets.%s: a path, or {\"path\", \"taken\", \"issued-by\", \"prompt\", \"owner\"}: %w", name, typedUnknown(err)) } - out[name] = OwnSecret{Path: long.Path, Taken: long.Taken, IssuedBy: long.IssuedBy, Prompt: long.Prompt} + out[name] = OwnSecret{Path: long.Path, Taken: long.Taken, IssuedBy: long.IssuedBy, Prompt: long.Prompt, Owner: long.Owner} } *o = out return nil @@ -3021,7 +3056,7 @@ func (o *OwnSecrets) UnmarshalJSON(raw []byte) error { func (o OwnSecrets) MarshalJSON() ([]byte, error) { entries := make(map[string]any, len(o)) for name, s := range o { - if s.Taken == "" && s.IssuedBy == "" && s.Prompt == "" { + if s.Taken == "" && s.IssuedBy == "" && s.Prompt == "" && s.Owner == "" { entries[name] = s.Path continue } @@ -3035,6 +3070,9 @@ func (o OwnSecrets) MarshalJSON() ([]byte, error) { if s.Prompt != "" { long["prompt"] = s.Prompt } + if s.Owner != "" { + long["owner"] = s.Owner + } entries[name] = long } return json.Marshal(entries) @@ -3074,7 +3112,7 @@ func (o OwnSecrets) Lookup(name string) (s OwnSecret, family string, ok bool) { } if family == "" || len(declared) > len(family) { s, family, ok = OwnSecret{Path: strings.Replace(f.Path, "*", rest, 1), Taken: f.Taken, IssuedBy: f.IssuedBy, - Prompt: f.Prompt}, declared, true + Prompt: f.Prompt, Owner: f.Owner}, declared, true } } return s, family, ok diff --git a/internal/catalogue/own_secret_owner_test.go b/internal/catalogue/own_secret_owner_test.go new file mode 100644 index 00000000..c154e476 --- /dev/null +++ b/internal/catalogue/own_secret_owner_test.go @@ -0,0 +1,114 @@ +package catalogue + +import ( + "strings" + "testing" +) + +// An own secret (or a family) may declare `owner` (novox/hq ADR 0436 §1): the account the files holding it +// belong to on the machine, when that account is not the module's secrets-owner default. The fixture is +// mesh-mail's future manifest shape (ADR 0436 §7): a process account `mesh-mail-send` the module makes, and its +// own-secret family `mail-credential-*` owned by it. + +// mailSenderManifest is the fixture every mail-sender issue builds on: the real parts of mesh-mail's manifest +// taken from mesh-catalog/modules/mesh-mail/module.json, extended as ADR 0436 §7-§8 describe, with `own` naming +// the account that owns the family's files. +func mailSenderManifest(t *testing.T, own string) Manifest { + t.Helper() + ownSecrets := `"mail-credential-*":{"path":"/var/lib/mesh-mail/mail-credential-*.secret","issued-by":"outside"` + if own != "" { + ownSecrets += `,"owner":"` + own + `"` + } + ownSecrets += `}` + m, err := ParseManifest([]byte(`{"module":"mesh-mail","version":"1","secrets-owner":"mesh-mail","own-secrets":{` + + ownSecrets + `},"resources":[{"id":"account","type":"user","name":"mesh-mail-send"}]}`)) + if err != nil { + t.Fatalf("a manifest with own=%q was refused: %v", own, err) + } + return m +} + +// 1170/AC1: a family owned by the module's own account lands at 0600, owned by that account — and a module's +// other own secrets (no per-secret owner declared) keep secrets-owner, unchanged. +func TestAnOwnSecretsOwnerIsGivenToItsFiles(t *testing.T) { + m := mailSenderManifest(t, "mesh-mail-send") + m.OwnSecrets["smtp-relay-key"] = OwnSecret{Path: "/var/lib/mesh-mail/smtp-relay-key", Taken: TakenAtStart} + r := Resolution{Node: "novox", Modules: []Manifest{m}} + out, err := r.Declaration(Rendering{ + Needed: map[string]map[string]string{"mesh-mail": {"mail-credential-smtp": "sealed-credential", "smtp-relay-key": "sealed-relay"}}, + }) + if err != nil { + t.Fatalf("composing the declaration: %v", err) + } + var family, plain map[string]any + for _, res := range out { + if res["type"] != "file" { + continue + } + switch res["path"] { + case "/var/lib/mesh-mail/mail-credential-smtp.secret": + family = res + case "/var/lib/mesh-mail/smtp-relay-key": + plain = res + } + } + if family == nil { + t.Fatalf("the family member was not composed at all: %v", out) + } + if got := family["owner"]; got != "mesh-mail-send" { + t.Fatalf("the family member's owner: %v; want mesh-mail-send", got) + } + if got := family["mode"]; got != "" && got != nil { + // mode is not set by ownedBy; the host applies 0600 to every secret file regardless (ADR 0086). Still, + // a mode this test sees must not have been changed by the owner clause. + t.Logf("family mode: %v", got) + } + if plain == nil { + t.Fatalf("the module's other own secret was not composed at all: %v", out) + } + if got := plain["owner"]; got != "mesh-mail" { + t.Fatalf("a secret with no per-secret owner kept %v; want the module's secrets-owner %q", got, "mesh-mail") + } +} + +// 1170/AC2: an owner naming an account the module does not make, root, or the operator account's placeholder is +// refused, naming the secret and the account. +func TestAnOwnSecretsOwnerIsRefusedWhenItIsNotAnAccountTheModuleMakes(t *testing.T) { + for name, c := range map[string]struct{ owner, says string }{ + "root": {"root", "root"}, + "the operator account": {"${machine:account}", "operator account"}, + "an account not declared": {"not-made", "not-made"}, + } { + _, err := ParseManifest([]byte(`{"module":"mesh-mail","version":"1","own-secrets":{` + + `"mail-credential-*":{"path":"/var/lib/mesh-mail/mail-credential-*.secret","issued-by":"outside","owner":"` + + c.owner + `"}},"resources":[{"id":"account","type":"user","name":"mesh-mail-send"}]}`)) + if err == nil || !strings.Contains(err.Error(), "mail-credential-*") || !strings.Contains(err.Error(), c.says) { + t.Errorf("%s: %v; want a refusal naming the secret and %q", name, err, c.says) + } + } +} + +// 1170/AC3: a manifest with no owner on any own secret composes exactly as it did before this clause existed: +// every secret file belongs to the module's secrets-owner (or nothing, when that is absent too). +func TestNoOwnerOnAnySecretComposesAsMainDoes(t *testing.T) { + m := mailSenderManifest(t, "") + r := Resolution{Node: "novox", Modules: []Manifest{m}} + out, err := r.Declaration(Rendering{ + Needed: map[string]map[string]string{"mesh-mail": {"mail-credential-smtp": "sealed-credential"}}, + }) + if err != nil { + t.Fatalf("composing the declaration: %v", err) + } + var found bool + for _, res := range out { + if res["type"] == "file" && res["path"] == "/var/lib/mesh-mail/mail-credential-smtp.secret" { + found = true + if got := res["owner"]; got != "mesh-mail" { + t.Fatalf("with no per-secret owner declared, the file's owner: %v; want secrets-owner %q", got, "mesh-mail") + } + } + } + if !found { + t.Fatalf("the family member was not composed at all: %v", out) + } +}