[v2] jws: allow specifing parse format/jwt: disallow JSON (#1078)

* jws: allow specifing parse format/jwt: disallow JSON

* preserve back compat

* appease linter

* Fix comment formatting

* be more pedantic about the format branches

* Update Changes

* typo

* typo

Changes

@@ -6,9 +6,18 @@ v1 and v2, please read the Changes-v2.md file (https://github.com/lestrrat-go/jw
 
 v2.0.20 UNRELEASED
 [New Features]
-  * [jwe] Added jwe.Settings(WithMaxBufferSize(int64)) to set the maximum size of
+  * [jwe] Added `jwe.Settings(WithMaxBufferSize(int64))` to set the maximum size of
     internal buffers. The default value is 256MB. Most users do not need to change
     this value.
+  * [jws] Allow `jws.WithCompact()` and `jws.WithJSON()` to be passed to `jws.Parse()` and
+    `jws.Verify()`. These options control the expected serialization format for the
+    JWS message.
+  * [jwt] Add `jwt.WithCompactOnly()` to specify that only compact serialization can
+    be used for `jwt.Parse()`. Previously, by virtue of `jws.Parse()` allowing either
+    JSON or Compact serialization format, `jwt.Parse()` also alloed JSON serialization
+    where as RFC7519 explicitly states that only compact serialization should be
+    used. For backward compatibility the default behavior is not changed, but you
+    can set this global option for jwt: `jwt.Settings(jwt.WithCompactOnly(true))`
 
 [Miscellaneous]
   * Internal key conversions should now allow private keys to be used in place of

jws/jws.go

@@ -103,7 +103,7 @@ func makeSigner(alg jwa.SignatureAlgorithm, key interface{}, public, protected H
 }
 
 const (
-	fmtInvalid = iota
+	fmtInvalid = 1 << iota
 	fmtCompact
 	fmtJSON
 	fmtJSONPretty
@@ -314,6 +314,7 @@ var allowNoneWhitelist = jwk.WhitelistFunc(func(string) bool {
 // accept messages with "none" signature algorithm, use `jws.Parse` to get the
 // raw JWS message.
 func Verify(buf []byte, options ...VerifyOption) ([]byte, error) {
+	var parseOptions []ParseOption
 	var dst *Message
 	var detachedPayload []byte
 	var keyProviders []KeyProvider
@@ -347,6 +348,8 @@ func Verify(buf []byte, options ...VerifyOption) ([]byte, error) {
 			ctx = option.Value().(context.Context)
 		case identValidateKey{}:
 			validateKey = option.Value().(bool)
+		case identSerialization{}:
+			parseOptions = append(parseOptions, option.(ParseOption))
 		default:
 			return nil, fmt.Errorf(`invalid jws.VerifyOption %q passed`, `With`+strings.TrimPrefix(fmt.Sprintf(`%T`, option.Ident()), `jws.ident`))
 		}
@@ -356,7 +359,7 @@ func Verify(buf []byte, options ...VerifyOption) ([]byte, error) {
 		return nil, fmt.Errorf(`jws.Verify: no key providers have been provided (see jws.WithKey(), jws.WithKeySet(), jws.WithVerifyAuto(), and jws.WithKeyProvider()`)
 	}
 
-	msg, err := Parse(buf)
+	msg, err := Parse(buf, parseOptions...)
 	if err != nil {
 		return nil, fmt.Errorf(`failed to parse jws: %w`, err)
 	}
@@ -523,23 +526,47 @@ func readAll(rdr io.Reader) ([]byte, bool) {
 }
 
 // Parse parses contents from the given source and creates a jws.Message
-// struct. The input can be in either compact or full JSON serialization.
+// struct. By default the input can be in either compact or full JSON serialization.
 //
-// Parse() currently does not take any options, but the API accepts it
-// in anticipation of future addition.
-func Parse(src []byte, _ ...ParseOption) (*Message, error) {
-	for i := 0; i < len(src); i++ {
-		r := rune(src[i])
-		if r >= utf8.RuneSelf {
-			r, _ = utf8.DecodeRune(src)
+// You may pass `jws.WithJSON()` and/or `jws.WithCompact()` to specify
+// explicitly which format to use. If neither or both is specified, the function
+// will attempt to autodetect the format. If one or the other is specified,
+// only the specified format will be attempted.
+func Parse(src []byte, options ...ParseOption) (*Message, error) {
+	var formats int
+	for _, option := range options {
+		//nolint:forcetypeassert
+		switch option.Ident() {
+		case identSerialization{}:
+			switch option.Value().(int) {
+			case fmtJSON:
+				formats |= fmtJSON
+			case fmtCompact:
+				formats |= fmtCompact
+			}
 		}
-		if !unicode.IsSpace(r) {
-			if r == '{' {
-				return parseJSON(src)
+	}
+
+	// if format is 0 or both JSON/Compact, auto detect
+	if v := formats & (fmtJSON | fmtCompact); v == 0 || v == fmtJSON|fmtCompact {
+		for i := 0; i < len(src); i++ {
+			r := rune(src[i])
+			if r >= utf8.RuneSelf {
+				r, _ = utf8.DecodeRune(src)
+			}
+			if !unicode.IsSpace(r) {
+				if r == '{' {
+					return parseJSON(src)
+				}
+				return parseCompact(src)
 			}
-			return parseCompact(src)
 		}
+	} else if formats&fmtCompact == fmtCompact {
+		return parseCompact(src)
+	} else if formats&fmtJSON == fmtJSON {
+		return parseJSON(src)
 	}
+
 	return nil, fmt.Errorf(`invalid byte sequence`)
 }
 

jws/jws_test.go

@@ -1887,3 +1887,53 @@ func TestEmptyProtectedField(t *testing.T) {
 	_, err = jws.Parse(invalidMessage)
 	require.Error(t, err, `jws.Parse should fail`)
 }
+
+func TestParseFormat(t *testing.T) {
+	privKey, err := jwxtest.GenerateRsaJwk()
+	require.NoError(t, err, `jwxtest.GenerateRsaJwk should succeed`)
+
+	signedCompact, err := jws.Sign([]byte("Lorem Ipsum"), jws.WithKey(jwa.RS256, privKey), jws.WithValidateKey(true))
+	require.NoError(t, err, `jws.Sign should succeed`)
+
+	signedJSON, err := jws.Sign([]byte("Lorem Ipsum"), jws.WithKey(jwa.RS256, privKey), jws.WithValidateKey(true), jws.WithJSON())
+	require.NoError(t, err, `jws.Sign should succeed`)
+
+	// Only compact formats should succeed
+	_, err = jws.Verify(signedCompact, jws.WithKey(jwa.RS256, privKey), jws.WithCompact())
+	require.NoError(t, err, `jws.Verify should succeed`)
+	_, err = jws.Verify(signedJSON, jws.WithKey(jwa.RS256, privKey), jws.WithCompact())
+	require.Error(t, err, `jws.Verify should fail`)
+	_, err = jws.Parse(signedCompact, jws.WithCompact())
+	require.NoError(t, err, `jws.Parse should succeed`)
+	_, err = jws.Parse(signedJSON, jws.WithCompact())
+	require.Error(t, err, `jws.Parse should fail`)
+
+	// Only JSON formats should succeed
+	_, err = jws.Verify(signedCompact, jws.WithKey(jwa.RS256, privKey), jws.WithJSON())
+	require.Error(t, err, `jws.Verify should fail`)
+	_, err = jws.Verify(signedJSON, jws.WithKey(jwa.RS256, privKey), jws.WithJSON())
+	require.NoError(t, err, `jws.Verify should succeed`)
+	_, err = jws.Parse(signedJSON, jws.WithJSON())
+	require.NoError(t, err, `jws.Parse should succeed`)
+	_, err = jws.Parse(signedCompact, jws.WithJSON())
+	require.Error(t, err, `jws.Parse should fail`)
+
+	// Either format should succeed
+	_, err = jws.Verify(signedCompact, jws.WithKey(jwa.RS256, privKey))
+	require.NoError(t, err, `jws.Verify should succeed`)
+	_, err = jws.Verify(signedCompact, jws.WithKey(jwa.RS256, privKey), jws.WithJSON(), jws.WithCompact())
+	require.NoError(t, err, `jws.Verify should succeed`)
+	_, err = jws.Parse(signedCompact)
+	require.NoError(t, err, `jws.Parse should succeed`)
+	_, err = jws.Parse(signedCompact, jws.WithJSON(), jws.WithCompact())
+	require.NoError(t, err, `jws.Parse should succeed`)
+
+	_, err = jws.Verify(signedJSON, jws.WithKey(jwa.RS256, privKey))
+	require.NoError(t, err, `jws.Verify should succeed`)
+	_, err = jws.Verify(signedJSON, jws.WithKey(jwa.RS256, privKey), jws.WithJSON(), jws.WithCompact())
+	require.NoError(t, err, `jws.Verify should succeed`)
+	_, err = jws.Parse(signedJSON)
+	require.NoError(t, err, `jws.Parse should succeed`)
+	_, err = jws.Parse(signedJSON, jws.WithJSON(), jws.WithCompact())
+	require.NoError(t, err, `jws.Parse should succeed`)
+}

jws/options.go

@@ -20,7 +20,7 @@ func WithHeaders(h Headers) SignOption {
 //
 // If you pass multiple keys to `jws.Sign()`, it will fail unless
 // you also pass this option.
-func WithJSON(options ...WithJSONSuboption) SignOption {
+func WithJSON(options ...WithJSONSuboption) SignVerifyParseOption {
 	var pretty bool
 	for _, option := range options {
 		//nolint:forcetypeassert
@@ -34,7 +34,7 @@ func WithJSON(options ...WithJSONSuboption) SignOption {
 	if pretty {
 		format = fmtJSONPretty
 	}
-	return &signOption{option.New(identSerialization{}, format)}
+	return &signVerifyParseOption{option.New(identSerialization{}, format)}
 }
 
 type withKey struct {

jws/options.yaml

@@ -7,13 +7,17 @@ interfaces:
   - name: VerifyOption
     comment: |
       VerifyOption describes options that can be passed to `jws.Verify`
+    methods:
+      - verifyOption
+      - parseOption
   - name: SignOption
     comment: |
       SignOption describes options that can be passed to `jws.Sign`
   - name: SignVerifyOption
     methods:
       - signOption
       - verifyOption
+      - parseOption
     comment: |
       SignVerifyOption describes options that can be passed to either `jws.Verify` or `jws.Sign`
   - name: WithJSONSuboption
@@ -35,14 +39,20 @@ interfaces:
   - name: ReadFileOption
     comment: |
       ReadFileOption is a type of `Option` that can be passed to `jws.ReadFile`
+  - name: SignVerifyParseOption
+    methods:
+      - signOption
+      - verifyOption
+      - parseOption
+      - readFileOption
 options:
   - ident: Key
     skip_option: true
   - ident: Serialization
     skip_option: true
   - ident: Serialization
     option_name: WithCompact
-    interface: SignOption
+    interface: SignVerifyParseOption
     constant_value: fmtCompact
     comment: |
       WithCompact specifies that the result of `jws.Sign()` is serialized in

jwt/jwt.go

@@ -17,6 +17,7 @@ import (
 	"github.com/lestrrat-go/jwx/v2/jwt/internal/types"
 )
 
+var compactOnly uint32
 var errInvalidJWT = errors.New(`invalid JWT`)
 
 // ErrInvalidJWT returns the opaque error value that is returned when
@@ -28,7 +29,8 @@ func ErrInvalidJWT() error {
 
 // Settings controls global settings that are specific to JWTs.
 func Settings(options ...GlobalOption) {
-	var flattenAudienceBool bool
+	var flattenAudience bool
+	var compactOnlyBool bool
 	var parsePedantic bool
 	var parsePrecision = types.MaxPrecision + 1  // illegal value, so we can detect nothing was set
 	var formatPrecision = types.MaxPrecision + 1 // illegal value, so we can detect nothing was set
@@ -37,7 +39,9 @@ func Settings(options ...GlobalOption) {
 	for _, option := range options {
 		switch option.Ident() {
 		case identFlattenAudience{}:
-			flattenAudienceBool = option.Value().(bool)
+			flattenAudience = option.Value().(bool)
+		case identCompactOnly{}:
+			compactOnlyBool = option.Value().(bool)
 		case identNumericDateParsePedantic{}:
 			parsePedantic = option.Value().(bool)
 		case identNumericDateParsePrecision{}:
@@ -80,9 +84,20 @@ func Settings(options ...GlobalOption) {
 		}
 	}
 
+	{
+		v := atomic.LoadUint32(&compactOnly)
+		if (v == 1) != compactOnlyBool {
+			var newVal uint32
+			if compactOnlyBool {
+				newVal = 1
+			}
+			atomic.CompareAndSwapUint32(&compactOnly, v, newVal)
+		}
+	}
+
 	{
 		defaultOptionsMu.Lock()
-		if flattenAudienceBool {
+		if flattenAudience {
 			defaultOptions.Enable(FlattenAudience)
 		} else {
 			defaultOptions.Disable(FlattenAudience)
@@ -244,7 +259,11 @@ func verifyJWS(ctx *parseCtx, payload []byte) ([]byte, int, error) {
 		return nil, _JwsVerifySkipped, nil
 	}
 
-	verified, err := jws.Verify(payload, ctx.verifyOpts...)
+	verifyOpts := ctx.verifyOpts
+	if atomic.LoadUint32(&compactOnly) == 1 {
+		verifyOpts = append(verifyOpts, jws.WithCompact())
+	}
+	verified, err := jws.Verify(payload, verifyOpts...)
 	return verified, _JwsVerifyDone, err
 }
 
@@ -330,7 +349,11 @@ OUTER:
 			}
 
 			// No verification.
-			m, err := jws.Parse(data)
+			var parseOptions []jws.ParseOption
+			if atomic.LoadUint32(&compactOnly) == 1 {
+				parseOptions = append(parseOptions, jws.WithCompact())
+			}
+			m, err := jws.Parse(data, parseOptions...)
 			if err != nil {
 				return nil, fmt.Errorf(`invalid jws message: %w`, err)
 			}

jwt/jwt_test.go

@@ -1773,3 +1773,33 @@ func TestGH1007(t *testing.T) {
 	_, err = jwt.ParseInsecure(signed, jwt.WithKey(jwa.RS256, wrongPubKey))
 	require.NoError(t, err, `jwt.ParseInsecure with jwt.WithKey() should succeed`)
 }
+
+func TestParseJSON(t *testing.T) {
+	// NOTE: jwt.Settings has global effect!
+	defer jwt.Settings(jwt.WithCompactOnly(false))
+	for _, compactOnly := range []bool{true, false} {
+		t.Run("compactOnly="+strconv.FormatBool(compactOnly), func(t *testing.T) {
+			jwt.Settings(jwt.WithCompactOnly(compactOnly))
+
+			privKey, err := jwxtest.GenerateRsaJwk()
+			require.NoError(t, err, `jwxtest.GenerateRsaJwk should succeed`)
+
+			signedJSON, err := jws.Sign([]byte(`{}`), jws.WithKey(jwa.RS256, privKey), jws.WithValidateKey(true), jws.WithJSON())
+			require.NoError(t, err, `jws.Sign should succeed`)
+
+			// jws.Verify should succeed
+			_, err = jws.Verify(signedJSON, jws.WithKey(jwa.RS256, privKey))
+			require.NoError(t, err, `jws.Parse should succeed`)
+
+			if compactOnly {
+				// jwt.Parse should fail
+				_, err = jwt.Parse(signedJSON, jwt.WithKey(jwa.RS256, privKey))
+				require.Error(t, err, `jws.Parse should fail`)
+			} else {
+				// for backward compatibility, this should succeed
+				_, err = jwt.Parse(signedJSON, jwt.WithKey(jwa.RS256, privKey))
+				require.NoError(t, err, `jws.Parse should succeed`)
+			}
+		})
+	}
+}