Handling malformed JWT (includes padding)

[is-alnilam]

I'm trying to integrate with Amazon Cognito behind an AWS load balancer. Cognito supplies a JWT, but the token includes padding. Yes, this makes it a malformed token, but it's not a token which I can change. (Specifically, when running behind an Application Load Balancer, I need to validate the x-amzn-oidc-data header. Infuriatingly, they also provide a second JWT, which is not malformed, but doesn't include some specific details which I need...)

v3.2.2 included PR#33, which changes how the library handles this situation. Prior to the change, the Base64 text was correctly parsed, as the decoded expected padding (and this was added if it was missing). Now, the base64 parser returns an error ("illegal base64 data").

Stripping the padding before passing to the library allows the base64 deserialisation to succeed, but the signature then fails to validate.

Currently my only option (other than looking for a different library) seems to be to stick to v3.2.1... Any other suggestions would be very welcome!

[mfridman]

iirc AWS Cognito returns 3 tokens: refresh, id and access token. Can you clarify which of the id or access tokens is malformed?

If possible, it'd be ideal to get sample tokens (for both id and access) so we can plug them into tests and then fix the behaviour. If not, I'll see if I can dig up old tokens.

[is-alnilam]

From the Amazon docs, I don't think it's any of those -- it's an additional JWT. The load balancer handles the refresh and access tokens (although it does pass the access token through), but also adds a user claims token. That's the one that's causing problems. The access token validates correctly.

The actual tokens contain some details from our app that I can't share, so providing real tokens will be a bit of a challenge. I could try to generate some equivalents, though, if that would work? Basically the payload and signature fields have '=' padding characters -- the header does not, on the examples I've seen, but this could just be chance given the length of the content.

[is-alnilam]

Sorry for the slow update... I've created an example JWT based on the Cognito token.

The token itself:
eyJ0eXAiOiJKV1QiLCJraWQiOiIxMjM0NTY3OC1hYmNkLTEyMzQtYWJjZC0xMjM0NTY3OGFiY2QiLCJhbGciOiJFUzI1NiIsImlzcyI6Imh0dHBzOi8vY29nbml0by1pZHAuZXUtd2VzdC0yLmFtYXpvbmF3cy5jb20vIiwiY2xpZW50IjoiN0xUY29QWnJWNDR6ZVg2WUs5VktBcHZPM3EiLCJzaWduZXIiOiJhcm46YXdzOmVsYXN0aWNsb2FkYmFsYW5jaW5nIiwiZXhwIjoxNjI5NDcwMTAxfQ==.eyJzdWIiOiIxMjM0NTY3OC1hYmNkLTEyMzQtYWJjZC0xMjM0NTY3OGFiY2QiLCJlbWFpbF92ZXJpZmllZCI6InRydWUiLCJlbWFpbCI6InVzZXJAZXhhbXBsZS5jb20iLCJ1c2VybmFtZSI6IjEyMzQ1Njc4LWFiY2QtMTIzNC1hYmNkLTEyMzQ1Njc4YWJjZCIsImV4cCI6MTYyOTQ3MDEwMSwiaXNzIjoiaHR0cHM6Ly9jb2duaXRvLWlkcC5ldS13ZXN0LTIuYW1hem9uYXdzLmNvbS8ifQ==.sx0muJ754glJvwWgkHaPrOI3L1gaPjRLLUvOQRk0WitnqC5Dtt1knorcbOzlEcH9zwPM2jYYIAYQz_qEyM3grw==

To validate that, you can use this key:

-----BEGIN PUBLIC KEY-----
MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEIcaUjXhC7Mn2OonyfHF+zjblKkns
4GLbILnHrZr+aQwddiff5urCDAZ177t81Mn39CDs3uhlNDxfRIRheGnK/Q==
-----END PUBLIC KEY-----

I can validate that token successfully using 3.2.1 (although it's expired), but switching to 3.2.2 or later causes the error illegal base64 data at input byte 294

The test code I'm using is basically:

pubkey, err := jwt.ParseECPublicKeyFromPEM(raw_pubkey)
if err != nil {
    fmt.Println(err)
    return
}

tokenParser := new(jwt.Parser)
tokenParser.SkipClaimsValidation = true
token, err := tokenParser.Parse(test_jwt, func(token *jwt.Token) (interface{}, error) {
    return pubkey, nil
})
if token.Valid {
    fmt.Println("Token valid")
    fmt.Println(token.Claims)
} else {
    fmt.Println("Validation failed, err: ", err)
}

[mfridman]

Thanks for providing a repro.

I'll take a closer look to see if this can be fixed in a backwards-compatible way following #33

[ajermaky]

Hi @mfridman not sure if you got a chance to review this, but hoping we can resolve this soon. I am also running into issues where some repos are stuck at v3.2.1, and would love to upgrade.

I think the cleanest solution is to revert back specifically for the encoding, but then it will be a potential loss of optimization. Thus next option is to somehow switch between the old and new encodings. I quickly forked and implemented a quick PoC - it involves the creation of a global boolean, and using a function to set the value of the boolean to switch between using the current RawURLEncoding, or the previously used URLEncoding. Not a big fan of this solution, but could not see a better way to propogate this flag down without breaking changes: ajermaky@87f13f9

Below are metrics before and after the change. Note that before and after the change, the allocations are essentially identical - only enabling padding do we see that increase in allocations. Thus the change proposed above will allow the default behavior to keep the new optimizations, while giving an option for backwards compatibilty at the cost of a couple allocs.

Let me know if we should be going down a different route for this. Willing to help move this forward!

goos: linux
goarch: amd64
pkg: github.com/golang-jwt/jwt/v4
BenchmarkECDSAParsing/Basic_ES256-16         	  643087	      1725 ns/op	    1224 B/op	      39 allocs/op
BenchmarkECDSAParsing/Basic_ES384-16         	    1474	    761221 ns/op	 1737768 B/op	   14375 allocs/op
BenchmarkECDSAParsing/Basic_ES512-16         	    6580	    167854 ns/op	    1968 B/op	      44 allocs/op
BenchmarkECDSAParsing/basic_ES256_invalid:_foo_=>_bar-16         	  694071	      1691 ns/op	    1224 B/op	      39 allocs/op
BenchmarkECDSASigning/Basic_ES256-16                             	  330040	      3899 ns/op	    3871 B/op	      58 allocs/op
BenchmarkECDSASigning/Basic_ES256/sign-only-16                   	   44526	     27040 ns/op	    2946 B/op	      37 allocs/op
BenchmarkECDSASigning/Basic_ES384-16                             	    1507	    800376 ns/op	 1751594 B/op	   14474 allocs/op
BenchmarkECDSASigning/Basic_ES384/sign-only-16                   	     248	   4833601 ns/op	 1742866 B/op	   14392 allocs/op
BenchmarkECDSASigning/Basic_ES512-16                             	    5558	    182327 ns/op	    9157 B/op	      87 allocs/op
BenchmarkECDSASigning/Basic_ES512/sign-only-16                   	     772	   1551958 ns/op	    8114 B/op	      66 allocs/op
BenchmarkECDSASigning/basic_ES256_invalid:_foo_=>_bar-16         	  295987	      3931 ns/op	    3871 B/op	      58 allocs/op
BenchmarkHS256Signing-16                                                   	 1916628	       630.9 ns/op	    1554 B/op	      30 allocs/op
BenchmarkHS384Signing-16                                                   	 1563625	       757.1 ns/op	    1939 B/op	      30 allocs/op
BenchmarkHS512Signing-16                                                   	 1500824	       779.9 ns/op	    2035 B/op	      30 allocs/op
BenchmarkParseUnverified/map_claims-16                                     	 1519772	       797.8 ns/op	    2192 B/op	      35 allocs/op
BenchmarkParseUnverified/map_claims#01-16                                  	 1350411	       895.9 ns/op	    2328 B/op	      41 allocs/op
BenchmarkParseUnverified/map_claims#02-16                                  	 1339419	       903.0 ns/op	    2327 B/op	      41 allocs/op
BenchmarkParseUnverified/map_claims#03-16                                  	 1000000	      1004 ns/op	    2431 B/op	      46 allocs/op
BenchmarkParseUnverified/map_claims#04-16                                  	 1486888	       803.0 ns/op	    2192 B/op	      35 allocs/op
BenchmarkParseUnverified/map_claims#05-16                                  	 1501429	       802.0 ns/op	    2192 B/op	      35 allocs/op
BenchmarkParseUnverified/map_claims#06-16                                  	 1473259	       818.7 ns/op	    2192 B/op	      35 allocs/op
BenchmarkParseUnverified/map_claims#07-16                                  	 1459118	       822.5 ns/op	    2192 B/op	      35 allocs/op
BenchmarkParseUnverified/map_claims#08-16                                  	 1472215	       822.2 ns/op	    2192 B/op	      35 allocs/op
BenchmarkParseUnverified/map_claims#09-16                                  	 1484450	       815.4 ns/op	    2192 B/op	      35 allocs/op
BenchmarkParseUnverified/map_claims#10-16                                  	 1477605	       828.9 ns/op	    2192 B/op	      35 allocs/op
BenchmarkParseUnverified/map_claims#11-16                                  	 1479944	       812.6 ns/op	    2192 B/op	      35 allocs/op
BenchmarkParseUnverified/map_claims#12-16                                  	 1476196	       816.8 ns/op	    2192 B/op	      35 allocs/op
BenchmarkParseUnverified/standard_claims-16                                	 1412841	       841.5 ns/op	    2215 B/op	      35 allocs/op
BenchmarkParseUnverified/map_claims#13-16                                  	 1314297	       918.4 ns/op	    2328 B/op	      41 allocs/op
BenchmarkParseUnverified/map_claims#14-16                                  	 1307355	       932.0 ns/op	    2328 B/op	      41 allocs/op
BenchmarkParseUnverified/map_claims#15-16                                  	 1000000	      1034 ns/op	    2432 B/op	      46 allocs/op
BenchmarkParseUnverified/map_claims#16-16                                  	 1282791	       939.1 ns/op	    2328 B/op	      41 allocs/op
BenchmarkRSAParsing-16                                                     	  194726	      5517 ns/op	    9611 B/op	      93 allocs/op
BenchmarkRS256Signing-16                                                   	    8001	    150577 ns/op	   32934 B/op	     136 allocs/op
BenchmarkRS384Signing-16                                                   	    7653	    150839 ns/op	   33101 B/op	     136 allocs/op
BenchmarkRS512Signing-16                                                   	    7750	    154075 ns/op	   33193 B/op	     137 allocs/op
PASS
ok  	github.com/golang-jwt/jwt/v4	59.658s

After Change:
goos: linux
goarch: amd64
pkg: github.com/golang-jwt/jwt/v4
BenchmarkECDSAParsing/Basic_ES256-16         	  645340	      1703 ns/op	    1224 B/op	      39 allocs/op
BenchmarkECDSAParsing/Basic_ES384-16         	    1568	    761963 ns/op	 1737776 B/op	   14375 allocs/op
BenchmarkECDSAParsing/Basic_ES512-16         	    6567	    167055 ns/op	    1968 B/op	      44 allocs/op
BenchmarkECDSAParsing/basic_ES256_invalid:_foo_=>_bar-16         	  752683	      1713 ns/op	    1224 B/op	      39 allocs/op
BenchmarkECDSASigning/Basic_ES256-16                             	  309788	      3832 ns/op	    3871 B/op	      58 allocs/op
BenchmarkECDSASigning/Basic_ES256/sign-only-16                   	   44324	     26806 ns/op	    2946 B/op	      37 allocs/op
BenchmarkECDSASigning/Basic_ES384-16                             	    1521	    789826 ns/op	 1751615 B/op	   14475 allocs/op
BenchmarkECDSASigning/Basic_ES384/sign-only-16                   	     254	   4980440 ns/op	 1746695 B/op	   14422 allocs/op
BenchmarkECDSASigning/Basic_ES512-16                             	    5548	    181079 ns/op	    9156 B/op	      87 allocs/op
BenchmarkECDSASigning/Basic_ES512/sign-only-16                   	     764	   1529155 ns/op	    8114 B/op	      66 allocs/op
BenchmarkECDSASigning/basic_ES256_invalid:_foo_=>_bar-16         	  305359	      3860 ns/op	    3871 B/op	      58 allocs/op
BenchmarkHS256Signing-16                                                   	 1911904	       621.9 ns/op	    1554 B/op	      30 allocs/op
BenchmarkHS384Signing-16                                                   	 1585579	       767.0 ns/op	    1939 B/op	      30 allocs/op
BenchmarkHS512Signing-16                                                   	 1519774	       775.7 ns/op	    2035 B/op	      30 allocs/op
BenchmarkParseUnverified/map_claims-16                                     	 1513990	       795.4 ns/op	    2192 B/op	      35 allocs/op
BenchmarkParseUnverified/map_claims#01-16                                  	 1329654	       904.2 ns/op	    2328 B/op	      41 allocs/op
BenchmarkParseUnverified/map_claims#02-16                                  	 1311788	       896.8 ns/op	    2328 B/op	      41 allocs/op
BenchmarkParseUnverified/map_claims#03-16                                  	 1000000	      1006 ns/op	    2431 B/op	      46 allocs/op
BenchmarkParseUnverified/map_claims#04-16                                  	 1463382	       812.7 ns/op	    2192 B/op	      35 allocs/op
BenchmarkParseUnverified/map_claims#05-16                                  	 1474338	       817.7 ns/op	    2192 B/op	      35 allocs/op
BenchmarkParseUnverified/map_claims#06-16                                  	 1461229	       810.8 ns/op	    2192 B/op	      35 allocs/op
BenchmarkParseUnverified/map_claims#07-16                                  	 1465208	       822.9 ns/op	    2192 B/op	      35 allocs/op
BenchmarkParseUnverified/map_claims#08-16                                  	 1470631	       816.1 ns/op	    2192 B/op	      35 allocs/op
BenchmarkParseUnverified/map_claims#09-16                                  	 1448349	       818.6 ns/op	    2192 B/op	      35 allocs/op
BenchmarkParseUnverified/map_claims#10-16                                  	 1453848	       828.4 ns/op	    2192 B/op	      35 allocs/op
BenchmarkParseUnverified/map_claims#11-16                                  	 1480766	       821.6 ns/op	    2192 B/op	      35 allocs/op
BenchmarkParseUnverified/map_claims#12-16                                  	 1462684	       825.1 ns/op	    2192 B/op	      35 allocs/op
BenchmarkParseUnverified/standard_claims-16                                	 1395154	       857.1 ns/op	    2215 B/op	      35 allocs/op
BenchmarkParseUnverified/map_claims#13-16                                  	 1304074	       915.1 ns/op	    2328 B/op	      41 allocs/op
BenchmarkParseUnverified/map_claims#14-16                                  	 1316992	       921.3 ns/op	    2328 B/op	      41 allocs/op
BenchmarkParseUnverified/map_claims#15-16                                  	 1000000	      1025 ns/op	    2432 B/op	      46 allocs/op
BenchmarkParseUnverified/map_claims#16-16                                  	 1291711	       996.5 ns/op	    2328 B/op	      41 allocs/op
BenchmarkRSAParsing-16                                                     	  223902	      5870 ns/op	    9609 B/op	      93 allocs/op
BenchmarkRS256Signing-16                                                   	    7053	    156942 ns/op	   32917 B/op	     136 allocs/op
BenchmarkRS384Signing-16                                                   	    7232	    154037 ns/op	   33100 B/op	     136 allocs/op
BenchmarkRS512Signing-16                                                   	    7755	    155995 ns/op	   33186 B/op	     137 allocs/op
PASS
ok  	github.com/golang-jwt/jwt/v4	59.912s

After Change but with padding enabled:
goos: linux
goarch: amd64
pkg: github.com/golang-jwt/jwt/v4
BenchmarkECDSAParsing/Basic_ES256-16         	  701953	      1678 ns/op	    1224 B/op	      39 allocs/op
BenchmarkECDSAParsing/Basic_ES384-16         	    1636	    766376 ns/op	 1737703 B/op	   14374 allocs/op
BenchmarkECDSAParsing/Basic_ES512-16         	    6580	    166476 ns/op	    1968 B/op	      44 allocs/op
BenchmarkECDSAParsing/basic_ES256_invalid:_foo_=>_bar-16         	  704119	      1685 ns/op	    1224 B/op	      39 allocs/op
BenchmarkECDSASigning/Basic_ES256-16                             	  325940	      3882 ns/op	    3947 B/op	      61 allocs/op
BenchmarkECDSASigning/Basic_ES256/sign-only-16                   	   43352	     26958 ns/op	    2970 B/op	      38 allocs/op
BenchmarkECDSASigning/Basic_ES384-16                             	    1534	    788007 ns/op	 1749262 B/op	   14458 allocs/op
BenchmarkECDSASigning/Basic_ES384/sign-only-16                   	     241	   4876526 ns/op	 1746323 B/op	   14421 allocs/op
BenchmarkECDSASigning/Basic_ES512-16                             	    5846	    181743 ns/op	    9236 B/op	      90 allocs/op
BenchmarkECDSASigning/Basic_ES512/sign-only-16                   	     776	   1528120 ns/op	    8137 B/op	      67 allocs/op
BenchmarkECDSASigning/basic_ES256_invalid:_foo_=>_bar-16         	  275374	      4000 ns/op	    3947 B/op	      61 allocs/op
BenchmarkHS256Signing-16                                                   	 1847550	       664.5 ns/op	    1626 B/op	      33 allocs/op
BenchmarkHS384Signing-16                                                   	 1509555	       811.8 ns/op	    2011 B/op	      33 allocs/op
BenchmarkHS512Signing-16                                                   	 1416283	       851.4 ns/op	    2107 B/op	      33 allocs/op
BenchmarkParseUnverified/map_claims-16                                     	 1459735	       826.5 ns/op	    2216 B/op	      37 allocs/op
BenchmarkParseUnverified/map_claims#01-16                                  	 1331353	       918.0 ns/op	    2328 B/op	      41 allocs/op
BenchmarkParseUnverified/map_claims#02-16                                  	 1311770	       907.7 ns/op	    2327 B/op	      41 allocs/op
BenchmarkParseUnverified/map_claims#03-16                                  	 1000000	      1039 ns/op	    2495 B/op	      48 allocs/op
BenchmarkParseUnverified/map_claims#04-16                                  	 1430544	       844.0 ns/op	    2216 B/op	      37 allocs/op
BenchmarkParseUnverified/map_claims#05-16                                  	 1417198	       836.3 ns/op	    2216 B/op	      37 allocs/op
BenchmarkParseUnverified/map_claims#06-16                                  	 1410175	       839.6 ns/op	    2216 B/op	      37 allocs/op
BenchmarkParseUnverified/map_claims#07-16                                  	 1366664	       854.5 ns/op	    2216 B/op	      37 allocs/op
BenchmarkParseUnverified/map_claims#08-16                                  	 1391052	       854.7 ns/op	    2216 B/op	      37 allocs/op
BenchmarkParseUnverified/map_claims#09-16                                  	 1389924	       860.2 ns/op	    2216 B/op	      37 allocs/op
BenchmarkParseUnverified/map_claims#10-16                                  	 1381544	       858.4 ns/op	    2216 B/op	      37 allocs/op
BenchmarkParseUnverified/map_claims#11-16                                  	 1395728	       866.5 ns/op	    2216 B/op	      37 allocs/op
BenchmarkParseUnverified/map_claims#12-16                                  	 1365612	       878.5 ns/op	    2216 B/op	      37 allocs/op
BenchmarkParseUnverified/standard_claims-16                                	 1337755	       999.0 ns/op	    2215 B/op	      35 allocs/op
BenchmarkParseUnverified/map_claims#13-16                                  	 1291942	       934.1 ns/op	    2328 B/op	      41 allocs/op
BenchmarkParseUnverified/map_claims#14-16                                  	 1297461	       934.6 ns/op	    2328 B/op	      41 allocs/op
BenchmarkParseUnverified/map_claims#15-16                                  	 1000000	      1064 ns/op	    2496 B/op	      48 allocs/op
BenchmarkParseUnverified/map_claims#16-16                                  	 1282748	       936.4 ns/op	    2328 B/op	      41 allocs/op
BenchmarkRSAParsing-16                                                     	  230604	      5547 ns/op	    9611 B/op	      93 allocs/op
BenchmarkRS256Signing-16                                                   	    7684	    149654 ns/op	   33026 B/op	     139 allocs/op
BenchmarkRS384Signing-16                                                   	    7689	    149702 ns/op	   33195 B/op	     140 allocs/op
BenchmarkRS512Signing-16                                                   	    7432	    154962 ns/op	   33283 B/op	     140 allocs/op
PASS
ok  	github.com/golang-jwt/jwt/v4	61.688s

[lggomez]

@ajermaky I'm not sure about extending the package API this way as it won't scale nicely on changes or new value requirements (because the general problem to solve here is configuring the codec) but we're also limited on what we can configure, so I can be on board with this solution if @mfridman and @oxisto agree to save compatibility and be able to provide a fix to this problem

A suggestion I'd like to make though, since this will be public on a package level is to either:

• Point out it is not goroutine safe (since not even bool variable assignents are atomic)
• Go the sync/atomic way and use atomic.SwapUint64 having allowPadding as a *uint64 field

[ajermaky]

@lggomez Firstly, thank you for the suggestions! And I do agree that this isn't really solving the problem, just putting a band-aid on the problem.

If I may suggest, can we instead rollback the changes specific to the Codec change that occured in #33? We would lose optimization in exchange for compatibility, without introducing new changes. (Again feel like this is the cleanest). Then we can open up a new issue specifically on applying optimization/making the codec backwards compatible, and close this issue out?

[ajermaky]

Created a PR here to undo change: #117

[oxisto]

Closed by #117