Skip to content

Commit

Permalink
Fix to release system resource after reading JKWSet file
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Yogesh Gaikwad committed Oct 30, 2019
1 parent e6924cb commit 0c8d8f3
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);

}

/**
Expand Down Expand Up @@ -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());
Expand All @@ -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);
}
Expand All @@ -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);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit 0c8d8f3

Please sign in to comment.