From 85bfe8382d06928d2bb2aa14d7957ab0e9dc879f Mon Sep 17 00:00:00 2001 From: Bas van Kervel Date: Thu, 25 Aug 2016 09:55:31 +0200 Subject: [PATCH] accept message and hash before sign --- accounts/abi/bind/auth.go | 2 +- accounts/account_manager.go | 25 ++++++++++++++++++----- common/bytes.go | 2 +- common/registrar/ethreg/api.go | 2 +- console/bridge.go | 10 ++++----- core/types/block_test.go | 2 +- core/types/transaction.go | 6 ++++-- core/types/transaction_test.go | 2 +- crypto/crypto.go | 20 ++++++++++++++++++ crypto/crypto_test.go | 37 +++++++++++++++++++++++++++------- internal/ethapi/api.go | 32 ++++++++++++++++++----------- internal/web3ext/web3ext.go | 2 +- 12 files changed, 105 insertions(+), 37 deletions(-) diff --git a/accounts/abi/bind/auth.go b/accounts/abi/bind/auth.go index 2cf22768cf..cd6adc7462 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 bfb7556d6a..c8601c3c05 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 4fb016a976..b9fb3b2da6 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 6dd0ef46f3..10050a5457 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 14509ea428..24a777d78d 100644 --- a/console/bridge.go +++ b/console/bridge.go @@ -131,19 +131,19 @@ func (b *bridge) UnlockAccount(call otto.FunctionCall) (response otto.Value) { // jeth.sign) with it to actually execute the RPC call. func (b *bridge) Sign(call otto.FunctionCall) (response otto.Value) { var ( - hash = call.Argument(0) + message = call.Argument(0) account = call.Argument(1) passwd = call.Argument(2) ) - if !hash.IsString() { - throwJSException("first argument must be the hash to sign") + 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, otherwise ensure it's a string + // 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 { @@ -157,7 +157,7 @@ func (b *bridge) Sign(call otto.FunctionCall) (response otto.Value) { } // Send the request to the backend and return - val, err := call.Otto.Call("jeth.sign", nil, hash, account, passwd) + val, err := call.Otto.Call("jeth.sign", nil, message, account, passwd) if err != nil { throwJSException(err.Error()) } diff --git a/core/types/block_test.go b/core/types/block_test.go index cdd8431f4d..ac7f17c0d8 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 c71c98aa7d..f811bdb693 100644 --- a/core/types/transaction.go +++ b/core/types/transaction.go @@ -244,6 +244,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))) @@ -251,13 +253,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 62420e71f8..ecb7d55b4b 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 85f0970956..aa4fa45a2c 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" an Ethereum signature adjust. func Ecrecover(hash, sig []byte) ([]byte, error) { return secp256k1.RecoverPubkey(hash, sig) } @@ -192,6 +198,10 @@ func SigToPub(hash, sig []byte) (*ecdsa.PublicKey, error) { return &ecdsa.PublicKey{Curve: secp256k1.S256(), X: x, Y: y}, nil } +// Sign calculates an ECDSA signature. +// Note: the 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(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)) @@ -203,6 +213,16 @@ func Sign(hash []byte, prv *ecdsa.PrivateKey) (sig []byte, err error) { return } +// SignEthereum calculates an Ethereum ECDSA signature. +func SignEthereum(hash []byte, prv *ecdsa.PrivateKey) ([]byte, error) { + sig, err := Sign(hash, 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 58b29da490..80c9a9aaee 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 aa3546c357..ac78cd1362 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -298,14 +298,18 @@ func (s *PrivateAccountAPI) SendTransaction(ctx context.Context, args SendTxArgs return submitTransaction(ctx, s.b, tx, signature) } -// Sign decrypts the private key associated with the given address with the given password. -// If the key was successful decrypted the given hash is signed and the signature is returned. -func (s *PrivateAccountAPI) Sign(ctx context.Context, hash common.Hash, addr common.Address, passwd string) (string, error) { - signature, err := s.b.AccountManager().SignWithPassphrase(addr, passwd, hash.Bytes()) +// Sign calculates an ECDSA signature from keccack226(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) { + // always hash, this prevents choosen plaintext attacks that can extract the key + hash := crypto.Keccak256(common.FromHex(message)) + signature, err := s.b.AccountManager().SignWithPassphrase(addr, passwd, hash) if err != nil { return "0x", err } - return "0x" + hex.EncodeToString(signature), nil + return common.ToHex(signature), nil } // SignAndSendTransaction was renamed to SendTransaction. This method is deprecated @@ -898,7 +902,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 } @@ -980,7 +984,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 } @@ -1014,11 +1018,15 @@ 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 from keccack226(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) { + // always hash, this prevents choosen plaintext attacks that can extract the key + hash := crypto.Keccak256(common.FromHex(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 db276da5e1..f7d5e67c66 100644 --- a/internal/web3ext/web3ext.go +++ b/internal/web3ext/web3ext.go @@ -436,7 +436,7 @@ web3._extend({ inputFormatter: [null, web3._extend.formatters.inputAddressFormatter, null] }) ] -}); +}) ` const RPC_JS = `