go-jose icon indicating copy to clipboard operation
go-jose copied to clipboard

Add "kty" to jose.JSONWebKey

Open aeneasr opened this issue 6 years ago • 11 comments
trafficstars

While fields like kid or alg are added to jose.JSONWebKey, kty is not. In some cases however (e.g. when fetching JWKs from remote) it is important to have that information available. I therefore suggest to add the kty field to jose.JSONWebKey.

aeneasr avatar May 06 '19 11:05 aeneasr

To a certain extent, you can get this information by looking at the type of the key embedded in the struct, e.g. to see if it's *rsa.PublicKey or *ecdsa.PrivateKey etc. Would that address your particular use-case?

csstaub avatar May 06 '19 17:05 csstaub

Yup, but that needs a larger type assertion with all the different Public/Private types - wouldn't it be easier to just add the kty field which is available per default?

aeneasr avatar May 07 '19 10:05 aeneasr

I'd be happy to expose kty also, just wanted to note an alternate solution you could use in the interim. I don't have a lot of time on my hands right now but if you send me a pull request I can merge it in and release.

csstaub avatar May 07 '19 17:05 csstaub

Same here, I'll try to look into it however!

aeneasr avatar May 13 '19 07:05 aeneasr

Looking into this, we set the kty from the type fo the key when marshalling into JSON. If we expose the key type directly, it would be possible to set up a JSONWebKey struct where the type of key and the string in kty conflict. What should we do then?

csstaub avatar May 28 '19 20:05 csstaub

So, i think i'm actually running into a bug because of this.

We're using the AzureAD support in go golang.org/x/oauth2, and we use jose.v2 to check signatures on our JWTs. With Azure AD, the alg field is omitted from the JWKs. The kty field is present.

According to the RFC, the alg field is actually optional, and only kty is compulsory. However alg is a compulsory header in JWTs, looking at the JWT RFC.

Instead of relying on the existence of the alg header in the JWK, could we check the JWT headers for an alg and use that in JSONWebToken.Claims()?

What's happening now is, the Azure AD JWKs omit the alg header, and the signature verification failed with

square/go-jose: error in cryptographic primitive

Or maybe i'm way off, and i've got things a bit screwed up on my end.

mogthesprog avatar Aug 09 '19 20:08 mogthesprog

Do you have an example token/code I could take a look at? alg is mandatory in a JWT, but not in a JWK. The absence of alg in the JWK should not affect how go-jose handles the token. Looking at the alg header from the JWK to decide how to verify the JWT wouldn't work.

csstaub avatar Aug 09 '19 21:08 csstaub

Sure. I've got some now revoked tokens, and the JWKs that should work but they have some some company IDs in them. Do you have an e-mail address I can send them to?

mogthesprog avatar Aug 09 '19 21:08 mogthesprog

Yeah, you can email them to css (at) css.bio. An excerpt of the code you're using to verify etc. would also help me understand what you're trying to do.

csstaub avatar Aug 09 '19 22:08 csstaub

I may be running into this. Have you been able to replicate?

cgarvis avatar Oct 08 '19 19:10 cgarvis

Could we add key_ops too?

As far as I've looked, that is also the only other missing parameter. (See the registry under the "JSON Web Key Parameters" heading)

zamicol avatar Nov 11 '19 08:11 zamicol