Replies: 9 comments 16 replies
-
|
Proposal: Provide explicit management for two or multiple secrets which are all valid if set. No need for primary and secondary semantics. An optional description ids helpful, to mark the secret as "current" or as "new one". |
Beta Was this translation helpful? Give feedback.
-
|
Was thinking about something like that, but from my perspective that seems more complicated both to use and implement. Isn't a flow with a explicit new (primary) and and old (secondary) simpler to use? You then just follow these steps:
If you do ids and timestamps that means you need to remember what ID was the old one, to then remove that specific ID at the end, which seems completely unnecessary to me. |
Beta Was this translation helpful? Give feedback.
-
|
The two manageable secrets is just the approach that is used for AWS access keys. I would prefer this atomic operations of secret creation and deletion. Convenience actions could be supported on top.
|
Beta Was this translation helpful? Give feedback.
-
|
It would also be beneficial if Keycloak could emit some sort of warning when a 'deprecated' or 'secondary' key is used. That will make it easier to notice if a client hasn't been updated to use the new secret. |
Beta Was this translation helpful? Give feedback.
-
|
Warning to the client? That would be good but the OAuth protocol only defines successful authentication or an error. https://www.oauth.com/oauth2-servers/access-tokens/access-token-response/ Warning to an Admin? At admin console client secret screen? |
Beta Was this translation helpful? Give feedback.
-
|
I was thinking just a warning event that can be emitted to an event listener - similar to other login events. |
Beta Was this translation helpful? Give feedback.
-
|
I agree, if the secret gets a "deprecated" flag/state then such an event would be nice. However this would be some new kind of synthetic event in addition to the existing ones. Currently events like CLIENT_LOGIN or CODE_TO_TOKEN are emitted. In addition there could be an event like CLIENT_SECRET_DEPRECATED. |
Beta Was this translation helpful? Give feedback.
-
|
I am not sure about introducing another event type for this, however I agree that we can possibly add "detail" (attribute) to an existing events to inform that client authentication was done with the "deprecated" secret. This should be possible to do for any functionality, which uses client authentication (EG. code-to-token, refresh token, token introspection etc). |
Beta Was this translation helpful? Give feedback.
-
|
The way Azure AD does it is by allowing arbitrary numbers of secrets, each having an ID, a description and an expiration date. Pretty flexible, even allowing to give different secrets to different people with different expirations. But classic rotation has to be done by hand. Maybe that's a good compromise? |
Beta Was this translation helpful? Give feedback.
-
|
I think that's an overcomplicated way to do something that should be much simpler;
I'd argue that a client should only have a single credential, with the exception of during a rotation. |
Beta Was this translation helpful? Give feedback.
-
|
I agree with @stianst - even allowing an automatic commit once the new client key has been used once or a few times, or after a certain amount of time. Google Cloud does a similar thing for certificate rotation on a managed kubernetes cluster. |
Beta Was this translation helpful? Give feedback.
-
|
You could do things like use different secrets for teams that share a client. I asked around and most people just use different clients for different teams, so I don't think its strictly necessary. Just want to make it clear that this requirement wouldn't be supported. |
Beta Was this translation helpful? Give feedback.
-
|
Thinking about it, there is another reason why Azure AD might have implemented it in this way. When you create a key, it only becomes visible shortly and you have to take note of it. If you fail to do that, you can still delete that key and add a new one. Assuming we also want to have that "only visible on creation", we should be really careful with how that works i.e. definitely allow to always create a new primary/secondary secret and not force to always switch to the other secret when changing secrets. |
Beta Was this translation helpful? Give feedback.
-
|
We'll be adding support for hashing client secrets (#8455), which will make secrets only visible on create/update time, and not possible to retrieve later. I think what I commented in #8475 should work in this case though. It prevents you from creating yet another secret until you have either cancelled or committed the first rotation. |
Beta Was this translation helpful? Give feedback.
-
|
Starting to wonder if we can do something smarter here; we can add new secret, and remove the old when the new one has been used. Or, if new isn't used within X minutes rollbacks to the old one and add an error event. |
Beta Was this translation helpful? Give feedback.
-
|
Devops teams may use one client for multiple applications, even though dedicated clients would be more secure. A first authentication with new credentials doesn't mean a that rotation is complete. We just need the possibility for devops teams to do automated rotation maybe even up to a daily rate. The approach doesn't need to be smart for that. You just need a minimum of two identifiable and manageable secrets. It helps if one is identified as "active" (via tag, label, description, etc. ). But even this is optional if you have creation time. Then you can derive the younger one as active before start of rotation. |
Beta Was this translation helpful? Give feedback.
-
|
True, I think perhaps then a simper approach is better, where we basically support:
"Start rotation" would optionally support a auto-commit after X minutes. It would also fail if a rotation is currently in progress, and would require doing either cancel or commit to do a new attempt. |
Beta Was this translation helpful? Give feedback.
-
|
Q: Do we need support for "Cancel rotation" ? Maybe initially not needed and add it just if there is explicit ask for that? |
Beta Was this translation helpful? Give feedback.
-
|
@mposolda I think the "Cancel rotation" is important if you have a protocol error. |
Beta Was this translation helpful? F438 Give feedback.
-
|
Few questions:
I suppose that if we want to support it, the "client read request" (either admin REST API or dynamic client registration read request) will need to return new client secret. But that may not be possible due the secret hashing proposal in the other discussion?
Currently we have some "hybrid", which works in a way that:
Regarding support for "set", the client management specification [2] does not allow clients to set their own secret. According to https://datatracker.ietf.org/doc/html/rfc7592#section-2.2 : Currently we do not support "set" for client management, however we support it for REST API. That is maybe bit strange (even if formally it does not break specification). On the other hand, skip possibility for "set" in admin REST endpoint will break backwards compatibility and will require really lots of test changes. In shortcut: I am fine with keep current behaviour (regenerate in admin console, "set" support in REST endpoint only), but just wanted to doublecheck this... [1] https://openid.net/specs/openid-connect-registration-1_0.html |
Beta Was this translation helpful? Give feedback.
-
|
Regarding 1.: Unfortunately the spec [2] does not specify a client triggered secret re-generation, but only a server controlled via Now there is still the option that re-generation as part of a Client read request is Option a) is nice, as two active secrets enable fully automatable credentials rotation in systems which may need to distribute the new secret to multiple nodes. Regarding 2.: |
Beta Was this translation helpful? Give feedback.
-
|
One thing to note about a time based expiration is that it will be good for some use-cases, but for others it could possible lead to problems, especially in complex distributed systems. Communication links can be down and this might mean a system might keep using the old password for longer than the predicted expiration time that is set on the secondary secret (say if it's in minutes).. So, keycloak should also offer the user the ability to take control of the retirement of the old/second secret explicitly (i.e. expose a specific API call to retire the old secret for example). |
Beta Was this translation helpful? Give feedback.
-
|
Reading through the discussions so far I'm quite inclined to think of a simpler solution. Here's a draft flow I've built based on what's been discussed so far (or at least as I understand it).
|
Beta Was this translation helpful? Give feedback.
-
|
I've added an alternative proposal here: #9156 This introduces a realm level client secret policy instead of having the options at the client API level. I believe this is a far better approach than this one. |
Beta Was this translation helpful? Give feedback.
-
|
Hi, is there any update on this topic? |
Beta Was this translation helpful? Give feedback.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
-
Rejected in favour of #9156
Alternative proposal here: #9156
Best practices require secrets to be rotated on a regular basis. This is currently difficult to achieve since its not possible to seamlessly rotate client secrets as a client can only have a single secret at a time. This results in having to synchronously update both the client secret in Keycloak and in the application at the same time, which is in most cases very hard, if not impossible to achieve.
To support seamless rotation of client secrets Keycloak should support the ability to have a primary and a secondary secret for a client. In addition it should be possible to set an expiration time on the secondary secret.
When updating the client secret the Admin Console and Admin APIs should have the following options:
JIRA: https://issues.redhat.com/browse/KEYCLOAK-14567
Beta Was this translation helpful? Give feedback.
All reactions