Skip to content

Commit 547a3cf

Browse files
authored
Fix to release system resource after reading JKWSet file (elastic#48666) (elastic#48678)
When we load a JSON Web Key (JWKSet) from the specified file using JWKSet.load it internally uses IOUtils.readFileToString but the opened FileInputStream is never closed after usage. https://bitbucket.org/connect2id/nimbus-jose-jwt/issues/342 This commit reads the file and parses the JWKSet from the string. This also fixes an issue wherein if the underlying file changed, for every change event it would add another file watcher. The change is to only add the file watcher at the start. Closes elastic#44942
1 parent a7a45ba commit 547a3cf

File tree

2 files changed

+14
-12
lines changed

2 files changed

+14
-12
lines changed

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectAuthenticator.java

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,6 @@
7676
import org.elasticsearch.common.CheckedRunnable;
7777
import org.elasticsearch.common.Nullable;
7878
import org.elasticsearch.common.Strings;
79-
import org.elasticsearch.common.SuppressForbidden;
8079
import org.elasticsearch.common.collect.Tuple;
8180
import org.elasticsearch.common.util.concurrent.EsExecutors;
8281
import org.elasticsearch.common.util.concurrent.ListenableFuture;
@@ -95,10 +94,10 @@
9594
import java.net.URI;
9695
import java.net.URISyntaxException;
9796
import java.net.URL;
98-
9997
import java.net.URLEncoder;
10098
import java.nio.charset.Charset;
10199
import java.nio.charset.StandardCharsets;
100+
import java.nio.file.Files;
102101
import java.nio.file.Path;
103102
import java.security.AccessController;
104103
import java.security.PrivilegedAction;
@@ -111,8 +110,8 @@
111110
import java.util.concurrent.atomic.AtomicReference;
112111

113112
import static org.elasticsearch.xpack.core.security.authc.oidc.OpenIdConnectRealmSettings.ALLOWED_CLOCK_SKEW;
114-
import static org.elasticsearch.xpack.core.security.authc.oidc.OpenIdConnectRealmSettings.HTTP_CONNECT_TIMEOUT;
115113
import static org.elasticsearch.xpack.core.security.authc.oidc.OpenIdConnectRealmSettings.HTTP_CONNECTION_READ_TIMEOUT;
114+
import static org.elasticsearch.xpack.core.security.authc.oidc.OpenIdConnectRealmSettings.HTTP_CONNECT_TIMEOUT;
116115
import static org.elasticsearch.xpack.core.security.authc.oidc.OpenIdConnectRealmSettings.HTTP_MAX_CONNECTIONS;
117116
import static org.elasticsearch.xpack.core.security.authc.oidc.OpenIdConnectRealmSettings.HTTP_MAX_ENDPOINT_CONNECTIONS;
118117
import static org.elasticsearch.xpack.core.security.authc.oidc.OpenIdConnectRealmSettings.HTTP_SOCKET_TIMEOUT;
@@ -142,7 +141,7 @@ public OpenIdConnectAuthenticator(RealmConfig realmConfig, OpenIdConnectProvider
142141
this.sslService = sslService;
143142
this.httpClient = createHttpClient();
144143
this.watcherService = watcherService;
145-
this.idTokenValidator.set(createIdTokenValidator());
144+
this.idTokenValidator.set(createIdTokenValidator(true));
146145
}
147146

148147
// For testing
@@ -314,10 +313,11 @@ private void validateAccessToken(AccessToken accessToken, JWT idToken) {
314313
* @throws ParseException if the file cannot be parsed
315314
* @throws IOException if the file cannot be read
316315
*/
317-
@SuppressForbidden(reason = "uses toFile")
318316
private JWKSet readJwkSetFromFile(String jwkSetPath) throws IOException, ParseException {
319317
final Path path = realmConfig.env().configFile().resolve(jwkSetPath);
320-
return JWKSet.load(path.toFile());
318+
// avoid using JWKSet.loadFile() as it does not close FileInputStream internally
319+
String jwkSet = new String(Files.readAllBytes(path), StandardCharsets.UTF_8);
320+
return JWKSet.parse(jwkSet);
321321
}
322322

323323
/**
@@ -589,7 +589,7 @@ private CloseableHttpAsyncClient createHttpClient() {
589589
/*
590590
* Creates an {@link IDTokenValidator} based on the current Relying Party configuration
591591
*/
592-
IDTokenValidator createIdTokenValidator() {
592+
IDTokenValidator createIdTokenValidator(boolean addFileWatcherIfRequired) {
593593
try {
594594
final JWSAlgorithm requestedAlgorithm = rpConfig.getSignatureAlgorithm();
595595
final int allowedClockSkew = Math.toIntExact(realmConfig.getSetting(ALLOWED_CLOCK_SKEW).getMillis());
@@ -600,12 +600,16 @@ IDTokenValidator createIdTokenValidator() {
600600
new IDTokenValidator(opConfig.getIssuer(), rpConfig.getClientId(), requestedAlgorithm, clientSecret);
601601
} else {
602602
String jwkSetPath = opConfig.getJwkSetPath();
603-
if (jwkSetPath.startsWith("https://")) {
603+
if (jwkSetPath.startsWith("http://")) {
604+
throw new IllegalArgumentException("The [http] protocol is not supported as it is insecure. Use [https] instead");
605+
} else if (jwkSetPath.startsWith("https://")) {
604606
final JWSVerificationKeySelector keySelector = new JWSVerificationKeySelector(requestedAlgorithm,
605607
new ReloadableJWKSource(new URL(jwkSetPath)));
606608
idTokenValidator = new IDTokenValidator(opConfig.getIssuer(), rpConfig.getClientId(), keySelector, null);
607609
} else {
608-
setMetadataFileWatcher(jwkSetPath);
610+
if (addFileWatcherIfRequired) {
611+
setMetadataFileWatcher(jwkSetPath);
612+
}
609613
final JWKSet jwkSet = readJwkSetFromFile(jwkSetPath);
610614
idTokenValidator = new IDTokenValidator(opConfig.getIssuer(), rpConfig.getClientId(), requestedAlgorithm, jwkSet);
611615
}
@@ -620,7 +624,7 @@ IDTokenValidator createIdTokenValidator() {
620624
private void setMetadataFileWatcher(String jwkSetPath) throws IOException {
621625
final Path path = realmConfig.env().configFile().resolve(jwkSetPath);
622626
FileWatcher watcher = new FileWatcher(path);
623-
watcher.addListener(new FileListener(LOGGER, () -> this.idTokenValidator.set(createIdTokenValidator())));
627+
watcher.addListener(new FileListener(LOGGER, () -> this.idTokenValidator.set(createIdTokenValidator(false))));
624628
watcherService.add(watcher, ResourceWatcherService.Frequency.MEDIUM);
625629
}
626630

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77

88
import org.apache.logging.log4j.LogManager;
99
import org.apache.logging.log4j.Logger;
10-
import org.apache.lucene.util.Constants;
1110
import org.elasticsearch.common.Strings;
1211
import org.elasticsearch.common.settings.MockSecureSettings;
1312
import org.elasticsearch.common.settings.Settings;
@@ -128,7 +127,6 @@ protected boolean transportSSLEnabled() {
128127
}
129128

130129
public void testClusterStarted() {
131-
assumeFalse("https://github.com/elastic/elasticsearch/issues/44942", Constants.WINDOWS);
132130
final AuthenticateRequest request = new AuthenticateRequest();
133131
request.username(nodeClientUsername());
134132
final AuthenticateResponse authenticate = client().execute(AuthenticateAction.INSTANCE, request).actionGet(10, TimeUnit.SECONDS);

0 commit comments

Comments
 (0)