Skip to content

Commit 2a2c4ca

Browse files
ankit--sethiankit--sethi
committed
Configurable HTTP read and connect timeouts for url based SAML metadata resolution (#136058)
* Fix + tests for #133542 * [CI] Auto commit changes from spotless * Update docs/changelog/136058.yaml * update summary --------- Co-authored-by: elasticsearchmachine <[email protected]> (cherry picked from commit ddf9b68)
1 parent 71d54f4 commit 2a2c4ca

File tree

4 files changed

+146
-1
lines changed

4 files changed

+146
-1
lines changed

‎docs/changelog/136058.yaml‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
pr: 136058
2+
summary: "Configurable HTTP read and connect timeouts for url based SAML metadata resolution"
3+
area: Security
4+
type: bug
5+
issues: []

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

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import java.util.List;
2424
import java.util.Map;
2525
import java.util.Set;
26+
import java.util.function.Function;
2627

2728
import static org.elasticsearch.xpack.core.security.authc.support.SecuritySettingsUtil.verifyNonNullNotEmpty;
2829

@@ -63,6 +64,20 @@ public class SamlRealmSettings {
6364
key -> Setting.boolSetting(key, false, Setting.Property.NodeScope)
6465
);
6566

67+
public static final Setting.AffixSetting<TimeValue> IDP_METADATA_HTTP_CONNECT_TIMEOUT = Setting
68+
.affixKeySetting(
69+
RealmSettings.realmSettingPrefix(TYPE),
70+
IDP_METADATA_SETTING_PREFIX + "http.connect_timeout",
71+
key -> Setting.timeSetting(key, TimeValue.timeValueSeconds(5), Setting.Property.NodeScope)
72+
);
73+
74+
public static final Setting.AffixSetting<TimeValue> IDP_METADATA_HTTP_READ_TIMEOUT = Setting
75+
.affixKeySetting(
76+
RealmSettings.realmSettingPrefix(TYPE),
77+
IDP_METADATA_SETTING_PREFIX + "http.read_timeout",
78+
key -> Setting.timeSetting(key, TimeValue.timeValueSeconds(10), Setting.Property.NodeScope)
79+
);
80+
6681
public static final Setting.AffixSetting<Boolean> IDP_SINGLE_LOGOUT = Setting.affixKeySetting(
6782
RealmSettings.realmSettingPrefix(TYPE),
6883
"idp.use_single_logout",
@@ -200,6 +215,8 @@ public static Set<Setting.AffixSetting<?>> getSettings() {
200215
IDP_METADATA_HTTP_REFRESH,
201216
IDP_METADATA_HTTP_MIN_REFRESH,
202217
IDP_METADATA_HTTP_FAIL_ON_ERROR,
218+
IDP_METADATA_HTTP_CONNECT_TIMEOUT,
219+
IDP_METADATA_HTTP_READ_TIMEOUT,
203220
IDP_SINGLE_LOGOUT,
204221
SP_ENTITY_ID,
205222
SP_ACS,

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

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import net.shibboleth.utilities.java.support.xml.BasicParserPool;
1313

1414
import org.apache.http.client.HttpClient;
15+
import org.apache.http.client.config.RequestConfig;
1516
import org.apache.http.conn.ssl.SSLConnectionSocketFactory;
1617
import org.apache.http.impl.client.HttpClientBuilder;
1718
import org.apache.logging.log4j.LogManager;
@@ -120,8 +121,10 @@
120121
import static org.elasticsearch.xpack.core.security.authc.saml.SamlRealmSettings.FORCE_AUTHN;
121122
import static org.elasticsearch.xpack.core.security.authc.saml.SamlRealmSettings.GROUPS_ATTRIBUTE;
122123
import static org.elasticsearch.xpack.core.security.authc.saml.SamlRealmSettings.IDP_ENTITY_ID;
124+
import static org.elasticsearch.xpack.core.security.authc.saml.SamlRealmSettings.IDP_METADATA_HTTP_CONNECT_TIMEOUT;
123125
import static org.elasticsearch.xpack.core.security.authc.saml.SamlRealmSettings.IDP_METADATA_HTTP_FAIL_ON_ERROR;
124126
import static org.elasticsearch.xpack.core.security.authc.saml.SamlRealmSettings.IDP_METADATA_HTTP_MIN_REFRESH;
127+
import static org.elasticsearch.xpack.core.security.authc.saml.SamlRealmSettings.IDP_METADATA_HTTP_READ_TIMEOUT;
125128
import static org.elasticsearch.xpack.core.security.authc.saml.SamlRealmSettings.IDP_METADATA_HTTP_REFRESH;
126129
import static org.elasticsearch.xpack.core.security.authc.saml.SamlRealmSettings.IDP_METADATA_PATH;
127130
import static org.elasticsearch.xpack.core.security.authc.saml.SamlRealmSettings.IDP_SINGLE_LOGOUT;
@@ -701,6 +704,14 @@ private static Tuple<AbstractReloadingMetadataResolver, Supplier<EntityDescripto
701704
SSLConnectionSocketFactory factory = new SSLConnectionSocketFactory(sslService.sslSocketFactory(sslConfiguration), verifier);
702705
builder.setSSLSocketFactory(factory);
703706

707+
TimeValue connectTimeout = config.getSetting(IDP_METADATA_HTTP_CONNECT_TIMEOUT);
708+
TimeValue readTimeout = config.getSetting(IDP_METADATA_HTTP_READ_TIMEOUT);
709+
RequestConfig requestConfig = RequestConfig.custom()
710+
.setConnectTimeout(Math.toIntExact(connectTimeout.millis()))
711+
.setSocketTimeout(Math.toIntExact(readTimeout.millis()))
712+
.build();
713+
builder.setDefaultRequestConfig(requestConfig);
714+
704715
TimeValue maxRefresh = config.getSetting(IDP_METADATA_HTTP_REFRESH);
705716
TimeValue minRefresh = config.getSetting(IDP_METADATA_HTTP_MIN_REFRESH);
706717
if (minRefresh.compareTo(maxRefresh) > 0) {
@@ -795,7 +806,13 @@ private static Tuple<AbstractReloadingMetadataResolver, Supplier<EntityDescripto
795806
final Path path = config.env().configDir().resolve(metadataPath);
796807
final FilesystemMetadataResolver resolver = new SamlFilesystemMetadataResolver(path.toFile());
797808

798-
for (var httpSetting : List.of(IDP_METADATA_HTTP_REFRESH, IDP_METADATA_HTTP_MIN_REFRESH, IDP_METADATA_HTTP_FAIL_ON_ERROR)) {
809+
for (var httpSetting : List.of(
810+
IDP_METADATA_HTTP_REFRESH,
811+
IDP_METADATA_HTTP_MIN_REFRESH,
812+
IDP_METADATA_HTTP_FAIL_ON_ERROR,
813+
IDP_METADATA_HTTP_CONNECT_TIMEOUT,
814+
IDP_METADATA_HTTP_READ_TIMEOUT
815+
)) {
799816
if (config.hasSetting(httpSetting)) {
800817
logger.info(
801818
"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‎

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1373,6 +1373,112 @@ private TestsSSLService buildTestSslService() {
13731373
return new TestsSSLService(TestEnvironment.newEnvironment(settings));
13741374
}
13751375

1376+
public void testHttpMetadataWithCustomTimeouts() throws Exception {
1377+
final Path path = getDataPath("idp1.xml");
1378+
final String body = Files.readString(path);
1379+
TestsSSLService sslService = buildTestSslService();
1380+
try (MockWebServer proxyServer = new MockWebServer(sslService.sslContext("xpack.security.http.ssl"), false)) {
1381+
proxyServer.start();
1382+
proxyServer.enqueue(new MockResponse().setResponseCode(200).setBody(body).addHeader("Content-Type", "application/xml"));
1383+
1384+
final TimeValue customConnectTimeout = TimeValue.timeValueSeconds(3);
1385+
final TimeValue customReadTimeout = TimeValue.timeValueSeconds(15);
1386+
1387+
Tuple<RealmConfig, SSLService> config = buildConfig("https://localhost:" + proxyServer.getPort(), builder -> {
1388+
builder.put(
1389+
SingleSpSamlRealmSettings.getFullSettingKey(REALM_NAME, SamlRealmSettings.IDP_METADATA_HTTP_CONNECT_TIMEOUT),
1390+
customConnectTimeout.getStringRep()
1391+
);
1392+
builder.put(
1393+
SingleSpSamlRealmSettings.getFullSettingKey(REALM_NAME, SamlRealmSettings.IDP_METADATA_HTTP_READ_TIMEOUT),
1394+
customReadTimeout.getStringRep()
1395+
);
1396+
});
1397+
1398+
// Verify settings are correctly configured
1399+
assertThat(config.v1().getSetting(SamlRealmSettings.IDP_METADATA_HTTP_CONNECT_TIMEOUT), equalTo(customConnectTimeout));
1400+
assertThat(config.v1().getSetting(SamlRealmSettings.IDP_METADATA_HTTP_READ_TIMEOUT), equalTo(customReadTimeout));
1401+
1402+
final ResourceWatcherService watcherService = mock(ResourceWatcherService.class);
1403+
Tuple<AbstractReloadingMetadataResolver, Supplier<EntityDescriptor>> tuple = SamlRealm.initializeResolver(
1404+
logger,
1405+
config.v1(),
1406+
config.v2(),
1407+
watcherService
1408+
);
1409+
1410+
try {
1411+
assertThat(proxyServer.requests().size(), greaterThanOrEqualTo(1));
1412+
assertIdp1MetadataParsedCorrectly(tuple.v2().get());
1413+
} finally {
1414+
tuple.v1().destroy();
1415+
}
1416+
}
1417+
}
1418+
1419+
public void testHttpMetadataWithDefaultTimeouts() throws Exception {
1420+
final Path path = getDataPath("idp1.xml");
1421+
final String body = Files.readString(path);
1422+
TestsSSLService sslService = buildTestSslService();
1423+
try (MockWebServer proxyServer = new MockWebServer(sslService.sslContext("xpack.security.http.ssl"), false)) {
1424+
proxyServer.start();
1425+
proxyServer.enqueue(new MockResponse().setResponseCode(200).setBody(body).addHeader("Content-Type", "application/xml"));
1426+
1427+
Tuple<RealmConfig, SSLService> config = buildConfig("https://localhost:" + proxyServer.getPort());
1428+
1429+
// Verify default timeout values are used
1430+
assertThat(config.v1().getSetting(SamlRealmSettings.IDP_METADATA_HTTP_CONNECT_TIMEOUT), equalTo(TimeValue.timeValueSeconds(5)));
1431+
assertThat(config.v1().getSetting(SamlRealmSettings.IDP_METADATA_HTTP_READ_TIMEOUT), equalTo(TimeValue.timeValueSeconds(10)));
1432+
1433+
final ResourceWatcherService watcherService = mock(ResourceWatcherService.class);
1434+
Tuple<AbstractReloadingMetadataResolver, Supplier<EntityDescriptor>> tuple = SamlRealm.initializeResolver(
1435+
logger,
1436+
config.v1(),
1437+
config.v2(),
1438+
watcherService
1439+
);
1440+
1441+
try {
1442+
assertThat(proxyServer.requests().size(), greaterThanOrEqualTo(1));
1443+
assertIdp1MetadataParsedCorrectly(tuple.v2().get());
1444+
} finally {
1445+
tuple.v1().destroy();
1446+
}
1447+
}
1448+
}
1449+
1450+
public void testHttpMetadataConnectionTimeout() throws Exception {
1451+
// Use a non-routable IP address to simulate connection timeout
1452+
// 192.0.2.1 is reserved for documentation and will not be routable
1453+
final String unreachableUrl = "https://192.0.2.1:9999/metadata.xml";
1454+
final TimeValue shortConnectTimeout = TimeValue.timeValueMillis(100);
1455+
1456+
Tuple<RealmConfig, SSLService> config = buildConfig(unreachableUrl, builder -> {
1457+
builder.put(
1458+
SingleSpSamlRealmSettings.getFullSettingKey(REALM_NAME, SamlRealmSettings.IDP_METADATA_HTTP_CONNECT_TIMEOUT),
1459+
shortConnectTimeout.getStringRep()
1460+
);
1461+
builder.put(SingleSpSamlRealmSettings.getFullSettingKey(REALM_NAME, SamlRealmSettings.IDP_METADATA_HTTP_FAIL_ON_ERROR), false);
1462+
});
1463+
1464+
final ResourceWatcherService watcherService = mock(ResourceWatcherService.class);
1465+
1466+
// initialization should complete even though the connection fails
1467+
Tuple<AbstractReloadingMetadataResolver, Supplier<EntityDescriptor>> tuple = SamlRealm.initializeResolver(
1468+
logger,
1469+
config.v1(),
1470+
config.v2(),
1471+
watcherService
1472+
);
1473+
1474+
try {
1475+
EntityDescriptor descriptor = tuple.v2().get();
1476+
assertThat(descriptor, instanceOf(UnresolvedEntity.class));
1477+
} finally {
1478+
tuple.v1().destroy();
1479+
}
1480+
}
1481+
13761482
private void assertIdp1MetadataParsedCorrectly(EntityDescriptor descriptor) {
13771483
try {
13781484
IDPSSODescriptor idpssoDescriptor = descriptor.getIDPSSODescriptor(SAMLConstants.SAML20P_NS);

0 commit comments

Comments
 (0)