Confusing error when processing JWT with trailing garbage.

[setrofim]

Contribution Guidelines

Before filing an issue, please read the contents of CONTRIBUTING.md, and follow its instructions.

Describe the bug

When attempting to process a JWT in JWS Compact format that does not have the "typ" header set and contains trailing data, jwt.Parse attempts to decode the token as uncompacted JSON, and produces an error about invalid initial character that misrepresents the actual issue.

Please attach the output of go version

$ go version
go version go1.19.3 linux/amd64

To Reproduce / Expected behavior
For example:

package main

import (
	"log"

	"github.com/lestrrat-go/jwx/v2/jwa"
	"github.com/lestrrat-go/jwx/v2/jwk"
	"github.com/lestrrat-go/jwx/v2/jwt"
)

var (
	testECDSAPublicKey = `{
		"kty": "EC",
		"crv": "P-256",
		"x": "usWxHK2PmfnHKwXPS54m0kTcGJ90UiglWiGahtagnv8",
		"y": "IBOL-C3BttVivg-lSreASjpkttcsz-1rb7btKLv8EX4"
	}`

	testToken = `eyJhbGciOiJFUzI1NiJ9.eyJzdWIiOiJ0ZXN0IiwiaWF0IjoxNjY2MDkxMzczLCJmb28iOiJiYXIifQ.3GWevx1z2_uCBB9Vj-D0rsT_CMsMeP9GP2rEqGDWpesoG8nHEjAXJOEQV1jOVkkCtTnS18JhcQdb7dW4i-zmqg.trailing-rubbish`
)

func main() {
	key, err := jwk.ParseKey([]byte(testECDSAPublicKey))
	if err != nil {
		log.Fatal(err)
	}

	_, err = jwt.Parse([]byte(testToken), jwt.WithKey(jwa.ES256, key))
	if err != nil {
		log.Fatal(err)
	}

}

Results in

2022/11/22 13:36:37 failed to parse token: invalid character 'e' looking for beginning of value

The real problem is not with the initial 'e' in the payload, but with the trailing ".trailing-rubbish" after the signature. The reason this occurs is that parse() inside jwt.go

1. Initially calls jwx.GuessFormat() which returns jwx.UnknownFormat because the input (a) doesn't start with a '{', and (b) doesn't have either two or four '.'-separated parts. This makes sense. However,
2. It then calls verifyJWS() assuming that the unknown format is in fact a JWS, and discards the returned base64-decoded JSON, only checking the error. The error is nil as the supplied data is in fact a valid JWS (with additional trailing data, which verifyJWS() ignores). Contrast this to the jwx.JWS, rather than jwx.UnknownFormat path, where verifyJWS() is also called, but the returned based64-decode JSON is reassigned as payload.
3. Finally, json.Unmarshal is called. Since the output of verifyJWS has been discarded, and the payload is still base64-encoded, and this fails with the error reported above.

Expected Behavior

Since the supplied data has trailing garbage, and is not as a whole, a valid JWT, it makes sense that an attempt to parse it produces an error. However, the error itself is misleading, and can result in time being wasted in looking for the problem in the wrong place. As such any of the following alternative would be preferable:

• Do not attempt to verify the unknown format and immediately report that as the error, without attempting to parse it.
• Detect possible trailing data (possibly inside `jwx.GuessFormat(), or elsewhere) report that fact as an error (more work than the above, but results in a more informative error).
• (less preferable) Update the unknown format branch inside parse() to behave analogously to the JWS branch and update the payload with the decode version if the call to verifyJWS succeeds. docs for jwt.Parse should mention this handling of trailing data.

Additional context
N/A

[lestrrat]

@setrofim Want to take this for a whirl? #851

[setrofim]

Works great. Thanks for the quick response!

[lestrrat]

Cool. I merged it, but please be aware that I won't be making a release for a few days at least, in order to make sure that nothing else broke or I missed anything. Please use the unreleased version until then. Thanks for the heads up

[lestrrat]

fixed by #851