Hot-reloadable LDAP bind password #104320
Conversation
slobodanadamovic
added
>enhancement
:Security/Authentication
|
Hi @slobodanadamovic, I've created a changelog YAML for you. |
…d-secure-bind-password
…d-secure-bind-password
…d-secure-bind-password
| @@ -1960,16 +1969,6 @@ public void reload(Settings settings) throws Exception { | |||
| } | |||
| } | |||
|
|
|||
| private void reloadSharedSecretsForJwtRealms(Settings settingsWithKeystore) { | |||
There was a problem hiding this comment.
Moved this logic to JwtRealm.
…d-secure-bind-password
…d-secure-bind-password
…d-secure-bind-password
…d-secure-bind-password
…d-secure-bind-password
…d-secure-bind-password
…d-secure-bind-password
…d-secure-bind-password
…d-secure-bind-password
| * The implementors of this interface will be called when the values of {@code SecureSetting}s should be reloaded by security plugin. | ||
| * For more information about reloading plugin secure settings, see {@link ReloadablePlugin}. | ||
| */ | ||
| public interface ReloadableSecurityComponent { |
There was a problem hiding this comment.
This change was inspired by Tim's POC. I liked the generic approach and decided to adopt it.
|
Pinging @elastic/es-security (Team:Security) |
| * For additional info, see also: {@link ReloadablePlugin#reload(Settings)}. | ||
| * | ||
| * @param settings | ||
| * Settings used at the time of reloading the component. |
There was a problem hiding this comment.
Can you clarify this abit ? i.e. Settings used at the time of reloading. If there were updates to the secure settings those updates will be present in this Settings object.
There was a problem hiding this comment.
Will try to clarify it. I was mostly trying to echo the same description that ReloadablePlugin has, but by omitting some details it ended up not making sense.
In the end the fact is that the settings passed to the reloadable component will contain all node settings (loaded during node startup) merged with the decrypted secure ones (loaded from the keystore). The secure settings can only be accessed during the method call (should not be stored or attempted to be accessed async), because once the reload method finishes, the KeyStoreWrapper will be closed, which will wipe all secure settings.
| * if the operation fails. The component should continue to operate as | ||
| * if the failing call didn't happen. | ||
| */ | ||
| void reload(Settings settings) throws Exception; |
There was a problem hiding this comment.
I am not a fan of interfaces that require explicity handling of overly generic checked exceptions. Is there any way to make this a more specific exception, or just omit the checked requirement entirely ?
There was a problem hiding this comment.
My intention was to be consistent with higher-level ReloadablePlugin interface. But on a second thought it is not necessary to replicate it here. I will remove it.
| // the existing pooled connections. LDAP connections are stateful, and once a connection is | ||
| // established and bound, it remains open until explicitly closed or until a connection | ||
| // timeout occurs. Changing the bind password on the LDAP server does not automatically | ||
| // invalidate existing connections. Hence, simply setting the new bind request is sufficient. |
There was a problem hiding this comment.
thanks for the addtional comments.
| // established and bound, it remains open until explicitly closed or until a connection | ||
| // timeout occurs. Changing the bind password on the LDAP server does not automatically | ||
| // invalidate existing connections. Hence, simply setting the new bind request is sufficient. | ||
| connectionPool.setBindRequest(bindRequest.get()); |
There was a problem hiding this comment.
In my earlier POC, I attempted to reload the connection pool by triggering the health check.
That means that any connections that are using a now-expired password would get dropped and replaced.
Is it intentional that you don't do that?
There was a problem hiding this comment.
Yes, this is intentional. The health check does not help in detecting connections whose bind credentials have changed. Once connection is bound, it remains open even if the bind password changes. Both default health check implementation LDAPConnectionPoolHealthCheck (used when nothing is configured) and GetEntryLDAPConnectionPoolHealthCheck (used when health check is enabled) are doing only simple checks - none of these health checks can detect the password change. The only way to detect that bind password has changed is to attempt to re-bind again.
Based on my testing, after reloading bind password, the already bound connection can still be used for searching and authenticating users. The catch is that it can only be used once. This is because we call maybeForkThenBindAndRevert which under the hood calls LDAPConnectionPool#bindAndRevertAuthentication. The bindAndRevertAuthentication will attempt to re-bind (revert) the connection's authentication using old bind credentials which will fail and connection will be terminated. Polling new connections from the connection pool will use the new bind credentials.
I've also spent some time unit testing this today and running local integration tests against OpenLDAP server. The behaviour seems consistent.
…d-secure-bind-password
…d-secure-bind-password
This is a followup to #104320 which adds validation during secure setting reload of a `bind_password`. The reload of `secure_bind_password` will now fail with an exception instead of logging a deprecation warning.
This is a followup to #104320, which updates the docs for `secure_bind_password` settings and marks them as reloadable.
This PR enables
secure_bind_passwordsecure setting to be updated without the need to restart nodes.The
secure_bind_passwordmust be updated on the AD/LDAP server, then changed in the Elasticsearch keystore and reloaded viareload_secure_settingsAPI.Note: This PR does not support grace period where both old and new passwords are active.