Make sure duplicate keys are detected

node/node.go

@@ -1739,6 +1739,7 @@ func (app *App) MigrateExistingIdentity() error {
 // LoadIdentities loads all existing identities from the config directory.
 func (app *App) LoadIdentities() error {
 	signers := make([]*signing.EdSigner, 0)
+	pubKeys := make(map[string]*signing.PublicKey)
 
 	dir := filepath.Join(app.Config.DataDir(), keyDir)
 	if err := os.MkdirAll(dir, 0o700); err != nil {
@@ -1787,12 +1788,32 @@ func (app *App) LoadIdentities() error {
 			signer.PublicKey(),
 		)
 		signers = append(signers, signer)
+		pubKeys[d.Name()] = signer.PublicKey()
 		return nil
 	})
 	if err != nil {
 		return err
 	}
 
+	// make sure all public keys are unique
+	seen := make(map[string]string)
+	collision := false
+	for f1, pk := range pubKeys {
+		if f2, ok := seen[pk.String()]; ok {
+			app.log.With().Error("duplicate key",
+				log.String("filename1", f1),
+				log.String("filename2", f2),
+				pk,
+			)
+			collision = true
+			continue
+		}
+		seen[pk.String()] = f1
+	}
+	if collision {
+		return fmt.Errorf("duplicate key found in identity files")
+	}
+
 	if len(signers) > 0 {
 		app.signers = signers
 		return nil

node/node_test.go

@@ -3,7 +3,6 @@ package node
 import (
 	"bytes"
 	"context"
-	"crypto/ed25519"
 	"encoding/hex"
 	"fmt"
 	"io"
@@ -26,6 +25,7 @@ import (
 	"github.com/stretchr/testify/require"
 	"go.uber.org/zap"
 	"go.uber.org/zap/zapcore"
+	"go.uber.org/zap/zaptest/observer"
 	"golang.org/x/sync/errgroup"
 	"google.golang.org/genproto/googleapis/rpc/code"
 	"google.golang.org/grpc"
@@ -61,89 +61,91 @@ func TestMain(m *testing.M) {
 }
 
 func TestSpacemeshApp_getEdIdentity(t *testing.T) {
-	tempDir := t.TempDir()
-
-	// setup spacemesh app
-	app := New(WithLog(logtest.New(t)))
-	app.Config.DataDirParent = tempDir
-	app.log = logtest.New(t)
-
-	// Create new identity.
-	err := app.LoadIdentities()
-	require.NoError(t, err)
-	require.Len(t, app.signers, 1)
-	before := app.signers[0].PublicKey()
-
-	infos, err := os.ReadDir(tempDir)
-	require.NoError(t, err)
-	require.Len(t, infos, 1)
+	t.Run("no key", func(t *testing.T) {
+		app := New(WithLog(logtest.New(t)))
+		app.Config.DataDirParent = t.TempDir()
+		err := app.LoadIdentities()
+		require.NoError(t, err)
+		require.NotEmpty(t, app.signers)
+	})
 
-	// Load existing identity.
-	err = app.LoadIdentities()
-	require.NoError(t, err)
-	require.Len(t, app.signers, 1)
-	after := app.signers[0].PublicKey()
+	setupAppWithKeys := func(tb testing.TB, data ...[]byte) (*App, *observer.ObservedLogs) {
+		observer, observedLogs := observer.New(zapcore.WarnLevel)
+		logger := zap.New(observer)
+		app := New(WithLog(log.NewFromLog(logger)))
+		app.Config.DataDirParent = t.TempDir()
+		if len(data) == 0 {
+			return app, observedLogs
+		}
 
-	infos, err = os.ReadDir(tempDir)
-	require.NoError(t, err)
-	require.Len(t, infos, 1)
+		key := data[0]
+		keyFile := filepath.Join(app.Config.DataDirParent, keyDir, defaultKeyFileName)
+		require.NoError(t, os.MkdirAll(filepath.Dir(keyFile), 0o700))
+		require.NoError(t, os.WriteFile(keyFile, key, 0o600))
 
-	require.Equal(t, before, after)
+		for i, key := range data[1:] {
+			keyFile = filepath.Join(app.Config.DataDirParent, keyDir, fmt.Sprintf("identity_%d.key", i))
+			require.NoError(t, os.WriteFile(keyFile, key, 0o600))
+		}
+		return app, observedLogs
+	}
 
-	// Invalidate the identity by changing its file name.
-	filename := filepath.Join(tempDir, infos[0].Name())
-	err = os.Rename(filename, filename+"_")
-	require.NoError(t, err)
+	t.Run("good key", func(t *testing.T) {
+		signer, err := signing.NewEdSigner()
+		require.NoError(t, err)
 
-	// Create new identity.
-	err = app.LoadIdentities()
-	require.NoError(t, err)
-	require.Len(t, app.signers, 1)
-	after = app.signers[0].PublicKey()
+		app, _ := setupAppWithKeys(t, []byte(hex.EncodeToString(signer.PrivateKey())))
+		err = app.LoadIdentities()
+		require.NoError(t, err)
+		require.NotEmpty(t, app.signers)
+		before := app.signers[0].PublicKey()
 
-	infos, err = os.ReadDir(tempDir)
-	require.NoError(t, err)
-	require.Len(t, infos, 2)
-	require.NotEqual(t, before, after)
+		err = app.LoadIdentities()
+		require.NoError(t, err)
+		require.NotEmpty(t, app.signers)
+		after := app.signers[0].PublicKey()
+		require.Equal(t, before, after)
+	})
 
 	t.Run("bad length", func(t *testing.T) {
-		testLoadIdentities(t,
-			bytes.Repeat([]byte("ab"), signing.PrivateKeySize-1),
-			"invalid key size 63/64",
-		)
+		app, _ := setupAppWithKeys(t, bytes.Repeat([]byte("ab"), signing.PrivateKeySize-1))
+		err := app.LoadIdentities()
+		require.ErrorContains(t, err, "invalid key size 63/64")
+		require.Nil(t, app.signers)
 	})
+
 	t.Run("bad hex", func(t *testing.T) {
-		testLoadIdentities(t,
-			bytes.Repeat([]byte("CV"), signing.PrivateKeySize),
-			"decoding private key: encoding/hex: invalid byte",
-		)
+		app, _ := setupAppWithKeys(t, bytes.Repeat([]byte("CV"), signing.PrivateKeySize))
+		err := app.LoadIdentities()
+		require.ErrorContains(t, err, "decoding private key: encoding/hex: invalid byte")
+		require.Nil(t, app.signers)
 	})
-	t.Run("good key", func(t *testing.T) {
-		_, priv, err := ed25519.GenerateKey(nil)
+
+	t.Run("duplicate keys", func(t *testing.T) {
+		key1, err := signing.NewEdSigner()
+		require.NoError(t, err)
+		key2, err := signing.NewEdSigner()
 		require.NoError(t, err)
-		testLoadIdentities(t,
-			[]byte(hex.EncodeToString(priv)),
-			"",
+
+		app, observedLogs := setupAppWithKeys(t,
+			[]byte(hex.EncodeToString(key1.PrivateKey())),
+			[]byte(hex.EncodeToString(key2.PrivateKey())),
+			[]byte(hex.EncodeToString(key1.PrivateKey())),
+			[]byte(hex.EncodeToString(key2.PrivateKey())),
 		)
+		err = app.LoadIdentities()
+		require.ErrorContains(t, err, "duplicate key")
+
+		require.Len(t, observedLogs.All(), 2)
+		log1 := observedLogs.FilterField(zap.String("public_key", key1.PublicKey().ShortString()))
+		require.Len(t, log1.All(), 1)
+		require.Contains(t, log1.All()[0].Message, "duplicate key")
+		log2 := observedLogs.FilterField(zap.String("public_key", key2.PublicKey().ShortString()))
+		require.Len(t, log2.All(), 1)
+		require.Contains(t, log2.All()[0].Message, "duplicate key")
 	})
 }
 
-func testLoadIdentities(t *testing.T, data []byte, expect string) {
-	app := New(WithLog(logtest.New(t)))
-	app.Config.DataDirParent = t.TempDir()
-	keyFile := filepath.Join(app.Config.DataDirParent, keyDir, defaultKeyFileName)
-	require.NoError(t, os.MkdirAll(filepath.Dir(keyFile), 0o700))
-	require.NoError(t, os.WriteFile(keyFile, data, 0o600))
-	err := app.LoadIdentities()
-	if len(expect) > 0 {
-		require.ErrorContains(t, err, expect)
-		require.Nil(t, app.signers)
-	} else {
-		require.NoError(t, err)
-		require.NotEmpty(t, app.signers)
-	}
-}
-
 func newLogger(buf *bytes.Buffer) log.Log {
 	lvl := zap.NewAtomicLevelAt(zapcore.InfoLevel)
 	syncer := zapcore.AddSync(buf)