You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2009/11/10 15:26:02 UTC
svn commit: r834477 - in /tomcat/trunk:
java/org/apache/tomcat/util/net/jsse/JSSESocketFactory.java
test/org/apache/catalina/startup/TestTomcatSSL.java
Author: markt
Date: Tue Nov 10 14:26:01 2009
New Revision: 834477
URL: http://svn.apache.org/viewvc?rev=834477&view=rev
Log:
Use a connector attribute rather than a system property to control renegotiation
Fix some trivial Eclispe warnings in the test
Don't try and invalidate the session in the client - an attacker probably won't do this
Add a test that checks the connector attribute can be used to enable renegotiation
Modified:
tomcat/trunk/java/org/apache/tomcat/util/net/jsse/JSSESocketFactory.java
tomcat/trunk/test/org/apache/catalina/startup/TestTomcatSSL.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=834477&r1=834476&r2=834477&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 Tue Nov 10 14:26:01 2009
@@ -95,9 +95,6 @@
private static final int defaultSessionCacheSize = 0;
private static final int defaultSessionTimeout = 86400;
- private static final boolean midmMode =
- "true".equals(System.getProperty("enable_ssl_mitm_vulnerability"));
-
static org.apache.juli.logging.Log log =
org.apache.juli.logging.LogFactory.getLog(JSSESocketFactory.class);
@@ -105,6 +102,7 @@
protected String clientAuth = "false";
protected SSLServerSocketFactory sslProxy = null;
protected String[] enabledCiphers;
+ protected boolean enableMitmVulnerability = false;
/**
* Flag to state that we require client authentication.
@@ -159,7 +157,7 @@
SSLSocket asock = null;
try {
asock = (SSLSocket)socket.accept();
- if (!midmMode) {
+ if (!enableMitmVulnerability) {
asock.addHandshakeCompletedListener(
new DisableSslRenegotiation());
}
@@ -492,6 +490,9 @@
getEnabledCiphers(requestedCiphers,
sslProxy.getSupportedCipherSuites());
+ enableMitmVulnerability =
+ "true".equals(attributes.get("enableMitmVulnerability"));
+
// Check the SSL config is OK
checkConfig();
Modified: tomcat/trunk/test/org/apache/catalina/startup/TestTomcatSSL.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/startup/TestTomcatSSL.java?rev=834477&r1=834476&r2=834477&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/catalina/startup/TestTomcatSSL.java (original)
+++ tomcat/trunk/test/org/apache/catalina/startup/TestTomcatSSL.java Tue Nov 10 14:26:01 2009
@@ -20,11 +20,11 @@
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.SSLSession;
import javax.net.ssl.SSLSocket;
import javax.net.ssl.SSLSocketFactory;
import javax.net.ssl.TrustManager;
@@ -41,12 +41,16 @@
public class TestTomcatSSL extends TomcatBaseTest {
static TrustManager[] trustAllCerts = new TrustManager[] {
new X509TrustManager() {
- public java.security.cert.X509Certificate[] getAcceptedIssuers() {
+ public X509Certificate[] getAcceptedIssuers() {
return null;
}
- public void checkClientTrusted(java.security.cert.X509Certificate[] certs, String authType) {
- }
- public void checkServerTrusted(java.security.cert.X509Certificate[] certs, String authType) {
+ public void checkClientTrusted(X509Certificate[] certs,
+ String authType) {
+ // NOOP - Trust everything
+ }
+ public void checkServerTrusted(X509Certificate[] certs,
+ String authType) {
+ // NOOP - Trust everything
}
}
};
@@ -63,10 +67,9 @@
public void testSimpleSsl() throws Exception {
- // Install the all-trusting trust manager so https:// works
+ // Install the all-trusting trust manager so https:// works
// with unsigned certs.
- // TODO: cleanup ?
try {
SSLContext sc = SSLContext.getInstance("SSL");
sc.init(null, trustAllCerts, new java.security.SecureRandom());
@@ -91,7 +94,7 @@
boolean handshakeDone = false;
- public void testReHandshake() throws Exception {
+ public void testRenegotiateFail() throws Exception {
Tomcat tomcat = getTomcatInstance();
File appDir =
@@ -100,6 +103,7 @@
tomcat.addWebapp(null, "/examples", appDir.getAbsolutePath());
initSsl(tomcat);
+ // Default - MITM not enabled
tomcat.start();
SSLContext sslCtx = SSLContext.getInstance("TLS");
@@ -120,8 +124,6 @@
InputStream is = socket.getInputStream();
- // Doesn't seem to work..
- socket.getSession().invalidate();
socket.startHandshake();
handshakeDone = false;
byte[] b = new byte[0];
@@ -137,7 +139,6 @@
break;
}
}
- SSLSession session = socket.getSession();
os = socket.getOutputStream();
try {
@@ -150,4 +151,61 @@
fail("Re-negotiation worked");
}
+
+ public void testRenegotiateWorks() 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);
+ // Enable MITM attack
+ tomcat.getConnector().setAttribute("enableMitmVulnerability", "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/servlets/servlet/HelloWorldExample HTTP/1.0\n".getBytes());
+ os.flush();
+
+ InputStream is = socket.getInputStream();
+
+ socket.startHandshake();
+ handshakeDone = false;
+ byte[] b = new byte[0];
+ int maxTries = 60; // 60 * 1000 = example 1 minute time out
+ 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");
+ }
+
+ }
+
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r834477 - in /tomcat/trunk: java/org/apache/tomcat/util/net/jsse/JSSESocketFactory.java
test/org/apache/catalina/startup/TestTomcatSSL.java
Posted by Mark Thomas <ma...@apache.org>.
Rainer Jung wrote:
> On 10.11.2009 16:32, Ian Darwin wrote:
>>> Isn't the naming a bit harsh? OpenSSl names it legacy renegotiation (to
>>> make it differ from the future renegotiation with TLS extension). So
>>> maybe enableLegacyRenegotiation would be better? Of course it wouldn't
>>> keep people from activating as much as the proposed name does, but on
>>> the other hand (unfortunately) there are valid use cases to activate it.
>>>
>> FWIW, the OpenBSD people have committed their change to their
>> OpenSSL library, and they used the name
>>
>> SSL3_FLAGS_ALLOW_UNSAFE_LEGACY_RENEGOTIATION
>
> Yes, you are right. Omitting the "Unsafe" part of it isn't good, so I
> would say either allowUnsafeLegacyRenegotiation or
> enableUnsafeLegacyRenegotiation.
I've already committed the first one :)
My tests all went well. I am in the process of creating patches for
6.0.x and 5.5.x.
Remy - are you able to roll a release? I'm guessing either later today
or early tomorrow.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r834477 - in /tomcat/trunk: java/org/apache/tomcat/util/net/jsse/JSSESocketFactory.java
test/org/apache/catalina/startup/TestTomcatSSL.java
Posted by Rainer Jung <ra...@kippdata.de>.
On 10.11.2009 16:32, Ian Darwin wrote:
>
>>
>> Isn't the naming a bit harsh? OpenSSl names it legacy renegotiation (to
>> make it differ from the future renegotiation with TLS extension). So
>> maybe enableLegacyRenegotiation would be better? Of course it wouldn't
>> keep people from activating as much as the proposed name does, but on
>> the other hand (unfortunately) there are valid use cases to activate it.
>>
> FWIW, the OpenBSD people have committed their change to their
> OpenSSL library, and they used the name
>
> SSL3_FLAGS_ALLOW_UNSAFE_LEGACY_RENEGOTIATION
Yes, you are right. Omitting the "Unsafe" part of it isn't good, so I
would say either allowUnsafeLegacyRenegotiation or
enableUnsafeLegacyRenegotiation.
Regards,
Rainer
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r834477 - in /tomcat/trunk: java/org/apache/tomcat/util/net/jsse/JSSESocketFactory.java
test/org/apache/catalina/startup/TestTomcatSSL.java
Posted by Ian Darwin <ia...@darwinsys.com>.
>
> Isn't the naming a bit harsh? OpenSSl names it legacy renegotiation (to
> make it differ from the future renegotiation with TLS extension). So
> maybe enableLegacyRenegotiation would be better? Of course it wouldn't
> keep people from activating as much as the proposed name does, but on
> the other hand (unfortunately) there are valid use cases to activate it.
>
FWIW, the OpenBSD people have committed their change to their
OpenSSL library, and they used the name
SSL3_FLAGS_ALLOW_UNSAFE_LEGACY_RENEGOTIATION
Ian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r834477 - in /tomcat/trunk: java/org/apache/tomcat/util/net/jsse/JSSESocketFactory.java
test/org/apache/catalina/startup/TestTomcatSSL.java
Posted by Mark Thomas <ma...@apache.org>.
Rainer Jung wrote:
>> + enableMitmVulnerability =
>> + "true".equals(attributes.get("enableMitmVulnerability"));
>> +
>> // Check the SSL config is OK
>> checkConfig();
>
> Isn't the naming a bit harsh? OpenSSl names it legacy renegotiation (to
> make it differ from the future renegotiation with TLS extension). So
> maybe enableLegacyRenegotiation would be better? Of course it wouldn't
> keep people from activating as much as the proposed name does, but on
> the other hand (unfortunately) there are valid use cases to activate it.
I'm happy with changing the name to be consistent with OpenSSL. More
consistency == less confusion which I think is a good thing.
I'm still doing testing but so far it looks good. If any one is able to
do their own testing please do so. The more eyes on this the better.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r834477 - in /tomcat/trunk: java/org/apache/tomcat/util/net/jsse/JSSESocketFactory.java
test/org/apache/catalina/startup/TestTomcatSSL.java
Posted by Rainer Jung <ra...@kippdata.de>.
On 10.11.2009 15:26, markt@apache.org wrote:
> @@ -105,6 +102,7 @@
> protected String clientAuth = "false";
> protected SSLServerSocketFactory sslProxy = null;
> protected String[] enabledCiphers;
> + protected boolean enableMitmVulnerability = false;
>
> /**
> * Flag to state that we require client authentication.
> @@ -492,6 +490,9 @@
> getEnabledCiphers(requestedCiphers,
> sslProxy.getSupportedCipherSuites());
>
> + enableMitmVulnerability =
> + "true".equals(attributes.get("enableMitmVulnerability"));
> +
> // Check the SSL config is OK
> checkConfig();
>
>
Isn't the naming a bit harsh? OpenSSl names it legacy renegotiation (to
make it differ from the future renegotiation with TLS extension). So
maybe enableLegacyRenegotiation would be better? Of course it wouldn't
keep people from activating as much as the proposed name does, but on
the other hand (unfortunately) there are valid use cases to activate it.
Regards,
Rainer
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org