honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

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

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
This commit is contained in:
Yogesh Gaikwad 2019-10-31 10:16:33 +11:00 committed by GitHub
parent a876760848
commit c7342dde29
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 14 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -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 = new String(Files.readAllBytes(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);
}

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

@ -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);