Repository: lestrrat-go/jwx
Pull request #1078 (2024-02-19 01:52:05)

[v2] jws: allow specifing parse format/jwt: disallow JSON
Solution #1077 for v2

Adds the ability to control which serialization formats are accepted in `jws.Parse` and `jws.Verify`, and further adds `jwt.WithCompactOnly(bool)` to the global settings to disallow parsing of JSON formats.

This makes jwx better adhere to the RFC while keeping backwards compatibility.

lestrrat: @P3ngu1nW

Patch:
diff --git a/Changes b/Changes
index 7c6ae61b..e1793867 100644
--- a/Changes
+++ b/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
diff --git a/jws/jws.go b/jws/jws.go
index 4a9e8194..096e4f22 100644
--- a/jws/jws.go
+++ b/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`)
 }
 
diff --git a/jws/options.go b/jws/options.go
index e374a85d..a6cc472c 100644
--- a/jws/options.go
+++ b/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 {
diff --git a/jws/options.yaml b/jws/options.yaml
index 65b5352c..75a2de82 100644
--- a/jws/options.yaml
+++ b/jws/options.yaml
@@ -7,6 +7,9 @@ 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`
@@ -14,6 +17,7 @@ interfaces:
     methods:
       - signOption
       - verifyOption
+      - parseOption
     comment: |
       SignVerifyOption describes options that can be passed to either `jws.Verify` or `jws.Sign`
   - name: WithJSONSuboption
@@ -35,6 +39,12 @@ 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
@@ -42,7 +52,7 @@ options:
     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
diff --git a/jws/options_gen.go b/jws/options_gen.go
index ca834e10..fbef1ef3 100644
--- a/jws/options_gen.go
+++ b/jws/options_gen.go
@@ -64,6 +64,7 @@ type SignVerifyOption interface {
 	Option
 	signOption()
 	verifyOption()
+	parseOption()
 }
 
 type signVerifyOption struct {
@@ -74,10 +75,33 @@ func (*signVerifyOption) signOption() {}
 
 func (*signVerifyOption) verifyOption() {}
 
+func (*signVerifyOption) parseOption() {}
+
+type SignVerifyParseOption interface {
+	Option
+	signOption()
+	verifyOption()
+	parseOption()
+	readFileOption()
+}
+
+type signVerifyParseOption struct {
+	Option
+}
+
+func (*signVerifyParseOption) signOption() {}
+
+func (*signVerifyParseOption) verifyOption() {}
+
+func (*signVerifyParseOption) parseOption() {}
+
+func (*signVerifyParseOption) readFileOption() {}
+
 // VerifyOption describes options that can be passed to `jws.Verify`
 type VerifyOption interface {
 	Option
 	verifyOption()
+	parseOption()
 }
 
 type verifyOption struct {
@@ -86,6 +110,8 @@ type verifyOption struct {
 
 func (*verifyOption) verifyOption() {}
 
+func (*verifyOption) parseOption() {}
+
 // JSONSuboption describes suboptions that can be passed to `jws.WithJSON()` option
 type WithJSONSuboption interface {
 	Option
@@ -329,8 +355,8 @@ func WithRequireKid(v bool) WithKeySetSuboption {
 //
 // By default `jws.Sign()` will opt to use compact format, so you usually
 // do not need to specify this option other than to be explicit about it
-func WithCompact() SignOption {
-	return &signOption{option.New(identSerialization{}, fmtCompact)}
+func WithCompact() SignVerifyParseOption {
+	return &signVerifyParseOption{option.New(identSerialization{}, fmtCompact)}
 }
 
 // WithUseDefault specifies that if and only if a jwk.Key contains
diff --git a/jwt/jwt.go b/jwt/jwt.go
index fa0c2021..c09ea8bf 100644
--- a/jwt/jwt.go
+++ b/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)
 			}
diff --git a/jwt/options.yaml b/jwt/options.yaml
index 2a11b9b4..5e257801 100644
--- a/jwt/options.yaml
+++ b/jwt/options.yaml
@@ -82,6 +82,16 @@ options:
 
       See the documentation for `jwt.TokenOptionSet`, `(jwt.Token).Options`, and
       `jwt.FlattenAudience` for more details
+  - ident: CompactOnly
+    interface: GlobalOption
+    argument_type: bool
+    comment: |
+      WithCompactOnly option controls whether jwt.Parse should accept only tokens
+      that are in compact serialization format. RFC7519 specifies that JWTs
+      should be serialized in JWS compact form only, but historically this library
+      allowed for deserialization of JWTs in JWS's JSON serialization format.
+      Specifying this option will disable this behavior, and will report
+      errots if the token is not in compact serialization format.
   - ident: FormKey
     interface: ParseOption
     argument_type: string
diff --git a/jwt/options_gen.go b/jwt/options_gen.go
index ebde2d61..375d704e 100644
--- a/jwt/options_gen.go
+++ b/jwt/options_gen.go
@@ -124,6 +124,7 @@ func (*validateOption) validateOption() {}
 
 type identAcceptableSkew struct{}
 type identClock struct{}
+type identCompactOnly struct{}
 type identContext struct{}
 type identEncryptOption struct{}
 type identFS struct{}
@@ -150,6 +151,10 @@ func (identClock) String() string {
 	return "WithClock"
 }
 
+func (identCompactOnly) String() string {
+	return "WithCompactOnly"
+}
+
 func (identContext) String() string {
 	return "WithContext"
 }
@@ -230,6 +235,16 @@ func WithClock(v Clock) ValidateOption {
 	return &validateOption{option.New(identClock{}, v)}
 }
 
+// WithCompactOnly option controls whether jwt.Parse should accept only tokens
+// that are in compact serialization format. RFC7519 specifies that JWTs
+// should be serialized in JWS compact form only, but historically this library
+// allowed for deserialization of JWTs in JWS's JSON serialization format.
+// Specifying this option will disable this behavior, and will report
+// errots if the token is not in compact serialization format.
+func WithCompactOnly(v bool) GlobalOption {
+	return &globalOption{option.New(identCompactOnly{}, v)}
+}
+
 // WithContext allows you to specify a context.Context object to be used
 // with `jwt.Validate()` option.
 //


Test patch:
diff --git a/jws/jws_test.go b/jws/jws_test.go
index 04fdb9d9..ecf54085 100644
--- a/jws/jws_test.go
+++ b/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`)
+}
diff --git a/jwt/jwt_test.go b/jwt/jwt_test.go
index 8e94faa3..86f323d7 100644
--- a/jwt/jwt_test.go
+++ b/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`)
+			}
+		})
+	}
+}
diff --git a/jwt/options_gen_test.go b/jwt/options_gen_test.go
index bf7b55ed..1ad69c2b 100644
--- a/jwt/options_gen_test.go
+++ b/jwt/options_gen_test.go
@@ -11,6 +11,7 @@ import (
 func TestOptionIdent(t *testing.T) {
 	require.Equal(t, "WithAcceptableSkew", identAcceptableSkew{}.String())
 	require.Equal(t, "WithClock", identClock{}.String())
+	require.Equal(t, "WithCompactOnly", identCompactOnly{}.String())
 	require.Equal(t, "WithContext", identContext{}.String())
 	require.Equal(t, "WithEncryptOption", identEncryptOption{}.String())
 	require.Equal(t, "WithFS", identFS{}.String())