Merge pull request #439 from guusdk/OF-973

OF-973: TLS negotiation failures should come in two flavors.

src/java/org/jivesoftware/openfire/net/SocketConnection.java

@@ -473,6 +473,16 @@ public class SocketConnection implements Connection {
         return backupDeliverer;
     }
 
+    /**
+     * Closes the connection without sending any data (not even a stream end-tag).
+     */
+    public void forceClose() {
+        close( true );
+    }
+
+    /**
+     * Closes the connection after trying to send a stream end tag.
+     */
     @Override
     public void close() {
         close( false );
@@ -484,7 +494,7 @@ public class SocketConnection implements Connection {
      * when sending data over the socket has taken a long time and we need to close the socket, discard
      * the connection and its session.
      */
-    private void close(boolean force) {   	
+    private void close(boolean force) {
     	if (state.compareAndSet(State.OPEN, State.CLOSED)) {
     		
             if (session != null) {
@@ -554,7 +564,7 @@ public class SocketConnection implements Connection {
                 Log.debug("Closing connection: " + this + " that started sending data at: " +
                         new Date(writeTimestamp));
             }
-            close(true); // force
+            forceClose();
             return true;
         }
         else {
@@ -567,7 +577,7 @@ public class SocketConnection implements Connection {
                 if (Log.isDebugEnabled()) {
                     Log.debug("Closing connection that has been idle: " + this);
                 }
-                close(true); // force
+                forceClose();
                 return true;
             }
         }

src/java/org/jivesoftware/openfire/net/SocketReadingMode.java

@@ -32,6 +32,8 @@ import org.slf4j.LoggerFactory;
 import org.xmlpull.v1.XmlPullParserException;
 import org.xmpp.packet.StreamError;
 
+import javax.net.ssl.SSLHandshakeException;
+
 /**
  * Abstract class for {@link BlockingReadingMode}.
  *
@@ -84,8 +86,15 @@ abstract class SocketReadingMode {
             // This code is only used for s2s
             socketReader.connection.startTLS(false);
         }
-        catch (IOException e) {
-            Log.error("Error while negotiating TLS: " + socketReader.connection, e);
+        catch (SSLHandshakeException e) {
+            // RFC3620, section 5.4.3.2 "STARTTLS Failure" - close the socket *without* sending any more data (<failure/> nor </stream>).
+            Log.info( "STARTTLS negotiation (with: {}) failed.", socketReader.connection, e );
+            socketReader.connection.forceClose();
+            return false;
+        }
+        catch (IOException | RuntimeException e) {
+            // RFC3620, section 5.4.2.2 "Failure case" - Send a <failure/> element, then close the socket.
+            Log.warn( "An exception occurred while performing STARTTLS negotiation (with: {})", socketReader.connection, e);
             socketReader.connection.deliverRawText("<failure xmlns=\"urn:ietf:params:xml:ns:xmpp-tls\"/>");
             socketReader.connection.close();
             return false;

src/java/org/jivesoftware/openfire/session/LocalOutgoingServerSession.java

@@ -331,19 +331,22 @@ public class LocalOutgoingServerSession extends LocalServerSession implements Ou
                 connection.close();
             }
         }
-        catch (SSLHandshakeException e) {
-            Log.debug("LocalOutgoingServerSession: Handshake error while creating secured outgoing session to remote " +
-                    "server: " + hostname + "(DNS lookup: " + realHostname + ":" + realPort +
-                    "):", e);
-            // Close the connection
+        catch (SSLHandshakeException e)
+        {
+            // This is a failure as described in RFC3620, section 5.4.3.2 "STARTTLS Failure".
+            Log.info( "STARTTLS negotiation (with {} at {}:{}) failed.", hostname, realHostname, realPort, e );
+
+            // The receiving entity is expected to close the socket *without* sending any more data (<failure/> nor </stream>).
+            // It is probably (see OF-794) best if we, as the initiating entity, therefor don't send any data either.
             if (connection != null) {
-                connection.close();
+                connection.forceClose();
             }
         }
-        catch (Exception e) {
-            Log.error("Error creating secured outgoing session to remote server: " + hostname +
-                    "(DNS lookup: " + realHostname + ":" + realPort + ")", e);
-            // Close the connection
+        catch (Exception e)
+        {
+            // This might be RFC3620, section 5.4.2.2 "Failure Case" or even an unrelated problem. Handle 'normally'.
+            Log.warn( "An exception occurred while creating an encrypted session (with {} at {}:{})",  hostname, realHostname, realPort, e );
+
             if (connection != null) {
                 connection.close();
             }