accounts, core, crypto, internal: use normalised V during signature handling (#3455)

To address increasing complexity in code that handles signatures, this PR
discards all notion of "different" signature types at the library level. Both
the crypto and accounts package is reduced to only be able to produce plain
canonical secp256k1 signatures. This makes the crpyto APIs much cleaner,
simpler and harder to abuse.

2 files changed, 29 insertions, 76 deletions

diff --git a/crypto/crypto.go b/crypto/crypto.go
index e611bd8f4..f1a4b774c 100644
--- a/crypto/crypto.go
+++ b/crypto/crypto.go
@@ -167,25 +167,19 @@ func GenerateKey() (*ecdsa.PrivateKey, error) {
 	return ecdsa.GenerateKey(secp256k1.S256(), rand.Reader)
 }
 
+// ValidateSignatureValues verifies whether the signature values are valid with
+// the given chain rules. The v value is assumed to be either 0 or 1.
 func ValidateSignatureValues(v byte, r, s *big.Int, homestead bool) bool {
 	if r.Cmp(common.Big1) < 0 || s.Cmp(common.Big1) < 0 {
 		return false
 	}
-	vint := uint32(v)
 	// reject upper range of s values (ECDSA malleability)
 	// see discussion in secp256k1/libsecp256k1/include/secp256k1.h
 	if homestead && s.Cmp(secp256k1.HalfN) > 0 {
 		return false
 	}
 	// Frontier: allow s to be in full N range
-	if s.Cmp(secp256k1.N) >= 0 {
-		return false
-	}
-	if r.Cmp(secp256k1.N) < 0 && (vint == 27 || vint == 28) {
-		return true
-	} else {
-		return false
-	}
+	return r.Cmp(secp256k1.N) < 0 && s.Cmp(secp256k1.N) < 0 && (v == 0 || v == 1)
 }
 
 func SigToPub(hash, sig []byte) (*ecdsa.PublicKey, error) {
@@ -199,14 +193,13 @@ 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 [R || S || V] format where V is 0 or 1.
 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 +211,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 80c9a9aae..86a582306 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)
@@ -207,38 +181,38 @@ func TestValidateSignatureValues(t *testing.T) {
 	secp256k1nMinus1 := new(big.Int).Sub(secp256k1.N, common.Big1)
 
 	// correct v,r,s
-	check(true, 27, one, one)
-	check(true, 28, one, one)
+	check(true, 0, one, one)
+	check(true, 1, one, one)
 	// incorrect v, correct r,s,
-	check(false, 30, one, one)
-	check(false, 26, one, one)
+	check(false, 2, one, one)
+	check(false, 3, one, one)
 
 	// incorrect v, combinations of incorrect/correct r,s at lower limit
+	check(false, 2, zero, zero)
+	check(false, 2, zero, one)
+	check(false, 2, one, zero)
+	check(false, 2, one, one)
+
+	// correct v for any combination of incorrect r,s
 	check(false, 0, zero, zero)
 	check(false, 0, zero, one)
 	check(false, 0, one, zero)
-	check(false, 0, one, one)
-
-	// correct v for any combination of incorrect r,s
-	check(false, 27, zero, zero)
-	check(false, 27, zero, one)
-	check(false, 27, one, zero)
 
-	check(false, 28, zero, zero)
-	check(false, 28, zero, one)
-	check(false, 28, one, zero)
+	check(false, 1, zero, zero)
+	check(false, 1, zero, one)
+	check(false, 1, one, zero)
 
 	// correct sig with max r,s
-	check(true, 27, secp256k1nMinus1, secp256k1nMinus1)
+	check(true, 0, secp256k1nMinus1, secp256k1nMinus1)
 	// correct v, combinations of incorrect r,s at upper limit
-	check(false, 27, secp256k1.N, secp256k1nMinus1)
-	check(false, 27, secp256k1nMinus1, secp256k1.N)
-	check(false, 27, secp256k1.N, secp256k1.N)
+	check(false, 0, secp256k1.N, secp256k1nMinus1)
+	check(false, 0, secp256k1nMinus1, secp256k1.N)
+	check(false, 0, secp256k1.N, secp256k1.N)
 
 	// current callers ensures r,s cannot be negative, but let's test for that too
 	// as crypto package could be used stand-alone
-	check(false, 27, minusOne, one)
-	check(false, 27, one, minusOne)
+	check(false, 0, minusOne, one)
+	check(false, 0, one, minusOne)
 }
 
 func checkhash(t *testing.T, name string, f func([]byte) []byte, msg, exp []byte) {