-
Notifications
You must be signed in to change notification settings - Fork 124
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
Proposal for KeyStore SPI #345
base: main
Are you sure you want to change the base?
Conversation
@tsaarni Nice write-up. If I got it correctly, we are going to have multiple identifiers so that we can use key|trust stores for different purposes: LDAP client, SMTP, HTTP Client, HTTP Server, etc. These identifiers are going to potentially map to configuration options that we should make available from the distribution. Does it make sense? Also, should the proposed SPI be used too to configure realm keys? The hot-reload capabilities being proposed might be useful there too. |
Thanks @pedroigor!
Yes, exactly.
Oh, that sounds interesting! I assume that possibility must apply only those cases when realm keys are loaded from keystore, right? So would there need to be a "realm dimension" when loading keystores? I guess same problem applies to some other cases too. For example, LDAP federation is configured per realm, so in theory administrator might need multiple LDAP keystores (one per realm at max)? Realm specific config seems difficult to "model" with config properties. |
I'm not pushing for including realm keys at this point but just something we might consider in the future given that we do have realm keys based on keystores. Although key rotation is already possible with realm keys, doing it automatically as herein proposed, would be an improvement. So, back to the scope of the proposed design, using an identifier to map keystores should work for "server-level" key stores such as the one used to enable HTTPS. However, I'm unsure if the same is true for components (such as user federation providers like LDAP) as they are bound to a realm. We could potentially append to the identifier the name of the realm to map keystores to a realm but realms are dynamic and you would need to lazily load keystores from the filesystem. If you keep using the same keystore for all realms you are kinda in the same situation where the same keys and trust anchors are shared across distinct TLS domains. |
@pedroigor I was also thinking a bit about realm specific trust|key store problem when responding to keycloak/keycloak#13300 (comment). In a way there is discrepancy in component configuration when it comes to trust anchors and credentials. For example, today, in component configuration for LDAP user federation we set everything else needed to connect to the server except the path to trust store. Admin had to anticipate this and set trust store as config properties before starting Keycloak. The same would be true even if we would add realm part into the identifier in the proposed KeyStore SPI - administrator would still need to set the configuration in advance. This discrepancy has been OK for me so far, but I can understand why this can be a problem for others. If going one step further, I could imagine that the administrator provisions relevant trust|key store as part of component config. In simplest form, it could be just paths pointing to local files on the server - what was previously given as startup config. It is doable: split the KeyStore SPI between server-level vs realm-specific domains. Server-level would be using the identifiers mapped from parameters given at startup. In realm-specific case, the consumer of the SPI would need to pass the config directly. It would add new impact to SPI consumers. For example, LDAP user federation implementation needs to store and pass the config for KeyStore SPI. There is more complicated scenario as well: imagine the realm admin is not able or allowed to put the files on the server(s) and therefore trust|key stores should be part of component config somehow: they could be uploaded / downloaded, stored to / fetched from database etc. In that case the identifier passed by consumer is not path but maybe e.g. some other reference that can be resolved by the KeyStore SPI implementation. There might be some resemblance here to Vault SPI. One shortcut that crossed my mind previously, was that component config could refer to a key alias in a store that was pre-configured at startup. After hot-reload is supported, it would be possible to add new entries on demand e.g., lazily at the time when LDAP user federation is configured. But I do not think this is the optimal user experience. |
Thanks. As I mentioned before, it should be fine for now to support server-level keystores. If we agree that the current proposal is OK even though realms would still be sharing the same keystores, I agree to move it forward. I was thinking about how certs/keys are going to be reloaded in kubernetes. Looks like we would need to restart the server pods anyways, right? |
Thank you @pedroigor.
An alternative approach could be to have KeyStore SPI load key stores and PEM files only by file path (assuming credentials are always files and not e.g. in SQL database). The caller could be then responsible for resolving the config into paths, instead of the KeyStore SPI implementation doing that. That could avoid namespacing server-level vs realm-level and move the problem towards the caller.
I would like to avoid requiring restart for server pods. I'm also running Keycloak on Kubernetes so this use case is close to me. Assuming the certs/keys are loaded from secret mount, the complication is due to the way how Kubernetes updates the files. I've previously recorded details about the update process and inotify events emitted by it (link). It is bit challenging to use inotify with this update scheme, and most importantly, the same implementation would not work for every other file update schemes. For example, Envoy ended up later adding more configurability for administrator to set the watched directory via config. It seems bit complicated to me, and I wish administrator would not need to care. So far, I did not include default implementation details in the KeyStore SPI proposal document, but my personal preference for watching cert/key file updates would be to avoid inotify (with some additional reasons I collected here in this paragraph) and I would prefer just "clever" polling instead: when Entries are fetched from KeyStore (i.e. during TLS handshake), lazily check if the files on disk have been modified since they were last loaded. Define a "cache TTL" period that caps how frequently this check is executed at most. The benefit in this approach is that it works regardless of how the files are updated, including the scheme that Kubernetes uses for secret mount updates. |
@pedroigor and @mposolda May I ask for your help, it would be great if we could proceed with this feature! @mposolda from your comment keycloak/keycloak#7365 (comment) I understand that you prefer that KeyStore SPI would support single keystore, similarly as TrustStore SPI does today. I can modify the proposal accordingly. But I wonder what is your opinion on my follow up question in keycloak/keycloak#7365 (comment)? As far as I can see, user would need to configure multiple keystores if targeting both LDAP SASL EXTERNAL and HTTPS server certificate hot-reload use case as well. |
I wanted to ask again if it would be possible to push keystore SPI / certificate hot-reload forward? Keycloak lost hot-reload support when moving from Wildfly to Quarkus (Elytron default SSL context could be hot-reloaded via Wildfly CLI). In my use case we cannot live without hot-reload so I've needed to fork Keycloak for now, and it is getting painful to us. |
The SPI should naturally support both levels because it is going to have access to the realm. For now, it should be fine to move forward with the server-level key material.
+1 |
I've updated the proposal: Instead of using "keystore identifiers" to request a keystore for a specific use case, I simply added just file paths. I'm hoping this could avoid the "namespace" discussion (e.g. here and here) and move the "addressing" problem to SPI consumers instead. As a result, for example LDAP provider would need to become aware of the files. I'm thinking: if we assume that certs/key + CA cert becomes part of LDAP config in realm, isn't it then natural consequence that LDAP provider needs to become aware of the files? This raises new questions to me:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not quite following this proposal. Is this basically about supporting multiple different truststores for different contexts? Or, is it about providing the keystore part for Keycloak's own private certs when connecting to external services? Or, both? How does this overlap with vault for example?
Can you explain how an end-user would use what is proposed here? For example to "configure LDAP or SMTP" using mTLS authentication?
@stianst Thank you for your comment! Originally, this proposal included only the ability to configure multiple keystores globally. The intention is to allow end-user to set specific client certificate (+ chain certificates) when communicating towards LDAP servers using mutually authenticated LDAPS or LDAP + StartTLS connections. I received comments (above) where it was highlighted that the configuration is in fact realm specific. Therefore, I'm proposing the ability to configure multiple keystores and truststores (both are technically keystores). The intention is to allow end-user to set specific set of client/server certs and CA trust anchors for different contexts, both in global and in realm specific context. Another use case that I'd like to address is hot-reload of client and server certificates, without requiring restart of Keycloak. Since it is very strongly related to the keystore implementation, it is included in this proposal. Example use case here is rotation and hot-reload of HTTPS server certificate. It is also an example of a globally configured certificate (in contrast to realm context). The situation today:
|
I do not see overlap with Vault SPI, since it is addressing opaque data blobs and it does not support There is overlap with TrustStore SPI. My thinking here is that KeyStore SPI could allow unifying the provisioning of both client/server certificates and CA certificates. Technically truststores are |
CC @shawkins |
This PR proposes new SPI called "KeyStore". The purpose of the SPI is to allow an arbitrary number of key stores to be provisioned.
This work is split from old PR keycloak/keycloak#7365. I'm hoping that by creating a separate design document, and splitting the work into smaller chunks it would be possible to get reviews and feedback.