Merge pull request 'Seal core credentials by the controller while the vault does not hold them (issue 908, ADR 0396)' (#356) from fix/908-seal-core-until-vault-holds into main

cmd/mesh-controller/core_box.go

@@ -9,6 +9,7 @@ import (
 	"encoding/pem"
 	"errors"
 	"fmt"
+	"slices"
 	"sort"
 	"strings"
 
@@ -31,10 +32,13 @@ import (
 // vault's generation and the machine's key are the same: a declaration that changed with every answer would never be
 // current. Every question but a send reads the kept box and asks nothing.
 //
-// **When the vault does not answer, the controller never falls back to its own seal.** A credential with a kept box
-// keeps it — the machine keeps the file it has — and one with none leaves its module out of the declaration (the
-// machine keeps what it holds of it, and a module not there yet is not placed). Either way the send raises a
-// condition naming the vault and the machine, cleared by the first send the vault answers whole.
+// **Until the vault holds a machine's core credentials, the controller seals them itself** (novox/hq ADR 0396, issue
+// 908; it replaces the ADR 0349 §16 rule that the controller never does). When the vault does not answer, a credential
+// with a kept box keeps it — the machine keeps the file it has — and one with none has its module composed as for an
+// engine below the contract: with the controller's own sealed value, never left out (a core module left out took the
+// bus's port from the packet filter and cut the remote machines off). Either way the send raises a condition naming
+// the vault and the machine, cleared by the first send the vault answers whole. Only a machine with no sealing key, or
+// one whose key cannot be read, leaves its module out.
 
 // engineOpensVaultBox says whether a machine's node-engine opens the vault's box, by its own newest statement.
 func engineOpensVaultBox(ctx context.Context, inv *inventory.Inventory, node string) (bool, error) {
@@ -64,6 +68,9 @@ type coreBoxes struct {
 	notPlaced map[string]string
 	// keep is each box newly answered, to keep.
 	keep []inventory.CoreBox
+	// byController is each core module the vault did not answer for and no box was kept for, which is composed with
+	// the controller's own seal until the vault holds it (ADR 0396).
+	byController []string
 	// held is each credential the vault did not answer for whose kept box is carried again; silent is why the vault
 	// did not answer, per credential.
 	held, silent []string
@@ -110,8 +117,8 @@ func sealingKeyPEM(raw string) (string, error) {
 }
 
 // coreBoxesOf decides each core credential of a machine: on a send, the vault's answer (the kept box while its
-// generation and key are the same), the kept box where the vault did not answer, or the module left out where nothing
-// was kept; on any other question, the kept box, or a stand-in the next send asks for. Pure: what to keep and what to
+// generation and key are the same), the kept box where the vault did not answer, or the module left to the controller's
+// own seal where nothing was kept; on any other question, the kept box, or a stand-in the next send asks for. Pure: what to keep and what to
 // say is answered, not done.
 func coreBoxesOf(node, sealingKey string, core map[string][]string, kept map[string]map[string]inventory.CoreBox,
 	choosing Choosing, ask coreAsk) coreBoxes {
@@ -156,9 +163,7 @@ func coreBoxesOf(node, sealingKey string, core map[string][]string, kept map[str
 					out.held = append(out.held, module+"/"+name)
 					continue
 				}
-				out.notPlaced[module] = fmt.Sprintf("the vault did not give %s's credential %s for %s (%v), and the "+
-					"controller never seals it itself (novox/hq ADR 0349 §16): not placed until the vault answers",
-					module, name, node, err)
+				out.byController = append(out.byController, module)
 				break
 			}
 			if valid && k.Generation == got.Generation {
@@ -170,7 +175,7 @@ func coreBoxesOf(node, sealingKey string, core map[string][]string, kept map[str
 			out.keep = append(out.keep, inventory.CoreBox{Module: module, Name: name, Generation: got.Generation,
 				SealedTo: sealingKey, Box: box})
 		}
-		if _, left := out.notPlaced[module]; !left {
+		if _, left := out.notPlaced[module]; !left && !slices.Contains(out.byController, module) {
 			out.sealed[module] = boxes
 		}
 	}
@@ -262,6 +267,9 @@ func vaultSilentObservation(node string, b coreBoxes) conditions.Observation {
 		sort.Strings(left)
 		said = append(said, "not placed: "+strings.Join(left, ", "))
 	}
+	if len(b.byController) > 0 {
+		said = append(said, "sealed by the controller until the vault holds it: "+strings.Join(b.byController, ", "))
+	}
 	if len(b.held) > 0 {
 		said = append(said, "kept as last sent: "+strings.Join(b.held, ", "))
 	}

cmd/mesh-controller/core_box_test.go

@@ -19,7 +19,7 @@ import (
 // The core's own credentials travel sealed by the vault to their machine (novox/hq ADR 0349 §16, issue 755; the lead's
 // decision, issue 755 notes 4 and 5): to an engine that opens the vault's box, the send carries the box the vault
 // answered the controller and never the controller's own seal; when the vault does not answer, a kept box is carried
-// again and a module with none is not placed, with a condition naming the vault and the machine. An engine below the
+// again and a module with none is sealed by the controller (ADR 0396, issue 908), with a condition naming the vault and the machine. An engine below the
 // contract composes exactly as before, and the vault is not asked.
 
 // coreStandIn answers `get` with a box per name and generation, or its silence.
@@ -141,7 +141,7 @@ func TestASilentVaultHoldsTheKeptBoxAndSaysSo(t *testing.T) {
 	}
 }
 
-func TestACoreModuleFirstPlacedWhileTheVaultIsSilentIsNotPlaced(t *testing.T) {
+func TestACoreModuleFirstPlacedWhileTheVaultIsSilentIsSealedByTheController(t *testing.T) {
 	open, vault := aCoreMesh(t, link.VaultBoxContract)
 	vault.silent = errors.New("no responders")
 	plan, settings, err := planFor(t.Context(), open, "laptop")
@@ -152,20 +152,44 @@ func TestACoreModuleFirstPlacedWhileTheVaultIsSilentIsNotPlaced(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	for _, r := range declared.Resources {
-		if strings.HasPrefix(r["id"].(string), "mesh-provisioner-client.") {
-			t.Fatalf("a core module was placed though the vault gave nothing for it: %v", r)
-		}
+	f := clientsBroker(declared.Resources)
+	if f == nil || f["sealed"] == nil || f[catalogue.VaultSealedField] != nil {
+		t.Fatalf("the core credential is composed as %v: the controller's own seal, until the vault holds it", f)
 	}
-	if why := declared.leftOutWhy["mesh-provisioner-client"]; !strings.Contains(why, "never seals it itself") {
-		t.Fatalf("left out %v", declared.leftOutWhy)
+	if why, left := declared.leftOutWhy["mesh-provisioner-client"]; left {
+		t.Fatalf("the core module was left out: %v", why)
 	}
 	c, found := vaultSilence(t, "laptop")
-	if !found || c.Severity != conditions.Urgent || !strings.Contains(c.Summary, "not placed") {
+	if !found || !strings.Contains(c.Summary, "sealed by the controller until the vault holds it") {
 		t.Fatalf("the condition does not say why: %+v", c)
 	}
 }
 
+// Issue 908: a machine at the vault-box contract, the vault silent, no kept box: nats is composed with the
+// controller's own seal, never left out, and the vault-silent condition names it.
+func TestReplay908(t *testing.T) {
+	key := "AAECAwQFBgcICQoLDA0ODxAREhMUFRYXGBkaGxwdHh8="
+	core := map[string][]string{"nats": {"password"}}
+	ask := func(name, publicKey string) (coreAnswer, error) { return coreAnswer{}, errors.New("no responders") }
+	got := coreBoxesOf("novox", key, core, nil, Allocating, ask)
+	if _, sealed := got.sealed["nats"]; sealed || len(got.notPlaced) != 0 {
+		t.Fatalf("nats is vault-sealed %v or not placed %v", got.sealed, got.notPlaced)
+	}
+	if !reflect.DeepEqual(got.byController, []string{"nats"}) {
+		t.Fatalf("nats is not left to the controller's seal: %+v", got)
+	}
+	o := vaultSilentObservation("novox", got)
+	if o.Severity != conditions.Warning || !strings.Contains(o.Summary, "sealed by the controller until the vault holds it: nats") {
+		t.Fatalf("the condition does not name nats: %+v", o)
+	}
+	// A kept box is still carried while the vault is silent.
+	kept := map[string]map[string]inventory.CoreBox{"nats": {"password": {SealedTo: key, Box: json.RawMessage(`{"v":1}`)}}}
+	got = coreBoxesOf("novox", key, core, kept, Allocating, ask)
+	if string(got.sealed["nats"]["password"]) != `{"v":1}` || len(got.byController) != 0 {
+		t.Fatalf("the kept box was not held: %+v", got)
+	}
+}
+
 // An engine below VaultBoxContract is sent the controller's own seal, as before, and the vault is not asked.
 func TestAnEngineBelowTheContractIsSentTheControllersSealAsBefore(t *testing.T) {
 	open, vault := aCoreMesh(t, link.HandedContract)
@@ -200,7 +224,7 @@ func TestOnlyASendAsksTheVault(t *testing.T) {
 	// A box kept for another key of the machine is never carried.
 	other := coreBoxesOf("anchor", aPublicKey(t), core, kept, Allocating,
 		func(string, string) (coreAnswer, error) { return coreAnswer{}, errors.New("silent") })
-	if _, left := other.notPlaced["nats"]; !left {
+	if _, vaultSealed := other.sealed["nats"]; vaultSealed || !reflect.DeepEqual(other.byController, []string{"nats"}) {
 		t.Fatalf("a box sealed to the machine's old key was carried: %+v", other)
 	}
 }

cmd/mesh-controller/plain_words.go

@@ -192,7 +192,7 @@ var plainWordings = map[string]func(conditions.Observation) words{
 			Needs: "check the vault answers (vault_status); the next send asks it again.",
 			Explanation: fmt.Sprintf("The bus's and the provisioner's credentials reach %s sealed by the vault, and "+
 				"the vault did not answer the last send for them. %s keeps the files it has; a core module not "+
-				"there yet is not placed. The mesh never seals them itself instead.", m, m),
+				"there yet is sealed by the controller until the vault holds it.", m, m),
 			Resolved: "Resolved: the vault gave " + m + " the core's credentials"}
 	}),
 	kindCoreDowngrade: worded(func(o conditions.Observation) words {

testdata/store-tests

@@ -110,7 +110,7 @@
 ./internal/inventory TestACoreCredentialsVaultBoxIsKeptPerMachineAndReplaced
 ./cmd/mesh-controller TestTheSendCarriesTheVaultsBoxForACoreCredentialAndKeepsIt
 ./cmd/mesh-controller TestASilentVaultHoldsTheKeptBoxAndSaysSo
-./cmd/mesh-controller TestACoreModuleFirstPlacedWhileTheVaultIsSilentIsNotPlaced
+./cmd/mesh-controller TestACoreModuleFirstPlacedWhileTheVaultIsSilentIsSealedByTheController
 ./cmd/mesh-controller TestAnEngineBelowTheContractIsSentTheControllersSealAsBefore
 ./cmd/mesh-controller TestModuleIssueHandsACoveredAccountToTheVaultAndOnlyThat
 ./cmd/mesh-controller TestModuleIssueFailsWhenTheVaultDoesNotTakeACoveredAccount