Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JWK Set jwt.Verifier implementation #128

Closed
MicahParks opened this issue Apr 20, 2022 · 7 comments
Closed

JWK Set jwt.Verifier implementation #128

MicahParks opened this issue Apr 20, 2022 · 7 comments
Labels
feature New feature or request

Comments

@MicahParks
Copy link

I'm looking to create a separate helper Go package for this project. This helper Go package will verify JWTs are signed with a key in a JWK Set (JWKS). It helps users of this package by turning a JWKS into a jwt.Verifier to more conveniently verify keys.

This would be similar to how github.com/MicahParks/keyfunc is a helper Go package for github.com/golang-jwt/jwt/v4.

However, the jwt.Verifier interface has a method called Algorithm that must be implemented. From the Go docs, I can't infer how to implement this method, but looking at the source code, it seems to me that the jwt.Algorithm return parameter's value is the set of values I often see as the "alg" in JWT headers. Also glancing at the source code, it seems to me that the only use of the Algorithm method is in a _test.go file, so I'm not sure if an improper implementation of this method would affect anything.

Before I continue developing this package, I wanted to confirm how to implement this Algorithm method, due to the fact that it's a part of the public facing API. Since a JWKS can contain multiple key types, it's not appropriate to return a single "alg" value. Perhaps one of the two options may be ideal?

  1. Add a new jwt.Algorithm value that indicates any algorithm can be verified.
  2. Remove the Algorithm method from the jwt.Verifier interface (would be a breaking change and require a major semver bump).
@cristaloleg
Copy link
Member

Hi @MicahParks

Yes, Algorithm() Algorithm is just a string that represents short (RFC-compliant? I don't remember) name of the algorithm.

  1. I had algo none in the beginning but it's a horrible idea to have something like that :) even in RFC
  2. This cannot be dropped 'cause during alg token verification we also need to verify that token alg and verifier represents same algorithm. That's pretty critical and I think Auth0 had security breach due to this mistake.

Well, I don't have right answer now, one of the thing can be make a Verifier that wraps other verifiers and during verification it just pick ups the same as token alg.

func (mf *MultiVerifier) Verify(token *Token) error {
	switch token.Alg() {
	case "abc":
		// do this
	case "xyz":
		// do that
	default:
		return errors.New("unsupported alg")
	}
}

sure this can be a map and not a switch + token.Alg() can be splitted by , or whatever is presented in a token alg field.

One more clarification: are you planning to make repo on your own or you're proposing to make it under @cristalhq org?

@MicahParks
Copy link
Author

I'd like to keep it under my own GitHub account, at least until it's ready for v1.0.0. With keyfunc I tried to move it into the parent repository in this PR: golang-jwt/jwt#105, but that went stale, so I closed it. There has been talk about moving keyfunc into a separate project under the organization. I'm open to it, but haven't actively pushed for that. A big reason I think it made sense to be in the same repository as the parent project is because it'd be versioned the same way. So if it moves to /v5, I don't have to update keyfunc. However, I understand there are quite a few downsides to moving the code into the parent project, so I see why some would prefer a separate project. For now, I think the reference in the parent project's README.md helps guide users to keyfunc if their use cases could benefit from it.

Currently for keyfunc, the cryptographic key selected is only done by matching thekid in the JWT's header to one in the JWKS. This is because multiple keys using the same algorithm are possible within a JWKS. So yes, the underlying data structure I've chosen for keyfunc is a map[string]interface{} (key IDs to cryptographic keys).

That's just for key selection, after the key is selected, the key needs to be used to verify the JWT, I'd probably use a switch statement like the one suggested in the comment above, to select the Algorithm to pass to a function such as jwt.NewVerifierHS.

during alg token verification we also need to verify that token alg and verifier represents same algorithm

Hmmm, maybe the jwt.Verifier interface could be changed from this:

type Verifier interface {
	Algorithm() Algorithm
	Verify(token *Token) error
}

to this?

type Verifier interface {
	Verify(token *Token) (Algorithm, error)
}

The reason for this is because for a JWKS jwt.Verifier implementation, the Algorithm could vary depending on the input. But, I hate to suggest breaking changes, especially when I don't own the repository. And I would encourage no change quite yet.

I'd like to dig up the mentioned article about the Auth0 vulnerability, just to make sure I don't suggest anything that would repeat that mistake. I'd also like to confirm that keyfunc doesn't have a similar problem.

What do you think of the mentioned breaking change and Algorithm selection idea?

@MicahParks
Copy link
Author

MicahParks commented Apr 20, 2022

As a note, I think I found the initial set of "alg" in this RFC: https://datatracker.ietf.org/doc/html/rfc7518#section-3.1, which is where I think the values for jwt.Algorithm come from.

This is the "initial set" because some follow-up RFCs have added new ones since then.

@cristaloleg
Copy link
Member

Yep, "alg" is from RFC as I mentioned before.

This seems obfuscated just to solve external case, looks like this doesn't align well with cristalhq/jwt library goal 😢

type Verifier interface {
	Verify(token *Token) (Algorithm, error)
}

I assume you came from this thread https://www.reddit.com/r/golang/comments/u68a7e/fast_simple_jwt_for_go_v400_released/ and I'm planning to touch JWS/JWK this weekend so probably this case will be solved in a separate library (probably based on current or even as a dependency, not sure yet).

Regarding Auth0, looks like this is the article https://auth0.com/blog/critical-vulnerabilities-in-json-web-token-libraries/

@MicahParks
Copy link
Author

No, I don't really use Reddit. I saw this repo on GitHub Explore and the post on Gopher's Slack. I made a Google Keep note to look into it later. https://gophers.slack.com/archives/C02A3DRK6/p1650285292828819

Best of luck implementing JWS/JWK this weekend! Since you're already on the case, I'll pause the project I originally mentioned, wouldn't want to duplicate any work. Thanks for the link to the Auth0 article.

@cristaloleg
Copy link
Member

Ah, so Gopher Slack is alive, nice to hear) Well, I'm not giving 100% guarantees that I will finish it but at least I'm eager to try and see what we have in RFCs.

@cristaloleg cristaloleg added the feature New feature or request label Nov 8, 2023
@MicahParks
Copy link
Author

I updated github.com/MicahParks/jwkset to be a more complete implementation of a JWK Set.

I've made a small wrapper around the project to allow users to parse JWTs signed from keys residing in JWK Sets when using go get github.com/cristalhq/jwt/v5. This new project can be found at github.com/MicahParks/verifier. It works by parsing a raw token for it's key ID in the header, finding the matching JWK, then using that key to determine which built-in jwt.Verifier to select. It also provides a .Parse() method. There are more advanced features, like X.509 support, but that requires accessing the JWK Set storage directly.

Here is some basic usage.

import "github.com/MicahParks/verifier"

Step 1: Create the verifier.JWKSetVerifier

// Create the verifier.JWKSetVerifier.
jwks, err := verifier.NewDefault([]string{server.URL})
if err != nil {
	log.Fatalf("Failed to create a verifier.JWKSetVerifier from the server's URL.\nError: %s", err)
}

When using the verifier.NewDefault function, the JWK Set will be automatically refreshed using
jwkset.NewDefaultHTTPClient.

Step 2: Use the verifier.JWKSetVerifier to parse and verify JWTs

// Parse the JWT.
token, err = jwks.Parse(signed)
if err != nil {
	log.Fatalf("Failed to parse the JWT.\nError: %s", err)
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants