[java-idp-plugin-duo] branch maint-1 updated: JDUO-74 - Possible leaks in HTTP response handling
Phil Smart
philip.smart at jisc.ac.uk
Wed Sep 6 15:23:03 UTC 2023
This is an automated email from the git hooks/post-receive script.
philsmart pushed a commit to branch maint-1
in repository java-idp-plugin-duo.
View the commit online:
http://git.shibboleth.net/view/?p=java-idp-plugin-duo.git;a=commit;h=a0fe1233765fb9ec614a2f805d10b42ff57f516f
The following commit(s) were added to refs/heads/maint-1 by this push:
new a0fe123 JDUO-74 - Possible leaks in HTTP response handling
a0fe123 is described below
commit a0fe1233765fb9ec614a2f805d10b42ff57f516f
Author: Phil Smart <philip.smart at jisc.ac.uk>
AuthorDate: Fri Sep 1 11:28:02 2023 +0100
JDUO-74 - Possible leaks in HTTP response handling
- Ensure HTTP input streams and entities are closed.
https://shibboleth.atlassian.net/browse/JDUO-74
---
.../plugin/authn/duo/nimbus/impl/NimbusClient.java | 219 ++++++++++++---------
.../idp/plugin/authn/duo/nimbus/plugin.properties | 2 +-
.../idp/plugin/authn/duo/sdk/plugin.properties | 2 +-
3 files changed, 128 insertions(+), 95 deletions(-)
diff --git a/idp-duo-nimbus-client-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/nimbus/impl/NimbusClient.java b/idp-duo-nimbus-client-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/nimbus/impl/NimbusClient.java
index 71e49d4..52e3ec2 100644
--- a/idp-duo-nimbus-client-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/nimbus/impl/NimbusClient.java
+++ b/idp-duo-nimbus-client-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/nimbus/impl/NimbusClient.java
@@ -18,6 +18,7 @@
package net.shibboleth.idp.plugin.authn.duo.nimbus.impl;
import java.io.IOException;
+import java.io.InputStream;
import java.net.URI;
import java.net.URISyntaxException;
import java.text.ParseException;
@@ -27,6 +28,7 @@ import javax.annotation.Nullable;
import javax.annotation.concurrent.Immutable;
import javax.annotation.concurrent.ThreadSafe;
+import org.apache.http.HttpEntity;
import org.apache.http.HttpResponse;
import org.apache.http.HttpStatus;
import org.apache.http.client.HttpClient;
@@ -34,6 +36,7 @@ import org.apache.http.client.methods.HttpUriRequest;
import org.apache.http.client.methods.RequestBuilder;
import org.apache.http.client.protocol.HttpClientContext;
import org.apache.http.client.utils.URIBuilder;
+import org.apache.http.util.EntityUtils;
import org.opensaml.security.httpclient.HttpClientSecurityParameters;
import org.opensaml.security.httpclient.HttpClientSecuritySupport;
import org.slf4j.Logger;
@@ -57,7 +60,7 @@ import net.shibboleth.utilities.java.support.logic.Constraint;
*/
@ThreadSafe
@Immutable
-public final class NimbusClient extends AbstractDuoOIDCClient{
+public final class NimbusClient extends AbstractDuoOIDCClient {
/** The only supported client assertion type.*/
@Nonnull @NotEmpty private static final String CLIENT_ASSERTION_TYPE =
@@ -65,13 +68,13 @@ public final class NimbusClient extends AbstractDuoOIDCClient{
/** The HTTPS scheme.*/
@Nonnull @NotEmpty private static final String HTTPS = "https";
-
+
/** Class logger. */
@Nonnull private final Logger log = LoggerFactory.getLogger(NimbusClient.class);
-
- /** The integration to help generate the JWT.*/
+
+ /** The integration to help generate the JWT. */
@Nonnull private final DuoOIDCIntegration duoIntegration;
-
+
/** HttpClient for contacting Duo. */
@Nonnull private final HttpClient httpClient;
@@ -81,64 +84,72 @@ public final class NimbusClient extends AbstractDuoOIDCClient{
/** JSON object mapper. */
@Nonnull private final ObjectMapper objectMapper;
-
/**
* Package-private Constructor.
*
- * <p>Should only be instantiated by the {@link NimbusClientFactory}.</p>
+ * <p>
+ * Should only be instantiated by the {@link NimbusClientFactory}.
+ * </p>
*
- * @param integration the integration to create the client for, never {@code null}
- * @param client the Http client to use to execute HTTP requests, never {@code null}
- * @param params any security parameters to use for the Http client, can be {@code null}.
- * @param oMapper the JSON object mapper, never {@code null}.
+ * @param integration
+ * the integration to create the client for, never {@code null}
+ * @param client
+ * the Http client to use to execute HTTP requests, never
+ * {@code null}
+ * @param params
+ * any security parameters to use for the Http client, can be
+ * {@code null}.
+ * @param oMapper
+ * the JSON object mapper, never {@code null}.
*/
NimbusClient(@Nonnull final DuoOIDCIntegration integration, @Nonnull final HttpClient client,
@Nullable final HttpClientSecurityParameters params, @Nonnull final ObjectMapper oMapper) {
super();
- duoIntegration = Constraint.isNotNull(integration,"Nimbus Client requires a non-null Duo Integration");
- httpClient = Constraint.isNotNull(client,"Nimbus Client requires a non-null http client");
+ duoIntegration = Constraint.isNotNull(integration, "Nimbus Client requires a non-null Duo Integration");
+ httpClient = Constraint.isNotNull(client, "Nimbus Client requires a non-null http client");
httpClientSecurityParameters = params;
objectMapper = oMapper;
- //We do not validate the integration parameters e.g. because we know Duo use an invalid secret key
+ // We do not validate the integration parameters e.g. because we know Duo use an
+ // invalid secret key
}
@Override
- @Nonnull public DuoHealthCheck healthCheck() throws DuoClientException {
- try {
+ @Nonnull
+ public DuoHealthCheck healthCheck() throws DuoClientException {
+ try {
final URI uri = new URIBuilder().setScheme(HTTPS).setHost(duoIntegration.getAPIHost())
- .setPath(duoIntegration.getHealthCheckEndpoint()).build();
- log.trace("Using health check endpoint '{}'",uri);
-
- final RequestBuilder rb =
- RequestBuilder.post().setUri(uri).addParameter("client_id",duoIntegration.getClientId())
- .addParameter("client_assertion",
- NimbusClientSupport.createJWS(uri.toString(),
- duoIntegration.getClientId(), duoIntegration.getSecretKey()));
+ .setPath(duoIntegration.getHealthCheckEndpoint()).build();
+ log.trace("Using health check endpoint '{}'", uri);
+
+ final RequestBuilder rb = RequestBuilder.post().setUri(uri)
+ .addParameter("client_id", duoIntegration.getClientId())
+ .addParameter("client_assertion", NimbusClientSupport.createJWS(uri.toString(),
+ duoIntegration.getClientId(), duoIntegration.getSecretKey()));
return executeRequest(rb.build(), new TypeReference<DuoHealthCheck>() {});
-
+
} catch (final URISyntaxException e) {
- throw new DuoClientException("Error performing a Duo health check",e);
- }
+ throw new DuoClientException("Error performing a Duo health check", e);
+ }
}
@Override
- @Nonnull public String createAuthUrl(@Nonnull @NotEmpty final String username,
- @Nonnull @NotEmpty final String state, @Nullable final String nonce,
- @Nullable final String redirectURIOverride) throws DuoClientException {
+ @Nonnull
+ public String createAuthUrl(@Nonnull @NotEmpty final String username, @Nonnull @NotEmpty final String state,
+ @Nullable final String nonce, @Nullable final String redirectURIOverride) throws DuoClientException {
Constraint.isNotEmpty(username, "Username can not be null or empty");
Constraint.isNotEmpty(state, "State can not be null or empty");
Constraint.isNotEmpty(nonce, "Nonce can not be null or empty for this client");
Constraint.isGreaterThan(21, state.length(), "State must be at least 22 characters");
- Constraint.isLessThan(1025, state.length(),"State must be at maximum 1024 characters");
-
+ Constraint.isLessThan(1025, state.length(), "State must be at maximum 1024 characters");
+
try {
- final String redirectURI = redirectURIOverride != null ?
- redirectURIOverride : duoIntegration.getRedirectURI();
-
+ final String redirectURI = redirectURIOverride != null ? redirectURIOverride
+ : duoIntegration.getRedirectURI();
+
if (redirectURI == null) {
throw new DuoClientException("A redirect_uri was not supplied but is required "
- + "for creating an authorization request, for client '"+duoIntegration.getClientId()+"'");
+ + "for creating an authorization request, for client '" + duoIntegration.getClientId() + "'");
}
final String request = NimbusClientSupport.createJWSRequestObject(
@@ -157,93 +168,115 @@ public final class NimbusClient extends AbstractDuoOIDCClient{
return uri.toString();
} catch (final URISyntaxException e) {
- throw new DuoClientException("Unable to create a Duo authorization URL",e);
- }
-
+ throw new DuoClientException("Unable to create a Duo authorization URL", e);
+ }
+
}
@Override
- public JWT exchangeAuthorizationCodeFor2FAResult(@Nonnull final String code,
- @Nonnull final String username, @Nullable final String redirectURIOverride) throws DuoClientException {
+ public JWT exchangeAuthorizationCodeFor2FAResult(@Nonnull final String code, @Nonnull final String username,
+ @Nullable final String redirectURIOverride) throws DuoClientException {
Constraint.isNotEmpty(code, "Auth_code can not be null");
-
- try {
+
+ try {
final URI uri = new URIBuilder().setScheme(HTTPS).setHost(duoIntegration.getAPIHost())
- .setPath(duoIntegration.getTokenEndpoint()).build();
- log.trace("Using authorization endpoint and audience '{}'",uri);
-
- final String redirectURI = redirectURIOverride != null ?
- redirectURIOverride : duoIntegration.getRedirectURI();
-
+ .setPath(duoIntegration.getTokenEndpoint()).build();
+ log.trace("Using authorization endpoint and audience '{}'", uri);
+
+ final String redirectURI = redirectURIOverride != null ? redirectURIOverride
+ : duoIntegration.getRedirectURI();
+
if (redirectURI == null) {
throw new DuoClientException("A redirect_uri was not supplied but is required "
- + "for acquiring a 2FA result, for client '"+duoIntegration.getClientId()+"'");
+ + "for acquiring a 2FA result, for client '" + duoIntegration.getClientId() + "'");
}
-
- final RequestBuilder rb =
- RequestBuilder.post().setUri(uri)
- .addParameter("grant_type","authorization_code")
- .addParameter("code",code)
- .addParameter("redirect_uri",redirectURI)
- .addParameter("client_assertion_type",CLIENT_ASSERTION_TYPE)
- .addParameter("client_assertion",
- NimbusClientSupport.createJWS(uri.toString(),duoIntegration.getClientId(),
- duoIntegration.getSecretKey()));
-
- final TokenResponse response = executeRequest(rb.build(),new TypeReference<TokenResponse>() {});
- log.trace("Duo token response: '{}'",response);
- return SignedJWT.parse(response.getIdToken());
-
+
+ final RequestBuilder rb = RequestBuilder.post().setUri(uri)
+ .addParameter("grant_type", "authorization_code")
+ .addParameter("code", code)
+ .addParameter("redirect_uri", redirectURI)
+ .addParameter("client_assertion_type", CLIENT_ASSERTION_TYPE)
+ .addParameter("client_assertion", NimbusClientSupport.createJWS(uri.toString(),
+ duoIntegration.getClientId(), duoIntegration.getSecretKey()));
+
+ final TokenResponse response = executeRequest(rb.build(), new TypeReference<TokenResponse>() {});
+ log.trace("Duo token response: '{}'", response);
+ return SignedJWT.parse(response.getIdToken());
+
} catch (final URISyntaxException | ParseException e) {
- throw new DuoClientException("Unable to swap auth_code for id_token",e);
- }
+ throw new DuoClientException("Unable to swap auth_code for id_token", e);
+ }
}
-
-
+
/**
- * Performs a call to a Duo OIDC endpoint. Iff successful, the JSON response is mapped into the appropriate
- * type.
+ * Performs a call to a Duo OIDC endpoint. Iff successful, the JSON response is
+ * mapped into the appropriate type.
*
- * @param <T> the response type
- * @param request the prepared HTTP request
- * @param wrapperTypeRef the type to deserialise the JSON into
+ * @param <T>
+ * the response type
+ * @param request
+ * the prepared HTTP request
+ * @param wrapperTypeRef
+ * the type to deserialise the JSON into
*
* @return the response type, never {@code null}.
*
- * @throws DuoClientException if there is an error producing a response
+ * @throws DuoClientException
+ * if there is an error producing a response
*/
- private <T> T executeRequest(@Nonnull final HttpUriRequest request,
- @Nonnull final TypeReference<T> wrapperTypeRef) throws DuoClientException{
-
+ private <T> T executeRequest(@Nonnull final HttpUriRequest request, @Nonnull final TypeReference<T> wrapperTypeRef)
+ throws DuoClientException {
+
+ HttpResponse httpResponse = null;
try {
final HttpClientContext clientContext = HttpClientContext.create();
HttpClientSecuritySupport.marshalSecurityParameters(clientContext, httpClientSecurityParameters, true);
HttpClientSecuritySupport.addDefaultTLSTrustEngineCriteria(clientContext, request);
- final HttpResponse httpResponse = httpClient.execute(request, clientContext);
+ httpResponse = httpClient.execute(request, clientContext);
HttpClientSecuritySupport.checkTLSCredentialEvaluated(clientContext, request.getURI().getScheme());
-
+
+ if (httpResponse == null || httpResponse.getStatusLine() == null) {
+ throw new DuoClientException("No response or response status returned by Duo");
+ }
final int httpStatusCode = httpResponse.getStatusLine().getStatusCode();
if (httpStatusCode != HttpStatus.SC_OK) {
- //dump the body for logging - if one exists
- if (httpResponse.getEntity() != null && httpResponse.getEntity().getContent() != null) {
- final String errorContent = IOUtils.readInputStreamToString(httpResponse.getEntity().getContent());
- log.error("Duo returned a Non-ok message of '{}'",errorContent);
+ // dump the body for logging - if one exists
+ final HttpEntity entity = httpResponse.getEntity();
+ if (entity != null) {
+ try (final InputStream content = entity.getContent()) {
+ final String errorContent = IOUtils.readInputStreamToString(content);
+ log.error("Duo returned a Non-ok message of '{}'", errorContent);
+ }
}
throw new DuoClientException("Non-ok status code (" + httpStatusCode + ") returned from Duo: "
+ httpResponse.getStatusLine().getReasonPhrase());
- } else if (httpResponse.getEntity() == null) {
+
+ }
+
+ final HttpEntity entity = httpResponse.getEntity();
+ if (entity == null) {
throw new DuoClientException("No response body returned from Duo");
}
-
+
// Parse the JSON response.
- final T duoResponse = objectMapper.readValue(httpResponse.getEntity().getContent(),wrapperTypeRef);
- if (duoResponse == null) {
- throw new DuoClientException("Unable to parse JSON response");
- }
- return duoResponse;
-
+ try (final InputStream content = entity.getContent()) {
+ final T duoResponse = objectMapper.readValue(content, wrapperTypeRef);
+ if (duoResponse == null) {
+ throw new DuoClientException("Unable to parse JSON response");
+ }
+ return duoResponse;
+ }
+
} catch (final IOException e) {
- throw new DuoClientException("Could not execute Duo HTTP request",e);
+ throw new DuoClientException("Could not execute Duo HTTP request", e);
+ } finally {
+ if (httpResponse != null) {
+ try {
+ EntityUtils.consume(httpResponse.getEntity());
+ } catch (final IOException e) {
+ throw new DuoClientException(e);
+ }
+ }
}
}
diff --git a/idp-duo-nimbus-client-impl/src/main/resources/net/shibboleth/idp/plugin/authn/duo/nimbus/plugin.properties b/idp-duo-nimbus-client-impl/src/main/resources/net/shibboleth/idp/plugin/authn/duo/nimbus/plugin.properties
index e689a9f..c79d8f4 100644
--- a/idp-duo-nimbus-client-impl/src/main/resources/net/shibboleth/idp/plugin/authn/duo/nimbus/plugin.properties
+++ b/idp-duo-nimbus-client-impl/src/main/resources/net/shibboleth/idp/plugin/authn/duo/nimbus/plugin.properties
@@ -2,7 +2,7 @@
plugin.id = net.shibboleth.idp.plugin.authn.duo.nimbus
# Only used when package manifest is not available
-plugin.version = 1.4.0
+plugin.version = 1.4.1
# prereqs
plugin.modules.required = idp.oidc.common.3
diff --git a/idp-duo-sdk-client-impl/src/main/resources/net/shibboleth/idp/plugin/authn/duo/sdk/plugin.properties b/idp-duo-sdk-client-impl/src/main/resources/net/shibboleth/idp/plugin/authn/duo/sdk/plugin.properties
index 17bfd89..655ef76 100644
--- a/idp-duo-sdk-client-impl/src/main/resources/net/shibboleth/idp/plugin/authn/duo/sdk/plugin.properties
+++ b/idp-duo-sdk-client-impl/src/main/resources/net/shibboleth/idp/plugin/authn/duo/sdk/plugin.properties
@@ -2,7 +2,7 @@
plugin.id = net.shibboleth.idp.plugin.authn.duo.sdk
# Only used when package manifest is not available
-plugin.version = 1.4.0
+plugin.version = 1.4.1
plugin.license =/net/shibboleth/idp/plugin/authn/duo/sdk/doc/Duo-LICENSE.txt
# prereqs
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list