-
Notifications
You must be signed in to change notification settings - Fork 760
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
Overly strict JWT decoding? #3508
Comments
can you give an example jwt simple token for us to debug,fix/test against? |
Sure,
you can see Payload and Header in debugger: https://jwt.io/#debugger-io?token=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpYXQiOjE3MjExNDQzMjksImlkIjoiaGVsbG8ifQ.ShrhYX0U8FUC3KtuctIrYpibgAT7Grzm0Ni1QqnjReo |
I am assuming you are using the JWT secret in the client by using the You are trying to use this JWT secret for both our client, and a CL - right? |
Are you using a "simple" 32-byte hex encoded JWT secret? What CL are you testing against? And how did you stumble upon this problem? 😄 So many questions 😅 |
So but just to drop here: we "internalized" (in the sense of: take the code over in the code base with a license on top to avoid drawing in the dependencies) the JWT library we are using recently and there was some discussion around padding taking place along: Do not have the full overview about the details but it might very well be that there changed something along (padding now applied while it was not before or so). |
I'm testing interoperability between EthereumJS and Grandine CL client on Kurtosis. Though this issue can be observed with other CL clients as well if specific CLI params are given. JWT secret is managed by Kurtosis and yes, it is simple 32-byte hex encoded secret. The same secret is passed to both EL & CL. Nevermind the secret though. JWT token is split into 3 parts separated by '.' character and first two parts do not depend on the secret. They are just base64url encoded JSON (with or without padding). EthereumJS raises error when trying to decode second part of the JWT token due to the lack of padding (if some extra optional claims are present) and request fails with authentication error. |
Hi there, |
Sure. Here it is. It only additionally contains an optional "id" claim set to 'hello'. "id" and "clv" claims are permitted by the spec: |
Hey there, if I setup a Grandine <-> EthereumJS connection I get 401 errors. How do I make Grandine spit out a more verbose log of that 401 message? I am trying to see anything in our verbose logs but I cannot yet find any errors there 🤔 I am assuming this 401 error on communication is related to this issue? You can also join our discord, if you want 😄 https://discord.gg/TNwARpR |
Thanks for the fix & quick reaction! 👏 🔥 🔥 |
Hi,
I've noticed that ethereumjs expects base64url encoded jwt tokens with padding and it fails if padding is not present.
ethereumjs-monorepo/packages/client/src/ext/jwt-simple.ts
Line 124 in 80fcde6
https://github.com/paulmillr/scure-base/blob/a131a619960dd000ad7d4f24a5a64d02ecf655d9/index.ts#L367
https://github.com/paulmillr/scure-base/blob/a131a619960dd000ad7d4f24a5a64d02ecf655d9/index.ts#L132
Surprisingly, creating JWT payload with the only required claims field ( 'iat' + timestamp ) fits the padding scheme, so no padding is required and it works. However, the issue arises when more optional claim fields are added (exp, nbf, id, clv).
To complicate things even more, when considering interop with consensus layer clients, most Rust libraries used by consensus clients use base64url encoding without padding:
https://docs.rs/jsonwebtoken/9.3.0/src/jsonwebtoken/serialization.rs.html#7 used by Lighthouse
https://github.com/jedisct1/rust-jwt-simple/blob/2f951bb7a8623e374b17ea76f855986c283e1459/src/token.rs#L111 used by Grandine
As far as I understand, it is because padding is not mandatory.
Is there a particular reason why padding is strictly required on ethereumjs side?
The text was updated successfully, but these errors were encountered: