Skip to content

Commit 5057b47

Browse files
ankit--sethielasticsearchmachine
ankit--sethi
and
elasticsearchmachine
authored
Configurable HTTP read and connect timeouts for url based SAML metadata resolution (#136058) (#136830)
* Fix + tests for #133542 * [CI] Auto commit changes from spotless * Update docs/changelog/136058.yaml * update summary --------- Co-authored-by: elasticsearchmachine <[email protected]>
1 parent 9f99de4 commit 5057b47

File tree

4 files changed

+145
-1
lines changed

4 files changed

+145
-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: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,20 @@ public class SamlRealmSettings {
6262
key -> Setting.boolSetting(key, false, Setting.Property.NodeScope)
6363
);
6464

65+
public static final Function<String, Setting.AffixSetting<TimeValue>> IDP_METADATA_HTTP_CONNECT_TIMEOUT = (type) -> Setting
66+
.affixKeySetting(
67+
RealmSettings.realmSettingPrefix(type),
68+
IDP_METADATA_SETTING_PREFIX + "http.connect_timeout",
69+
key -> Setting.timeSetting(key, TimeValue.timeValueSeconds(5), Setting.Property.NodeScope)
70+
);
71+
72+
public static final Function<String, Setting.AffixSetting<TimeValue>> IDP_METADATA_HTTP_READ_TIMEOUT = (type) -> Setting
73+
.affixKeySetting(
74+
RealmSettings.realmSettingPrefix(type),
75+
IDP_METADATA_SETTING_PREFIX + "http.read_timeout",
76+
key -> Setting.timeSetting(key, TimeValue.timeValueSeconds(10), Setting.Property.NodeScope)
77+
);
78+
6579
public static final Function<String, Setting.AffixSetting<Boolean>> IDP_SINGLE_LOGOUT = (type) -> Setting.affixKeySetting(
6680
RealmSettings.realmSettingPrefix(type),
6781
"idp.use_single_logout",
@@ -191,6 +205,8 @@ public static Set<Setting.AffixSetting<?>> getSettings(String type) {
191205
IDP_METADATA_HTTP_REFRESH.apply(type),
192206
IDP_METADATA_HTTP_MIN_REFRESH.apply(type),
193207
IDP_METADATA_HTTP_FAIL_ON_ERROR.apply(type),
208+
IDP_METADATA_HTTP_CONNECT_TIMEOUT.apply(type),
209+
IDP_METADATA_HTTP_READ_TIMEOUT.apply(type),
194210
IDP_SINGLE_LOGOUT.apply(type),
195211
NAMEID_FORMAT.apply(type),
196212
NAMEID_ALLOW_CREATE.apply(type),

‎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;
@@ -689,6 +692,14 @@ private static Tuple<AbstractReloadingMetadataResolver, Supplier<EntityDescripto
689692
SSLConnectionSocketFactory factory = new SSLConnectionSocketFactory(sslService.sslSocketFactory(sslConfiguration), verifier);
690693
builder.setSSLSocketFactory(factory);
691694

695+
TimeValue connectTimeout = config.getSetting(IDP_METADATA_HTTP_CONNECT_TIMEOUT);
696+
TimeValue readTimeout = config.getSetting(IDP_METADATA_HTTP_READ_TIMEOUT);
697+
RequestConfig requestConfig = RequestConfig.custom()
698+
.setConnectTimeout(Math.toIntExact(connectTimeout.millis()))
699+
.setSocketTimeout(Math.toIntExact(readTimeout.millis()))
700+
.build();
701+
builder.setDefaultRequestConfig(requestConfig);
702+
692703
TimeValue maxRefresh = config.getSetting(IDP_METADATA_HTTP_REFRESH);
693704
TimeValue minRefresh = config.getSetting(IDP_METADATA_HTTP_MIN_REFRESH);
694705
if (minRefresh.compareTo(maxRefresh) > 0) {
@@ -783,7 +794,13 @@ private static Tuple<AbstractReloadingMetadataResolver, Supplier<EntityDescripto
783794
final Path path = config.env().configDir().resolve(metadataPath);
784795
final FilesystemMetadataResolver resolver = new SamlFilesystemMetadataResolver(path.toFile());
785796

786-
for (var httpSetting : List.of(IDP_METADATA_HTTP_REFRESH, IDP_METADATA_HTTP_MIN_REFRESH, IDP_METADATA_HTTP_FAIL_ON_ERROR)) {
797+
for (var httpSetting : List.of(
798+
IDP_METADATA_HTTP_REFRESH,
799+
IDP_METADATA_HTTP_MIN_REFRESH,
800+
IDP_METADATA_HTTP_FAIL_ON_ERROR,
801+
IDP_METADATA_HTTP_CONNECT_TIMEOUT,
802+
IDP_METADATA_HTTP_READ_TIMEOUT
803+
)) {
787804
if (config.hasSetting(httpSetting.apply(config.type()))) {
788805
logger.info(
789806
"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
@@ -1476,6 +1476,112 @@ private TestsSSLService buildTestSslService() {
14761476
return new TestsSSLService(TestEnvironment.newEnvironment(settings));
14771477
}
14781478

1479+
public void testHttpMetadataWithCustomTimeouts() throws Exception {
1480+
final Path path = getDataPath("idp1.xml");
1481+
final String body = Files.readString(path);
1482+
TestsSSLService sslService = buildTestSslService();
1483+
try (MockWebServer proxyServer = new MockWebServer(sslService.sslContext("xpack.security.http.ssl"), false)) {
1484+
proxyServer.start();
1485+
proxyServer.enqueue(new MockResponse().setResponseCode(200).setBody(body).addHeader("Content-Type", "application/xml"));
1486+
1487+
final TimeValue customConnectTimeout = TimeValue.timeValueSeconds(3);
1488+
final TimeValue customReadTimeout = TimeValue.timeValueSeconds(15);
1489+
1490+
Tuple<RealmConfig, SSLService> config = buildConfig("https://localhost:" + proxyServer.getPort(), builder -> {
1491+
builder.put(
1492+
SingleSpSamlRealmSettings.getFullSettingKey(REALM_NAME, SamlRealmSettings.IDP_METADATA_HTTP_CONNECT_TIMEOUT),
1493+
customConnectTimeout.getStringRep()
1494+
);
1495+
builder.put(
1496+
SingleSpSamlRealmSettings.getFullSettingKey(REALM_NAME, SamlRealmSettings.IDP_METADATA_HTTP_READ_TIMEOUT),
1497+
customReadTimeout.getStringRep()
1498+
);
1499+
});
1500+
1501+
// Verify settings are correctly configured
1502+
assertThat(config.v1().getSetting(SamlRealmSettings.IDP_METADATA_HTTP_CONNECT_TIMEOUT), equalTo(customConnectTimeout));
1503+
assertThat(config.v1().getSetting(SamlRealmSettings.IDP_METADATA_HTTP_READ_TIMEOUT), equalTo(customReadTimeout));
1504+
1505+
final ResourceWatcherService watcherService = mock(ResourceWatcherService.class);
1506+
Tuple<AbstractReloadingMetadataResolver, Supplier<EntityDescriptor>> tuple = SamlRealm.initializeResolver(
1507+
logger,
1508+
config.v1(),
1509+
config.v2(),
1510+
watcherService
1511+
);
1512+
1513+
try {
1514+
assertThat(proxyServer.requests().size(), greaterThanOrEqualTo(1));
1515+
assertIdp1MetadataParsedCorrectly(tuple.v2().get());
1516+
} finally {
1517+
tuple.v1().destroy();
1518+
}
1519+
}
1520+
}
1521+
1522+
public void testHttpMetadataWithDefaultTimeouts() throws Exception {
1523+
final Path path = getDataPath("idp1.xml");
1524+
final String body = Files.readString(path);
1525+
TestsSSLService sslService = buildTestSslService();
1526+
try (MockWebServer proxyServer = new MockWebServer(sslService.sslContext("xpack.security.http.ssl"), false)) {
1527+
proxyServer.start();
1528+
proxyServer.enqueue(new MockResponse().setResponseCode(200).setBody(body).addHeader("Content-Type", "application/xml"));
1529+
1530+
Tuple<RealmConfig, SSLService> config = buildConfig("https://localhost:" + proxyServer.getPort());
1531+
1532+
// Verify default timeout values are used
1533+
assertThat(config.v1().getSetting(SamlRealmSettings.IDP_METADATA_HTTP_CONNECT_TIMEOUT), equalTo(TimeValue.timeValueSeconds(5)));
1534+
assertThat(config.v1().getSetting(SamlRealmSettings.IDP_METADATA_HTTP_READ_TIMEOUT), equalTo(TimeValue.timeValueSeconds(10)));
1535+
1536+
final ResourceWatcherService watcherService = mock(ResourceWatcherService.class);
1537+
Tuple<AbstractReloadingMetadataResolver, Supplier<EntityDescriptor>> tuple = SamlRealm.initializeResolver(
1538+
logger,
1539+
config.v1(),
1540+
config.v2(),
1541+
watcherService
1542+
);
1543+
1544+
try {
1545+
assertThat(proxyServer.requests().size(), greaterThanOrEqualTo(1));
1546+
assertIdp1MetadataParsedCorrectly(tuple.v2().get());
1547+
} finally {
1548+
tuple.v1().destroy();
1549+
}
1550+
}
1551+
}
1552+
1553+
public void testHttpMetadataConnectionTimeout() throws Exception {
1554+
// Use a non-routable IP address to simulate connection timeout
1555+
// 192.0.2.1 is reserved for documentation and will not be routable
1556+
final String unreachableUrl = "https://192.0.2.1:9999/metadata.xml";
1557+
final TimeValue shortConnectTimeout = TimeValue.timeValueMillis(100);
1558+
1559+
Tuple<RealmConfig, SSLService> config = buildConfig(unreachableUrl, builder -> {
1560+
builder.put(
1561+
SingleSpSamlRealmSettings.getFullSettingKey(REALM_NAME, SamlRealmSettings.IDP_METADATA_HTTP_CONNECT_TIMEOUT),
1562+
shortConnectTimeout.getStringRep()
1563+
);
1564+
builder.put(SingleSpSamlRealmSettings.getFullSettingKey(REALM_NAME, SamlRealmSettings.IDP_METADATA_HTTP_FAIL_ON_ERROR), false);
1565+
});
1566+
1567+
final ResourceWatcherService watcherService = mock(ResourceWatcherService.class);
1568+
1569+
// initialization should complete even though the connection fails
1570+
Tuple<AbstractReloadingMetadataResolver, Supplier<EntityDescriptor>> tuple = SamlRealm.initializeResolver(
1571+
logger,
1572+
config.v1(),
1573+
config.v2(),
1574+
watcherService
1575+
);
1576+
1577+
try {
1578+
EntityDescriptor descriptor = tuple.v2().get();
1579+
assertThat(descriptor, instanceOf(UnresolvedEntity.class));
1580+
} finally {
1581+
tuple.v1().destroy();
1582+
}
1583+
}
1584+
14791585
private void assertIdp1MetadataParsedCorrectly(EntityDescriptor descriptor) {
14801586
try {
14811587
IDPSSODescriptor idpssoDescriptor = descriptor.getIDPSSODescriptor(SAMLConstants.SAML20P_NS);

0 commit comments

Comments
 (0)