Improve workaround for CVE-2009-3555

On the plus side, it doesn't rely on an async event to close the connection
On the down side, I haven't yet found a way to log client initiated handshakes before they get closed

git-svn-id: https://svn.apache.org/repos/asf/tomcat/trunk@882320 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/java/org/apache/tomcat/util/net/jsse/JSSESocketFactory.java b/java/org/apache/tomcat/util/net/jsse/JSSESocketFactory.java
index ee2c234..7d1bddd 100644 (file)
--- a/java/org/apache/tomcat/util/net/jsse/JSSESocketFactory.java
+++ b/java/org/apache/tomcat/util/net/jsse/JSSESocketFactory.java
@@ -42,8 +42,6 @@ import java.util.Collection;
 import java.util.Vector;
 
 import javax.net.ssl.CertPathTrustManagerParameters;
-import javax.net.ssl.HandshakeCompletedEvent;
-import javax.net.ssl.HandshakeCompletedListener;
 import javax.net.ssl.KeyManager;
 import javax.net.ssl.KeyManagerFactory;
 import javax.net.ssl.ManagerFactoryParameters;
@@ -159,42 +157,23 @@ public class JSSESocketFactory
         SSLSocket asock = null;
         try {
              asock = (SSLSocket)socket.accept();
-             if (!allowUnsafeLegacyRenegotiation) {
-                 asock.addHandshakeCompletedListener(
-                         new DisableSslRenegotiation());
-             }
         } catch (SSLException e){
           throw new SocketException("SSL handshake error" + e.toString());
         }
         return asock;
     }
     
-    private static class DisableSslRenegotiation 
-            implements HandshakeCompletedListener {
-        private volatile boolean completed = false;
-
-        public void handshakeCompleted(HandshakeCompletedEvent event) {
-            if (completed) {
-                try {
-                    log.warn("SSL renegotiation is disabled, closing connection");
-                    event.getSession().invalidate();
-                    event.getSocket().close();
-                } catch (IOException e) {
-                    // ignore
-                }
-            }
-            completed = true;
-        }
-    }
-
-
     @Override
     public void handshake(Socket sock) throws IOException {
-        //we do getSession instead of startHandshake() so we can call this multiple times
+        // We do getSession instead of startHandshake() so we can call this multiple times
        SSLSession session = ((SSLSocket)sock).getSession();
         if (session.getCipherSuite().equals("SSL_NULL_WITH_NULL_NULL"))
                throw new IOException("SSL handshake failed. Ciper suite in SSL Session is SSL_NULL_WITH_NULL_NULL");
-       //((SSLSocket)sock).startHandshake();
+
+        if (!allowUnsafeLegacyRenegotiation) {
+            // Prevent futher handshakes by removing all cipher suites
+            ((SSLSocket) sock).setEnabledCipherSuites(new String[0]);
+        }
     }
 
     /*

diff --git a/java/org/apache/tomcat/util/net/jsse/JSSESupport.java b/java/org/apache/tomcat/util/net/jsse/JSSESupport.java
index 8924748..30c72ff 100644 (file)
--- a/java/org/apache/tomcat/util/net/jsse/JSSESupport.java
+++ b/java/org/apache/tomcat/util/net/jsse/JSSESupport.java
@@ -149,6 +149,15 @@ class JSSESupport implements SSLSupport, SSLSessionManager {
             ssl.setNeedClientAuth(true);
         }
 
+        if (ssl.getEnabledCipherSuites().length == 0) {
+            // Handshake is never going to be successful.
+            // Assume this is because handshakes are disabled
+            log.warn("SSL server initiated renegotiation is disabled, closing connection");
+            session.invalidate();
+            ssl.close();
+            return;
+        }
+
         InputStream in = ssl.getInputStream();
         int oldTimeout = ssl.getSoTimeout();
         ssl.setSoTimeout(1000);
@@ -171,10 +180,7 @@ class JSSESupport implements SSLSupport, SSLSessionManager {
                 break;
             }
         }
-        // If legacy re-negotiation is disabled, socked could be closed here 
-        if (!ssl.isClosed()) {
-            ssl.setSoTimeout(oldTimeout);
-        }
+        ssl.setSoTimeout(oldTimeout);
         if (listener.completed == false) {
             throw new SocketException("SSL Cert handshake timeout");
         }