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

Support for looking up public keys via OIDC discovery #53

Open
raymcdermott opened this issue Aug 17, 2017 · 19 comments
Open

Support for looking up public keys via OIDC discovery #53

raymcdermott opened this issue Aug 17, 2017 · 19 comments

Comments

@raymcdermott
Copy link

When using auth0 / google / etc... you can obtain public keys to assist with verifying JWTs via .well-known endpoints.

The domain for the endpoints can be deduced from the JWT :iss property and a map of the .well-known paths for commonly used authentication providers (or provided by the client).

Are you considering support for this?

I have a rudimentary implementation so can create a PR if there is interest.

@niwinz
Copy link
Member

niwinz commented Aug 29, 2017

Hmm, I consider adding this kind of feature if this does not make anything automatic. I mean this can be an additional ns with specific functions for public key lookups but nothing automatic on unsign or decrypt functions. Can you post some example code on how you proposed api will be used?

@raymcdermott
Copy link
Author

Here is a gist of the way I have it working now

https://gist.github.com/raymcdermott/1f38ec455df433b96da789a70a4dd346

@raymcdermott
Copy link
Author

The obvious downside is that it calls out to external services.

Maybe the httpclient function and discovery endpoints could be passed in as parameters to jwt-discovery-info

@niwinz
Copy link
Member

niwinz commented Aug 31, 2017

Nice example, thanks! One question, verify-jwt performs double decoding of the token? I mean, I don't see the decoded-jwt impl but i supporse that it just decodes the token without verification, i'm wrong?

@raymcdermott
Copy link
Author

Correct. I need to check again now that you have 2.2.0 released but this is what I did to get it working before that ...

https://gist.github.com/raymcdermott/bad0a3c8cc683db5ddcaafcb7cfb91f2

@niwinz
Copy link
Member

niwinz commented Aug 31, 2017

Hmm, I think the first step would be provide a better approach for decode the token un payload and header in one step (just avoido the additional decode overhead).

On the other hand, I'm not understand why the iss claim comes in the payload. It will be make more sense if it comes in header... I'm saying that because you are accessing the iss claim before verifying the token, and I'm not very convinced on the security of that. What do you think? You have the control of tokens generation? It make sense send the iss claim in the header instead of payload?

@raymcdermott
Copy link
Author

Perhaps I misunderstand. The JWT is structured such that the iss property is in the payload not the header.

I take your point about not trusting it until it's verified although this is the means that the verification doc is discovered ;-)

Another option might be to pass in the issuer that you are expecting to verify against rather than looking it up in the JWT.

@raymcdermott
Copy link
Author

+1 on the one step decode

@niwinz
Copy link
Member

niwinz commented Aug 31, 2017

The JWT RFC specifies the iss as informative and optional header about the issuer, but it does not specifies that it should be used to use this kind of verifications.

On the other hand JWT spec does not disallow put any kind of additional headers, so the iss can be on the header also. That will facilitate a lot.

And on the other hand (:P) JWS/JWT maybe has specific headers for this purpose: x5u (https://tools.ietf.org/html/rfc7515#page-11). Maybe I'm wrong and this referes to something different, but it seems that it coveres just the use case, no?

@niwinz
Copy link
Member

niwinz commented Aug 31, 2017

With that I'm trying to say is that the functionality is very nice, but using payload claims for verify the payload I think it does not make sense and we need to think on how this should be aproached without decoding payload without verifying it.

@raymcdermott
Copy link
Author

Sadly x5u is optional and not provided by auth0

@raymcdermott
Copy link
Author

The flow I use supports verifying signed JWTs from known providers (via their TLDs / .well-known endpoints) over TLS.

The variation that is supported for auth0 and other such multi-tenant providers is for xyz.auth0.com endpoints. But xyz certs must be accessed using TLS provided by auth0 (or other provider).

In other words that sub-domain cannot re-direct to another provider and still be validated / verified.

Now that I have had further time to reflect on it, I'm not convinced that it isn't secure. But open to further understanding / persuasion.

@niwinz
Copy link
Member

niwinz commented Aug 31, 2017

Having the iss on header has the same security as having it on payload, you will use iss claim content before verifying it in both cases...

Is not about security, is about the fact of need to decode payload before verifying it. We need to have all the data for verify the signature without decoding the payload.

The header is the correct location for that kind of stuff. Let see on JWS RFC defined headers https://tools.ietf.org/html/rfc7515#page-11), they are much related to the same thing: provide a way to retrieve the pubkey for the token...; this is why I think that the header is the correct location for the iss claim.

That providers can't provide the issuer related information on the header of the token?

@raymcdermott
Copy link
Author

In the case of auth0 I can't see a way to add that to the header.

All of their libs (I have looked at node, java and ruby) work on having the JWT issuer endpoint as an ENV variable and they work everything out from that.

This means that buddy would have to trust the caller to provide the JKS endpoint for the JWT being verified. Not as good as having all the data to hand from the JWT but also seems OK. What do you think?

@niwinz
Copy link
Member

niwinz commented Aug 31, 2017

I think that is much more approachable, I can imagine an api that workis in this manner.

Something like that maybe:

(def key (jwt/oidc-store "https://auth0.com/some/path/.well-known/jwks.json"))
(jwt/unsign token key)

Don't care on the names I have used for the example, they are just temporal names for represent the idea. With an api similar to that, with explicit issuer specified by the user of the library, we can provide automatic fetching the keys and validation of the tokens.

@raymcdermott
Copy link
Author

excellent... I propose that I make a PR based on 2.2.0

@raymcdermott
Copy link
Author

quick question ... I use the aleph HTTP client. We would need to add that as a dependency.

How do you feel about that?

@niwinz
Copy link
Member

niwinz commented Sep 1, 2017

It is ok for use aleph, is a good library. But we'll need to think about how to make this dependency optional. But don't worry about that right now, we can do it later :D

@raymcdermott raymcdermott mentioned this issue Sep 4, 2017
@orb
Copy link

orb commented Apr 4, 2019

I'd point out that jwks.json supports multiple keys, and though it's not as common a practice as it should be, a sight should advertise multiple keys when they are rotating keys at the very least.

Maybe there needs to be an unsign helper that takes a collection of keys for validation purposes?

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

No branches or pull requests

3 participants