[ARCH-327] Address security comments

keystore/admin.go

@@ -9,6 +9,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"maps"
+	"slices"
 	"time"
 
 	gethkeystore "github.com/ethereum/go-ethereum/accounts/keystore"
@@ -21,6 +22,11 @@ import (
 	"github.com/smartcontractkit/chainlink-common/keystore/serialization"
 )
 
+const (
+	MaxKeyNameLength  = 1000
+	MaxMetadataLength = 1024 * 1024 // 1mb
+)
+
 var (
 	ErrKeyAlreadyExists   = fmt.Errorf("key already exists")
 	ErrInvalidKeyName     = fmt.Errorf("invalid key name")
@@ -165,8 +171,8 @@ func ValidKeyName(name string) error {
 		return fmt.Errorf("key name cannot be empty")
 	}
 	// Just a sanity bound.
-	if len(name) > 1_000 {
-		return fmt.Errorf("key name cannot be longer than 1000 characters")
+	if len(name) > MaxKeyNameLength {
+		return fmt.Errorf("key name cannot be longer than %d characters", MaxKeyNameLength)
 	}
 	return nil
 }
@@ -202,9 +208,8 @@ func (ks *keystore) CreateKeys(ctx context.Context, req CreateKeysRequest) (Crea
 			if err != nil {
 				return CreateKeysResponse{}, fmt.Errorf("failed to generate ECDSA_S256 key: %w", err)
 			}
-			// Must copy the private key into 32 byte slice because leading zeros are stripped.
 			privateKeyBytes := make([]byte, 32)
-			copy(privateKeyBytes, privateKey.D.Bytes())
+			privateKey.D.FillBytes(privateKeyBytes)
 			publicKey, err := publicKeyFromPrivateKey(internal.NewRaw(privateKeyBytes), keyReq.KeyType)
 			if err != nil {
 				return CreateKeysResponse{}, fmt.Errorf("failed to get public key from private key: %w", err)
@@ -291,6 +296,9 @@ func (ks *keystore) ImportKeys(ctx context.Context, req ImportKeysRequest) (Impo
 		}
 		pkRaw := internal.NewRaw(keypb.PrivateKey)
 		keyType := KeyType(keypb.KeyType)
+		if !slices.Contains(AllKeyTypes, keyType) {
+			return ImportKeysResponse{}, fmt.Errorf("%w: %s, available key types: %s", ErrUnsupportedKeyType, keyType, AllKeyTypes.String())
+		}
 		publicKey, err := publicKeyFromPrivateKey(pkRaw, keyType)
 		if err != nil {
 			return ImportKeysResponse{}, fmt.Errorf("key num = %d, failed to get public key from private key: %w", i, err)
@@ -301,6 +309,9 @@ func (ks *keystore) ImportKeys(ctx context.Context, req ImportKeysRequest) (Impo
 		if metadata == nil {
 			metadata = []byte{}
 		}
+		if len(metadata) > MaxMetadataLength {
+			return ImportKeysResponse{}, fmt.Errorf("key num = %d, metadata of length %d exceeds maximum length of %d bytes", i, len(metadata), MaxMetadataLength)
+		}
 
 		keyName := keyReq.NewKeyName
 		if keyName == "" {
@@ -366,6 +377,9 @@ func (ks *keystore) SetMetadata(ctx context.Context, req SetMetadataRequest) (Se
 
 	ksCopy := maps.Clone(ks.keystore)
 	for _, metReq := range req.Updates {
+		if len(metReq.Metadata) > MaxMetadataLength {
+			return SetMetadataResponse{}, fmt.Errorf("metadata for key %s exceeds maximum length of %d bytes", metReq.KeyName, MaxMetadataLength)
+		}
 		key, ok := ksCopy[metReq.KeyName]
 		if !ok {
 			return SetMetadataResponse{}, fmt.Errorf("%w: %s", ErrKeyNotFound, metReq.KeyName)

keystore/admin_test.go

@@ -3,13 +3,16 @@ package keystore_test
 import (
 	"context"
 	"fmt"
+	"math/big"
 	"sort"
 	"sync"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
+	gethcrypto "github.com/ethereum/go-ethereum/crypto"
+
 	"github.com/smartcontractkit/chainlink-common/keystore"
 )
 
@@ -258,7 +261,16 @@ func TestKeystore_ExportImport(t *testing.T) {
 		key1ks1, err := ks1.GetKeys(t.Context(), keystore.GetKeysRequest{KeyNames: []string{"key1"}})
 		require.NoError(t, err)
 		key1ks2, err := ks2.GetKeys(t.Context(), keystore.GetKeysRequest{KeyNames: []string{"key1"}})
-		require.Equal(t, key1ks1, key1ks2)
+		require.NoError(t, err)
+		// Test equality of the keys except of the CreatedAt field.
+		require.Len(t, key1ks1.Keys, 1)
+		require.Len(t, key1ks2.Keys, 1)
+		key1ks1Info := key1ks1.Keys[0].KeyInfo
+		key1ks2Info := key1ks2.Keys[0].KeyInfo
+		require.Equal(t, key1ks1Info.Name, key1ks2Info.Name)
+		require.Equal(t, key1ks1Info.PublicKey, key1ks2Info.PublicKey)
+		require.Equal(t, key1ks1Info.KeyType, key1ks2Info.KeyType)
+		require.Equal(t, key1ks1Info.Metadata, key1ks2Info.Metadata)
 
 		testData := []byte("hello world")
 		signature, err := ks2.Sign(t.Context(), keystore.SignRequest{
@@ -411,3 +423,20 @@ func TestKeystore_RenameKey(t *testing.T) {
 		require.EqualError(t, err, "key not found: key1")
 	})
 }
+
+func TestECDSA_Serialization_WithPadding(t *testing.T) {
+	// This test ensures that ECDSA private keys that serialize to less than 32 bytes
+	// are correctly padded with leading zeros during serialization and deserialization.
+	// This is important for compatibility with Ethereum's crypto library which expects
+	// 32-byte private keys.
+
+	// The example key has been found randomly such that it has 2 leading zero bytes when serialized.
+	key, ok := big.NewInt(0).SetString("57269542458293433845411819226400606954116463824740942170224417652371448", 10)
+	require.True(t, ok)
+	privateKeyBytes := make([]byte, 32)
+	key.FillBytes(privateKeyBytes)
+	require.Equal(t, []byte{0, 0, 8, 76, 62, 209, 247, 104, 97, 108, 141, 217, 255, 150, 114, 196, 223, 66, 254, 101, 209, 14, 233, 174, 149, 89, 207, 141, 2, 188, 111, 248}, privateKeyBytes)
+	deserializedKey, err := gethcrypto.ToECDSA(privateKeyBytes)
+	require.NoError(t, err)
+	require.Equal(t, key, deserializedKey.D)
+}

keystore/internal/raw.go

@@ -2,6 +2,8 @@
 // only available for use in the keystore sub-tree.
 package internal
 
+import "fmt"
+
 // Raw is a wrapper type that holds private key bytes
 // and is designed to prevent accidental logging.
 // The only way to access the internal bytes (without reflection) is to use Bytes,
@@ -22,6 +24,10 @@ func (raw Raw) GoString() string {
 	return raw.String()
 }
 
+func (raw Raw) Format(state fmt.State, _ rune) {
+	_, _ = fmt.Fprint(state, raw.String())
+}
+
 // Bytes is a func for accessing the internal bytes field of Raw.
 // It is not declared as a method, because that would allow access from callers which cannot otherwise access this internal package.
 func Bytes(raw Raw) []byte { return raw.bytes }

keystore/keystore.go

@@ -8,14 +8,13 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"log/slog"
 	"slices"
 	"strings"
 	"sync"
 	"testing"
 	"time"
 
-	"log/slog"
-
 	"golang.org/x/crypto/curve25519"
 
 	gethkeystore "github.com/ethereum/go-ethereum/accounts/keystore"
@@ -201,6 +200,9 @@ type EncryptionParams struct {
 func publicKeyFromPrivateKey(privateKeyBytes internal.Raw, keyType KeyType) ([]byte, error) {
 	switch keyType {
 	case Ed25519:
+		if len(internal.Bytes(privateKeyBytes)) != ed25519.PrivateKeySize {
+			return nil, fmt.Errorf("invalid Ed25519 private key size: %d", len(internal.Bytes(privateKeyBytes)))
+		}
 		privateKey := ed25519.PrivateKey(internal.Bytes(privateKeyBytes))
 		publicKey := privateKey.Public().(ed25519.PublicKey)
 		return publicKey, nil
@@ -216,6 +218,9 @@ func publicKeyFromPrivateKey(privateKeyBytes internal.Raw, keyType KeyType) ([]b
 		pubKey := gethcrypto.FromECDSAPub(&privateKey.PublicKey)
 		return pubKey, nil
 	case X25519:
+		if len(internal.Bytes(privateKeyBytes)) != curve25519.ScalarSize {
+			return nil, fmt.Errorf("invalid X25519 private key size: %d", len(internal.Bytes(privateKeyBytes)))
+		}
 		pubKey, err := curve25519.X25519(internal.Bytes(privateKeyBytes)[:], curve25519.Basepoint)
 		if err != nil {
 			return nil, fmt.Errorf("failed to derive shared secret: %w", err)

keystore/reader.go

@@ -63,12 +63,7 @@ func (k *keystore) GetKeys(ctx context.Context, req GetKeysRequest) (GetKeysResp
 		}
 		seen[name] = true
 		responses = append(responses, GetKeyResponse{
-			KeyInfo: KeyInfo{
-				Name:      name,
-				KeyType:   key.keyType,
-				PublicKey: key.publicKey,
-				Metadata:  key.metadata,
-			},
+			KeyInfo: newKeyInfo(name, key.keyType, key.createdAt, key.publicKey, key.metadata),
 		})
 	}
 	sort.Slice(responses, func(i, j int) bool { return responses[i].KeyInfo.Name < responses[j].KeyInfo.Name })