From e7aaa8420eacaa9b5c4f32504339dea895c81bbd Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 26 Oct 2026 09:25:35 +0100 Subject: [PATCH] Vault: name a hash record's secret in a field called of, so the runtime accepts it The runtime refuses a state field whose name ends in secret, so no bus-account hash could be written on novox. The test state now applies the same check. Issue: 1945 --- modules/mesh-vault/cmd/mesh-vault/.gitignore | 1 + modules/mesh-vault/cmd/mesh-vault/accounts.go | 15 +++-- .../cmd/mesh-vault/credential_field_test.go | 67 +++++++++++++++++++ .../cmd/mesh-vault/issued_account_test.go | 7 ++ modules/mesh-vault/cmd/mesh-vault/tools.go | 4 +- 5 files changed, 85 insertions(+), 9 deletions(-) create mode 100644 modules/mesh-vault/cmd/mesh-vault/.gitignore create mode 100644 modules/mesh-vault/cmd/mesh-vault/credential_field_test.go diff --git a/modules/mesh-vault/cmd/mesh-vault/.gitignore b/modules/mesh-vault/cmd/mesh-vault/.gitignore new file mode 100644 index 00000000..512a0107 --- /dev/null +++ b/modules/mesh-vault/cmd/mesh-vault/.gitignore @@ -0,0 +1 @@ +/mesh-vault diff --git a/modules/mesh-vault/cmd/mesh-vault/accounts.go b/modules/mesh-vault/cmd/mesh-vault/accounts.go index 1accbc27..32566f5b 100644 --- a/modules/mesh-vault/cmd/mesh-vault/accounts.go +++ b/modules/mesh-vault/cmd/mesh-vault/accounts.go @@ -42,8 +42,9 @@ type AccountHash struct { User string `json:"user"` // Hash is the bcrypt hash of the account's password, at the composer's cost. Hash string `json:"hash"` - // Secret, Generation and Fingerprint are the vault's own secret the hash is of: absent for an adopted one. - Secret string `json:"secret,omitempty"` + // Of, Generation and Fingerprint name the vault's own secret the hash is of: absent for an adopted one. The field + // is not called "secret": the runtime refuses a state field whose name says it is a credential (ADR 0201). + Of string `json:"of,omitempty"` Generation int `json:"generation,omitempty"` Fingerprint string `json:"fingerprint,omitempty"` At time.Time `json:"at"` @@ -149,7 +150,7 @@ func accountOf(secret, value string) (user, password string, err error) { // Hashed is what one hashing did: the key written or found, never the hash. type Hashed struct { - Secret string `json:"secret"` + Of string `json:"of"` User string `json:"user"` Generation int `json:"generation"` Fingerprint string `json:"fingerprint"` @@ -178,13 +179,13 @@ func (v *Vault) HashAccount(secret string, force bool) (Hashed, error) { return Hashed{}, err } fp := Fingerprint(value) - h := Hashed{Secret: secret, User: user, Generation: rec.Generation, Fingerprint: fp} + h := Hashed{Of: secret, User: user, Generation: rec.Generation, Fingerprint: fp} was, err := v.Accounts.Get(user) if err != nil { v.hashes.owe(secret, "its key could not be read: "+err.Error()) return Hashed{}, fmt.Errorf("%s %s could not be read: %w", BusAccountsState, user, err) } - if !force && was != nil && !was.Adopted && was.Secret == secret && was.Generation == rec.Generation && was.Fingerprint == fp { + if !force && was != nil && !was.Adopted && was.Of == secret && was.Generation == rec.Generation && was.Fingerprint == fp { v.hashes.paid(secret) h.At = was.At return h, nil @@ -194,7 +195,7 @@ func (v *Vault) HashAccount(secret string, force bool) (Hashed, error) { return Hashed{}, err } h.At = v.Now().UTC() - if err := v.Accounts.Put(AccountHash{User: user, Hash: string(hash), Secret: secret, Generation: rec.Generation, + if err := v.Accounts.Put(AccountHash{User: user, Hash: string(hash), Of: secret, Generation: rec.Generation, Fingerprint: fp, At: h.At}); err != nil { v.hashes.owe(secret, "its key could not be written: "+err.Error()) return Hashed{}, fmt.Errorf("the hash of %s (generation %d) could not be written to %s %s, tried again at the "+ @@ -203,7 +204,7 @@ func (v *Vault) HashAccount(secret string, force bool) (Hashed, error) { v.hashes.paid(secret) h.Written = true if v.Emit != nil { - if err := v.Emit(AccountHashedEvent, map[string]any{"user": user, "secret": secret, "generation": rec.Generation, + if err := v.Emit(AccountHashedEvent, map[string]any{"user": user, "of": secret, "generation": rec.Generation, "fingerprint": fp, "at": h.At.Format(time.RFC3339Nano)}); err != nil { v.log("the hash of %s is written, but %s could not be said: %v", secret, AccountHashedEvent, err) } diff --git a/modules/mesh-vault/cmd/mesh-vault/credential_field_test.go b/modules/mesh-vault/cmd/mesh-vault/credential_field_test.go new file mode 100644 index 00000000..e1668a23 --- /dev/null +++ b/modules/mesh-vault/cmd/mesh-vault/credential_field_test.go @@ -0,0 +1,67 @@ +package main + +import ( + "encoding/json" + "strings" + "testing" +) + +// credentialEndings and credentialField mirror the runtime's refusal of a state or log value with a field whose name says +// it is a credential (mesh-tools node-tools/internal/bus/state.go, credentialField, ADR 0201): the runtime's check is +// unexported and in another repository, so this is a copy and must follow it. A fake state that skips it let a +// field named "secret" pass every test and be refused on novox (issue 1945). +var credentialEndings = []string{"password", "passwd", "secret", "token", "credential", "credentials", + "authorization", "apikey", "privatekey", "accesskey", "cookie"} + +func credentialField(value []byte) string { + var v any + if json.Unmarshal(value, &v) != nil { + return "" + } + var walk func(any) string + walk = func(v any) string { + switch t := v.(type) { + case map[string]any: + for k, x := range t { + norm := strings.NewReplacer("-", "", "_", "", " ", "").Replace(strings.ToLower(k)) + for _, end := range credentialEndings { + if strings.HasSuffix(norm, end) { + return k + } + } + if f := walk(x); f != "" { + return f + } + } + case []any: + for _, x := range t { + if f := walk(x); f != "" { + return f + } + } + } + return "" + } + return walk(v) +} + +// The vault's bus-accounts records pass the runtime's credential-field check; a record with a "secret" field does not. +func TestStateRecordsPassTheRuntimesCredentialFieldCheck(t *testing.T) { + for _, c := range []struct { + name string + value any + field string + }{ + {"a hash record of the vault's own secret", AccountHash{User: "ace.mesh-build-go", Hash: "$2a$10$h", Of: "own/ace/mesh-build-go/broker", Generation: 1, Fingerprint: "ab12"}, ""}, + {"an adopted hash record", AccountHash{User: "ace.x", Hash: "$2a$10$h", Adopted: true}, ""}, + {"a record with a secret field is refused", map[string]any{"user": "u", "secret": "own/x"}, "secret"}, + } { + raw, err := json.Marshal(c.value) + if err != nil { + t.Fatal(err) + } + if got := credentialField(raw); got != c.field { + t.Errorf("%s: credential field %q, want %q (%s)", c.name, got, c.field, raw) + } + } +} diff --git a/modules/mesh-vault/cmd/mesh-vault/issued_account_test.go b/modules/mesh-vault/cmd/mesh-vault/issued_account_test.go index ef6a1de5..fb2f5486 100644 --- a/modules/mesh-vault/cmd/mesh-vault/issued_account_test.go +++ b/modules/mesh-vault/cmd/mesh-vault/issued_account_test.go @@ -1,7 +1,9 @@ package main import ( + "encoding/json" "errors" + "fmt" "strings" "sync" "testing" @@ -36,6 +38,11 @@ func (f *fakeAccounts) Put(a AccountHash) error { if f.keys == nil { f.keys = map[string]AccountHash{} } + // What the runtime does to every state value (credential_field_test.go): the vault's own record is checked as written. + raw, _ := json.Marshal(a) + if field := credentialField(raw); field != "" { + return fmt.Errorf("%s carries a field %q, which names a credential: no secret is kept in state", a.User, field) + } f.keys[a.User] = a f.puts++ return nil diff --git a/modules/mesh-vault/cmd/mesh-vault/tools.go b/modules/mesh-vault/cmd/mesh-vault/tools.go index 0d1732b0..bbf416aa 100644 --- a/modules/mesh-vault/cmd/mesh-vault/tools.go +++ b/modules/mesh-vault/cmd/mesh-vault/tools.go @@ -761,8 +761,8 @@ func hashedRow(v *Vault, r Record, fp string, row map[string]any) { row["hashed"], row["hashWhy"] = false, "no key" case h.Adopted: row["hashed"], row["hashedAt"], row["hashWhy"] = false, h.At, "the key is adopted from the loaded list, not of this value" - case h.Secret != r.Name || h.Generation != r.Generation || h.Fingerprint != fp: - row["hashed"], row["hashedAt"], row["hashWhy"] = false, h.At, fmt.Sprintf("the key is of %s generation %d", h.Secret, h.Generation) + case h.Of != r.Name || h.Generation != r.Generation || h.Fingerprint != fp: + row["hashed"], row["hashedAt"], row["hashWhy"] = false, h.At, fmt.Sprintf("the key is of %s generation %d", h.Of, h.Generation) default: row["hashed"], row["hashedAt"] = true, h.At } -- 2.54.0