jwt.Parse silently fails if alg isn't included in JWK

[anderspitman]

Describe the bug

I'm building an OIDC server with a RSA JWK key. I wasn't returning alg: "RS256" in the JSON. I traced the problem to jwx silently failing this check:

jwx/jws/key_provider.go

Line 117 in 976a058

if v := key.Algorithm(); v.String() != "" {

It returns from here without ever setting the sink:

jwx/jws/key_provider.go

Line 150 in 976a058

return nil

Note that for my server I'm generating the keys using JWX as well, and it doesn't include alg by default, which is why it was missing. Not sure if this is intended behavior. In order to fix my bug I just needed to add key.Set("alg", "RS256")

Please attach the output of go version

go version go1.20.4 linux/amd64

[lestrrat]

before I make any wild guesses, can you please provide a standalone test case that reproduces this behavior?

[anderspitman]

I might be able to, but I want to make sure I understand the intent of the keySetProvider.selectKey function first. From inspecting it, it seems like the criteria for success is that sink.Key is called. But if it isn't called, I would expect the default return to be an error, but the default return is nil. So the function can literally do nothing and then return a nil error, which is what's happening in my case. Am I understanding the intent correctly?

[lestrrat]

Not returning an error is the intended behavior.

[anderspitman]

Here is a minimal repro of my issue:

package main

import (
        "os"
        "fmt"
	"crypto/rand"
	"crypto/rsa"

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

func main() {
        raw, _ := rsa.GenerateKey(rand.Reader, 2048)
	key, _ := jwk.FromRaw(raw)

	_ = jwk.AssignKeyID(key)

	//key.Set("alg", "RS256")

        token, _ := openid.NewBuilder().
                Subject("username").
                Audience([]string{"audience"}).
                Issuer("issuer").
                Claim("nonce", "random").
                Build()

        signedToken, _ := jwt.Sign(token, jwt.WithKey(jwa.RS256, key))

        keyset := jwk.NewSet()
        keyset.AddKey(key)

	publicKeyset, _ := jwk.PublicSetOf(keyset)

        _, err := jwt.Parse([]byte(signedToken), jwt.WithKeySet(publicKeyset), jwt.WithToken(openid.New()))
        if err != nil {
                fmt.Fprintf(os.Stderr, err.Error())
                return
        }
}

The parse fails. If you uncomment key.Set("alg", "RS256"), it works. It took me an hour digging deeper into the JWX code to find the root problem, which I explained above. It would potentially save others a lot of time in the future if a more specific error was returned, which is very possible by returning an error from keySetProvider.selectKey if v.String() == ""

[lestrrat]

Ah, I see. tl;dr use jwk.WithInferAlgorithmFromKey(). More explanation follows.

---

That's documented

// By default both alg and kid fields in the JWS and the
// key must match for a key in the JWKS to be considered to be used.

Though I agree that it could have been more explicit.

---

This behavior is slightly different between using a single key and a key set for verify, because of the possible security implications. The basic premise is that this library does not allow verification to succeed unless the intended algorithm for both the JWS message and the key provided matches.

If it's a single key, the user (you) are explicit in wanting to use the key with the algorithm specified in the jws.WithKey(alg, key) option. However, if it's a key set, then you are relying on the validity of the key set, guessing which of the key are the intended target -- and usually the key set is something that is provided by a third party, from out of your control, and thus the default behavior is NOT to implicitly trust the data, including the knowing exactly what algorithm the key should be used for.

To alleviate this problem, however, is the jwk.WithInferAlgorithmFromKey() option, which will list a possible list of algorithms that a key type (kty) can be used for, and looks for a match. This is an explicit option and not a done automatically, because we want the users to know that they are doing it.

[github-actions]

This issue is stale because it has been open 14 days with no activity. Remove stale label or comment or this will be closed in 7 days.

[github-actions]

This issue was closed because it has been stalled for 7 days with no activity. This does not mean your issue is rejected, but rather it is done to hide it from the view of the maintains for the time being. Feel free to reopen if you have new comments