Supported Signing Algorithms: Add Support to the Provider struct for parsing the supported signing algorithms

[yanniszark]

Hi all!
We have been using the go-oidc library to build an oidc-authservice for the Kubeflow Istio Gateway and a user stumbled on an unsupported signing algorithm error while using a different from RS256 signing algorithm. (kubeflow/kubeflow#4550).

The code we use for getting a verifier is:

verifier := s.provider.Verifier(&oidc.Config{ClientID: s.oauth2Config.ClientID})

I would expect that the provider would save the supported signing algorithms for idtoken and userinfo in its internal struct. Those are give in the standard fields id_token_signing_alg_values_supported and userinfo_signing_alg_values_supported.

I think this is something we would want to support, otherwise I have to manually scrape the well-known endpoint for that info.
If you're ok with it, I can put the cycles to prepare a PR for this.

/cc @ericchiang

[ericchiang]

I'd be okay with a PR for verifiers to automatically support the algorithms from discovery.

But HS512 is a symmetric signing algorithm, which this pack age doesn't support. In these cases you need to provide symmetric verification keys through a different channel anyway since they can't be a part of discovery. I believe it's actually the client secret that does the signing.

https://tools.ietf.org/html/rfc7518#section-3.1
https://openid.net/specs/openid-connect-core-1_0.html#IDTokenValidation

[yanniszark]

@ericchiang thanks a lot for explaining.
You are right, HMAC requires the Client and Provider to agree on a secret.

But HS512 is a symmetric signing algorithm, which this pack age doesn't support.

Is this a limitation of this package or of the underlying jose library?
Do you plan (or would you accept PRs) that add support for this kind of algorithms?
If not, do you have a recommendation on how to verify this type of algorithm?
It would also be great if there was a list of supported algorithms, it isn't obvious to me from the code.
Edit: I saw the jose.go file, I guess that's the list I'm looking for?

[ericchiang]

Is this a limitation of this package or of the underlying jose library?

This is an intentional limitation of this package. Mixing symmetric and asymmetric algorithms is a recipe for API misuse :)

https://auth0.com/blog/critical-vulnerabilities-in-json-web-token-libraries/#RSA-or-HMAC-

Do you plan (or would you accept PRs) that add support for this kind of algorithms?

If we did add this, I'd want to be very careful about the API to make it clear that this is a different trust model. Spit-balling, this should probably be a different Verifier entirely. As mentioned in kubeflow/kubeflow#4550 (comment), if the provider supports RS256 or some other asymmetric algorithm, it's much safer to stick to that instead of assuming the client_secret stays secret.

If not, do you have a recommendation on how to verify this type of algorithm?

You'd have to roll your own verifier. This package isn't meant to be used with HS256.

It would also be great if there was a list of supported algorithms, it isn't obvious to me from the code.

I'll send a PR to automatically support algorithms advertised by the provider and add an example of doing something custom there.

[ericchiang]

Actually, looking at our code I think you can provide a KeySet to support symmetric algorithms... That's terrifying. Please give me a sec to think if that's actually want our clients to be able to do that before using that.

[yanniszark]

@ericchiang thanks a lot for taking the time to write such an informative answer, I really appreciate it and I think I understand much better what the problem is 😄

This is an intentional limitation of this package. Mixing symmetric and asymmetric algorithms is a recipe for API misuse :)

Very interesting. So the library needs to make sure the correct key is used for each algorithm.

I'll send a PR to automatically support algorithms advertised by the provider and add an example of doing something custom there.

That would be awesome!

Actually, looking at our code I think you can provide a KeySet to support symmetric algorithms...

You mean by making your own KeySet interface implementation?
If so, then yes I think that you can.
Although, if a user reaches the point where they do that, I would expect that they are advanced enough to know the implications.

[yanniszark]

@ericchiang kind ping on this in case you have any updates on:

• Supporting more asymmetrical algorithms with the current interface.
• How to introduce a symmetric algorithm interface.

Again, I am very open to getting my hands dirty and submitting for PRs for both of these, but I'd like to get your advice first.

[ericchiang]

@yanniszark can you take a look at #227? That should ensure that if a provider advertises support for more algorithms, the verifier will automatically pick up those values.

Given that in kubeflow/kubeflow#4550 the user just switched to an asymmetric signing algorithm, I don't think it's worth supporting symmetric algorithms.

[yanniszark]

@ericchiang thanks! I'll take a look asap! :)

Given that in kubeflow/kubeflow#4550 the user just switched to an asymmetric signing algorithm, I don't think it's worth supporting symmetric algorithms.

It's true that the original use-case seems to be resolved.
I'm ok with leaving symmetric algorithm support out of scope for this issue.