From 811ca7f0457e11765137eca7fa3923938f702f12 Mon Sep 17 00:00:00 2001 From: Federico Kunze <31522760+fedekunze@users.noreply.github.com> Date: Wed, 30 Sep 2020 02:34:01 +0200 Subject: [PATCH] rpc: implement personal_importRawKey (#552) --- CHANGELOG.md | 4 + crypto/algorithm.go | 4 +- rpc/apis.go | 2 +- rpc/eth_api.go | 2 +- rpc/personal_api.go | 166 ++++++++++++++++++++--------------------- tests/personal_test.go | 23 +++++- 6 files changed, 112 insertions(+), 89 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3c0eca06d..579d097ec 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,6 +37,10 @@ Ref: https://keepachangelog.com/en/1.0.0/ ## Unreleased +### Features + +* (rpc) [\#552](https://github.com/ChainSafe/ethermint/pull/552) Implement Eth Personal namespace `personal_importRawKey`. + ### Bug fixes * (keys) [\#554](https://github.com/ChainSafe/ethermint/pull/554) Fix private key derivation. diff --git a/crypto/algorithm.go b/crypto/algorithm.go index ee448ef3f..901ed46e5 100644 --- a/crypto/algorithm.go +++ b/crypto/algorithm.go @@ -18,8 +18,10 @@ import ( ) const ( + // EthSecp256k1Type string constant for the EthSecp256k1 algorithm + EthSecp256k1Type = "eth_secp256k1" // EthSecp256k1 defines the ECDSA secp256k1 used on Ethereum - EthSecp256k1 = keys.SigningAlgo("eth_secp256k1") + EthSecp256k1 = keys.SigningAlgo(EthSecp256k1Type) ) // SupportedAlgorithms defines the list of signing algorithms used on Ethermint: diff --git a/rpc/apis.go b/rpc/apis.go index 0dfb4ae0c..eeeaf0d45 100644 --- a/rpc/apis.go +++ b/rpc/apis.go @@ -40,7 +40,7 @@ func GetRPCAPIs(cliCtx context.CLIContext, keys []emintcrypto.PrivKeySecp256k1) { Namespace: PersonalNamespace, Version: apiVersion, - Service: NewPersonalEthAPI(cliCtx, ethAPI, nonceLock, keys), + Service: NewPersonalEthAPI(ethAPI), Public: false, }, { diff --git a/rpc/eth_api.go b/rpc/eth_api.go index a360fcfbb..059dfa846 100644 --- a/rpc/eth_api.go +++ b/rpc/eth_api.go @@ -46,7 +46,7 @@ type PublicEthAPI struct { chainIDEpoch *big.Int logger log.Logger backend Backend - keys []crypto.PrivKeySecp256k1 + keys []crypto.PrivKeySecp256k1 // unlocked keys nonceLock *AddrLocker keybaseLock sync.Mutex } diff --git a/rpc/personal_api.go b/rpc/personal_api.go index 2b783dd15..439d17039 100644 --- a/rpc/personal_api.go +++ b/rpc/personal_api.go @@ -5,43 +5,34 @@ import ( "context" "fmt" "os" - "sync" "time" - sdkcontext "github.com/cosmos/cosmos-sdk/client/context" + "github.com/spf13/viper" + "github.com/cosmos/cosmos-sdk/client/flags" "github.com/cosmos/cosmos-sdk/crypto/keys" + "github.com/cosmos/cosmos-sdk/crypto/keys/mintkey" sdk "github.com/cosmos/cosmos-sdk/types" - emintcrypto "github.com/cosmos/ethermint/crypto" - params "github.com/cosmos/ethermint/rpc/args" - "github.com/spf13/viper" - "github.com/tendermint/tendermint/libs/log" "github.com/ethereum/go-ethereum/accounts" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/common/hexutil" "github.com/ethereum/go-ethereum/crypto" + + emintcrypto "github.com/cosmos/ethermint/crypto" + params "github.com/cosmos/ethermint/rpc/args" ) -// PersonalEthAPI is the eth_ prefixed set of APIs in the Web3 JSON-RPC spec. +// PersonalEthAPI is the personal_ prefixed set of APIs in the Web3 JSON-RPC spec. type PersonalEthAPI struct { - logger log.Logger - cliCtx sdkcontext.CLIContext - ethAPI *PublicEthAPI - nonceLock *AddrLocker - keys []emintcrypto.PrivKeySecp256k1 - keyInfos []keys.Info - keybaseLock sync.Mutex + ethAPI *PublicEthAPI + keyInfos []keys.Info // all keys, both locked and unlocked. unlocked keys are stored in ethAPI.keys } -// NewPersonalEthAPI creates an instance of the public ETH Web3 API. -func NewPersonalEthAPI(cliCtx sdkcontext.CLIContext, ethAPI *PublicEthAPI, nonceLock *AddrLocker, keys []emintcrypto.PrivKeySecp256k1) *PersonalEthAPI { +// NewPersonalEthAPI creates an instance of the public Personal Eth API. +func NewPersonalEthAPI(ethAPI *PublicEthAPI) *PersonalEthAPI { api := &PersonalEthAPI{ - logger: log.NewTMLogger(log.NewSyncWriter(os.Stdout)).With("module", "json-rpc"), - cliCtx: cliCtx, - ethAPI: ethAPI, - nonceLock: nonceLock, - keys: keys, + ethAPI: ethAPI, } infos, err := api.getKeybaseInfo() @@ -54,43 +45,68 @@ func NewPersonalEthAPI(cliCtx sdkcontext.CLIContext, ethAPI *PublicEthAPI, nonce } func (e *PersonalEthAPI) getKeybaseInfo() ([]keys.Info, error) { - e.keybaseLock.Lock() - defer e.keybaseLock.Unlock() + e.ethAPI.keybaseLock.Lock() + defer e.ethAPI.keybaseLock.Unlock() - if e.cliCtx.Keybase == nil { + if e.ethAPI.cliCtx.Keybase == nil { keybase, err := keys.NewKeyring( sdk.KeyringServiceName(), viper.GetString(flags.FlagKeyringBackend), viper.GetString(flags.FlagHome), - e.cliCtx.Input, + e.ethAPI.cliCtx.Input, emintcrypto.EthSecp256k1Options()..., ) if err != nil { return nil, err } - e.cliCtx.Keybase = keybase + e.ethAPI.cliCtx.Keybase = keybase } - return e.cliCtx.Keybase.List() + return e.ethAPI.cliCtx.Keybase.List() } -// ImportRawKey stores the given hex encoded ECDSA key into the key directory, -// encrypting it with the passphrase. -// Currently, this is not implemented since the feature is not supported by the keys. +// ImportRawKey armors and encrypts a given raw hex encoded ECDSA key and stores it into the key directory. +// The name of the key will have the format "personal_", where is the total number of +// keys stored on the keyring. +// NOTE: The key will be both armored and encrypted using the same passphrase. func (e *PersonalEthAPI) ImportRawKey(privkey, password string) (common.Address, error) { - e.logger.Debug("personal_importRawKey", "error", "not implemented") - _, err := crypto.HexToECDSA(privkey) + e.ethAPI.logger.Debug("personal_importRawKey") + priv, err := crypto.HexToECDSA(privkey) if err != nil { return common.Address{}, err } - return common.Address{}, nil + privKey := emintcrypto.PrivKeySecp256k1(crypto.FromECDSA(priv)) + + armor := mintkey.EncryptArmorPrivKey(privKey, password, emintcrypto.EthSecp256k1Type) + + // ignore error as we only care about the length of the list + list, _ := e.ethAPI.cliCtx.Keybase.List() + privKeyName := fmt.Sprintf("personal_%d", len(list)) + + if err := e.ethAPI.cliCtx.Keybase.ImportPrivKey(privKeyName, armor, password); err != nil { + return common.Address{}, err + } + + addr := common.BytesToAddress(privKey.PubKey().Address().Bytes()) + + info, err := e.ethAPI.cliCtx.Keybase.Get(privKeyName) + if err != nil { + return common.Address{}, err + } + + // append key and info to be able to lock and list the account + //e.ethAPI.keys = append(e.ethAPI.keys, privKey) + e.keyInfos = append(e.keyInfos, info) + e.ethAPI.logger.Info("key successfully imported", "name", privKeyName, "address", addr.String()) + + return addr, nil } // ListAccounts will return a list of addresses for accounts this node manages. func (e *PersonalEthAPI) ListAccounts() ([]common.Address, error) { - e.logger.Debug("personal_listAccounts") + e.ethAPI.logger.Debug("personal_listAccounts") addrs := []common.Address{} for _, info := range e.keyInfos { addressBytes := info.GetPubKey().Address().Bytes() @@ -103,18 +119,7 @@ func (e *PersonalEthAPI) ListAccounts() ([]common.Address, error) { // LockAccount will lock the account associated with the given address when it's unlocked. // It removes the key corresponding to the given address from the API's local keys. func (e *PersonalEthAPI) LockAccount(address common.Address) bool { - e.logger.Debug("personal_lockAccount", "address", address) - for i, key := range e.keys { - if !bytes.Equal(key.PubKey().Address().Bytes(), address.Bytes()) { - continue - } - - tmp := make([]emintcrypto.PrivKeySecp256k1, len(e.keys)-1) - copy(tmp[:i], e.keys[:i]) - copy(tmp[i:], e.keys[i+1:]) - e.keys = tmp - return true - } + e.ethAPI.logger.Debug("personal_lockAccount", "address", address.String()) for i, key := range e.ethAPI.keys { if !bytes.Equal(key.PubKey().Address().Bytes(), address.Bytes()) { @@ -125,6 +130,8 @@ func (e *PersonalEthAPI) LockAccount(address common.Address) bool { copy(tmp[:i], e.ethAPI.keys[:i]) copy(tmp[i:], e.ethAPI.keys[i+1:]) e.ethAPI.keys = tmp + + e.ethAPI.logger.Debug("account unlocked", "address", address.String()) return true } @@ -133,37 +140,24 @@ func (e *PersonalEthAPI) LockAccount(address common.Address) bool { // NewAccount will create a new account and returns the address for the new account. func (e *PersonalEthAPI) NewAccount(password string) (common.Address, error) { - e.logger.Debug("personal_newAccount") + e.ethAPI.logger.Debug("personal_newAccount") _, err := e.getKeybaseInfo() if err != nil { return common.Address{}, err } name := "key_" + time.Now().UTC().Format(time.RFC3339) - info, _, err := e.cliCtx.Keybase.CreateMnemonic(name, keys.English, password, emintcrypto.EthSecp256k1) + info, _, err := e.ethAPI.cliCtx.Keybase.CreateMnemonic(name, keys.English, password, emintcrypto.EthSecp256k1) if err != nil { return common.Address{}, err } e.keyInfos = append(e.keyInfos, info) - // update ethAPI - privKey, err := e.cliCtx.Keybase.ExportPrivateKeyObject(name, password) - if err != nil { - return common.Address{}, err - } - - emintKey, ok := privKey.(emintcrypto.PrivKeySecp256k1) - if !ok { - return common.Address{}, fmt.Errorf("invalid private key type: %T", privKey) - } - e.ethAPI.keys = append(e.ethAPI.keys, emintKey) - e.logger.Debug("personal_newAccount", "address", fmt.Sprintf("0x%x", emintKey.PubKey().Address().Bytes())) - addr := common.BytesToAddress(info.GetPubKey().Address().Bytes()) - e.logger.Info("Your new key was generated", "address", addr) - e.logger.Info("Please backup your key file!", "path", os.Getenv("HOME")+"/.ethermintcli/"+name) - e.logger.Info("Please remember your password!") + e.ethAPI.logger.Info("Your new key was generated", "address", addr.String()) + e.ethAPI.logger.Info("Please backup your key file!", "path", os.Getenv("HOME")+"/.ethermintcli/"+name) + e.ethAPI.logger.Info("Please remember your password!") return addr, nil } @@ -171,24 +165,30 @@ func (e *PersonalEthAPI) NewAccount(password string) (common.Address, error) { // the given password for duration seconds. If duration is nil it will use a // default of 300 seconds. It returns an indication if the account was unlocked. // It exports the private key corresponding to the given address from the keyring and stores it in the API's local keys. -func (e *PersonalEthAPI) UnlockAccount(ctx context.Context, addr common.Address, password string, _ *uint64) (bool, error) { - e.logger.Debug("personal_unlockAccount", "address", addr) +func (e *PersonalEthAPI) UnlockAccount(_ context.Context, addr common.Address, password string, _ *uint64) (bool, error) { // nolint: interfacer + e.ethAPI.logger.Debug("personal_unlockAccount", "address", addr.String()) // TODO: use duration - name := "" + var keyInfo keys.Info + for _, info := range e.keyInfos { addressBytes := info.GetPubKey().Address().Bytes() if bytes.Equal(addressBytes, addr[:]) { - name = info.GetName() + keyInfo = info + break } } - if name == "" { - return false, fmt.Errorf("cannot find key with given address") + if keyInfo == nil { + return false, fmt.Errorf("cannot find key with given address %s", addr.String()) } - // TODO: this only works on local keys - privKey, err := e.cliCtx.Keybase.ExportPrivateKeyObject(name, password) + // exporting private key only works on local keys + if keyInfo.GetType() != keys.TypeLocal { + return false, fmt.Errorf("key type must be %s, got %s", keys.TypeLedger.String(), keyInfo.GetType().String()) + } + + privKey, err := e.ethAPI.cliCtx.Keybase.ExportPrivateKeyObject(keyInfo.GetName(), password) if err != nil { return false, err } @@ -198,17 +198,15 @@ func (e *PersonalEthAPI) UnlockAccount(ctx context.Context, addr common.Address, return false, fmt.Errorf("invalid private key type: %T", privKey) } - e.keys = append(e.keys, emintKey) e.ethAPI.keys = append(e.ethAPI.keys, emintKey) - e.logger.Debug("personal_unlockAccount", "address", fmt.Sprintf("0x%x", emintKey.PubKey().Address().Bytes())) - + e.ethAPI.logger.Debug("account unlocked", "address", addr.String()) return true, nil } // SendTransaction will create a transaction from the given arguments and // tries to sign it with the key associated with args.To. If the given password isn't // able to decrypt the key it fails. -func (e *PersonalEthAPI) SendTransaction(ctx context.Context, args params.SendTxArgs, passwd string) (common.Hash, error) { +func (e *PersonalEthAPI) SendTransaction(_ context.Context, args params.SendTxArgs, _ string) (common.Hash, error) { return e.ethAPI.SendTransaction(args) } @@ -221,12 +219,12 @@ func (e *PersonalEthAPI) SendTransaction(ctx context.Context, args params.SendTx // The key used to calculate the signature is decrypted with the given password. // // https://github.com/ethereum/go-ethereum/wiki/Management-APIs#personal_sign -func (e *PersonalEthAPI) Sign(ctx context.Context, data hexutil.Bytes, addr common.Address, passwd string) (hexutil.Bytes, error) { - e.logger.Debug("personal_sign", "data", data, "address", addr) +func (e *PersonalEthAPI) Sign(_ context.Context, data hexutil.Bytes, addr common.Address, _ string) (hexutil.Bytes, error) { + e.ethAPI.logger.Debug("personal_sign", "data", data, "address", addr.String()) - key, ok := checkKeyInKeyring(e.keys, addr) + key, ok := checkKeyInKeyring(e.ethAPI.keys, addr) if !ok { - return nil, fmt.Errorf("cannot find key with given address") + return nil, fmt.Errorf("cannot find key with address %s", addr.String()) } sig, err := crypto.Sign(accounts.TextHash(data), key.ToECDSA()) @@ -248,8 +246,8 @@ func (e *PersonalEthAPI) Sign(ctx context.Context, data hexutil.Bytes, addr comm // the V value must be 27 or 28 for legacy reasons. // // https://github.com/ethereum/go-ethereum/wiki/Management-APIs#personal_ecRecove -func (e *PersonalEthAPI) EcRecover(ctx context.Context, data, sig hexutil.Bytes) (common.Address, error) { - e.logger.Debug("personal_ecRecover", "data", data, "sig", sig) +func (e *PersonalEthAPI) EcRecover(_ context.Context, data, sig hexutil.Bytes) (common.Address, error) { + e.ethAPI.logger.Debug("personal_ecRecover", "data", data, "sig", sig) if len(sig) != crypto.SignatureLength { return common.Address{}, fmt.Errorf("signature must be %d bytes long", crypto.SignatureLength) @@ -259,9 +257,9 @@ func (e *PersonalEthAPI) EcRecover(ctx context.Context, data, sig hexutil.Bytes) } sig[crypto.RecoveryIDOffset] -= 27 // Transform yellow paper V from 27/28 to 0/1 - rpk, err := crypto.SigToPub(accounts.TextHash(data), sig) + pubkey, err := crypto.SigToPub(accounts.TextHash(data), sig) if err != nil { return common.Address{}, err } - return crypto.PubkeyToAddress(*rpk), nil + return crypto.PubkeyToAddress(*pubkey), nil } diff --git a/tests/personal_test.go b/tests/personal_test.go index 914183e2e..94a881ebc 100644 --- a/tests/personal_test.go +++ b/tests/personal_test.go @@ -6,6 +6,7 @@ import ( "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/common/hexutil" + ethcrypto "github.com/ethereum/go-ethereum/crypto" "github.com/stretchr/testify/require" ) @@ -42,6 +43,24 @@ func TestPersonal_Sign(t *testing.T) { // TODO: check that signature is same as with geth, requires importing a key } +func TestPersonal_ImportRawKey(t *testing.T) { + privkey, err := ethcrypto.GenerateKey() + require.NoError(t, err) + + // parse priv key to hex + hexPriv := common.Bytes2Hex(ethcrypto.FromECDSA(privkey)) + rpcRes := call(t, "personal_importRawKey", []string{hexPriv, "password"}) + + var res hexutil.Bytes + err = json.Unmarshal(rpcRes.Result, &res) + require.NoError(t, err) + + addr := ethcrypto.PubkeyToAddress(privkey.PublicKey) + resAddr := common.BytesToAddress(res) + + require.Equal(t, addr.String(), resAddr.String()) +} + func TestPersonal_EcRecover(t *testing.T) { data := hexutil.Bytes{0x88} rpcRes := call(t, "personal_sign", []interface{}{data, hexutil.Bytes(from), ""}) @@ -67,7 +86,7 @@ func TestPersonal_UnlockAccount(t *testing.T) { // try to sign, should be locked _, err = callWithError("personal_sign", []interface{}{hexutil.Bytes{0x88}, addr, ""}) - require.NotNil(t, err) + require.Error(t, err) rpcRes = call(t, "personal_unlockAccount", []interface{}{addr, ""}) var unlocked bool @@ -104,5 +123,5 @@ func TestPersonal_LockAccount(t *testing.T) { // try to sign, should be locked _, err = callWithError("personal_sign", []interface{}{hexutil.Bytes{0x88}, addr, ""}) - require.NotNil(t, err) + require.Error(t, err) }