ruby-jwt icon indicating copy to clipboard operation
ruby-jwt copied to clipboard

Wrong `alg` used in header for Ed25519 keys

Open zblach opened this issue 6 years ago • 14 comments

The example tokens in this library use "alg": "ED25519", but the related spec seems to suggest that it be "EdDSA" instead.

https://tools.ietf.org/html/rfc8037#appendix-A.4

zblach avatar Sep 20 '19 16:09 zblach

I think you are correct. Now I wonder how we could make this right. The feature has been out in the wild for a pretty long time already and encoded tokens get this incorrect alg header.

I would suggest:

  • Not to change the generated alg header (The decoding should not care about the given header anyway)
  • Start supporting the correct algorithm (EdDSA) in the decode method

Maybe start printing some warning of the usage of the algorithm ED25519 so we could remove that at some point...

anakinj avatar Dec 03 '20 20:12 anakinj

Not to change the generated alg header (The decoding should not care about the given header anyway)

I just ran into this when trying to use a Ruby-generated token in an Elixir app. The Elixir library is using the alg header to choose the decryption algorithm and is case-sensitive, so ED25519 causes a signature error as it can't work out how to handle it, but Ed25519 works.

mattgibson avatar Feb 26 '21 12:02 mattgibson

Just out of curiosity, what Elixir library is this?

My biggest concern is that if it is just changed to something else old ruby-jwt<-> new ruby-jwt compatibility will at least break.

anakinj avatar Feb 26 '21 12:02 anakinj

If you are in control of the token generation you could change the constant controlling this locally: JWT::Algos::Eddsa::SUPPORTED = ["Ed25519"]

anakinj avatar Feb 26 '21 12:02 anakinj

I was using Joken, but the underlying low level library it uses is JOSE

mattgibson avatar Feb 26 '21 12:02 mattgibson

If you are in control of the token generation you could change the constant controlling this locally: JWT::Algos::Eddsa::SUPPORTED = ["Ed25519"]

Yes, I found that and it worked when generating the tokens. I think for now I'm going to fork the gem and ensure it will decrypt both Ed25519 and ED25519 tokens. Thanks for the pointer!

mattgibson avatar Feb 26 '21 12:02 mattgibson

But apparently this Joken component is expecting the EdDSA algorithms to have the exact curve used in the alg field. So changing the alg to EdDSA will not make this gem compatible either.

anakinj avatar Feb 26 '21 12:02 anakinj

Ah, that's a good point. Possibly Joken is not implementing the RFC correctly either.

mattgibson avatar Feb 26 '21 18:02 mattgibson

Btw. I think the easiest workaround is just to monkeypatch the JWT::Algos::Eddsa::SUPPORTED constant. No need for forks.

require 'jwt'
require 'rbnacl'

JWT::Algos::Eddsa::SUPPORTED = ['Ed25519']

signing_key = ::RbNaCl::Signatures::Ed25519::SigningKey.new('a'*32)
token = JWT.encode({con: 'tent'}, signing_key.verify_key, 'Ed25519')

anakinj avatar Feb 26 '21 19:02 anakinj

I was looking into actually generating the tokens with the correct "alg" value (in my case, because I wanted them to be verifiable by ruby-jose), and had to modify a bit more than just the constant.

Specifically, the conditionals on eddsa.rb were comparing alg to the key primitive ("EdDSA" vs :ed25519) and raising IncorrectAlgorithm as a result.

I've made a quick monkeypatch on my side to override this and allow EdDSA anyway. Is this something that would be sensible to add to the project? If so, I would be happy to open a PR.

EDIT: nevermind, I was mistaken. By changing ED25519 to Ed25519, as was suggested above, ruby-jose is also able to verify the tokens

renato-zannon avatar Mar 16 '21 19:03 renato-zannon

I would suggest introducing a breaking change and change the header handling to be inline what the RFC and world is expecting for the version 3.0.0

anakinj avatar Feb 02 '23 19:02 anakinj