Merge pull request #12195 from birarda/bug/connection-refused

clear silent connection attempt counter on DomainHandler soft reset

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
@@ -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

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(), SIGNAL(limitOfSilentDomainCheckInsReached()),
+            nodeList.data(), SLOT(reset()));
 
     auto dialogsManager = DependencyManager::get<DialogsManager>();
     connect(accountManager.data(), &AccountManager::authRequired, dialogsManager.data(), &DialogsManager::showLoginDialog);

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");
@@ -382,6 +383,9 @@ void DomainHandler::processDomainServerConnectionDeniedPacket(QSharedPointer<Rec
     // we're hearing from this domain-server, don't need to refresh API info
     _apiRefreshTimer.stop();
 
+    // this counts as a reply from the DS after a check in or connect packet, so reset that counter now
+    _checkInPacketsSinceLastReply = 0;
+
     // Read deny reason from packet
     uint8_t reasonCodeWire;
 
@@ -426,3 +430,14 @@ void DomainHandler::processDomainServerConnectionDeniedPacket(QSharedPointer<Rec
         }
     }
 }
+
+void DomainHandler::sentCheckInPacket() {
+    ++_checkInPacketsSinceLastReply;
+
+    if (_checkInPacketsSinceLastReply >= 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();
+    }
+}

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
      * <p>The reasons that you may be refused connection to a domain are defined by numeric values:</p>
      * <table>
@@ -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<QString> _domainConnectionRefusals;
     bool _hasCheckedForAccessToken { false };
     int _connectionDenialsSinceKeypairRegen { 0 };
+    int _checkInPacketsSinceLastReply { 0 };
 
     QTimer _apiRefreshTimer;
 };

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)
 {
@@ -75,7 +74,7 @@ NodeList::NodeList(char newOwnerType, int socketListenPort, int dtlsListenPort)
     connect(this, &LimitedNodeList::publicSockAddrChanged, this, &NodeList::sendDomainServerCheckIn);
 
     // clear our NodeList when the domain changes
-    connect(&_domainHandler, &DomainHandler::disconnectedFromDomain, this, &NodeList::reset);
+    connect(&_domainHandler, SIGNAL(disconnectedFromDomain()), this, SLOT(resetFromDomainHandler()));
 
     // send an ICE heartbeat as soon as we get ice server information
     connect(&_domainHandler, &DomainHandler::iceSocketAndIDReceived, this, &NodeList::handleICEConnectionToDomainServer);
@@ -92,10 +91,10 @@ NodeList::NodeList(char newOwnerType, int socketListenPort, int dtlsListenPort)
     connect(accountManager.data(), &AccountManager::newKeypair, this, &NodeList::sendDomainServerCheckIn);
 
     // clear out NodeList when login is finished
-    connect(accountManager.data(), &AccountManager::loginComplete , this, &NodeList::reset);
+    connect(accountManager.data(), SIGNAL(loginComplete()) , this, SLOT(reset()));
 
     // clear our NodeList when logout is requested
-    connect(accountManager.data(), &AccountManager::logoutComplete , this, &NodeList::reset);
+    connect(accountManager.data(), SIGNAL(logoutComplete()) , this, SLOT(reset()));
 
     // anytime we get a new node we will want to attempt to punch to it
     connect(this, &LimitedNodeList::nodeAdded, this, &NodeList::startNodeHolePunch);
@@ -231,16 +230,14 @@ void NodeList::processICEPingPacket(QSharedPointer<ReceivedMessage> 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;
     }
 
     LimitedNodeList::reset();
 
-    _numNoReplyDomainCheckIns = 0;
-
     // lock and clear our set of ignored IDs
     _ignoredSetLock.lockForWrite();
     _ignoredNodeIDs.clear();
@@ -255,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();
     }
@@ -410,15 +407,8 @@ void NodeList::sendDomainServerCheckIn() {
 
         sendPacket(std::move(domainPacket), _domainHandler.getSockAddr());
 
-        if (_numNoReplyDomainCheckIns >= MAX_SILENT_DOMAIN_SERVER_CHECK_INS) {
+        // let the domain handler know we sent another check in or connect packet
-            // we haven't heard back from DS in MAX_SILENT_DOMAIN_SERVER_CHECK_INS
+        _domainHandler.sentCheckInPacket();
-            // 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++;
     }
 }
 
@@ -585,7 +575,7 @@ void NodeList::processDomainServerList(QSharedPointer<ReceivedMessage> 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();

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<NLPacket>, std::unique_ptr<NLPacketList>>;
 using NodePacketOrPacketListPair = std::pair<SharedNodePointer, PacketOrPacketList>;
 
@@ -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; }
@@ -96,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);
 
@@ -119,7 +118,6 @@ public slots:
 #endif
 
 signals:
-    void limitOfSilentDomainCheckInsReached();
     void receivedDomainServerList();
     void ignoredNode(const QUuid& nodeID, bool enabled);
     void ignoreRadiusEnabledChanged(bool isIgnored);
@@ -161,7 +159,6 @@ private:
     std::atomic<NodeType_t> _ownerType;
     NodeSet _nodeTypesOfInterest;
     DomainHandler _domainHandler;
-    int _numNoReplyDomainCheckIns;
     HifiSockAddr _assignmentServerSocket;
     bool _isShuttingDown { false };
     QTimer _keepAlivePingTimer;