[java-opensaml] 01/03: OSJ-265: TLS socket factory clears client TLS credential too early

[Brent Putman]

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

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

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

commit 76c3f54dc7ac49832137d89f7c199df34fb59194
Author: Brent Putman <putmanb at georgetown.edu>
AuthorDate: Wed Mar 13 22:55:43 2019 -0400

    OSJ-265: TLS socket factory clears client TLS credential too early
---
 .../impl/SecurityEnhancedTLSSocketFactory.java     | 37 +++++++++------
 .../ThreadLocalClientTLSCredentialHandler.java     | 55 ++++++++++++++++++++++
 2 files changed, 78 insertions(+), 14 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 98dea7d..78b7f04 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
@@ -32,6 +32,7 @@ import javax.net.ssl.SSLSession;
 import javax.net.ssl.SSLSocket;
 
 import org.apache.http.HttpHost;
+import org.apache.http.client.protocol.HttpClientContext;
 import org.apache.http.conn.socket.LayeredConnectionSocketFactory;
 import org.apache.http.conn.ssl.X509HostnameVerifier;
 import org.apache.http.protocol.HttpContext;
@@ -47,6 +48,7 @@ import org.opensaml.security.x509.tls.impl.ThreadLocalX509CredentialContext;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import net.shibboleth.utilities.java.support.httpclient.HttpClientSupport;
 import net.shibboleth.utilities.java.support.logic.Constraint;
 import net.shibboleth.utilities.java.support.primitive.DeprecationSupport;
 import net.shibboleth.utilities.java.support.primitive.DeprecationSupport.ObjectType;
@@ -117,6 +119,10 @@ import net.shibboleth.utilities.java.support.resolver.CriteriaSet;
  */
 public class SecurityEnhancedTLSSocketFactory implements LayeredConnectionSocketFactory {
     
+    /** Instance of {@link ThreadLocalClientTLSCredentialHandler} to use.  */
+    private static final ThreadLocalClientTLSCredentialHandler CLIENT_TLS_HANDLER =
+            new ThreadLocalClientTLSCredentialHandler();
+
     /** Logger. */
     private final Logger log = LoggerFactory.getLogger(SecurityEnhancedTLSSocketFactory.class);
     
@@ -394,30 +400,33 @@ public class SecurityEnhancedTLSSocketFactory implements LayeredConnectionSocket
             log.trace("HttpContext was null, skipping thread-local setup");
             return;
         }
-        if (!ThreadLocalX509CredentialContext.haveCurrent()) {
-            final X509Credential credential = 
-                    (X509Credential) context.getAttribute(
-                            HttpClientSecurityConstants.CONTEXT_KEY_CLIENT_TLS_CREDENTIAL);
-            if (credential != null) {
-                log.trace("Loading ThreadLocalX509CredentialContext with client TLS credential: {}", credential);
-                ThreadLocalX509CredentialContext.loadCurrent(credential);
-            } else {
-                log.trace("HttpContext did not contain a client TLS credential, nothing to do");
+
+        final X509Credential credential =
+                (X509Credential) context.getAttribute(HttpClientSecurityConstants.CONTEXT_KEY_CLIENT_TLS_CREDENTIAL);
+        if (credential != null) {
+            log.trace("Loading ThreadLocalX509CredentialContext with client TLS credential: {}", credential);
+            if (ThreadLocalX509CredentialContext.haveCurrent()) {
+                log.trace("ThreadLocalX509CredentialContext was already loaded with client TLS credential, "
+                        + "will be overwritten with credential from HttpContext");
             }
+            ThreadLocalX509CredentialContext.loadCurrent(credential);
         } else {
-            log.trace("ThreadLocalX509CredentialContext was already loaded with client TLS credential, skipping setup");
+            log.trace("HttpContext did not contain a client TLS credential, nothing to do");
         }
+
     }
     
     /**
-     * Clear the {@link ThreadLocalX509CredentialContext} of the client TLS credential obtained from 
-     * the {@link HttpContext}.
+     * Schedule the deferred clearing of the {@link ThreadLocalX509CredentialContext} of the client TLS credential
+     * obtained from the {@link HttpContext}.
      * 
      * @param context the HttpContext instance
      */
     protected void teardown(@Nullable final HttpContext context) {
-        log.trace("Clearing thread-local client TLS X509Credential");
-        ThreadLocalX509CredentialContext.clearCurrent();
+        if (ThreadLocalX509CredentialContext.haveCurrent()) {
+            log.trace("Scheduling deferred clearing of thread-local client TLS X509Credential");
+            HttpClientSupport.addDynamicContextHandlerLast(HttpClientContext.adapt(context), CLIENT_TLS_HANDLER);
+        }
     }
 
 }
diff --git a/opensaml-security-impl/src/main/java/org/opensaml/security/httpclient/impl/ThreadLocalClientTLSCredentialHandler.java b/opensaml-security-impl/src/main/java/org/opensaml/security/httpclient/impl/ThreadLocalClientTLSCredentialHandler.java
new file mode 100644
index 0000000..877c87c
--- /dev/null
+++ b/opensaml-security-impl/src/main/java/org/opensaml/security/httpclient/impl/ThreadLocalClientTLSCredentialHandler.java
@@ -0,0 +1,55 @@
+/*
+ * Licensed to the University Corporation for Advanced Internet Development,
+ * Inc. (UCAID) under one or more contributor license agreements.  See the
+ * NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The UCAID licenses this file to You under the Apache
+ * License, Version 2.0 (the "License"); you may not use this file except in
+ * compliance with the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.opensaml.security.httpclient.impl;
+
+import java.io.IOException;
+
+import javax.annotation.Nonnull;
+
+import org.apache.http.client.methods.HttpUriRequest;
+import org.apache.http.client.protocol.HttpClientContext;
+import org.opensaml.security.x509.tls.impl.ThreadLocalX509CredentialContext;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import net.shibboleth.utilities.java.support.httpclient.HttpClientContextHandler;
+
+/**
+ * An implementation of {@link HttpClientContextHandler} which clears the thread local client TLS credential
+ * held by {@link ThreadLocalX509CredentialContext}.
+ */
+public class ThreadLocalClientTLSCredentialHandler implements HttpClientContextHandler {
+    
+    /** Logger. */
+    private final Logger log = LoggerFactory.getLogger(ThreadLocalClientTLSCredentialHandler.class);
+
+    /** {@inheritDoc} */
+    public void invokeBefore(@Nonnull final HttpClientContext context, @Nonnull final HttpUriRequest request)
+            throws IOException {
+        // Do nothing here
+        
+    }
+
+    /** {@inheritDoc} */
+    public void invokeAfter(@Nonnull final HttpClientContext context, @Nonnull final HttpUriRequest request)
+            throws IOException {
+        log.trace("Clearing thread-local client TLS X509Credential");
+        ThreadLocalX509CredentialContext.clearCurrent();
+    }
+
+}

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