[java-idp-plugin-duo] branch main updated: JDUO-74 - Possible leaks in HTTP response handling

Scott Cantor cantor.2 at osu.edu
Tue Aug 29 12:45:09 UTC 2023


This is an automated email from the git hooks/post-receive script.

scantor pushed a commit to branch main
in repository java-idp-plugin-duo.

View the commit online:
http://git.shibboleth.net/view/?p=java-idp-plugin-duo.git;a=commit;h=d07f13f51428c1443363efa1a24a41f5249981d1

The following commit(s) were added to refs/heads/main by this push:
     new d07f13f  JDUO-74 - Possible leaks in HTTP response handling
d07f13f is described below

commit d07f13f51428c1443363efa1a24a41f5249981d1
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Tue Aug 29 08:45:06 2023 -0400

    JDUO-74 - Possible leaks in HTTP response handling
    
    https://shibboleth.atlassian.net/browse/JDUO-74
    
    Also need to close InputStreams.
    Add additional null checks.
---
 .../authn/duo/impl/AbstractDuoAuthenticator.java   | 54 ++++++++++++++--------
 .../plugin/authn/duo/nimbus/impl/NimbusClient.java | 31 ++++++++-----
 2 files changed, 54 insertions(+), 31 deletions(-)

diff --git a/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/AbstractDuoAuthenticator.java b/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/AbstractDuoAuthenticator.java
index 630004c..85c8670 100644
--- a/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/AbstractDuoAuthenticator.java
+++ b/idp-duo-impl/src/main/java/net/shibboleth/idp/plugin/authn/duo/impl/AbstractDuoAuthenticator.java
@@ -25,6 +25,7 @@ import org.apache.hc.client5.http.classic.HttpClient;
 import org.apache.hc.client5.http.protocol.HttpClientContext;
 import org.apache.hc.core5.http.ClassicHttpRequest;
 import org.apache.hc.core5.http.ClassicHttpResponse;
+import org.apache.hc.core5.http.HttpEntity;
 import org.apache.hc.core5.http.HttpStatus;
 import org.opensaml.security.httpclient.HttpClientSecurityParameters;
 import org.opensaml.security.httpclient.HttpClientSecuritySupport;
@@ -97,6 +98,7 @@ public abstract class AbstractDuoAuthenticator extends AbstractInitializableComp
         }
     }
 
+// Checkstyle: CyclomaticComplexity OFF
     /**
      * Performs a call to the Duo AuthAPI. Upon a successful call, the JSON response is mapped into the appropriate type
      * of {@link DuoResponseWrapper}.
@@ -119,7 +121,7 @@ public abstract class AbstractDuoAuthenticator extends AbstractInitializableComp
         assert clientContext != null;
         HttpClientSecuritySupport.marshalSecurityParameters(clientContext, httpClientSecurityParameters, true);
         HttpClientSecuritySupport.addDefaultTLSTrustEngineCriteria(clientContext, request);
-        try (final ClassicHttpResponse httpResponse = httpClient.executeOpen(null, request, clientContext)){
+        try (final ClassicHttpResponse httpResponse = httpClient.executeOpen(null, request, clientContext)) {
             final String scheme = request.getScheme();
             assert scheme != null;
             HttpClientSecuritySupport.checkTLSCredentialEvaluated(clientContext, scheme);
@@ -127,33 +129,45 @@ public abstract class AbstractDuoAuthenticator extends AbstractInitializableComp
             // Check the HTTP response code.
             final int httpStatusCode = httpResponse.getCode();
             if (httpStatusCode == HttpStatus.SC_BAD_REQUEST) {
-                final InputStream httpContent = httpResponse.getEntity().getContent();
-                final DuoFailureResponse msg = objectMapper.readValue(httpContent, DuoFailureResponse.class);
-                final StringBuilder builder = new StringBuilder();
-                builder.append(msg.getMessage() != null ? msg.getMessage() : "no message")
-                    .append(" (")
-                    .append(msg.getMessageDetail() != null ? msg.getMessageDetail() : "no detail")
-                    .append(")");
-                throw new DuoException(builder.toString());
+                final HttpEntity entity = httpResponse.getEntity();
+                if (entity == null) {
+                    throw new IOException("Bad request status code (" + httpStatusCode + ") returned from Duo: "
+                            + (httpResponse.getReasonPhrase() != null ? httpResponse.getReasonPhrase() : "none"));
+                }
+                try (final InputStream httpContent = entity.getContent()) {
+                    final DuoFailureResponse msg = objectMapper.readValue(httpContent, DuoFailureResponse.class);
+                    final StringBuilder builder = new StringBuilder();
+                    builder.append(msg.getMessage() != null ? msg.getMessage() : "no message")
+                        .append(" (")
+                        .append(msg.getMessageDetail() != null ? msg.getMessageDetail() : "no detail")
+                        .append(")");
+                    throw new DuoException(builder.toString());
+                }
             }
             if (httpStatusCode != HttpStatus.SC_OK) {
                 throw new IOException("Non-ok status code (" + httpStatusCode + ") returned from Duo: "
-                        + httpResponse.getReasonPhrase());
-            } else if (httpResponse.getEntity() == null) {
+                        + (httpResponse.getReasonPhrase() != null ? httpResponse.getReasonPhrase() : "none"));
+            }
+            
+            final HttpEntity entity = httpResponse.getEntity();
+            if (entity == null) {
                 throw new IOException("No response body returned from Duo");
             }
     
             // Parse the JSON response.
-            final T duoResponse = objectMapper.readValue(httpResponse.getEntity().getContent(), wrapperTypeRef);
+            try (final InputStream content = entity.getContent()) {
+                final T duoResponse = objectMapper.readValue(entity.getContent(), wrapperTypeRef);
+        
+                if (duoResponse == null) {
+                    throw new DuoException("Unable to parse JSON response");
+                } else if (!"OK".equals(duoResponse.getStat())) {
+                    throw new DuoException("Unexpected status value in JSON response: " + duoResponse.getStat());
+                }
     
-            if (duoResponse == null) {
-                throw new DuoException("Unable to parse JSON response");
-            } else if (!"OK".equals(duoResponse.getStat())) {
-                throw new DuoException("Unexpected status value in JSON response: " + duoResponse.getStat());
+                return duoResponse;
             }
-
-            return duoResponse;
         }
     }
-
-}
+// Checkstyle: CyclomaticComplexity ON
+    
+}
\ No newline at end of file
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 5c26ec3..d814f88 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
@@ -15,6 +15,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;
@@ -28,6 +29,7 @@ import org.apache.hc.client5.http.classic.HttpClient;
 import org.apache.hc.client5.http.protocol.HttpClientContext;
 import org.apache.hc.core5.http.ClassicHttpRequest;
 import org.apache.hc.core5.http.ClassicHttpResponse;
+import org.apache.hc.core5.http.HttpEntity;
 import org.apache.hc.core5.http.HttpStatus;
 import org.apache.hc.core5.http.io.support.ClassicRequestBuilder;
 import org.apache.hc.core5.net.URIBuilder;
@@ -224,23 +226,30 @@ public final class NimbusClient extends AbstractDuoOIDCClient{
                 final int httpStatusCode = httpResponse.getCode();
                 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);
+                    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.getReasonPhrase());
-                } else if (httpResponse.getEntity() == null) {
+                            + (httpResponse.getReasonPhrase() != null ? httpResponse.getReasonPhrase() : "none"));
+                }
+                
+                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 | URISyntaxException e) {
             throw new DuoClientException("Could not execute Duo HTTP request",e);

-- 
To stop receiving notification emails like this one, please contact
the administrator of this repository.


More information about the commits mailing list