Author: markt Date: Wed Dec 16 16:31:54 2009 New Revision: 891292 URL: http://svn.apache.org/viewvc?rev=891292&view=rev Log: Alternative fix for CVE-2009-3555 SSL MITN The previous approach used an async callback to close the socket. It is technically possible an attack may succeed before the socket is closed Note: The new patch only logs failed server initiated negotiations
Modified: tomcat/tc6.0.x/trunk/STATUS.txt tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/net/jsse/JSSESocketFactory.java tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/net/jsse/JSSESupport.java tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml Modified: tomcat/tc6.0.x/trunk/STATUS.txt URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/STATUS.txt?rev=891292&r1=891291&r2=891292&view=diff ============================================================================== --- tomcat/tc6.0.x/trunk/STATUS.txt (original) +++ tomcat/tc6.0.x/trunk/STATUS.txt Wed Dec 16 16:31:54 2009 @@ -307,14 +307,6 @@ +1: markt, jim -1: -* Alternative fix for CVE-2009-3555 SSL MITN - The current patch uses an async callback to close the socket. It is - technically possible an attack may suceed before the socket is closed - The new patch only logs failed server initiated negotiations - http://people.apache.org/~markt/patches/2009-11-20-cve2009-3555-v2.patch - +1: markt, jim, jfclere - -1: - * Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=48252 Patch attached to BZ +1: fhanik, markt, jim Modified: tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/net/jsse/JSSESocketFactory.java URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/net/jsse/JSSESocketFactory.java?rev=891292&r1=891291&r2=891292&view=diff ============================================================================== --- tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/net/jsse/JSSESocketFactory.java (original) +++ tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/net/jsse/JSSESocketFactory.java Wed Dec 16 16:31:54 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; @@ -152,10 +150,6 @@ SSLSocket asock = null; try { asock = (SSLSocket)socket.accept(); - if (!allowUnsafeLegacyRenegotiation) { - asock.addHandshakeCompletedListener( - new DisableSslRenegotiation()); - } configureClientAuth(asock); } catch (SSLException e){ throw new SocketException("SSL handshake error" + e.toString()); @@ -163,27 +157,13 @@ 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; - } - } - - public void handshake(Socket sock) throws IOException { ((SSLSocket)sock).startHandshake(); + + if (!allowUnsafeLegacyRenegotiation) { + // Prevent futher handshakes by removing all cipher suites + ((SSLSocket) sock).setEnabledCipherSuites(new String[0]); + } } /* Modified: tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/net/jsse/JSSESupport.java URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/net/jsse/JSSESupport.java?rev=891292&r1=891291&r2=891292&view=diff ============================================================================== --- tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/net/jsse/JSSESupport.java (original) +++ tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/net/jsse/JSSESupport.java Wed Dec 16 16:31:54 2009 @@ -148,6 +148,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); @@ -170,10 +179,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"); } Modified: tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml?rev=891292&r1=891291&r2=891292&view=diff ============================================================================== --- tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml (original) +++ tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml Wed Dec 16 16:31:54 2009 @@ -349,7 +349,7 @@ determine if SSL should be used. (fhanik) </fix> <fix> - Provide a workaround for CVE-2009-3555, the TLS renegotiation issue for + Provide a workaround for CVE-2009-3555, the TLS renegotiation issue, for the default Blocking IO Java connector. </fix> <fix> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org