From c122b22dfc7c1af8d0b5c82831433f73017c2359 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Wed, 23 Aug 2017 15:32:25 -0700 Subject: [PATCH] add additional IP check to DS packet filter operator --- domain-server/src/DomainGatekeeper.cpp | 1 + domain-server/src/DomainServer.cpp | 50 ++++++++++++++++++-- domain-server/src/DomainServer.h | 4 +- libraries/networking/src/HifiSockAddr.cpp | 12 +++++ libraries/networking/src/HifiSockAddr.h | 2 + libraries/networking/src/LimitedNodeList.cpp | 22 +++++---- libraries/networking/src/LimitedNodeList.h | 6 ++- 7 files changed, 80 insertions(+), 17 deletions(-) diff --git a/domain-server/src/DomainGatekeeper.cpp b/domain-server/src/DomainGatekeeper.cpp index 6951a90261..620d593ebc 100644 --- a/domain-server/src/DomainGatekeeper.cpp +++ b/domain-server/src/DomainGatekeeper.cpp @@ -54,6 +54,7 @@ void DomainGatekeeper::processConnectRequestPacket(QSharedPointergetSize() == 0) { return; } + QDataStream packetStream(message->getMessage()); // read a NodeConnectionData object from the packet so we can pass around this data while we're inspecting it diff --git a/domain-server/src/DomainServer.cpp b/domain-server/src/DomainServer.cpp index 163bd48f1b..3171fc066d 100644 --- a/domain-server/src/DomainServer.cpp +++ b/domain-server/src/DomainServer.cpp @@ -458,7 +458,7 @@ const QString DISABLED_AUTOMATIC_NETWORKING_VALUE = "disabled"; -bool DomainServer::packetVersionMatch(const udt::Packet& packet) { +bool DomainServer::isPacketVerified(const udt::Packet& packet) { PacketType headerType = NLPacket::typeInHeader(packet); PacketVersion headerVersion = NLPacket::versionInHeader(packet); @@ -471,7 +471,50 @@ bool DomainServer::packetVersionMatch(const udt::Packet& packet) { DomainGatekeeper::sendProtocolMismatchConnectionDenial(packet.getSenderSockAddr()); } - // let the normal nodeList implementation handle all other packets. + if (!PacketTypeEnum::getNonSourcedPackets().contains(headerType)) { + // this is a sourced packet - first check if we have a node that matches + QUuid sourceID = NLPacket::sourceIDInHeader(packet); + SharedNodePointer sourceNode = nodeList->nodeWithUUID(sourceID); + + if (sourceNode) { + // unverified DS packets (due to a lack of connection secret between DS + node) + // must come either from the same public IP address or a local IP address (set by RFC 1918) + + DomainServerNodeData* nodeData = static_cast(sourceNode->getLinkedData()); + + bool exactAddressMatch = nodeData->getSendingSockAddr() == packet.getSenderSockAddr(); + bool bothPrivateAddresses = nodeData->getSendingSockAddr().hasPrivateAddress() + && packet.getSenderSockAddr().hasPrivateAddress(); + + qDebug() << exactAddressMatch << bothPrivateAddresses; + + if (nodeData && (exactAddressMatch || bothPrivateAddresses)) { + // to the best of our ability we've verified that this packet comes from the right place + // let the NodeList do its checks now (but pass it the sourceNode so it doesn't need to look it up again) + return nodeList->isPacketVerifiedWithSource(packet, sourceNode.data()); + } else { + static const QString UNKNOWN_REGEX = "Packet of type \\d+ \\([\\sa-zA-Z:]+\\) received from unmatched IP for UUID"; + static QString repeatedMessage + = LogHandler::getInstance().addRepeatedMessageRegex(UNKNOWN_REGEX); + + qDebug() << "Packet of type" << headerType + << "received from unmatched IP for UUID" << uuidStringWithoutCurlyBraces(sourceID); + + return false; + } + } else { + static const QString UNKNOWN_REGEX = "Packet of type \\d+ \\([\\sa-zA-Z:]+\\) received from unknown node with UUID"; + static QString repeatedMessage + = LogHandler::getInstance().addRepeatedMessageRegex(UNKNOWN_REGEX); + + qDebug() << "Packet of type" << headerType + << "received from unknown node with UUID" << uuidStringWithoutCurlyBraces(sourceID); + + return false; + } + } + + // fallback to allow the normal NodeList implementation to verify packets return nodeList->isPacketVerified(packet); } @@ -570,7 +613,7 @@ void DomainServer::setupNodeListAndAssignments() { addStaticAssignmentsToQueue(); // set a custom packetVersionMatch as the verify packet operator for the udt::Socket - nodeList->setPacketFilterOperator(&DomainServer::packetVersionMatch); + nodeList->setPacketFilterOperator(&DomainServer::isPacketVerified); } const QString ACCESS_TOKEN_KEY_PATH = "metaverse.access_token"; @@ -853,7 +896,6 @@ void DomainServer::populateDefaultStaticAssignmentsExcludingTypes(const QSet message, SharedNodePointer sendingNode) { - QDataStream packetStream(message->getMessage()); NodeConnectionData nodeRequestData = NodeConnectionData::fromDataStream(packetStream, message->getSenderSockAddr(), false); diff --git a/domain-server/src/DomainServer.h b/domain-server/src/DomainServer.h index 4808297c89..03ad76d313 100644 --- a/domain-server/src/DomainServer.h +++ b/domain-server/src/DomainServer.h @@ -71,6 +71,7 @@ public slots: void restart(); +private slots: void processRequestAssignmentPacket(QSharedPointer packet); void processListRequestPacket(QSharedPointer packet, SharedNodePointer sendingNode); void processNodeJSONStatsPacket(QSharedPointer packetList, SharedNodePointer sendingNode); @@ -79,7 +80,6 @@ public slots: void processICEServerHeartbeatDenialPacket(QSharedPointer message); void processICEServerHeartbeatACK(QSharedPointer message); -private slots: void setupPendingAssignmentCredits(); void sendPendingTransactionsToServer(); @@ -129,7 +129,7 @@ private: void getTemporaryName(bool force = false); - static bool packetVersionMatch(const udt::Packet& packet); + static bool isPacketVerified(const udt::Packet& packet); bool resetAccountManagerAccessToken(); diff --git a/libraries/networking/src/HifiSockAddr.cpp b/libraries/networking/src/HifiSockAddr.cpp index 217d3096f5..a053610df0 100644 --- a/libraries/networking/src/HifiSockAddr.cpp +++ b/libraries/networking/src/HifiSockAddr.cpp @@ -113,6 +113,18 @@ QString HifiSockAddr::toString() const { return _address.toString() + ":" + QString::number(_port); } +bool HifiSockAddr::hasPrivateAddress() const { + // an address is private if it falls in any of the RFC1918 address spaces + const QPair TWENTY_FOUR_BIT_BLOCK = { QHostAddress("10.0.0.0"), 8 }; + const QPair TWENTY_BIT_BLOCK = { QHostAddress("172.16.0.0") , 12 }; + const QPair SIXTEEN_BIT_BLOCK = { QHostAddress("192.168.0.0"), 16 }; + + return _address.isLoopback() + || _address.isInSubnet(TWENTY_FOUR_BIT_BLOCK) + || _address.isInSubnet(TWENTY_BIT_BLOCK) + || _address.isInSubnet(SIXTEEN_BIT_BLOCK); +} + QDebug operator<<(QDebug debug, const HifiSockAddr& sockAddr) { debug.nospace() << sockAddr._address.toString().toLocal8Bit().constData() << ":" << sockAddr._port; return debug.space(); diff --git a/libraries/networking/src/HifiSockAddr.h b/libraries/networking/src/HifiSockAddr.h index af939de736..b582198139 100644 --- a/libraries/networking/src/HifiSockAddr.h +++ b/libraries/networking/src/HifiSockAddr.h @@ -55,6 +55,8 @@ public: QString toString() const; + bool hasPrivateAddress() const; // checks if the address behind this sock addr is private per RFC 1918 + friend QDebug operator<<(QDebug debug, const HifiSockAddr& sockAddr); friend QDataStream& operator<<(QDataStream& dataStream, const HifiSockAddr& sockAddr); friend QDataStream& operator>>(QDataStream& dataStream, HifiSockAddr& sockAddr); diff --git a/libraries/networking/src/LimitedNodeList.cpp b/libraries/networking/src/LimitedNodeList.cpp index 1d094d923c..954b9685af 100644 --- a/libraries/networking/src/LimitedNodeList.cpp +++ b/libraries/networking/src/LimitedNodeList.cpp @@ -202,12 +202,12 @@ QUdpSocket& LimitedNodeList::getDTLSSocket() { return *_dtlsSocket; } -bool LimitedNodeList::isPacketVerified(const udt::Packet& packet) { +bool LimitedNodeList::isPacketVerifiedWithSource(const udt::Packet& packet, Node* sourceNode) { // We track bandwidth when doing packet verification to avoid needing to do a node lookup // later when we already do it in packetSourceAndHashMatchAndTrackBandwidth. A node lookup // incurs a lock, so it is ideal to avoid needing to do it 2+ times for each packet // received. - return packetVersionMatch(packet) && packetSourceAndHashMatchAndTrackBandwidth(packet); + return packetVersionMatch(packet) && packetSourceAndHashMatchAndTrackBandwidth(packet, sourceNode); } bool LimitedNodeList::packetVersionMatch(const udt::Packet& packet) { @@ -256,7 +256,7 @@ bool LimitedNodeList::packetVersionMatch(const udt::Packet& packet) { } } -bool LimitedNodeList::packetSourceAndHashMatchAndTrackBandwidth(const udt::Packet& packet) { +bool LimitedNodeList::packetSourceAndHashMatchAndTrackBandwidth(const udt::Packet& packet, Node* sourceNode) { PacketType headerType = NLPacket::typeInHeader(packet); @@ -298,14 +298,18 @@ bool LimitedNodeList::packetSourceAndHashMatchAndTrackBandwidth(const udt::Packe } else { QUuid sourceID = NLPacket::sourceIDInHeader(packet); - // figure out which node this is from - SharedNodePointer matchingNode = nodeWithUUID(sourceID); + // check if we were passed a sourceNode hint or if we need to look it up + if (!sourceNode) { + // figure out which node this is from + SharedNodePointer matchingNode = nodeWithUUID(sourceID); + sourceNode = matchingNode.data(); + } - if (matchingNode) { + if (sourceNode) { if (!PacketTypeEnum::getNonVerifiedPackets().contains(headerType)) { QByteArray packetHeaderHash = NLPacket::verificationHashInHeader(packet); - QByteArray expectedHash = NLPacket::hashForPacketAndSecret(packet, matchingNode->getConnectionSecret()); + QByteArray expectedHash = NLPacket::hashForPacketAndSecret(packet, sourceNode->getConnectionSecret()); // check if the md5 hash in the header matches the hash we would expect if (packetHeaderHash != expectedHash) { @@ -323,9 +327,9 @@ bool LimitedNodeList::packetSourceAndHashMatchAndTrackBandwidth(const udt::Packe // No matter if this packet is handled or not, we update the timestamp for the last time we heard // from this sending node - matchingNode->setLastHeardMicrostamp(usecTimestampNow()); + sourceNode->setLastHeardMicrostamp(usecTimestampNow()); - emit dataReceived(matchingNode->getType(), packet.getPayloadSize()); + emit dataReceived(sourceNode->getType(), packet.getPayloadSize()); return true; diff --git a/libraries/networking/src/LimitedNodeList.h b/libraries/networking/src/LimitedNodeList.h index f4ec47636b..f730bcfa17 100644 --- a/libraries/networking/src/LimitedNodeList.h +++ b/libraries/networking/src/LimitedNodeList.h @@ -286,7 +286,9 @@ public: void setPacketFilterOperator(udt::PacketFilterOperator filterOperator) { _nodeSocket.setPacketFilterOperator(filterOperator); } bool packetVersionMatch(const udt::Packet& packet); - bool isPacketVerified(const udt::Packet& packet); + + bool isPacketVerifiedWithSource(const udt::Packet& packet, Node* sourceNode = nullptr); + bool isPacketVerified(const udt::Packet& packet) { return isPacketVerifiedWithSource(packet); } static void makeSTUNRequestPacket(char* stunRequestPacket); @@ -352,7 +354,7 @@ protected: void setLocalSocket(const HifiSockAddr& sockAddr); - bool packetSourceAndHashMatchAndTrackBandwidth(const udt::Packet& packet); + bool packetSourceAndHashMatchAndTrackBandwidth(const udt::Packet& packet, Node* sourceNode = nullptr); void processSTUNResponse(std::unique_ptr packet); void handleNodeKill(const SharedNodePointer& node);