From 5bc4686aabfd6ac6bd1572082dd90305c8c71fc1 Mon Sep 17 00:00:00 2001
From: Evan Phoenix <evan@phx.io>
Date: Wed, 16 Sep 2015 19:53:08 -0700
Subject: [PATCH] Fix ES signature serialization

---
 ecdsa.go      | 61 ++++++++++++++++++++++++++++++---------------------
 ecdsa_test.go |  6 ++---
 2 files changed, 39 insertions(+), 28 deletions(-)

diff --git a/ecdsa.go b/ecdsa.go
index 9cd538f..0518ed1 100644
--- a/ecdsa.go
+++ b/ecdsa.go
@@ -4,7 +4,6 @@ import (
 	"crypto"
 	"crypto/ecdsa"
 	"crypto/rand"
-	"encoding/asn1"
 	"errors"
 	"math/big"
 )
@@ -16,14 +15,10 @@ var (
 
 // Implements the ECDSA family of signing methods signing methods
 type SigningMethodECDSA struct {
-	Name string
-	Hash crypto.Hash
-}
-
-// Marshalling structure for r, s EC point
-type ECPoint struct {
-	R *big.Int
-	S *big.Int
+	Name      string
+	Hash      crypto.Hash
+	KeySize   int
+	CurveBits int
 }
 
 // Specific instances for EC256 and company
@@ -35,19 +30,19 @@ var (
 
 func init() {
 	// ES256
-	SigningMethodES256 = &SigningMethodECDSA{"ES256", crypto.SHA256}
+	SigningMethodES256 = &SigningMethodECDSA{"ES256", crypto.SHA256, 32, 256}
 	RegisterSigningMethod(SigningMethodES256.Alg(), func() SigningMethod {
 		return SigningMethodES256
 	})
 
 	// ES384
-	SigningMethodES384 = &SigningMethodECDSA{"ES384", crypto.SHA384}
+	SigningMethodES384 = &SigningMethodECDSA{"ES384", crypto.SHA384, 48, 384}
 	RegisterSigningMethod(SigningMethodES384.Alg(), func() SigningMethod {
 		return SigningMethodES384
 	})
 
 	// ES512
-	SigningMethodES512 = &SigningMethodECDSA{"ES512", crypto.SHA512}
+	SigningMethodES512 = &SigningMethodECDSA{"ES512", crypto.SHA512, 66, 521}
 	RegisterSigningMethod(SigningMethodES512.Alg(), func() SigningMethod {
 		return SigningMethodES512
 	})
@@ -77,12 +72,13 @@ func (m *SigningMethodECDSA) Verify(signingString, signature string, key interfa
 		return ErrInvalidKey
 	}
 
-	// Unmarshal asn1 ECPoint
-	var ecpoint = new(ECPoint)
-	if _, err := asn1.Unmarshal(sig, ecpoint); err != nil {
-		return err
+	if len(sig) != 2*m.KeySize {
+		return ErrECDSAVerification
 	}
 
+	r := big.NewInt(0).SetBytes(sig[:m.KeySize])
+	s := big.NewInt(0).SetBytes(sig[m.KeySize:])
+
 	// Create hasher
 	if !m.Hash.Available() {
 		return ErrHashUnavailable
@@ -91,7 +87,7 @@ func (m *SigningMethodECDSA) Verify(signingString, signature string, key interfa
 	hasher.Write([]byte(signingString))
 
 	// Verify the signature
-	if verifystatus := ecdsa.Verify(ecdsaKey, hasher.Sum(nil), ecpoint.R, ecpoint.S); verifystatus == true {
+	if verifystatus := ecdsa.Verify(ecdsaKey, hasher.Sum(nil), r, s); verifystatus == true {
 		return nil
 	} else {
 		return ErrECDSAVerification
@@ -120,16 +116,31 @@ func (m *SigningMethodECDSA) Sign(signingString string, key interface{}) (string
 
 	// Sign the string and return r, s
 	if r, s, err := ecdsa.Sign(rand.Reader, ecdsaKey, hasher.Sum(nil)); err == nil {
-		// asn1 marhsal r, s using ecPoint as the structure
-		var ecpoint = new(ECPoint)
-		ecpoint.R = r
-		ecpoint.S = s
+		curveBits := ecdsaKey.Curve.Params().BitSize
 
-		if signature, err := asn1.Marshal(*ecpoint); err != nil {
-			return "", err
-		} else {
-			return EncodeSegment(signature), nil
+		if m.CurveBits != curveBits {
+			return "", ErrInvalidKey
 		}
+
+		keyBytes := curveBits / 8
+		if curveBits%8 > 0 {
+			keyBytes += 1
+		}
+
+		// We serialize the outpus (r and s) into big-endian byte arrays and pad
+		// them with zeros on the left to make sure the sizes work out. Both arrays
+		// must be keyBytes long, and the output must be 2*keyBytes long.
+		rBytes := r.Bytes()
+		rBytesPadded := make([]byte, keyBytes)
+		copy(rBytesPadded[keyBytes-len(rBytes):], rBytes)
+
+		sBytes := s.Bytes()
+		sBytesPadded := make([]byte, keyBytes)
+		copy(sBytesPadded[keyBytes-len(sBytes):], sBytes)
+
+		out := append(rBytesPadded, sBytesPadded...)
+
+		return EncodeSegment(out), nil
 	} else {
 		return "", err
 	}
diff --git a/ecdsa_test.go b/ecdsa_test.go
index 98e3e5e..753047b 100644
--- a/ecdsa_test.go
+++ b/ecdsa_test.go
@@ -20,7 +20,7 @@ var ecdsaTestData = []struct {
 	{
 		"Basic ES256",
 		map[string]string{"private": "test/ec256-private.pem", "public": "test/ec256-public.pem"},
-		"eyJhbGciOiJFUzI1NiIsInR5cCI6IkpXVCJ9.eyJmb28iOiJiYXIifQ.MEQCIHoSJnmGlPaVQDqacx_2XlXEhhqtWceVopjomc2PJLtdAiAUTeGPoNYxZw0z8mgOnnIcjoxRuNDVZvybRZF3wR1l8w",
+		"eyJ0eXAiOiJKV1QiLCJhbGciOiJFUzI1NiJ9.eyJmb28iOiJiYXIifQ.feG39E-bn8HXAKhzDZq7yEAPWYDhZlwTn3sePJnU9VrGMmwdXAIEyoOnrjreYlVM_Z4N13eK9-TmMTWyfKJtHQ",
 		"ES256",
 		map[string]interface{}{"foo": "bar"},
 		true,
@@ -28,7 +28,7 @@ var ecdsaTestData = []struct {
 	{
 		"Basic ES384",
 		map[string]string{"private": "test/ec384-private.pem", "public": "test/ec384-public.pem"},
-		"eyJhbGciOiJFUzM4NCIsInR5cCI6IkpXVCJ9.eyJmb28iOiJiYXIifQ.MGUCMQCHBr61FXDuFY9xUhyp8iWQAuBIaSgaf1z2j_8XrKcCfzTPzoSa3SZKq-m3L492xe8CMG3kafRMeuaN5Aw8ZJxmOLhkTo4D3-LaGzcaUWINvWvkwFMl7dMC863s0gov6xvXuA",
+		"eyJ0eXAiOiJKV1QiLCJhbGciOiJFUzM4NCJ9.eyJmb28iOiJiYXIifQ.ngAfKMbJUh0WWubSIYe5GMsA-aHNKwFbJk_wq3lq23aPp8H2anb1rRILIzVR0gUf4a8WzDtrzmiikuPWyCS6CN4-PwdgTk-5nehC7JXqlaBZU05p3toM3nWCwm_LXcld",
 		"ES384",
 		map[string]interface{}{"foo": "bar"},
 		true,
@@ -36,7 +36,7 @@ var ecdsaTestData = []struct {
 	{
 		"Basic ES512",
 		map[string]string{"private": "test/ec512-private.pem", "public": "test/ec512-public.pem"},
-		"eyJhbGciOiJFUzUxMiIsInR5cCI6IkpXVCJ9.eyJmb28iOiJiYXIifQ.MIGIAkIAmVKjdJE5lG1byOFgZZVTeNDRp6E7SNvUj0UrvpzoBH6nrleWVTcwfHzbwWuooNpPADDSFR_Ql3ze-Vwwi8hBqQsCQgHn-ZooL8zegkOVeEEsqd7WHWdhb8UekFCYw3X8JnNP-D3wvZQ1-tkkHakt5gZ2-xO29TxfSPun4ViGkMYa7Q4N-Q",
+		"eyJ0eXAiOiJKV1QiLCJhbGciOiJFUzUxMiJ9.eyJmb28iOiJiYXIifQ.AAU0TvGQOcdg2OvrwY73NHKgfk26UDekh9Prz-L_iWuTBIBqOFCWwwLsRiHB1JOddfKAls5do1W0jR_F30JpVd-6AJeTjGKA4C1A1H6gIKwRY0o_tFDIydZCl_lMBMeG5VNFAjO86-WCSKwc3hqaGkq1MugPRq_qrF9AVbuEB4JPLyL5",
 		"ES512",
 		map[string]interface{}{"foo": "bar"},
 		true,