From 8fa65a0afc152d8f1fe16f8699ccd730c5d22666 Mon Sep 17 00:00:00 2001 From: alexvanin Date: Tue, 12 Nov 2019 11:13:12 +0300 Subject: [PATCH] Use consistent parameter names for Sign and Verify functions It may be misleading when verify function takes signature as a hash parameter. This commit suggested to use rfc6979 original naming for the parameters: - `msg` as the message to sign, - `sig` as the signature of message. All hashing operations are encapsulated inside of the Sign and Verify functions. Also there are comment fixes and re-usage of `hashBytes()` in rfc6979. --- ecdsa.go | 42 +++++++++++++++++++++--------------------- rfc6979.go | 22 +++++++++------------- wif.go | 2 +- 3 files changed, 31 insertions(+), 35 deletions(-) diff --git a/ecdsa.go b/ecdsa.go index 0843e87..1ed5178 100644 --- a/ecdsa.go +++ b/ecdsa.go @@ -13,10 +13,10 @@ import ( ) const ( - // ErrEmptyPublicKey when PK passed to Verify method is nil + // ErrEmptyPublicKey when PK passed to Verify method is nil. ErrEmptyPublicKey = internal.Error("empty public key") - // ErrInvalidSignature when signature passed to Verify method is mismatch + // ErrInvalidSignature when signature passed to Verify method is mismatch. ErrInvalidSignature = internal.Error("invalid signature") // ErrCannotUnmarshal when signature ([]byte) passed to Verify method has wrong format @@ -32,11 +32,11 @@ const ( // PublicKeyUncompressedSize is constant with uncompressed size of public key (PK). // First byte always should be 0x4 other 64 bytes is X and Y (32 bytes per coordinate). - // 2 * 32 + 1. + // 2 * 32 + 1 PublicKeyUncompressedSize = 65 ) -// P256 is base elliptic curve +// P256 is base elliptic curve. var curve = elliptic.P256() // Marshal converts a points into the uncompressed form specified in section 4.3.6 of ANSI X9.62. @@ -50,7 +50,7 @@ func marshalXY(x, y *big.Int) []byte { // Unlike the original version of the code, we ignore that x or y not on the curve // -------------- // It's copy-paste elliptic.Unmarshal(curve, data) stdlib function, without last line -// of code 🤔 +// of code. // Link - https://golang.org/pkg/crypto/elliptic/#Unmarshal func unmarshalXY(data []byte) (x *big.Int, y *big.Int) { if len(data) != PublicKeyUncompressedSize { @@ -138,7 +138,7 @@ func decodePoint(data []byte) (*big.Int, *big.Int) { return nil, nil } -// MarshalPublicKey to bytes +// MarshalPublicKey to bytes. func MarshalPublicKey(key *ecdsa.PublicKey) []byte { if key == nil || key.X == nil || key.Y == nil { return nil @@ -147,7 +147,7 @@ func MarshalPublicKey(key *ecdsa.PublicKey) []byte { return encodePoint(key.X, key.Y) } -// UnmarshalPublicKey from bytes +// UnmarshalPublicKey from bytes. func UnmarshalPublicKey(data []byte) *ecdsa.PublicKey { if x, y := decodePoint(data); x != nil && y != nil && curve.IsOnCurve(x, y) { return &ecdsa.PublicKey{ @@ -160,8 +160,9 @@ func UnmarshalPublicKey(data []byte) *ecdsa.PublicKey { return nil } -// UnmarshalPrivateKey method to parse SK from bytes. -// It is similar to `ecdsa.Generate()` but uses pre-defined big.Int and curve for NEO Blockchain +// UnmarshalPrivateKey from bytes. +// It is similar to `ecdsa.Generate()` but uses pre-defined big.Int and +// curve for NEO Blockchain (elliptic.P256) // Link - https://golang.org/pkg/crypto/ecdsa/#GenerateKey func UnmarshalPrivateKey(data []byte) (*ecdsa.PrivateKey, error) { if len(data) == PrivateKeyCompressedSize { // todo: consider using only NEO blockchain private keys @@ -177,7 +178,7 @@ func UnmarshalPrivateKey(data []byte) (*ecdsa.PrivateKey, error) { return x509.ParseECPrivateKey(data) } -// MarshalPrivateKey to bytes +// MarshalPrivateKey to bytes. func MarshalPrivateKey(key *ecdsa.PrivateKey) []byte { return key.D.Bytes() } @@ -188,27 +189,26 @@ func hashBytes(data []byte) []byte { return buf[:] } -// Verify verifies the signature in r, s of hash using the public key, pub. Its -// return value records whether the signature is valid. -func Verify(pub *ecdsa.PublicKey, hash, data []byte) error { - if r, s := unmarshalXY(hash); r == nil || s == nil { - // panic("could not unmarshal r / s") +// Verify verifies the signature of msg using the public key pub. It returns +// nil only if signature is valid. +func Verify(pub *ecdsa.PublicKey, sig, msg []byte) error { + if r, s := unmarshalXY(sig); r == nil || s == nil { return ErrCannotUnmarshal } else if pub == nil { return ErrEmptyPublicKey - } else if !ecdsa.Verify(pub, hashBytes(data), r, s) { + } else if !ecdsa.Verify(pub, hashBytes(msg), r, s) { return errors.Wrapf(ErrInvalidSignature, "%0x : %0x", r, s) } return nil } -// Sign signs a data using the private key. If the data is longer than -// the bit-length of the private key's curve order, the hash will be -// truncated to that length. It returns the signature as slice bytes. +// Sign signs a message using the private key. If the sha256 hash of msg +// is longer than the bit-length of the private key's curve order, the hash +// will be truncated to that length. It returns the signature as slice bytes. // The security of the private key depends on the entropy of rand. -func Sign(key *ecdsa.PrivateKey, data []byte) ([]byte, error) { - x, y, err := ecdsa.Sign(rand.Reader, key, hashBytes(data)) +func Sign(key *ecdsa.PrivateKey, msg []byte) ([]byte, error) { + x, y, err := ecdsa.Sign(rand.Reader, key, hashBytes(msg)) if err != nil { return nil, err } diff --git a/rfc6979.go b/rfc6979.go index fe85687..f591001 100644 --- a/rfc6979.go +++ b/rfc6979.go @@ -14,10 +14,10 @@ const ( // RFC6979SignatureSize contains r and s coordinates (32 bytes) RFC6979SignatureSize = 64 - // ErrWrongHashSize when passed signature to VerifyRFC6979 has wrong size + // ErrWrongHashSize when passed signature to VerifyRFC6979 has wrong size. ErrWrongHashSize = internal.Error("wrong hash size") - // ErrWrongSignature when passed signature to VerifyRFC6979 isn't valid + // ErrWrongSignature when passed signature to VerifyRFC6979 isn't valid. ErrWrongSignature = internal.Error("wrong signature") ) @@ -26,11 +26,9 @@ const ( // signature as a pair of integers. // // Note that FIPS 186-3 section 4.6 specifies that the hash should be truncated -// to the byte-length of the subgroup. This function does not perform that +// to the byte-length of the subgroup. This function does not perform that. func SignRFC6979(key *ecdsa.PrivateKey, msg []byte) ([]byte, error) { - msgHash := sha256.Sum256(msg) - - r, s, err := rfc6979.SignECDSA(key, msgHash[:], sha256.New) + r, s, err := rfc6979.SignECDSA(key, hashBytes(msg), sha256.New) if err != nil { return nil, err } @@ -46,14 +44,12 @@ func decodeSignature(sig []byte) (*big.Int, *big.Int, error) { return new(big.Int).SetBytes(sig[:32]), new(big.Int).SetBytes(sig[32:]), nil } -// VerifyRFC6979 verifies the signature in r, s of hash using the public key, pub. Its -// return value records whether the signature is valid. -func VerifyRFC6979(key *ecdsa.PublicKey, hash, data []byte) error { - msgHash := sha256.Sum256(data) - - if r, s, err := decodeSignature(hash); err != nil { +// VerifyRFC6979 verifies the signature of msg using the public key. It +// return nil only if signature is valid. +func VerifyRFC6979(key *ecdsa.PublicKey, sig, msg []byte) error { + if r, s, err := decodeSignature(sig); err != nil { return err - } else if !ecdsa.Verify(key, msgHash[:], r, s) { + } else if !ecdsa.Verify(key, hashBytes(msg), r, s) { return ErrWrongSignature } diff --git a/wif.go b/wif.go index 3ada5a2..8c3a407 100644 --- a/wif.go +++ b/wif.go @@ -21,7 +21,7 @@ const ( // by last 4 bytes signature. ErrBadChecksum = internal.Error("bad checksum") - // ErrEmptyPrivateKey when PK passed into WIFEncode method is nil + // ErrEmptyPrivateKey when PK passed into WIFEncode method is nil. ErrEmptyPrivateKey = internal.Error("empty private key") )