internal/identity/identity.go

@@ -148,6 +148,16 @@ func Save(path string, i Identity) error {
 	if len(i.Private) != ed25519.PrivateKeySize {
 		return errors.New("refusing to save an identity with no usable private key")
 	}
+	// Save refuses exactly what Load refuses. Without this, a caller can write a file that
+	// cannot be read back — and it would be read back on the next start, on a machine nobody is
+	// watching, by which time the token that could have fixed it is spent.
+	if strings.TrimSpace(i.Node) == "" {
+		return errors.New("refusing to save an identity that names no node")
+	}
+	if !i.Membership.Joined() {
+		return errors.New("refusing to save an identity that does not say how to reach its " +
+			"mesh: it could not be used after a restart, and Load will not accept it")
+	}
 	if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil {
 		return err
 	}

internal/identity/identity_test.go

@@ -40,12 +40,47 @@ func TestAnUnreadableIdentityIsNotTheSameAsHavingNone(t *testing.T) {
 	}
 }
 
-func TestWhatIsSavedIsWhatIsLoaded(t *testing.T) {
-	path := Path(filepath.Join(t.TempDir(), "state.json"))
-	made, err := Generate("workstation")
+// joined is an identity as it exists after enrolment, which is the only kind ever saved:
+// Generate makes the keypair, and the mesh supplies everything under Membership.
+func joined(t *testing.T, name string) Identity {
+	t.Helper()
+	made, err := Generate(name)
 	if err != nil {
 		t.Fatal(err)
 	}
+	signer, _, err := ed25519.GenerateKey(nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	made.Membership = Membership{
+		Broker:      "192.0.2.10:5671",
+		Fingerprint: "sha256:" + strings.Repeat("ab", 32),
+		Signer:      signer,
+		Password:    "this node's own",
+	}
+	return made
+}
+
+func TestSaveRefusesWhatLoadWouldRefuse(t *testing.T) {
+	// The two must agree, or a caller can write a file that cannot be read back — and it would
+	// be read back on the next start, on a machine nobody is watching, by which time the token
+	// that could have fixed it is spent.
+	path := Path(filepath.Join(t.TempDir(), "state.json"))
+	unenrolled, err := Generate("workstation")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := Save(path, unenrolled); err == nil {
+		t.Fatal("an identity with no membership was saved; Load will not accept it")
+	}
+	if _, err := os.Stat(path); err == nil {
+		t.Error("the refused identity was written anyway")
+	}
+}
+
+func TestWhatIsSavedIsWhatIsLoaded(t *testing.T) {
+	path := Path(filepath.Join(t.TempDir(), "state.json"))
+	made := joined(t, "workstation")
 	if err := Save(path, made); err != nil {
 		t.Fatal(err)
 	}
@@ -58,6 +93,12 @@ func TestWhatIsSavedIsWhatIsLoaded(t *testing.T) {
 		string(back.Private) != string(made.Private) {
 		t.Error("the identity changed across a save and load")
 	}
+	if back.Membership.Broker != made.Membership.Broker ||
+		back.Membership.Fingerprint != made.Membership.Fingerprint ||
+		back.Membership.Password != made.Membership.Password ||
+		string(back.Membership.Signer) != string(made.Membership.Signer) {
+		t.Error("the membership changed across a save and load; this node could not come back")
+	}
 }
 
 func TestTheIdentityIsNotReadableByAnybodyElse(t *testing.T) {
@@ -65,11 +106,7 @@ func TestTheIdentityIsNotReadableByAnybodyElse(t *testing.T) {
 	// this machine read it would make "compromise of a node is compromise of that node" false in
 	// the other direction — any local user could become the node.
 	path := Path(filepath.Join(t.TempDir(), "state.json"))
-	made, err := Generate("workstation")
-	if err != nil {
-		t.Fatal(err)
-	}
-	if err := Save(path, made); err != nil {
+	if err := Save(path, joined(t, "workstation")); err != nil {
 		t.Fatal(err)
 	}
 
@@ -89,10 +126,7 @@ func TestSavingLeavesNoHalfWrittenIdentity(t *testing.T) {
 	// the old public key, and a new one needs a person with a new token.
 	dir := t.TempDir()
 	path := Path(filepath.Join(dir, "state.json"))
-	made, err := Generate("workstation")
-	if err != nil {
-		t.Fatal(err)
-	}
+	made := joined(t, "workstation")
 	for i := 0; i < 3; i++ {
 		if err := Save(path, made); err != nil {
 			t.Fatal(err)
@@ -258,18 +292,14 @@ func TestAnIdentityThatCannotBeReadIsNotReportedAsAbsent(t *testing.T) {
 		t.Skip("running as root, which can read anything")
 	}
 	path := Path(filepath.Join(t.TempDir(), "state.json"))
-	made, err := Generate("workstation")
-	if err != nil {
-		t.Fatal(err)
-	}
-	if err := Save(path, made); err != nil {
+	if err := Save(path, joined(t, "workstation")); err != nil {
 		t.Fatal(err)
 	}
 	if err := os.Chmod(path, 0o000); err != nil {
 		t.Fatal(err)
 	}
 
-	_, err = Load(path)
+	_, err := Load(path)
 	if err == nil {
 		t.Fatal("an unreadable identity loaded")
 	}