Fix + tests for #133542

docs/changelog/136058.yaml

@@ -0,0 +1,5 @@
+pr: 136058
+summary: "Fix + tests for #133542"
+area: Security
+type: bug
+issues: []

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/saml/SamlRealmSettings.java

@@ -63,6 +63,20 @@ public class SamlRealmSettings {
         key -> Setting.boolSetting(key, false, Setting.Property.NodeScope)
     );
 
+    public static final Function<String, Setting.AffixSetting<TimeValue>> IDP_METADATA_HTTP_CONNECT_TIMEOUT = (type) -> Setting
+        .affixKeySetting(
+            RealmSettings.realmSettingPrefix(type),
+            IDP_METADATA_SETTING_PREFIX + "http.connect_timeout",
+            key -> Setting.timeSetting(key, TimeValue.timeValueSeconds(5), Setting.Property.NodeScope)
+        );
+
+    public static final Function<String, Setting.AffixSetting<TimeValue>> IDP_METADATA_HTTP_READ_TIMEOUT = (type) -> Setting
+        .affixKeySetting(
+            RealmSettings.realmSettingPrefix(type),
+            IDP_METADATA_SETTING_PREFIX + "http.read_timeout",
+            key -> Setting.timeSetting(key, TimeValue.timeValueSeconds(10), Setting.Property.NodeScope)
+        );
+
     public static final Function<String, Setting.AffixSetting<Boolean>> IDP_SINGLE_LOGOUT = (type) -> Setting.affixKeySetting(
         RealmSettings.realmSettingPrefix(type),
         "idp.use_single_logout",
@@ -242,6 +256,8 @@ public static Set<Setting.AffixSetting<?>> getSettings(String type) {
             IDP_METADATA_HTTP_REFRESH.apply(type),
             IDP_METADATA_HTTP_MIN_REFRESH.apply(type),
             IDP_METADATA_HTTP_FAIL_ON_ERROR.apply(type),
+            IDP_METADATA_HTTP_CONNECT_TIMEOUT.apply(type),
+            IDP_METADATA_HTTP_READ_TIMEOUT.apply(type),
             IDP_SINGLE_LOGOUT.apply(type),
             NAMEID_FORMAT.apply(type),
             NAMEID_ALLOW_CREATE.apply(type),

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlRealm.java

@@ -12,6 +12,7 @@
 import net.shibboleth.utilities.java.support.xml.BasicParserPool;
 
 import org.apache.http.client.HttpClient;
+import org.apache.http.client.config.RequestConfig;
 import org.apache.http.conn.ssl.SSLConnectionSocketFactory;
 import org.apache.http.impl.client.HttpClientBuilder;
 import org.apache.logging.log4j.LogManager;
@@ -123,8 +124,10 @@
 import static org.elasticsearch.xpack.core.security.authc.saml.SamlRealmSettings.FORCE_AUTHN;
 import static org.elasticsearch.xpack.core.security.authc.saml.SamlRealmSettings.GROUPS_ATTRIBUTE;
 import static org.elasticsearch.xpack.core.security.authc.saml.SamlRealmSettings.IDP_ENTITY_ID;
+import static org.elasticsearch.xpack.core.security.authc.saml.SamlRealmSettings.IDP_METADATA_HTTP_CONNECT_TIMEOUT;
 import static org.elasticsearch.xpack.core.security.authc.saml.SamlRealmSettings.IDP_METADATA_HTTP_FAIL_ON_ERROR;
 import static org.elasticsearch.xpack.core.security.authc.saml.SamlRealmSettings.IDP_METADATA_HTTP_MIN_REFRESH;
+import static org.elasticsearch.xpack.core.security.authc.saml.SamlRealmSettings.IDP_METADATA_HTTP_READ_TIMEOUT;
 import static org.elasticsearch.xpack.core.security.authc.saml.SamlRealmSettings.IDP_METADATA_HTTP_REFRESH;
 import static org.elasticsearch.xpack.core.security.authc.saml.SamlRealmSettings.IDP_METADATA_PATH;
 import static org.elasticsearch.xpack.core.security.authc.saml.SamlRealmSettings.IDP_SINGLE_LOGOUT;
@@ -705,6 +708,14 @@ private static Tuple<AbstractReloadingMetadataResolver, Supplier<EntityDescripto
         final SSLConnectionSocketFactory factory = sslProfile.connectionSocketFactory();
         builder.setSSLSocketFactory(factory);
 
+        TimeValue connectTimeout = config.getSetting(IDP_METADATA_HTTP_CONNECT_TIMEOUT);
+        TimeValue readTimeout = config.getSetting(IDP_METADATA_HTTP_READ_TIMEOUT);
+        RequestConfig requestConfig = RequestConfig.custom()
+            .setConnectTimeout(Math.toIntExact(connectTimeout.millis()))
+            .setSocketTimeout(Math.toIntExact(readTimeout.millis()))
+            .build();
+        builder.setDefaultRequestConfig(requestConfig);
+
         TimeValue maxRefresh = config.getSetting(IDP_METADATA_HTTP_REFRESH);
         TimeValue minRefresh = config.getSetting(IDP_METADATA_HTTP_MIN_REFRESH);
         if (minRefresh.compareTo(maxRefresh) > 0) {
@@ -799,7 +810,13 @@ private static Tuple<AbstractReloadingMetadataResolver, Supplier<EntityDescripto
         final Path path = config.env().configDir().resolve(metadataPath);
         final FilesystemMetadataResolver resolver = new SamlFilesystemMetadataResolver(path.toFile());
 
-        for (var httpSetting : List.of(IDP_METADATA_HTTP_REFRESH, IDP_METADATA_HTTP_MIN_REFRESH, IDP_METADATA_HTTP_FAIL_ON_ERROR)) {
+        for (var httpSetting : List.of(
+            IDP_METADATA_HTTP_REFRESH,
+            IDP_METADATA_HTTP_MIN_REFRESH,
+            IDP_METADATA_HTTP_FAIL_ON_ERROR,
+            IDP_METADATA_HTTP_CONNECT_TIMEOUT,
+            IDP_METADATA_HTTP_READ_TIMEOUT
+        )) {
             if (config.hasSetting(httpSetting.apply(config.type()))) {
                 logger.info(
                     "Ignoring setting [{}] because the IdP metadata is being loaded from a file",

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

@@ -1576,6 +1576,112 @@ private TestsSSLService buildTestSslService() {
         return new TestsSSLService(TestEnvironment.newEnvironment(settings));
     }
 
+    public void testHttpMetadataWithCustomTimeouts() throws Exception {
+        final Path path = getDataPath("idp1.xml");
+        final String body = Files.readString(path);
+        TestsSSLService sslService = buildTestSslService();
+        try (MockWebServer proxyServer = new MockWebServer(sslService.sslContext("xpack.security.http.ssl"), false)) {
+            proxyServer.start();
+            proxyServer.enqueue(new MockResponse().setResponseCode(200).setBody(body).addHeader("Content-Type", "application/xml"));
+
+            final TimeValue customConnectTimeout = TimeValue.timeValueSeconds(3);
+            final TimeValue customReadTimeout = TimeValue.timeValueSeconds(15);
+
+            Tuple<RealmConfig, SSLService> config = buildConfig("https://localhost:" + proxyServer.getPort(), builder -> {
+                builder.put(
+                    SingleSpSamlRealmSettings.getFullSettingKey(REALM_NAME, SamlRealmSettings.IDP_METADATA_HTTP_CONNECT_TIMEOUT),
+                    customConnectTimeout.getStringRep()
+                );
+                builder.put(
+                    SingleSpSamlRealmSettings.getFullSettingKey(REALM_NAME, SamlRealmSettings.IDP_METADATA_HTTP_READ_TIMEOUT),
+                    customReadTimeout.getStringRep()
+                );
+            });
+
+            // Verify settings are correctly configured
+            assertThat(config.v1().getSetting(SamlRealmSettings.IDP_METADATA_HTTP_CONNECT_TIMEOUT), equalTo(customConnectTimeout));
+            assertThat(config.v1().getSetting(SamlRealmSettings.IDP_METADATA_HTTP_READ_TIMEOUT), equalTo(customReadTimeout));
+
+            final ResourceWatcherService watcherService = mock(ResourceWatcherService.class);
+            Tuple<AbstractReloadingMetadataResolver, Supplier<EntityDescriptor>> tuple = SamlRealm.initializeResolver(
+                logger,
+                config.v1(),
+                config.v2(),
+                watcherService
+            );
+
+            try {
+                assertThat(proxyServer.requests().size(), greaterThanOrEqualTo(1));
+                assertIdp1MetadataParsedCorrectly(tuple.v2().get());
+            } finally {
+                tuple.v1().destroy();
+            }
+        }
+    }
+
+    public void testHttpMetadataWithDefaultTimeouts() throws Exception {
+        final Path path = getDataPath("idp1.xml");
+        final String body = Files.readString(path);
+        TestsSSLService sslService = buildTestSslService();
+        try (MockWebServer proxyServer = new MockWebServer(sslService.sslContext("xpack.security.http.ssl"), false)) {
+            proxyServer.start();
+            proxyServer.enqueue(new MockResponse().setResponseCode(200).setBody(body).addHeader("Content-Type", "application/xml"));
+
+            Tuple<RealmConfig, SSLService> config = buildConfig("https://localhost:" + proxyServer.getPort());
+
+            // Verify default timeout values are used
+            assertThat(config.v1().getSetting(SamlRealmSettings.IDP_METADATA_HTTP_CONNECT_TIMEOUT), equalTo(TimeValue.timeValueSeconds(5)));
+            assertThat(config.v1().getSetting(SamlRealmSettings.IDP_METADATA_HTTP_READ_TIMEOUT), equalTo(TimeValue.timeValueSeconds(10)));
+
+            final ResourceWatcherService watcherService = mock(ResourceWatcherService.class);
+            Tuple<AbstractReloadingMetadataResolver, Supplier<EntityDescriptor>> tuple = SamlRealm.initializeResolver(
+                logger,
+                config.v1(),
+                config.v2(),
+                watcherService
+            );
+
+            try {
+                assertThat(proxyServer.requests().size(), greaterThanOrEqualTo(1));
+                assertIdp1MetadataParsedCorrectly(tuple.v2().get());
+            } finally {
+                tuple.v1().destroy();
+            }
+        }
+    }
+
+    public void testHttpMetadataConnectionTimeout() throws Exception {
+        // Use a non-routable IP address to simulate connection timeout
+        // 192.0.2.1 is reserved for documentation and will not be routable
+        final String unreachableUrl = "https://192.0.2.1:9999/metadata.xml";
+        final TimeValue shortConnectTimeout = TimeValue.timeValueMillis(100);
+
+        Tuple<RealmConfig, SSLService> config = buildConfig(unreachableUrl, builder -> {
+            builder.put(
+                SingleSpSamlRealmSettings.getFullSettingKey(REALM_NAME, SamlRealmSettings.IDP_METADATA_HTTP_CONNECT_TIMEOUT),
+                shortConnectTimeout.getStringRep()
+            );
+            builder.put(SingleSpSamlRealmSettings.getFullSettingKey(REALM_NAME, SamlRealmSettings.IDP_METADATA_HTTP_FAIL_ON_ERROR), false);
+        });
+
+        final ResourceWatcherService watcherService = mock(ResourceWatcherService.class);
+
+        // initialization should complete even though the connection fails
+        Tuple<AbstractReloadingMetadataResolver, Supplier<EntityDescriptor>> tuple = SamlRealm.initializeResolver(
+            logger,
+            config.v1(),
+            config.v2(),
+            watcherService
+        );
+
+        try {
+            EntityDescriptor descriptor = tuple.v2().get();
+            assertThat(descriptor, instanceOf(UnresolvedEntity.class));
+        } finally {
+            tuple.v1().destroy();
+        }
+    }
+
     private void assertIdp1MetadataParsedCorrectly(EntityDescriptor descriptor) {
         try {
             IDPSSODescriptor idpssoDescriptor = descriptor.getIDPSSODescriptor(SAMLConstants.SAML20P_NS);