From f0731cc17e6f1edfa2c5733ac10faa42ff250e5a Mon Sep 17 00:00:00 2001 From: markt Date: Thu, 19 Nov 2009 22:06:43 +0000 Subject: [PATCH] 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 --- .../tomcat/util/net/jsse/JSSESocketFactory.java | 33 ++++------------------ .../apache/tomcat/util/net/jsse/JSSESupport.java | 14 ++++++--- 2 files changed, 16 insertions(+), 31 deletions(-) diff --git a/java/org/apache/tomcat/util/net/jsse/JSSESocketFactory.java b/java/org/apache/tomcat/util/net/jsse/JSSESocketFactory.java index ee2c234af..7d1bdddf4 100644 --- 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 89247486d..30c72ff9e 100644 --- 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"); } -- 2.11.0