From e16a3859c2a67df3b277af554088406b514f42f2 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Wed, 17 Jan 2018 11:55:46 -0800 Subject: [PATCH 1/4] reset silent check in packet check on domain connection reset --- interface/src/Application.cpp | 3 ++- libraries/networking/src/DomainHandler.cpp | 12 ++++++++++++ libraries/networking/src/DomainHandler.h | 9 +++++++++ libraries/networking/src/NodeList.cpp | 16 +++------------- libraries/networking/src/NodeList.h | 5 ----- 5 files changed, 26 insertions(+), 19 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 4f4c528abf..e12f16d3cd 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -1046,7 +1046,8 @@ Application::Application(int& argc, char** argv, QElapsedTimer& startupTimer, bo connect(nodeList.data(), &NodeList::packetVersionMismatch, this, &Application::notifyPacketVersionMismatch); // you might think we could just do this in NodeList but we only want this connection for Interface - connect(nodeList.data(), &NodeList::limitOfSilentDomainCheckInsReached, nodeList.data(), &NodeList::reset); + connect(&nodeList->getDomainHandler(), &DomainHandler::limitOfSilentDomainCheckInsReached, + nodeList.data(), &NodeList::reset); auto dialogsManager = DependencyManager::get(); connect(accountManager.data(), &AccountManager::authRequired, dialogsManager.data(), &DialogsManager::showLoginDialog); diff --git a/libraries/networking/src/DomainHandler.cpp b/libraries/networking/src/DomainHandler.cpp index 3c20c3798f..bc56389810 100644 --- a/libraries/networking/src/DomainHandler.cpp +++ b/libraries/networking/src/DomainHandler.cpp @@ -98,6 +98,7 @@ void DomainHandler::softReset() { clearSettings(); _connectionDenialsSinceKeypairRegen = 0; + _checkInPacketsSinceLastReply = 0; // cancel the failure timeout for any pending requests for settings QMetaObject::invokeMethod(&_settingsTimer, "stop"); @@ -426,3 +427,14 @@ void DomainHandler::processDomainServerConnectionDeniedPacket(QSharedPointer= MAX_SILENT_DOMAIN_SERVER_CHECK_INS) { + // we haven't heard back from DS in MAX_SILENT_DOMAIN_SERVER_CHECK_INS + // so emit our signal that says that + qCDebug(networking) << "Limit of silent domain checkins reached"; + emit limitOfSilentDomainCheckInsReached(); + } +} diff --git a/libraries/networking/src/DomainHandler.h b/libraries/networking/src/DomainHandler.h index 42ef2f627d..b72c172c3e 100644 --- a/libraries/networking/src/DomainHandler.h +++ b/libraries/networking/src/DomainHandler.h @@ -31,6 +31,8 @@ const unsigned short DEFAULT_DOMAIN_SERVER_DTLS_PORT = 40103; const quint16 DOMAIN_SERVER_HTTP_PORT = 40100; const quint16 DOMAIN_SERVER_HTTPS_PORT = 40101; +const int MAX_SILENT_DOMAIN_SERVER_CHECK_INS = 5; + class DomainHandler : public QObject { Q_OBJECT public: @@ -84,6 +86,10 @@ public: void softReset(); + int getCheckInPacketsSinceLastReply() const { return _checkInPacketsSinceLastReply; } + void sentCheckInPacket(); + void domainListReceived() { _checkInPacketsSinceLastReply = 0; } + /**jsdoc *

The reasons that you may be refused connection to a domain are defined by numeric values:

* @@ -165,6 +171,8 @@ signals: void domainConnectionRefused(QString reasonMessage, int reason, const QString& extraInfo); + void limitOfSilentDomainCheckInsReached(); + private: bool reasonSuggestsLogin(ConnectionRefusedReason reasonCode); void sendDisconnectPacket(); @@ -187,6 +195,7 @@ private: QSet _domainConnectionRefusals; bool _hasCheckedForAccessToken { false }; int _connectionDenialsSinceKeypairRegen { 0 }; + int _checkInPacketsSinceLastReply { 0 }; QTimer _apiRefreshTimer; }; diff --git a/libraries/networking/src/NodeList.cpp b/libraries/networking/src/NodeList.cpp index 1c18125433..ff9af91060 100644 --- a/libraries/networking/src/NodeList.cpp +++ b/libraries/networking/src/NodeList.cpp @@ -44,7 +44,6 @@ NodeList::NodeList(char newOwnerType, int socketListenPort, int dtlsListenPort) _ownerType(newOwnerType), _nodeTypesOfInterest(), _domainHandler(this), - _numNoReplyDomainCheckIns(0), _assignmentServerSocket(), _keepAlivePingTimer(this) { @@ -239,8 +238,6 @@ void NodeList::reset() { LimitedNodeList::reset(); - _numNoReplyDomainCheckIns = 0; - // lock and clear our set of ignored IDs _ignoredSetLock.lockForWrite(); _ignoredNodeIDs.clear(); @@ -410,15 +407,8 @@ void NodeList::sendDomainServerCheckIn() { sendPacket(std::move(domainPacket), _domainHandler.getSockAddr()); - if (_numNoReplyDomainCheckIns >= MAX_SILENT_DOMAIN_SERVER_CHECK_INS) { - // we haven't heard back from DS in MAX_SILENT_DOMAIN_SERVER_CHECK_INS - // so emit our signal that says that - qCDebug(networking) << "Limit of silent domain checkins reached"; - emit limitOfSilentDomainCheckInsReached(); - } - - // increment the count of un-replied check-ins - _numNoReplyDomainCheckIns++; + // let the domain handler know we sent another check in or connect packet + _domainHandler.sentCheckInPacket(); } } @@ -585,7 +575,7 @@ void NodeList::processDomainServerList(QSharedPointer message) } // this is a packet from the domain server, reset the count of un-replied check-ins - _numNoReplyDomainCheckIns = 0; + _domainHandler.domainListReceived(); // emit our signal so listeners know we just heard from the DS emit receivedDomainServerList(); diff --git a/libraries/networking/src/NodeList.h b/libraries/networking/src/NodeList.h index 0ebc1f0b22..081c10b97d 100644 --- a/libraries/networking/src/NodeList.h +++ b/libraries/networking/src/NodeList.h @@ -38,8 +38,6 @@ const quint64 DOMAIN_SERVER_CHECK_IN_MSECS = 1 * 1000; -const int MAX_SILENT_DOMAIN_SERVER_CHECK_INS = 5; - using PacketOrPacketList = std::pair, std::unique_ptr>; using NodePacketOrPacketListPair = std::pair; @@ -62,7 +60,6 @@ public: Q_INVOKABLE qint64 sendStats(QJsonObject statsObject, HifiSockAddr destination); Q_INVOKABLE qint64 sendStatsToDomainServer(QJsonObject statsObject); - int getNumNoReplyDomainCheckIns() const { return _numNoReplyDomainCheckIns; } DomainHandler& getDomainHandler() { return _domainHandler; } const NodeSet& getNodeInterestSet() const { return _nodeTypesOfInterest; } @@ -119,7 +116,6 @@ public slots: #endif signals: - void limitOfSilentDomainCheckInsReached(); void receivedDomainServerList(); void ignoredNode(const QUuid& nodeID, bool enabled); void ignoreRadiusEnabledChanged(bool isIgnored); @@ -161,7 +157,6 @@ private: std::atomic _ownerType; NodeSet _nodeTypesOfInterest; DomainHandler _domainHandler; - int _numNoReplyDomainCheckIns; HifiSockAddr _assignmentServerSocket; bool _isShuttingDown { false }; QTimer _keepAlivePingTimer; From 98d581acd19f64d47a987dd0386f8d471ee1efa5 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Wed, 17 Jan 2018 12:04:21 -0800 Subject: [PATCH 2/4] fix debug lines in DomainGatekeeper with extra parens --- domain-server/src/DomainGatekeeper.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/domain-server/src/DomainGatekeeper.cpp b/domain-server/src/DomainGatekeeper.cpp index c6eaa03634..9c3ea60499 100644 --- a/domain-server/src/DomainGatekeeper.cpp +++ b/domain-server/src/DomainGatekeeper.cpp @@ -161,7 +161,7 @@ NodePermissions DomainGatekeeper::setPermissionsForUser(bool isLocalUser, QStrin } else if (_server->_settingsManager.hasPermissionsForMachineFingerprint(machineFingerprint)) { userPerms = _server->_settingsManager.getPermissionsForMachineFingerprint(machineFingerprint); #ifdef WANT_DEBUG - qDebug(() << "| user-permissions: specific Machine Fingerprint matches, so: " << userPerms; + qDebug() << "| user-permissions: specific Machine Fingerprint matches, so: " << userPerms; #endif } else if (_server->_settingsManager.hasPermissionsForIP(senderAddress)) { // this user comes from an IP we have in our permissions table, apply those permissions @@ -187,7 +187,7 @@ NodePermissions DomainGatekeeper::setPermissionsForUser(bool isLocalUser, QStrin } else if (_server->_settingsManager.hasPermissionsForMachineFingerprint(machineFingerprint)) { userPerms = _server->_settingsManager.getPermissionsForMachineFingerprint(machineFingerprint); #ifdef WANT_DEBUG - qDebug(() << "| user-permissions: specific Machine Fingerprint matches, so: " << userPerms; + qDebug() << "| user-permissions: specific Machine Fingerprint matches, so: " << userPerms; #endif } else if (_server->_settingsManager.hasPermissionsForIP(senderAddress)) { // this user comes from an IP we have in our permissions table, apply those permissions From eb9d100fc8777e1eadb872c2fb63e6af9f1e0285 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Wed, 17 Jan 2018 13:29:20 -0800 Subject: [PATCH 3/4] reset count on conn refusal, soft reset DH in Interface on limit reached --- interface/src/Application.cpp | 4 ++-- libraries/networking/src/DomainHandler.cpp | 3 +++ libraries/networking/src/NodeList.cpp | 12 ++++++------ libraries/networking/src/NodeList.h | 4 +++- 4 files changed, 14 insertions(+), 9 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index e12f16d3cd..ae6b985821 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -1046,8 +1046,8 @@ Application::Application(int& argc, char** argv, QElapsedTimer& startupTimer, bo connect(nodeList.data(), &NodeList::packetVersionMismatch, this, &Application::notifyPacketVersionMismatch); // you might think we could just do this in NodeList but we only want this connection for Interface - connect(&nodeList->getDomainHandler(), &DomainHandler::limitOfSilentDomainCheckInsReached, - nodeList.data(), &NodeList::reset); + connect(&nodeList->getDomainHandler(), SIGNAL(limitOfSilentDomainCheckInsReached()), + nodeList.data(), SLOT(reset())); auto dialogsManager = DependencyManager::get(); connect(accountManager.data(), &AccountManager::authRequired, dialogsManager.data(), &DialogsManager::showLoginDialog); diff --git a/libraries/networking/src/DomainHandler.cpp b/libraries/networking/src/DomainHandler.cpp index bc56389810..411f8f5be2 100644 --- a/libraries/networking/src/DomainHandler.cpp +++ b/libraries/networking/src/DomainHandler.cpp @@ -383,6 +383,9 @@ void DomainHandler::processDomainServerConnectionDeniedPacket(QSharedPointer message) { sendPacket(std::move(replyPacket), message->getSenderSockAddr()); } -void NodeList::reset() { +void NodeList::reset(bool skipDomainHandlerReset) { if (thread() != QThread::currentThread()) { - QMetaObject::invokeMethod(this, "reset"); + QMetaObject::invokeMethod(this, "reset", Q_ARG(bool, skipDomainHandlerReset)); return; } @@ -252,7 +252,7 @@ void NodeList::reset() { _avatarGainMap.clear(); _avatarGainMapLock.unlock(); - if (sender() != &_domainHandler) { + if (!skipDomainHandlerReset) { // clear the domain connection information, unless they're the ones that asked us to reset _domainHandler.softReset(); } diff --git a/libraries/networking/src/NodeList.h b/libraries/networking/src/NodeList.h index 081c10b97d..2dd9d3c869 100644 --- a/libraries/networking/src/NodeList.h +++ b/libraries/networking/src/NodeList.h @@ -93,7 +93,9 @@ public: void removeFromIgnoreMuteSets(const QUuid& nodeID); public slots: - void reset(); + void reset(bool skipDomainHandlerReset = false); + void resetFromDomainHandler() { reset(true); } + void sendDomainServerCheckIn(); void handleDSPathQuery(const QString& newPath); From 7a21ae8d89b9f3510adab017c9df6ddda7394e08 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Fri, 2 Feb 2018 15:25:43 -0800 Subject: [PATCH 4/4] re-send a connection token packet if the current one is null --- domain-server/src/DomainGatekeeper.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/domain-server/src/DomainGatekeeper.cpp b/domain-server/src/DomainGatekeeper.cpp index 9c3ea60499..3aab7b4563 100644 --- a/domain-server/src/DomainGatekeeper.cpp +++ b/domain-server/src/DomainGatekeeper.cpp @@ -393,9 +393,12 @@ SharedNodePointer DomainGatekeeper::processAgentConnectRequest(const NodeConnect QString verifiedUsername; // if this remains empty, consider this an anonymous connection attempt if (!username.isEmpty()) { - if (usernameSignature.isEmpty()) { + const QUuid& connectionToken = _connectionTokenHash.value(username.toLower()); + + if (usernameSignature.isEmpty() || connectionToken.isNull()) { // user is attempting to prove their identity to us, but we don't have enough information sendConnectionTokenPacket(username, nodeConnection.senderSockAddr); + // ask for their public key right now to make sure we have it requestUserPublicKey(username, true); getGroupMemberships(username); // optimistically get started on group memberships