From b59c8399fbe42390a3d41e945d03b1f21c1a9b8d Mon Sep 17 00:00:00 2001 From: bas-vk Date: Fri, 28 Oct 2016 21:25:49 +0200 Subject: [PATCH] internal/ethapi: add personal_sign and fix eth_sign to hash message (#2940) This commit includes several API changes: - The behavior of eth_sign is changed. It now accepts an arbitrary message, prepends the well-known string \x19Ethereum Signed Message:\n hashes the result using keccak256 and calculates the signature of the hash. This breaks backwards compatability! - personal_sign(hash, address [, password]) is added. It has the same semantics as eth_sign but also accepts a password. The private key used to sign the hash is temporarily unlocked in the scope of the request. - personal_recover(message, signature) is added and returns the address for the account that created a signature. --- accounts/abi/bind/auth.go | 2 +- accounts/account_manager.go | 25 ++++++++--- common/bytes.go | 2 +- common/registrar/ethreg/api.go | 2 +- console/bridge.go | 38 ++++++++++++++++ console/console.go | 11 +++-- core/types/block_test.go | 2 +- core/types/transaction.go | 6 ++- core/types/transaction_test.go | 2 +- crypto/crypto.go | 37 ++++++++++++++-- crypto/crypto_test.go | 37 +++++++++++++--- internal/ethapi/api.go | 79 +++++++++++++++++++++++++++++++--- internal/web3ext/web3ext.go | 13 +++++- 13 files changed, 221 insertions(+), 35 deletions(-) diff --git a/accounts/abi/bind/auth.go b/accounts/abi/bind/auth.go index 2cf22768c..cd6adc746 100644 --- a/accounts/abi/bind/auth.go +++ b/accounts/abi/bind/auth.go @@ -52,7 +52,7 @@ func NewKeyedTransactor(key *ecdsa.PrivateKey) *TransactOpts { if address != keyAddr { return nil, errors.New("not authorized to sign this account") } - signature, err := crypto.Sign(tx.SigHash().Bytes(), key) + signature, err := crypto.SignEthereum(tx.SigHash().Bytes(), key) if err != nil { return nil, err } diff --git a/accounts/account_manager.go b/accounts/account_manager.go index bfb7556d6..c8601c3c0 100644 --- a/accounts/account_manager.go +++ b/accounts/account_manager.go @@ -136,8 +136,11 @@ func (am *Manager) DeleteAccount(a Account, passphrase string) error { return err } -// Sign signs hash with an unlocked private key matching the given address. -func (am *Manager) Sign(addr common.Address, hash []byte) (signature []byte, err error) { +// Sign calculates a ECDSA signature for the given hash. +// Note, Ethereum signatures have a particular format as described in the +// yellow paper. Use the SignEthereum function to calculate a signature +// in Ethereum format. +func (am *Manager) Sign(addr common.Address, hash []byte) ([]byte, error) { am.mu.RLock() defer am.mu.RUnlock() unlockedKey, found := am.unlocked[addr] @@ -147,8 +150,20 @@ func (am *Manager) Sign(addr common.Address, hash []byte) (signature []byte, err return crypto.Sign(hash, unlockedKey.PrivateKey) } -// SignWithPassphrase signs hash if the private key matching the given address can be -// decrypted with the given passphrase. +// SignEthereum calculates a ECDSA signature for the given hash. +// The signature has the format as described in the Ethereum yellow paper. +func (am *Manager) SignEthereum(addr common.Address, hash []byte) ([]byte, error) { + am.mu.RLock() + defer am.mu.RUnlock() + unlockedKey, found := am.unlocked[addr] + if !found { + return nil, ErrLocked + } + return crypto.SignEthereum(hash, unlockedKey.PrivateKey) +} + +// SignWithPassphrase signs hash if the private key matching the given +// address can be decrypted with the given passphrase. func (am *Manager) SignWithPassphrase(addr common.Address, passphrase string, hash []byte) (signature []byte, err error) { _, key, err := am.getDecryptedKey(Account{Address: addr}, passphrase) if err != nil { @@ -156,7 +171,7 @@ func (am *Manager) SignWithPassphrase(addr common.Address, passphrase string, ha } defer zeroKey(key.PrivateKey) - return crypto.Sign(hash, key.PrivateKey) + return crypto.SignEthereum(hash, key.PrivateKey) } // Unlock unlocks the given account indefinitely. diff --git a/common/bytes.go b/common/bytes.go index 4fb016a97..b9fb3b2da 100644 --- a/common/bytes.go +++ b/common/bytes.go @@ -37,7 +37,7 @@ func ToHex(b []byte) string { func FromHex(s string) []byte { if len(s) > 1 { - if s[0:2] == "0x" { + if s[0:2] == "0x" || s[0:2] == "0X" { s = s[2:] } if len(s)%2 == 1 { diff --git a/common/registrar/ethreg/api.go b/common/registrar/ethreg/api.go index 6dd0ef46f..10050a545 100644 --- a/common/registrar/ethreg/api.go +++ b/common/registrar/ethreg/api.go @@ -257,7 +257,7 @@ func (be *registryAPIBackend) Transact(fromStr, toStr, nonceStr, valueStr, gasSt tx = types.NewTransaction(nonce, to, value, gas, price, data) } - signature, err := be.am.Sign(from, tx.SigHash().Bytes()) + signature, err := be.am.SignEthereum(from, tx.SigHash().Bytes()) if err != nil { return "", err } diff --git a/console/bridge.go b/console/bridge.go index 22ed7192b..24a777d78 100644 --- a/console/bridge.go +++ b/console/bridge.go @@ -126,6 +126,44 @@ func (b *bridge) UnlockAccount(call otto.FunctionCall) (response otto.Value) { return val } +// Sign is a wrapper around the personal.sign RPC method that uses a non-echoing password +// prompt to aquire the passphrase and executes the original RPC method (saved in +// jeth.sign) with it to actually execute the RPC call. +func (b *bridge) Sign(call otto.FunctionCall) (response otto.Value) { + var ( + message = call.Argument(0) + account = call.Argument(1) + passwd = call.Argument(2) + ) + + if !message.IsString() { + throwJSException("first argument must be the message to sign") + } + if !account.IsString() { + throwJSException("second argument must be the account to sign with") + } + + // if the password is not given or null ask the user and ensure password is a string + if passwd.IsUndefined() || passwd.IsNull() { + fmt.Fprintf(b.printer, "Give password for account %s\n", account) + if input, err := b.prompter.PromptPassword("Passphrase: "); err != nil { + throwJSException(err.Error()) + } else { + passwd, _ = otto.ToValue(input) + } + } + if !passwd.IsString() { + throwJSException("third argument must be the password to unlock the account") + } + + // Send the request to the backend and return + val, err := call.Otto.Call("jeth.sign", nil, message, account, passwd) + if err != nil { + throwJSException(err.Error()) + } + return val +} + // Sleep will block the console for the specified number of seconds. func (b *bridge) Sleep(call otto.FunctionCall) (response otto.Value) { if call.Argument(0).IsNumber() { diff --git a/console/console.go b/console/console.go index f224f0c2e..3cde9b8f5 100644 --- a/console/console.go +++ b/console/console.go @@ -156,10 +156,9 @@ func (c *Console) init(preload []string) error { if err != nil { return err } - // Override the unlockAccount and newAccount methods since these require user interaction. - // Assign the jeth.unlockAccount and jeth.newAccount in the Console the original web3 callbacks. - // These will be called by the jeth.* methods after they got the password from the user and send - // the original web3 request to the backend. + // Override the unlockAccount, newAccount and sign methods since these require user interaction. + // Assign these method in the Console the original web3 callbacks. These will be called by the jeth.* + // methods after they got the password from the user and send the original web3 request to the backend. if obj := personal.Object(); obj != nil { // make sure the personal api is enabled over the interface if _, err = c.jsre.Run(`jeth.unlockAccount = personal.unlockAccount;`); err != nil { return fmt.Errorf("personal.unlockAccount: %v", err) @@ -167,8 +166,12 @@ func (c *Console) init(preload []string) error { if _, err = c.jsre.Run(`jeth.newAccount = personal.newAccount;`); err != nil { return fmt.Errorf("personal.newAccount: %v", err) } + if _, err = c.jsre.Run(`jeth.sign = personal.sign;`); err != nil { + return fmt.Errorf("personal.sign: %v", err) + } obj.Set("unlockAccount", bridge.UnlockAccount) obj.Set("newAccount", bridge.NewAccount) + obj.Set("sign", bridge.Sign) } } // The admin.sleep and admin.sleepBlocks are offered by the console and not by the RPC layer. diff --git a/core/types/block_test.go b/core/types/block_test.go index cdd8431f4..ac7f17c0d 100644 --- a/core/types/block_test.go +++ b/core/types/block_test.go @@ -51,7 +51,7 @@ func TestBlockEncoding(t *testing.T) { check("Size", block.Size(), common.StorageSize(len(blockEnc))) tx1 := NewTransaction(0, common.HexToAddress("095e7baea6a6c7c4c2dfeb977efac326af552d87"), big.NewInt(10), big.NewInt(50000), big.NewInt(10), nil) - tx1, _ = tx1.WithSignature(common.Hex2Bytes("9bea4c4daac7c7c52e093e6a4c35dbbcf8856f1af7b059ba20253e70848d094f8a8fae537ce25ed8cb5af9adac3f141af69bd515bd2ba031522df09b97dd72b100")) + tx1, _ = tx1.WithSignature(common.Hex2Bytes("9bea4c4daac7c7c52e093e6a4c35dbbcf8856f1af7b059ba20253e70848d094f8a8fae537ce25ed8cb5af9adac3f141af69bd515bd2ba031522df09b97dd72b11b")) check("len(Transactions)", len(block.Transactions()), 1) check("Transactions[0].Hash", block.Transactions()[0].Hash(), tx1.Hash()) diff --git a/core/types/transaction.go b/core/types/transaction.go index f0512ae7e..ceea4f959 100644 --- a/core/types/transaction.go +++ b/core/types/transaction.go @@ -318,6 +318,8 @@ func (tx *Transaction) publicKey(homestead bool) ([]byte, error) { return pub, nil } +// WithSignature returns a new transaction with the given signature. +// This signature needs to be formatted as described in the yellow paper (v+27). func (tx *Transaction) WithSignature(sig []byte) (*Transaction, error) { if len(sig) != 65 { panic(fmt.Sprintf("wrong size for signature: got %d, want 65", len(sig))) @@ -325,13 +327,13 @@ func (tx *Transaction) WithSignature(sig []byte) (*Transaction, error) { cpy := &Transaction{data: tx.data} cpy.data.R = new(big.Int).SetBytes(sig[:32]) cpy.data.S = new(big.Int).SetBytes(sig[32:64]) - cpy.data.V = sig[64] + 27 + cpy.data.V = sig[64] return cpy, nil } func (tx *Transaction) SignECDSA(prv *ecdsa.PrivateKey) (*Transaction, error) { h := tx.SigHash() - sig, err := crypto.Sign(h[:], prv) + sig, err := crypto.SignEthereum(h[:], prv) if err != nil { return nil, err } diff --git a/core/types/transaction_test.go b/core/types/transaction_test.go index 8b0b02c3e..98a78d221 100644 --- a/core/types/transaction_test.go +++ b/core/types/transaction_test.go @@ -46,7 +46,7 @@ var ( big.NewInt(1), common.FromHex("5544"), ).WithSignature( - common.Hex2Bytes("98ff921201554726367d2be8c804a7ff89ccf285ebc57dff8ae4c44b9c19ac4a8887321be575c8095f789dd4c743dfe42c1820f9231f98a962b210e3ac2452a301"), + common.Hex2Bytes("98ff921201554726367d2be8c804a7ff89ccf285ebc57dff8ae4c44b9c19ac4a8887321be575c8095f789dd4c743dfe42c1820f9231f98a962b210e3ac2452a31c"), ) ) diff --git a/crypto/crypto.go b/crypto/crypto.go index 85f097095..e611bd8f4 100644 --- a/crypto/crypto.go +++ b/crypto/crypto.go @@ -78,6 +78,12 @@ func Ripemd160(data []byte) []byte { return ripemd.Sum(nil) } +// Ecrecover returns the public key for the private key that was used to +// calculate the signature. +// +// Note: secp256k1 expects the recover id to be either 0, 1. Ethereum +// signatures have a recover id with an offset of 27. Callers must take +// this into account and if "recovering" from an Ethereum signature adjust. func Ecrecover(hash, sig []byte) ([]byte, error) { return secp256k1.RecoverPubkey(hash, sig) } @@ -192,17 +198,40 @@ func SigToPub(hash, sig []byte) (*ecdsa.PublicKey, error) { return &ecdsa.PublicKey{Curve: secp256k1.S256(), X: x, Y: y}, nil } -func Sign(hash []byte, prv *ecdsa.PrivateKey) (sig []byte, err error) { - if len(hash) != 32 { - return nil, fmt.Errorf("hash is required to be exactly 32 bytes (%d)", len(hash)) +// Sign calculates an ECDSA signature. +// This function is susceptible to choosen plaintext attacks that can leak +// information about the private key that is used for signing. Callers must +// be aware that the given hash cannot be choosen by an adversery. Common +// solution is to hash any input before calculating the signature. +// +// Note: the calculated signature is not Ethereum compliant. The yellow paper +// dictates Ethereum singature to have a V value with and offset of 27 v in [27,28]. +// Use SignEthereum to get an Ethereum compliant signature. +func Sign(data []byte, prv *ecdsa.PrivateKey) (sig []byte, err error) { + if len(data) != 32 { + return nil, fmt.Errorf("hash is required to be exactly 32 bytes (%d)", len(data)) } seckey := common.LeftPadBytes(prv.D.Bytes(), prv.Params().BitSize/8) defer zeroBytes(seckey) - sig, err = secp256k1.Sign(hash, seckey) + sig, err = secp256k1.Sign(data, seckey) return } +// SignEthereum calculates an Ethereum ECDSA signature. +// This function is susceptible to choosen plaintext attacks that can leak +// information about the private key that is used for signing. Callers must +// be aware that the given hash cannot be freely choosen by an adversery. +// Common solution is to hash the message before calculating the signature. +func SignEthereum(data []byte, prv *ecdsa.PrivateKey) ([]byte, error) { + sig, err := Sign(data, prv) + if err != nil { + return nil, err + } + sig[64] += 27 // as described in the yellow paper + return sig, err +} + func Encrypt(pub *ecdsa.PublicKey, message []byte) ([]byte, error) { return ecies.Encrypt(rand.Reader, ecies.ImportECDSAPublic(pub), message, nil, nil) } diff --git a/crypto/crypto_test.go b/crypto/crypto_test.go index 58b29da49..80c9a9aae 100644 --- a/crypto/crypto_test.go +++ b/crypto/crypto_test.go @@ -80,20 +80,28 @@ func Test0Key(t *testing.T) { } } -func TestSign(t *testing.T) { +func testSign(signfn func([]byte, *ecdsa.PrivateKey) ([]byte, error), t *testing.T) { key, _ := HexToECDSA(testPrivHex) addr := common.HexToAddress(testAddrHex) msg := Keccak256([]byte("foo")) - sig, err := Sign(msg, key) + sig, err := signfn(msg, key) if err != nil { t.Errorf("Sign error: %s", err) } + + // signfn can return a recover id of either [0,1] or [27,28]. + // In the latter case its an Ethereum signature, adjust recover id. + if sig[64] == 27 || sig[64] == 28 { + sig[64] -= 27 + } + recoveredPub, err := Ecrecover(msg, sig) if err != nil { t.Errorf("ECRecover error: %s", err) } - recoveredAddr := PubkeyToAddress(*ToECDSAPub(recoveredPub)) + pubKey := ToECDSAPub(recoveredPub) + recoveredAddr := PubkeyToAddress(*pubKey) if addr != recoveredAddr { t.Errorf("Address mismatch: want: %x have: %x", addr, recoveredAddr) } @@ -107,21 +115,36 @@ func TestSign(t *testing.T) { if addr != recoveredAddr2 { t.Errorf("Address mismatch: want: %x have: %x", addr, recoveredAddr2) } - } -func TestInvalidSign(t *testing.T) { - _, err := Sign(make([]byte, 1), nil) +func TestSign(t *testing.T) { + testSign(Sign, t) +} + +func TestSignEthereum(t *testing.T) { + testSign(SignEthereum, t) +} + +func testInvalidSign(signfn func([]byte, *ecdsa.PrivateKey) ([]byte, error), t *testing.T) { + _, err := signfn(make([]byte, 1), nil) if err == nil { t.Errorf("expected sign with hash 1 byte to error") } - _, err = Sign(make([]byte, 33), nil) + _, err = signfn(make([]byte, 33), nil) if err == nil { t.Errorf("expected sign with hash 33 byte to error") } } +func TestInvalidSign(t *testing.T) { + testInvalidSign(Sign, t) +} + +func TestInvalidSignEthereum(t *testing.T) { + testInvalidSign(SignEthereum, t) +} + func TestNewContractAddress(t *testing.T) { key, _ := HexToECDSA(testPrivHex) addr := common.HexToAddress(testAddrHex) diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index b9275518e..0e8e905aa 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -287,6 +287,66 @@ func (s *PrivateAccountAPI) SendTransaction(ctx context.Context, args SendTxArgs return submitTransaction(ctx, s.b, tx, signature) } +// signHash is a helper function that calculates a hash for the given message that can be +// safely used to calculate a signature from. The hash is calulcated with: +// keccak256("\x19Ethereum Signed Message:\n"${message length}${message}). +func signHash(message string) []byte { + data := common.FromHex(message) + // Give context to the signed message. This prevents an adversery to sign a tx. + // It has no cryptographic purpose. + msg := fmt.Sprintf("\x19Ethereum Signed Message:\n%d%s", len(data), data) + // Always hash, this prevents choosen plaintext attacks that can extract key information + return crypto.Keccak256([]byte(msg)) +} + +// Sign calculates an Ethereum ECDSA signature for: +// keccack256("\x19Ethereum Signed Message:\n" + len(message) + message)) +// +// 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 (s *PrivateAccountAPI) Sign(ctx context.Context, message string, addr common.Address, passwd string) (string, error) { + hash := signHash(message) + signature, err := s.b.AccountManager().SignWithPassphrase(addr, passwd, hash) + if err != nil { + return "0x", err + } + return common.ToHex(signature), nil +} + +// EcRecover returns the address for the account that was used to create the signature. +// Note, this function is compatible with eth_sign and personal_sign. As such it recovers +// the address of: +// hash = keccak256("\x19Ethereum Signed Message:\n"${message length}${message}) +// addr = ecrecover(hash, signature) +// +// https://github.com/ethereum/go-ethereum/wiki/Management-APIs#personal_ecRecover +func (s *PrivateAccountAPI) EcRecover(ctx context.Context, message string, signature string) (common.Address, error) { + var ( + hash = signHash(message) + sig = common.FromHex(signature) + ) + + if len(sig) != 65 { + return common.Address{}, fmt.Errorf("signature must be 65 bytes long") + } + + // see crypto.Ecrecover description + if sig[64] == 27 || sig[64] == 28 { + sig[64] -= 27 + } + + rpk, err := crypto.Ecrecover(hash, sig) + if err != nil { + return common.Address{}, err + } + + pubKey := crypto.ToECDSAPub(rpk) + recoveredAddr := crypto.PubkeyToAddress(*pubKey) + + return recoveredAddr, nil +} + // SignAndSendTransaction was renamed to SendTransaction. This method is deprecated // and will be removed in the future. It primary goal is to give clients time to update. func (s *PrivateAccountAPI) SignAndSendTransaction(ctx context.Context, args SendTxArgs, passwd string) (common.Hash, error) { @@ -929,7 +989,7 @@ func (s *PublicTransactionPoolAPI) GetTransactionReceipt(txHash common.Hash) (ma // sign is a helper function that signs a transaction with the private key of the given address. func (s *PublicTransactionPoolAPI) sign(addr common.Address, tx *types.Transaction) (*types.Transaction, error) { - signature, err := s.b.AccountManager().Sign(addr, tx.SigHash().Bytes()) + signature, err := s.b.AccountManager().SignEthereum(addr, tx.SigHash().Bytes()) if err != nil { return nil, err } @@ -1011,7 +1071,7 @@ func (s *PublicTransactionPoolAPI) SendTransaction(ctx context.Context, args Sen tx = types.NewTransaction(args.Nonce.Uint64(), *args.To, args.Value.BigInt(), args.Gas.BigInt(), args.GasPrice.BigInt(), common.FromHex(args.Data)) } - signature, err := s.b.AccountManager().Sign(args.From, tx.SigHash().Bytes()) + signature, err := s.b.AccountManager().SignEthereum(args.From, tx.SigHash().Bytes()) if err != nil { return common.Hash{}, err } @@ -1045,11 +1105,16 @@ func (s *PublicTransactionPoolAPI) SendRawTransaction(ctx context.Context, encod return tx.Hash().Hex(), nil } -// Sign signs the given hash using the key that matches the address. The key must be -// unlocked in order to sign the hash. -func (s *PublicTransactionPoolAPI) Sign(addr common.Address, hash common.Hash) (string, error) { - signature, error := s.b.AccountManager().Sign(addr, hash[:]) - return common.ToHex(signature), error +// Sign calculates an ECDSA signature for: +// keccack256("\x19Ethereum Signed Message:\n" + len(message) + message). +// +// The account associated with addr must be unlocked. +// +// https://github.com/ethereum/wiki/wiki/JSON-RPC#eth_sign +func (s *PublicTransactionPoolAPI) Sign(addr common.Address, message string) (string, error) { + hash := signHash(message) + signature, err := s.b.AccountManager().SignEthereum(addr, hash) + return common.ToHex(signature), err } // SignTransactionArgs represents the arguments to sign a transaction. diff --git a/internal/web3ext/web3ext.go b/internal/web3ext/web3ext.go index f935cadca..c8a0cac8c 100644 --- a/internal/web3ext/web3ext.go +++ b/internal/web3ext/web3ext.go @@ -587,9 +587,20 @@ web3._extend({ call: 'personal_sendTransaction', params: 2, inputFormatter: [web3._extend.formatters.inputTransactionFormatter, null] + }), + new web3._extend.Method({ + name: 'sign', + call: 'personal_sign', + params: 3, + inputFormatter: [null, web3._extend.formatters.inputAddressFormatter, null] + }), + new web3._extend.Method({ + name: 'ecRecover', + call: 'personal_ecRecover', + params: 2 }) ] -}); +}) ` const RPC_JS = `