Skip to content

Commit

Permalink
Improve workaround for CVE-2009-3555
Browse files Browse the repository at this point in the history
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
  • Loading branch information
markt-asf committed Nov 19, 2009
1 parent c839120 commit 3d315ac
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 31 deletions.
33 changes: 6 additions & 27 deletions java/org/apache/tomcat/util/net/jsse/JSSESocketFactory.java
Expand Up @@ -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;
Expand Down Expand Up @@ -159,42 +157,23 @@ public Socket acceptSocket(ServerSocket socket)
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]);
}
}

/*
Expand Down
14 changes: 10 additions & 4 deletions java/org/apache/tomcat/util/net/jsse/JSSESupport.java
Expand Up @@ -149,6 +149,15 @@ protected void handShake() throws IOException {
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);
Expand All @@ -171,10 +180,7 @@ protected void handShake() throws IOException {
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");
}
Expand Down

0 comments on commit 3d315ac

Please sign in to comment.