Author: markt
Date: Thu Nov 19 22:06:43 2009
New Revision: 882320

URL: http://svn.apache.org/viewvc?rev=882320&view=rev
Log:
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

Modified:
    tomcat/trunk/java/org/apache/tomcat/util/net/jsse/JSSESocketFactory.java
    tomcat/trunk/java/org/apache/tomcat/util/net/jsse/JSSESupport.java

Modified: 
tomcat/trunk/java/org/apache/tomcat/util/net/jsse/JSSESocketFactory.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/net/jsse/JSSESocketFactory.java?rev=882320&r1=882319&r2=882320&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/util/net/jsse/JSSESocketFactory.java 
(original)
+++ tomcat/trunk/java/org/apache/tomcat/util/net/jsse/JSSESocketFactory.java 
Thu Nov 19 22:06:43 2009
@@ -42,8 +42,6 @@
 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 @@
         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]);
+        }
     }
 
     /*

Modified: tomcat/trunk/java/org/apache/tomcat/util/net/jsse/JSSESupport.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/net/jsse/JSSESupport.java?rev=882320&r1=882319&r2=882320&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/util/net/jsse/JSSESupport.java 
(original)
+++ tomcat/trunk/java/org/apache/tomcat/util/net/jsse/JSSESupport.java Thu Nov 
19 22:06:43 2009
@@ -149,6 +149,15 @@
             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 @@
                 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");
         }



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Reply via email to