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<length of message>

  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.

2 files changed, 62 insertions, 10 deletions

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 TestSign(t *testing.T) {
+	testSign(Sign, t)
 }
 
-func TestInvalidSign(t *testing.T) {
-	_, err := Sign(make([]byte, 1), nil)
+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)