diff --git a/keystore/admin.go b/keystore/admin.go index f6754ba053..4686cc0214 100644 --- a/keystore/admin.go +++ b/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) diff --git a/keystore/admin_test.go b/keystore/admin_test.go index 6a72d261db..de8f51abad 100644 --- a/keystore/admin_test.go +++ b/keystore/admin_test.go @@ -3,6 +3,7 @@ package keystore_test import ( "context" "fmt" + "math/big" "sort" "sync" "testing" @@ -10,6 +11,8 @@ import ( "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) +} diff --git a/keystore/internal/raw.go b/keystore/internal/raw.go index ae97519851..bde7aad8e1 100644 --- a/keystore/internal/raw.go +++ b/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 } diff --git a/keystore/keystore.go b/keystore/keystore.go index 206ee4685a..15306cda22 100644 --- a/keystore/keystore.go +++ b/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) diff --git a/keystore/reader.go b/keystore/reader.go index f05c1509ca..a35023ac55 100644 --- a/keystore/reader.go +++ b/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 })