From 0c8d8f31df5d3be8231b7f43a5a3cd59f62a9550 Mon Sep 17 00:00:00 2001 From: Yogesh Gaikwad Date: Wed, 30 Oct 2019 12:16:08 +1100 Subject: [PATCH] Fix to release system resource after reading JKWSet file 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 #44942 --- .../oidc/OpenIdConnectAuthenticator.java | 23 ++++++++++++------- .../authc/SecurityRealmSettingsTests.java | 1 - 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectAuthenticator.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectAuthenticator.java index a3218fc90552f..5d47ad4b6be9e 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectAuthenticator.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectAuthenticator.java @@ -95,10 +95,10 @@ import java.net.URI; import java.net.URISyntaxException; import java.net.URL; - import java.net.URLEncoder; import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; +import java.nio.file.Files; import java.nio.file.Path; import java.security.AccessController; import java.security.PrivilegedAction; @@ -111,8 +111,8 @@ import java.util.concurrent.atomic.AtomicReference; import static org.elasticsearch.xpack.core.security.authc.oidc.OpenIdConnectRealmSettings.ALLOWED_CLOCK_SKEW; -import static org.elasticsearch.xpack.core.security.authc.oidc.OpenIdConnectRealmSettings.HTTP_CONNECT_TIMEOUT; 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; import static org.elasticsearch.xpack.core.security.authc.oidc.OpenIdConnectRealmSettings.HTTP_MAX_ENDPOINT_CONNECTIONS; import static org.elasticsearch.xpack.core.security.authc.oidc.OpenIdConnectRealmSettings.HTTP_SOCKET_TIMEOUT; @@ -142,7 +142,7 @@ public OpenIdConnectAuthenticator(RealmConfig realmConfig, OpenIdConnectProvider this.sslService = sslService; this.httpClient = createHttpClient(); this.watcherService = watcherService; - this.idTokenValidator.set(createIdTokenValidator()); + this.idTokenValidator.set(createIdTokenValidator(true)); } // For testing @@ -317,7 +317,10 @@ private void validateAccessToken(AccessToken accessToken, JWT idToken) { @SuppressForbidden(reason = "uses toFile") private JWKSet readJwkSetFromFile(String jwkSetPath) throws IOException, ParseException { final Path path = realmConfig.env().configFile().resolve(jwkSetPath); - return JWKSet.load(path.toFile()); + // avoid using JWKSet.loadFile() as it does not close FileInputStream internally + String jwkSet = new String(Files.readAllBytes(path), StandardCharsets.UTF_8); + return JWKSet.parse(jwkSet); + } /** @@ -589,7 +592,7 @@ private CloseableHttpAsyncClient createHttpClient() { /* * Creates an {@link IDTokenValidator} based on the current Relying Party configuration */ - IDTokenValidator createIdTokenValidator() { + IDTokenValidator createIdTokenValidator(boolean addFileWatcherIfRequired) { try { final JWSAlgorithm requestedAlgorithm = rpConfig.getSignatureAlgorithm(); final int allowedClockSkew = Math.toIntExact(realmConfig.getSetting(ALLOWED_CLOCK_SKEW).getMillis()); @@ -600,12 +603,16 @@ IDTokenValidator createIdTokenValidator() { new IDTokenValidator(opConfig.getIssuer(), rpConfig.getClientId(), requestedAlgorithm, clientSecret); } else { String jwkSetPath = opConfig.getJwkSetPath(); - if (jwkSetPath.startsWith("https://")) { + if (jwkSetPath.startsWith("http://")) { + throw new IllegalArgumentException("The [http] protocol is not supported as it is insecure. Use [https] instead"); + } else if (jwkSetPath.startsWith("https://")) { final JWSVerificationKeySelector keySelector = new JWSVerificationKeySelector(requestedAlgorithm, new ReloadableJWKSource(new URL(jwkSetPath))); idTokenValidator = new IDTokenValidator(opConfig.getIssuer(), rpConfig.getClientId(), keySelector, null); } else { - setMetadataFileWatcher(jwkSetPath); + if (addFileWatcherIfRequired) { + setMetadataFileWatcher(jwkSetPath); + } final JWKSet jwkSet = readJwkSetFromFile(jwkSetPath); idTokenValidator = new IDTokenValidator(opConfig.getIssuer(), rpConfig.getClientId(), requestedAlgorithm, jwkSet); } @@ -620,7 +627,7 @@ IDTokenValidator createIdTokenValidator() { private void setMetadataFileWatcher(String jwkSetPath) throws IOException { final Path path = realmConfig.env().configFile().resolve(jwkSetPath); FileWatcher watcher = new FileWatcher(path); - watcher.addListener(new FileListener(LOGGER, () -> this.idTokenValidator.set(createIdTokenValidator()))); + watcher.addListener(new FileListener(LOGGER, () -> this.idTokenValidator.set(createIdTokenValidator(false)))); watcherService.add(watcher, ResourceWatcherService.Frequency.MEDIUM); } diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/SecurityRealmSettingsTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/SecurityRealmSettingsTests.java index 4566715d6f4d3..9e33b4f405d80 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/SecurityRealmSettingsTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/SecurityRealmSettingsTests.java @@ -128,7 +128,6 @@ protected boolean transportSSLEnabled() { } public void testClusterStarted() { - assumeFalse("https://github.com/elastic/elasticsearch/issues/44942", Constants.WINDOWS); final AuthenticateRequest request = new AuthenticateRequest(); request.username(nodeClientUsername()); final AuthenticateResponse authenticate = client().execute(AuthenticateAction.INSTANCE, request).actionGet(10, TimeUnit.SECONDS);