Skip to content

Commit e717536

Browse files
jpwsuttonjpwsutton
authored
Patch contributed from Issue #28 (#166)
* Issue: #28 Bug: 469947 Implementing Patches contributed by Cristiano to fix Java SSL session resumption Signed-off-by: Cristiano De Alti <cristiano.dealti@eurotech.com> * Fixing Copyright header in test and changing to use Automated test properties. Signed-off-by: James Sutton <james.sutton@uk.ibm.com>
1 parent 7088ae9 commit e717536

7 files changed

Lines changed: 329 additions & 36 deletions

File tree

‎org.eclipse.paho.client.mqttv3.test/src/test/java/org/eclipse/paho/client/mqttv3/test/SSLSessionResumptionTest.java‎

Lines changed: 193 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,193 @@
1+
/**
2+
* Copyright (c) 2011, 2014 Eurotech and/or its affiliates
3+
*
4+
* All rights reserved. This program and the accompanying materials
5+
* are made available under the terms of the Eclipse Public License v1.0
6+
* which accompanies this distribution, and is available at
7+
* http://www.eclipse.org/legal/epl-v10.html
8+
*
9+
* Contributors:
10+
* Cristiano De Alti - Eurotech (Initial contribution)
11+
* James Sutton - IBM (Fixing Copyright header and adding getSocketFactory)
12+
*/
13+
14+
package org.eclipse.paho.client.mqttv3.test;
15+
16+
import java.io.IOException;
17+
import java.io.InputStream;
18+
import java.security.KeyStore;
19+
import java.security.SecureRandom;
20+
import java.security.cert.Certificate;
21+
import java.security.cert.CertificateFactory;
22+
import java.util.Arrays;
23+
import java.util.Enumeration;
24+
import java.util.logging.Level;
25+
import java.util.logging.Logger;
26+
27+
import javax.net.ssl.SSLContext;
28+
import javax.net.ssl.SSLSessionContext;
29+
import javax.net.ssl.SSLSocket;
30+
import javax.net.ssl.SSLSocketFactory;
31+
import javax.net.ssl.TrustManagerFactory;
32+
33+
import org.eclipse.paho.client.mqttv3.MqttClient;
34+
import org.eclipse.paho.client.mqttv3.MqttConnectOptions;
35+
import org.eclipse.paho.client.mqttv3.test.logging.LoggingUtilities;
36+
import org.eclipse.paho.client.mqttv3.test.properties.TestProperties;
37+
import org.eclipse.paho.client.mqttv3.test.utilities.Utility;
38+
import org.junit.BeforeClass;
39+
import org.junit.Test;
40+
41+
42+
/**
43+
* This test aims to run some basic SSL functionality tests of the MQTT client
44+
*/
45+
46+
public class SSLSessionResumptionTest {
47+
48+
static final Class<?> cclass = SSLSessionResumptionTest.class;
49+
private static final String className = cclass.getName();
50+
private static final Logger log = Logger.getLogger(className);
51+
52+
53+
private static String serverURI;
54+
private static String serverHost;
55+
private static int serverPort;
56+
private static final String certificateName = "iot.eclipse.org.crt";
57+
58+
59+
/**
60+
* @throws Exception
61+
*/
62+
@BeforeClass
63+
public static void setUpBeforeClass() throws Exception {
64+
65+
try {
66+
String methodName = Utility.getMethodName();
67+
LoggingUtilities.banner(log, cclass, methodName);
68+
serverURI = "ssl://" + TestProperties.getServerURI().getHost();
69+
serverHost = TestProperties.getServerURI().getHost();
70+
serverPort = TestProperties.getServerSSLPort();
71+
72+
}
73+
catch (Exception exception) {
74+
log.log(Level.SEVERE, "caught exception:", exception);
75+
throw exception;
76+
}
77+
}
78+
79+
/**
80+
* This test involves inspecting the SSL debug log
81+
* and the Wireshark capture.
82+
*
83+
* Paho defeats the default SSL session caching which
84+
* allows to have abbreviated SSL handshakes on
85+
* following connections.
86+
*
87+
* @throws Exception
88+
*/
89+
@Test
90+
public void testSSLSessionInvalidated() throws Exception {
91+
//System.setProperty("javax.net.debug", "all");
92+
93+
SSLSocketFactory factory = getSocketFactory(certificateName);
94+
95+
MqttConnectOptions options = new MqttConnectOptions();
96+
options.setServerURIs(new String[] {serverURI});
97+
options.setKeepAliveInterval(60);
98+
options.setSocketFactory(factory);
99+
100+
MqttClient mqttClient = new MqttClient(serverURI, MqttClient.generateClientId());
101+
102+
103+
log.info("Connecting to: " + serverURI);
104+
mqttClient.connect(options);
105+
106+
Thread.sleep(2000);
107+
108+
log.info("Disconnetting...");
109+
mqttClient.disconnect();
110+
111+
Thread.sleep(2000);
112+
113+
log.info("Connecting again... Paho will not be able to perform an abbreviated SSL handshake");
114+
mqttClient.connect(options);
115+
116+
Thread.sleep(2000);
117+
118+
log.info("Disconnetting...");
119+
mqttClient.disconnect();
120+
121+
}
122+
123+
/**
124+
* This test involves inspecting the SSL debug log
125+
* and the Wireshark capture.
126+
*
127+
* By default, Java caches SSL sessions which
128+
* allows to have abbreviated SSL handshakes on
129+
* following connections.
130+
*
131+
* @throws Exception
132+
*/
133+
@Test
134+
public void testSSLSessionCached() throws Exception {
135+
// System.setProperty("javax.net.debug", "all");
136+
137+
SSLSocketFactory factory = getSocketFactory(certificateName);
138+
139+
140+
log.info("Do handshake...");
141+
doHandshake(factory, serverHost, serverPort);
142+
143+
log.info("Done! Redo handshake... An abbreviated SSL handshake will be performed");
144+
doHandshake(factory, serverHost, serverPort);
145+
146+
log.info("Done!");
147+
}
148+
149+
private static void doHandshake(SSLSocketFactory factory, String host, int port) {
150+
SSLSocket socket = null;
151+
try {
152+
socket = (SSLSocket) factory.createSocket(host, port);
153+
154+
socket.startHandshake();
155+
156+
SSLSessionContext ctx = socket.getSession().getSessionContext();
157+
if (ctx != null) {
158+
Enumeration<byte[]> ids = ctx.getIds();
159+
while (ids.hasMoreElements()) {
160+
byte[] id = ids.nextElement();
161+
log.info("Session ID: " + Arrays.toString(id));
162+
log.info("Cypher suite: " + ctx.getSession(id).getCipherSuite());
163+
}
164+
} else {
165+
log.info("null SSLSessionContext");
166+
}
167+
} catch (Exception e) {
168+
e.printStackTrace();
169+
} finally {
170+
if (socket != null) {
171+
try {
172+
socket.close();
173+
} catch (IOException e) {
174+
e.printStackTrace();
175+
}
176+
}
177+
}
178+
}
179+
180+
private static SSLSocketFactory getSocketFactory(String certificateName) throws Exception {
181+
InputStream certStream = SSLSessionResumptionTest.class.getClassLoader().getResourceAsStream(certificateName);
182+
CertificateFactory certFactory = CertificateFactory.getInstance("X509");
183+
Certificate certificate = certFactory.generateCertificate(certStream);
184+
SSLContext sslContext = SSLContext.getInstance("TLS");
185+
TrustManagerFactory trustManagerFactory = TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm());
186+
KeyStore keyStore = KeyStore.getInstance(KeyStore.getDefaultType());
187+
keyStore.load(null);
188+
keyStore.setCertificateEntry("alias", certificate);
189+
trustManagerFactory.init(keyStore);
190+
sslContext.init(null, trustManagerFactory.getTrustManagers(), new SecureRandom());
191+
return sslContext.getSocketFactory();
192+
}
193+
}

‎org.eclipse.paho.client.mqttv3.test/src/test/resources/iot.eclipse.org.crt‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
-----BEGIN CERTIFICATE-----
2+
MIIEVDCCAwygAwIBAgIEVYPx2jANBgkqhkiG9w0BAQsFADCBhzELMAkGA1UEBhMC
3+
Q0ExJTAjBgNVBAoTHEVjbGlwc2Uub3JnIEZvdW5kYXRpb24sIEluYy4xFDASBgNV
4+
BAsTC2lvdC10ZXN0aW5nMQ8wDQYDVQQHEwZOZXBlYW4xEDAOBgNVBAgTB09udGFy
5+
aW8xGDAWBgNVBAMTD2lvdC5lY2xpcHNlLm9yZzAeFw0xNTA2MTkxMDQxMzFaFw0y
6+
MDA1MjMxMDQxMzRaMIGHMQswCQYDVQQGEwJDQTElMCMGA1UEChMcRWNsaXBzZS5v
7+
cmcgRm91bmRhdGlvbiwgSW5jLjEUMBIGA1UECxMLaW90LXRlc3RpbmcxDzANBgNV
8+
BAcTBk5lcGVhbjEQMA4GA1UECBMHT250YXJpbzEYMBYGA1UEAxMPaW90LmVjbGlw
9+
c2Uub3JnMIIBUjANBgkqhkiG9w0BAQEFAAOCAT8AMIIBOgKCATEA0xUc7iXRrDG3
10+
WBCM6kzBv7eDVJUj8fMt7yhVS+os3QwKXfirzvRn1kj3KQkBzR/Nkeqk4go5EkYI
11+
ZCvG3svmfbyxJaWxl8VBhhiVf8ytd0CkNbbho5VlV2BrDuMpAMiQ1MZZqXV544wf
12+
BXAChXPgDjsjP/2QDAR52jJjqwoGm+KFAp9ZTFpHqi1Yajt2J7M1EOacnkasDdcD
13+
GzgxDIA1oo6XOM1sisOc11d+L1JyOwtSHIaQRO9BChU5CAZCTRF9wITdmBnhb+ha
14+
mEgHPFIeF8uPXnjvsXJgHM/GqOoIlb671DOVNdZPxDhg+Pp47qGQJrcTz+ptTLw4
15+
bgcSCbHQXK+TDk2GxUS27PbZ9trK2Rfo6MLe4x0kT8k+9zZkPDKdJzCYDVzfcakf
16+
Qj8cDxplrwIDAQABo2YwZDAMBgNVHRMBAf8EAjAAMCAGA1UdEQQZMBeCD2lvdC5l
17+
Y2xpcHNlLm9yZ4cExike8TATBgNVHSUEDDAKBggrBgEFBQcDATAdBgNVHQ4EFgQU
18+
sXXbkSdzImOC1cQcnXOPeGdghvowDQYJKoZIhvcNAQELBQADggExAHueO/jU8LNE
19+
M5GyA0ENlPLoeBgtsa2HGXGH1vrc9HEESaFLlQX/V0hEa6MtFTtZTGz4J4zfJK0s
20+
Zb7aMxQqlyDE0tGHb6p4XiyjyEcIpAaRuOdMFvsELBugLQIGyJU+FRhrJ6we0G/9
21+
aZg+PPhiLRsRQtxgS6zWJ3zQt8Vxj7gnjMjUzuJ30LeborKOZih0By1zY127TbgK
22+
0+7CjI3kzUzgX5gy6/YKJLoEl7In7GgLCXy4TdRAC4jrmiBZA3xhn2YbpEGG383z
23+
JL7gwrTxilGPdpUkPqsh3QPFybe//IuZnv0asPY4t9GgVqObomUGkafBcz1kDnlh
24+
eCQzrplSaCcePR3cawnX/x6ZjnRSfKGJzKYWBVtxLT6TW95AyGyv0bpsH5d3TpKO
25+
ayb6IYpBphw=
26+
-----END CERTIFICATE-----

‎org.eclipse.paho.client.mqttv3/src/main/java-templates/org/eclipse/paho/client/mqttv3/internal/ClientComms.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -319,6 +319,9 @@ public void shutdownConnection(MqttToken token, MqttException reason) {
319319
// when actions complete
320320
if (callback!= null) {callback.stop(); }
321321

322+
// Stop the thread that handles inbound work from the network
323+
if (receiver != null) {receiver.stop();}
324+
322325
// Stop the network module, send and receive now not possible
323326
try {
324327
if (networkModules != null) {
@@ -331,9 +334,6 @@ public void shutdownConnection(MqttToken token, MqttException reason) {
331334
// Ignore as we are shutting down
332335
}
333336

334-
// Stop the thread that handles inbound work from the network
335-
if (receiver != null) {receiver.stop();}
336-
337337
// Stop any new tokens being saved by app and throwing an exception if they do
338338
tokenStore.quiesce(new MqttException(MqttException.REASON_CODE_CLIENT_DISCONNECTING));
339339

‎org.eclipse.paho.client.mqttv3/src/main/java/org/eclipse/paho/client/mqttv3/internal/CommsReceiver.java‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,12 @@ public void stop() {
8080
if (!Thread.currentThread().equals(recThread)) {
8181
try {
8282
// Wait for the thread to finish.
83-
recThread.join();
83+
// It is always a good idea to set a timeout.
84+
// WARNING: the timeout must be correlated with the
85+
// socket read timeout.
86+
// Unfortunately we cannot access the socket here to
87+
// get its read timeout.
88+
recThread.join(1500);
8489
}
8590
catch (InterruptedException ex) {
8691
}
@@ -107,6 +112,7 @@ public void run() {
107112
MqttWireMessage message = in.readMqttWireMessage();
108113
receiving = false;
109114

115+
// instanceof checks if message is null
110116
if (message instanceof MqttAck) {
111117
token = tokenStore.getToken(message);
112118
if (token!=null) {
@@ -123,8 +129,10 @@ public void run() {
123129
throw new MqttException(MqttException.REASON_CODE_UNEXPECTED_ERROR);
124130
}
125131
} else {
126-
// A new message has arrived
127-
clientState.notifyReceivedMsg(message);
132+
if (message != null) {
133+
// A new message has arrived
134+
clientState.notifyReceivedMsg(message);
135+
}
128136
}
129137
}
130138
catch (MqttException ex) {

‎org.eclipse.paho.client.mqttv3/src/main/java/org/eclipse/paho/client/mqttv3/internal/SSLNetworkModule.java‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -86,10 +86,8 @@ public void start() throws IOException, MqttException {
8686
super.start();
8787
setEnabledCiphers(enabledCiphers);
8888
int soTimeout = socket.getSoTimeout();
89-
if ( soTimeout == 0 ) {
90-
// RTC 765: Set a timeout to avoid the SSL handshake being blocked indefinitely
91-
socket.setSoTimeout(this.handshakeTimeoutSecs*1000);
92-
}
89+
// RTC 765: Set a timeout to avoid the SSL handshake being blocked indefinitely
90+
socket.setSoTimeout(this.handshakeTimeoutSecs*1000);
9391
((SSLSocket)socket).startHandshake();
9492
// reset timeout to default value
9593
socket.setSoTimeout(soTimeout);

‎org.eclipse.paho.client.mqttv3/src/main/java/org/eclipse/paho/client/mqttv3/internal/TCPNetworkModule.java‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,10 @@ public void start() throws IOException, MqttException {
6767
log.fine(CLASS_NAME,methodName, "252", new Object[] {host, new Integer(port), new Long(conTimeout*1000)});
6868
SocketAddress sockaddr = new InetSocketAddress(host, port);
6969
socket = factory.createSocket();
70+
// Set a read timeout on the socket.
71+
// If you change the value here you should also change
72+
// the value in CommsReceiver.stop().
73+
socket.setSoTimeout(1000);
7074
socket.connect(sockaddr, conTimeout*1000);
7175

7276
// SetTcpNoDelay was originally set ot true disabling Nagle's algorithm.
@@ -93,6 +97,25 @@ public OutputStream getOutputStream() throws IOException {
9397
*/
9498
public void stop() throws IOException {
9599
if (socket != null) {
100+
// CDA: an attempt is made to stop the receiver cleanly before closing the socket.
101+
// If the socket is forcibly closed too early, the blocking socket read in
102+
// the receiver thread throws a SocketException.
103+
// While this causes the receiver thread to exit, it also invalidates the
104+
// SSL session preventing to perform an accelerated SSL handshake in the
105+
// next connection.
106+
//
107+
// Also note that due to the blocking socket reads in the receiver thread,
108+
// it's not possible to interrupt the thread. Using non blocking reads in
109+
// combination with a socket timeout (see setSoTimeout()) would be a better approach.
110+
//
111+
// Please note that the Javadoc only says that an EOF is returned on
112+
// subsequent reads of the socket stream.
113+
// Anyway, at least with Oracle Java SE 7 on Linux systems, this causes a blocked read
114+
// to return EOF immediately.
115+
// This workaround should not cause any harm in general but you might
116+
// want to move it in SSLNetworkModule.
117+
118+
socket.shutdownInput();
96119
socket.close();
97120
}
98121
}

0 commit comments

Comments
 (0)