Repository navigation
Replies: 8 comments 37 replies
|
One complicating factor here is that currently Admin APIs doesn't return the created the client representation when it is created, which makes it impossible to return the plain text secret if it's generated by Keycloak and stored as a hash. Another issue is that client secrets are for some reason validated by the ClientModel, which doesn't have access to the KeycloakSession and can't get access to a client secret hashing provider. |
|
When updating a client as "confidential", the secret is not returned. But Keycloak already has the endpoint to generate a new secret for the client: https://www.keycloak.org/docs-api/15.0/rest-api/index.html#_regeneratesecret Setting a client to "confidential" could either create a plain text secret (as is) or it could create no secret (maybe breaking change which needs compatibility mode?). Getting a hashed secret is then possible with the generate endpoint. |
|
I like that idea. If hashing is not enabled it would just continue working as is. If hashing is enabled that is a "breaking change" anyways as you can no longer retrieve the secret for a client, so needs to be taken into account regardless. I'd say if hashing is enabled and it is a confidential client we could flag it as not having a valid secret, and require calling the regenerate secret separately. Not ideal, but I'm worried that changing the create client to return a full client representation would be a breaking change as well. |
|
It is a breaking change but returning the client representation is what provides the best experience from a client's perspective. Perhaps, even from a UX too when using the administration console. From a REST perspective, the specs are not 100% clear if returning the newly created resource in the response body is OK but use links to fetch the newly created resource. At the same time, we see a lot of APIs returning a payload in the response representing the newly created resource and we also have endpoints doing the same. |
|
Not sure about the "not necessary to salt". If I am not mistaken, salts are not only used to add entropy to weak passwords but also to prevent using precomputed rainbow tables with key/hash values. |
|
Sure, but that's for short/poor human rememberable passwords. Keycloak generates 32 character random client secrets, so there really is no point in salting something that's already random. |
|
To salt, or not to salt, that is the question. I reckon we'll just do a salt, even though with long random secrets it shouldn't really be needed. |
|
It shouldn't but salt is recommended as per https://cheatsheetseries.owasp.org/cheatsheets/Password_Storage_Cheat_Sheet.html. Even though secrets are generated with enough entropy it seems a common approach to salt (as we do for user password) to make it harder to crack hashes or even correlate secrets from different clients as being the same. I like the proposal about still allowing using plain text. For me, salt can be optional and not a default when hashing secrets. |
|
Started some rough prototype here: https://github.com/stianst/keycloak/tree/client-secret-hashing |
|
As we discussed, another small issue (in addition to admin console and admin REST API) is client registration read request https://datatracker.ietf.org/doc/html/rfc7592#section-3 and similar request in client management specs https://datatracker.ietf.org/doc/html/rfc7592#section-3 . These are supposed to return client secret, which we won't be able to do with hashing. Also there is this "Installation" tab, which is supposed to return keycloak.json with client secret. If we want to improve this, we may need to encrypt the secret before storing to the DB with some "secret value", which is not stored in the DB (like vault option or something like that). But that is another complexity though? So I guess we just live with the fact that secrets are not returned from any of those places if secret hashing is enabled? |
|
We should be able to return the client's secret during creation, no? Not true though if you are querying the client. |
|
Sure, creation is not a problem and we already return in the response to creation. This is per OIDC client registration specification where metadata about newly created client (including client secret) are returned https://openid.net/specs/openid-connect-registration-1_0.html#RegistrationResponse . The problem is the GET request as I've mentioned. And also the UPDATE request (unless the client secret is rotated/re-generated during update request). |
|
For those operations, I think it is OK to not return it in the response as long as people are aware about the implications.
I don't think we update the secret during updates, do we? Accordingly to https://www.rfc-editor.org/rfc/rfc7592#page-7, we should not allow clients to manage their secrets as this is a AS-only concern. |
|
Yes, true. However with client-secret-rotation enabled, the client secret can be automatically updated during client UPDATE request (to the new value generated by Keycloak server, not value provisioned by client himself). So in this case when client-secret-rotation is enabled, we will be able to return newly generated secret from the UPDATE request. When the client secret was not rotated (which is always with disabled client-secret-rotation), we won't be able to return client secret from UPDATE request. |
|
Hashing the client secret also impacts clients using the |
|
But it's still a matter of plausible deniability: If auth server stores no client secrets, it's very plausible that the client is fully responsibe in case of secret leakage. Like with user passwords, nobody can claim that a provider lost clear text passwords or encrypted passwords if you can proof there were never saved like that in the first place. That said, for the case where passwords are stored it's still very good to store them encrypted. Just want to point out that this approach still differs very much from storing hash only. |
|
@mposolda I don't think it is an option and I share the arguments from @jbman. I would also add that it adds a lot of complexity and that will probably make life really hard when enabling the capability herein proposed. Based on OWASP, encryption should not be used for password/secrets. And the reason is pretty much what @jbman mentioned. The original text can still be retrieved. One thing I was wondering though, I know very few people use encryption at rest. Isn't this an alternative to not only client secrets but any other sensitive data? Including those that involve privacy? |
@jbman @pedroigor Thanks for these points guys. Yes, the added complexity with "valve" is also something, which worries me...
I am not sure what you mean? Could you please elaborate a bit more? |
|
Database fully encrypted. Most vendors enabled this including cloud database services like RDS. |
|
Yes, true. So with regards to it, I wonder what is exactly the priority of this task (considering that there are also disadvantages of this...) |
|
Was there a continuation of this discussion? The last comment seems to be asking to clarify the priority. It surprises me that the client secrets are stored in plain text in the database. Are they considered that much less sensitive than user passwords? |
|
I'd say this is still important, and I agree with client credentials being sensitive even if the credentials aren't re-used elsewhere. This especially apply to service accounts. End of the day there are a numbers of ways client credentials can be exposed one important one being through the Admin APIs/console, and is more secure to hash it than not. |
|
Do you have any indication when this will be ready/started? Or where is this on the roadmap? We would really like this feature. |
|
We don't have any plans on working on this anytime soon I'm afraid, but if anyone would like to contribute something that'd be great. |
|
Ok thank you. Do you have any documentation on how to get a client credential flow working with x509 certificates? We have been struggling with finding the right resources to implement this. I thought this would be easy to find seeing this is the preferred way to implement this. |
|
You need to add a CA certificate to the truststore, then just configure the x509 subject dn for the client. Look at the server guides for the first and the admin guide for the second. |
|
I want a hashing secret feature, but it seems to be a hard work and breaking change. |
|
I agree with signed jwt or x509 certificates being what should be recommended, as end of the day those are stronger credentials than a shared secret (hashed or not). I don't think hashing secrets is that hard work though. It is a breaking change sure, which is why it would need to be optional. |
|
Hi team, It's possible ? |
|
Is what possible? |
|
I can take a stab on implementing client secret hashing. I collected some notes from here about possible impacts https://gist.github.com/tsaarni/be561c8ea8bdcdb2ebe02d30937eb7e0. Any other pointers? @stianst You mentioned you had a PoC at one point #8455 (comment) but it does not seem to exist anymore? It seems to me that the current password hashing support and |
|
I dropped attempting client secret hashing. |
|
Sorry what do you mean by this ? You made a PR ? Unfortunately Vault SPI add unnecessary complexity for something which are very common in 2026 ( hashing stored secrets). |
I mean that I studied the impacts and found them too disruptive. Too many use cases require cleartext client secrets, all of which would somehow need to be somehow disabled, breaking the backwards compatibility of the interfaces. Since you responded specifically in my comment where I had written I can work on this, I gave an update that I dropped the hashing idea and went the other way to protect client secrets. |
|
Ok I understand sorry for that. Do you talk about this impact analysis : https://gist.github.com/tsaarni/be561c8ea8bdcdb2ebe02d30937eb7e0 ? I prefered to ask again because there was no clear statement from KC team that it will not be implemented at all or at least I did not find him . But yes I understand thanks to the impact assessment why it is difficult. I also ask because that issue asking for something similar has not been closed. So for me, the situation was confusing. So If I understand well :
Thanks for the work |
|
Yes, exactly. |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
For clients, and especially service accounts, it would be beneficial to enabling hashing of client secrets, to prevent abuse if they are leaked.
Client secrets should be long and randomly generated as such do not need to be hashed with as computationally expensive hashing algorithms as user passwords, nor do they need to be salted as they should already be randomly generated. As such a straight-forward one-way hash is more than sufficient.
A realm should have an option to configure client secret hashing algorithm from:
This should be done through a ClientSecretHashingSPI to make it easy to introduce further hashing algorithms in the future if needed.
After the client secret hashing algorithm has been updated new client secrets should be hashed with the new algorithm. It is also required to store which hashing algorithm was used alongside the hashed client secret, since this can change overtime for a realm.
One thing to note is that once a client secret is hashed it is no longer possible to retrieve the client secret through the admin console after the client has initially been created, or the password has been updated. This can be considered an additional hardening as it prevents further leak of client secrets.
JIRA: https://issues.redhat.com/browse/KEYCLOAK-15842
All reactions