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

storing tokens securely #387

Closed
corbosman opened this issue Oct 10, 2015 · 8 comments
Closed

storing tokens securely #387

corbosman opened this issue Oct 10, 2015 · 8 comments

Comments

@corbosman
Copy link

Hi, lucadegasperi/oauth2-server-laravel uses this package to implement an oauth2 server using laravel. Unfortunately they dont hash the tokens before storing it in the database so the tokens end up stored plaintext. When an issue was opened to address this, the author claims that thephpleague/oauth2-server should implement the hashing.

To me it seems that the storage implementation should implement hashing, but id love to hear your opinion about this.

@alexbilbie
Copy link
Contributor

I guess it should be at the storage layer but what sort of hashing are you proposing?

@corbosman
Copy link
Author

A one way hash like bcrypt, so tokens are treated the same as one would store passwords. Same goes for client secrets, which in this specific application are stored plaintext in a database as well. The authors claims this hashing should be done in thephpleague/oauth2-server.

@alexbilbie
Copy link
Contributor

I'd definitely hash client secrets.

As for tokens the issue is you need to validate them. A client will send you a plaintext token which you need to validate; if they are stored bcrypt-hashed how do you now look them up in your database to verify them?

@corbosman
Copy link
Author

You hash them again, and compare the hashes, just like how passwords work.

@alexbilbie
Copy link
Contributor

That won't work. With password hashing you search by the username to get the password hash and then do a hash compare to check the password is correct.

With your proposal you don't have anything other than the plaintext token to go by.

<?php

$token = password_hash('my token', PASSWORD_BCRYPT);

var_dump($token); // string(60) "$2y$10$aPRdl7LU3CNw3z.zXIBo3eN5qPEAxEoT7E6ajrclPvIhLu0nEDx3W"

var_dump(password_hash('my token', PASSWORD_BCRYPT)); // string(60) "$2y$10$b0bj7kkGPcPREMyZTcRPUeOetI/zcaL9wWHV9qQ81P.fCN3epatFa"

Unless you use hashing algorithm that always produces the same output then you won't be able to perform a database lookup. But then you get into the same arguments about salting and hashing yourself and it's a mess.

@corbosman
Copy link
Author

Ah yes, you're right, i didnt realise there is no way to identify the token. But then at least they could be encrypted with a secret right? Seems better than storing them plaintext in a database that could escape into the wild, as we often see happen.

@alexbilbie
Copy link
Contributor

A better strategy would be to only store short lived tokens so if they did escape they're only useful for an hour or so at most

@spronkey
Copy link

@alexbilbie @corbosman I just want to point out that the only purpose of a salt is to prevent the same plaintext being revealed multiple times over i.e. cracking a bunch of the same passwords. Tokens should have enough entropy in them that they are unique enough, so there's not a huge downside I can see for hashing without salting them - just use a high rounds hash and it should be fine.

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

3 participants