crypto.Sign panics for some inputs

[fjl]

There are two problems, both of which are related to this change. Note that the change is not mentioned in the commit message.

One problem is that package crypto uses privateKey.D.Bytes() to marshal the private key. The value returned by Bytes will not have length 32 in all cases because math/big returns the minimum number of bytes required to hold the number.

I think we should pad the key with zeroes.

The other problem is that the data being signed cannot be larger than 32 bytes, because
nonce would be accessed with an index > 31.
If it is smaller than 32 bytes, the remaining bytes of the nonce will be the rest of the private key.

The parameter for the data is called hash in package crypto. I think we should document that the value
for this parameter must have length 32 and check that it does in the Sign function.

Crash:

Panic at i=534.
Key: (len=31) [139 238 11 176 202 152 178 4 145 65 239 166 32 93 76 229 172 233 51 179 54 143 168 210 31 183 235 209 38 188 56]
runtime error: index out of range
goroutine 1 [running]:
main.func·001()
    /Users/fjl/develop/eth/src/github.com/ethereum/go-ethereum/ktest.go:31 +0x3d2
github.com/obscuren/secp256k1-go.Sign(0xc20809deb8, 0x20, 0x20, 0xc20801fc01, 0x1f, 0x1f, 0x0, 0x0, 0x0, 0x0, ...)
    /Users/fjl/develop/eth/src/github.com/obscuren/secp256k1-go/secp256.go:130 +0x492
github.com/ethereum/go-ethereum/crypto.Sign(0xc20809deb8, 0x20, 0x20, 0xc208097770, 0x0, 0x0, 0x0, 0x0, 0x0)
    /Users/fjl/develop/eth/src/github.com/ethereum/go-ethereum/crypto/crypto.go:108 +0xc0
main.main()
    /Users/fjl/develop/eth/src/github.com/ethereum/go-ethereum/ktest.go:42 +0x306
exit status 1

Repro tool:

import (
    "crypto/ecdsa"
    "crypto/rand"
    "fmt"
    "os"
    "runtime"

    "github.com/ethereum/go-ethereum/crypto"
)

func main() {
    var (
        hash = make([]byte, 32)
        k    *ecdsa.PrivateKey
        i    int
        err  error
    )
    defer func() {
        if err := recover(); err != nil {
            fmt.Printf("Panic at i=%d.\n", i)
            if k != nil {
                enc := k.D.Bytes() // this is what package crypto does to marshal the key
                fmt.Printf("Key: (len=%d) %v\n", len(enc), enc)
            }
            fmt.Println(err)
            sb := make([]byte, 1000)
            fmt.Print(string(sb[:runtime.Stack(sb, false)]))
            os.Exit(1)
        }
    }()

    for ; i < 10000; i++ {
        k, err = ecdsa.GenerateKey(crypto.S256(), rand.Reader)
        if err != nil {
            fmt.Println("cannot generate key:", err)
            os.Exit(1)
        }
        crypto.Sign(hash, k)
    }
}

[obscuren]

Input hashes for secp256k1 are always 32 bytes. No more no less. It's required by secp it self. To be more specific, the input is expected to be the 32 l hash of more complex data (in our case, for transactions, the hash of the tx). The reason why it remained unspecified is because signing is pretty specific by itself for transactions. A transaction's Hash is computed using sha3 which is always 32 bytes.

I'll put a length check in sign.

[obscuren]

0dfe511
d4cc2d3