websocket server: add timeout to abort incomplete handshakes

A websocket connection can involve two types of handshakes. First an
optional SSL handshake and second the websocket handshake itself. Either
one can get stalled/stuck if the other side does not answer.

To be robust by default and for easy mitigation by users of websockets
let's introduce a handshake timeout. We introduce a default timeout of 10
seconds which can be customized by the newly introduced setHandshakeTimeout()
method.

One major location where connections got stuck was when the connection
queue was filled with connections waiting for the SSL handshake. Only
connections that have finished this handshake can be processed anyway so
we now add them to the queue once they are fully ready to start the
websocket handshake.

Task-number: QTBUG-63312
Task-number: QTBUG-57026
Change-Id: Ia286221f1d8da1000e98973496280fde16ed811d
Reviewed-by: Alf Crüger <a.crueger@baxi-innotech.de>
Reviewed-by: Edward Welbourne <edward.welbourne@qt.io>
Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>

7 files changed, 253 insertions, 19 deletions

diff --git a/src/websockets/qsslserver.cpp b/src/websockets/qsslserver.cpp
index 586e520..ec645c9 100644
--- a/src/websockets/qsslserver.cpp
+++ b/src/websockets/qsslserver.cpp
@@ -118,11 +118,11 @@ void QSslServer::incomingConnection(qintptr socket)
             connect(pSslSocket, QOverload<const QList<QSslError>&>::of(&QSslSocket::sslErrors),
                     this, &QSslServer::sslErrors);
             connect(pSslSocket, &QSslSocket::encrypted,
-                    this, &QSslServer::newEncryptedConnection);
+                    this, &QSslServer::socketEncrypted);
             connect(pSslSocket, &QSslSocket::preSharedKeyAuthenticationRequired,
                     this, &QSslServer::preSharedKeyAuthenticationRequired);
 
-            addPendingConnection(pSslSocket);
+            Q_EMIT startedEncryptionHandshake(pSslSocket);
 
             pSslSocket->startServerEncryption();
         } else {
@@ -131,4 +131,15 @@ void QSslServer::incomingConnection(qintptr socket)
     }
 }
 
+void QSslServer::socketEncrypted()
+{
+    QSslSocket *pSslSocket = qobject_cast<QSslSocket *>(sender());
+
+    // We do not add the connection until the encryption handshake is complete.
+    // In case the handshake is aborted, we would be left with a stale
+    // connection in the queue otherwise.
+    addPendingConnection(pSslSocket);
+    Q_EMIT newEncryptedConnection();
+}
+
 QT_END_NAMESPACE
diff --git a/src/websockets/qsslserver_p.h b/src/websockets/qsslserver_p.h
index 10f8fea..6283058 100644
--- a/src/websockets/qsslserver_p.h
+++ b/src/websockets/qsslserver_p.h
@@ -59,6 +59,8 @@
 
 QT_BEGIN_NAMESPACE
 
+class QSslSocket;
+
 class QSslServer : public QTcpServer
 {
     Q_OBJECT
@@ -76,10 +78,14 @@ Q_SIGNALS:
     void peerVerifyError(const QSslError &error);
     void newEncryptedConnection();
     void preSharedKeyAuthenticationRequired(QSslPreSharedKeyAuthenticator *authenticator);
+    void startedEncryptionHandshake(QSslSocket *socket);
 
 protected:
     void incomingConnection(qintptr socket) override;
 
+private slots:
+    void socketEncrypted();
+
 private:
     QSslConfiguration m_sslConfiguration;
 };
diff --git a/src/websockets/qwebsocketserver.cpp b/src/websockets/qwebsocketserver.cpp
index 7790250..9717401 100644
--- a/src/websockets/qwebsocketserver.cpp
+++ b/src/websockets/qwebsocketserver.cpp
@@ -77,6 +77,9 @@
 
     QWebSocketServer only supports version 13 of the WebSocket protocol, as outlined in \l{RFC 6455}.
 
+    There is a default connection handshake timeout of 10 seconds to avoid denial of service,
+    which can be customized using setHandshakeTimeout().
+
     \sa {WebSocket Server Example}, QWebSocket
 */
 
@@ -350,6 +353,20 @@ int QWebSocketServer::maxPendingConnections() const
 }
 
 /*!
+    Returns the handshake timeout for new connections in milliseconds.
+
+    The default is 10 seconds. If a peer uses more time to complete the
+    handshake their connection is closed.
+
+    \sa setHandshakeTimeout()
+ */
+int QWebSocketServer::handshakeTimeout() const
+{
+    Q_D(const QWebSocketServer);
+    return d->handshakeTimeout();
+}
+
+/*!
     Returns the next pending connection as a connected QWebSocket object.
     QWebSocketServer does not take ownership of the returned QWebSocket object.
     It is up to the caller to delete the object explicitly when it will no longer be used,
@@ -574,6 +591,21 @@ void QWebSocketServer::setMaxPendingConnections(int numConnections)
     d->setMaxPendingConnections(numConnections);
 }
 
+/*!
+    Sets the handshake timeout for new connections to \a msec milliseconds.
+
+    By default this is set to 10 seconds. If a peer uses more time to
+    complete the handshake, their connection is closed. You can pass a
+    negative value (e.g. -1) to disable the timeout.
+
+    \sa handshakeTimeout()
+ */
+void QWebSocketServer::setHandshakeTimeout(int msec)
+{
+    Q_D(QWebSocketServer);
+    d->setHandshakeTimeout(msec);
+}
+
 #if (QT_VERSION >= QT_VERSION_CHECK(6, 0, 0))
 /*!
     Sets the socket descriptor this server should use when listening for incoming connections to
diff --git a/src/websockets/qwebsocketserver.h b/src/websockets/qwebsocketserver.h
index 17d5376..dd06448 100644
--- a/src/websockets/qwebsocketserver.h
+++ b/src/websockets/qwebsocketserver.h
@@ -86,6 +86,9 @@ public:
     void setMaxPendingConnections(int numConnections);
     int maxPendingConnections() const;
 
+    void setHandshakeTimeout(int msec);
+    int handshakeTimeout() const;
+
     quint16 serverPort() const;
     QHostAddress serverAddress() const;
     QUrl serverUrl() const;
diff --git a/src/websockets/qwebsocketserver_p.cpp b/src/websockets/qwebsocketserver_p.cpp
index 4c3eeaf..8225b59 100644
--- a/src/websockets/qwebsocketserver_p.cpp
+++ b/src/websockets/qwebsocketserver_p.cpp
@@ -49,6 +49,7 @@
 #include "qwebsocket_p.h"
 #include "qwebsocketcorsauthenticator.h"
 
+#include <QtCore/QTimer>
 #include <QtNetwork/QTcpServer>
 #include <QtNetwork/QTcpSocket>
 #include <QtNetwork/QNetworkProxy>
@@ -73,7 +74,8 @@ QWebSocketServerPrivate::QWebSocketServerPrivate(const QString &serverName,
     m_pendingConnections(),
     m_error(QWebSocketProtocol::CloseCodeNormal),
     m_errorString(),
-    m_maxPendingConnections(30)
+    m_maxPendingConnections(30),
+    m_handshakeTimeout(10000)
 {}
 
 /*!
@@ -97,6 +99,8 @@ void QWebSocketServerPrivate::init()
             QObjectPrivate::connect(pSslServer, &QSslServer::newEncryptedConnection,
                                     this, &QWebSocketServerPrivate::onNewConnection,
                                     Qt::QueuedConnection);
+            QObjectPrivate::connect(pSslServer, &QSslServer::startedEncryptionHandshake,
+                                    this, &QWebSocketServerPrivate::startHandshakeTimeout);
             QObject::connect(pSslServer, &QSslServer::peerVerifyError,
                              q, &QWebSocketServer::peerVerifyError);
             QObject::connect(pSslServer, &QSslServer::sslErrors,
@@ -381,8 +385,12 @@ void QWebSocketServerPrivate::setError(QWebSocketProtocol::CloseCode code, const
  */
 void QWebSocketServerPrivate::onNewConnection()
 {
-    while (m_pTcpServer->hasPendingConnections())
-        handleConnection(m_pTcpServer->nextPendingConnection());
+    while (m_pTcpServer->hasPendingConnections()) {
+        QTcpSocket *pTcpSocket = m_pTcpServer->nextPendingConnection();
+        if (Q_LIKELY(pTcpSocket) && m_secureMode == NonSecureMode)
+            startHandshakeTimeout(pTcpSocket);
+        handleConnection(pTcpSocket);
+    }
 }
 
 /*!
@@ -463,6 +471,7 @@ void QWebSocketServerPrivate::handshakeReceived()
                                                                         request,
                                                                         response);
                 if (pWebSocket) {
+                    finishHandshakeTimeout(pTcpSocket);
                     addPendingConnection(pWebSocket);
                     Q_EMIT q->newConnection();
                     success = true;
@@ -502,4 +511,26 @@ void QWebSocketServerPrivate::handleConnection(QTcpSocket *pTcpSocket) const
     }
 }
 
+void QWebSocketServerPrivate::startHandshakeTimeout(QTcpSocket *pTcpSocket)
+{
+    if (m_handshakeTimeout < 0)
+        return;
+
+    QTimer *handshakeTimer = new QTimer(pTcpSocket);
+    handshakeTimer->setSingleShot(true);
+    handshakeTimer->setObjectName(QStringLiteral("handshakeTimer"));
+    QObject::connect(handshakeTimer, &QTimer::timeout, [=]() {
+        pTcpSocket->close();
+    });
+    handshakeTimer->start(m_handshakeTimeout);
+}
+
+void QWebSocketServerPrivate::finishHandshakeTimeout(QTcpSocket *pTcpSocket)
+{
+    if (QTimer *handshakeTimer = pTcpSocket->findChild<QTimer *>(QStringLiteral("handshakeTimer"))) {
+        handshakeTimer->stop();
+        delete handshakeTimer;
+    }
+}
+
 QT_END_NAMESPACE
diff --git a/src/websockets/qwebsocketserver_p.h b/src/websockets/qwebsocketserver_p.h
index be7744f..1f1cbd9 100644
--- a/src/websockets/qwebsocketserver_p.h
+++ b/src/websockets/qwebsocketserver_p.h
@@ -90,6 +90,9 @@ public:
     bool isListening() const;
     bool listen(const QHostAddress &address = QHostAddress::Any, quint16 port = 0);
     int maxPendingConnections() const;
+    int handshakeTimeout() const {
+        return m_handshakeTimeout;
+    }
     virtual QWebSocket *nextPendingConnection();
     void pauseAccepting();
 #ifndef QT_NO_NETWORKPROXY
@@ -101,6 +104,9 @@ public:
     QWebSocketProtocol::CloseCode serverError() const;
     quint16 serverPort() const;
     void setMaxPendingConnections(int numConnections);
+    void setHandshakeTimeout(int msec) {
+        m_handshakeTimeout = msec;
+    }
     bool setSocketDescriptor(qintptr socketDescriptor);
     qintptr socketDescriptor() const;
 
@@ -122,6 +128,9 @@ public:
 
     void handleConnection(QTcpSocket *pTcpSocket) const;
 
+private slots:
+    void startHandshakeTimeout(QTcpSocket *pTcpSocket);
+
 private:
     QTcpServer *m_pTcpServer;
     QString m_serverName;
@@ -130,6 +139,7 @@ private:
     QWebSocketProtocol::CloseCode m_error;
     QString m_errorString;
     int m_maxPendingConnections;
+    int m_handshakeTimeout;
 
     void addPendingConnection(QWebSocket *pWebSocket);
     void setErrorFromSocketError(QAbstractSocket::SocketError error,
@@ -138,6 +148,7 @@ private:
     void onNewConnection();
     void onSocketDisconnected();
     void handshakeReceived();
+    void finishHandshakeTimeout(QTcpSocket *pTcpSocket);
 };
 
 QT_END_NAMESPACE
diff --git a/tests/auto/websockets/qwebsocketserver/tst_qwebsocketserver.cpp b/tests/auto/websockets/qwebsocketserver/tst_qwebsocketserver.cpp
index 64b2489..80a3bc6 100644
--- a/tests/auto/websockets/qwebsocketserver/tst_qwebsocketserver.cpp
+++ b/tests/auto/websockets/qwebsocketserver/tst_qwebsocketserver.cpp
@@ -29,6 +29,7 @@
 #include <QtTest>
 #include <QNetworkProxy>
 #include <QTcpServer>
+#include <QtCore/QScopedPointer>
 #ifndef QT_NO_OPENSSL
 #include <QtNetwork/qsslpresharedkeyauthenticator.h>
 #endif
@@ -113,6 +114,7 @@ private Q_SLOTS:
     void tst_serverDestroyedWhileSocketConnected();
     void tst_scheme(); // qtbug-55927
     void tst_handleConnection();
+    void tst_handshakeTimeout(); // qtbug-63312, qtbug-57026
 
 private:
     bool m_shouldSkipUnsupportedIpv6Test;
@@ -573,6 +575,26 @@ void tst_QWebSocketServer::tst_serverDestroyedWhileSocketConnected()
     QTRY_COMPARE(socketDisconnectedSpy.count(), 1);
 }
 
+#ifndef QT_NO_SSL
+static void setupSecureServer(QWebSocketServer *secureServer)
+{
+    QSslConfiguration sslConfiguration;
+    QFile certFile(QStringLiteral(":/localhost.cert"));
+    QFile keyFile(QStringLiteral(":/localhost.key"));
+    QVERIFY(certFile.open(QIODevice::ReadOnly));
+    QVERIFY(keyFile.open(QIODevice::ReadOnly));
+    QSslCertificate certificate(&certFile, QSsl::Pem);
+    QSslKey sslKey(&keyFile, QSsl::Rsa, QSsl::Pem);
+    certFile.close();
+    keyFile.close();
+    sslConfiguration.setPeerVerifyMode(QSslSocket::VerifyNone);
+    sslConfiguration.setLocalCertificate(certificate);
+    sslConfiguration.setPrivateKey(sslKey);
+    sslConfiguration.setProtocol(QSsl::TlsV1SslV3);
+    secureServer->setSslConfiguration(sslConfiguration);
+}
+#endif
+
 void tst_QWebSocketServer::tst_scheme()
 {
     if (m_shouldSkipUnsupportedIpv6Test)
@@ -594,20 +616,9 @@ void tst_QWebSocketServer::tst_scheme()
 
 #ifndef QT_NO_SSL
     QWebSocketServer secureServer(QString(), QWebSocketServer::SecureMode);
-    QSslConfiguration sslConfiguration;
-    QFile certFile(QStringLiteral(":/localhost.cert"));
-    QFile keyFile(QStringLiteral(":/localhost.key"));
-    QVERIFY(certFile.open(QIODevice::ReadOnly));
-    QVERIFY(keyFile.open(QIODevice::ReadOnly));
-    QSslCertificate certificate(&certFile, QSsl::Pem);
-    QSslKey sslKey(&keyFile, QSsl::Rsa, QSsl::Pem);
-    certFile.close();
-    keyFile.close();
-    sslConfiguration.setPeerVerifyMode(QSslSocket::VerifyNone);
-    sslConfiguration.setLocalCertificate(certificate);
-    sslConfiguration.setPrivateKey(sslKey);
-    sslConfiguration.setProtocol(QSsl::TlsV1SslV3);
-    secureServer.setSslConfiguration(sslConfiguration);
+    setupSecureServer(&secureServer);
+    if (QTest::currentTestFailed())
+        return;
     QSignalSpy secureServerConnectionSpy(&secureServer, SIGNAL(newConnection()));
 
     QVERIFY(secureServer.listen());
@@ -668,6 +679,135 @@ void tst_QWebSocketServer::tst_handleConnection()
     QCOMPARE(arguments.first().toString(), QString("hello"));
 }
 
+struct SocketSpy {
+    QTcpSocket *socket;
+    QSignalSpy *disconnectSpy;
+    ~SocketSpy() {
+        delete socket;
+        delete disconnectSpy;
+    }
+};
+
+static void openManyConnections(QList<SocketSpy *> *sockets, quint16 port)
+{
+    /* do some incomplete connections */
+    for (int i = 0; i < 50; i++) {
+        QTcpSocket *c = new QTcpSocket;
+        QSignalSpy *spy = new QSignalSpy(c, &QTcpSocket::disconnected);
+
+        QString hostname = "localhost";
+        c->connectToHost("localhost", port);
+
+        sockets->append(new SocketSpy{c, spy});
+    }
+}
+
+void tst_QWebSocketServer::tst_handshakeTimeout()
+{
+    { // No Timeout
+        QWebSocketServer plainServer(QString(), QWebSocketServer::NonSecureMode);
+        plainServer.setHandshakeTimeout(-1);
+        QSignalSpy plainServerConnectionSpy(&plainServer, SIGNAL(newConnection()));
+
+        QVERIFY(plainServer.listen());
+
+        QWebSocket socket;
+        socket.open(plainServer.serverUrl().toString());
+
+        QTRY_COMPARE(plainServerConnectionSpy.count(), 1);
+        QScopedPointer<QWebSocket> plainServerSocket(plainServer.nextPendingConnection());
+        QVERIFY(!plainServerSocket.isNull());
+
+        plainServer.close();
+    }
+
+    { // Unencrypted
+        QWebSocketServer plainServer(QString(), QWebSocketServer::NonSecureMode);
+        plainServer.setHandshakeTimeout(500);
+        QSignalSpy plainServerConnectionSpy(&plainServer, SIGNAL(newConnection()));
+
+        QVERIFY(plainServer.listen());
+
+        QList<SocketSpy *> sockets;
+        auto cleaner = qScopeGuard([&sockets]() { qDeleteAll(sockets); });
+        openManyConnections(&sockets, plainServer.serverPort());
+
+        QCoreApplication::processEvents();
+        QCOMPARE(plainServerConnectionSpy.count(), 0);
+
+        QWebSocket socket;
+        socket.open(plainServer.serverUrl().toString());
+
+        QTRY_COMPARE(plainServerConnectionSpy.count(), 1);
+        QScopedPointer<QWebSocket> plainServerSocket(plainServer.nextPendingConnection());
+        QVERIFY(!plainServerSocket.isNull());
+
+        for (auto s : sockets)
+            QTRY_COMPARE(s->disconnectSpy->count(), 1);
+
+        plainServer.close();
+    }
+
+#if QT_CONFIG(ssl)
+    { // Encrypted
+        QWebSocketServer secureServer(QString(), QWebSocketServer::SecureMode);
+        setupSecureServer(&secureServer);
+        if (QTest::currentTestFailed())
+            return;
+        secureServer.setHandshakeTimeout(500);
+
+        QSignalSpy secureServerConnectionSpy(&secureServer, SIGNAL(newConnection()));
+
+        QVERIFY(secureServer.listen());
+
+        QList<SocketSpy *> sockets;
+        auto cleaner = qScopeGuard([&sockets]() { qDeleteAll(sockets); });
+        openManyConnections(&sockets, secureServer.serverPort());
+
+        QCoreApplication::processEvents();
+        QCOMPARE(secureServerConnectionSpy.count(), 0);
+
+        QWebSocket secureSocket;
+        QSslConfiguration config = secureSocket.sslConfiguration();
+        config.setPeerVerifyMode(QSslSocket::VerifyNone);
+        secureSocket.setSslConfiguration(config);
+
+        secureSocket.open(secureServer.serverUrl().toString());
+
+        QTRY_COMPARE(secureServerConnectionSpy.count(), 1);
+        QScopedPointer<QWebSocket> serverSocket(secureServer.nextPendingConnection());
+        QVERIFY(!serverSocket.isNull());
+
+        for (auto s : sockets)
+            QTRY_COMPARE(s->disconnectSpy->count(), 1);
+
+        secureServer.close();
+    }
+#endif
+
+    { // Ensure properly handshaked connections are not timed out
+        QWebSocketServer plainServer(QString(), QWebSocketServer::NonSecureMode);
+        plainServer.setHandshakeTimeout(250);
+        QSignalSpy plainServerConnectionSpy(&plainServer, SIGNAL(newConnection()));
+
+        QVERIFY(plainServer.listen());
+
+        QWebSocket socket;
+        QSignalSpy socketConnectedSpy(&socket, &QWebSocket::connected);
+        QSignalSpy socketDisconnectedSpy(&socket, &QWebSocket::disconnected);
+        socket.open(plainServer.serverUrl().toString());
+
+        QTRY_COMPARE(plainServerConnectionSpy.count(), 1);
+        QTRY_COMPARE(socketConnectedSpy.count(), 1);
+
+        QEventLoop loop;
+        QTimer::singleShot(500, &loop, &QEventLoop::quit);
+        loop.exec();
+
+        QCOMPARE(socketDisconnectedSpy.count(), 0);
+    }
+}
+
 QTEST_MAIN(tst_QWebSocketServer)
 
 #include "tst_qwebsocketserver.moc"