DolphFlynn / jwt-editor

A Burp Suite extension for creating and editing JSON Web Tokens. This tool supports signing and verification of JWS, encryption and decryption of JWE and automation of several well-known attacks against applications that consume JWT.
Apache License 2.0
12 stars 11 forks source link

[FEATURE] Decode timestamps #43

Closed exploide closed 4 months ago

exploide commented 4 months ago

What is the problem you are trying to solve?

JWTs often contain timestamps, typically iat, exp or maybe nbf. These are Unix timestamps but it would be nice if somewhere in the "JSON Web Token" tab, the values would be decoded into human-readable date/time values. Additionally, a certain highlighting could also be used to indicate when the token already expired.

How are you currently being hindered by this problem?

Currently, it is now obvious how long the token is valid or if it already expired. Manually decoding timestamps is cumbersome.

How would you like this problem to be solved?

Maybe similar to how the JWT4B extension does this. But maybe there are better ways.

DolphFlynn commented 4 months ago

Hi @exploide, thanks for the suggestion. I think it's a great idea. There's a few ways one could go about it. Some options would be:

  1. Adding a passive scan check that raises informational issues around suspicious iat, exp or nbf values. This would only work in Pro and wouldn't apply to any JWT's within WebSocket messages.
  2. The Proxy history highlighting could draw attention to issues around iat, exp or nbf values. This would work in Community and with WebSocket messages. I guess the downside would be that there's two 'levels' of highlighting here.
  3. Add the info to the editor somewhere. The problem here is where to add this as there's not much real estate left on the panel. You could alternatively have an option to automatically convert the epoch seconds to a human-readable string.

Anyway those are some initial ideas. Please let me know if any of these interest you or if you have any better ones!

exploide commented 4 months ago
  1. Not sure if this would be useful. How to decide what is suspicious. I mean an expired token is odd, but I guess this barely happens during the normal flow. It's more of an issue when you use one token in repeater for some time. That's the reason I would prefer an indication in JWT tab as in option 3. A long validity period might be an issue but I think it depends on the use case, so the pentester needs to decide anyway.
  2. Highlighting a request with an expired token might be an option, but again I don't think this is what I'm actually looking for. The reason is that I still think the issue will mostly occur when using repeater. However, during an active scan, it might also happen that the token eventually expires. Having such a highlighting in the Logger tab (not only in regular Proxy history) would help to see that something happened and the results might be crap. But this is all somewhat optional for me.
  3. This is what I would need the most. When considering to convert the timestamp within the editable editor area, I think you need to be careful to not confuse the reader as it might be a bit opaque that conversion is happening. I would prefer a simple non-editable info below or besides the editor area. With a red marking or something similar when the exp value already expired. But you are right that there is not much space currently. Maybe the editor needs a redesign. The JWT4B extension looks a little bit more clear for example. But I'm not a UX designer and unfortunately don't know how to do it best. :man_shrugging:

In any case, thank you for considering this feature idea :)

DolphFlynn commented 4 months ago

@exploide Thanks for your thoughts; your reasoning makes sense.

There's a few options within the editor:

  1. On the bottom right of the panel. There should be enough room here, although it's a little separated from the payload.
  2. You could try and fit something underneath 'Compact JSON' checkbox, although it's pretty restricted with horizontal space
  3. One could add a label within the payload section that's only visible when there's something funky with the time claims. Obviously you lose vertical real estate here, so arguably not great on a laptop.

I guess the first one would be my choice, but I'm not a UX designer either :)

exploide commented 4 months ago

I'm also tending towards the first option.

But I'm opposed to the third option because there are use cases for viewing decoded timestamps even if there isn't anything funky at all. For example I would like to see how long the time frame is where I can reuse the token. Or whether it has an untypically long validity period and I want to report that to the customer.

DolphFlynn commented 4 months ago

OK I'll have a go at option 1.

Happy to include messaging if the token validity is unusually long as well. What would be your definition of an untypically long validity period?

exploide commented 4 months ago

Not quite sure. Might depend on some circumstances, whether it is "just" a JWT, whether it is OAuth/OIDC and there also is a refresh token in place... My focus shifted a bit away from web pentesting. Maybe I get a chance to get the opinion from our OAuth expert, don't know.

exploide commented 4 months ago

@JonasPrimbs any opinion on whether it looks feasible to decide if a JWT token's validity is unusually long or does it always "depend"? Context is if a programmatic check in the extension would make sense.

JonasPrimbs commented 4 months ago

In context of an OAuth Access Token or an OIDC ID Token, a JWT should not be valid for longer than 1 hour (3600s). In case of critical environments (banking, critical infrastructure) it should be even less (about 10 minutes). But this is just a personal recommendation. Afaik, there is no official document which specifies this explicitly. They always describe the access token validity as "short-lived".

But JWT is a very universally used format.

For example: In context of OpenID for Verifiable Credentials, JWTs are used as a digital replacement for id cards which should be usable offline. In that case, JWTs would be valid for at least 24 hours, typically for a week, and sometimes even for years. But those JWTs can be identified with their "typ" parameter in the JOSE header, e.g. "vc+sd-jwt" see here for Verifiable Credentials, while access tokens should have the "typ" "at+jwt" see here.

DolphFlynn commented 4 months ago

@exploide I've added a warning label when any of 'iat', 'exp' or 'nbf' are invalid. I'm sure it can be improved, so let me know what you think.

I've not added any logic around 'exp' dates in the future for now.

exploide commented 4 months ago

Thank you for the effort implementing this!

I built the extension from source and had a quick look. If I see correctly, you implemented a warning label but no actual decoding of the timestamps? Being able to read them in human-readable format is what I was actually asking for. Sorry if there was a misunderstanding. I just need a convenient way to see what dates and times are represented by the timestamps.

Warnings about invalid values are nice to have, but not that essential for me. However, combining it would actually be really nice, e.g. by highlighting the human-readable datetime in red if expired or something like this.

Example on how the JWT4B extension is doing it:

jwt4b

This is what I would need :)

Side note: During testing I stumbled upon a page delivering the claim "iat": "2020-04-24T19:14:06.967" and jwt-editor reported iat value is invalid. I think it is uncommon to not have epoch seconds in the claim but it might happen and I don't know whether this should really be considered invalid.

Another side note concerning the visual representation: I found the page https://token.dev/ where they implemented the decoding via hover tooltip over the timestamp. And if it is expired, it gets a red underlining. But I don't know if this is fun to implement in Burp.

DolphFlynn commented 4 months ago

Thanks for the feedback. It's pretty easy to decode the values and display the dates from where we are currently, but we're kind of back to the start in terms of real estate on the panel, i.e. where to put them :)

Based on the RFC (for what they are worth!), "iat": "2020-04-24T19:14:06.967" would be considered invalid.

Tooltips and underlining for the payload panel would be a fair bit of work. I think it would require a JWT payload specific lexer instead of using a generic JSON one. I've written a few of these in the past, so it's doable, but not a five minute job.

DolphFlynn commented 4 months ago

@exploide I'm pretty familiar with the RSyntaxTextArea code so if you do want to go down the route of underlining and tooltips then I can definitely point you in the right direction.

exploide commented 4 months ago

Based on the RFC (for what they are worth!), "iat": "2020-04-24T19:14:06.967" would be considered invalid.

I also had a look on the RFC and I think it is not that clear. For the exp claim it is stated that

Its value MUST be a number containing a NumericDate value.

And then the NumericDate section states:

This is equivalent to the [...] "Seconds Since the Epoch", in which each day is accounted for by exactly 86400 seconds, other than that non-integer values can be represented. See RFC 3339 [RFC3339] for details regarding date/times in general and UTC in particular.

The unclear statement is that "other than that non-integer values can be represented" and then the linked RFC3339 gives examples of Internet date/time format like 1985-04-12T23:20:50.52Z.

But maybe I misunderstand something. And we should not overthink it, this was just a side note. Feel free to mark it as invalid as long as no more complaints are coming :D

if you do want to go down the route of underlining and tooltips then I can definitely point you in the right direction

Nah, this was just some idea, I don't really require something that fancy.

DolphFlynn commented 4 months ago

Thanks I missed the bit about non-integer values. As you suggest that can wait for now :)

So back to the main event. We don't have enough room at the bottom of the panel, so how about another checkbox in the payloads panel to convert iat, nbf, exp epoch times to date strings? One could add a global default in the config. I'd probably leave it off for starters.

exploide commented 4 months ago

We don't have enough room at the bottom of the panel, so how about another checkbox in the payloads panel to convert iat, nbf, exp epoch times to date strings? One could add a global default in the config.

I have some usability concerns regarding another checkbox/option. The less knobs the user has to bother with, the better. "Configurability is the root of all evil."

I would rather go with some layout changes. As I already mentioned, I'm not a UX designer but some ideas are:

DolphFlynn commented 4 months ago

Thanks for your thoughts. They are really useful!

I hear what you're saying about yet another config option.

The space to the right of the JWS signature is definitely an option. The only downside I can think of is that it's going to be empty if there's no dates in the payload.

I'm not mad about more scrolling. Plenty of people use Burp on laptops with small screens.

I'll have a go with adding a new panel to the right of the signature.

DolphFlynn commented 4 months ago

@exploide It's not totally finished, but if you get a chance, can you have a look and see if this is more what you're after? All feedback welcome.

exploide commented 4 months ago

Yes, this is what I had in mind :) Thank you very much for your ongoing effort.

Feel free to close this issue once you are done.

DolphFlynn commented 4 months ago

@exploide Excellent! Thanks for your patience. I'll make a new version with these changes in then email PortSwigger to get them into the BAppStore.