From da7298b8bde004947d4a748d723f074b1f95d29b Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Mon, 19 Mar 2018 15:28:44 -0700 Subject: [PATCH] Support only HMAC - take out passing around of secret UUID Also other clean-up for production use. --- libraries/networking/src/LimitedNodeList.cpp | 38 +++++++++----------- libraries/networking/src/LimitedNodeList.h | 13 +++---- libraries/networking/src/NLPacket.cpp | 21 ++--------- libraries/networking/src/NLPacket.h | 4 +-- libraries/shared/src/HmacAuth.cpp | 10 ++++-- libraries/shared/src/HmacAuth.h | 11 ++++-- 6 files changed, 43 insertions(+), 54 deletions(-) diff --git a/libraries/networking/src/LimitedNodeList.cpp b/libraries/networking/src/LimitedNodeList.cpp index 861629fd72..d09e379909 100644 --- a/libraries/networking/src/LimitedNodeList.cpp +++ b/libraries/networking/src/LimitedNodeList.cpp @@ -320,7 +320,7 @@ bool LimitedNodeList::packetSourceAndHashMatchAndTrackBandwidth(const udt::Packe if (verifiedPacket && !ignoreVerification) { QByteArray packetHeaderHash = NLPacket::verificationHashInHeader(packet); - QByteArray expectedHash = NLPacket::hashForPacketAndSecret(packet, sourceNode->getConnectionSecret(), sourceNode->getAuthenticateHash()); + QByteArray expectedHash = NLPacket::hashForPacketAndSecret(packet, sourceNode->getAuthenticateHash()); // check if the md5 hash in the header matches the hash we would expect if (packetHeaderHash != expectedHash) { @@ -364,15 +364,15 @@ void LimitedNodeList::collectPacketStats(const NLPacket& packet) { _numCollectedBytes += packet.getDataSize(); } -void LimitedNodeList::fillPacketHeader(const NLPacket& packet, HmacAuth& hmacAuth, const QUuid& connectionSecret) { +void LimitedNodeList::fillPacketHeader(const NLPacket& packet, HmacAuth * hmacAuth) { if (!PacketTypeEnum::getNonSourcedPackets().contains(packet.getType())) { packet.writeSourceID(getSessionUUID()); } - if (!connectionSecret.isNull() + if (hmacAuth && !PacketTypeEnum::getNonSourcedPackets().contains(packet.getType()) && !PacketTypeEnum::getNonVerifiedPackets().contains(packet.getType())) { - packet.writeVerificationHashGivenSecret(hmacAuth, connectionSecret); + packet.writeVerificationHashGivenSecret(*hmacAuth); } } @@ -388,18 +388,17 @@ qint64 LimitedNodeList::sendUnreliablePacket(const NLPacket& packet, const Node& emit dataSent(destinationNode.getType(), packet.getDataSize()); destinationNode.recordBytesSent(packet.getDataSize()); - return sendUnreliablePacket(packet, *destinationNode.getActiveSocket(), destinationNode.getAuthenticateHash(), - destinationNode.getConnectionSecret()); + return sendUnreliablePacket(packet, *destinationNode.getActiveSocket(), &destinationNode.getAuthenticateHash()); } qint64 LimitedNodeList::sendUnreliablePacket(const NLPacket& packet, const HifiSockAddr& sockAddr, - HmacAuth& hmacAuth, const QUuid& connectionSecret) { + HmacAuth * hmacAuth) { Q_ASSERT(!packet.isPartOfMessage()); Q_ASSERT_X(!packet.isReliable(), "LimitedNodeList::sendUnreliablePacket", "Trying to send a reliable packet unreliably."); collectPacketStats(packet); - fillPacketHeader(packet, hmacAuth, connectionSecret); + fillPacketHeader(packet, hmacAuth); return _nodeSocket.writePacket(packet, sockAddr); } @@ -412,8 +411,7 @@ qint64 LimitedNodeList::sendPacket(std::unique_ptr packet, const Node& emit dataSent(destinationNode.getType(), packet->getDataSize()); destinationNode.recordBytesSent(packet->getDataSize()); - return sendPacket(std::move(packet), *activeSocket, destinationNode.getAuthenticateHash(), - destinationNode.getConnectionSecret()); + return sendPacket(std::move(packet), *activeSocket, &destinationNode.getAuthenticateHash()); } else { qCDebug(networking) << "LimitedNodeList::sendPacket called without active socket for node" << destinationNode << "- not sending"; return ERROR_SENDING_PACKET_BYTES; @@ -421,18 +419,18 @@ qint64 LimitedNodeList::sendPacket(std::unique_ptr packet, const Node& } qint64 LimitedNodeList::sendPacket(std::unique_ptr packet, const HifiSockAddr& sockAddr, - HmacAuth& hmacAuth, const QUuid& connectionSecret) { + HmacAuth * hmacAuth) { Q_ASSERT(!packet->isPartOfMessage()); if (packet->isReliable()) { collectPacketStats(*packet); - fillPacketHeader(*packet, hmacAuth, connectionSecret); + fillPacketHeader(*packet, hmacAuth); auto size = packet->getDataSize(); _nodeSocket.writePacket(std::move(packet), sockAddr); return size; } else { - return sendUnreliablePacket(*packet, sockAddr, hmacAuth, connectionSecret); + return sendUnreliablePacket(*packet, sockAddr, hmacAuth); } } @@ -448,7 +446,7 @@ qint64 LimitedNodeList::sendUnreliableUnorderedPacketList(NLPacketList& packetLi while (!packetList._packets.empty()) { bytesSent += sendPacket(packetList.takeFront(), *activeSocket, - destinationNode.getAuthenticateHash(), connectionSecret); + &destinationNode.getAuthenticateHash()); } emit dataSent(destinationNode.getType(), bytesSent); @@ -461,14 +459,14 @@ qint64 LimitedNodeList::sendUnreliableUnorderedPacketList(NLPacketList& packetLi } qint64 LimitedNodeList::sendUnreliableUnorderedPacketList(NLPacketList& packetList, const HifiSockAddr& sockAddr, - HmacAuth& hmacAuth, const QUuid& connectionSecret) { + HmacAuth * hmacAuth) { qint64 bytesSent = 0; // close the last packet in the list packetList.closeCurrentPacket(); while (!packetList._packets.empty()) { - bytesSent += sendPacket(packetList.takeFront(), sockAddr, hmacAuth, connectionSecret); + bytesSent += sendPacket(packetList.takeFront(), sockAddr, hmacAuth); } return bytesSent; @@ -478,11 +476,10 @@ qint64 LimitedNodeList::sendPacketList(std::unique_ptr packetList, // close the last packet in the list packetList->closeCurrentPacket(); - HmacAuth unusedHmac; for (std::unique_ptr& packet : packetList->_packets) { NLPacket* nlPacket = static_cast(packet.get()); collectPacketStats(*nlPacket); - fillPacketHeader(*nlPacket, unusedHmac); + fillPacketHeader(*nlPacket, nullptr); } return _nodeSocket.writePacketList(std::move(packetList), sockAddr); @@ -497,7 +494,7 @@ qint64 LimitedNodeList::sendPacketList(std::unique_ptr packetList, for (std::unique_ptr& packet : packetList->_packets) { NLPacket* nlPacket = static_cast(packet.get()); collectPacketStats(*nlPacket); - fillPacketHeader(*nlPacket, destinationNode.getAuthenticateHash(), destinationNode.getConnectionSecret()); + fillPacketHeader(*nlPacket, &destinationNode.getAuthenticateHash()); } return _nodeSocket.writePacketList(std::move(packetList), *activeSocket); @@ -520,8 +517,7 @@ qint64 LimitedNodeList::sendPacket(std::unique_ptr packet, const Node& auto& destinationSockAddr = (overridenSockAddr.isNull()) ? *destinationNode.getActiveSocket() : overridenSockAddr; - return sendPacket(std::move(packet), destinationSockAddr, destinationNode.getAuthenticateHash(), - destinationNode.getConnectionSecret()); + return sendPacket(std::move(packet), destinationSockAddr, &destinationNode.getAuthenticateHash()); } int LimitedNodeList::updateNodeWithDataFromPacket(QSharedPointer message, SharedNodePointer sendingNode) { diff --git a/libraries/networking/src/LimitedNodeList.h b/libraries/networking/src/LimitedNodeList.h index 638f3efefc..6d546d4d65 100644 --- a/libraries/networking/src/LimitedNodeList.h +++ b/libraries/networking/src/LimitedNodeList.h @@ -128,22 +128,19 @@ public: virtual QUuid getDomainUUID() const { assert(false); return QUuid(); } virtual HifiSockAddr getDomainSockAddr() const { assert(false); return HifiSockAddr(); } - // use sendUnreliablePacket to send an unrelaible packet (that you do not need to move) + // use sendUnreliablePacket to send an unreliable packet (that you do not need to move) // either to a node (via its active socket) or to a manual sockaddr qint64 sendUnreliablePacket(const NLPacket& packet, const Node& destinationNode); - qint64 sendUnreliablePacket(const NLPacket& packet, const HifiSockAddr& sockAddr, - HmacAuth& hmacAuth = HmacAuth::nullHmacAuth, const QUuid& connectionSecret = QUuid()); + qint64 sendUnreliablePacket(const NLPacket & packet, const HifiSockAddr & sockAddr, HmacAuth * hmacAuth = nullptr); // use sendPacket to send a moved unreliable or reliable NL packet to a node's active socket or manual sockaddr qint64 sendPacket(std::unique_ptr packet, const Node& destinationNode); - qint64 sendPacket(std::unique_ptr packet, const HifiSockAddr& sockAddr, - HmacAuth& hmacAuth = HmacAuth::nullHmacAuth, const QUuid& connectionSecret = QUuid()); + qint64 sendPacket(std::unique_ptr packet, const HifiSockAddr & sockAddr, HmacAuth * hmacAuth = nullptr); // use sendUnreliableUnorderedPacketList to unreliably send separate packets from the packet list // either to a node's active socket or to a manual sockaddr qint64 sendUnreliableUnorderedPacketList(NLPacketList& packetList, const Node& destinationNode); - qint64 sendUnreliableUnorderedPacketList(NLPacketList& packetList, const HifiSockAddr& sockAddr, - HmacAuth& hmacAuth = HmacAuth::nullHmacAuth, const QUuid& connectionSecret = QUuid()); + qint64 sendUnreliableUnorderedPacketList(NLPacketList & packetList, const HifiSockAddr & sockAddr, HmacAuth * hmacAuth); // use sendPacketList to send reliable packet lists (ordered or unordered) to a node's active socket // or to a manual sock addr @@ -364,7 +361,7 @@ protected: qint64 writePacket(const NLPacket& packet, const HifiSockAddr& destinationSockAddr, const QUuid& connectionSecret = QUuid()); void collectPacketStats(const NLPacket& packet); - void fillPacketHeader(const NLPacket& packet, HmacAuth& hmacAuth, const QUuid& connectionSecret = QUuid()); + void fillPacketHeader(const NLPacket& packet, HmacAuth * hmacAuth); void setLocalSocket(const HifiSockAddr& sockAddr); diff --git a/libraries/networking/src/NLPacket.cpp b/libraries/networking/src/NLPacket.cpp index b32c1f1f7f..99313247e9 100644 --- a/libraries/networking/src/NLPacket.cpp +++ b/libraries/networking/src/NLPacket.cpp @@ -157,27 +157,12 @@ QByteArray NLPacket::verificationHashInHeader(const udt::Packet& packet) { return QByteArray(packet.getData() + offset, NUM_BYTES_MD5_HASH); } -QByteArray NLPacket::hashForPacketAndSecret(const udt::Packet& packet, const QUuid& connectionSecret, HmacAuth& hash) { -#define HIFI_USE_HMAC -#ifdef HIFI_USE_HMAC +QByteArray NLPacket::hashForPacketAndSecret(const udt::Packet& packet, HmacAuth& hash) { int offset = Packet::totalHeaderSize(packet.isPartOfMessage()) + sizeof(PacketType) + sizeof(PacketVersion) + NUM_BYTES_RFC4122_UUID + NUM_BYTES_MD5_HASH; hash.addData(packet.getData() + offset, packet.getDataSize() - offset); auto hashResult(hash.result()); return QByteArray((const char*) hashResult.data(), (int) hashResult.size()); -#else - QCryptographicHash hash(QCryptographicHash::Md5); - - int offset = Packet::totalHeaderSize(packet.isPartOfMessage()) + sizeof(PacketType) + sizeof(PacketVersion) - + NUM_BYTES_RFC4122_UUID + NUM_BYTES_MD5_HASH; - - // add the packet payload and the connection UUID - hash.addData(packet.getData() + offset, packet.getDataSize() - offset); - hash.addData(connectionSecret.toRfc4122()); - - // return the hash - return hash.result(); -#endif } void NLPacket::writeTypeAndVersion() { @@ -229,7 +214,7 @@ void NLPacket::writeSourceID(const QUuid& sourceID) const { _sourceID = sourceID; } -void NLPacket::writeVerificationHashGivenSecret(HmacAuth& hmacAuth, const QUuid& connectionSecret) const { +void NLPacket::writeVerificationHashGivenSecret(HmacAuth& hmacAuth) const { Q_ASSERT(!PacketTypeEnum::getNonSourcedPackets().contains(_type) && !PacketTypeEnum::getNonVerifiedPackets().contains(_type)); @@ -240,7 +225,7 @@ void NLPacket::writeVerificationHashGivenSecret(HmacAuth& hmacAuth, const QUuid& static int timedHashes = 0; quint64 startTime = usecTimestampNow(); #endif - QByteArray verificationHash = hashForPacketAndSecret(*this, connectionSecret, hmacAuth); + QByteArray verificationHash = hashForPacketAndSecret(*this, hmacAuth); #ifdef HIFI_HASH_TIMINGS quint64 endTime = usecTimestampNow(); totalTime += endTime - startTime; diff --git a/libraries/networking/src/NLPacket.h b/libraries/networking/src/NLPacket.h index f38f29ec36..8f73475530 100644 --- a/libraries/networking/src/NLPacket.h +++ b/libraries/networking/src/NLPacket.h @@ -73,7 +73,7 @@ public: static QUuid sourceIDInHeader(const udt::Packet& packet); static QByteArray verificationHashInHeader(const udt::Packet& packet); - static QByteArray hashForPacketAndSecret(const udt::Packet& packet, const QUuid& connectionSecret, HmacAuth& hash); + static QByteArray hashForPacketAndSecret(const udt::Packet & packet, HmacAuth & hash); PacketType getType() const { return _type; } void setType(PacketType type); @@ -84,7 +84,7 @@ public: const QUuid& getSourceID() const { return _sourceID; } void writeSourceID(const QUuid& sourceID) const; - void writeVerificationHashGivenSecret(HmacAuth& hmacAuth, const QUuid& connectionSecret) const; + void writeVerificationHashGivenSecret(HmacAuth& hmacAuth) const; protected: diff --git a/libraries/shared/src/HmacAuth.cpp b/libraries/shared/src/HmacAuth.cpp index 47f0e4d224..5d04bb96a4 100644 --- a/libraries/shared/src/HmacAuth.cpp +++ b/libraries/shared/src/HmacAuth.cpp @@ -1,5 +1,13 @@ // // HmacAuth.cpp +// libraries/shared/src +// +// Created by Simon Walton on 3/19/2018. +// Copyright 2018 High Fidelity, Inc. +// +// Distributed under the Apache License, Version 2.0. +// See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html +// #include @@ -7,8 +15,6 @@ #include -HmacAuth HmacAuth::nullHmacAuth; - HmacAuth::HmacAuth(AuthMethod authMethod) : _hmacContext(new(HMAC_CTX)) , _authMethod(authMethod) { diff --git a/libraries/shared/src/HmacAuth.h b/libraries/shared/src/HmacAuth.h index 4970f08ca6..dfc79e8e47 100644 --- a/libraries/shared/src/HmacAuth.h +++ b/libraries/shared/src/HmacAuth.h @@ -1,6 +1,13 @@ // // HmacAuth.h // libraries/shared/src +// +// Created by Simon Walton on 3/19/2018. +// Copyright 2018 High Fidelity, Inc. +// +// Distributed under the Apache License, Version 2.0. +// See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html +// #ifndef hifi_HmacAuth_h #define hifi_HmacAuth_h @@ -14,7 +21,7 @@ class QUuid; class HmacAuth { public: enum AuthMethod { MD5, SHA1, SHA224, SHA256, RIPEMD160 }; - typedef std::vector HmacHash; + using HmacHash = std::vector; explicit HmacAuth(AuthMethod authMethod = MD5); ~HmacAuth(); @@ -24,8 +31,6 @@ public: bool addData(const char * data, int dataLen); HmacHash result(); - static HmacAuth nullHmacAuth; - private: QMutex _lock; std::unique_ptr _hmacContext;