Hot-reloadable LDAP bind password (elastic#104320)

This PR enables `secure_bind_password` setting to be updated via `reload_secure_settings` API, without the need to restart nodes. The `secure_bind_password` must be updated on the AD/LDAP server, changed in the Elasticsearch keystore and reloaded via `reload_secure_settings` API. This change does not include a support for the grace period, where both old and new passwords are active. The new password change is active immediately after reload and will be used when establishing new LDAP 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 will not affect and invalidate existing connections.
jimczi · Feb 2, 2024 · e87f58d · e87f58d
1 parent c552bc1
commit e87f58d
Show file tree

Hide file tree

Showing 17 changed files with 531 additions and 94 deletions.
diff --git a/docs/changelog/104320.yaml b/docs/changelog/104320.yaml
@@ -0,0 +1,5 @@
+pr: 104320
+summary: Hot-reloadable LDAP bind password
+area: Authentication
+type: enhancement
+issues: []
diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java
@@ -179,7 +179,6 @@
 import org.elasticsearch.xpack.core.security.authc.Realm;
 import org.elasticsearch.xpack.core.security.authc.RealmConfig;
 import org.elasticsearch.xpack.core.security.authc.RealmSettings;
-import org.elasticsearch.xpack.core.security.authc.jwt.JwtRealmSettings;
 import org.elasticsearch.xpack.core.security.authc.support.UsernamePasswordToken;
 import org.elasticsearch.xpack.core.security.authz.AuthorizationEngine;
 import org.elasticsearch.xpack.core.security.authz.AuthorizationServiceField;
@@ -372,6 +371,7 @@
 import org.elasticsearch.xpack.security.rest.action.user.RestSetEnabledAction;
 import org.elasticsearch.xpack.security.support.CacheInvalidatorRegistry;
 import org.elasticsearch.xpack.security.support.ExtensionComponents;
+import org.elasticsearch.xpack.security.support.ReloadableSecurityComponent;
 import org.elasticsearch.xpack.security.support.SecuritySystemIndices;
 import org.elasticsearch.xpack.security.transport.SecurityHttpSettings;
 import org.elasticsearch.xpack.security.transport.SecurityServerTransportInterceptor;
@@ -410,7 +410,6 @@
 import static org.elasticsearch.xpack.core.XPackSettings.API_KEY_SERVICE_ENABLED_SETTING;
 import static org.elasticsearch.xpack.core.XPackSettings.HTTP_SSL_ENABLED;
 import static org.elasticsearch.xpack.core.security.SecurityField.FIELD_LEVEL_SECURITY_FEATURE;
-import static org.elasticsearch.xpack.core.security.authc.jwt.JwtRealmSettings.CLIENT_AUTHENTICATION_SHARED_SECRET;
 import static org.elasticsearch.xpack.core.security.authz.store.ReservedRolesStore.INCLUDED_RESERVED_ROLES_SETTING;
 import static org.elasticsearch.xpack.security.operator.OperatorPrivileges.OPERATOR_PRIVILEGES_ENABLED;
 import static org.elasticsearch.xpack.security.transport.SSLEngineUtils.extractClientCertificates;
@@ -564,6 +563,7 @@ public class Security extends Plugin
     private final SetOnce<WorkflowService> workflowService = new SetOnce<>();
     private final SetOnce<Realms> realms = new SetOnce<>();
     private final SetOnce<Client> client = new SetOnce<>();
+    private final SetOnce<List<ReloadableSecurityComponent>> reloadableComponents = new SetOnce<>();
 
     public Security(Settings settings) {
         this(settings, Collections.emptyList());
@@ -635,8 +635,8 @@ protected Client getClient() {
         return client.get();
     }
 
-    protected Realms getRealms() {
-        return realms.get();
+    protected List<ReloadableSecurityComponent> getReloadableSecurityComponents() {
+        return this.reloadableComponents.get();
     }
 
     @Override
@@ -1046,6 +1046,13 @@ Collection<Object> createComponents(
 
         cacheInvalidatorRegistry.validate();
 
+        this.reloadableComponents.set(
+            components.stream()
+                .filter(ReloadableSecurityComponent.class::isInstance)
+                .map(ReloadableSecurityComponent.class::cast)
+                .collect(Collectors.toUnmodifiableList())
+        );
+
         return components;
     }
 
@@ -1948,11 +1955,13 @@ public void reload(Settings settings) throws Exception {
                 reloadExceptions.add(ex);
             }
 
-            try {
-                reloadSharedSecretsForJwtRealms(settings);
-            } catch (Exception ex) {
-                reloadExceptions.add(ex);
-            }
+            this.getReloadableSecurityComponents().forEach(component -> {
+                try {
+                    component.reload(settings);
+                } catch (Exception ex) {
+                    reloadExceptions.add(ex);
+                }
+            });
 
             if (false == reloadExceptions.isEmpty()) {
                 final var combinedException = new ElasticsearchException(
@@ -1966,16 +1975,6 @@ public void reload(Settings settings) throws Exception {
         }
     }
 
-    private void reloadSharedSecretsForJwtRealms(Settings settingsWithKeystore) {
-        getRealms().stream().filter(r -> JwtRealmSettings.TYPE.equals(r.realmRef().getType())).forEach(realm -> {
-            if (realm instanceof JwtRealm jwtRealm) {
-                jwtRealm.rotateClientSecret(
-                    CLIENT_AUTHENTICATION_SHARED_SECRET.getConcreteSettingForNamespace(realm.realmRef().getName()).get(settingsWithKeystore)
-                );
-            }
-        });
-    }
-
     /**
      * This method uses a transport action internally to access classes that are injectable but not part of the plugin contract.
      * See {@link TransportReloadRemoteClusterCredentialsAction} for more context.

diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/Realms.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/Realms.java
@@ -8,6 +8,7 @@
 
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
+import org.elasticsearch.ElasticsearchException;
 import org.elasticsearch.action.ActionListener;
 import org.elasticsearch.action.support.RefCountingRunnable;
 import org.elasticsearch.common.Strings;
@@ -35,6 +36,7 @@
 import org.elasticsearch.xpack.core.security.authc.kerberos.KerberosRealmSettings;
 import org.elasticsearch.xpack.security.Security;
 import org.elasticsearch.xpack.security.authc.esnative.ReservedRealm;
+import org.elasticsearch.xpack.security.support.ReloadableSecurityComponent;
 
 import java.io.Closeable;
 import java.io.IOException;
@@ -57,7 +59,7 @@
 /**
  * Serves as a realms registry (also responsible for ordering the realms appropriately)
  */
-public class Realms extends AbstractLifecycleComponent implements Iterable<Realm> {
+public class Realms extends AbstractLifecycleComponent implements Iterable<Realm>, ReloadableSecurityComponent {
 
     private static final Logger logger = LogManager.getLogger(Realms.class);
     private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(logger.getName());
@@ -566,4 +568,23 @@ private static Map<String, Object> convertToMapOfLists(Map<String, Object> map)
         }
         return converted;
     }
+
+    @Override
+    public void reload(Settings settings) {
+        final List<Exception> reloadExceptions = new ArrayList<>();
+        for (Realm realm : this.allConfiguredRealms) {
+            if (realm instanceof ReloadableSecurityComponent reloadableRealm) {
+                try {
+                    reloadableRealm.reload(settings);
+                } catch (Exception e) {
+                    reloadExceptions.add(e);
+                }
+            }
+        }
+        if (false == reloadExceptions.isEmpty()) {
+            final var combinedException = new ElasticsearchException("secure settings reload failed for one or more realms");
+            reloadExceptions.forEach(combinedException::addSuppressed);
+            throw combinedException;
+        }
+    }
 }
diff --git a/...ck/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/jwt/JwtRealm.java b/...ck/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/jwt/JwtRealm.java
@@ -19,6 +19,7 @@
 import org.elasticsearch.common.cache.CacheBuilder;
 import org.elasticsearch.common.settings.RotatableSecret;
 import org.elasticsearch.common.settings.SecureString;
+import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.settings.SettingsException;
 import org.elasticsearch.common.util.concurrent.ReleasableLock;
 import org.elasticsearch.common.util.concurrent.ThreadContext;
@@ -40,6 +41,7 @@
 import org.elasticsearch.xpack.core.ssl.SSLService;
 import org.elasticsearch.xpack.security.authc.support.ClaimParser;
 import org.elasticsearch.xpack.security.authc.support.DelegatedAuthorizationSupport;
+import org.elasticsearch.xpack.security.support.ReloadableSecurityComponent;
 
 import java.util.Collection;
 import java.util.Collections;
@@ -51,13 +53,14 @@
 
 import static java.lang.String.join;
 import static org.elasticsearch.core.Strings.format;
+import static org.elasticsearch.xpack.core.security.authc.jwt.JwtRealmSettings.CLIENT_AUTHENTICATION_SHARED_SECRET;
 import static org.elasticsearch.xpack.core.security.authc.jwt.JwtRealmSettings.CLIENT_AUTH_SHARED_SECRET_ROTATION_GRACE_PERIOD;
 
 /**
  * JWT realms supports JWTs as bearer tokens for authenticating to Elasticsearch.
  * For security, it is recommended to authenticate the client too.
  */
-public class JwtRealm extends Realm implements CachingRealm, Releasable {
+public class JwtRealm extends Realm implements CachingRealm, ReloadableSecurityComponent, Releasable {
 
     private static final String LATEST_MALFORMED_JWT = "_latest_malformed_jwt";
 
@@ -399,7 +402,9 @@ public void usageStats(final ActionListener<Map<String, Object>> listener) {
         }, listener::onFailure));
     }
 
-    public void rotateClientSecret(SecureString clientSecret) {
+    @Override
+    public void reload(Settings settings) {
+        var clientSecret = CLIENT_AUTHENTICATION_SHARED_SECRET.getConcreteSettingForNamespace(this.realmRef().getName()).get(settings);
         this.clientAuthenticationSharedSecret.rotate(clientSecret, config.getSetting(CLIENT_AUTH_SHARED_SECRET_ROTATION_GRACE_PERIOD));
     }
 

diff --git a/.../main/java/org/elasticsearch/xpack/security/authc/ldap/ActiveDirectorySessionFactory.java b/.../main/java/org/elasticsearch/xpack/security/authc/ldap/ActiveDirectorySessionFactory.java
@@ -46,6 +46,7 @@
 import java.util.List;
 import java.util.Optional;
 import java.util.concurrent.ExecutionException;
+import java.util.function.Supplier;
 
 import static org.elasticsearch.xpack.security.authc.ldap.support.LdapUtils.attributesToSearchFor;
 import static org.elasticsearch.xpack.security.authc.ldap.support.LdapUtils.createFilter;
@@ -102,7 +103,8 @@ class ActiveDirectorySessionFactory extends PoolingSessionFactory {
             groupResolver,
             metadataResolver,
             domainDN,
-            threadPool
+            threadPool,
+            this::getBindRequest
         );
         downLevelADAuthenticator = new DownLevelADAuthenticator(
             config,
@@ -117,7 +119,8 @@ class ActiveDirectorySessionFactory extends PoolingSessionFactory {
             ldapPort,
             ldapsPort,
             gcLdapPort,
-            gcLdapsPort
+            gcLdapsPort,
+            this::getBindRequest
         );
         upnADAuthenticator = new UpnADAuthenticator(
             config,
@@ -127,7 +130,8 @@ class ActiveDirectorySessionFactory extends PoolingSessionFactory {
             groupResolver,
             metadataResolver,
             domainDN,
-            threadPool
+            threadPool,
+            this::getBindRequest
         );
 
     }
@@ -187,7 +191,7 @@ void getUnauthenticatedSessionWithoutPool(String user, ActionListener<LdapSessio
         }
         try {
             final LDAPConnection connection = LdapUtils.privilegedConnect(serverSet::getConnection);
-            LdapUtils.maybeForkThenBind(connection, bindCredentials, true, threadPool, new AbstractRunnable() {
+            LdapUtils.maybeForkThenBind(connection, getBindRequest(), true, threadPool, new AbstractRunnable() {
 
                 @Override
                 public void onFailure(Exception e) {
@@ -274,8 +278,7 @@ abstract static class ADAuthenticator {
         final String userSearchDN;
         final LdapSearchScope userSearchScope;
         final String userSearchFilter;
-        final String bindDN;
-        final SecureString bindPassword;
+        final Supplier<SimpleBindRequest> bindRequestSupplier;
         final ThreadPool threadPool;
 
         ADAuthenticator(
@@ -288,19 +291,16 @@ abstract static class ADAuthenticator {
             String domainDN,
             Setting.AffixSetting<String> userSearchFilterSetting,
             String defaultUserSearchFilter,
-            ThreadPool threadPool
+            ThreadPool threadPool,
+            Supplier<SimpleBindRequest> bindRequestSupplier
         ) {
             this.realm = realm;
             this.timeout = timeout;
             this.ignoreReferralErrors = ignoreReferralErrors;
             this.logger = logger;
             this.groupsResolver = groupsResolver;
             this.metadataResolver = metadataResolver;
-            this.bindDN = getBindDN(realm);
-            this.bindPassword = realm.getSetting(
-                PoolingSessionFactorySettings.SECURE_BIND_PASSWORD,
-                () -> realm.getSetting(PoolingSessionFactorySettings.LEGACY_BIND_PASSWORD)
-            );
+            this.bindRequestSupplier = bindRequestSupplier;
             this.threadPool = threadPool;
             userSearchDN = realm.getSetting(ActiveDirectorySessionFactorySettings.AD_USER_SEARCH_BASEDN_SETTING, () -> domainDN);
             userSearchScope = LdapSearchScope.resolve(
@@ -348,10 +348,10 @@ protected void doRun() throws Exception {
                             }, e -> { listener.onFailure(e); }));
                         }
                     };
-                    if (bindDN.isEmpty()) {
+                    final SimpleBindRequest bind = bindRequestSupplier.get();
+                    if (bind.getBindDN().isEmpty()) {
                         searchRunnable.run();
                     } else {
-                        final SimpleBindRequest bind = new SimpleBindRequest(bindDN, CharArrays.toUtf8Bytes(bindPassword.getChars()));
                         LdapUtils.maybeForkThenBind(connection, bind, true, threadPool, searchRunnable);
                     }
                 }
@@ -423,7 +423,8 @@ static class DefaultADAuthenticator extends ADAuthenticator {
             GroupsResolver groupsResolver,
             LdapMetadataResolver metadataResolver,
             String domainDN,
-            ThreadPool threadPool
+            ThreadPool threadPool,
+            Supplier<SimpleBindRequest> bindRequestSupplier
         ) {
             super(
                 realm,
@@ -435,7 +436,8 @@ static class DefaultADAuthenticator extends ADAuthenticator {
                 domainDN,
                 ActiveDirectorySessionFactorySettings.AD_USER_SEARCH_FILTER_SETTING,
                 "(&(objectClass=user)(|(sAMAccountName={0})(userPrincipalName={0}@" + domainName(realm) + ")))",
-                threadPool
+                threadPool,
+                bindRequestSupplier
             );
             domainName = domainName(realm);
         }
@@ -503,7 +505,8 @@ static class DownLevelADAuthenticator extends ADAuthenticator {
             int ldapPort,
             int ldapsPort,
             int gcLdapPort,
-            int gcLdapsPort
+            int gcLdapsPort,
+            Supplier<SimpleBindRequest> bindRequestSupplier
         ) {
             super(
                 config,
@@ -515,7 +518,8 @@ static class DownLevelADAuthenticator extends ADAuthenticator {
                 domainDN,
                 ActiveDirectorySessionFactorySettings.AD_DOWN_LEVEL_USER_SEARCH_FILTER_SETTING,
                 DOWN_LEVEL_FILTER,
-                threadPool
+                threadPool,
+                bindRequestSupplier
             );
             this.domainDN = domainDN;
             this.sslService = sslService;
@@ -605,10 +609,9 @@ void netBiosDomainNameToDn(
                         )
                     );
                     final byte[] passwordBytes = CharArrays.toUtf8Bytes(password.getChars());
-                    final boolean bindAsAuthenticatingUser = this.bindDN.isEmpty();
-                    final SimpleBindRequest bind = bindAsAuthenticatingUser
-                        ? new SimpleBindRequest(username, passwordBytes)
-                        : new SimpleBindRequest(bindDN, CharArrays.toUtf8Bytes(bindPassword.getChars()));
+                    final SimpleBindRequest bindRequest = bindRequestSupplier.get();
+                    final boolean bindAsAuthenticatingUser = bindRequest.getBindDN().isEmpty();
+                    final SimpleBindRequest bind = bindAsAuthenticatingUser ? new SimpleBindRequest(username, passwordBytes) : bindRequest;
                     ActionRunnable<String> body = new ActionRunnable<>(listener) {
                         @Override
                         protected void doRun() throws Exception {
@@ -705,7 +708,8 @@ static class UpnADAuthenticator extends ADAuthenticator {
             GroupsResolver groupsResolver,
             LdapMetadataResolver metadataResolver,
             String domainDN,
-            ThreadPool threadPool
+            ThreadPool threadPool,
+            Supplier<SimpleBindRequest> bindRequestSupplier
         ) {
             super(
                 config,
@@ -717,7 +721,8 @@ static class UpnADAuthenticator extends ADAuthenticator {
                 domainDN,
                 ActiveDirectorySessionFactorySettings.AD_UPN_USER_SEARCH_FILTER_SETTING,
                 UPN_USER_FILTER,
-                threadPool
+                threadPool,
+                bindRequestSupplier
             );
             if (userSearchFilter.contains("{0}")) {
                 deprecationLogger.warn(

diff --git a/.../plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ldap/LdapRealm.java b/.../plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ldap/LdapRealm.java
@@ -13,6 +13,7 @@
 import org.elasticsearch.action.ActionListener;
 import org.elasticsearch.action.support.ContextPreservingActionListener;
 import org.elasticsearch.common.settings.Setting;
+import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.util.concurrent.AbstractRunnable;
 import org.elasticsearch.common.util.concurrent.EsExecutors;
 import org.elasticsearch.common.util.concurrent.ThreadContext;
@@ -40,6 +41,7 @@
 import org.elasticsearch.xpack.security.authc.support.DelegatedAuthorizationSupport;
 import org.elasticsearch.xpack.security.authc.support.mapper.CompositeRoleMapper;
 import org.elasticsearch.xpack.security.authc.support.mapper.NativeRoleMappingStore;
+import org.elasticsearch.xpack.security.support.ReloadableSecurityComponent;
 
 import java.util.HashMap;
 import java.util.List;
@@ -57,7 +59,7 @@
 /**
  * Authenticates username/password tokens against ldap, locates groups and maps them to roles.
  */
-public final class LdapRealm extends CachingUsernamePasswordRealm {
+public final class LdapRealm extends CachingUsernamePasswordRealm implements ReloadableSecurityComponent {
 
     private final SessionFactory sessionFactory;
     private final UserRoleMapper roleMapper;
@@ -217,6 +219,11 @@ public void usageStats(ActionListener<Map<String, Object>> listener) {
         }, listener::onFailure));
     }
 
+    @Override
+    public void reload(Settings settings) {
+        this.sessionFactory.reload(settings);
+    }
+
     private static void buildUser(
         LdapSession session,
         String username,