[java-opensaml] branch main updated: OSJ-223: HttpClient configured TLS trust engine is not evaluated on ...

Brent Putman putmanb at georgetown.edu
Thu Sep 3 03:22:08 UTC 2020


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

putmanb pushed a commit to branch main
in repository java-opensaml.

View the commit online:
http://git.shibboleth.net/view/?p=java-opensaml.git;a=commit;h=0405be835796b417b84bef1e8f1f1096ec8a3009

The following commit(s) were added to refs/heads/main by this push:
       new  0405be835 OSJ-223: HttpClient configured TLS trust engine is not evaluated on ...
0405be835 is described below

commit 0405be835796b417b84bef1e8f1f1096ec8a3009
Author: Brent Putman <putmanb at georgetown.edu>
AuthorDate: Wed Sep 2 22:11:02 2020 -0400

    OSJ-223: HttpClient configured TLS trust engine is not evaluated on ...
    
    HttpClient configured TLS trust engine is not evaluated on TLS session
    resumption.
---
 .../impl/SecurityEnhancedTLSSocketFactory.java     | 41 ++++++++++-
 ...java => ThreadLocalX509TrustEngineSupport.java} | 85 +++++++++++++---------
 .../x509/tls/impl/ThreadLocalX509TrustManager.java | 84 +--------------------
 3 files changed, 90 insertions(+), 120 deletions(-)

diff --git a/opensaml-security-impl/src/main/java/org/opensaml/security/httpclient/impl/SecurityEnhancedTLSSocketFactory.java b/opensaml-security-impl/src/main/java/org/opensaml/security/httpclient/impl/SecurityEnhancedTLSSocketFactory.java
index 34840e365..c6f195073 100644
--- a/opensaml-security-impl/src/main/java/org/opensaml/security/httpclient/impl/SecurityEnhancedTLSSocketFactory.java
+++ b/opensaml-security-impl/src/main/java/org/opensaml/security/httpclient/impl/SecurityEnhancedTLSSocketFactory.java
@@ -25,6 +25,7 @@ import java.util.Collections;
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 import javax.net.ssl.SSLPeerUnverifiedException;
+import javax.net.ssl.SSLSocket;
 import javax.net.ssl.X509TrustManager;
 
 import org.apache.http.HttpHost;
@@ -39,6 +40,7 @@ import org.opensaml.security.x509.TrustedNamesCriterion;
 import org.opensaml.security.x509.X509Credential;
 import org.opensaml.security.x509.tls.impl.ThreadLocalX509CredentialContext;
 import org.opensaml.security.x509.tls.impl.ThreadLocalX509TrustEngineContext;
+import org.opensaml.security.x509.tls.impl.ThreadLocalX509TrustEngineSupport;
 import org.opensaml.security.x509.tls.impl.ThreadLocalX509TrustManager;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -144,7 +146,10 @@ public class SecurityEnhancedTLSSocketFactory implements LayeredConnectionSocket
         log.trace("In connectSocket");
         try {
             setup(context, host.getHostName());
-            return wrappedFactory.connectSocket(connectTimeout, sock, host, remoteAddress, localAddress, context);
+            final Socket socket =
+                    wrappedFactory.connectSocket(connectTimeout, sock, host, remoteAddress, localAddress, context);
+            checkAndEvaluateServerTLS(socket);
+            return socket;
         } finally {
             teardown(context);
         }
@@ -157,12 +162,44 @@ public class SecurityEnhancedTLSSocketFactory implements LayeredConnectionSocket
         log.trace("In createLayeredSocket");
         try {
             setup(context, target);
-            return wrappedFactory.createLayeredSocket(socket, target, port, context);
+            final Socket layeredSocket = wrappedFactory.createLayeredSocket(socket, target, port, context);
+            checkAndEvaluateServerTLS(socket);
+            return layeredSocket;
         } finally {
             teardown(context);
         }
     }
     
+    /**
+     * Check that the evaluation of the socket certificate using the data in
+     * {@link ThreadLocalX509TrustEngineContext} has been performed, if applicable,
+     * and if not, evaluate it.
+     *
+     * <p>
+     * This will usually be called only in the case of TLS session resumption, when the standard
+     * JSSE trust manager evaluation has not run.
+     * </p>
+     *
+     * @param socket the current socket being evaluated
+     * @throws IOException
+     */
+    protected void checkAndEvaluateServerTLS(@Nonnull final Socket socket) throws IOException {
+        if (!SSLSocket.class.isInstance(socket)) {
+           return;
+        }
+
+        if (ThreadLocalX509TrustEngineContext.getTrustEngine() != null) {
+            if (ThreadLocalX509TrustEngineContext.getTrusted() == null) {
+                log.trace("Have TrustEngine but was not previously evaluated, likely due to TLS session resumption. "
+                        + "Evaluating now.");
+                ThreadLocalX509TrustEngineSupport.evaluate(SSLSocket.class.cast(socket));
+            } else {
+                log.trace("Had TrustEngine and was previously evaluated as trusted={}",
+                        ThreadLocalX509TrustEngineContext.getTrusted());
+            }
+        }
+    }
+
     /**
      * Setup calling execution environment for server TLS and client TLS based on information supplied in the
      * {@link HttpContext}.
diff --git a/opensaml-security-impl/src/main/java/org/opensaml/security/x509/tls/impl/ThreadLocalX509TrustManager.java b/opensaml-security-impl/src/main/java/org/opensaml/security/x509/tls/impl/ThreadLocalX509TrustEngineSupport.java
similarity index 63%
copy from opensaml-security-impl/src/main/java/org/opensaml/security/x509/tls/impl/ThreadLocalX509TrustManager.java
copy to opensaml-security-impl/src/main/java/org/opensaml/security/x509/tls/impl/ThreadLocalX509TrustEngineSupport.java
index 9e059030b..b07894cee 100644
--- a/opensaml-security-impl/src/main/java/org/opensaml/security/x509/tls/impl/ThreadLocalX509TrustManager.java
+++ b/opensaml-security-impl/src/main/java/org/opensaml/security/x509/tls/impl/ThreadLocalX509TrustEngineSupport.java
@@ -17,14 +17,15 @@
 
 package org.opensaml.security.x509.tls.impl;
 
+import java.security.cert.Certificate;
 import java.security.cert.CertificateException;
 import java.security.cert.X509Certificate;
 import java.util.Arrays;
 import java.util.List;
 
 import javax.annotation.Nonnull;
+import javax.net.ssl.SSLPeerUnverifiedException;
 import javax.net.ssl.SSLSocket;
-import javax.net.ssl.X509TrustManager;
 
 import org.opensaml.security.trust.TrustEngine;
 import org.opensaml.security.x509.BasicX509Credential;
@@ -36,52 +37,64 @@ import net.shibboleth.utilities.java.support.annotation.constraint.NotEmpty;
 import net.shibboleth.utilities.java.support.resolver.CriteriaSet;
 
 /**
- * An implementation of {@link X509TrustManager} which performs its evaluation using trust engine and criteria
- * instances available from {@link ThreadLocalX509TrustEngineContext}.
+ * Support class for centralizing evaluation of a certificate chain using trust engine and criteria
+ * from {@link ThreadLocalX509TrustEngineContext}.
  */
-public class ThreadLocalX509TrustManager implements X509TrustManager {
+public final class ThreadLocalX509TrustEngineSupport {
     
     /** Logger. */
-    private Logger log = LoggerFactory.getLogger(ThreadLocalX509TrustManager.class);
-
-    /** {@inheritDoc} */
-    public X509Certificate[] getAcceptedIssuers() {
-        return new X509Certificate[]{};
-    }
+    private static final Logger LOG = LoggerFactory.getLogger(ThreadLocalX509TrustEngineSupport.class);
     
-    /** {@inheritDoc} */
-    public void checkClientTrusted(final X509Certificate[] chain, final String authType) throws CertificateException {
-        performTrustEval(chain, authType);
-    }
+    /** Constructor. */
+    private ThreadLocalX509TrustEngineSupport() { }
     
-    /** {@inheritDoc} */
-    public void checkServerTrusted(final X509Certificate[] chain, final String authType) throws CertificateException {
-        performTrustEval(chain, authType);
+    /**
+     * Perform trust evaluation on the specified {@link SSLSocket} using the current data in
+     * {@link ThreadLocalX509TrustEngineContext}.
+     * 
+     * @param sslSocket the socket whose certificates are to be evaluated
+     * 
+     * @throws SSLPeerUnverifiedException if the certificate chain was not trusted by the supplied TrustEngine
+     */
+    public static void evaluate(@Nonnull final SSLSocket sslSocket) throws SSLPeerUnverifiedException {
+        final Certificate[] chain = sslSocket.getSession().getPeerCertificates();
+        
+        if (chain == null || chain.length == 0) {
+            throw new IllegalArgumentException("Certificate chain was null or empty");
+        }
+        
+        // Just sanity checking this
+        if (X509Certificate.class.isInstance(chain[0])) {
+            try {
+                evaluate((X509Certificate[]) chain);
+            } catch (final CertificateException e) {
+                throw new SSLPeerUnverifiedException(e.getMessage());
+            }
+        } else {
+            throw new SSLPeerUnverifiedException("Certificate chain was not instance of X509Certificate");
+        }
     }
 
     /**
-     * Perform trust evaluation on the specified certificate chain using the trust engine and criteria
-     * available from {@link ThreadLocalX509TrustEngineContext}.
+     * Perform trust evaluation on the specified certificate chain using the current data in
+     * {@link ThreadLocalX509TrustEngineContext}.
      * 
-     * @param chain the peer certificate chain
-     * @param authType the authentication type based on the client certificate
+     * @param chain the certificate chain to be evaluated
      * 
-     * @throws CertificateException if the certificate chain is not trusted by this TrustManager.
+     * @throws CertificateException if the certificate chain is not trusted by the supplied TrustEngine
      */
-    protected void performTrustEval(final X509Certificate[] chain, final String authType) throws CertificateException {
-        // These checks are per the documentation for this interface
+    public static void evaluate(@Nonnull final X509Certificate[] chain) throws CertificateException {
         if (chain == null || chain.length == 0) {
             throw new IllegalArgumentException("Certificate chain was null or empty");
         }
-        if (authType == null || authType.isEmpty()) {
-            throw new IllegalArgumentException("AuthType was null or empty");
-        }
         
         if (!ThreadLocalX509TrustEngineContext.haveCurrent()) {
             throw new CertificateException("Trust of X509Certificate could not be established, "
                     + "ThreadLocalX509TrustEngineContext is not populated");
         }
         
+        LOG.trace("Evaluating X509Certificate[] chain against ThreadLocalX509TrustEngineContext");
+        
         if (performTrustEval(chain,
                 ThreadLocalX509TrustEngineContext.getTrustEngine(),
                 ThreadLocalX509TrustEngineContext.getCriteria())) {
@@ -89,10 +102,10 @@ public class ThreadLocalX509TrustManager implements X509TrustManager {
         } else {
             ThreadLocalX509TrustEngineContext.setTrusted(false);
             if (ThreadLocalX509TrustEngineContext.isFailureFatal()) {
-                log.debug("Credential evaluated as untrusted, failure indicated as fatal");
+                LOG.debug("Credential evaluated as untrusted, failure indicated as fatal");
                 throw new CertificateException("Trust engine could not establish trust of presented TLS credential");
             }
-            log.debug("Credential evaluated as untrusted, failure indicated as non-fatal");
+            LOG.debug("Credential evaluated as untrusted, failure indicated as non-fatal");
         }
     }
     
@@ -107,25 +120,25 @@ public class ThreadLocalX509TrustManager implements X509TrustManager {
      * 
      * @throws CertificateException if the trust of the certificate
      */
-    protected boolean performTrustEval(@Nonnull final X509Certificate[] chain,
+    private static boolean performTrustEval(@Nonnull final X509Certificate[] chain,
             @Nonnull final TrustEngine<? super X509Credential> trustEngine,
             @Nonnull final CriteriaSet criteriaSet) throws CertificateException {
         
-        log.debug("Attempting to evaluate server TLS credential against supplied TrustEngine and CriteriaSet");
+        LOG.debug("Attempting to evaluate server TLS credential against supplied TrustEngine and CriteriaSet");
         
         final X509Credential credential = extractCredential(chain);
         
-        log.trace("Saw trust engine of type: {}", trustEngine.getClass().getName());
+        LOG.trace("Saw trust engine of type: {}", trustEngine.getClass().getName());
 
         try {
             if (trustEngine.validate(credential, criteriaSet)) {
-                log.debug("Credential evaluated as trusted");
+                LOG.debug("Credential evaluated as trusted");
                 return true;
             }
-            log.debug("Credential evaluated as untrusted");
+            LOG.debug("Credential evaluated as untrusted");
             return false;
         } catch (final Throwable t) {
-            log.error("Fatal trust engine error evaluating credential", t);
+            LOG.error("Fatal trust engine error evaluating credential", t);
             return false;
         }
         
@@ -139,7 +152,7 @@ public class ThreadLocalX509TrustManager implements X509TrustManager {
      *          supplied supporting intermediate certificate chain (if any)
      * @throws CertificateException if credential data can not be extracted from the socket
      */
-    @Nonnull protected X509Credential extractCredential(@Nonnull @NotEmpty final X509Certificate[] chain)
+    @Nonnull private static X509Credential extractCredential(@Nonnull @NotEmpty final X509Certificate[] chain)
             throws CertificateException {
         
         final List<X509Certificate> certChain = Arrays.asList(chain);
diff --git a/opensaml-security-impl/src/main/java/org/opensaml/security/x509/tls/impl/ThreadLocalX509TrustManager.java b/opensaml-security-impl/src/main/java/org/opensaml/security/x509/tls/impl/ThreadLocalX509TrustManager.java
index 9e059030b..130a39dd8 100644
--- a/opensaml-security-impl/src/main/java/org/opensaml/security/x509/tls/impl/ThreadLocalX509TrustManager.java
+++ b/opensaml-security-impl/src/main/java/org/opensaml/security/x509/tls/impl/ThreadLocalX509TrustManager.java
@@ -19,22 +19,12 @@ package org.opensaml.security.x509.tls.impl;
 
 import java.security.cert.CertificateException;
 import java.security.cert.X509Certificate;
-import java.util.Arrays;
-import java.util.List;
 
-import javax.annotation.Nonnull;
-import javax.net.ssl.SSLSocket;
 import javax.net.ssl.X509TrustManager;
 
-import org.opensaml.security.trust.TrustEngine;
-import org.opensaml.security.x509.BasicX509Credential;
-import org.opensaml.security.x509.X509Credential;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import net.shibboleth.utilities.java.support.annotation.constraint.NotEmpty;
-import net.shibboleth.utilities.java.support.resolver.CriteriaSet;
-
 /**
  * An implementation of {@link X509TrustManager} which performs its evaluation using trust engine and criteria
  * instances available from {@link ThreadLocalX509TrustEngineContext}.
@@ -76,80 +66,10 @@ public class ThreadLocalX509TrustManager implements X509TrustManager {
         if (authType == null || authType.isEmpty()) {
             throw new IllegalArgumentException("AuthType was null or empty");
         }
-        
-        if (!ThreadLocalX509TrustEngineContext.haveCurrent()) {
-            throw new CertificateException("Trust of X509Certificate could not be established, "
-                    + "ThreadLocalX509TrustEngineContext is not populated");
-        }
-        
-        if (performTrustEval(chain,
-                ThreadLocalX509TrustEngineContext.getTrustEngine(),
-                ThreadLocalX509TrustEngineContext.getCriteria())) {
-            ThreadLocalX509TrustEngineContext.setTrusted(true);
-        } else {
-            ThreadLocalX509TrustEngineContext.setTrusted(false);
-            if (ThreadLocalX509TrustEngineContext.isFailureFatal()) {
-                log.debug("Credential evaluated as untrusted, failure indicated as fatal");
-                throw new CertificateException("Trust engine could not establish trust of presented TLS credential");
-            }
-            log.debug("Credential evaluated as untrusted, failure indicated as non-fatal");
-        }
-    }
-    
-    /**
-     * Perform trust evaluation on the specified certificate chain using the supplied trust engine and criteria.
-     * 
-     * @param chain the certificate chain to be evaluated
-     * @param trustEngine the trust engine
-     * @param criteriaSet the criteria set
-     * 
-     * @return true if certificate was established as trusted, false if not
-     * 
-     * @throws CertificateException if the trust of the certificate
-     */
-    protected boolean performTrustEval(@Nonnull final X509Certificate[] chain,
-            @Nonnull final TrustEngine<? super X509Credential> trustEngine,
-            @Nonnull final CriteriaSet criteriaSet) throws CertificateException {
-        
-        log.debug("Attempting to evaluate server TLS credential against supplied TrustEngine and CriteriaSet");
-        
-        final X509Credential credential = extractCredential(chain);
-        
-        log.trace("Saw trust engine of type: {}", trustEngine.getClass().getName());
 
-        try {
-            if (trustEngine.validate(credential, criteriaSet)) {
-                log.debug("Credential evaluated as trusted");
-                return true;
-            }
-            log.debug("Credential evaluated as untrusted");
-            return false;
-        } catch (final Throwable t) {
-            log.error("Fatal trust engine error evaluating credential", t);
-            return false;
-        }
-        
-    }
+        log.trace("Evaluating certificate chain against ThreadLocalX509TrustEngineContext data");
 
-    /**
-     * Extract the server TLS {@link X509Credential} from the supplied {@link SSLSocket}.
-     * 
-     * @param chain the chain of X509 certificates
-     * @return an X509Credential representing the entity certificate as well as the 
-     *          supplied supporting intermediate certificate chain (if any)
-     * @throws CertificateException if credential data can not be extracted from the socket
-     */
-    @Nonnull protected X509Credential extractCredential(@Nonnull @NotEmpty final X509Certificate[] chain)
-            throws CertificateException {
-        
-        final List<X509Certificate> certChain = Arrays.asList(chain);
-        
-        final X509Certificate entityCert = certChain.get(0);
-        
-        final BasicX509Credential credential = new BasicX509Credential(entityCert);
-        credential.setEntityCertificateChain(certChain);
-        
-        return credential;
+        ThreadLocalX509TrustEngineSupport.evaluate(chain);
     }
 
 }

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


More information about the commits mailing list