A secret the mesh was given is not one the mesh can reinvent

Two kinds live in module_secret and they behaved identically, which is right
for one of them. A made secret is the mesh's: when a node regenerates its
sealing key the mesh makes another and nothing is lost, because nothing else
ever knew the old one.

An accepted secret is not. A broker account's password exists because the
broker was told about it. Regenerating one puts 32 random bytes where a working
credential was — and the machine applies it, reports success, and the program
reading it fails to authenticate somewhere else entirely, with the mesh
insisting the secret was delivered, which it was.

The row now records where the value came from, and a rejoined machine asking
for an accepted one is refused with the remedy named: issue it again. No amount
of pushing produces a password the broker has never heard of.

Found while making the builder a module, which is the first thing to hold one.

Makefile

@@ -32,6 +32,16 @@ image:
 	@echo
 	@docker image inspect $(IMAGE) --format 'built {{.RepoTags}}  {{.Size}} bytes'
 
+# The builder ships as an image too, because it is a module the mesh assigns rather than a program
+# somebody starts on a machine by hand.
+BUILDER_IMAGE ?= mesh-builder:$(VERSION)
+BUILDER_DEV_TAG ?= mesh-builder:development
+
+builder-image:
+	docker build -f cmd/mesh-builder/Dockerfile -t $(BUILDER_IMAGE) -t $(BUILDER_DEV_TAG) .
+	@echo
+	@docker image inspect $(BUILDER_IMAGE) --format 'built {{.RepoTags}}  {{.Size}} bytes'
+
 # The whole gate. Raises a database, runs everything against it, and takes it down again --
 # including when the tests fail, which is why the teardown is not conditional.
 check: fmt vet postgres

internal/inventory/migrations/0013-a-secret-the-mesh-cannot-reinvent.sql

@@ -0,0 +1,16 @@
+-- Where a module's own secret came from.
+--
+-- Two kinds live in this table and they behaved identically, which was wrong in one direction
+-- only. A *made* secret is the mesh's: if the node regenerates its sealing key, the mesh makes
+-- another and nothing is lost, because nothing else ever knew the old one.
+--
+-- An *accepted* secret is not the mesh's to invent. A broker account's password exists because
+-- the broker was told about it; a licence key exists because somebody bought it. Regenerating one
+-- produces 32 random bytes where a working credential used to be -- and the machine applies it,
+-- reports success, and the program reading it fails to authenticate somewhere else entirely.
+--
+-- Everything already here was made by the mesh: accepting one is newer than this table, and the
+-- only caller that accepts is the builder's broker account, which is issued per push.
+
+alter table module_secret add column origin text not null default 'made'
+    check (origin in ('made', 'accepted'));

internal/inventory/secrets.go

@@ -156,13 +156,24 @@ func (i *Inventory) SecretForModule(ctx context.Context, node, module, name stri
 		return "", err
 	}
 
-	var sealed, against string
+	var sealed, against, origin string
 	err = i.store.Pool().QueryRow(ctx,
-		`select sealed, node_key from module_secret where node = $1 and module = $2 and name = $3`,
-		record.ID, module, name).Scan(&sealed, &against)
+		`select sealed, node_key, origin from module_secret
+		  where node = $1 and module = $2 and name = $3`,
+		record.ID, module, name).Scan(&sealed, &against, &origin)
 	if err == nil && against == key {
 		return sealed, nil
 	}
+	if err == nil && origin == "accepted" {
+		// Sealed to a key this node no longer has, and not the mesh's to invent again. Making one
+		// would put 32 random bytes where a working credential was: the machine would apply it,
+		// report success, and whatever reads it would fail to authenticate somewhere else
+		// entirely — with the mesh insisting the secret was delivered, which it was.
+		return "", fmt.Errorf(
+			"%s on %s holds %q, which was given to the mesh rather than made by it, and %s has "+
+				"since generated a new sealing key. The mesh cannot make another; issue it again",
+			module, node, name, node)
+	}
 
 	made, err := secrets.Make(key, key)
 	if err != nil {
@@ -171,10 +182,11 @@ func (i *Inventory) SecretForModule(ctx context.Context, node, module, name stri
 	// Sealed once, to one recipient. Make seals to two ends because a provision has two; here
 	// both are the same machine, and only one copy is kept.
 	if _, err := i.store.Pool().Exec(ctx,
-		`insert into module_secret (node, module, name, sealed, node_key)
-		 values ($1, $2, $3, $4, $5)
+		`insert into module_secret (node, module, name, sealed, node_key, origin)
+		 values ($1, $2, $3, $4, $5, 'made')
 		 on conflict (node, module, name) do update set
-		   sealed = excluded.sealed, node_key = excluded.node_key, made_at = now()`,
+		   sealed = excluded.sealed, node_key = excluded.node_key,
+		   origin = excluded.origin, made_at = now()`,
 		record.ID, module, name, made.ForConsumer, key); err != nil {
 		return "", err
 	}
@@ -210,10 +222,11 @@ func (i *Inventory) AcceptSecretForModule(ctx context.Context, node, module, nam
 		return err
 	}
 	_, err = i.store.Pool().Exec(ctx,
-		`insert into module_secret (node, module, name, sealed, node_key)
-		 values ($1, $2, $3, $4, $5)
+		`insert into module_secret (node, module, name, sealed, node_key, origin)
+		 values ($1, $2, $3, $4, $5, 'accepted')
 		 on conflict (node, module, name) do update set
-		   sealed = excluded.sealed, node_key = excluded.node_key, made_at = now()`,
+		   sealed = excluded.sealed, node_key = excluded.node_key,
+		   origin = excluded.origin, made_at = now()`,
 		record.ID, module, name, sealed.ForConsumer, key)
 	return err
 }

internal/inventory/secrets_test.go

@@ -384,3 +384,65 @@ func TestACredentialGoesWhenEitherMachineDoes(t *testing.T) {
 		t.Fatalf("a departed machine's credential is still granted: %+v", issued)
 	}
 }
+
+// The other half of that, and the one that must not behave the same way.
+//
+// A secret the mesh made, it can make again — nothing else ever knew the old one. A secret the
+// mesh was *given* it cannot: the broker knows a password, and the mesh inventing another puts 32
+// random bytes where a working credential was. The machine applies it, reports success, and
+// whatever reads it fails to authenticate somewhere else entirely, with the mesh insisting the
+// secret was delivered — which it was.
+func TestASecretTheMeshWasGivenIsNotReinventedWhenTheMachineRejoins(t *testing.T) {
+	inv, ctx := twoNodesWithKeys(t)
+	if err := inv.RegisterModule(ctx, catalogue.Manifest{Module: "builder", Version: "1"},
+		Source{}); err != nil {
+		t.Fatal(err)
+	}
+	const url = "amqps://builder:the-password-the-broker-was-told@broker/"
+	if err := inv.AcceptSecretForModule(ctx, "consumer", "builder", "broker", url); err != nil {
+		t.Fatal(err)
+	}
+	node, err := inv.NodeByName(ctx, "consumer")
+	if err != nil {
+		t.Fatal(err)
+	}
+	fresh, _ := aSealingKey(t)
+	if err := inv.RecordSealingKey(ctx, node.ID, fresh); err != nil {
+		t.Fatal(err)
+	}
+
+	_, err = inv.SecretForModule(ctx, "consumer", "builder", "broker")
+	if err == nil {
+		t.Fatal("the mesh invented a broker password, which the broker has never heard of")
+	}
+	// And says what to do about it, because the remedy is a command somebody runs and no amount
+	// of pushing will produce one.
+	if !strings.Contains(err.Error(), "issue it again") {
+		t.Fatalf("refused without saying what would fix it: %v", err)
+	}
+}
+
+// Until the key changes, a given secret is handed back unchanged — the ordinary case, and the one
+// that would make the refusal above useless if it were wrong.
+func TestASecretTheMeshWasGivenSurvivesAnOrdinaryPush(t *testing.T) {
+	inv, ctx := twoNodesWithKeys(t)
+	if err := inv.RegisterModule(ctx, catalogue.Manifest{Module: "builder", Version: "1"},
+		Source{}); err != nil {
+		t.Fatal(err)
+	}
+	if err := inv.AcceptSecretForModule(ctx, "consumer", "builder", "broker",
+		"amqps://builder:password@broker/"); err != nil {
+		t.Fatal(err)
+	}
+	first, err := inv.SecretForModule(ctx, "consumer", "builder", "broker")
+	if err != nil {
+		t.Fatal(err)
+	}
+	second, err := inv.SecretForModule(ctx, "consumer", "builder", "broker")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if first != second || first == "" {
+		t.Fatal("a given secret changed between two pushes, so the machine was handed two")
+	}
+}