From 5136c786e51c7e95a6db6d55e5527dca53e2e945 Mon Sep 17 00:00:00 2001 From: Juan Batiz-Benet Date: Sun, 5 Oct 2014 15:56:52 -0700 Subject: [PATCH] Bugfix: "Malformed Public Key" Error This commit fixed the notoriously annoying "Malformed Public Key" problem. The issue was that sometimes the byte representation of the points (x,y in big.Int) generated would be one less byte than expected. This is simply because (* big.Int) Write uses the least amount of bytes needed for the int. I instead changed the marshalling/unmarshalling to do exactly what stdlib crypto/tls does: use `ellipctic.Marshal` which marshals according to the ANSI X9.62 standard. http://golang.org/pkg/crypto/elliptic/#Marshal http://golang.org/src/pkg/crypto/tls/key_agreement.go#L214 ```Go // crypto/tls ka.privateKey, x, y, err = elliptic.GenerateKey(ka.curve, config.rand()) ecdhePublic := elliptic.Marshal(ka.curve, x, y) // ipfs/crypto priv, x, y, err := elliptic.GenerateKey(curve, rand.Reader) pubKey := elliptic.Marshal(curve, x, y) ``` ((Warning: we're using `rand.Reader` directly, which we shouldn't do, as it can be seeded. We should use a configured source, as crypto/tls. Flagged in #143)) This makes me think we should re-use a lot of their datastructures and functions directly (e.g. ecdheKeyAgreement) Fixed: #135 cc @bren2010 @whyrusleeping --- crypto/key.go | 23 ++++++----------------- crypto/spipe/handshake.go | 4 ++-- 2 files changed, 8 insertions(+), 19 deletions(-) diff --git a/crypto/key.go b/crypto/key.go index 32c6488ff59..8fcc13e7de2 100644 --- a/crypto/key.go +++ b/crypto/key.go @@ -13,7 +13,6 @@ import ( "crypto/sha256" "crypto/sha512" "hash" - "math/big" "github.com/jbenet/go-ipfs/Godeps/_workspace/src/code.google.com/p/goprotobuf/proto" @@ -97,26 +96,16 @@ func GenerateEKeyPair(curveName string) ([]byte, GenSharedKey, error) { return nil, nil, err } - var pubKey bytes.Buffer - pubKey.Write(x.Bytes()) - pubKey.Write(y.Bytes()) + pubKey := elliptic.Marshal(curve, x, y) + u.PErr("GenerateEKeyPair %d\n", len(pubKey)) done := func(theirPub []byte) ([]byte, error) { // Verify and unpack node's public key. - curveSize := curve.Params().BitSize - - if len(theirPub) != (curveSize / 4) { - u.PErr("Malformed public key: %v", theirPub) - return nil, fmt.Errorf("Malformed public key: %v != %v", len(theirPub), (curveSize / 4)) + x, y := elliptic.Unmarshal(curve, theirPub) + if x == nil { + return nil, fmt.Errorf("Malformed public key: %d %v", len(theirPub), theirPub) } - bound := (curveSize / 8) - x := big.NewInt(0) - y := big.NewInt(0) - - x.SetBytes(theirPub[0:bound]) - y.SetBytes(theirPub[bound : bound*2]) - if !curve.IsOnCurve(x, y) { return nil, errors.New("Invalid public key.") } @@ -127,7 +116,7 @@ func GenerateEKeyPair(curveName string) ([]byte, GenSharedKey, error) { return secret.Bytes(), nil } - return pubKey.Bytes(), done, nil + return pubKey, done, nil } // Generates a set of keys for each party by stretching the shared key. diff --git a/crypto/spipe/handshake.go b/crypto/spipe/handshake.go index f42f13aaa63..a29d3c1d2b8 100644 --- a/crypto/spipe/handshake.go +++ b/crypto/spipe/handshake.go @@ -119,7 +119,7 @@ func (s *SecurePipe) handshake() error { } // u.POut("Selected %s %s %s\n", exchange, cipherType, hashType) - epubkey, done, err := ci.GenerateEKeyPair(exchange) // Generate EphemeralPubKey + epubkey, genSharedKey, err := ci.GenerateEKeyPair(exchange) // Generate EphemeralPubKey var handshake bytes.Buffer // Gather corpus to sign. handshake.Write(encoded) @@ -173,7 +173,7 @@ func (s *SecurePipe) handshake() error { return errors.New("Bad signature!") } - secret, err := done(exchangeResp.GetEpubkey()) + secret, err := genSharedKey(exchangeResp.GetEpubkey()) if err != nil { return err }