Deprecate timeout.tcp_read AD/LDAP realm setting (#47305)

The timeout.tcp_read AD/LDAP realm setting, despite the low-level
allusion, controls the time interval the realms wait for a response for
a query (search or bind). If the connection to the server is synchronous
(un-pooled) the response timeout is analogous to the tcp read timeout.
But the tcp read timeout is irrelevant in the common case of a pooled
connection (when a Bind DN is specified).

The timeout.tcp_read qualifier is hereby deprecated in favor of
timeout.response.

In addition, the default value for both timeout.tcp_read and
timeout.response is that of timeout.ldap_search, instead of the 5s (but
the default for timeout.ldap_search is still 5s). The
timeout.ldap_search defines the server-controlled timeout of a search
request. There is no practical use case to have a smaller tcp_read
timeout compared to ldap_search (in this case the request would time-out
on the client but continue to be processed on the server). The proposed
change aims to simplify configuration so that the more common
configuration change, adjusting timeout.ldap_search up, has the expected
result (no timeout during searches) without any additional
modifications.

Closes #46028

Author: albertzaharovits

docs/reference/settings/security-settings.asciidoc

@@ -442,12 +442,19 @@ An `s` at the end indicates seconds, or `ms` indicates milliseconds.
 Defaults to `5s` (5 seconds ).
 
 `timeout.tcp_read`::
-The TCP read timeout period after establishing an LDAP connection.
-An `s` at the end indicates seconds, or `ms` indicates milliseconds.
-Defaults to `5s` (5 seconds ).
+deprecated[7.7] The TCP read timeout period after establishing an LDAP
+connection. This is equivalent to and is deprecated in favor of
+`timeout.response` and they cannot be used simultaneously. An `s` at the end
+indicates seconds, or `ms` indicates milliseconds.
+
+`timeout.response`::
+The time interval to wait for the response from the LDAP server. An `s` at the
+end indicates seconds, or `ms` indicates milliseconds. Defaults to the value of
+`timeout.ldap_search`.
 
 `timeout.ldap_search`::
-The LDAP Server enforced timeout period for an LDAP search.
+The timeout period for an LDAP search. The value is specified in the request
+and is enforced by the receiving LDAP Server.
 An `s` at the end indicates seconds, or `ms` indicates milliseconds.
 Defaults to `5s` (5 seconds ).
 
@@ -691,12 +698,20 @@ An `s` at the end indicates seconds, or `ms` indicates milliseconds.
 Defaults to `5s` (5 seconds ).
 
 `timeout.tcp_read`::
-The TCP read timeout period after establishing an LDAP connection.
-An `s` at the end indicates seconds, or `ms` indicates milliseconds.
-Defaults to `5s` (5 seconds ).
+deprecated[7.7] The TCP read timeout period after establishing an LDAP
+connection.  This is equivalent to and is deprecated in favor of
+`timeout.response` and they cannot be used simultaneously. An `s` at the end
+indicates seconds, or `ms` indicates milliseconds. Defaults to the value of
+`timeout.ldap_search`.
+
+`timeout.response`::
+The time interval to wait for the response from the AD server. An `s` at the
+end indicates seconds, or `ms` indicates milliseconds. Defaults to the value of
+`timeout.ldap_search`.
 
 `timeout.ldap_search`::
-The LDAP Server enforced timeout period for an LDAP search.
+The timeout period for an LDAP search. The value is specified in the request
+and is enforced by the receiving LDAP Server.
 An `s` at the end indicates seconds, or `ms` indicates milliseconds.
 Defaults to `5s` (5 seconds ).
 

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/ldap/support/SessionFactorySettings.java

@@ -25,12 +25,16 @@ public final class SessionFactorySettings {
     public static final Function<String, Setting.AffixSetting<TimeValue>> TIMEOUT_TCP_CONNECTION_SETTING = RealmSettings.affixSetting(
             "timeout.tcp_connect", key -> Setting.timeSetting(key, TIMEOUT_DEFAULT, Setting.Property.NodeScope));
 
-    public static final Function<String, Setting.AffixSetting<TimeValue>> TIMEOUT_TCP_READ_SETTING = RealmSettings.affixSetting(
-            "timeout.tcp_read", key -> Setting.timeSetting(key, TIMEOUT_DEFAULT, Setting.Property.NodeScope));
-
     public static final Function<String, Setting.AffixSetting<TimeValue>> TIMEOUT_LDAP_SETTING = RealmSettings.affixSetting(
             "timeout.ldap_search", key -> Setting.timeSetting(key, TIMEOUT_DEFAULT, Setting.Property.NodeScope));
 
+    public static final Function<String, Setting.AffixSetting<TimeValue>> TIMEOUT_TCP_READ_SETTING = RealmSettings.affixSetting(
+            "timeout.tcp_read", key -> Setting.timeSetting(key, TimeValue.MINUS_ONE, Setting.Property.NodeScope,
+                    Setting.Property.Deprecated));
+
+    public static final Function<String, Setting.AffixSetting<TimeValue>> TIMEOUT_RESPONSE_SETTING = RealmSettings.affixSetting(
+            "timeout.response", key -> Setting.timeSetting(key, TimeValue.MINUS_ONE, Setting.Property.NodeScope));
+
     public static final Function<String, Setting.AffixSetting<Boolean>> HOSTNAME_VERIFICATION_SETTING = RealmSettings.affixSetting(
             "hostname_verification", key -> Setting.boolSetting(key, true, Setting.Property.NodeScope, Setting.Property.Filtered));
 
@@ -49,6 +53,7 @@ public static Set<Setting.AffixSetting<?>> getSettings(String realmType) {
         settings.add(URLS_SETTING.apply(realmType));
         settings.add(TIMEOUT_TCP_CONNECTION_SETTING.apply(realmType));
         settings.add(TIMEOUT_TCP_READ_SETTING.apply(realmType));
+        settings.add(TIMEOUT_RESPONSE_SETTING.apply(realmType));
         settings.add(TIMEOUT_LDAP_SETTING.apply(realmType));
         settings.add(HOSTNAME_VERIFICATION_SETTING.apply(realmType));
         settings.add(FOLLOW_REFERRALS_SETTING.apply(realmType));

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ldap/support/SessionFactory.java

@@ -121,7 +121,22 @@ protected static LDAPConnectionOptions connectionOptions(RealmConfig config,
         LDAPConnectionOptions options = new LDAPConnectionOptions();
         options.setConnectTimeoutMillis(Math.toIntExact(config.getSetting(SessionFactorySettings.TIMEOUT_TCP_CONNECTION_SETTING).millis()));
         options.setFollowReferrals(config.getSetting(SessionFactorySettings.FOLLOW_REFERRALS_SETTING));
-        options.setResponseTimeoutMillis(config.getSetting(SessionFactorySettings.TIMEOUT_TCP_READ_SETTING).millis());
+        final long responseTimeoutMillis;
+        if (config.hasSetting(SessionFactorySettings.TIMEOUT_RESPONSE_SETTING)) {
+            if (config.hasSetting(SessionFactorySettings.TIMEOUT_TCP_READ_SETTING)) {
+                throw new IllegalArgumentException("[" + RealmSettings.getFullSettingKey(config,
+                        SessionFactorySettings.TIMEOUT_TCP_READ_SETTING) + "] and [" + RealmSettings.getFullSettingKey(config,
+                        SessionFactorySettings.TIMEOUT_RESPONSE_SETTING) + "] may not be used at the same time");
+            }
+            responseTimeoutMillis = config.getSetting(SessionFactorySettings.TIMEOUT_RESPONSE_SETTING).millis();
+        } else {
+            if (config.hasSetting(SessionFactorySettings.TIMEOUT_TCP_READ_SETTING)) {
+                responseTimeoutMillis = config.getSetting(SessionFactorySettings.TIMEOUT_TCP_READ_SETTING).millis();
+            } else {
+                responseTimeoutMillis = config.getSetting(SessionFactorySettings.TIMEOUT_LDAP_SETTING).millis();
+            }
+        }
+        options.setResponseTimeoutMillis(responseTimeoutMillis);
         options.setAllowConcurrentSocketFactoryUse(true);
 
         final boolean verificationModeExists = config.hasSetting(SSLConfigurationSettings.VERIFICATION_MODE_SETTING_REALM);

x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/RealmSettingsTests.java

@@ -184,7 +184,7 @@ private Settings.Builder commonLdapSettings(String type, boolean configureSSL) {
                 .put("unmapped_groups_as_roles", randomBoolean())
                 .put("files.role_mapping", "x-pack/" + randomAlphaOfLength(8) + ".yml")
                 .put("timeout.tcp_connect", randomPositiveTimeValue())
-                .put("timeout.tcp_read", randomPositiveTimeValue())
+                .put("timeout.response", randomPositiveTimeValue())
                 .put("timeout.ldap_search", randomPositiveTimeValue());
         if (configureSSL) {
             configureSsl("ssl.", builder, randomBoolean(), randomBoolean());

x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ldap/LdapSessionFactoryTests.java

@@ -92,7 +92,7 @@ public void testBindWithReadTimeout() throws Exception {
         Settings settings = Settings.builder()
                 .put(globalSettings)
                 .put(buildLdapSettings(ldapUrl, userTemplates, groupSearchBase, LdapSearchScope.SUB_TREE))
-                .put(RealmSettings.getFullSettingKey(REALM_IDENTIFIER, SessionFactorySettings.TIMEOUT_TCP_READ_SETTING), "1ms")
+                .put(RealmSettings.getFullSettingKey(REALM_IDENTIFIER, SessionFactorySettings.TIMEOUT_RESPONSE_SETTING), "1ms")
                 .put("path.home", createTempDir())
                 .build();
 

x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ldap/support/SessionFactoryTests.java

@@ -11,6 +11,7 @@
 import org.elasticsearch.action.ActionListener;
 import org.elasticsearch.action.support.PlainActionFuture;
 import org.elasticsearch.common.settings.SecureString;
+import org.elasticsearch.common.settings.Setting;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.util.concurrent.ThreadContext;
 import org.elasticsearch.env.Environment;
@@ -62,13 +63,67 @@ public void testConnectionFactoryReturnsCorrectLDAPConnectionOptionsWithDefaultS
         assertThat(options.getSSLSocketVerifier(), is(instanceOf(HostNameSSLSocketVerifier.class)));
     }
 
+    public void testSessionFactoryWithResponseTimeout() throws Exception {
+        final RealmConfig.RealmIdentifier realmId = new RealmConfig.RealmIdentifier("ldap", "response_settings");
+        final Path pathHome = createTempDir();
+        {
+            Settings settings = Settings.builder()
+                    .put(getFullSettingKey(realmId, SessionFactorySettings.TIMEOUT_RESPONSE_SETTING), "10s")
+                    .put("path.home", pathHome)
+                    .build();
+
+            final Environment environment = TestEnvironment.newEnvironment(settings);
+            RealmConfig realmConfig = new RealmConfig(realmId, settings, environment, new ThreadContext(settings));
+            LDAPConnectionOptions options = SessionFactory.connectionOptions(realmConfig, new SSLService(settings, environment), logger);
+            assertThat(options.getResponseTimeoutMillis(), is(equalTo(10000L)));
+        }
+        {
+            Settings settings = Settings.builder()
+                    .put(getFullSettingKey(realmId, SessionFactorySettings.TIMEOUT_TCP_READ_SETTING), "7s")
+                    .put("path.home", pathHome)
+                    .build();
+
+            final Environment environment = TestEnvironment.newEnvironment(settings);
+            RealmConfig realmConfig = new RealmConfig(realmId, settings, environment, new ThreadContext(settings));
+            LDAPConnectionOptions options = SessionFactory.connectionOptions(realmConfig, new SSLService(settings, environment), logger);
+            assertThat(options.getResponseTimeoutMillis(), is(equalTo(7000L)));
+            assertSettingDeprecationsAndWarnings(new Setting<?>[]{SessionFactorySettings.TIMEOUT_TCP_READ_SETTING.apply("ldap")
+                    .getConcreteSettingForNamespace("response_settings")});
+        }
+        {
+            Settings settings = Settings.builder()
+                    .put(getFullSettingKey(realmId, SessionFactorySettings.TIMEOUT_RESPONSE_SETTING), "11s")
+                    .put(getFullSettingKey(realmId, SessionFactorySettings.TIMEOUT_TCP_READ_SETTING), "6s")
+                    .put("path.home", pathHome)
+                    .build();
+
+            final Environment environment = TestEnvironment.newEnvironment(settings);
+            RealmConfig realmConfig = new RealmConfig(realmId, settings, environment, new ThreadContext(settings));
+            IllegalArgumentException ex = expectThrows(IllegalArgumentException.class, () -> SessionFactory.connectionOptions(realmConfig
+                    , new SSLService(settings, environment), logger));
+            assertThat(ex.getMessage(), is("[xpack.security.authc.realms.ldap.response_settings.timeout.tcp_read] and [xpack.security" +
+                    ".authc.realms.ldap.response_settings.timeout.response] may not be used at the same time"));
+        }
+        {
+            Settings settings = Settings.builder()
+                    .put(getFullSettingKey(realmId, SessionFactorySettings.TIMEOUT_LDAP_SETTING), "750ms")
+                    .put("path.home", pathHome)
+                    .build();
+
+            final Environment environment = TestEnvironment.newEnvironment(settings);
+            RealmConfig realmConfig = new RealmConfig(realmId, settings, environment, new ThreadContext(settings));
+            LDAPConnectionOptions options = SessionFactory.connectionOptions(realmConfig, new SSLService(settings, environment), logger);
+            assertThat(options.getResponseTimeoutMillis(), is(equalTo(750L)));
+        }
+    }
+
     public void testConnectionFactoryReturnsCorrectLDAPConnectionOptions() throws Exception {
         final RealmConfig.RealmIdentifier realmId = new RealmConfig.RealmIdentifier("ldap", "conn_settings");
         final Path pathHome = createTempDir();
         Settings settings = getSettingsBuilder()
                 .put(getFullSettingKey(realmId, SessionFactorySettings.TIMEOUT_TCP_CONNECTION_SETTING), "10ms")
                 .put(getFullSettingKey(realmId, SessionFactorySettings.HOSTNAME_VERIFICATION_SETTING), "false")
-                .put(getFullSettingKey(realmId, SessionFactorySettings.TIMEOUT_TCP_READ_SETTING), "20ms")
+                .put(getFullSettingKey(realmId, SessionFactorySettings.TIMEOUT_RESPONSE_SETTING), "20ms")
                 .put(getFullSettingKey(realmId, SessionFactorySettings.FOLLOW_REFERRALS_SETTING), "false")
                 .put("path.home", pathHome)
                 .build();