realtime icon indicating copy to clipboard operation
realtime copied to clipboard

feat: support rs256 signing

Open ecchochan opened this issue 2 years ago • 7 comments

What kind of change does this PR introduce?

  • ~Added API_JWT_SIGNING_METHOD config (api_jwt_signing_method)~
  • ~Added METRICS_JWT_SIGNING_METHOD config (metrics_jwt_signing_method)~
  • ~Added jwt_signing_method field in tenant~
  • Increase jwt_secret field size in tenant (so that it can store long private long)
  • Support RS sign algorithms in lib/realtime_web/channels/auth/jwt_verification.ex
  • ~Added jwt_verification_test_rs.exs similar to jwt_verification_test.exs but for RS~

What is the current behavior?

No Support of RS256

What is the new behavior?

Support RS256

Additional context

  • https://github.com/supabase/supabase/discussions/12759

ecchochan avatar Mar 02 '23 16:03 ecchochan

@ecchochan is attempting to deploy a commit to the Supabase Team on Vercel.

A member of the Team first needs to authorize it.

vercel[bot] avatar Mar 02 '23 16:03 vercel[bot]

@abc3 this looks pretty complete. Let's review tomorrow.

chasers avatar Jun 06 '23 21:06 chasers

@ecchochan thanks for your contribution! this looks great.

~can we keep it simpler for the initial version and just check for the alg from the JWT header?~ never mind, see: https://github.com/ticarpi/jwt_tool/wiki/Known-Exploits-and-Attacks#cve-2016-5431---key-confusion-attack

w3b6x9 avatar Jun 07 '23 04:06 w3b6x9

@w3b6x9 Thanks for reviewing! Did a cleanup and you can have a look!

Edit: however this comment has to be addressed before merging

ecchochan avatar Jul 16 '23 12:07 ecchochan

@Lan-Hekary Yes, the current implementation in this PR is respecting the alg in the header, which according to your comment is a security risk!

This PR has to change to address the issue, thank you for raising that concern!

ecchochan avatar Jul 16 '23 13:07 ecchochan

@ecchochan, First of all, Thank you for your effort in this PR. I did some digging in the matter of Key Confusion attacks. The problem rises from passing the key to the verification function along with the alg header without checking which key should be used.

I am not very good with Elixir Programming Language, but from what I see in the commit you pass the same jwt_secret as PEM to create a signer. I am afraid this is risky, because if an attacker passed the same jwt_secret for RS256 which is the public key and modify the alg to HS256 he can sign his own JWT with your public key and the validation will be ok. The Correct approach is to store multiple keys in a Key set, The standard is JWK

In PostgREST for example you set the keys in a JWKs object and the library choose the correct key based on the alg header. and the JWKs can have multiple keys, symmetric ones for HS signing and asymmetric ones for RSA, EC, and Even EdDSA. I tested this concept with Supabase and I can generate keys of various types and their respective JWTs and validate them successfully, but I can't confuse the "jose" library in PostgREST because they separate the keys into groups for every type.

we need a way to store the JWKs for realtime as well. and choose which keys to use based on alg header

This also got me thinking that in PostgREST it's very easy to modify the JWKs by: alter role authenticator set pgrst.jwt_secret = {jwks_json_object} and then issue and notification to pgrst channel.

how can we do that same in Realtime ? Also we should be able to change it without restarting the project or the container (Can't set it via env variable) and preferably there should be one configuration the reflect in all the components ( PostgREST, Realtime, GoTrue, etc)

Lan-Hekary avatar Jul 16 '23 13:07 Lan-Hekary

@ecchochan, can you check https://hexdocs.pm/joken/2.6.0/Joken.Signer.html#parse_config/1 I think it takes care of all the key separation, and it even supports multiple key representations. All you have to do is pass it the keys and the algorithms, which is by defentions is what a JWKs (JSON Web Key Set) is. take a look at the 'kty' key in JWK and JWS RFC.

All we need now is a way to configure a JWKs in Supabase globally and we can use the same JWKs across Realtime and PostgREST without any hassle.

I feel optimistic that this is an easy implantation that will not break exciting behavior nor be a security risk.

Lan-Hekary avatar Jul 16 '23 14:07 Lan-Hekary