QSslServer: Implement handshake timeouts

If a client doesn't send any data then we would leave the socket open
for as long as it needed, wasting resources. Add timeouts to limit the
amount of time this can happen for.

Since there is a limit on number of sockets that the server will have
queued, having idle sockets stick around forever is a vector for ddos.

Pick-to: 6.4
Change-Id: Ida6251c92c625eeadf2065861b840b14255654b8
Reviewed-by: Ievgenii Meshcheriakov <ievgenii.meshcheriakov@qt.io>
Reviewed-by: Timur Pocheptsov <timur.pocheptsov@qt.io>
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>

4 files changed, 95 insertions, 3 deletions

diff --git a/src/network/ssl/qsslserver.cpp b/src/network/ssl/qsslserver.cpp
index 1611046d0e..2ec42bf3ce 100644
--- a/src/network/ssl/qsslserver.cpp
+++ b/src/network/ssl/qsslserver.cpp
@@ -228,6 +228,42 @@ QSslConfiguration QSslServer::sslConfiguration() const
 }
 
 /*!
+    Sets the \a timeout to use for all incoming handshakes, in milliseconds.
+
+    This is relevant in the scenario where a client, whether malicious or
+    accidental, connects to the server but makes no attempt at communicating or
+    initiating a handshake. QSslServer will then automatically end the
+    connection after \a timeout milliseconds have elapsed.
+
+    By default the timeout is 5000 milliseconds (5 seconds).
+
+    \note The underlying TLS framework may have their own timeout logic now or
+    in the future, this function does not affect that.
+
+    \note The \a timeout passed to this function will only apply to \e{new}
+    connections. If a client is already connected it will use the timeout which
+    was set when it connected.
+
+    \sa handshakeTimeout()
+*/
+void QSslServer::setHandshakeTimeout(int timeout)
+{
+    Q_D(QSslServer);
+    d->handshakeTimeout = timeout;
+}
+
+/*!
+    Returns the currently configured handshake timeout.
+
+    \sa setHandshakeTimeout()
+*/
+int QSslServer::handshakeTimeout() const
+{
+    const Q_D(QSslServer);
+    return d->handshakeTimeout;
+}
+
+/*!
     Called when a new connection is established.
 
     Converts \a socket to a QSslSocket.
@@ -296,7 +332,11 @@ void QSslServerPrivate::initializeHandshakeProcess(QSslSocket *socket)
                 // QObject dtor, but we only use the pointer value!
                 removeSocketData(quintptr(obj));
             });
-    socketData.emplace(quintptr(socket), readyRead, destroyed);
+    auto it = socketData.emplace(quintptr(socket), readyRead, destroyed, std::make_shared<QTimer>());
+    it->timeoutTimer->setSingleShot(true);
+    it->timeoutTimer->callOnTimeout([this, socket]() { handleHandshakeTimedOut(socket); });
+    it->timeoutTimer->setInterval(handshakeTimeout);
+    it->timeoutTimer->start();
 }
 
 // This function may be called while in the socket's QObject dtor, __never__ use
@@ -335,8 +375,21 @@ void QSslServerPrivate::checkClientHelloAndContinue()
         return;
     }
 
+    // Be nice and restart the timeout timer since some progress was made
+    if (foundData)
+        it->timeoutTimer->start();
+
     socket->startServerEncryption();
     Q_EMIT q->startedEncryptionHandshake(socket);
 }
 
+void QSslServerPrivate::handleHandshakeTimedOut(QSslSocket *socket)
+{
+    Q_Q(QSslServer);
+    removeSocketData(quintptr(socket));
+    socket->disconnectFromHost();
+    Q_EMIT q->errorOccurred(socket, QAbstractSocket::SocketTimeoutError);
+    socket->deleteLater();
+}
+
 QT_END_NAMESPACE
diff --git a/src/network/ssl/qsslserver.h b/src/network/ssl/qsslserver.h
index d2f3abc456..aaa0f43c35 100644
--- a/src/network/ssl/qsslserver.h
+++ b/src/network/ssl/qsslserver.h
@@ -33,6 +33,9 @@ public:
     void setSslConfiguration(const QSslConfiguration &sslConfiguration);
     QSslConfiguration sslConfiguration() const;
 
+    void setHandshakeTimeout(int timeout);
+    int handshakeTimeout() const;
+
 Q_SIGNALS:
     void sslErrors(QSslSocket *socket, const QList<QSslError> &errors);
     void peerVerifyError(QSslSocket *socket, const QSslError &error);
diff --git a/src/network/ssl/qsslserver_p.h b/src/network/ssl/qsslserver_p.h
index 3c7cce0355..71359f6cff 100644
--- a/src/network/ssl/qsslserver_p.h
+++ b/src/network/ssl/qsslserver_p.h
@@ -19,6 +19,7 @@
 #include <QtNetwork/private/qtnetworkglobal_p.h>
 
 #include <QtCore/qhash.h>
+#include <QtCore/qtimer.h>
 
 #include <QtNetwork/QSslConfiguration>
 #include <QtNetwork/private/qtcpserver_p.h>
@@ -28,6 +29,7 @@ QT_BEGIN_NAMESPACE
 
 class Q_NETWORK_EXPORT QSslServerPrivate : public QTcpServerPrivate
 {
+    static constexpr int DefaultHandshakeTimeout = 5'000; // 5 seconds
 public:
     Q_DECLARE_PUBLIC(QSslServer)
 
@@ -35,13 +37,18 @@ public:
     void checkClientHelloAndContinue();
     void initializeHandshakeProcess(QSslSocket *socket);
     void removeSocketData(quintptr socket);
+    void handleHandshakeTimedOut(QSslSocket *socket);
 
     struct SocketData {
         QMetaObject::Connection readyReadConnection;
         QMetaObject::Connection destroyedConnection;
+        std::shared_ptr<QTimer> timeoutTimer; // shared_ptr because QHash demands copying
 
-        SocketData(QMetaObject::Connection readyRead, QMetaObject::Connection destroyed)
-            : readyReadConnection(readyRead), destroyedConnection(destroyed)
+        SocketData(QMetaObject::Connection readyRead, QMetaObject::Connection destroyed,
+                   std::shared_ptr<QTimer> &&timer)
+            : readyReadConnection(readyRead),
+              destroyedConnection(destroyed),
+              timeoutTimer(std::move(timer))
         {
         }
 
@@ -54,6 +61,7 @@ public:
     QHash<quintptr, SocketData> socketData;
 
     QSslConfiguration sslConfiguration;
+    int handshakeTimeout = DefaultHandshakeTimeout;
 };
 
 
diff --git a/tests/auto/network/ssl/qsslserver/tst_qsslserver.cpp b/tests/auto/network/ssl/qsslserver/tst_qsslserver.cpp
index fb8a74d8de..088f0170f4 100644
--- a/tests/auto/network/ssl/qsslserver/tst_qsslserver.cpp
+++ b/tests/auto/network/ssl/qsslserver/tst_qsslserver.cpp
@@ -24,6 +24,7 @@ private slots:
     void testPreSharedKeyAuthenticationRequired();
 #endif
     void plaintextClient();
+    void quietClient();
 
 private:
     QString testDataDir;
@@ -459,6 +460,33 @@ void tst_QSslServer::plaintextClient()
     QCOMPARE(socket.state(), QAbstractSocket::SocketState::UnconnectedState);
 }
 
+void tst_QSslServer::quietClient()
+{
+    QSslConfiguration serverConfiguration = selfSignedServerQSslConfiguration();
+    SslServerSpy server(serverConfiguration);
+    server.server.setHandshakeTimeout(1'000);
+    QVERIFY(server.server.listen());
+
+    quint16 serverPeerPort = 0;
+    auto grabServerPeerPort = [&serverPeerPort](QSslSocket *socket) {
+        serverPeerPort = socket->peerPort();
+    };
+    QObject::connect(&server.server, &QSslServer::errorOccurred, &server.server,
+                     grabServerPeerPort);
+
+    QTcpSocket socket;
+    QSignalSpy socketDisconnectedSpy(&socket, &QTcpSocket::disconnected);
+    socket.connectToHost(QHostAddress::LocalHost, server.server.serverPort());
+    quint16 clientLocalPort = socket.localPort();
+    QVERIFY(socket.waitForConnected());
+    // Disconnects after overlong break:
+    QVERIFY(socketDisconnectedSpy.wait(5'000));
+    QCOMPARE(socket.state(), QAbstractSocket::SocketState::UnconnectedState);
+
+    QCOMPARE_GT(server.errorOccurredSpy.size(), 0);
+    QCOMPARE(serverPeerPort, clientLocalPort);
+}
+
 QTEST_MAIN(tst_QSslServer)
 
 #include "tst_qsslserver.moc"