From 5c84ef1e699172f8d1dfd8139a81d6720c1659a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20St=C3=BCrz?= Date: Wed, 8 Jan 2020 15:02:57 +0100 Subject: [PATCH] Allow multiple tunnels for one valid token --- libnymea-remoteproxy/proxyclient.cpp | 5 + libnymea-remoteproxy/proxyclient.h | 2 + libnymea-remoteproxy/proxyserver.cpp | 35 +++--- libnymea-remoteproxy/proxyserver.h | 2 +- libnymea-remoteproxy/tunnelconnection.cpp | 16 +++ libnymea-remoteproxy/tunnelconnection.h | 2 + .../nymea-remoteproxy-tests-offline.cpp | 57 +++++++-- .../nymea-remoteproxy-tests-offline.h | 1 + tests/testbase/basetest.cpp | 113 ++++++++++++++++++ tests/testbase/basetest.h | 2 + 10 files changed, 204 insertions(+), 31 deletions(-) diff --git a/libnymea-remoteproxy/proxyclient.cpp b/libnymea-remoteproxy/proxyclient.cpp index ed9c854..4fa4659 100644 --- a/libnymea-remoteproxy/proxyclient.cpp +++ b/libnymea-remoteproxy/proxyclient.cpp @@ -123,6 +123,11 @@ void ProxyClient::setName(const QString &name) m_name = name; } +QString ProxyClient::tunnelIdentifier() const +{ + return m_token + m_nonce; +} + QString ProxyClient::token() const { return m_token; diff --git a/libnymea-remoteproxy/proxyclient.h b/libnymea-remoteproxy/proxyclient.h index d09783e..e5a89ab 100644 --- a/libnymea-remoteproxy/proxyclient.h +++ b/libnymea-remoteproxy/proxyclient.h @@ -63,6 +63,8 @@ public: QString name() const; void setName(const QString &name); + QString tunnelIdentifier() const; + QString token() const; void setToken(const QString &token); diff --git a/libnymea-remoteproxy/proxyserver.cpp b/libnymea-remoteproxy/proxyserver.cpp index fb1af26..5eb2399 100644 --- a/libnymea-remoteproxy/proxyserver.cpp +++ b/libnymea-remoteproxy/proxyserver.cpp @@ -141,13 +141,13 @@ void ProxyServer::saveStatistics() ProxyClient *ProxyServer::getRemoteClient(ProxyClient *proxyClient) { - if (!m_tunnels.contains(proxyClient->token())) + if (!m_tunnels.contains(proxyClient->tunnelIdentifier())) return nullptr; - if (proxyClient == m_tunnels.value(proxyClient->token()).clientOne()) { - return m_tunnels.value(proxyClient->token()).clientTwo(); - } else if (proxyClient == m_tunnels.value(proxyClient->token()).clientTwo()) { - return m_tunnels.value(proxyClient->token()).clientOne(); + if (proxyClient == m_tunnels.value(proxyClient->tunnelIdentifier()).clientOne()) { + return m_tunnels.value(proxyClient->tunnelIdentifier()).clientTwo(); + } else if (proxyClient == m_tunnels.value(proxyClient->tunnelIdentifier()).clientTwo()) { + return m_tunnels.value(proxyClient->tunnelIdentifier()).clientOne(); } return nullptr; @@ -163,7 +163,7 @@ void ProxyServer::establishTunnel(ProxyClient *firstClient, ProxyClient *secondC //FIXME: } - m_tunnels.insert(tunnel.token(), tunnel); + m_tunnels.insert(tunnel.tunnelIdentifier(), tunnel); // Tell both clients the tunnel has been established QVariantMap notificationParamsFirst; @@ -235,11 +235,11 @@ void ProxyServer::onClientDisconnected(const QUuid &clientId) m_jsonRpcServer->unregisterClient(proxyClient); // Check if - if (m_tunnels.contains(proxyClient->token())) { + if (m_tunnels.contains(proxyClient->tunnelIdentifier())) { // There is a tunnel connection for this client, remove the tunnel and disconnect also the other client ProxyClient *remoteClient = getRemoteClient(proxyClient); - TunnelConnection tunnelConnection = m_tunnels.take(proxyClient->token()); + TunnelConnection tunnelConnection = m_tunnels.take(proxyClient->tunnelIdentifier()); Engine::instance()->logEngine()->logTunnel(tunnelConnection); if (remoteClient) { remoteClient->killConnection("Tunnel client disconnected"); @@ -279,7 +279,7 @@ void ProxyServer::onClientDataAvailable(const QUuid &clientId, const QByteArray if (proxyClient->isAuthenticated() && proxyClient->isTunnelConnected()) { ProxyClient *remoteClient = getRemoteClient(proxyClient); Q_ASSERT_X(remoteClient, "ProxyServer", "Tunnel existing but not tunnel client available"); - Q_ASSERT_X(m_tunnels.contains(proxyClient->token()), "ProxyServer", "Tunnel connect but not existing"); + Q_ASSERT_X(m_tunnels.contains(proxyClient->tunnelIdentifier()), "ProxyServer", "Tunnel connect but not existing"); // Calculate server statisitcs m_troughputCounter += data.count(); @@ -304,18 +304,17 @@ void ProxyServer::onProxyClientAuthenticated() qCDebug(dcProxyServer()) << "Client authenticated" << proxyClient; - // Check if we already have a tunnel for this token - if (m_tunnels.contains(proxyClient->token())) { - // A new connection attempt with the same token, kill the old tunnel - // connection and allow the new connection to stablish the tunnel - qCWarning(dcProxyServer()) << "New authenticated client which already has a tunnel connection. Closing and clean up the old tunnel."; + //FIXME: limit the amount of connection with one token - TunnelConnection tunnel = m_tunnels.take(proxyClient->token()); - qCDebug(dcProxyServer()) << "Killing " << tunnel; - tunnel.clientOne()->killConnection("Clean up for new connection."); - tunnel.clientTwo()->killConnection("Clean up for new connection."); + // Check if we already have a tunnel with this identifier + if (m_tunnels.contains(proxyClient->tunnelIdentifier())) { + qCWarning(dcProxyServer()) << "There is already a tunnel with this token and nonce. The client has to take a new nonce or a new token."; + // There is already a tunnel with this token and nonce. Reject the connection + proxyClient->killConnection("Tunnel already exists for this token nonce combination."); + return; } + // FIXME: for backwards compatibility if (proxyClient->nonce().isEmpty()) { // Check if we have an other authenticated client with this token diff --git a/libnymea-remoteproxy/proxyserver.h b/libnymea-remoteproxy/proxyserver.h index 388c576..0199c83 100644 --- a/libnymea-remoteproxy/proxyserver.h +++ b/libnymea-remoteproxy/proxyserver.h @@ -57,7 +57,7 @@ private: // FIXME: Token, ProxyClient QHash m_authenticatedClients; - // Nonce, ProxyClient + // TunnelIdentifier (token + nonce), ProxyClient QHash m_authenticatedClientsNonce; // Token, Tunnel diff --git a/libnymea-remoteproxy/tunnelconnection.cpp b/libnymea-remoteproxy/tunnelconnection.cpp index 6269f09..9d441a0 100644 --- a/libnymea-remoteproxy/tunnelconnection.cpp +++ b/libnymea-remoteproxy/tunnelconnection.cpp @@ -40,6 +40,22 @@ QString TunnelConnection::token() const return m_clientOne->token(); } +QString TunnelConnection::nonce() const +{ + if (!isValid()) + return QString(); + + return m_clientOne->nonce(); +} + +QString TunnelConnection::tunnelIdentifier() const +{ + if (!isValid()) + return QString(); + + return m_clientOne->tunnelIdentifier(); +} + uint TunnelConnection::creationTime() const { return m_creationTimeStamp; diff --git a/libnymea-remoteproxy/tunnelconnection.h b/libnymea-remoteproxy/tunnelconnection.h index 8180d2e..9fe4386 100644 --- a/libnymea-remoteproxy/tunnelconnection.h +++ b/libnymea-remoteproxy/tunnelconnection.h @@ -32,6 +32,8 @@ public: TunnelConnection(ProxyClient *clientOne = nullptr, ProxyClient *clientTwo = nullptr); QString token() const; + QString nonce() const; + QString tunnelIdentifier() const; uint creationTime() const; QString creationTimeString() const; diff --git a/tests/test-offline/nymea-remoteproxy-tests-offline.cpp b/tests/test-offline/nymea-remoteproxy-tests-offline.cpp index fc7b240..4b6b0b7 100644 --- a/tests/test-offline/nymea-remoteproxy-tests-offline.cpp +++ b/tests/test-offline/nymea-remoteproxy-tests-offline.cpp @@ -693,6 +693,36 @@ void RemoteProxyOfflineTests::remoteConnection() stopServer(); } +void RemoteProxyOfflineTests::multipleRemoteConnection() +{ + // Start the server + startServer(); + + // Configure moch authenticator + m_mockAuthenticator->setExpectedAuthenticationError(); + m_mockAuthenticator->setTimeoutDuration(1000); + m_configuration->setAuthenticationTimeout(2000); + m_configuration->setJsonRpcTimeout(5000); + m_configuration->setInactiveTimeout(5000); + + // Create multiple tunnels with one token, but different nonces for each connection + QObject *parent = new QObject(this); + for (int i = 0; i < 5; i++) { + qDebug() << "============== Create remote connection" << i; + bool connectionResult = createRemoteConnection(m_testToken, QUuid::createUuid().toString(), parent); + if (!connectionResult) { + qWarning() << "Test failed because could not create remote connection"; + delete parent; + } + QVERIFY(connectionResult); + } + + delete parent; + + // Clean up + stopServer(); +} + void RemoteProxyOfflineTests::trippleConnection() { // Start the server @@ -702,6 +732,8 @@ void RemoteProxyOfflineTests::trippleConnection() m_mockAuthenticator->setTimeoutDuration(100); m_mockAuthenticator->setExpectedAuthenticationError(); + QString nonce = QUuid::createUuid().toString(); + // Create two connection RemoteProxyConnection *connectionOne = new RemoteProxyConnection(QUuid::createUuid(), "Test client one", this); connect(connectionOne, &RemoteProxyConnection::sslErrors, this, &BaseTest::ignoreConnectionSslError); @@ -714,7 +746,6 @@ void RemoteProxyOfflineTests::trippleConnection() // Connect one QSignalSpy connectionOneReadySpy(connectionOne, &RemoteProxyConnection::ready); - QSignalSpy connectionOneDisconnectedSpy(connectionOne, &RemoteProxyConnection::disconnected); QVERIFY(connectionOne->connectServer(m_serverUrl)); connectionOneReadySpy.wait(); QVERIFY(connectionOneReadySpy.count() == 1); @@ -722,7 +753,6 @@ void RemoteProxyOfflineTests::trippleConnection() // Connect two QSignalSpy connectionTwoReadySpy(connectionTwo, &RemoteProxyConnection::ready); - QSignalSpy connectionTwoDisconnectedSpy(connectionTwo, &RemoteProxyConnection::disconnected); QVERIFY(connectionTwo->connectServer(m_serverUrl)); connectionTwoReadySpy.wait(); QVERIFY(connectionTwoReadySpy.count() == 1); @@ -730,7 +760,7 @@ void RemoteProxyOfflineTests::trippleConnection() // Authenticate one QSignalSpy connectionOneAuthenticatedSpy(connectionOne, &RemoteProxyConnection::authenticated); - QVERIFY(connectionOne->authenticate(m_testToken)); + QVERIFY(connectionOne->authenticate(m_testToken, nonce)); connectionOneAuthenticatedSpy.wait(); QVERIFY(connectionOneAuthenticatedSpy.count() == 1); QVERIFY(connectionOne->isConnected()); @@ -742,7 +772,7 @@ void RemoteProxyOfflineTests::trippleConnection() // Authenticate two QSignalSpy remoteConnectionEstablishedTwo(connectionTwo, &RemoteProxyConnection::remoteConnectionEstablished); QSignalSpy connectionTwoAuthenticatedSpy(connectionTwo, &RemoteProxyConnection::authenticated); - QVERIFY(connectionTwo->authenticate(m_testToken)); + QVERIFY(connectionTwo->authenticate(m_testToken, nonce)); connectionTwoAuthenticatedSpy.wait(); QVERIFY(connectionTwoAuthenticatedSpy.count() == 1); QVERIFY(connectionTwo->isConnected()); @@ -751,10 +781,11 @@ void RemoteProxyOfflineTests::trippleConnection() // Wait for both to be connected remoteConnectionEstablishedOne.wait(500); - // Now connect a third connection and make sure the tunnel gets closed + // Now connect a third connection and make sure the client will be closed // Connect three QSignalSpy connectionThreeReadySpy(connectionThree, &RemoteProxyConnection::ready); + QSignalSpy connectionThreeDisconnectedSpy(connectionThree, &RemoteProxyConnection::disconnected); QVERIFY(connectionThree->connectServer(m_serverUrl)); connectionThreeReadySpy.wait(); QVERIFY(connectionThreeReadySpy.count() == 1); @@ -762,17 +793,16 @@ void RemoteProxyOfflineTests::trippleConnection() // Authenticate three QSignalSpy connectionThreeAuthenticatedSpy(connectionThree, &RemoteProxyConnection::authenticated); - QVERIFY(connectionThree->authenticate(m_testToken)); + QVERIFY(connectionThree->authenticate(m_testToken, nonce)); connectionThreeAuthenticatedSpy.wait(); QVERIFY(connectionOneAuthenticatedSpy.count() == 1); - connectionOneDisconnectedSpy.wait(200); - connectionTwoDisconnectedSpy.wait(200); + connectionThreeDisconnectedSpy.wait(200); + QVERIFY(connectionThreeDisconnectedSpy.count() >= 1); - QVERIFY(connectionOneDisconnectedSpy.count() >= 1); - QVERIFY(connectionTwoDisconnectedSpy.count() >= 1); - QVERIFY(connectionOne->state() == RemoteProxyConnection::StateDisconnected); - QVERIFY(connectionTwo->state() == RemoteProxyConnection::StateDisconnected); + // Make sure the one and two are still connected + QVERIFY(connectionOne->state() == RemoteProxyConnection::StateRemoteConnected); + QVERIFY(connectionTwo->state() == RemoteProxyConnection::StateRemoteConnected); // Clean up stopServer(); @@ -879,6 +909,9 @@ void RemoteProxyOfflineTests::jsonRpcTimeout() // Configure result (authentication takes longer than json rpc timeout m_mockAuthenticator->setExpectedAuthenticationError(); m_mockAuthenticator->setTimeoutDuration(4000); + m_configuration->setAuthenticationTimeout(4000); + m_configuration->setJsonRpcTimeout(1000); + m_configuration->setInactiveTimeout(2000); // Create request QVariantMap params; diff --git a/tests/test-offline/nymea-remoteproxy-tests-offline.h b/tests/test-offline/nymea-remoteproxy-tests-offline.h index 28b0d2a..a2b6618 100644 --- a/tests/test-offline/nymea-remoteproxy-tests-offline.h +++ b/tests/test-offline/nymea-remoteproxy-tests-offline.h @@ -64,6 +64,7 @@ private slots: // Client lib void clientConnection(); void remoteConnection(); + void multipleRemoteConnection(); void trippleConnection(); void duplicateUuid(); void sslConfigurations(); diff --git a/tests/testbase/basetest.cpp b/tests/testbase/basetest.cpp index acdcbac..f17dee8 100644 --- a/tests/testbase/basetest.cpp +++ b/tests/testbase/basetest.cpp @@ -189,6 +189,119 @@ QVariant BaseTest::injectSocketData(const QByteArray &data) return QVariant(); } +bool BaseTest::createRemoteConnection(const QString &token, const QString &nonce, QObject *parent) +{ + // Configure mock authenticator + m_mockAuthenticator->setTimeoutDuration(100); + m_mockAuthenticator->setExpectedAuthenticationError(); + + QString nameConnectionOne = "Test client one"; + QUuid uuidConnectionOne = QUuid::createUuid(); + + QString nameConnectionTwo = "Test client two"; + QUuid uuidConnectionTwo = QUuid::createUuid(); + + QByteArray dataOne = "Hello from client one :-)"; + QByteArray dataTwo = "Hello from client two :-)"; + + // Create two connection + RemoteProxyConnection *connectionOne = new RemoteProxyConnection(uuidConnectionOne, nameConnectionOne, parent); + connect(connectionOne, &RemoteProxyConnection::sslErrors, this, &BaseTest::ignoreConnectionSslError); + + RemoteProxyConnection *connectionTwo = new RemoteProxyConnection(uuidConnectionTwo, nameConnectionTwo, parent); + connect(connectionTwo, &RemoteProxyConnection::sslErrors, this, &BaseTest::ignoreConnectionSslError); + + // Connect one + QSignalSpy connectionOneReadySpy(connectionOne, &RemoteProxyConnection::ready); + if (!connectionOne->connectServer(m_serverUrl)) { + qWarning() << "Could not connect client one"; + return false; + } + + connectionOneReadySpy.wait(); + if (connectionOneReadySpy.count() != 1) { + qWarning() << "Could not connect client one"; + return false; + } + + if (!connectionOne->isConnected()) { + qWarning() << "Could not connect client one"; + return false; + } + + // Connect two + QSignalSpy connectionTwoReadySpy(connectionTwo, &RemoteProxyConnection::ready); + if (!connectionTwo->connectServer(m_serverUrl)) { + qWarning() << "Could not connect client two"; + return false; + } + + connectionTwoReadySpy.wait(); + if (connectionTwoReadySpy.count() != 1) { + qWarning() << "Could not connect client two"; + return false; + } + + if (!connectionTwo->isConnected()) { + qWarning() << "Could not connect client two"; + return false; + } + + // Authenticate one + QSignalSpy remoteConnectionEstablishedOne(connectionOne, &RemoteProxyConnection::remoteConnectionEstablished); + QSignalSpy connectionOneAuthenticatedSpy(connectionOne, &RemoteProxyConnection::authenticated); + if (!connectionOne->authenticate(token, nonce)) { + qWarning() << "Could not authenticate client one"; + return false; + } + + connectionOneAuthenticatedSpy.wait(500); + if (connectionOneAuthenticatedSpy.count() != 1) { + qWarning() << "Could not authenticate client one"; + return false; + } + + if (connectionOne->state() != RemoteProxyConnection::StateAuthenticated) { + qWarning() << "Could not authenticate client one"; + return false; + } + + // Authenticate two + QSignalSpy remoteConnectionEstablishedTwo(connectionTwo, &RemoteProxyConnection::remoteConnectionEstablished); + QSignalSpy connectionTwoAuthenticatedSpy(connectionTwo, &RemoteProxyConnection::authenticated); + if (!connectionTwo->authenticate(token, nonce)) { + qWarning() << "Could not authenticate client two"; + return false; + } + + connectionTwoAuthenticatedSpy.wait(500); + if (connectionTwoAuthenticatedSpy.count() != 1) { + qWarning() << "Could not authenticate client two"; + return false; + } + + if (connectionTwo->state() != RemoteProxyConnection::StateAuthenticated && connectionTwo->state() != RemoteProxyConnection::StateRemoteConnected) { + qWarning() << "Could not authenticate client two"; + return false; + } + + // Wait for both to be connected + remoteConnectionEstablishedOne.wait(500); + remoteConnectionEstablishedTwo.wait(500); + + if (remoteConnectionEstablishedOne.count() != 1 || remoteConnectionEstablishedTwo.count() != 1) { + qWarning() << "Could not establish remote connection"; + return false; + } + + if (connectionOne->state() != RemoteProxyConnection::StateRemoteConnected || connectionTwo->state() != RemoteProxyConnection::StateRemoteConnected) { + qWarning() << "Could not establish remote connection"; + return false; + } + + return true; +} + void BaseTest::initTestCase() { qRegisterMetaType(); diff --git a/tests/testbase/basetest.h b/tests/testbase/basetest.h index b0aa1ed..511b69a 100644 --- a/tests/testbase/basetest.h +++ b/tests/testbase/basetest.h @@ -75,6 +75,8 @@ protected: QVariant invokeApiCall(const QString &method, const QVariantMap params = QVariantMap(), bool remainsConnected = true); QVariant injectSocketData(const QByteArray &data); + bool createRemoteConnection(const QString &token, const QString &nonce, QObject *parent); + protected slots: void initTestCase(); void cleanupTestCase();