From 876296f6b078d3cbcbfbc7fdae5c613b794b2ccc Mon Sep 17 00:00:00 2001 From: ia Date: Sat, 14 Apr 2018 05:58:45 -0500 Subject: [PATCH 1/2] problem: nondeterministic K-value in ECDSA algo solution: move to a deterministic k-value using RFC6979 rel https://github.com/ethereum/go-ethereum/issues/2190 rel https://github.com/ethereum/go-ethereum/pull/3561 rel #245 --- crypto/crypto_test.go | 3 ++ crypto/secp256k1/secp256.go | 45 ++++++++++++++++++++++++++- crypto/secp256k1/secp256_test.go | 53 ++++++++++++++++++++++++++++++++ 3 files changed, 100 insertions(+), 1 deletion(-) diff --git a/crypto/crypto_test.go b/crypto/crypto_test.go index 50acde270..e3691715d 100644 --- a/crypto/crypto_test.go +++ b/crypto/crypto_test.go @@ -89,6 +89,9 @@ func TestSign(t *testing.T) { if err != nil { t.Errorf("Sign error: %s", err) } + if len(sig) != 65 { + t.Error("wrong len sig", len(sig)) + } recoveredPub, err := Ecrecover(msg, sig) if err != nil { t.Errorf("ECRecover error: %s", err) diff --git a/crypto/secp256k1/secp256.go b/crypto/secp256k1/secp256.go index 8c6601a83..3dd237f2f 100644 --- a/crypto/secp256k1/secp256.go +++ b/crypto/secp256k1/secp256.go @@ -76,6 +76,10 @@ var ( ErrInvalidMsgLen = errors.New("invalid message length for signature recovery") ErrInvalidSignatureLen = errors.New("invalid signature length") ErrInvalidRecoveryID = errors.New("invalid signature recovery id") + ErrInvalidKey = errors.New("invalid private key") + ErrInvalidPubkey = errors.New("invalid public key") + ErrSignFailed = errors.New("signing failed") + ErrRecoverFailed = errors.New("recovery failed") ) func GenerateKeyPair() ([]byte, []byte) { @@ -132,7 +136,9 @@ func GeneratePubKey(seckey []byte) ([]byte, error) { return pubkey, nil } -func Sign(msg []byte, seckey []byte) ([]byte, error) { +// SignNondeterministic generates nondeterministic signature b/c of a random k-value in the ECDSA algorithm. This function is included +// only for purpose of demonstration and comparison with the deterministic Sign function. +func SignNondeterministic(msg []byte, seckey []byte) ([]byte, error) { msg_ptr := (*C.uchar)(unsafe.Pointer(&msg[0])) seckey_ptr := (*C.uchar)(unsafe.Pointer(&seckey[0])) @@ -178,6 +184,43 @@ func Sign(msg []byte, seckey []byte) ([]byte, error) { } +// Sign creates a recoverable ECDSA signature. +// The produced signature is in the 65-byte [R || S || V] format where V is 0 or 1. +// +// The caller is responsible for ensuring that msg cannot be chosen +// directly by an attacker. It is usually preferable to use a cryptographic +// hash function on any input before handing it to this function. +func Sign(msg []byte, seckey []byte) ([]byte, error) { + if len(msg) != 32 { + return nil, ErrInvalidMsgLen + } + if len(seckey) != 32 { + return nil, ErrInvalidKey + } + seckeydata := (*C.uchar)(unsafe.Pointer(&seckey[0])) + if C.secp256k1_ec_seckey_verify(context, seckeydata) != 1 { + return nil, ErrInvalidKey + } + + var ( + msgdata = (*C.uchar)(unsafe.Pointer(&msg[0])) + noncefunc = C.secp256k1_nonce_function_rfc6979 + sigstruct C.secp256k1_ecdsa_recoverable_signature + ) + if C.secp256k1_ecdsa_sign_recoverable(context, &sigstruct, msgdata, seckeydata, noncefunc, nil) == 0 { + return nil, ErrSignFailed + } + + var ( + sig = make([]byte, 65) + sigdata = (*C.uchar)(unsafe.Pointer(&sig[0])) + recid C.int + ) + C.secp256k1_ecdsa_recoverable_signature_serialize_compact(context, sigdata, &recid, &sigstruct) + sig[64] = byte(recid) // add back recid to get 65 bytes sig + return sig, nil +} + func VerifySeckeyValidity(seckey []byte) error { if len(seckey) != 32 { return errors.New("priv key is not 32 bytes") diff --git a/crypto/secp256k1/secp256_test.go b/crypto/secp256k1/secp256_test.go index 0e8aedcc7..dd52965cf 100644 --- a/crypto/secp256k1/secp256_test.go +++ b/crypto/secp256k1/secp256_test.go @@ -21,6 +21,7 @@ import ( "encoding/hex" "testing" + "github.com/ethereumproject/go-ethereum/common/hexutil" "github.com/ethereumproject/go-ethereum/crypto/randentropy" ) @@ -67,6 +68,58 @@ func TestInvalidRecoveryID(t *testing.T) { } } +func TestSignDeterministic(t *testing.T) { + _, seckey := GenerateKeyPair() + msg := randentropy.GetEntropyCSPRNG(32) + sig1, err := Sign(msg, seckey) + if err != nil { + t.Errorf("signature 1 error: %s", err) + } + sig2, err := Sign(msg, seckey) + if err != nil { + t.Errorf("signature 2 error: %s", err) + } + if !bytes.Equal(sig1, sig2) { + t.Fatal("sig1 != sig2", hexutil.Encode(sig1), hexutil.Encode(sig2)) + } +} + +// TestSignNondeterministic shows that TestSignNondeterministic does not generate deterministic signatures. +func TestSignNondeterministic(t *testing.T) { + _, seckey := GenerateKeyPair() + msg := randentropy.GetEntropyCSPRNG(32) + sig1, err := SignNondeterministic(msg, seckey) + if err != nil { + t.Errorf("signature 1 error: %s", err) + } + sig2, err := SignNondeterministic(msg, seckey) + if err != nil { + t.Errorf("signature 2 error: %s", err) + } + // Of course the signatures should be the same (1:1), but they are not. + if bytes.Equal(sig1, sig2) { + t.Error("sig1 == sig2", hexutil.Encode(sig1), hexutil.Encode(sig2)) + } +} + +// TestSignNondeterministic2 shows that TestSignNondeterministic does not generate the same signature as deterministic Sign. +func TestSignNondeterministic2(t *testing.T) { + _, seckey := GenerateKeyPair() + msg := randentropy.GetEntropyCSPRNG(32) + sig1, err := SignNondeterministic(msg, seckey) + if err != nil { + t.Errorf("signature 1 error: %s", err) + } + sig2, err := Sign(msg, seckey) + if err != nil { + t.Errorf("signature 2 error: %s", err) + } + // Of course the signatures should be the same (1:1), but they are not. + if bytes.Equal(sig1, sig2) { + t.Error("sig1 == sig2", hexutil.Encode(sig1), hexutil.Encode(sig2)) + } +} + func TestSignAndRecover(t *testing.T) { pubkey1, seckey := GenerateKeyPair() msg := randentropy.GetEntropyCSPRNG(32) From 040b617bfef1ce4eb2184dc591d8d7142bf028a8 Mon Sep 17 00:00:00 2001 From: ia Date: Wed, 2 May 2018 19:54:42 +0900 Subject: [PATCH 2/2] solution: improve test error ln for descriptiveness --- crypto/crypto_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crypto/crypto_test.go b/crypto/crypto_test.go index e3691715d..5e2beba51 100644 --- a/crypto/crypto_test.go +++ b/crypto/crypto_test.go @@ -90,7 +90,7 @@ func TestSign(t *testing.T) { t.Errorf("Sign error: %s", err) } if len(sig) != 65 { - t.Error("wrong len sig", len(sig)) + t.Error("wrong signature length", len(sig)) } recoveredPub, err := Ecrecover(msg, sig) if err != nil {