From b746cd7d312bac22d3f8057e45bd8b6eeefd48a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20Szil=C3=A1gyi?= Date: Fri, 16 Dec 2016 14:18:18 +0200 Subject: [PATCH] accounts, core, crypto, internal: use canonical sigs internally --- accounts/abi/bind/auth.go | 2 +- accounts/account_manager.go | 27 +++++------------- core/types/block_test.go | 2 +- core/types/transaction.go | 47 +------------------------------ core/types/transaction_signing.go | 39 ++++++++----------------- core/types/transaction_test.go | 2 +- crypto/crypto.go | 21 +++----------- crypto/crypto_test.go | 36 ++++------------------- internal/ethapi/api.go | 35 +++++++++++++++++------ 9 files changed, 58 insertions(+), 153 deletions(-) diff --git a/accounts/abi/bind/auth.go b/accounts/abi/bind/auth.go index a20852fca0..dbb235c144 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.SignEthereum(signer.Hash(tx).Bytes(), key) + signature, err := crypto.Sign(signer.Hash(tx).Bytes(), key) if err != nil { return nil, err } diff --git a/accounts/account_manager.go b/accounts/account_manager.go index abe442388e..bd7175aa91 100644 --- a/accounts/account_manager.go +++ b/accounts/account_manager.go @@ -136,13 +136,12 @@ func (am *Manager) DeleteAccount(a Account, passphrase string) error { return err } -// 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. +// Sign calculates a ECDSA signature for the given hash. The produced signature +// is in the canonical secp256k1 format (V = 0 or 1). func (am *Manager) Sign(addr common.Address, hash []byte) ([]byte, error) { am.mu.RLock() defer am.mu.RUnlock() + unlockedKey, found := am.unlocked[addr] if !found { return nil, ErrLocked @@ -150,28 +149,16 @@ func (am *Manager) Sign(addr common.Address, hash []byte) ([]byte, error) { return crypto.Sign(hash, unlockedKey.PrivateKey) } -// 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. +// SignWithPassphrase signs hash if the private key matching the given address +// can be decrypted with the given passphrase. The produced signature is in the +// canonical secp256k1 format (V = 0 or 1). 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 { return nil, err } - defer zeroKey(key.PrivateKey) - return crypto.SignEthereum(hash, key.PrivateKey) + return crypto.Sign(hash, key.PrivateKey) } // Unlock unlocks the given account indefinitely. diff --git a/core/types/block_test.go b/core/types/block_test.go index b95bddcfce..93435ca00c 100644 --- a/core/types/block_test.go +++ b/core/types/block_test.go @@ -53,7 +53,7 @@ func TestBlockEncoding(t *testing.T) { tx1 := NewTransaction(0, common.HexToAddress("095e7baea6a6c7c4c2dfeb977efac326af552d87"), big.NewInt(10), big.NewInt(50000), big.NewInt(10), nil) - tx1, _ = tx1.WithSignature(HomesteadSigner{}, common.Hex2Bytes("9bea4c4daac7c7c52e093e6a4c35dbbcf8856f1af7b059ba20253e70848d094f8a8fae537ce25ed8cb5af9adac3f141af69bd515bd2ba031522df09b97dd72b11b")) + tx1, _ = tx1.WithSignature(HomesteadSigner{}, common.Hex2Bytes("9bea4c4daac7c7c52e093e6a4c35dbbcf8856f1af7b059ba20253e70848d094f8a8fae537ce25ed8cb5af9adac3f141af69bd515bd2ba031522df09b97dd72b100")) fmt.Println(block.Transactions()[0].Hash()) fmt.Println(tx1.data) fmt.Println(tx1.Hash()) diff --git a/core/types/transaction.go b/core/types/transaction.go index f566dc365f..6baab42961 100644 --- a/core/types/transaction.go +++ b/core/types/transaction.go @@ -199,7 +199,7 @@ func (tx *Transaction) UnmarshalJSON(input []byte) error { var V byte if isProtectedV((*big.Int)(dec.V)) { - V = normaliseV(NewEIP155Signer(deriveChainId((*big.Int)(dec.V))), (*big.Int)(dec.V)) + V = byte((new(big.Int).Sub((*big.Int)(dec.V), deriveChainId((*big.Int)(dec.V))).Uint64()) - 35 + 27) } else { V = byte(((*big.Int)(dec.V)).Uint64()) } @@ -272,51 +272,6 @@ func (tx *Transaction) Size() common.StorageSize { return common.StorageSize(c) } -/* -// From returns the address derived from the signature (V, R, S) using secp256k1 -// elliptic curve and an error if it failed deriving or upon an incorrect -// signature. -// -// From Uses the homestead consensus rules to determine whether the signature is -// valid. -// -// From caches the address, allowing it to be used regardless of -// Frontier / Homestead. however, the first time called it runs -// signature validations, so we need two versions. This makes it -// easier to ensure backwards compatibility of things like package rpc -// where eth_getblockbynumber uses tx.From() and needs to work for -// both txs before and after the first homestead block. Signatures -// valid in homestead are a subset of valid ones in Frontier) -func (tx *Transaction) From() (common.Address, error) { - if tx.signer == nil { - return common.Address{}, errNoSigner - } - - if from := tx.from.Load(); from != nil { - return from.(common.Address), nil - } - - pubkey, err := tx.signer.PublicKey(tx) - if err != nil { - return common.Address{}, err - } - var addr common.Address - copy(addr[:], crypto.Keccak256(pubkey[1:])[12:]) - tx.from.Store(addr) - return addr, nil -} - -// SignatureValues returns the ECDSA signature values contained in the transaction. -func (tx *Transaction) SignatureValues() (v byte, r *big.Int, s *big.Int, err error) { - if tx.signer == nil { - return 0, nil, nil,errNoSigner - } - - return normaliseV(tx.signer, tx.data.V), new(big.Int).Set(tx.data.R),new(big.Int).Set(tx.data.S), nil -} - -*/ - // AsMessage returns the transaction as a core.Message. // // AsMessage requires a signer to derive the sender. diff --git a/core/types/transaction_signing.go b/core/types/transaction_signing.go index 7d571643f2..caae15fb5b 100644 --- a/core/types/transaction_signing.go +++ b/core/types/transaction_signing.go @@ -53,7 +53,7 @@ func MakeSigner(config *params.ChainConfig, blockNumber *big.Int) Signer { // SignECDSA signs the transaction using the given signer and private key func SignECDSA(s Signer, tx *Transaction, prv *ecdsa.PrivateKey) (*Transaction, error) { h := s.Hash(tx) - sig, err := crypto.SignEthereum(h[:], prv) + sig, err := crypto.Sign(h[:], prv) if err != nil { return nil, err } @@ -91,11 +91,6 @@ func Sender(signer Signer, tx *Transaction) (common.Address, error) { return addr, nil } -// SignatureValues returns the ECDSA signature values contained in the transaction. -func SignatureValues(signer Signer, tx *Transaction) (v byte, r *big.Int, s *big.Int) { - return normaliseV(signer, tx.data.V), new(big.Int).Set(tx.data.R), new(big.Int).Set(tx.data.S) -} - type Signer interface { // Hash returns the rlp encoded hash for signatures Hash(tx *Transaction) common.Hash @@ -143,7 +138,7 @@ func (s EIP155Signer) PublicKey(tx *Transaction) ([]byte, error) { return nil, ErrInvalidChainId } - V := normaliseV(s, tx.data.V) + V := byte((new(big.Int).Sub(tx.data.V, s.chainIdMul).Uint64()) - 35 + 27) if !crypto.ValidateSignatureValues(V, tx.data.R, tx.data.S, true) { return nil, ErrInvalidSig } @@ -168,7 +163,7 @@ func (s EIP155Signer) PublicKey(tx *Transaction) ([]byte, error) { } // WithSignature returns a new transaction with the given signature. -// This signature needs to be formatted as described in the yellow paper (v+27). +// This signature needs to be in the canonical secp256k1 form (v = 0 or 1). func (s EIP155Signer) WithSignature(tx *Transaction, sig []byte) (*Transaction, error) { if len(sig) != 65 { panic(fmt.Sprintf("wrong size for snature: got %d, want 65", len(sig))) @@ -179,7 +174,7 @@ func (s EIP155Signer) WithSignature(tx *Transaction, sig []byte) (*Transaction, cpy.data.S = new(big.Int).SetBytes(sig[32:64]) cpy.data.V = new(big.Int).SetBytes([]byte{sig[64]}) if s.chainId.BitLen() > 0 { - cpy.data.V = big.NewInt(int64(sig[64] - 27 + 35)) + cpy.data.V = big.NewInt(int64(sig[64] + 35)) cpy.data.V.Add(cpy.data.V, s.chainIdMul) } return cpy, nil @@ -201,7 +196,7 @@ func (s EIP155Signer) Hash(tx *Transaction) common.Hash { func (s EIP155Signer) SigECDSA(tx *Transaction, prv *ecdsa.PrivateKey) (*Transaction, error) { h := s.Hash(tx) - sig, err := crypto.SignEthereum(h[:], prv) + sig, err := crypto.Sign(h[:], prv) if err != nil { return nil, err } @@ -218,7 +213,7 @@ func (s HomesteadSigner) Equal(s2 Signer) bool { } // WithSignature returns a new transaction with the given snature. -// This snature needs to be formatted as described in the yellow paper (v+27). +// This signature needs to be in the canonical secp256k1 form (v = 0 or 1). func (hs HomesteadSigner) WithSignature(tx *Transaction, sig []byte) (*Transaction, error) { if len(sig) != 65 { panic(fmt.Sprintf("wrong size for snature: got %d, want 65", len(sig))) @@ -226,13 +221,13 @@ func (hs HomesteadSigner) WithSignature(tx *Transaction, sig []byte) (*Transacti 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 = new(big.Int).SetBytes([]byte{sig[64]}) + cpy.data.V = new(big.Int).SetBytes([]byte{sig[64] + 27}) return cpy, nil } func (hs HomesteadSigner) SignECDSA(tx *Transaction, prv *ecdsa.PrivateKey) (*Transaction, error) { h := hs.Hash(tx) - sig, err := crypto.SignEthereum(h[:], prv) + sig, err := crypto.Sign(h[:], prv) if err != nil { return nil, err } @@ -274,7 +269,7 @@ func (s FrontierSigner) Equal(s2 Signer) bool { } // WithSignature returns a new transaction with the given snature. -// This snature needs to be formatted as described in the yellow paper (v+27). +// This signature needs to be in the canonical secp256k1 form (v = 0 or 1). func (fs FrontierSigner) WithSignature(tx *Transaction, sig []byte) (*Transaction, error) { if len(sig) != 65 { panic(fmt.Sprintf("wrong size for snature: got %d, want 65", len(sig))) @@ -282,13 +277,13 @@ func (fs FrontierSigner) WithSignature(tx *Transaction, sig []byte) (*Transactio 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 = new(big.Int).SetBytes([]byte{sig[64]}) + cpy.data.V = new(big.Int).SetBytes([]byte{sig[64] + 27}) return cpy, nil } func (fs FrontierSigner) SignECDSA(tx *Transaction, prv *ecdsa.PrivateKey) (*Transaction, error) { h := fs.Hash(tx) - sig, err := crypto.SignEthereum(h[:], prv) + sig, err := crypto.Sign(h[:], prv) if err != nil { return nil, err } @@ -336,18 +331,6 @@ func (fs FrontierSigner) PublicKey(tx *Transaction) ([]byte, error) { return pub, nil } -// normaliseV returns the Ethereum version of the V parameter -func normaliseV(s Signer, v *big.Int) byte { - if s, ok := s.(EIP155Signer); ok { - stdV := v.BitLen() <= 8 && (v.Uint64() == 27 || v.Uint64() == 28) - if s.chainId.BitLen() > 0 && !stdV { - nv := byte((new(big.Int).Sub(v, s.chainIdMul).Uint64()) - 35 + 27) - return nv - } - } - return byte(v.Uint64()) -} - // deriveChainId derives the chain id from the given v parameter func deriveChainId(v *big.Int) *big.Int { if v.BitLen() <= 64 { diff --git a/core/types/transaction_test.go b/core/types/transaction_test.go index ca105566ad..4a38462e31 100644 --- a/core/types/transaction_test.go +++ b/core/types/transaction_test.go @@ -47,7 +47,7 @@ var ( common.FromHex("5544"), ).WithSignature( HomesteadSigner{}, - common.Hex2Bytes("98ff921201554726367d2be8c804a7ff89ccf285ebc57dff8ae4c44b9c19ac4a8887321be575c8095f789dd4c743dfe42c1820f9231f98a962b210e3ac2452a31c"), + common.Hex2Bytes("98ff921201554726367d2be8c804a7ff89ccf285ebc57dff8ae4c44b9c19ac4a8887321be575c8095f789dd4c743dfe42c1820f9231f98a962b210e3ac2452a301"), ) ) diff --git a/crypto/crypto.go b/crypto/crypto.go index e611bd8f40..bf4e5fe054 100644 --- a/crypto/crypto.go +++ b/crypto/crypto.go @@ -199,14 +199,15 @@ func SigToPub(hash, sig []byte) (*ecdsa.PublicKey, error) { } // 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. +// The produced signature is in the canonical secp256k1 format (V = 0 or 1), +// which can be transformed into the proper Ethereum signature accoring to +// the yellow paper within the transaction signer. 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)) @@ -218,20 +219,6 @@ func Sign(data []byte, prv *ecdsa.PrivateKey) (sig []byte, err error) { 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 80c9a9aaee..e3b4a6d09a 100644 --- a/crypto/crypto_test.go +++ b/crypto/crypto_test.go @@ -80,22 +80,15 @@ func Test0Key(t *testing.T) { } } -func testSign(signfn func([]byte, *ecdsa.PrivateKey) ([]byte, error), t *testing.T) { +func TestSign(t *testing.T) { key, _ := HexToECDSA(testPrivHex) addr := common.HexToAddress(testAddrHex) msg := Keccak256([]byte("foo")) - sig, err := signfn(msg, key) + sig, err := Sign(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) @@ -117,34 +110,15 @@ func testSign(signfn func([]byte, *ecdsa.PrivateKey) ([]byte, error), t *testing } } -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 { +func TestInvalidSign(t *testing.T) { + if _, err := Sign(make([]byte, 1), nil); err == nil { t.Errorf("expected sign with hash 1 byte to error") } - - _, err = signfn(make([]byte, 33), nil) - if err == nil { + if _, err := Sign(make([]byte, 33), nil); 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 b84bba5162..561e72b014 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -287,11 +287,19 @@ func signHash(data []byte) []byte { // Sign calculates an Ethereum ECDSA signature for: // keccack256("\x19Ethereum Signed Message:\n" + len(message) + message)) // +// Note, the produced signature conforms to the secp256k1 curve R, S and V values, +// where the V value will be 27 or 28 for legacy reasons. +// // 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, data hexutil.Bytes, addr common.Address, passwd string) (hexutil.Bytes, error) { - return s.b.AccountManager().SignWithPassphrase(addr, passwd, signHash(data)) + signature, err := s.b.AccountManager().SignWithPassphrase(addr, passwd, signHash(data)) + if err != nil { + return nil, err + } + signature[64] += 27 // SignWithPassphrase uses canonical secp256k1 signatures (v = 0 or 1), transform to yellow paper + return signature, nil } // EcRecover returns the address for the account that was used to create the signature. @@ -300,15 +308,19 @@ func (s *PrivateAccountAPI) Sign(ctx context.Context, data hexutil.Bytes, addr c // hash = keccak256("\x19Ethereum Signed Message:\n"${message length}${message}) // addr = ecrecover(hash, signature) // +// Note, the signature must conform to the secp256k1 curve R, S and V values, where +// the V value must be be 27 or 28 for legacy reasons. +// // https://github.com/ethereum/go-ethereum/wiki/Management-APIs#personal_ecRecover func (s *PrivateAccountAPI) EcRecover(ctx context.Context, data, sig hexutil.Bytes) (common.Address, error) { 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 + if sig[64] != 27 && sig[64] != 28 { + return common.Address{}, fmt.Errorf("invalid Ethereum signature (V is not 27 or 28)") } + sig[64] -= 27 // Transform yellow paper signatures to canonical secp256k1 form + rpk, err := crypto.Ecrecover(signHash(data), sig) if err != nil { return common.Address{}, err @@ -964,7 +976,7 @@ func (s *PublicTransactionPoolAPI) GetTransactionReceipt(txHash common.Hash) (ma func (s *PublicTransactionPoolAPI) sign(addr common.Address, tx *types.Transaction) (*types.Transaction, error) { signer := types.MakeSigner(s.b.ChainConfig(), s.b.CurrentBlock().Number()) - signature, err := s.b.AccountManager().SignEthereum(addr, signer.Hash(tx).Bytes()) + signature, err := s.b.AccountManager().Sign(addr, signer.Hash(tx).Bytes()) if err != nil { return nil, err } @@ -1046,11 +1058,10 @@ func (s *PublicTransactionPoolAPI) SendTransaction(ctx context.Context, args Sen } tx := args.toTransaction() signer := types.MakeSigner(s.b.ChainConfig(), s.b.CurrentBlock().Number()) - signature, err := s.b.AccountManager().SignEthereum(args.From, signer.Hash(tx).Bytes()) + signature, err := s.b.AccountManager().Sign(args.From, signer.Hash(tx).Bytes()) if err != nil { return common.Hash{}, err } - return submitTransaction(ctx, s.b, tx, signature) } @@ -1084,11 +1095,19 @@ func (s *PublicTransactionPoolAPI) SendRawTransaction(ctx context.Context, encod // Sign calculates an ECDSA signature for: // keccack256("\x19Ethereum Signed Message:\n" + len(message) + message). // +// Note, the produced signature conforms to the secp256k1 curve R, S and V values, +// where the V value will be 27 or 28 for legacy reasons. +// // 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, data hexutil.Bytes) (hexutil.Bytes, error) { - return s.b.AccountManager().SignEthereum(addr, signHash(data)) + signature, err := s.b.AccountManager().Sign(addr, signHash(data)) + if err == nil { + // Sign uses canonical secp256k1 signatures (v = 0 or 1), transform to yellow paper + signature[64] += 27 + } + return signature, err } // SignTransactionResult represents a RLP encoded signed transaction.