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

Token Sidejacking prevention #70

Closed
adonis28850 opened this issue Jul 18, 2017 · 15 comments
Closed

Token Sidejacking prevention #70

adonis28850 opened this issue Jul 18, 2017 · 15 comments

Comments

@adonis28850
Copy link

adonis28850 commented Jul 18, 2017

Hi mate,

I've been reading this attack vector from the OWASP: https://www.owasp.org/index.php/JSON_Web_Token_(JWT)_Cheat_Sheet_for_Java#Token_sidejacking

And I was wondering what you reckon about the option of adding "user context" and then validating it. It would be trivial to add this data as you describe here: https://flask-jwt-extended.readthedocs.io/en/latest/add_custom_data_claims.html, but I'm not sure how easy would be to then add that check to the JWT validation process as well, as opposed to having to do it manually for every request an API receives if there is no native support from this module.

Cheers!

@vimalloc
Copy link
Owner

This sounds related to #64 to me.

I think what I would like to do is provide something like a @jwt.claims_verification_loader decorator that allows you to provide your own callback method which is used to verify the claims of a token. This would be called after decoding the token and making sure it passed the signature verification and was not expired. You should be able to access the flask request in this callback method, to check if the IP address matched what you were expecting, etc, and should give you full control over how to verify your tokens are what you expected.

Let me think about this some more, and I'll try to get something implemented for you by next week.

Cheers 👍

@adonis28850
Copy link
Author

adonis28850 commented Jul 18, 2017

a @jwt.claims_verification_loader decorator that allows you to provide your own callback method which is used to verify the claims of a token. --> That sounds like a good idea to me.

Thanks for your prompt response and keep the good work up!

@adonis28850
Copy link
Author

Hey mate,

After having another think about JWT and security, I'm having some questions you may be able to help with or at least give me your opinion. The mean reason I'm choosing token-based authentication with JWT is to avoid all the overload of having stateful sessions and having to store all the details in the DB.

Now, if security is an issue for you, then you need to think about a mechanism to invalidate tokens if the situation requires it (token compromise, logout, user changing passwords, etc.). For this, most people recommend token revocation, for which you need to then keep the tokens stored in the server, and check them every time, etc., But then, why don't stick to good old cookies if you end up doing kind of the same thing?!

May it be the case that building on the concept of 'adding user context' to the tokens helps deal with this issue in a more effective way that avoids having to add statefulness to something that is meant to be stateless?

Cheers.

@vimalloc
Copy link
Owner

That's a tough question that I think really depends on the requirements of your application. As a general thought, regardless of what you add in the adding user context, it probably cannot be 100% secure in the case of a stolen token. If you are basing it off an ip address, the attacker might have access to the same computer or it might be a different user on the NAT. If you are relying on additional headers about the browser configuration, they could be spoofed.

An idea that might be worth considering is only checking the refresh tokens against the blacklist, and have short lived access tokens that are secured with the additional user context. The refresh tokens shouldn't be used often (maybe once ever 5 to 10 minutes depending on how long your access token is valid for), so it shouldn't take many resources on your application to check it against a blacklist. Combining this with an access token that will expire in the next 5 to 10 minutes and is only valid from a specific ip should also limit the amout of damage that could be done if the access token is stolen. You still have to worry about what damage could be done if the refresh token is stolen, but thanks to the blacklist you have a way to revoke those tokens.

Another option (that can be used by itself or in conjunction with the above paragraph), is to use the 'token_freshness' pattern . This basically causes any access tokens created by the refresh token to be marked as not fresh, and you can use the @fresh_jwt_required decorator to prevent these tokens from accessing the most sensitive parts of your application (changing passwords, submitting credit cards, making payments, etc). You would also need to add another endpoint to your application that checked the users password, and if it was valid return to them a new fresh access token. This way if a fresh access token was stolen, it would only be able to do damage for 5~10 minutes until it expired, and if the refresh token was stolen they wouldn't be able to use that to access your most critical endpoints.

Hopefully these give you some ideas to think about. If you have any questions about this (or anything else), don't hesitate to let me know.

Cheers :)

@adonis28850
Copy link
Author

adonis28850 commented Jul 20, 2017

Yeah I agree, I work in infosec, and it is obvious that there is nothing 100% secure, it's all about risk assessment.

In this particular case, I think the small project I'm working on does not actually have any really sensitive data, and therefore I think the approach of using the token_freshness pattern + adding some extra context to the access_token + lowering the life time of expiry_time to something like a few hours instead of days will do it.

A few thoughts though that I found interesting from your last comment:

  • If you are basing it off an ip address, the attacker might have access to the same computer or it might be a different user on the NAT.

True, but I believe that if as an attacker you are on the same local network, or even worse on the same computer, there are many more nasty things you can do. So I still find valid the approach of adding context to the tokens as a reasonable way of increasing the security of the tokens while keeping everything stateless.

  • On blacklists and token revocation. I don't think there is something wrong with this approach, and if you have an app that has sensitive data this is possibly a must in order to minimize damages in case of token compromise. However, I dislike it in the sense that it goes against the whole idea of tokens and statelessness, do you know what I mean?. If you are at the point where you need to add state to your architecture, why not using cookies then?

  • I was not familiar with the concept of fresh tokens, is that something you came up with?. I think it is a neat idea and reasonable compromise, and combining it to the idea of context can be a good alternative to blacklisting and revoking (depending on the nature of your app of course).

  • and you can use the @fresh_jwt_required decorator to prevent these tokens from accessing the most sensitive parts of your application (changing passwords, [...]

Quick note on this, changing password functionality should ask you for your old password, so I wouldn't consider necessary to flag this functionality, but of course, you can use it for other sensitive stuff such as adding CC details, modifying your profile, etc.

  • Last thought, and here I'm possibly missing something cause I'm not able to get my head around it. Aren't refresh_tokens redundant?! I mean, chances are that in your application you end up storing both, the access_token and the refresh_token, in the same location (local storage, cookies, whatever). So if an attacker can get hold of one it will be able to get hold of the other as well, right?. Then, what's the point of having both and not just having an access_token that instead of expiring every 5 minutes or whatever, expires at a reasonable time, say 4 hours?. I'm sure I'm missing something here!

All in all, and this is a personal opinion, I'd try to stay away from token revokation and anything that involves adding more load on the server, as I think this goes against the principle of JWT. If your app really needs this cause of security or other reasons, then maybe JWT is not the best option? Also, I believe combining the ideas of fresh tokens and context may give you enough control and reasonable security, but of course, this has to be assessed on a case by case basis.

BTW, thank for your time and effort, I really appreciate it!

Cheers!

@vimalloc
Copy link
Owner

On blacklists and token revocation. I don't think there is something wrong with this approach, and if you have an app that has sensitive data this is possibly a must in order to minimize damages in case of token compromise. However, I dislike it in the sense that it goes against the whole idea of tokens and statelessness, do you know what I mean?.

I totally know what you mean. In fact, in our use of this library we aren't using token blacklists, but if we ever have a stolen token causing problems that very well might change. I feel that it is important to have it as an option. I also think you could argue that there are other reasons to pick a token based system even if you have state on your server (but again, these will depend largely on your use cases, and there is no one size fits all here).

If you are at the point where you need to add state to your architecture, why not using cookies then?

To be over the top pedantic here, cookies and jwts are not mutually exclusive. There is no standard for how to send a jwt to your backend (I have seen them stored in cookie, sent in via headers, and as part of the url before). That said, knowing that you mean cookies with something like a sessions database on the backend, one reason you may prefer jwts is that if you are working outside of a browser (for example, on a mobile application or with another program that consumes a rest api), using headers is generally much easier than maintaining your own cookie jar. On the flip side, there are some benefits to using cookies in the browser (either with jwts stored in the cookie or with a more standards session database). In particular, the secureoption which insure the cookie is only sent over https, and the httponly only which insures the cookie cannot be read by javascript / xss attacks. On the flip flip side, now you have to add additional security on top of the cookie to prevent CSRF attacks.

Personally, we have our stuff setup to allow the jwts to be passed by either cookies or authorization headers. This allows us to take advantage of secure and httponly in the web browser (with addition CSRF protection baked on top of that), and use authorization headers for various other tools and cron jobs that interact with the rest api.

I was not familiar with the concept of fresh tokens, is that something you came up with?

It's not something I came up with per se, it's just something I noticed amazon.com did after spending too much ordering things from them, and thought it would be a really helpful feature to add to this extension :P

Quick note on this, changing password functionality should ask you for your old password

Very good point, changing passwords was a bad example there.

All in all, and this is a personal opinion, I'd try to stay away from token revokation and anything that involves adding more load on the server, as I think this goes against the principle of JWT. If your app really needs this cause of security or other reasons, then maybe JWT is not the best option?

I can absolutely see where you are coming from here. Personally I still like tokens for this, as it gives me a bit more control over how much load it will add to the server (do I check the blacklist for every token? Only refresh tokens? Some other metric?). This is of course personal prefence though, and I can totally see why someone would prefer not to go this route.

Thanks for the good conversation!

Cheers :)

@adonis28850
Copy link
Author

Fair points you made there, I'm indeed using cookies to transport the tokens as I think is more secure than using local storage. Not sure how that will work if I even decide to develop an Android or iOS app to interact with the API (it's easy for me as a security guy to suggest all this stuff to improve security, but when you put yourself in the shoes of a developer it's not that easy! 😄 )

Sorry, I updated my comment while you were replying, so I'll just add the changes here:

  • Last thought, and here I'm possibly missing something cause I'm not able to get my head around it. Aren't refresh_tokens redundant?! I mean, chances are that in your application you end up storing both, the access_token and the refresh_token, in the same location (local storage, cookies, whatever). So if an attacker can get hold of one it will be able to get hold of the other as well, right?. Then, what's the point of having both and not just having an access_token that instead of expiring every 5 minutes or whatever, expires at a reasonable time, say 4 hours?. I'm sure I'm missing something here!

And the most important bit:

BTW, thank for your time and effort, I really appreciate it!

@vimalloc
Copy link
Owner

vimalloc commented Jul 21, 2017

Last thought, and here I'm possibly missing something cause I'm not able to get my head around it. Aren't refresh_tokens redundant?! I mean, chances are that in your application you end up storing both, the access_token and the refresh_token, in the same location (local storage, cookies, whatever). So if an attacker can get hold of one it will be able to get hold of the other as well, right?

That's very true. It's mostly additional security for protecting your tokens over the wire. If your access token goes out with every request, but the refresh token only goes out every once in awhile, there is less surface area for the refresh token to be stolen (given, this isn't very likely to be the attack vector in the first place, but still). I have talked to other people who used jwts in their applications, and the approach you outlined is exactly what they did.

The other additional benefit for this is mostly for ease of using the backend. By separating access and refresh tokens it gives us the possibility to easily only check refresh tokens against the blacklist and limit the amount of checks you make against it (if one were to go that route). I'm sure this could be done with just access tokens as well, but it is just easier to manage and think about this way, at least in my opinion.

BTW, thank for your time and effort, I really appreciate it!

Anytime! :D I love being able to chat about stuff like this, it's a great way to learn about new ideas in the ever evolving field of security (and I am positive there is more security stuff like this out there for me to learn!)

Cheers :)

@adonis28850
Copy link
Author

That's very true. It's mostly additional security for protecting your tokens over the wire. If your access token goes out with every request, but the refresh token only goes out every once in awhile, there is less surface area for the refresh token to be stolen (given, this isn't very likely to be the attack vector in the first place, but still). I have talked to other people who used jwts in their applications, and the approach you outlined is exactly what they did.
The other additional benefit for this is mostly for ease of using the backend. By separating access and refresh tokens it gives us the possibility to easily only check refresh tokens against the blacklist and limit the amount of checks you make against it (if one were to go that route). I'm sure this could be done with just access tokens as well, but it is just easier to manage and think about this way, at least in my opinion.

Yeah fair points, but now that I'm thinking, assuming the API is only going to be consumed by SPAs and Mobile Apps, and doing an analogy with the different flows of OAuth2, wouldn't it be actually a bad idea to issue a refresh_token (in this case in particular)? In the implicit flow of OAuth2 refresh_tokens are not issued as this flow was designed for apps that keep the tokens client side (browser or mobile apps), and therefore the risk of issuing a refresh_token when this cannot be kept securely (relatively speaking) is too high!

Jesus, this stuff gives me a headache every time I need to deal with it! 😆

@vimalloc
Copy link
Owner

vimalloc commented Jul 21, 2017

That is a fair point. On the flip side, then you have to setup and manage oauth2 :p

But on a serious note, I think some of the stuff discussed above could help make jwts with refresh tokens more secure and lead to a better user experience for your site, while still being pretty easy to use for the developers. But if I was doing something like a banking application with jwts, I would probably use something like a single access token that expired after 30 minutes and forced you to re-login.

This stuff also makes my brain go in circles. I think that's a sign I should go get a drink ;)

@adonis28850
Copy link
Author

Hahaha that makes two of us needing one then! 😄

Yeah I think for my use case, an API that will be fed into a SPA, and potentially Mobile apps (have you ever done an API with your module to be consumed by mobile apps, where the JWT is sent via cookies and not through a header? if so, how easy is to do bearing in mind you don't have a browser enforcing the HTTPOnly flag?!), it is better to just return a access_token instead of both the access_token and the refresh_token!

But yeah, this is certainly not an easy topic, no wonder why there are so many messed up implementations, you try to do what you think is good in terms of security, but then you omit some nuances and you are screwed!

@vimalloc
Copy link
Owner

have you ever done an API with your module to be consumed by mobile apps, where the JWT is sent via cookies and not through a header? if so, how easy is to do bearing in mind you don't have a browser enforcing the HTTPOnly flag?!

I have not, the most I have done is curl with a cookie jar, and that was painful enough :p But I imagine cookie libraries outside of the browser would just ignore that option (maybe the secure option too, depending on the library).

And true that, getting security right is such a challenge. But hey, if it keeps us employable then I guess there is a silver lining ;)

@adonis28850
Copy link
Author

Indeed, well, have a good rest of the day, let me know when you get a chance to implement the extra features this ticket was originally about, and I'll keep you updated with my never ending questions/ideas!

Laters!

@vimalloc
Copy link
Owner

Ok, got this added, will push out a new version to pip soon.

This adds two new callback loader methods to verify user_claims and change the return value if the user_claims verification fails

# Old function, unchanged
@jwt.user_claims_loader
def add_custom_claims(identity):
    return {
        'foo': 'bar',
        'baz': 'boom'
     }

# New function, verify the user claims in an access token
@jwt.claims_verification_loader
def verify_user_claims(user_claims):
    expected_keys = ['foo', 'baz']
    for key in expected_keys:
        if key not in user_claims:
            return False
    return True

# New function, change the return value if user claims verification failed
@jwt.claims_verification_failed_loader
def failed_user_claim_verification_error():
    return jsonify({'msg': 'Access token is missing key 'foo' or 'baz'}), 404

Cheers

@vimalloc
Copy link
Owner

Released as version 3.2.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants