Author: jfrederic.clere(a)jboss.com Date: 2009-11-17 09:18:11 -0500 (Tue, 17 Nov 2009) New Revision: 1269 Added: trunk/test/java/org/apache/catalina/ trunk/test/java/org/apache/catalina/startup/ trunk/test/java/org/apache/catalina/startup/TestTomcatSSL.java trunk/test/java/org/apache/catalina/startup/TomcatBaseTest.java trunk/test/java/org/apache/catalina/startup/test.keystore trunk/test/webapps/simple/ trunk/test/webapps/simple/index.html Modified: trunk/java/org/apache/tomcat/util/net/jsse/JSSESocketFactory.java trunk/java/org/apache/tomcat/util/net/jsse/JSSESupport.java trunk/test/build.xml trunk/webapps/docs/changelog.xml trunk/webapps/docs/config/http.xml Log: Fix for cve-2009-3555. Modified: trunk/java/org/apache/tomcat/util/net/jsse/JSSESocketFactory.java =================================================================== --- trunk/java/org/apache/tomcat/util/net/jsse/JSSESocketFactory.java 2009-11-13 18:00:41 UTC (rev 1268) +++ trunk/java/org/apache/tomcat/util/net/jsse/JSSESocketFactory.java 2009-11-17 14:18:11 UTC (rev 1269) @@ -42,6 +42,8 @@ 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; @@ -99,6 +101,7 @@ protected String clientAuth = "false"; protected SSLServerSocketFactory sslProxy = null; protected String[] enabledCiphers; + protected boolean allowUnsafeLegacyRenegotiation = false; /** * Flag to state that we require client authentication. @@ -149,13 +152,36 @@ 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()); } 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(); } @@ -447,6 +473,9 @@ enabledCiphers = getEnabledCiphers(requestedCiphers, sslProxy.getSupportedCipherSuites()); + allowUnsafeLegacyRenegotiation = + "true".equals(attributes.get("allowUnsafeLegacyRenegotiation")); + // Check the SSL config is OK checkConfig(); Modified: trunk/java/org/apache/tomcat/util/net/jsse/JSSESupport.java =================================================================== --- trunk/java/org/apache/tomcat/util/net/jsse/JSSESupport.java 2009-11-13 18:00:41 UTC (rev 1268) +++ trunk/java/org/apache/tomcat/util/net/jsse/JSSESupport.java 2009-11-17 14:18:11 UTC (rev 1269) @@ -170,7 +170,10 @@ break; } } - ssl.setSoTimeout(oldTimeout); + // If legacy re-negotiation is disabled, socked could be closed here + if (!ssl.isClosed()) { + ssl.setSoTimeout(oldTimeout); + } if (listener.completed == false) { throw new SocketException("SSL Cert handshake timeout"); } Modified: trunk/test/build.xml =================================================================== --- trunk/test/build.xml 2009-11-13 18:00:41 UTC (rev 1268) +++ trunk/test/build.xml 2009-11-17 14:18:11 UTC (rev 1269) @@ -33,6 +33,8 @@ <property file="build.properties.default"/> <property name="test.classes" value="${basedir}/output/classes"/> + <property name="tomcat.classes" value="${basedir}/../output/classes"/> + <property name="tomcat.tmp" value="${basedir}/output/classes/output/"/> <property name="compile.source" value="1.5"/> @@ -41,6 +43,7 @@ <path id="jbossweb.test.classpath"> <pathelement location="${test.classes}"/> <pathelement location="${junit.jar}"/> + <pathelement location="${tomcat.classes}"/> </path> <target name="compile" depends="download"> @@ -56,6 +59,7 @@ optimize="${compile.optimize}"> <classpath refid="jbossweb.test.classpath" /> <include name="java/org/jboss/web/cookies/**" /> + <include name="java/org/apache/catalina/startup/**" /> </javac> </target> @@ -68,6 +72,21 @@ </java> </target> + <target name="ssl" depends="compile"> + <mkdir dir="${tomcat.tmp}" /> + <mkdir dir="${tomcat.tmp}/build/webapps/examples" /> + <copy todir="${tomcat.tmp}"> + <fileset dir="${basedir}/java/org/apache/catalina/startup" includes="*.keystore" /> + </copy> + <copy todir="${tomcat.tmp}/build/webapps/examples"> + <fileset dir="${basedir}/webapps/simple" includes="*.html" /> + </copy> + <java dir="${test.classes}" classname="${test.runner}" fork="yes"> + <arg value="org.apache.catalina.startup.TestTomcatSSL"/> + <classpath refid="jbossweb.test.classpath"/> + </java> + </target> + <!-- Download and dependency building --> <target name="proxyflags"> <!-- check proxy parameters. --> Added: trunk/test/java/org/apache/catalina/startup/TestTomcatSSL.java =================================================================== --- trunk/test/java/org/apache/catalina/startup/TestTomcatSSL.java (rev 0) +++ trunk/test/java/org/apache/catalina/startup/TestTomcatSSL.java 2009-11-17 14:18:11 UTC (rev 1269) @@ -0,0 +1,251 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.catalina.startup; + +import java.io.File; +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; +import java.security.cert.X509Certificate; + +import javax.net.ssl.HandshakeCompletedEvent; +import javax.net.ssl.HandshakeCompletedListener; +import javax.net.ssl.SSLContext; +import javax.net.ssl.SSLSocket; +import javax.net.ssl.SSLSocketFactory; +import javax.net.ssl.TrustManager; +import javax.net.ssl.X509TrustManager; + +import org.apache.catalina.connector.Connector; +import org.apache.tomcat.util.buf.ByteChunk; + +/** + * Requires test.keystore (checked in), generated with: + * keytool -genkey -alias tomcat -keyalg RSA + * pass: changeit + * CN: localhost ( for hostname validation ) + */ +public class TestTomcatSSL extends TomcatBaseTest { + static TrustManager[] trustAllCerts = new TrustManager[] { + new X509TrustManager() { + public X509Certificate[] getAcceptedIssuers() { + return null; + } + public void checkClientTrusted(X509Certificate[] certs, + String authType) { + // NOOP - Trust everything + } + public void checkServerTrusted(X509Certificate[] certs, + String authType) { + // NOOP - Trust everything + } + } + }; + + private void initSsl(Tomcat tomcat, boolean nio) throws Exception { + if (nio) { + Connector connector = + new Connector("org.apache.coyote.http11.Http11NioProtocol"); + connector.setPort(getPort()); + tomcat.getService().addConnector(connector); + tomcat.setConnector(connector); + tomcat.getConnector().setSecure(true); + } else { + tomcat.getConnector().setSecure(true); + } + tomcat.getConnector().setProperty("SSLEnabled", "true"); + tomcat.getConnector().setProperty("sslProtocol", + "tls"); + // test runs in output/tmp + tomcat.getConnector().setAttribute("keystore", + "../test.keystore"); + } + + public void testSimpleSsl() throws Exception { + simpleSsl(false); + } + + // No Nio in jbossweb + // public void testSimpleSslNio() throws Exception { + // simpleSsl(true); + // } + + public void simpleSsl(boolean nio) throws Exception { + // Install the all-trusting trust manager so https:// works + // with unsigned certs. + + try { + SSLContext sc = SSLContext.getInstance("SSL"); + sc.init(null, trustAllCerts, new java.security.SecureRandom()); + javax.net.ssl.HttpsURLConnection.setDefaultSSLSocketFactory( + sc.getSocketFactory()); + } catch (Exception e) { + e.printStackTrace(); + } + + Tomcat tomcat = getTomcatInstance(); + + File appDir = + new File("output/build/webapps/examples"); + tomcat.addWebapp(null, "/examples", appDir.getAbsolutePath()); + + initSsl(tomcat, nio); + + tomcat.start(); + ByteChunk res = getUrl("https://localhost:" + getPort() + + "/examples/"); + assertTrue(res.toString().indexOf("Hello World!") > 0); + } + + boolean handshakeDone = false; + + public void testRenegotiateFail() throws Exception { + renegotiateFail(false); + } + + public void renegotiateFail(boolean nio) throws Exception { + Tomcat tomcat = getTomcatInstance(); + + File appDir = + new File("output/build/webapps/examples"); + // app dir is relative to server home + tomcat.addWebapp(null, "/examples", appDir.getAbsolutePath()); + + initSsl(tomcat, nio); + // Default - MITM not enabled + + tomcat.start(); + SSLContext sslCtx = SSLContext.getInstance("TLS"); + sslCtx.init(null, trustAllCerts, new java.security.SecureRandom()); + SSLSocketFactory socketFactory = sslCtx.getSocketFactory(); + SSLSocket socket = (SSLSocket) socketFactory.createSocket("localhost", getPort()); + + socket.addHandshakeCompletedListener(new HandshakeCompletedListener() { + @Override + public void handshakeCompleted(HandshakeCompletedEvent event) { + handshakeDone = true; + } + }); + + OutputStream os = socket.getOutputStream(); + os.write("GET /examples/ HTTP/1.1\n".getBytes()); + os.flush(); + + InputStream is = socket.getInputStream(); + + socket.startHandshake(); + handshakeDone = false; + byte[] b = new byte[0]; + int maxTries = 5; // 5 sec should be enough - in NIO we'll timeout + socket.setSoTimeout(1000); + for (int i = 0; i < maxTries; i++) { + try { + is.read(b); + } catch (IOException e) { + // timeout + } + if (handshakeDone) { + break; + } + } + os = socket.getOutputStream(); + if (!handshakeDone) { + // success - we timedout without handshake + return; + } + try { + os.write("Host: localhost\n\n".getBytes()); + } catch (IOException ex) { + // success - connection closed + return; + } + + fail("Re-negotiation worked"); + + } + + public void testRenegotiateWorks() throws Exception { + renegotiateWorks(false); + } + + + // Re-negotiation not implemented in NIO + // public void testRenegotiateWorksNio() throws Exception { + // renegotiateWorks(true); + // } + + // public void testRenegotiateFailNio() throws Exception { + // renegotiateFail(true); + // } + + + public void renegotiateWorks(boolean nio) throws Exception { + Tomcat tomcat = getTomcatInstance(); + + File appDir = + new File("output/build/webapps/examples"); + // app dir is relative to server home + tomcat.addWebapp(null, "/examples", appDir.getAbsolutePath()); + + initSsl(tomcat, nio); + // Enable MITM attack + tomcat.getConnector().setAttribute("allowUnsafeLegacyRenegotiation", "true"); + + tomcat.start(); + SSLContext sslCtx = SSLContext.getInstance("TLS"); + sslCtx.init(null, trustAllCerts, new java.security.SecureRandom()); + SSLSocketFactory socketFactory = sslCtx.getSocketFactory(); + SSLSocket socket = (SSLSocket) socketFactory.createSocket("localhost", getPort()); + + socket.addHandshakeCompletedListener(new HandshakeCompletedListener() { + @Override + public void handshakeCompleted(HandshakeCompletedEvent event) { + handshakeDone = true; + } + }); + + OutputStream os = socket.getOutputStream(); + os.write("GET /examples/ HTTP/1.1\n".getBytes()); + os.flush(); + + InputStream is = socket.getInputStream(); + + socket.startHandshake(); + handshakeDone = false; + byte[] b = new byte[0]; + int maxTries = 5; + socket.setSoTimeout(1000); + for (int i = 0; i < maxTries; i++) { + try { + is.read(b); + } catch (IOException e) { + // timeout + } + if (handshakeDone) { + break; + } + } + os = socket.getOutputStream(); + + try { + os.write("Host: localhost\n\n".getBytes()); + } catch (IOException ex) { + fail("Re-negotiation failed"); + } + + } +} Added: trunk/test/java/org/apache/catalina/startup/TomcatBaseTest.java =================================================================== --- trunk/test/java/org/apache/catalina/startup/TomcatBaseTest.java (rev 0) +++ trunk/test/java/org/apache/catalina/startup/TomcatBaseTest.java 2009-11-17 14:18:11 UTC (rev 1269) @@ -0,0 +1,137 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.catalina.startup; + +import java.io.BufferedInputStream; +import java.io.File; +import java.io.IOException; +import java.io.InputStream; +import java.io.PrintWriter; +import java.net.HttpURLConnection; +import java.net.URL; +import java.util.List; +import java.util.Map; + +import javax.servlet.ServletException; +import javax.servlet.http.HttpServlet; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + +import org.apache.tomcat.util.buf.ByteChunk; + +import junit.framework.TestCase; + +/** + * Base test case that provides a Tomcat instance for each test - mainly so we + * don't have to keep writing the cleanup code. + */ +public abstract class TomcatBaseTest extends TestCase { + private Tomcat tomcat; + private File tempDir; + private static int port = 8001; + + /** + * Make Tomcat instance accessible to sub-classes. + */ + public Tomcat getTomcatInstance() { + return tomcat; + } + + /** + * Sub-classes need to know port so they can connect + */ + public int getPort() { + return port; + } + + /** + * Sub-classes may want to add connectors on a new port + */ + public int getNextPort() { + port++; + return getPort(); + } + + @Override + public void setUp() throws Exception { + tempDir = new File("output/tmp"); + tempDir.mkdir(); + + tomcat = new Tomcat(); + tomcat.setBaseDir(tempDir.getAbsolutePath()); + tomcat.getHost().setAppBase(tempDir.getAbsolutePath() + "/webapps"); + + // If each test is running on same port - they + // may interfere with each other (on unix at least) + port++; + tomcat.setPort(port); + } + + @Override + public void tearDown() throws Exception { + tomcat.stop(); + ExpandWar.delete(tempDir); + } + + /** + * Simple Hello World servlet for use by test cases + */ + public static final class HelloWorldServlet extends HttpServlet { + + private static final long serialVersionUID = 1L; + + @Override + protected void doGet(HttpServletRequest req, HttpServletResponse resp) + throws ServletException, IOException { + PrintWriter out = resp.getWriter(); + out.print("<html><body><p>Hello World</p></body></html>"); + } + } + + + /** + * Wrapper for getting the response. + */ + public static ByteChunk getUrl(String path) throws IOException { + ByteChunk out = new ByteChunk(); + getUrl(path, out, null); + return out; + } + + public static int getUrl(String path, + ByteChunk out, + Map<String, List<String>> resHead) throws IOException { + URL url = new URL(path); + HttpURLConnection connection = + (HttpURLConnection) url.openConnection(); + connection.setReadTimeout(100000); + connection.connect(); + int rc = connection.getResponseCode(); + if (resHead != null) { + Map<String, List<String>> head = connection.getHeaderFields(); + resHead.putAll(head); + } + InputStream is = connection.getInputStream(); + BufferedInputStream bis = new BufferedInputStream(is); + byte[] buf = new byte[2048]; + int rd = 0; + while((rd = bis.read(buf)) > 0) { + out.append(buf, 0, rd); + } + return rc; + } +} Added: trunk/test/java/org/apache/catalina/startup/test.keystore =================================================================== (Binary files differ) Property changes on: trunk/test/java/org/apache/catalina/startup/test.keystore ___________________________________________________________________ Name: svn:mime-type + application/octet-stream Added: trunk/test/webapps/simple/index.html =================================================================== --- trunk/test/webapps/simple/index.html (rev 0) +++ trunk/test/webapps/simple/index.html 2009-11-17 14:18:11 UTC (rev 1269) @@ -0,0 +1 @@ +<h1>Hello World!</h1> Modified: trunk/webapps/docs/changelog.xml =================================================================== --- trunk/webapps/docs/changelog.xml 2009-11-13 18:00:41 UTC (rev 1268) +++ trunk/webapps/docs/changelog.xml 2009-11-17 14:18:11 UTC (rev 1269) @@ -22,6 +22,9 @@ <update> Servlet 3.0 API. (remm) </update> + <fix> + Fix CVE-2009-3555, man-in-the-middle attack in TLS protocol. (markt) + </fix> <update> Commons Pool 1.5.2. (markt) </update> Modified: trunk/webapps/docs/config/http.xml =================================================================== --- trunk/webapps/docs/config/http.xml 2009-11-13 18:00:41 UTC (rev 1268) +++ trunk/webapps/docs/config/http.xml 2009-11-17 14:18:11 UTC (rev 1269) @@ -501,6 +501,13 @@ TrustStore then you are using for the KeyStore.</p> </attribute> + <attribute name="allowUnsafeLegacyRenegotiation" required="false"> + <p>Is unsafe legacy TLS renegotiation allowed which is likely to expose + users to CVE-2009-3555, a man-in-the-middle vulnerability in the TLS + protocol that allows an attacker to inject arbitrary data into the user's + request. If not specified, a default of <code>false</code> is used.</p> + </attribute> + </attributes> <p>For more information, see the