Keep a bus account in the vault only as an account: a random value for a broker name is not issued (issue 774)
own/novox/nats/broker-nats-tools (moved in today) and mesh-controller's broker-address and broker-management (moved in from the controller's store) are random values, not the account JSON a bus user is hashed from, so the hash step says "not JSON" and the grants part leaves those users off the bus. The controller made them (fixed there, c421629f); the vault kept them, since nothing it keeps judged a covered name's value. Quick fix: refuse a plain value in move-in. Thorough fix: keepLocked, which every way into the store passes, refuses for any covered name (broker, provisioner-broker, broker-<binary>, which catalogue.ToolsBusSecret names) a value that is no account, saying "not issued". Chose thorough: one rule where every value is kept.
This commit is contained in:
@@ -50,7 +50,7 @@ func pemOf(t *testing.T, raw string) string {
|
|||||||
return p
|
return p
|
||||||
}
|
}
|
||||||
|
|
||||||
const brokerValue = "pv-anchors-provisioner-broker-password-2025"
|
const brokerValue = `{"user":"provisioner.anchor","password":"pv-anchors-provisioner-broker-password-2025"}`
|
||||||
|
|
||||||
func clientsBroker(sealingKey string) MoveOf {
|
func clientsBroker(sealingKey string) MoveOf {
|
||||||
return MoveOf{Node: "anchor", Module: "mesh-provisioner-client", Name: "provisioner-broker", Origin: "made",
|
return MoveOf{Node: "anchor", Module: "mesh-provisioner-client", Name: "provisioner-broker", Origin: "made",
|
||||||
|
|||||||
@@ -41,7 +41,11 @@ func TestANodeEngineStatesItsOwnKeyAndAChangeRaisesItsCoreSecrets(t *testing.T)
|
|||||||
{Node: "ace", Module: "postgres", Name: "superuser-password", Origin: "made", Generation: "g-1"},
|
{Node: "ace", Module: "postgres", Name: "superuser-password", Origin: "made", Generation: "g-1"},
|
||||||
{Node: "g14", Module: "nats", Name: "broker", Origin: "made", Generation: "g-1"},
|
{Node: "g14", Module: "nats", Name: "broker", Origin: "made", Generation: "g-1"},
|
||||||
} {
|
} {
|
||||||
if _, err := moveIn(t, v, "node."+m.Node, m, movedValue); err != nil {
|
value := movedValue
|
||||||
|
if m.Name != "superuser-password" {
|
||||||
|
value = brokerValue // a bus account is kept only as one
|
||||||
|
}
|
||||||
|
if _, err := moveIn(t, v, "node."+m.Node, m, value); err != nil {
|
||||||
t.Fatal(err)
|
t.Fatal(err)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -157,15 +157,15 @@ func TestSecretOwnListsFingerprintsOfOwnSecretsOnly(t *testing.T) {
|
|||||||
v := newVault(t, AllowList{})
|
v := newVault(t, AllowList{})
|
||||||
acc := &fakeAccounts{}
|
acc := &fakeAccounts{}
|
||||||
v.Accounts = acc
|
v.Accounts = acc
|
||||||
for name, value := range map[string]string{"own/ace/mesh-build-go/broker": "pw-one", "own/ace/mesh-build-go/token": "tok-secret",
|
for name, value := range map[string]string{"own/ace/mesh-build-go/broker": pwOne, "own/ace/mesh-build-go/token": "tok-secret",
|
||||||
"own/g14/dunst/broker": `{"user":"g14.dunst","password":"pw-two"}`, "pair/postgres/gitea": "pair-value"} {
|
"own/g14/dunst/broker": `{"user":"g14.dunst","password":"pw-two"}`, "pair/postgres/gitea": "pair-value"} {
|
||||||
if _, err := v.Put(name, value, "the test"); err != nil {
|
if _, err := v.Put(name, value, "the test"); err != nil {
|
||||||
t.Fatal(err)
|
t.Fatal(err)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
if _, err := v.Put("own/ace/mesh-build-go/broker", "pw-one-b", "the test"); err != nil {
|
// A value held from before the vault kept bus accounts only as accounts (generation 2, plain), as a store may still
|
||||||
t.Fatal(err)
|
// hold one: written past keep, which refuses it now.
|
||||||
}
|
heldPlain(t, v.Vault, "own/ace/mesh-build-go/broker", "pw-one-b")
|
||||||
own := toolNamed(t, definitions(&Env{Vault: v.Vault}), "secret_own")
|
own := toolNamed(t, definitions(&Env{Vault: v.Vault}), "secret_own")
|
||||||
for _, c := range []struct {
|
for _, c := range []struct {
|
||||||
args map[string]any
|
args map[string]any
|
||||||
@@ -214,7 +214,7 @@ func TestSecretOwnListsFingerprintsOfOwnSecretsOnly(t *testing.T) {
|
|||||||
}
|
}
|
||||||
hash := toolNamed(t, definitions(&Env{Vault: v.Vault}), "account_hash")
|
hash := toolNamed(t, definitions(&Env{Vault: v.Vault}), "account_hash")
|
||||||
got, err := hash.Run(map[string]any{"user": "g14.dunst"})
|
got, err := hash.Run(map[string]any{"user": "g14.dunst"})
|
||||||
if h, _ := got.(Hashed); err != nil || !h.Written || h.Generation != 1 || acc.puts != 2 {
|
if h, _ := got.(Hashed); err != nil || !h.Written || h.Generation != 1 || acc.puts != 3 { // mesh-build-go's first value, an account, was hashed too
|
||||||
t.Errorf("account_hash = %+v, %v (%d writes)", got, err, acc.puts)
|
t.Errorf("account_hash = %+v, %v (%d writes)", got, err, acc.puts)
|
||||||
}
|
}
|
||||||
if _, err := hash.Run(map[string]any{"user": "ace.mesh-build-go"}); err == nil {
|
if _, err := hash.Run(map[string]any{"user": "ace.mesh-build-go"}); err == nil {
|
||||||
@@ -330,3 +330,24 @@ func TestSecretOwnListsFingerprintsOfOwnSecretsOnly(t *testing.T) {
|
|||||||
t.Errorf("secret_own narrowed to a module lists adopted accounts: %v", narrowed)
|
t.Errorf("secret_own narrowed to a module lists adopted accounts: %v", narrowed)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// The bus account values of TestSecretOwnListsFingerprintsOfOwnSecretsOnly: a bus account is kept only as one.
|
||||||
|
const pwOne = `{"password":"pw-one"}`
|
||||||
|
|
||||||
|
// heldPlain writes value as the next generation of name straight into the store, past keep: a value kept before keep
|
||||||
|
// refused a bus account that is no account.
|
||||||
|
func heldPlain(t *testing.T, v *Vault, name, value string) {
|
||||||
|
t.Helper()
|
||||||
|
rec, held, err := v.Store.Read(name)
|
||||||
|
if err != nil || !held {
|
||||||
|
t.Fatalf("%s is not held: %v", name, err)
|
||||||
|
}
|
||||||
|
if rec.AtRest, err = seal.Seal(value, v.Own.PublicKey); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
rec.Generation++
|
||||||
|
rec.ChangedAt = v.Now().UTC()
|
||||||
|
if err := v.Store.Write(rec); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -147,6 +147,13 @@ func (v *Vault) keepWith(name, value, by, via, origin string, opt keepOptions) (
|
|||||||
return c, err
|
return c, err
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// BusAccountValue says value is a bus account as secret names it: the controller's or the vault's JSON, with its user
|
||||||
|
// (or one its name gives) and a password (accountOf).
|
||||||
|
func BusAccountValue(secret, value string) bool {
|
||||||
|
_, _, err := accountOf(secret, value)
|
||||||
|
return err == nil
|
||||||
|
}
|
||||||
|
|
||||||
// openAtRest opens a record's value with the vault's own key.
|
// openAtRest opens a record's value with the vault's own key.
|
||||||
func openAtRest(r Record, v *Vault) (string, error) { return seal.Open(r.AtRest, v.Own.PrivateKey) }
|
func openAtRest(r Record, v *Vault) (string, error) { return seal.Open(r.AtRest, v.Own.PrivateKey) }
|
||||||
|
|
||||||
@@ -160,6 +167,14 @@ func (v *Vault) keepLocked(name, value, by, via, origin string, opt keepOptions)
|
|||||||
if privateKeyIn(value) {
|
if privateKeyIn(value) {
|
||||||
return Changed{}, fmt.Errorf("%s: that is a private key, and a private key stays where it was made and never reaches the vault (ADR 0113 §2)", name)
|
return Changed{}, fmt.Errorf("%s: that is a private key, and a private key stays where it was made and never reaches the vault (ADR 0113 §2)", name)
|
||||||
}
|
}
|
||||||
|
// A bus account is kept only as one (the sweep of 2026-10-30, issue 774): the controller had moved in random values
|
||||||
|
// for broker-nats-tools, broker-address and broker-management, which no hash could be taken of, so the grants part
|
||||||
|
// left those users out of the bus. Such a name is never kept with a value that is no account: not issued.
|
||||||
|
if Covered(name) && !BusAccountValue(name, value) {
|
||||||
|
return Changed{}, fmt.Errorf("%s is not issued: it names a bus account, and the value handed in is no account "+
|
||||||
|
"(the JSON the controller hands or the vault mints, with a user and a password); the vault keeps a bus "+
|
||||||
|
"account only as one, so nothing was kept", name)
|
||||||
|
}
|
||||||
if by == "" {
|
if by == "" {
|
||||||
by = "the vault"
|
by = "the vault"
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -905,3 +905,24 @@ func TestTheRecordOfHandOutsIsBounded(t *testing.T) {
|
|||||||
t.Errorf("the newest are not read back: %+v", h)
|
t.Errorf("the newest are not read back: %+v", h)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// A bus account is kept only as one (the sweep of 2026-10-30, issue 774: random values for broker-nats-tools,
|
||||||
|
// broker-address and broker-management were kept, no hash could be taken, and the users were left off the bus).
|
||||||
|
func TestABusAccountIsKeptOnlyAsAnAccount(t *testing.T) {
|
||||||
|
v := newVault(t, nil)
|
||||||
|
for _, row := range []struct {
|
||||||
|
name, value string
|
||||||
|
kept bool
|
||||||
|
}{
|
||||||
|
{"own/novox/nats/broker-nats-tools", "Xq3-a-random-value-the-controller-made", false},
|
||||||
|
{"own/novox/mesh-controller/broker-address", `{"password":"no user and no name that gives one"}`, false},
|
||||||
|
{"own/novox/nats/broker", `{"password":"pw"}`, true},
|
||||||
|
{"own/novox/nats/broker-nats-tools", `{"user":"novox.nats.nats-tools","password":"pw"}`, true},
|
||||||
|
{"own/novox/postgres/superuser-password", "Xq3-a-random-value-the-controller-made", true},
|
||||||
|
} {
|
||||||
|
_, err := v.Put(row.name, row.value, "the test")
|
||||||
|
if (err == nil) != row.kept || (err != nil && !strings.Contains(err.Error(), "is not issued")) {
|
||||||
|
t.Errorf("%s: kept %v, err %v", row.name, err == nil, err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user