Skip to content

Commit

Permalink
New setting to close idle connections in OIDC back-channel (#87773) (#…
Browse files Browse the repository at this point in the history
…88363)

* New setting to close idle connections in OIDC back-channel (#87773)

In some environment, the back-channel connection can be dropped
without sending a TCP RST to ES. When that happens, reusing the same
connection results into timeout error.

This PR adds a new http.connection_pool_ttl setting to control how long
a connection in the OIDC back-channel pool can be idle before it is
closed. This allows ES to more actively close idle connections to avoid
the timeout issue.

NOTE: This is a "safe" backport of #87773. The key difference here is
that the new setting is by default not configured, which means the PR
introduces *zero* behaviour change by default. Users need to actively
configure the new setting to enable the new behaviour ("automatically
closing idle connections).

Backport: #87773

* Make it safe by keeping default behaviour

* tweak

* Update x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectAuthenticatorTests.java

Co-authored-by: Tim Vernum <tim@adjective.org>

* address feedback

Co-authored-by: Tim Vernum <tim@adjective.org>
  • Loading branch information
ywangd and tvernum committed Jul 8, 2022
1 parent 5a572d8 commit dc31d03
Show file tree
Hide file tree
Showing 5 changed files with 294 additions and 0 deletions.
5 changes: 5 additions & 0 deletions docs/changelog/87773.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
pr: 87773
summary: New setting to close idle connections in OIDC back-channel
area: Security
type: enhancement
issues: []
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,12 @@ private OpenIdConnectRealmSettings() {}
"http.max_endpoint_connections",
key -> Setting.intSetting(key, 200, Setting.Property.NodeScope)
);

public static final Setting.AffixSetting<TimeValue> HTTP_CONNECTION_POOL_TTL = Setting.affixKeySetting(
RealmSettings.realmSettingPrefix(TYPE),
"http.connection_pool_ttl",
key -> Setting.timeSetting(key, TimeValue.MINUS_ONE, Setting.Property.NodeScope)
);
public static final Setting.AffixSetting<String> HTTP_PROXY_HOST = Setting.affixKeySetting(
RealmSettings.realmSettingPrefix(TYPE),
"http.proxy.host",
Expand Down Expand Up @@ -307,6 +313,7 @@ public static Set<Setting.AffixSetting<?>> getSettings() {
HTTP_SOCKET_TIMEOUT,
HTTP_MAX_CONNECTIONS,
HTTP_MAX_ENDPOINT_CONNECTIONS,
HTTP_CONNECTION_POOL_TTL,
HTTP_PROXY_HOST,
HTTP_PROXY_PORT,
HTTP_PROXY_SCHEME,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ testClusters.matching { it.name == 'javaRestTest' }.configureEach {
setting 'xpack.security.authc.realms.oidc.openid7.op.authorization_endpoint', 'https://op.example.com/auth'
setting 'xpack.security.authc.realms.oidc.openid7.op.jwkset_path', 'oidc-jwkset.json'
setting 'xpack.security.authc.realms.oidc.openid7.claims.principal', 'sub'
setting 'xpack.security.authc.realms.oidc.openid7.http.connection_pool_ttl', '1m'
keystore 'xpack.security.authc.realms.oidc.openid7.rp.client_secret', 'this-is-my-secret'
// - JWT (works)
setting 'xpack.security.authc.realms.jwt.jwt8.order', '8'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,10 @@
import org.apache.http.concurrent.FutureCallback;
import org.apache.http.config.Registry;
import org.apache.http.config.RegistryBuilder;
import org.apache.http.conn.ConnectionKeepAliveStrategy;
import org.apache.http.entity.ContentType;
import org.apache.http.impl.auth.BasicScheme;
import org.apache.http.impl.client.DefaultConnectionKeepAliveStrategy;
import org.apache.http.impl.nio.client.CloseableHttpAsyncClient;
import org.apache.http.impl.nio.client.HttpAsyncClientBuilder;
import org.apache.http.impl.nio.client.HttpAsyncClients;
Expand Down Expand Up @@ -114,6 +116,7 @@
import javax.net.ssl.SSLContext;

import static org.elasticsearch.xpack.core.security.authc.oidc.OpenIdConnectRealmSettings.ALLOWED_CLOCK_SKEW;
import static org.elasticsearch.xpack.core.security.authc.oidc.OpenIdConnectRealmSettings.HTTP_CONNECTION_POOL_TTL;
import static org.elasticsearch.xpack.core.security.authc.oidc.OpenIdConnectRealmSettings.HTTP_CONNECTION_READ_TIMEOUT;
import static org.elasticsearch.xpack.core.security.authc.oidc.OpenIdConnectRealmSettings.HTTP_CONNECT_TIMEOUT;
import static org.elasticsearch.xpack.core.security.authc.oidc.OpenIdConnectRealmSettings.HTTP_MAX_CONNECTIONS;
Expand Down Expand Up @@ -700,9 +703,17 @@ private CloseableHttpAsyncClient createHttpClient() {
.setConnectionRequestTimeout(Math.toIntExact(realmConfig.getSetting(HTTP_CONNECTION_READ_TIMEOUT).getSeconds()))
.setSocketTimeout(Math.toIntExact(realmConfig.getSetting(HTTP_SOCKET_TIMEOUT).getMillis()))
.build();

HttpAsyncClientBuilder httpAsyncClientBuilder = HttpAsyncClients.custom()
.setConnectionManager(connectionManager)
.setDefaultRequestConfig(requestConfig);

final ConnectionKeepAliveStrategy keepAliveStrategy = getKeepAliveStrategy();
if (keepAliveStrategy != null) {
LOGGER.debug("configuring keep-alive strategy for http client used by oidc back-channel");
httpAsyncClientBuilder.setKeepAliveStrategy(keepAliveStrategy);
}

if (realmConfig.hasSetting(HTTP_PROXY_HOST)) {
httpAsyncClientBuilder.setProxy(
new HttpHost(
Expand All @@ -721,6 +732,37 @@ private CloseableHttpAsyncClient createHttpClient() {
}
}

// Package private for testing
CloseableHttpAsyncClient getHttpClient() {
return httpClient;
}

// Package private for testing
ConnectionKeepAliveStrategy getKeepAliveStrategy() {
// Not customising keep-alive strategy if the setting is either not set (default) or explicitly configured to be negative (-1)
if (false == realmConfig.hasSetting(HTTP_CONNECTION_POOL_TTL) || realmConfig.getSetting(HTTP_CONNECTION_POOL_TTL).duration() < 0) {
return null;
}

final long userConfiguredKeepAlive = realmConfig.getSetting(HTTP_CONNECTION_POOL_TTL).millis();
return (response, context) -> {
var serverKeepAlive = DefaultConnectionKeepAliveStrategy.INSTANCE.getKeepAliveDuration(response, context);
long actualKeepAlive;
if (serverKeepAlive <= -1) {
actualKeepAlive = userConfiguredKeepAlive;
} else if (userConfiguredKeepAlive <= -1) {
actualKeepAlive = serverKeepAlive;
} else {
actualKeepAlive = Math.min(serverKeepAlive, userConfiguredKeepAlive);
}
if (actualKeepAlive < -1) {
actualKeepAlive = -1;
}
LOGGER.debug("effective HTTP connection keep-alive: [{}]ms", actualKeepAlive);
return actualKeepAlive;
};
}

/*
* Creates an {@link IDTokenValidator} based on the current Relying Party configuration
*/
Expand Down

0 comments on commit dc31d03

Please sign in to comment.