diff --git a/internal/identity/identity.go b/internal/identity/identity.go index 29c9ae2..eb48920 100644 --- a/internal/identity/identity.go +++ b/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 } diff --git a/internal/identity/identity_test.go b/internal/identity/identity_test.go index 57a0169..9f4c86a 100644 --- a/internal/identity/identity_test.go +++ b/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") }