Swift
summaryrefslogtreecommitdiffstats
path: root/src/com/isode/stroke/tls
diff options
context:
space:
mode:
authorNick Hudson <nick.hudson@isode.com>2013-08-12 14:12:41 (GMT)
committerNick Hudson <nick.hudson@isode.com>2013-09-02 16:19:08 (GMT)
commitd10646423d70934b598b5d8ff2d8910034941029 (patch)
tree982621f5c98459fb477e753f0a1589817df8d258 /src/com/isode/stroke/tls
parent34e195e8e2db1de0b71460afcce2417d34fc7717 (diff)
downloadstroke-d10646423d70934b598b5d8ff2d8910034941029.zip
stroke-d10646423d70934b598b5d8ff2d8910034941029.tar.bz2
Changes to handle close notify gracefully
It was noticed that in certain cases, Stroke got stuck when connecting to a server over TLS, and the server closed down the connection. Investigation showed that this appears to be caused by the JSSEContext code not properly coping with a "close notify" SSL message. What happens in this case is that the SSLEngine generates a response to the server's "close notify", and expects this to be sent back over the network. The original JSSEContext code saw the "CLOSED" status from the SSLEngine.unwrap(), but assumed that no more data would be generated by the engine (which was wrong, because the engine wants to send a close response back), and so got stuck in a loop. This patch therefore fixes the JSSEContext code to deal properly when it sees a CLOSED from unwrap. After the close has been received, an error will be emitted by JSSEContext so that the application knows that the SSLEngine can no longer be used (in practice we have always seen the socket closing, which generates its own error to the application, but it was recommended that we should have this check in case a server sends a close notify and does NOT close the socket as well). It appears that many servers don't actually send the "close notify", and just drop the connection, which is (presumably) why we'd not seen this behaviour before. Test-information: Tested by connecting to the aforementioned server. This time, when the connection times out (and the closenotify is sent), we no longer see a loop, but the application realises what's happens and attempts to reconnect. I have been running with this patch in my copy of MLC for two weeks and have noticed no difference in behaviour - so far as I can tell the code is not exercised when talking to M-Link but at any rate the patch isn't causing anything to break. Change-Id: Id007c923c510ef1b4ce53192105b00296c65c757
Diffstat (limited to 'src/com/isode/stroke/tls')
-rw-r--r--src/com/isode/stroke/tls/java/JSSEContext.java86
1 files changed, 66 insertions, 20 deletions
diff --git a/src/com/isode/stroke/tls/java/JSSEContext.java b/src/com/isode/stroke/tls/java/JSSEContext.java
index 8c351bb..77b9447 100644
--- a/src/com/isode/stroke/tls/java/JSSEContext.java
+++ b/src/com/isode/stroke/tls/java/JSSEContext.java
@@ -90,7 +90,12 @@ public class JSSEContext extends TLSContext {
* Whether the handshake has finished
*/
private boolean handshakeCompleted = false;
-
+
+ /**
+ * Whether the server has sent a close notification
+ */
+ private boolean closeNotifyReceived = false;
+
/**
* Determine whether an error has occurred. If an error has occurred, then
* you probably don't want to try doing any more stuff.
@@ -280,12 +285,26 @@ public class JSSEContext extends TLSContext {
*/
unwrapDone = true;
break;
- case CLOSED:
- /* Engine closed - don't expect this here */
- emitError(null, "SSLEngine.unwrap returned " + status);
- return bytesConsumed;
-
- case OK:
+ case CLOSED:
+ /* This is taken to mean that the server end has
+ * sent "SSL close notify alert". Once it sees this,
+ * ths SSLEngine will respond by generating an
+ * appropriate handshake response that should be
+ * sent to the server (as per RFC 2246 7.2.1).
+ * In this case, the SSLEngine will move into a
+ * NEED_WRAP state, and we should emit whatever data
+ * it's generated over the network.
+ */
+ closeNotifyReceived = true;
+
+ /* Tell the SSLEngine that the application won't be
+ * sending any more data (this probably has no effect
+ * but it does no harm).
+ */
+ sslEngine.closeOutbound();
+ return bytesConsumed;
+
+ case OK:
/* Some stuff was unwrapped. */
bytesConsumed += sslEngineResult.bytesConsumed();
bytesProduced = sslEngineResult.bytesProduced();
@@ -381,15 +400,16 @@ public class JSSEContext extends TLSContext {
*/
synchronized(sendMutex) {
plainToSend.flip();
- /* If the SSL handshake hasn't completed, then there may not be
- * any data from the client in plainToSend
- */
- if (!plainToSend.hasRemaining() && handshakeCompleted) {
- /* Nothing more to be encrypted */
- plainToSend.compact();
- return bytesSentToSocket;
- }
- try {
+ /* It does no harm to call SSLEngine.wrap if there is nothing in
+ * "plainToSend" and this will be required in at least two
+ * cases:
+ * - during the initial handshake, when the application hasn't
+ * yet sent anything, but wrap() must be called to generate
+ * the TLS handshake data
+ * - during closure, when wrap() will generate the response to
+ * the server's close notify message
+ */
+ try {
boolean wrapDone = false;
do {
sslEngineResult = sslEngine.wrap(plainToSend, wrappedToSend);
@@ -428,9 +448,21 @@ public class JSSEContext extends TLSContext {
}
switch (status) {
+ case CLOSED :
+ /* Engine closed - this is expected if a close notify has
+ * been sent by the server, and the SSLEngine has finished
+ * generating the response to that message.
+ */
+ if (!closeNotifyReceived) {
+ emitError(null, "SSLEngine.wrap returned " + status);
+ return (bytesSentToSocket);
+ }
+ /* CLOSED was expected, so fall through to OK, in order
+ * to send the close response back to the server
+ */
case OK:
- /* This is the only status we expect here. It means the
- * data was successfully wrapped and that there's something
+ /* This is the status we expect here. It means data
+ * was successfully wrapped and that there's something
* to be sent.
*/
wrappedToSend.flip();
@@ -444,8 +476,6 @@ public class JSSEContext extends TLSContext {
case BUFFER_UNDERFLOW:
/* Can't happen for a wrap */
- case CLOSED :
- /* Engine closed - don't expect this here */
case BUFFER_OVERFLOW:
/* We already dealt with this, so don't expect to come here
*/
@@ -935,6 +965,15 @@ public class JSSEContext extends TLSContext {
/* */
} while (processHandshakeStatus());
+ if (closeNotifyReceived) {
+ /* This is the only way to let the application know that close
+ * notify was received. This check is done after finishing with
+ * the handshake checks, so that the SSLEngine's response to
+ * the close notify has been dealt with.
+ */
+ emitError(null, "SSL Close notify received");
+ return;
+ }
/* Loop round so long as there are still bytes from the network
* to be processed
@@ -949,6 +988,13 @@ public class JSSEContext extends TLSContext {
onError.emit();
return;
}
+ if (closeNotifyReceived) {
+ /* After the server has closed the TLS connection, no more
+ * application data should be sent.
+ */
+ emitError(null,
+ "handleDataFromApplication called after SSLEngine closed");
+ }
byte[] b = data.getData();
/* Need to cope in the case that the application sends a ByteArray