Browse Source

Fix to release system resource after reading JKWSet file (#48666)

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
Yogesh Gaikwad 6 years ago
parent
commit
678492d798

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

@@ -76,7 +76,6 @@ import org.elasticsearch.action.ActionListener;
 import org.elasticsearch.common.CheckedRunnable;
 import org.elasticsearch.common.Nullable;
 import org.elasticsearch.common.Strings;
-import org.elasticsearch.common.SuppressForbidden;
 import org.elasticsearch.common.collect.Tuple;
 import org.elasticsearch.common.util.concurrent.EsExecutors;
 import org.elasticsearch.common.util.concurrent.ListenableFuture;
@@ -95,10 +94,10 @@ import java.io.UnsupportedEncodingException;
 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 +110,8 @@ import java.util.Map;
 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 +141,7 @@ public class OpenIdConnectAuthenticator {
         this.sslService = sslService;
         this.httpClient = createHttpClient();
         this.watcherService = watcherService;
-        this.idTokenValidator.set(createIdTokenValidator());
+        this.idTokenValidator.set(createIdTokenValidator(true));
     }
 
     // For testing
@@ -314,10 +313,11 @@ public class OpenIdConnectAuthenticator {
      * @throws ParseException if the file cannot be parsed
      * @throws IOException    if the file cannot be read
      */
-    @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 = Files.readString(path, StandardCharsets.UTF_8);
+        return JWKSet.parse(jwkSet);
     }
 
     /**
@@ -589,7 +589,7 @@ public class OpenIdConnectAuthenticator {
     /*
      * 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 +600,16 @@ public class OpenIdConnectAuthenticator {
                     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 +624,7 @@ public class OpenIdConnectAuthenticator {
     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);
     }
 

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

@@ -7,7 +7,6 @@ package org.elasticsearch.xpack.security.authc;
 
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
-import org.apache.lucene.util.Constants;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.settings.MockSecureSettings;
 import org.elasticsearch.common.settings.Settings;
@@ -128,7 +127,6 @@ public class SecurityRealmSettingsTests extends SecurityIntegTestCase {
     }
 
     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);