Merge pull request #282 from square/cs/jws-panic

Better error handling around invalid headers

jws.go

@@ -102,14 +102,14 @@ func (sig Signature) mergedHeaders() rawHeader {
 }
 
 // Compute data to be signed
-func (obj JSONWebSignature) computeAuthData(payload []byte, signature *Signature) []byte {
+func (obj JSONWebSignature) computeAuthData(payload []byte, signature *Signature) ([]byte, error) {
 	var authData bytes.Buffer
 
 	protectedHeader := new(rawHeader)
 
 	if signature.original != nil && signature.original.Protected != nil {
 		if err := json.Unmarshal(signature.original.Protected.bytes(), protectedHeader); err != nil {
-			panic(err)
+			return nil, err
 		}
 		authData.WriteString(signature.original.Protected.base64())
 	} else if signature.protected != nil {
@@ -134,7 +134,7 @@ func (obj JSONWebSignature) computeAuthData(payload []byte, signature *Signature
 		authData.Write(payload)
 	}
 
-	return authData.Bytes()
+	return authData.Bytes(), nil
 }
 
 // parseSignedFull parses a message in full format.

jws_test.go

@@ -18,8 +18,11 @@ package jose
 
 import (
 	"crypto/x509"
+	"encoding/base64"
 	"strings"
 	"testing"
+
+	"github.com/stretchr/testify/assert"
 )
 
 const trustedCA = `
@@ -647,3 +650,39 @@ func TestDetachedCompactSerialization(t *testing.T) {
 		t.Fatalf("got '%s', expected '%s'", ser, msg)
 	}
 }
+
+func TestJWSComputeAuthDataBase64(t *testing.T) {
+	jws := JSONWebSignature{}
+
+	_, err := jws.computeAuthData([]byte{0x01}, &Signature{
+		original: &rawSignatureInfo{
+			Protected: newBuffer([]byte("{!invalid-json}")),
+		},
+	})
+	// Invalid header, should return error
+	assert.NotNil(t, err)
+
+	payload := []byte{0x01}
+	encodedPayload := base64.RawURLEncoding.EncodeToString(payload)
+
+	b64TrueHeader := newBuffer([]byte(`{"alg":"RSA-OAEP","enc":"A256GCM","b64":true}`))
+	b64FalseHeader := newBuffer([]byte(`{"alg":"RSA-OAEP","enc":"A256GCM","b64":false}`))
+
+	data, err := jws.computeAuthData(payload, &Signature{
+		original: &rawSignatureInfo{
+			Protected: b64TrueHeader,
+		},
+	})
+	assert.Nil(t, err)
+	// Payload should be b64 encoded
+	assert.Len(t, data, len(b64TrueHeader.base64())+len(encodedPayload)+1)
+
+	data, err = jws.computeAuthData(payload, &Signature{
+		original: &rawSignatureInfo{
+			Protected: b64FalseHeader,
+		},
+	})
+	assert.Nil(t, err)
+	// Payload should *not* be b64 encoded
+	assert.Len(t, data, len(b64FalseHeader.base64())+len(payload)+1)
+}

signing.go

@@ -370,7 +370,11 @@ func (obj JSONWebSignature) DetachedVerify(payload []byte, verificationKey inter
 		}
 	}
 
-	input := obj.computeAuthData(payload, &signature)
+	input, err := obj.computeAuthData(payload, &signature)
+	if err != nil {
+		return ErrCryptoFailure
+	}
+
 	alg := headers.getSignatureAlgorithm()
 	err = verifier.verifyPayload(input, signature.Signature, alg)
 	if err == nil {
@@ -421,7 +425,11 @@ outer:
 			}
 		}
 
-		input := obj.computeAuthData(payload, &signature)
+		input, err := obj.computeAuthData(payload, &signature)
+		if err != nil {
+			continue
+		}
+
 		alg := headers.getSignatureAlgorithm()
 		err = verifier.verifyPayload(input, signature.Signature, alg)
 		if err == nil {