core, crypto: use canonical signature validation

core/types/transaction.go

@@ -199,9 +199,9 @@ func (tx *Transaction) UnmarshalJSON(input []byte) error {
 
 	var V byte
 	if isProtectedV((*big.Int)(dec.V)) {
-		V = byte((new(big.Int).Sub((*big.Int)(dec.V), deriveChainId((*big.Int)(dec.V))).Uint64()) - 35 + 27)
+		V = byte((new(big.Int).Sub((*big.Int)(dec.V), deriveChainId((*big.Int)(dec.V))).Uint64()) - 35)
 	} else {
-		V = byte(((*big.Int)(dec.V)).Uint64())
+		V = byte(((*big.Int)(dec.V)).Uint64() - 27)
 	}
 	if !crypto.ValidateSignatureValues(V, (*big.Int)(dec.R), (*big.Int)(dec.S), false) {
 		return ErrInvalidSig

core/types/transaction_signing.go

@@ -138,17 +138,16 @@ func (s EIP155Signer) PublicKey(tx *Transaction) ([]byte, error) {
 		return nil, ErrInvalidChainId
 	}
 
-	V := byte((new(big.Int).Sub(tx.data.V, s.chainIdMul).Uint64()) - 35 + 27)
+	V := byte(new(big.Int).Sub(tx.data.V, s.chainIdMul).Uint64() - 35)
 	if !crypto.ValidateSignatureValues(V, tx.data.R, tx.data.S, true) {
 		return nil, ErrInvalidSig
 	}
-
 	// encode the signature in uncompressed format
 	R, S := tx.data.R.Bytes(), tx.data.S.Bytes()
 	sig := make([]byte, 65)
 	copy(sig[32-len(R):32], R)
 	copy(sig[64-len(S):64], S)
-	sig[64] = V - 27
+	sig[64] = V
 
 	// recover the public key from the signature
 	hash := s.Hash(tx)
@@ -238,7 +237,7 @@ func (hs HomesteadSigner) PublicKey(tx *Transaction) ([]byte, error) {
 	if tx.data.V.BitLen() > 8 {
 		return nil, ErrInvalidSig
 	}
-	V := byte(tx.data.V.Uint64())
+	V := byte(tx.data.V.Uint64() - 27)
 	if !crypto.ValidateSignatureValues(V, tx.data.R, tx.data.S, true) {
 		return nil, ErrInvalidSig
 	}
@@ -247,7 +246,7 @@ func (hs HomesteadSigner) PublicKey(tx *Transaction) ([]byte, error) {
 	sig := make([]byte, 65)
 	copy(sig[32-len(r):32], r)
 	copy(sig[64-len(s):64], s)
-	sig[64] = V - 27
+	sig[64] = V
 
 	// recover the public key from the snature
 	hash := hs.Hash(tx)
@@ -308,7 +307,7 @@ func (fs FrontierSigner) PublicKey(tx *Transaction) ([]byte, error) {
 		return nil, ErrInvalidSig
 	}
 
-	V := byte(tx.data.V.Uint64())
+	V := byte(tx.data.V.Uint64() - 27)
 	if !crypto.ValidateSignatureValues(V, tx.data.R, tx.data.S, false) {
 		return nil, ErrInvalidSig
 	}
@@ -317,7 +316,7 @@ func (fs FrontierSigner) PublicKey(tx *Transaction) ([]byte, error) {
 	sig := make([]byte, 65)
 	copy(sig[32-len(r):32], r)
 	copy(sig[64-len(s):64], s)
-	sig[64] = V - 27
+	sig[64] = V
 
 	// recover the public key from the snature
 	hash := fs.Hash(tx)

core/vm/contracts.go

@@ -89,21 +89,15 @@ func ecrecoverFunc(in []byte) []byte {
 
 	r := common.BytesToBig(in[64:96])
 	s := common.BytesToBig(in[96:128])
-	// Treat V as a 256bit integer
+	v := in[63] - 27
-	vbig := common.Bytes2Big(in[32:64])
-	v := byte(vbig.Uint64())
 
 	// tighter sig s values in homestead only apply to tx sigs
-	if !crypto.ValidateSignatureValues(v, r, s, false) {
+	if common.Bytes2Big(in[32:63]).BitLen() > 0 || !crypto.ValidateSignatureValues(v, r, s, false) {
 		glog.V(logger.Detail).Infof("ECRECOVER error: v, r or s value invalid")
 		return nil
 	}
-
+	// v needs to be at the end for libsecp256k1
-	// v needs to be at the end and normalized for libsecp256k1
+	pubKey, err := crypto.Ecrecover(in[:32], append(in[64:128], v))
-	vbignormal := new(big.Int).Sub(vbig, big.NewInt(27))
-	vnormal := byte(vbignormal.Uint64())
-	rsv := append(in[64:128], vnormal)
-	pubKey, err := crypto.Ecrecover(in[:32], rsv)
 	// make sure the public key is a valid one
 	if err != nil {
 		glog.V(logger.Detail).Infoln("ECRECOVER error: ", err)

crypto/crypto.go

@@ -167,25 +167,20 @@ 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 the canonical secp256k1
+// value of 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 r.Cmp(secp256k1.N) < 0 && s.Cmp(secp256k1.N) < 0 && (v == 0 || v == 1)
-		return false
-	}
-	if r.Cmp(secp256k1.N) < 0 && (vint == 27 || vint == 28) {
-		return true
-	} else {
-		return false
-	}
 }
 
 func SigToPub(hash, sig []byte) (*ecdsa.PublicKey, error) {

crypto/crypto_test.go

@@ -181,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, 0, one, one)
-	check(true, 28, one, one)
+	check(true, 1, one, one)
 	// incorrect v, correct r,s,
-	check(false, 30, one, one)
+	check(false, 2, one, one)
-	check(false, 26, 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, 1, zero, zero)
-	check(false, 27, zero, zero)
+	check(false, 1, zero, one)
-	check(false, 27, zero, one)
+	check(false, 1, one, zero)
-	check(false, 27, one, zero)
-
-	check(false, 28, zero, zero)
-	check(false, 28, zero, one)
-	check(false, 28, 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, 0, secp256k1.N, secp256k1nMinus1)
-	check(false, 27, secp256k1nMinus1, secp256k1.N)
+	check(false, 0, secp256k1nMinus1, secp256k1.N)
-	check(false, 27, secp256k1.N, 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, 0, minusOne, one)
-	check(false, 27, one, minusOne)
+	check(false, 0, one, minusOne)
 }
 
 func checkhash(t *testing.T, name string, f func([]byte) []byte, msg, exp []byte) {