Repository: lestrrat-go/jwx
Pull request #248 (2020-12-16 04:47:31)

Fix ECDH-ES support
I don't think the key parameter padding is strictly *required*, but I noticed it was a difference, and I think it makes sense to fix it.


Patch:
diff --git a/README.md b/README.md
index c1d5bc48..bc1979c8 100644
--- a/README.md
+++ b/README.md
@@ -374,7 +374,7 @@ Supported key encryption algorithm:
 | AES key wrap (192)                       | YES        | jwa.A192KW             |
 | AES key wrap (256)                       | YES        | jwa.A256KW             |
 | Direct encryption                        | NO         | jwa.DIRECT             |
-| ECDH-ES                                  | NO (see #234) | jwa.ECDH_ES            |
+| ECDH-ES                                  | YES (1)    | jwa.ECDH_ES            |
 | ECDH-ES + AES key wrap (128)             | YES        | jwa.ECDH_ES_A128KW     |
 | ECDH-ES + AES key wrap (192)             | YES        | jwa.ECDH_ES_A192KW     |
 | ECDH-ES + AES key wrap (256)             | YES        | jwa.ECDH_ES_A256KW     |
@@ -385,6 +385,8 @@ Supported key encryption algorithm:
 | PBES2 + HMAC-SHA384 + AES key wrap (192) | YES        | jwa.PBES2_HS384_A192KW |
 | PBES2 + HMAC-SHA512 + AES key wrap (256) | YES        | jwa.PBES2_HS512_A256KW |
 
+Note 1: Single-recipient only
+
 Supported content encryption algorithm:
 
 | Algorithm                   | Supported? | Constant in go-jwx     |
diff --git a/jwe/encrypt.go b/jwe/encrypt.go
index 96175e64..f8c95a9e 100644
--- a/jwe/encrypt.go
+++ b/jwe/encrypt.go
@@ -82,8 +82,15 @@ func (e encryptCtx) Encrypt(plaintext []byte) (*Message, error) {
 			}
 			return nil, errors.Wrap(err, `failed to encrypt key`)
 		}
-		if err := r.SetEncryptedKey(enckey.Bytes()); err != nil {
-			return nil, errors.Wrap(err, "failed to set encrypted key")
+		if enc.Algorithm() == jwa.ECDH_ES {
+			if len(e.keyEncrypters) > 1 {
+				return nil, errors.Errorf("unable to support multiple recipients for ECDH-ES")
+			}
+			cek = enckey.Bytes()
+		} else {
+			if err := r.SetEncryptedKey(enckey.Bytes()); err != nil {
+				return nil, errors.Wrap(err, "failed to set encrypted key")
+			}
 		}
 		if hp, ok := enckey.(populater); ok {
 			if err := hp.Populate(r.Headers()); err != nil {
diff --git a/jwe/message.go b/jwe/message.go
index 3f818cb1..abfbd7da 100644
--- a/jwe/message.go
+++ b/jwe/message.go
@@ -476,15 +476,7 @@ func (m *Message) Decrypt(alg jwa.KeyEncryptionAlgorithm, key interface{}) ([]by
 		}
 
 		switch alg {
-		case jwa.ECDH_ES:
-			// XXX for ECDH-ES
-			// disable this entire case, and replace the next case statement with the following:
-			//
-			// ```START
-			// case jwa.ECDH_ES, jwa.ECDH_ES_A128KW, jwa.ECDH_ES_A192KW, jwa.ECDH_ES_A256KW:
-			// ```END
-			return nil, errors.New(`ECDH-ES is not yet supported`)
-		case jwa.ECDH_ES_A128KW, jwa.ECDH_ES_A192KW, jwa.ECDH_ES_A256KW:
+		case jwa.ECDH_ES, jwa.ECDH_ES_A128KW, jwa.ECDH_ES_A192KW, jwa.ECDH_ES_A256KW:
 			epkif, ok := h2.Get(EphemeralPublicKeyKey)
 			if !ok {
 				return nil, errors.New("failed to get 'epk' field")
diff --git a/jwk/ecdsa.go b/jwk/ecdsa.go
index 0afad70f..4841f686 100644
--- a/jwk/ecdsa.go
+++ b/jwk/ecdsa.go
@@ -35,8 +35,16 @@ func newECDSAPrivateKey() *ecdsaPrivateKey {
 }
 
 func (k *ecdsaPublicKey) FromRaw(rawKey *ecdsa.PublicKey) error {
-	k.x = rawKey.X.Bytes()
-	k.y = rawKey.Y.Bytes()
+	xbuf := ecutil.AllocECPointBuffer(rawKey.X, rawKey.Curve)
+	ybuf := ecutil.AllocECPointBuffer(rawKey.Y, rawKey.Curve)
+	defer ecutil.ReleaseECPointBuffer(xbuf)
+	defer ecutil.ReleaseECPointBuffer(ybuf)
+
+	k.x = make([]byte, len(xbuf))
+	copy(k.x, xbuf)
+	k.y = make([]byte, len(ybuf))
+	copy(k.y, ybuf)
+
 	switch rawKey.Curve {
 	case elliptic.P256():
 		if err := k.Set(ECDSACrvKey, jwa.P256); err != nil {
@@ -58,8 +66,20 @@ func (k *ecdsaPublicKey) FromRaw(rawKey *ecdsa.PublicKey) error {
 }
 
 func (k *ecdsaPrivateKey) FromRaw(rawKey *ecdsa.PrivateKey) error {
-	k.x = rawKey.X.Bytes()
-	k.y = rawKey.Y.Bytes()
+	xbuf := ecutil.AllocECPointBuffer(rawKey.X, rawKey.Curve)
+	ybuf := ecutil.AllocECPointBuffer(rawKey.Y, rawKey.Curve)
+	dbuf := ecutil.AllocECPointBuffer(rawKey.D, rawKey.Curve)
+	defer ecutil.ReleaseECPointBuffer(xbuf)
+	defer ecutil.ReleaseECPointBuffer(ybuf)
+	defer ecutil.ReleaseECPointBuffer(dbuf)
+
+	k.x = make([]byte, len(xbuf))
+	copy(k.x, xbuf)
+	k.y = make([]byte, len(ybuf))
+	copy(k.y, ybuf)
+	k.d = make([]byte, len(dbuf))
+	copy(k.d, dbuf)
+
 	switch rawKey.Curve {
 	case elliptic.P256():
 		if err := k.Set(ECDSACrvKey, jwa.P256); err != nil {
@@ -77,8 +97,6 @@ func (k *ecdsaPrivateKey) FromRaw(rawKey *ecdsa.PrivateKey) error {
 		return errors.Errorf(`invalid elliptic curve %s`, rawKey.Curve)
 	}
 
-	k.d = rawKey.D.Bytes()
-
 	return nil
 }
 


Test patch:
diff --git a/jwe/jwe_test.go b/jwe/jwe_test.go
index 90974723..c53543c9 100644
--- a/jwe/jwe_test.go
+++ b/jwe/jwe_test.go
@@ -375,9 +375,7 @@ func TestEncode_ECDH(t *testing.T) {
 	}
 
 	algorithms := []jwa.KeyEncryptionAlgorithm{
-		// XXX for ECDH-ES
-		// uncomment the next line
-		// jwa.ECDH_ES,
+		jwa.ECDH_ES,
 		jwa.ECDH_ES_A256KW,
 		jwa.ECDH_ES_A192KW,
 		jwa.ECDH_ES_A128KW,
@@ -417,10 +415,6 @@ func TestEncode_ECDH(t *testing.T) {
 }
 
 func Test_GHIssue207(t *testing.T) {
-	// XXX for ECDH-ES
-	// Remove the t.SkipNow()
-	t.SkipNow()
-
 	const plaintext = "hi\n"
 	var testcases = []struct {
 		Algorithm  jwa.KeyEncryptionAlgorithm
diff --git a/jwx_test.go b/jwx_test.go
index 4e09ec60..2cd82b26 100644
--- a/jwx_test.go
+++ b/jwx_test.go
@@ -169,7 +169,10 @@ func TestJoseCompatibility(t *testing.T) {
 			{jwa.RSA_OAEP_256, jwa.A128GCM},
 			{jwa.RSA_OAEP_256, jwa.A128CBC_HS256},
 			{jwa.RSA_OAEP_256, jwa.A256CBC_HS512},
-			// {jwa.ECDH_ES, jwa.A256CBC_HS512},
+			{jwa.ECDH_ES, jwa.A128GCM},
+			{jwa.ECDH_ES, jwa.A256GCM},
+			{jwa.ECDH_ES, jwa.A128CBC_HS256},
+			{jwa.ECDH_ES, jwa.A256CBC_HS512},
 			{jwa.ECDH_ES_A128KW, jwa.A128GCM},
 			{jwa.ECDH_ES_A128KW, jwa.A128CBC_HS256},
 			{jwa.ECDH_ES_A256KW, jwa.A256GCM},