From ed6b084875e1bcdcd00085a09e993657d8d7af2a Mon Sep 17 00:00:00 2001 From: Daisuke Maki Date: Fri, 27 Oct 2023 20:37:14 +0900 Subject: [PATCH 1/2] Fix ParseInsecure to parse the token even when a key is given --- Changes | 4 ++++ jwt/jwt.go | 4 ++++ jwt/jwt_test.go | 29 +++++++++++++++++++++++++++++ 3 files changed, 37 insertions(+) diff --git a/Changes b/Changes index bbd22e973..31428de0b 100644 --- a/Changes +++ b/Changes @@ -42,6 +42,10 @@ v2.0.16 UNRELEASED * [jwk] Added `jwk.IsKeyValidationError` that checks if an error is an error from `key.Validate()`. +[Bug Fixes] + * [jwt] `jwt.ParseInsecure()` was running verification if you provided a key + via `jwt.WithKey()` or `jwt.WithKeySet()` (#1007) + v2.0.15 19 20 Oct 2023 [Bug fixes] * [jws] jws.Sign() now properly check for valid algorithm / key type pair when diff --git a/jwt/jwt.go b/jwt/jwt.go index cd059a263..fa0c20214 100644 --- a/jwt/jwt.go +++ b/jwt/jwt.go @@ -209,6 +209,10 @@ func parseBytes(data []byte, options ...ParseOption) (Token, error) { } } + if !verification { + ctx.skipVerification = true + } + lvo := len(verifyOpts) if lvo == 0 && verification { return nil, fmt.Errorf(`jwt.Parse: no keys for verification are provided (use jwt.WithVerify(false) to explicitly skip)`) diff --git a/jwt/jwt_test.go b/jwt/jwt_test.go index 9b8554e2a..c4464bfa1 100644 --- a/jwt/jwt_test.go +++ b/jwt/jwt_test.go @@ -1743,3 +1743,32 @@ func TestGH951(t *testing.T) { require.True(t, jwt.Equal(verified, token), `tokens should be equal`) } + +func TestGH1007(t *testing.T) { + key, err := jwxtest.GenerateRsaJwk() + require.NoError(t, err, `jwxtest.GenerateRsaJwk should succeed`) + + tok, err := jwt.NewBuilder(). + Claim(`claim1`, `value1`). + Claim(`claim2`, `value2`). + Issuer(`github.com/lestrrat-go/jwx`). + Audience([]string{`users`}). + Build() + require.NoError(t, err, `jwt.NewBuilder should succeed`) + + signed, err := jwt.Sign(tok, jwt.WithKey(jwa.RS256, key)) + + // This was the intended usage (no WithKey). This worked from the beginning + _, err = jwt.ParseInsecure(signed) + require.NoError(t, err, `jwt.ParseInsecure should succeed`) + + // This is the problematic behavior reported in #1007. + // The fact that we're specifying a wrong key caused Parse() to check for + // verification and yet fail :/ + wrongPubKey, err := jwxtest.GenerateRsaPublicJwk() + require.NoError(t, err, `jwxtest.GenerateRsaPublicJwk should succeed`) + require.NoError(t, err, `jwk.PublicKeyOf should succeed`) + + _, err = jwt.ParseInsecure(signed, jwt.WithKey(jwa.RS256, wrongPubKey)) + require.NoError(t, err, `jwt.ParseInsecure with jwt.WithKey() should succeed`) +} From cf0769dbb064371d27d91326ed6db56c19e7ea01 Mon Sep 17 00:00:00 2001 From: Daisuke Maki Date: Fri, 27 Oct 2023 20:43:14 +0900 Subject: [PATCH 2/2] appease linter --- jwt/jwt_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/jwt/jwt_test.go b/jwt/jwt_test.go index c4464bfa1..8e94faa34 100644 --- a/jwt/jwt_test.go +++ b/jwt/jwt_test.go @@ -1757,12 +1757,13 @@ func TestGH1007(t *testing.T) { require.NoError(t, err, `jwt.NewBuilder should succeed`) signed, err := jwt.Sign(tok, jwt.WithKey(jwa.RS256, key)) + require.NoError(t, err, `jwt.Sign should succeed`) // This was the intended usage (no WithKey). This worked from the beginning _, err = jwt.ParseInsecure(signed) require.NoError(t, err, `jwt.ParseInsecure should succeed`) - // This is the problematic behavior reported in #1007. + // This is the problematic behavior reporded in #1007. // The fact that we're specifying a wrong key caused Parse() to check for // verification and yet fail :/ wrongPubKey, err := jwxtest.GenerateRsaPublicJwk()