verify_hash

[guoqi233]

when i use auth_token_required, the verify_hash function is too slow,how can i optimize it?

[jirikuncar]

We can make it optional under a configuration, but in general it should not be removed if we want to invalidate tokens when user changed her password (e.g. after a leak and someone could generate their own tokens).

[eregnier]

I did this : #839

edit: there is another issue about this topic : #731

[jwag956]

I have been thinking about this.
First - the token itself is signed, so we should be able to trust the contents.
So the issue seems to be - what if the user changed password or was deactivated/deleted - that token probably should be rejected (lets assume token has no max-age).
Currently - in request_loader - we are looking up the User in order to compare hashed passwords - this is really expensive. Couldn't we instead simply add a 'generation' number to the User model and bump that anytime we want to invalidate tokens for that user (such as deactivating or changing password). We would embed that generation # in the token (just as we do the hashed password today).
This then becomes a simple integer comparison.
basically - this shifts the work from something done all the time to something done very infrequently (changing password).
The main downside is that it would require folks to update their user model if they wanted the performance improvement...

[eregnier]

I really love the idea to avoir doing computation for each request, even more network / database accesses are required. So your idea @jwag956 looks cool.

However, for such issue, I don't want to break the current API, so the implementation you suggests should be at least optional as all user that want to use it have to change upgrade their models. This is not something easy to do, and I think such change must not be mandatory althrought it can be a great improvement.

Some other thought about your proposal jwag are :

• putting the integer in the hash looks like a security hole for me as some attacker may brute force a token with few combination to find a valid one (increment on password change does not happen often) and as long as flask-security code is open source (or not) the strategy of integrating a "version" in the token seems to be a bad idea to me.

• another point to me is the fact that such a processing for user authentication using tokens looks not standard to me. but maybe I am wrong. And I don't know if it is a good thing.

I really think this kind of problem should be solved using cache, but here again, I am not totally sure of what I say :)

[jwag956]

Thanks for your input.
First - of course we cant break current API - my thought was to implement this as - if the UserModel has the attribute 'fs_generation' then have the token contain the user_id and generation_id - otherwise it would behave exactly as today.

Second - (and I do have some experience in this) - the actual contents of the authorization token is application specific - the current flask-security auth token contains:
[str(self.id), hash_data(self.password)]
which is a list of user_id and encrypted password. Note that the authtoken itself is NOT encrypted - it is signed (this is similar to the more 'std' JWT formats). Signing is crytographically secure (depending on signing algorithm - which is another topic). You can see this - if you look at the returned authorization_token and take that and b64decode it - you will see the list.

SIgning is what allows the server to trust the incoming data - so an attacker can't just try different values since they can't sign the token correctly.

Note that more secure than the token is the cookie which you get as part of session (if you are a browser) - this cookie just has the user_id in it - much more secure that a UI trying to locally store the authentication token...

I think a cache is fine - but it has a couple shortcomings:

1. currently it is per thread - and while it'd be pretty easy to be per-process - unless we put reddis or something behind it - there will still be more misses than we would like
2. at short TTL -there will be spikes in request timings - with long TTL there is security concerns.