-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
oauth2: BCrypt hashs inputs (passwords) of maximum of 72 bytes, limiting OAuth 2.0 Client Secret length #1438
Comments
This is reproducible with: $ hydra clients create --id baz --secret 525348e77144a9cee9a7471a8b67c50ea85b9e3eb377a3c1a3a23db88f9150eefe76e6a339fdbc62b817595f53d72549d9ebe36438f8c2619846b963e9f43a94 --endpoint http://localhost:4445 --token-endpoint-auth-method client_secret_post --grant-types client_credentials
$ hydra token client --client-id baz --client-secret 525348e77144a9cee9a7471a8b67c50ea85b9e3eb377a3c1a3a23db88f9150eefe76e6a3 --endpoint http://localhost:4444 We are using BCrypt to hash secrets of clients. BCrypt has an upper limit:
Secrets longer than what is supported by BCrypt will therefore be truncated/discarded by the BCrypt algorithm. |
/cc @leonfancy |
Will hydra fix this limitation? One workaround is: Before Bcrypt, use SHA or other HASH function on the client_secrect to generate a shorter length hashed string, and then pass it to bcrypt hash function. |
From a standpoint of how secure the password hash is that wouldn't make sense. Generating a collision (or brute-forcing) a 56 byte password with the default BCrypt cost of 10 would take longer than the universe is old. Changing the hashing algorithm in a backwards incompatible way is not an option. If at all, we could add other hashing algorithms and make this configurable. Hashing an input before passing it down to another hash function will not increase security. In fact, choosing the wrong hash method will actually decrease it. This is especially true for SHA:
Another option is to disallow secrets longer than 56 bytes. However, limiting secret length is considered bad practice as it makes password managers that use very long secrets unusable. |
This is now a documented limitation: ory/docs#144 |
I did some research on other algorithms. PBKDF2 supports a maximum of 64 bytes (depending on its core hash function))�. An alternative to bcrypt is scrypt which however never got any real traction because of several shortcomings. The algorithm chosen by the PHC is argon2 which is also what systems that deal with user-generated passwords ( OAuth 2.0 Clients however are usually computer generated and the credentials are generated by default from a cryptographic random function (e.g. Secure mechanisms for authentication of OAuth 2.0 Clients are supported by ORY Hydra. You can, for example, use RS256 to authenticate OAuth 2.0 Clients, completely eliminating the need for a password. As this limitation has now been documented and for the reasons above, I'm closing this as a known limitation. TL;DR
|
This is documented now |
Describe the bug
Only the begining 72 characters of client_secrect is used to authentication
Reproducing the bug
Steps to reproduce the behavior:
525348e77144a9cee9a7471a8b67c50ea85b9e3eb377a3c1a3a23db88f9150eefe76e6a339fdbc62b817595f53d72549d9ebe36438f8c2619846b963e9f43a94
/oauth2/token
, send the begining 72 characters of client_secrect, which is525348e77144a9cee9a7471a8b67c50ea85b9e3eb377a3c1a3a23db88f9150eefe76e6a3
. It pass the authentication, and return an access_token.Server configuration
Expected behavior
Must reject the request if the client_secrect is incorrect.
Environment
The text was updated successfully, but these errors were encountered: