Skip to content

Commit d20f72c

Browse files
author
Matthew Sackman
committed
Added a test to ensure that if the client does not trust the server then it blows up. This is caught by setting an exceptionHandler, which inexplicably, isn't exposed through the ConnectionFactory (which is also the strangest Factory I've ever seen as it's stateful). So had to add API to the factory to allow the exceptionHandler to be set (which I *hate* because it'll be reused for every new connection which means if it's stateful then you're screwed, but the alternative is to multiply out the 3 overloaded newConnection methods to to 6 which I hate more).
And of course the test itself is racy, hence the synchronisation between the threads and the potential sitting there for 10 seconds doing nothing if it doesn't blow up as expected, and if the thread keeps being spuriously woken up then it'll continually wait. However, it does indeed work for me.
1 parent fe7b9cc commit d20f72c

File tree

4 files changed

+179
-1
lines changed

4 files changed

+179
-1
lines changed

build.xml

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,10 +140,24 @@
140140
<arg value="-dname"/>
141141
<arg value="CN=test, OU=test, O=test, L=test, S=test, C=pluto"/>
142142
</exec>
143+
<exec executable="mktemp" outputproperty="CLIENT_KEYSTORE_EMPTY" failonerror="true" osfamily="unix">
144+
<arg value="-u"/>
145+
</exec>
146+
<exec executable="keytool" failonerror="true" osfamily="unix" inputstring="\n\n">
147+
<arg value="-genkey"/>
148+
<arg value="-keystore"/>
149+
<arg value="${CLIENT_KEYSTORE_EMPTY}"/>
150+
<arg value="-noprompt"/>
151+
<arg value="-storepass"/>
152+
<arg value="${CLIENT_KEYSTORE_PHRASE}"/>
153+
<arg value="-dname"/>
154+
<arg value="CN=test, OU=test, O=test, L=test, S=test, C=pluto"/>
155+
</exec>
143156
</target>
144157

145158
<target name="remove-client-keystore" if="SSL_AVAILABLE">
146159
<delete file="${CLIENT_KEYSTORE}" failonerror="false"/>
160+
<delete file="${CLIENT_KEYSTORE_EMPTY}" failonerror="false"/>
147161
</target>
148162

149163
<target name="test-prepare">
@@ -322,6 +336,7 @@
322336
fork="yes">
323337
<classpath refid="test.classpath"/>
324338
<jvmarg value="-Dkeystore.path=${CLIENT_KEYSTORE}"/>
339+
<jvmarg value="-Dkeystore.empty.path=${CLIENT_KEYSTORE_EMPTY}"/>
325340
<jvmarg value="-Dkeystore.phrase=${CLIENT_KEYSTORE_PHRASE}"/>
326341

327342
<formatter type="plain"/>

src/com/rabbitmq/client/ConnectionFactory.java

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,8 @@
4141
import javax.net.ssl.TrustManager;
4242

4343
import com.rabbitmq.client.impl.AMQConnection;
44+
import com.rabbitmq.client.impl.DefaultExceptionHandler;
45+
import com.rabbitmq.client.impl.ExceptionHandler;
4446
import com.rabbitmq.client.impl.FrameHandler;
4547
import com.rabbitmq.client.impl.SocketFrameHandler;
4648

@@ -56,6 +58,11 @@ public class ConnectionFactory {
5658
*/
5759
private SocketFactory _factory = SocketFactory.getDefault();
5860

61+
/**
62+
* Holds the ExceptionHandler used in new AMQConnections
63+
*/
64+
private ExceptionHandler _exceptionHandler = new DefaultExceptionHandler();
65+
5966
/**
6067
* Instantiate a ConnectionFactory with a default set of parameters.
6168
*/
@@ -97,6 +104,23 @@ public void setSocketFactory(SocketFactory factory) {
97104
_factory = factory;
98105
}
99106

107+
/**
108+
* Retrieve the ExceptionHandler used in new AMQConnections
109+
* @return
110+
*/
111+
public ExceptionHandler getExceptionHandler() {
112+
return _exceptionHandler;
113+
}
114+
115+
/**
116+
* Set the ExceptionHandler used in new AMQConnections
117+
*
118+
* @param handler The ExceptionHandler to use
119+
*/
120+
public void setExceptionHandler(ExceptionHandler handler) {
121+
_exceptionHandler = handler;
122+
}
123+
100124
/**
101125
* Convenience method for setting up a SSL socket factory, using
102126
* the DEFAULT_SSL_PROTOCOL and a trusting TrustManager.
@@ -173,7 +197,7 @@ private Connection newConnection(Address[] addrs,
173197
redirectCount = 0;
174198
boolean allowRedirects = redirectCount < maxRedirects;
175199
try {
176-
return new AMQConnection(_params, !allowRedirects, frameHandler);
200+
return new AMQConnection(_params, !allowRedirects, frameHandler, _exceptionHandler);
177201
} catch (RedirectException e) {
178202
if (!allowRedirects) {
179203
//this should never happen with a well-behaved server
Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,138 @@
1+
// The contents of this file are subject to the Mozilla Public License
2+
// Version 1.1 (the "License"); you may not use this file except in
3+
// compliance with the License. You may obtain a copy of the License at
4+
// http://www.mozilla.org/MPL/
5+
//
6+
// Software distributed under the License is distributed on an "AS IS"
7+
// basis, WITHOUT WARRANTY OF ANY KIND, either express or implied. See the
8+
// License for the specific language governing rights and limitations
9+
// under the License.
10+
//
11+
// The Original Code is RabbitMQ.
12+
//
13+
// The Initial Developers of the Original Code are LShift Ltd,
14+
// Cohesive Financial Technologies LLC, and Rabbit Technologies Ltd.
15+
//
16+
// Portions created before 22-Nov-2008 00:00:00 GMT by LShift Ltd,
17+
// Cohesive Financial Technologies LLC, or Rabbit Technologies Ltd
18+
// are Copyright (C) 2007-2008 LShift Ltd, Cohesive Financial
19+
// Technologies LLC, and Rabbit Technologies Ltd.
20+
//
21+
// Portions created by LShift Ltd are Copyright (C) 2007-2009 LShift
22+
// Ltd. Portions created by Cohesive Financial Technologies LLC are
23+
// Copyright (C) 2007-2009 Cohesive Financial Technologies
24+
// LLC. Portions created by Rabbit Technologies Ltd are Copyright
25+
// (C) 2007-2009 Rabbit Technologies Ltd.
26+
//
27+
// All Rights Reserved.
28+
//
29+
// Contributor(s): ______________________________________.
30+
//
31+
package com.rabbitmq.client.test.ssl;
32+
33+
import java.io.FileInputStream;
34+
import java.io.IOException;
35+
import java.security.KeyManagementException;
36+
import java.security.KeyStore;
37+
import java.security.KeyStoreException;
38+
import java.security.NoSuchAlgorithmException;
39+
import java.security.UnrecoverableKeyException;
40+
import java.security.cert.CertificateException;
41+
42+
import javax.net.ssl.KeyManagerFactory;
43+
import javax.net.ssl.SSLContext;
44+
import javax.net.ssl.TrustManagerFactory;
45+
46+
import com.rabbitmq.client.Channel;
47+
import com.rabbitmq.client.Connection;
48+
import com.rabbitmq.client.ConnectionFactory;
49+
import com.rabbitmq.client.Consumer;
50+
import com.rabbitmq.client.impl.ExceptionHandler;
51+
52+
/**
53+
* Test for bug 19356 - SSL Support in rabbitmq
54+
*
55+
*/
56+
public class BadVerifiedConnection extends UnverifiedConnection {
57+
private boolean exceptionCaught = false;
58+
59+
public void openConnection()
60+
throws IOException
61+
{
62+
final Object lock = new Object();
63+
64+
connectionFactory = new ConnectionFactory();
65+
connectionFactory.setExceptionHandler(new ExceptionHandler() {
66+
67+
public void handleConsumerException(Channel channel,
68+
Throwable exception, Consumer consumer, String consumerTag,
69+
String methodName) {
70+
}
71+
72+
public void handleReturnListenerException(Channel channel,
73+
Throwable exception) {
74+
}
75+
76+
public void handleUnexpectedConnectionDriverException(
77+
Connection conn, Throwable exception) {
78+
synchronized (lock) {
79+
exceptionCaught = true;
80+
lock.notifyAll();
81+
}
82+
}});
83+
84+
try {
85+
String keystorePath = System.getProperty("keystore.empty.path");
86+
assertNotNull(keystorePath);
87+
String keystorePasswd = System.getProperty("keystore.phrase");
88+
assertNotNull(keystorePasswd);
89+
char [] passphrase = keystorePasswd.toCharArray();
90+
91+
KeyStore ks = KeyStore.getInstance("JKS");
92+
ks.load(new FileInputStream(keystorePath), passphrase);
93+
94+
KeyManagerFactory kmf = KeyManagerFactory.getInstance("SunX509");
95+
kmf.init(ks, passphrase);
96+
97+
TrustManagerFactory tmf = TrustManagerFactory.getInstance("SunX509");
98+
tmf.init(ks);
99+
100+
SSLContext c = SSLContext.getInstance("SSLv3");
101+
c.init(kmf.getKeyManagers(), tmf.getTrustManagers(), null);
102+
103+
connectionFactory.useSslProtocol(c);
104+
} catch (NoSuchAlgorithmException ex) {
105+
throw new IOException(ex.toString());
106+
} catch (KeyManagementException ex) {
107+
throw new IOException(ex.toString());
108+
} catch (KeyStoreException ex) {
109+
throw new IOException(ex.toString());
110+
} catch (CertificateException ex) {
111+
throw new IOException(ex.toString());
112+
} catch (UnrecoverableKeyException ex) {
113+
throw new IOException(ex.toString());
114+
}
115+
116+
if (connection == null) {
117+
try {
118+
connection = connectionFactory.newConnection("localhost", 5671);
119+
fail();
120+
} catch (Throwable e) {
121+
synchronized (lock) {
122+
while (! exceptionCaught) {
123+
try {
124+
lock.wait(10000);
125+
if (! exceptionCaught) {
126+
fail();
127+
}
128+
} catch (InterruptedException e1) {
129+
}
130+
}
131+
}
132+
}
133+
}
134+
}
135+
136+
public void openChannel() {}
137+
public void testSSL() {}
138+
}

test/src/com/rabbitmq/client/test/ssl/SSLTests.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ public static TestSuite suite() {
3939
TestSuite suite = new TestSuite("ssl");
4040
suite.addTestSuite(UnverifiedConnection.class);
4141
suite.addTestSuite(VerifiedConnection.class);
42+
suite.addTestSuite(BadVerifiedConnection.class);
4243
return suite;
4344
}
4445
}

0 commit comments

Comments
 (0)