From f963adb5dd74a26a4cf22d46c338a978f50109b2 Mon Sep 17 00:00:00 2001
From: Stephen Birarda <commit@birarda.com>
Date: Wed, 8 Jun 2016 11:47:58 -0700
Subject: [PATCH] send connection denial for connect version mismatch

---
 domain-server/src/DomainGatekeeper.cpp         | 16 ++++++++++------
 domain-server/src/DomainGatekeeper.h           |  6 ++++--
 domain-server/src/DomainServer.cpp             | 15 +++++----------
 libraries/networking/src/udt/PacketHeaders.cpp |  2 +-
 4 files changed, 20 insertions(+), 19 deletions(-)

diff --git a/domain-server/src/DomainGatekeeper.cpp b/domain-server/src/DomainGatekeeper.cpp
index b940d46849..680b5c277b 100644
--- a/domain-server/src/DomainGatekeeper.cpp
+++ b/domain-server/src/DomainGatekeeper.cpp
@@ -62,10 +62,7 @@ void DomainGatekeeper::processConnectRequestPacket(QSharedPointer<ReceivedMessag
 
     QByteArray myProtocolVersion = protocolVersionsSignature();
     if (nodeConnection.protocolVersion != myProtocolVersion) {
-        QString protocolVersionError = "Protocol version mismatch - Domain version:" + QCoreApplication::applicationVersion();
-        qDebug() << "Protocol Version mismatch - denying connection.";
-        sendConnectionDeniedPacket(protocolVersionError, message->getSenderSockAddr(), 
-                DomainHandler::ConnectionRefusedReason::ProtocolMismatch);
+        sendProtocolMismatchConnectionDenial(message->getSenderSockAddr());
         return;
     }
     
@@ -123,6 +120,13 @@ void DomainGatekeeper::processConnectRequestPacket(QSharedPointer<ReceivedMessag
     }
 }
 
+void DomainGatekeeper::sendProtocolMismatchConnectionDenial(const HifiSockAddr& senderSockAddr) {
+    QString protocolVersionError = "Protocol version mismatch - Domain version:" + QCoreApplication::applicationVersion();
+    qDebug() << "Protocol Version mismatch - denying connection.";
+    sendConnectionDeniedPacket(protocolVersionError, senderSockAddr,
+                               DomainHandler::ConnectionRefusedReason::ProtocolMismatch);
+}
+
 SharedNodePointer DomainGatekeeper::processAssignmentConnectRequest(const NodeConnectionData& nodeConnection,
                                                                     const PendingAssignedNodeData& pendingAssignment) {
     
@@ -531,14 +535,14 @@ void DomainGatekeeper::publicKeyJSONCallback(QNetworkReply& requestReply) {
 }
 
 void DomainGatekeeper::sendConnectionDeniedPacket(const QString& reason, const HifiSockAddr& senderSockAddr, 
-            DomainHandler::ConnectionRefusedReason reasonCode) {
+                                                  DomainHandler::ConnectionRefusedReason reasonCode) {
     // this is an agent and we've decided we won't let them connect - send them a packet to deny connection
     QByteArray utfString = reason.toUtf8();
     quint16 payloadSize = utfString.size();
    
     // setup the DomainConnectionDenied packet
     auto connectionDeniedPacket = NLPacket::create(PacketType::DomainConnectionDenied, 
-                                                payloadSize + sizeof(payloadSize) + sizeof(uint8_t));
+                                                   payloadSize + sizeof(payloadSize) + sizeof(uint8_t));
     
     // pack in the reason the connection was denied (the client displays this)
     if (payloadSize > 0) {
diff --git a/domain-server/src/DomainGatekeeper.h b/domain-server/src/DomainGatekeeper.h
index 09e3b04ed7..237f8d3185 100644
--- a/domain-server/src/DomainGatekeeper.h
+++ b/domain-server/src/DomainGatekeeper.h
@@ -42,6 +42,8 @@ public:
     void preloadAllowedUserPublicKeys();
     
     void removeICEPeer(const QUuid& peerUUID) { _icePeers.remove(peerUUID); }
+
+    static void sendProtocolMismatchConnectionDenial(const HifiSockAddr& senderSockAddr);
 public slots:
     void processConnectRequestPacket(QSharedPointer<ReceivedMessage> message);
     void processICEPingPacket(QSharedPointer<ReceivedMessage> message);
@@ -76,8 +78,8 @@ private:
                                        const HifiSockAddr& senderSockAddr);
     
     void sendConnectionTokenPacket(const QString& username, const HifiSockAddr& senderSockAddr);
-    void sendConnectionDeniedPacket(const QString& reason, const HifiSockAddr& senderSockAddr, 
-                    DomainHandler::ConnectionRefusedReason reasonCode = DomainHandler::ConnectionRefusedReason::Unknown);
+    static void sendConnectionDeniedPacket(const QString& reason, const HifiSockAddr& senderSockAddr,
+            DomainHandler::ConnectionRefusedReason reasonCode = DomainHandler::ConnectionRefusedReason::Unknown);
     
     void pingPunchForConnectingPeer(const SharedNetworkPeer& peer);
     
diff --git a/domain-server/src/DomainServer.cpp b/domain-server/src/DomainServer.cpp
index c73c55579b..7c20817353 100644
--- a/domain-server/src/DomainServer.cpp
+++ b/domain-server/src/DomainServer.cpp
@@ -318,16 +318,11 @@ bool DomainServer::packetVersionMatch(const udt::Packet& packet) {
 
     auto nodeList = DependencyManager::get<LimitedNodeList>();
 
-    // This implements a special case that handles OLD clients which don't know how to negotiate matching
-    // protocol versions. We know these clients will sent DomainConnectRequest with older versions. We also
-    // know these clients will show a warning dialog if they get an EntityData with a protocol version they
-    // don't understand, so we can send them an empty EntityData with our latest version and they will
-    // warn the user that the protocol is not compatible
-    if (headerType == PacketType::DomainConnectRequest &&
-        headerVersion <static_cast<PacketVersion>(DomainConnectRequestVersion::HasProtocolVersions)) {
-        auto packetWithBadVersion = NLPacket::create(PacketType::EntityData);
-        nodeList->sendPacket(std::move(packetWithBadVersion), packet.getSenderSockAddr());
-        return false;
+    // if this is a mismatching connect packet, we can't simply drop it on the floor
+    // send back a packet to the interface that tells them we refuse connection for a mismatch
+    if (headerType == PacketType::DomainConnectRequest
+        && headerVersion != versionForPacketType(PacketType::DomainConnectRequest)) {
+        DomainGatekeeper::sendProtocolMismatchConnectionDenial(packet.getSenderSockAddr());
     }
 
     // let the normal nodeList implementation handle all other packets.
diff --git a/libraries/networking/src/udt/PacketHeaders.cpp b/libraries/networking/src/udt/PacketHeaders.cpp
index db743f81e4..a891b1e422 100644
--- a/libraries/networking/src/udt/PacketHeaders.cpp
+++ b/libraries/networking/src/udt/PacketHeaders.cpp
@@ -67,7 +67,7 @@ PacketVersion versionForPacketType(PacketType packetType) {
             return static_cast<PacketVersion>(DomainConnectionDeniedVersion::IncludesReasonCode);
 
         case PacketType::DomainConnectRequest:
-            return static_cast<PacketVersion>(DomainConnectRequestVersion::HasProtocolVersions);
+            return static_cast<PacketVersion>(DomainConnectRequestVersion::HasHostname);
 
         default:
             return 17;