Merge branch 'cs/164590'

* cs/164590:
  Check for invalid ECDH keys in headers, JWKs
  Do not accept invalid public keys in DeriveECDHES function

Author: csstaub

asymmetric.go

@@ -370,6 +370,10 @@ func (ctx ecDecrypterSigner) decryptKey(headers rawHeader, recipient *recipientI
 		return nil, errors.New("square/go-jose: invalid epk header")
 	}
 
+	if !ctx.privateKey.Curve.IsOnCurve(publicKey.X, publicKey.Y) {
+		return nil, errors.New("square/go-jose: invalid public key in epk header")
+	}
+
 	apuData := headers.Apu.bytes()
 	apvData := headers.Apv.bytes()
 

asymmetric_test.go

@@ -18,6 +18,8 @@ package jose
 
 import (
 	"bytes"
+	"crypto/ecdsa"
+	"crypto/elliptic"
 	"crypto/rand"
 	"crypto/rsa"
 	"errors"
@@ -429,3 +431,31 @@ func TestInvalidEllipticCurve(t *testing.T) {
 		t.Error("should not generate ES384 signature with P-521 key")
 	}
 }
+
+func TestInvalidECPublicKey(t *testing.T) {
+	// Invalid key
+	invalid := &ecdsa.PrivateKey{
+		PublicKey: ecdsa.PublicKey{
+			Curve: elliptic.P256(),
+			X:     fromBase64Int("MTEx"),
+			Y:     fromBase64Int("MTEx"),
+		},
+		D: fromBase64Int("0_NxaRPUMQoAJt50Gz8YiTr8gRTwyEaCumd-MToTmIo="),
+	}
+
+	headers := rawHeader{
+		Alg: string(ECDH_ES),
+		Epk: &JsonWebKey{
+			Key: &invalid.PublicKey,
+		},
+	}
+
+	dec := ecDecrypterSigner{
+		privateKey: ecTestKey256,
+	}
+
+	_, err := dec.decryptKey(headers, nil, randomKeyGenerator{size: 16})
+	if err == nil {
+		t.Fatal("decrypter accepted JWS with invalid ECDH public key")
+	}
+}

cipher/ecdh_es.go

@@ -33,6 +33,10 @@ func DeriveECDHES(alg string, apuData, apvData []byte, priv *ecdsa.PrivateKey, p
 	supPubInfo := make([]byte, 4)
 	binary.BigEndian.PutUint32(supPubInfo, uint32(size)*8)
 
+	if !priv.PublicKey.Curve.IsOnCurve(pub.X, pub.Y) {
+		panic("public key not on same curve as private key")
+	}
+
 	z, _ := priv.PublicKey.Curve.ScalarMult(pub.X, pub.Y, priv.D.Bytes())
 	reader := NewConcatKDF(crypto.SHA256, z.Bytes(), algID, ptyUInfo, ptyVInfo, supPubInfo, []byte{})
 

cipher/ecdh_es_test.go

@@ -67,6 +67,23 @@ func TestVectorECDHES(t *testing.T) {
 	}
 }
 
+func TestInvalidECPublicKey(t *testing.T) {
+	defer func() { recover() }()
+
+	// Invalid key
+	invalid := &ecdsa.PrivateKey{
+		PublicKey: ecdsa.PublicKey{
+			Curve: elliptic.P256(),
+			X:     fromBase64Int("MTEx"),
+			Y:     fromBase64Int("MTEx"),
+		},
+		D: fromBase64Int("0_NxaRPUMQoAJt50Gz8YiTr8gRTwyEaCumd-MToTmIo="),
+	}
+
+	DeriveECDHES("A128GCM", []byte{}, []byte{}, bobKey, &invalid.PublicKey, 16)
+	t.Fatal("should panic if public key was invalid")
+}
+
 func BenchmarkECDHES_128(b *testing.B) {
 	apuData := []byte("APU")
 	apvData := []byte("APV")

jwk.go

@@ -23,6 +23,7 @@ import (
 	"crypto/rsa"
 	"crypto/x509"
 	"encoding/base64"
+	"errors"
 	"fmt"
 	"math/big"
 	"reflect"
@@ -277,13 +278,20 @@ func (key rawJsonWebKey) ecPublicKey() (*ecdsa.PublicKey, error) {
 	}
 
 	if key.X == nil || key.Y == nil {
-		return nil, fmt.Errorf("square/go-jose: invalid EC key, missing x/y values")
+		return nil, errors.New("square/go-jose: invalid EC key, missing x/y values")
+	}
+
+	x := key.X.bigInt()
+	y := key.Y.bigInt()
+
+	if !curve.IsOnCurve(x, y) {
+		return nil, errors.New("square/go-jose: invalid EC key, X/Y are not on declared curve")
 	}
 
 	return &ecdsa.PublicKey{
 		Curve: curve,
-		X:     key.X.bigInt(),
-		Y:     key.Y.bigInt(),
+		X:     x,
+		Y:     y,
 	}, nil
 }