diff --git a/domain-server/src/DomainServer.cpp b/domain-server/src/DomainServer.cpp index ea69b5bc95..7841381422 100644 --- a/domain-server/src/DomainServer.cpp +++ b/domain-server/src/DomainServer.cpp @@ -985,7 +985,7 @@ void DomainServer::sendDomainListToNode(const SharedNodePointer& node, const Hif // we need to break here and start a new packet // so send the current one - limitedNodeList->writeDatagram(broadcastPacket, node, senderSockAddr); + limitedNodeList->writeUnverifiedDatagram(broadcastPacket, node, senderSockAddr); // reset the broadcastPacket structure broadcastPacket.resize(numBroadcastPacketLeadBytes); @@ -998,9 +998,9 @@ void DomainServer::sendDomainListToNode(const SharedNodePointer& node, const Hif }); } } - + // always write the last broadcastPacket - limitedNodeList->writeDatagram(broadcastPacket, node, senderSockAddr); + limitedNodeList->writeUnverifiedDatagram(broadcastPacket, node, senderSockAddr); } void DomainServer::readAvailableDatagrams() { diff --git a/libraries/networking/src/LimitedNodeList.cpp b/libraries/networking/src/LimitedNodeList.cpp index 5d04122d19..d8d1e9289b 100644 --- a/libraries/networking/src/LimitedNodeList.cpp +++ b/libraries/networking/src/LimitedNodeList.cpp @@ -230,8 +230,7 @@ qint64 LimitedNodeList::readDatagram(QByteArray& incomingPacket, QHostAddress* a return result; } -qint64 LimitedNodeList::writeDatagram(const QByteArray& datagram, const HifiSockAddr& destinationSockAddr, - const QUuid& connectionSecret) { +qint64 LimitedNodeList::writeDatagram(const QByteArray& datagram, const HifiSockAddr& destinationSockAddr) { // XXX can BandwidthRecorder be used for this? // stat collection for packets ++_numCollectedPackets; @@ -251,6 +250,12 @@ qint64 LimitedNodeList::writeDatagram(const QByteArray& datagram, const SharedNodePointer& destinationNode, const HifiSockAddr& overridenSockAddr) { if (destinationNode) { + PacketType packetType = packetTypeForPacket(datagram); + + if (NON_VERIFIED_PACKETS.contains(packetType)) { + return writeUnverifiedDatagram(datagram, destinationNode, overridenSockAddr); + } + // if we don't have an overridden address, assume they want to send to the node's active socket const HifiSockAddr* destinationSockAddr = &overridenSockAddr; if (overridenSockAddr.isNull()) { @@ -264,8 +269,14 @@ qint64 LimitedNodeList::writeDatagram(const QByteArray& datagram, } QByteArray datagramCopy = datagram; - PacketType packetType = packetTypeForPacket(datagramCopy); + // if we're here and the connection secret is null, debug out - this could be a problem + if (destinationNode->getConnectionSecret().isNull()) { + qDebug() << "LimitedNodeList::writeDatagram called for verified datagram with null connection secret for" + << "destination node" << destinationNode->getUUID() << " - this is either not secure or will cause" + << "this packet to be unverifiable on the receiving side."; + } + // perform replacement of hash and optionally also sequence number in the header if (SEQUENCE_NUMBERED_PACKETS.contains(packetType)) { PacketSequenceNumber sequenceNumber = getNextSequenceNumberForPacket(destinationNode->getUUID(), packetType); @@ -276,7 +287,7 @@ qint64 LimitedNodeList::writeDatagram(const QByteArray& datagram, } emit dataSent(destinationNode->getType(), datagram.size()); - auto bytesWritten = writeDatagram(datagramCopy, *destinationSockAddr, destinationNode->getConnectionSecret()); + auto bytesWritten = writeDatagram(datagramCopy, *destinationSockAddr); // Keep track of per-destination-node bandwidth destinationNode->recordBytesSent(bytesWritten); return bytesWritten; @@ -301,8 +312,21 @@ qint64 LimitedNodeList::writeUnverifiedDatagram(const QByteArray& datagram, cons } } - // don't use the node secret! - return writeDatagram(datagram, *destinationSockAddr, QUuid()); + PacketType packetType = packetTypeForPacket(datagram); + + // optionally peform sequence number replacement in the header + if (SEQUENCE_NUMBERED_PACKETS.contains(packetType)) { + + QByteArray datagramCopy = datagram; + + PacketSequenceNumber sequenceNumber = getNextSequenceNumberForPacket(destinationNode->getUUID(), packetType); + replaceSequenceNumberInPacket(datagramCopy, sequenceNumber, packetType); + + // send the datagram with sequence number replaced in header + return writeDatagram(datagramCopy, *destinationSockAddr); + } else { + return writeDatagram(datagram, *destinationSockAddr); + } } // didn't have a destinationNode to send to, return 0 @@ -310,7 +334,7 @@ qint64 LimitedNodeList::writeUnverifiedDatagram(const QByteArray& datagram, cons } qint64 LimitedNodeList::writeUnverifiedDatagram(const QByteArray& datagram, const HifiSockAddr& destinationSockAddr) { - return writeDatagram(datagram, destinationSockAddr, QUuid()); + return writeDatagram(datagram, destinationSockAddr); } qint64 LimitedNodeList::writeDatagram(const char* data, qint64 size, const SharedNodePointer& destinationNode, @@ -548,11 +572,11 @@ void LimitedNodeList::removeSilentNodes() { eachNodeHashIterator([&](NodeHash::iterator& it){ SharedNodePointer node = it->second; node->getMutex().lock(); - + if ((usecTimestampNow() - node->getLastHeardMicrostamp()) > (NODE_SILENCE_THRESHOLD_MSECS * USECS_PER_MSEC)) { // call the NodeHash erase to get rid of this node it = _nodeHash.unsafe_erase(it); - + killedNodes.insert(node); } else { // we didn't erase this node, push the iterator forwards diff --git a/libraries/networking/src/LimitedNodeList.h b/libraries/networking/src/LimitedNodeList.h index 7c710185b1..9f8bf690bf 100644 --- a/libraries/networking/src/LimitedNodeList.h +++ b/libraries/networking/src/LimitedNodeList.h @@ -114,6 +114,7 @@ public: const HifiSockAddr& overridenSockAddr = HifiSockAddr()); qint64 writeUnverifiedDatagram(const QByteArray& datagram, const HifiSockAddr& destinationSockAddr); + qint64 writeDatagram(const char* data, qint64 size, const SharedNodePointer& destinationNode, const HifiSockAddr& overridenSockAddr = HifiSockAddr()); @@ -233,9 +234,7 @@ protected: LimitedNodeList(LimitedNodeList const&); // Don't implement, needed to avoid copies of singleton void operator=(LimitedNodeList const&); // Don't implement, needed to avoid copies of singleton - qint64 writeDatagram(const QByteArray& datagram, - const HifiSockAddr& destinationSockAddr, - const QUuid& connectionSecret); + qint64 writeDatagram(const QByteArray& datagram, const HifiSockAddr& destinationSockAddr); PacketSequenceNumber getNextSequenceNumberForPacket(const QUuid& nodeUUID, PacketType packetType); diff --git a/libraries/networking/src/NodeList.cpp b/libraries/networking/src/NodeList.cpp index 179f054e42..7a60c51986 100644 --- a/libraries/networking/src/NodeList.cpp +++ b/libraries/networking/src/NodeList.cpp @@ -369,7 +369,7 @@ void NodeList::sendDomainServerCheckIn() { } if (!isUsingDTLS) { - writeDatagram(domainServerPacket, _domainHandler.getSockAddr(), QUuid()); + writeUnverifiedDatagram(domainServerPacket, _domainHandler.getSockAddr()); } const int NUM_DOMAIN_SERVER_CHECKINS_PER_STUN_REQUEST = 5; @@ -424,10 +424,9 @@ int NodeList::processDomainServerList(const QByteArray& packet) { _domainHandler.setUUID(uuidFromPacketHeader(packet)); _domainHandler.setIsConnected(true); } - + int readNodes = 0; - - + QDataStream packetStream(packet); packetStream.skipRawData(numBytesForPacketHeader(packet));