From 3340b565931e04400029da2ef2a6ac811d7fbad5 Mon Sep 17 00:00:00 2001 From: Gustav Simonsson Date: Mon, 21 Sep 2015 15:48:15 +0200 Subject: [PATCH] crypto: correct sig validation, add more unit tests --- crypto/crypto.go | 15 +--- crypto/crypto_test.go | 181 ++++++++++++++++++++++++++++++++++++++---- 2 files changed, 169 insertions(+), 27 deletions(-) diff --git a/crypto/crypto.go b/crypto/crypto.go index b3a8d730b4..272050106d 100644 --- a/crypto/crypto.go +++ b/crypto/crypto.go @@ -172,10 +172,10 @@ func GenerateKey() (*ecdsa.PrivateKey, error) { } func ValidateSignatureValues(v byte, r, s *big.Int) bool { - vint := uint32(v) - if r.Cmp(common.Big0) == 0 || s.Cmp(common.Big0) == 0 { + if r.Cmp(common.Big1) < 0 || s.Cmp(common.Big1) < 0 { return false } + vint := uint32(v) if r.Cmp(secp256k1n) < 0 && s.Cmp(secp256k1n) < 0 && (vint == 27 || vint == 28) { return true } else { @@ -302,17 +302,6 @@ func aesCBCDecrypt(key, cipherText, iv []byte) ([]byte, error) { } // From https://leanpub.com/gocrypto/read#leanpub-auto-block-cipher-modes -func PKCS7Pad(in []byte) []byte { - padding := 16 - (len(in) % 16) - if padding == 0 { - padding = 16 - } - for i := 0; i < padding; i++ { - in = append(in, byte(padding)) - } - return in -} - func PKCS7Unpad(in []byte) []byte { if len(in) == 0 { return nil diff --git a/crypto/crypto_test.go b/crypto/crypto_test.go index b891f41a9a..fdd9c1ee85 100644 --- a/crypto/crypto_test.go +++ b/crypto/crypto_test.go @@ -18,8 +18,12 @@ package crypto import ( "bytes" + "crypto/ecdsa" "encoding/hex" "fmt" + "io/ioutil" + "math/big" + "os" "testing" "time" @@ -27,10 +31,12 @@ import ( "github.com/ethereum/go-ethereum/crypto/secp256k1" ) +var testAddrHex = "970e8128ab834e8eac17ab8e3812f010678cf791" +var testPrivHex = "289c2857d4598e37fb9647507e47a309d6133539bf21a8b9cb6df88fd5232032" + // These tests are sanity checks. // They should ensure that we don't e.g. use Sha3-224 instead of Sha3-256 // and that the sha3 library uses keccak-f permutation. - func TestSha3(t *testing.T) { msg := []byte("abc") exp, _ := hex.DecodeString("4e03657aea45a94fc7d47ba826c8d667c0d1e6e33a64a036ec44f58fa12d6c45") @@ -55,13 +61,6 @@ func TestRipemd160(t *testing.T) { checkhash(t, "Ripemd160", Ripemd160, msg, exp) } -func checkhash(t *testing.T, name string, f func([]byte) []byte, msg, exp []byte) { - sum := f(msg) - if bytes.Compare(exp, sum) != 0 { - t.Errorf("hash %s returned wrong result.\ngot: %x\nwant: %x", name, sum, exp) - } -} - func BenchmarkSha3(b *testing.B) { a := []byte("hello world") amount := 1000000 @@ -74,13 +73,41 @@ func BenchmarkSha3(b *testing.B) { } func Test0Key(t *testing.T) { - t.Skip() - key := common.Hex2Bytes("1111111111111111111111111111111111111111111111111111111111111111") + key := common.Hex2Bytes("0000000000000000000000000000000000000000000000000000000000000000") + _, err := secp256k1.GeneratePubKey(key) + if err == nil { + t.Errorf("expected error due to zero privkey") + } +} + +func TestSign(t *testing.T) { + key, _ := HexToECDSA(testPrivHex) + addr := common.HexToAddress(testAddrHex) + + msg := Sha3([]byte("foo")) + sig, err := Sign(msg, key) + if err != nil { + t.Errorf("Sign error: %s", err) + } + recoveredPub, err := Ecrecover(msg, sig) + if err != nil { + t.Errorf("ECRecover error: %s", err) + } + recoveredAddr := PubkeyToAddress(*ToECDSAPub(recoveredPub)) + if addr != recoveredAddr { + t.Errorf("Address mismatch: want: %x have: %x", addr, recoveredAddr) + } + + // should be equal to SigToPub + recoveredPub2, err := SigToPub(msg, sig) + if err != nil { + t.Errorf("ECRecover error: %s", err) + } + recoveredAddr2 := PubkeyToAddress(*recoveredPub2) + if addr != recoveredAddr2 { + t.Errorf("Address mismatch: want: %x have: %x", addr, recoveredAddr2) + } - p, err := secp256k1.GeneratePubKey(key) - addr := Sha3(p[1:])[12:] - fmt.Printf("%x\n", p) - fmt.Printf("%v %x\n", err, addr) } func TestInvalidSign(t *testing.T) { @@ -94,3 +121,129 @@ func TestInvalidSign(t *testing.T) { t.Errorf("expected sign with hash 33 byte to error") } } + +func TestNewContractAddress(t *testing.T) { + key, _ := HexToECDSA(testPrivHex) + addr := common.HexToAddress(testAddrHex) + genAddr := PubkeyToAddress(key.PublicKey) + // sanity check before using addr to create contract address + checkAddr(t, genAddr, addr) + + caddr0 := CreateAddress(addr, 0) + caddr1 := CreateAddress(addr, 1) + caddr2 := CreateAddress(addr, 2) + checkAddr(t, common.HexToAddress("333c3310824b7c685133f2bedb2ca4b8b4df633d"), caddr0) + checkAddr(t, common.HexToAddress("8bda78331c916a08481428e4b07c96d3e916d165"), caddr1) + checkAddr(t, common.HexToAddress("c9ddedf451bc62ce88bf9292afb13df35b670699"), caddr2) +} + +func TestLoadECDSAFile(t *testing.T) { + keyBytes := common.FromHex(testPrivHex) + fileName0 := "test_key0" + fileName1 := "test_key1" + checkKey := func(k *ecdsa.PrivateKey) { + checkAddr(t, PubkeyToAddress(k.PublicKey), common.HexToAddress(testAddrHex)) + loadedKeyBytes := FromECDSA(k) + if !bytes.Equal(loadedKeyBytes, keyBytes) { + t.Fatalf("private key mismatch: want: %x have: %x", keyBytes, loadedKeyBytes) + } + } + + ioutil.WriteFile(fileName0, []byte(testPrivHex), 0600) + defer os.Remove(fileName0) + + key0, err := LoadECDSA(fileName0) + if err != nil { + t.Fatal(err) + } + checkKey(key0) + + // again, this time with SaveECDSA instead of manual save: + err = SaveECDSA(fileName1, key0) + if err != nil { + t.Fatal(err) + } + defer os.Remove(fileName1) + + key1, err := LoadECDSA(fileName1) + if err != nil { + t.Fatal(err) + } + checkKey(key1) +} + +func TestValidateSignatureValues(t *testing.T) { + check := func(expected bool, v byte, r, s *big.Int) { + if ValidateSignatureValues(v, r, s) != expected { + t.Errorf("mismatch for v: %d r: %d s: %d want: %v", v, r, s, expected) + } + } + minusOne := big.NewInt(-1) + one := common.Big1 + zero := common.Big0 + secp256k1nMinus1 := new(big.Int).Sub(secp256k1n, common.Big1) + + // correct v,r,s + check(true, 27, one, one) + check(true, 28, one, one) + // incorrect v, correct r,s, + check(false, 30, one, one) + check(false, 26, one, one) + + // incorrect v, combinations of incorrect/correct r,s at lower limit + 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) + + // correct sig with max r,s + check(true, 27, secp256k1nMinus1, secp256k1nMinus1) + // correct v, combinations of incorrect r,s at upper limit + check(false, 27, secp256k1n, secp256k1nMinus1) + check(false, 27, secp256k1nMinus1, secp256k1n) + check(false, 27, secp256k1n, secp256k1n) + + // 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) +} + +func checkhash(t *testing.T, name string, f func([]byte) []byte, msg, exp []byte) { + sum := f(msg) + if bytes.Compare(exp, sum) != 0 { + t.Fatalf("hash %s mismatch: want: %x have: %x", name, exp, sum) + } +} + +func checkAddr(t *testing.T, addr0, addr1 common.Address) { + if addr0 != addr1 { + t.Fatalf("address mismatch: want: %x have: %x", addr0, addr1) + } +} + +// test to help Python team with integration of libsecp256k1 +// skip but keep it after they are done +func TestPythonIntegration(t *testing.T) { + kh := "289c2857d4598e37fb9647507e47a309d6133539bf21a8b9cb6df88fd5232032" + k0, _ := HexToECDSA(kh) + k1 := FromECDSA(k0) + + msg0 := Sha3([]byte("foo")) + sig0, _ := secp256k1.Sign(msg0, k1) + + msg1 := common.FromHex("00000000000000000000000000000000") + sig1, _ := secp256k1.Sign(msg0, k1) + + fmt.Printf("msg: %x, privkey: %x sig: %x\n", msg0, k1, sig0) + fmt.Printf("msg: %x, privkey: %x sig: %x\n", msg1, k1, sig1) +}