Skip to content
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

[BUG]Error parsing body of Firebase JWT token #4

Closed
xy4455 opened this issue May 27, 2020 · 6 comments
Closed

[BUG]Error parsing body of Firebase JWT token #4

xy4455 opened this issue May 27, 2020 · 6 comments
Assignees
Labels
bug Something isn't working

Comments

@xy4455
Copy link

xy4455 commented May 27, 2020

Describe the bug

When parsing a token generated by Firebase auth (some reference) l8w8jwt seems to be having some trouble parsing the body json.

I'll send the real token as a PM.

Parsing fails (or more accurately the failure is detected) on line 122 of decode.c:

if (i >= r || key.type != JSMN_STRING)
        {
            r = L8W8JWT_DECODE_FAILED_INVALID_TOKEN_FORMAT;
            goto exit;
        }

The token (stripped from sensitive data) is below:

{
  "iss": "somestirng",
  "aud": "proj",
  "auth_time": 1590563505,
  "user_id": "SomeId",
  "sub": "SomeId",
  "iat": 1590567577,
  "exp": 1590571177,
  "email": "[email protected]",
  "email_verified": false,
  "firebase": {
    "identities": {
      "email": [
        "[email protected]"
      ]
    },
    "sign_in_provider": "password"
  }
}

To Reproduce
Steps to reproduce the behavior:

Load firebase auth token using l8w8jwt

Expected behavior
A clear and concise description of what you expected to happen.

@xy4455 xy4455 added the bug Something isn't working label May 27, 2020
@GlitchedPolygons
Copy link
Owner

Thanks for submitting the issue. I replied to your PM :)
I'll paste part of my email here for everyone to see:

[...] from what I can already see from the decoded JSON: I'm afraid it has to do with the nested objects. I didn't explicitly implement support for nested JSON tokens as claims, as it's kind of controversial and I didn't need it personally for my own project where I originally planned to use l8w8jwt. I try to keep those hierarchies as flat as possible.

auth0/java-jwt#247
auth0/java-jwt#163

Nothing's unfixable though, but I'd have to do more research.
The thing I'm most scared of is the added complexity when mapping nested JSON objects in the claims to the l8w8jwt output claims C struct, which has no support for anything that isn't a flat key:value payload. Could be not so easy as the signature verification issue yesterday..... Any help is deeply appreciated obviously :)

@xy4455
Copy link
Author

xy4455 commented May 27, 2020

Thanks for the reply, and having flat hierarchies makes sense.

I don't know if it makes the fix easier, but for my use case it would be totally acceptable if the nested objects are ignored.

Without too much knowledge on JWT, I'd still guess that most claims in nested objects are for really particular use cases. So for me, not mapping them to a C-struct makes sense.

But for me a library that would to the following would be super useful:

  • Verify signature
  • Check normal / basic claims
  • Don't check nested objects
  • [Optionally] Have a string field in the claims return object that contains the body as a base64 decoded string.

But even if you don't return that string, with just ignoring the nested objects it would already be very useful. At least I know that whatever is claimed is signed. And I don't have to worry about the basic checks. Decoding and parsing the remaining json should be easy enough for most users who need advanced checking beyond that.

@xy4455
Copy link
Author

xy4455 commented May 27, 2020

After typing the last comment I went ahead and checked decode.c

I turns out that if I ignore this particular claim the verification passes successfully. Below is how I hacked to code to do that.

Anyway, I'm not sure how this would be useful elsewhere, but for me this means I can now finally verify firebase tokens :-)

        if (i >= r || key.type != JSMN_STRING)
        {
            continue;
//            r = L8W8JWT_DECODE_FAILED_INVALID_TOKEN_FORMAT;
//            goto exit;
        }

@GlitchedPolygons
Copy link
Owner

After typing the last comment I went ahead and checked decode.c

I turns out that if I ignore this particular claim the verification passes successfully. Below is how I hacked to code to do that.

Anyway, I'm not sure how this would be useful elsewhere, but for me this means I can now finally verify firebase tokens :-)

        if (i >= r || key.type != JSMN_STRING)
        {
            continue;
//            r = L8W8JWT_DECODE_FAILED_INVALID_TOKEN_FORMAT;
//            goto exit;
        }

Cool :) If you're happy I'm happy.

Yeah, if it works like that it means that it's definitively because of the nested claim objects (the check key.type != JSMN_STRING returns true because the json object claim is then a JSMN_OBJECT for jsmn.)

I still want to look into this as soon as I have the time, and see if there's something minimal I can do, but until then I think I'll close this issue and add it to my own personal to do list

@xy4455
Copy link
Author

xy4455 commented May 27, 2020

Cool, I've forked your repo and made those changes to my copy. Now I can finally validate my tokens. Thanks for your help. I've learned a lot about jwt :-D

@GlitchedPolygons
Copy link
Owner

GlitchedPolygons commented Jan 31, 2024

@xy4455 Hi there, hope you had a lovely 4 years ;D

Sorry for the insane delay.

Maybe you don't even remember this, but I never forgot about it. It was always kind of in the back of my head as an unclosed to-do item. I finally found some time to give due love to my FOSS libs on GH.

I have updated l8w8jwt's dependencies and addressed some of the issues/feature requests that users have reported. While it still is not recommended to have non-flat claim values, there is now an "official" workaround that at least allows users to decode and validate JWTs while receiving the decoded payload JSON string out from the l8w8jwt_decode_raw function, so that the critical claims can be parsed externally. Check out #15 for more details on how this was suggested.

Thanks again for the issue submission.

Check out release 2.3.1 here:

https://github.com/GlitchedPolygons/l8w8jwt/releases/tag/2.3.1

:)

Cheers, and have a wonderful 2024!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants