Merge branch 'master' into v1

* master:
  Better docs explaining embedded JWKs
  Reject invalid embedded public keys
  Improve multi-recipient/multi-sig handling

crypter.go

@@ -19,6 +19,7 @@ package jose
 import (
 	"crypto/ecdsa"
 	"crypto/rsa"
+	"errors"
 	"fmt"
 	"reflect"
 )
@@ -292,10 +293,16 @@ func (ctx *genericEncrypter) EncryptWithAuthData(plaintext, aad []byte) (*JsonWe
 	return obj, nil
 }
 
-// Decrypt and validate the object and return the plaintext.
+// Decrypt and validate the object and return the plaintext. Note that this
+// function does not support multi-recipient, if you desire multi-recipient
+// decryption use DecryptMulti instead.
 func (obj JsonWebEncryption) Decrypt(decryptionKey interface{}) ([]byte, error) {
 	headers := obj.mergedHeaders(nil)
 
+	if len(obj.recipients) > 1 {
+		return nil, errors.New("square/go-jose: too many recipients in payload; expecting only one")
+	}
+
 	if len(headers.Crit) > 0 {
 		return nil, fmt.Errorf("square/go-jose: unsupported crit header")
 	}
@@ -323,27 +330,87 @@ func (obj JsonWebEncryption) Decrypt(decryptionKey interface{}) ([]byte, error)
 	authData := obj.computeAuthData()
 
 	var plaintext []byte
-	for _, recipient := range obj.recipients {
+	recipient := obj.recipients[0]
+	recipientHeaders := obj.mergedHeaders(&recipient)
+
+	cek, err := decrypter.decryptKey(recipientHeaders, &recipient, generator)
+	if err == nil {
+		// Found a valid CEK -- let's try to decrypt.
+		plaintext, err = cipher.decrypt(cek, authData, parts)
+	}
+
+	if plaintext == nil {
+		return nil, ErrCryptoFailure
+	}
+
+	// The "zip" header parameter may only be present in the protected header.
+	if obj.protected.Zip != "" {
+		plaintext, err = decompress(obj.protected.Zip, plaintext)
+	}
+
+	return plaintext, err
+}
+
+// DecryptMulti decrypts and validates the object and returns the plaintexts,
+// with support for multiple recipients. It returns the index of the recipient
+// for which the decryption was successful, the merged headers for that recipient,
+// and the plaintext.
+func (obj JsonWebEncryption) DecryptMulti(decryptionKey interface{}) (int, JoseHeader, []byte, error) {
+	globalHeaders := obj.mergedHeaders(nil)
+
+	if len(globalHeaders.Crit) > 0 {
+		return -1, JoseHeader{}, nil, fmt.Errorf("square/go-jose: unsupported crit header")
+	}
+
+	decrypter, err := newDecrypter(decryptionKey)
+	if err != nil {
+		return -1, JoseHeader{}, nil, err
+	}
+
+	cipher := getContentCipher(globalHeaders.Enc)
+	if cipher == nil {
+		return -1, JoseHeader{}, nil, fmt.Errorf("square/go-jose: unsupported enc value '%s'", string(globalHeaders.Enc))
+	}
+
+	generator := randomKeyGenerator{
+		size: cipher.keySize(),
+	}
+
+	parts := &aeadParts{
+		iv:         obj.iv,
+		ciphertext: obj.ciphertext,
+		tag:        obj.tag,
+	}
+
+	authData := obj.computeAuthData()
+
+	index := -1
+	var plaintext []byte
+	var headers rawHeader
+
+	for i, recipient := range obj.recipients {
 		recipientHeaders := obj.mergedHeaders(&recipient)
 
 		cek, err := decrypter.decryptKey(recipientHeaders, &recipient, generator)
 		if err == nil {
 			// Found a valid CEK -- let's try to decrypt.
 			plaintext, err = cipher.decrypt(cek, authData, parts)
 			if err == nil {
+				index = i
+				headers = recipientHeaders
 				break
 			}
 		}
 	}
 
-	if plaintext == nil {
-		return nil, ErrCryptoFailure
+	if plaintext == nil || err != nil {
+		return -1, JoseHeader{}, nil, ErrCryptoFailure
 	}
 
 	// The "zip" header parameter may only be present in the protected header.
 	if obj.protected.Zip != "" {
 		plaintext, err = decompress(obj.protected.Zip, plaintext)
 	}
 
-	return plaintext, err
+	return index, headers.sanitized(), plaintext, err
 }

crypter_test.go

@@ -272,7 +272,7 @@ func TestMultiRecipientJWE(t *testing.T) {
 
 	err = enc.AddRecipient(RSA_OAEP, &rsaTestKey.PublicKey)
 	if err != nil {
-		t.Error("error when adding RSA recipient", err)
+		t.Fatal("error when adding RSA recipient", err)
 	}
 
 	sharedKey := []byte{
@@ -282,45 +282,46 @@ func TestMultiRecipientJWE(t *testing.T) {
 
 	err = enc.AddRecipient(A256GCMKW, sharedKey)
 	if err != nil {
-		t.Error("error when adding AES recipient: ", err)
-		return
+		t.Fatal("error when adding AES recipient: ", err)
 	}
 
 	input := []byte("Lorem ipsum dolor sit amet")
 	obj, err := enc.Encrypt(input)
 	if err != nil {
-		t.Error("error in encrypt: ", err)
-		return
+		t.Fatal("error in encrypt: ", err)
 	}
 
 	msg := obj.FullSerialize()
 
 	parsed, err := ParseEncrypted(msg)
 	if err != nil {
-		t.Error("error in parse: ", err)
-		return
+		t.Fatal("error in parse: ", err)
 	}
 
-	output, err := parsed.Decrypt(rsaTestKey)
+	i, _, output, err := parsed.DecryptMulti(rsaTestKey)
 	if err != nil {
-		t.Error("error on decrypt with RSA: ", err)
-		return
+		t.Fatal("error on decrypt with RSA: ", err)
+	}
+
+	if i != 0 {
+		t.Fatal("recipient index should be 0 for RSA key")
 	}
 
 	if bytes.Compare(input, output) != 0 {
-		t.Error("Decrypted output does not match input: ", output, input)
-		return
+		t.Fatal("Decrypted output does not match input: ", output, input)
 	}
 
-	output, err = parsed.Decrypt(sharedKey)
+	i, _, output, err = parsed.DecryptMulti(sharedKey)
 	if err != nil {
-		t.Error("error on decrypt with AES: ", err)
-		return
+		t.Fatal("error on decrypt with AES: ", err)
+	}
+
+	if i != 1 {
+		t.Fatal("recipient index should be 1 for shared key")
 	}
 
 	if bytes.Compare(input, output) != 0 {
-		t.Error("Decrypted output does not match input", output, input)
-		return
+		t.Fatal("Decrypted output does not match input", output, input)
 	}
 }
 

jwk.go

@@ -217,7 +217,17 @@ func (k *JsonWebKey) Thumbprint(hash crypto.Hash) ([]byte, error) {
 	return h.Sum(nil), nil
 }
 
-// Valid checks that the key contains the expected parameters
+// IsPublic returns true if the JWK represents a public key (not symmetric, not private).
+func (k *JsonWebKey) IsPublic() bool {
+	switch k.Key.(type) {
+	case *ecdsa.PublicKey, *rsa.PublicKey:
+		return true
+	default:
+		return false
+	}
+}
+
+// Valid checks that the key contains the expected parameters.
 func (k *JsonWebKey) Valid() bool {
 	if k.Key == nil {
 		return false

jws.go

@@ -17,6 +17,7 @@
 package jose
 
 import (
+	"errors"
 	"fmt"
 	"strings"
 
@@ -41,7 +42,10 @@ type rawSignatureInfo struct {
 
 // JsonWebSignature represents a signed JWS object after parsing.
 type JsonWebSignature struct {
-	payload    []byte
+	payload []byte
+	// Signatures attached to this object (may be more than one for multi-sig).
+	// Be careful about accessing these directly, prefer to use Verify() or
+	// VerifyMulti() to ensure that the data you're getting is verified.
 	Signatures []Signature
 }
 
@@ -126,6 +130,7 @@ func (parsed *rawJsonWebSignature) sanitized() (*JsonWebSignature, error) {
 			}
 		}
 
+		// Check that there is not a nonce in the unprotected header
 		if parsed.Header != nil && parsed.Header.Nonce != "" {
 			return nil, ErrUnprotectedNonce
 		}
@@ -148,6 +153,13 @@ func (parsed *rawJsonWebSignature) sanitized() (*JsonWebSignature, error) {
 		}
 
 		signature.Header = signature.mergedHeaders().sanitized()
+
+		// As per RFC 7515 Section 4.1.3, only public keys are allowed to be embedded.
+		jwk := signature.Header.JsonWebKey
+		if jwk != nil && (!jwk.Valid() || !jwk.IsPublic()) {
+			return nil, errors.New("square/go-jose: invalid embedded jwk, must be public key")
+		}
+
 		obj.Signatures = append(obj.Signatures, signature)
 	}
 
@@ -165,14 +177,20 @@ func (parsed *rawJsonWebSignature) sanitized() (*JsonWebSignature, error) {
 			return nil, ErrUnprotectedNonce
 		}
 
+		obj.Signatures[i].Header = obj.Signatures[i].mergedHeaders().sanitized()
 		obj.Signatures[i].Signature = sig.Signature.bytes()
 
+		// As per RFC 7515 Section 4.1.3, only public keys are allowed to be embedded.
+		jwk := obj.Signatures[i].Header.JsonWebKey
+		if jwk != nil && (!jwk.Valid() || !jwk.IsPublic()) {
+			return nil, errors.New("square/go-jose: invalid embedded jwk, must be public key")
+		}
+
 		// Copy value of sig
 		original := sig
 
 		obj.Signatures[i].header = sig.Header
 		obj.Signatures[i].original = &original
-		obj.Signatures[i].Header = obj.Signatures[i].mergedHeaders().sanitized()
 	}
 
 	return obj, nil

jws_test.go

@@ -22,6 +22,16 @@ import (
 	"testing"
 )
 
+func TestEmbeddedHMAC(t *testing.T) {
+	// protected: {"alg":"HS256", "jwk":{"kty":"oct", "k":"MTEx"}}, aka HMAC key.
+	msg := `{"payload":"TG9yZW0gaXBzdW0gZG9sb3Igc2l0IGFtZXQ","protected":"eyJhbGciOiJIUzI1NiIsICJqd2siOnsia3R5Ijoib2N0IiwgImsiOiJNVEV4In19","signature":"lvo41ZZsuHwQvSh0uJtEXRR3vmuBJ7in6qMoD7p9jyo"}`
+
+	_, err := ParseSigned(msg)
+	if err == nil {
+		t.Error("should not allow parsing JWS with embedded JWK with HMAC key")
+	}
+}
+
 func TestCompactParseJWS(t *testing.T) {
 	// Should parse
 	msg := "eyJhbGciOiJYWVoifQ.cGF5bG9hZA.c2lnbmF0dXJl"

signing.go

@@ -19,6 +19,7 @@ package jose
 import (
 	"crypto/ecdsa"
 	"crypto/rsa"
+	"errors"
 	"fmt"
 )
 
@@ -186,20 +187,59 @@ func (ctx *genericSigner) SetNonceSource(source NonceSource) {
 	ctx.nonceSource = source
 }
 
-// SetEmbedJwk specifies if the signing key should be embedded in the protected header,
-// if any. It defaults to 'true'.
+// SetEmbedJwk specifies if the signing key should be embedded in the protected
+// header, if any. It defaults to 'true', though that may change in the future.
+// Note that the use of embedded JWKs in the signature header can be dangerous,
+// as you cannot assume that the key received in a payload is trusted.
 func (ctx *genericSigner) SetEmbedJwk(embed bool) {
 	ctx.embedJwk = embed
 }
 
 // Verify validates the signature on the object and returns the payload.
+// This function does not support multi-signature, if you desire multi-sig
+// verification use VerifyMulti instead.
+//
+// Be careful when verifying signatures based on embedded JWKs inside the
+// payload header. You cannot assume that the key received in a payload is
+// trusted.
 func (obj JsonWebSignature) Verify(verificationKey interface{}) ([]byte, error) {
 	verifier, err := newVerifier(verificationKey)
 	if err != nil {
 		return nil, err
 	}
 
-	for _, signature := range obj.Signatures {
+	if len(obj.Signatures) > 1 {
+		return nil, errors.New("square/go-jose: too many signatures in payload; expecting only one")
+	}
+
+	signature := obj.Signatures[0]
+	headers := signature.mergedHeaders()
+	if len(headers.Crit) > 0 {
+		// Unsupported crit header
+		return nil, ErrCryptoFailure
+	}
+
+	input := obj.computeAuthData(&signature)
+	alg := SignatureAlgorithm(headers.Alg)
+	err = verifier.verifyPayload(input, signature.Signature, alg)
+	if err == nil {
+		return obj.payload, nil
+	}
+
+	return nil, ErrCryptoFailure
+}
+
+// VerifyMulti validates (one of the multiple) signatures on the object and
+// returns the index of the signature that was verified, along with the signature
+// object and the payload. We return the signature and index to guarantee that
+// callers are getting the verified value.
+func (obj JsonWebSignature) VerifyMulti(verificationKey interface{}) (int, Signature, []byte, error) {
+	verifier, err := newVerifier(verificationKey)
+	if err != nil {
+		return -1, Signature{}, nil, err
+	}
+
+	for i, signature := range obj.Signatures {
 		headers := signature.mergedHeaders()
 		if len(headers.Crit) > 0 {
 			// Unsupported crit header
@@ -210,9 +250,9 @@ func (obj JsonWebSignature) Verify(verificationKey interface{}) ([]byte, error)
 		alg := SignatureAlgorithm(headers.Alg)
 		err := verifier.verifyPayload(input, signature.Signature, alg)
 		if err == nil {
-			return obj.payload, nil
+			return i, signature, obj.payload, nil
 		}
 	}
 
-	return nil, ErrCryptoFailure
+	return -1, Signature{}, nil, ErrCryptoFailure
 }