From 63491588cbbe5d3eece780ad444e196db8c76fc0 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Mon, 23 Apr 2018 18:50:10 -0700 Subject: [PATCH] Revert "Merge pull request #12690 from SimonWalton-HiFi/hmac-auth" This reverts commit 7378c87adef98442317b2629c8d79df278c6e57a, reversing changes made to 92bf99f4363d47dca24decdd8ea25e42757fa683. --- libraries/networking/src/HMACAuth.cpp | 94 ------------------- libraries/networking/src/HMACAuth.h | 40 -------- libraries/networking/src/LimitedNodeList.cpp | 36 ++++--- libraries/networking/src/LimitedNodeList.h | 10 +- libraries/networking/src/NLPacket.cpp | 16 ++-- libraries/networking/src/NLPacket.h | 8 +- libraries/networking/src/Node.cpp | 14 +-- libraries/networking/src/Node.h | 5 +- .../networking/src/udt/PacketHeaders.cpp | 2 +- 9 files changed, 39 insertions(+), 186 deletions(-) delete mode 100644 libraries/networking/src/HMACAuth.cpp delete mode 100644 libraries/networking/src/HMACAuth.h diff --git a/libraries/networking/src/HMACAuth.cpp b/libraries/networking/src/HMACAuth.cpp deleted file mode 100644 index 42b5c48d93..0000000000 --- a/libraries/networking/src/HMACAuth.cpp +++ /dev/null @@ -1,94 +0,0 @@ -// -// HMACAuth.cpp -// libraries/networking/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 -#include - -#include "HMACAuth.h" - -#include - -#if OPENSSL_VERSION_NUMBER >= 0x10100000 -HMACAuth::HMACAuth(AuthMethod authMethod) - : _hmacContext(HMAC_CTX_new()) - , _authMethod(authMethod) { } - -HMACAuth::~HMACAuth() -{ - HMAC_CTX_free(_hmacContext); -} - -#else - -HMACAuth::HMACAuth(AuthMethod authMethod) - : _hmacContext(new HMAC_CTX()) - , _authMethod(authMethod) { - HMAC_CTX_init(_hmacContext); -} - -HMACAuth::~HMACAuth() { - HMAC_CTX_cleanup(_hmacContext); - delete _hmacContext; -} -#endif - -bool HMACAuth::setKey(const char* keyValue, int keyLen) { - const EVP_MD* sslStruct = nullptr; - - switch (_authMethod) { - case MD5: - sslStruct = EVP_md5(); - break; - - case SHA1: - sslStruct = EVP_sha1(); - break; - - case SHA224: - sslStruct = EVP_sha224(); - break; - - case SHA256: - sslStruct = EVP_sha256(); - break; - - case RIPEMD160: - sslStruct = EVP_ripemd160(); - break; - - default: - return false; - } - - QMutexLocker lock(&_lock); - return (bool) HMAC_Init_ex(_hmacContext, keyValue, keyLen, sslStruct, nullptr); -} - -bool HMACAuth::setKey(const QUuid& uidKey) { - const QByteArray rfcBytes(uidKey.toRfc4122()); - return setKey(rfcBytes.constData(), rfcBytes.length()); -} - -bool HMACAuth::addData(const char* data, int dataLen) { - QMutexLocker lock(&_lock); - return (bool) HMAC_Update(_hmacContext, reinterpret_cast(data), dataLen); -} - -HMACAuth::HMACHash HMACAuth::result() { - HMACHash hashValue(EVP_MAX_MD_SIZE); - unsigned int hashLen; - QMutexLocker lock(&_lock); - HMAC_Final(_hmacContext, &hashValue[0], &hashLen); - hashValue.resize((size_t) hashLen); - // Clear state for possible reuse. - HMAC_Init_ex(_hmacContext, nullptr, 0, nullptr, nullptr); - return hashValue; -} diff --git a/libraries/networking/src/HMACAuth.h b/libraries/networking/src/HMACAuth.h deleted file mode 100644 index 0bf7a86ec1..0000000000 --- a/libraries/networking/src/HMACAuth.h +++ /dev/null @@ -1,40 +0,0 @@ -// -// HMACAuth.h -// libraries/networking/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 - -#include -#include -#include - -class QUuid; - -class HMACAuth { -public: - enum AuthMethod { MD5, SHA1, SHA224, SHA256, RIPEMD160 }; - using HMACHash = std::vector; - - explicit HMACAuth(AuthMethod authMethod = MD5); - ~HMACAuth(); - - bool setKey(const char* keyValue, int keyLen); - bool setKey(const QUuid& uidKey); - bool addData(const char* data, int dataLen); - HMACHash result(); - -private: - QMutex _lock; - struct hmac_ctx_st* _hmacContext; - AuthMethod _authMethod; -}; - -#endif // hifi_HMACAuth_h diff --git a/libraries/networking/src/LimitedNodeList.cpp b/libraries/networking/src/LimitedNodeList.cpp index 8d177ca534..92385e99b0 100644 --- a/libraries/networking/src/LimitedNodeList.cpp +++ b/libraries/networking/src/LimitedNodeList.cpp @@ -36,7 +36,6 @@ #include "HifiSockAddr.h" #include "NetworkLogging.h" #include "udt/Packet.h" -#include "HMACAuth.h" static Setting::Handle LIMITED_NODELIST_LOCAL_PORT("LimitedNodeList.LocalPort", 0); @@ -331,7 +330,7 @@ bool LimitedNodeList::packetSourceAndHashMatchAndTrackBandwidth(const udt::Packe if (verifiedPacket && !ignoreVerification) { QByteArray packetHeaderHash = NLPacket::verificationHashInHeader(packet); - QByteArray expectedHash = NLPacket::hashForPacketAndHMAC(packet, sourceNode->getAuthenticateHash()); + QByteArray expectedHash = NLPacket::hashForPacketAndSecret(packet, sourceNode->getConnectionSecret()); // check if the md5 hash in the header matches the hash we would expect if (packetHeaderHash != expectedHash) { @@ -371,15 +370,15 @@ void LimitedNodeList::collectPacketStats(const NLPacket& packet) { _numCollectedBytes += packet.getDataSize(); } -void LimitedNodeList::fillPacketHeader(const NLPacket& packet, HMACAuth* hmacAuth) { +void LimitedNodeList::fillPacketHeader(const NLPacket& packet, const QUuid& connectionSecret) { if (!PacketTypeEnum::getNonSourcedPackets().contains(packet.getType())) { packet.writeSourceID(getSessionLocalID()); } - if (hmacAuth + if (!connectionSecret.isNull() && !PacketTypeEnum::getNonSourcedPackets().contains(packet.getType()) && !PacketTypeEnum::getNonVerifiedPackets().contains(packet.getType())) { - packet.writeVerificationHash(*hmacAuth); + packet.writeVerificationHashGivenSecret(connectionSecret); } } @@ -395,17 +394,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()); + return sendUnreliablePacket(packet, *destinationNode.getActiveSocket(), destinationNode.getConnectionSecret()); } qint64 LimitedNodeList::sendUnreliablePacket(const NLPacket& packet, const HifiSockAddr& sockAddr, - HMACAuth* hmacAuth) { + const QUuid& connectionSecret) { Q_ASSERT(!packet.isPartOfMessage()); Q_ASSERT_X(!packet.isReliable(), "LimitedNodeList::sendUnreliablePacket", "Trying to send a reliable packet unreliably."); collectPacketStats(packet); - fillPacketHeader(packet, hmacAuth); + fillPacketHeader(packet, connectionSecret); return _nodeSocket.writePacket(packet, sockAddr); } @@ -418,7 +417,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()); + return sendPacket(std::move(packet), *activeSocket, destinationNode.getConnectionSecret()); } else { qCDebug(networking) << "LimitedNodeList::sendPacket called without active socket for node" << destinationNode << "- not sending"; return ERROR_SENDING_PACKET_BYTES; @@ -426,18 +425,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) { Q_ASSERT(!packet->isPartOfMessage()); if (packet->isReliable()) { collectPacketStats(*packet); - fillPacketHeader(*packet, hmacAuth); + fillPacketHeader(*packet, connectionSecret); auto size = packet->getDataSize(); _nodeSocket.writePacket(std::move(packet), sockAddr); return size; } else { - return sendUnreliablePacket(*packet, sockAddr, hmacAuth); + return sendUnreliablePacket(*packet, sockAddr, connectionSecret); } } @@ -446,14 +445,13 @@ qint64 LimitedNodeList::sendUnreliableUnorderedPacketList(NLPacketList& packetLi if (activeSocket) { qint64 bytesSent = 0; - auto& connectionHash = destinationNode.getAuthenticateHash(); + auto connectionSecret = destinationNode.getConnectionSecret(); // close the last packet in the list packetList.closeCurrentPacket(); while (!packetList._packets.empty()) { - bytesSent += sendPacket(packetList.takeFront(), *activeSocket, - &connectionHash); + bytesSent += sendPacket(packetList.takeFront(), *activeSocket, connectionSecret); } emit dataSent(destinationNode.getType(), bytesSent); @@ -466,14 +464,14 @@ qint64 LimitedNodeList::sendUnreliableUnorderedPacketList(NLPacketList& packetLi } qint64 LimitedNodeList::sendUnreliableUnorderedPacketList(NLPacketList& packetList, const HifiSockAddr& sockAddr, - HMACAuth* hmacAuth) { + const QUuid& connectionSecret) { qint64 bytesSent = 0; // close the last packet in the list packetList.closeCurrentPacket(); while (!packetList._packets.empty()) { - bytesSent += sendPacket(packetList.takeFront(), sockAddr, hmacAuth); + bytesSent += sendPacket(packetList.takeFront(), sockAddr, connectionSecret); } return bytesSent; @@ -501,7 +499,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()); + fillPacketHeader(*nlPacket, destinationNode.getConnectionSecret()); } return _nodeSocket.writePacketList(std::move(packetList), *activeSocket); @@ -524,7 +522,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()); + return sendPacket(std::move(packet), destinationSockAddr, destinationNode.getConnectionSecret()); } int LimitedNodeList::updateNodeWithDataFromPacket(QSharedPointer message, SharedNodePointer sendingNode) { diff --git a/libraries/networking/src/LimitedNodeList.h b/libraries/networking/src/LimitedNodeList.h index 05374bbfbb..3d6fd0cd91 100644 --- a/libraries/networking/src/LimitedNodeList.h +++ b/libraries/networking/src/LimitedNodeList.h @@ -138,17 +138,19 @@ public: // 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 = nullptr); + qint64 sendUnreliablePacket(const NLPacket& packet, const HifiSockAddr& sockAddr, + const QUuid& connectionSecret = QUuid()); // 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 = nullptr); + qint64 sendPacket(std::unique_ptr packet, const HifiSockAddr& sockAddr, + const QUuid& connectionSecret = QUuid()); // 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 = nullptr); + const QUuid& connectionSecret = QUuid()); // use sendPacketList to send reliable packet lists (ordered or unordered) to a node's active socket // or to a manual sock addr @@ -370,7 +372,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 = nullptr); + void fillPacketHeader(const NLPacket& packet, const QUuid& connectionSecret = QUuid()); void setLocalSocket(const HifiSockAddr& sockAddr); diff --git a/libraries/networking/src/NLPacket.cpp b/libraries/networking/src/NLPacket.cpp index 3355e1cd6b..ac3fbc966b 100644 --- a/libraries/networking/src/NLPacket.cpp +++ b/libraries/networking/src/NLPacket.cpp @@ -11,8 +11,6 @@ #include "NLPacket.h" -#include "HMACAuth.h" - int NLPacket::localHeaderSize(PacketType type) { bool nonSourced = PacketTypeEnum::getNonSourcedPackets().contains(type); bool nonVerified = PacketTypeEnum::getNonVerifiedPackets().contains(type); @@ -152,14 +150,18 @@ QByteArray NLPacket::verificationHashInHeader(const udt::Packet& packet) { return QByteArray(packet.getData() + offset, NUM_BYTES_MD5_HASH); } -QByteArray NLPacket::hashForPacketAndHMAC(const udt::Packet& packet, HMACAuth& hash) { +QByteArray NLPacket::hashForPacketAndSecret(const udt::Packet& packet, const QUuid& connectionSecret) { + QCryptographicHash hash(QCryptographicHash::Md5); + int offset = Packet::totalHeaderSize(packet.isPartOfMessage()) + sizeof(PacketType) + sizeof(PacketVersion) + NUM_BYTES_LOCALID + NUM_BYTES_MD5_HASH; // add the packet payload and the connection UUID hash.addData(packet.getData() + offset, packet.getDataSize() - offset); - auto hashResult { hash.result() }; - return QByteArray((const char*) hashResult.data(), (int) hashResult.size()); + hash.addData(connectionSecret.toRfc4122()); + + // return the hash + return hash.result(); } void NLPacket::writeTypeAndVersion() { @@ -212,14 +214,14 @@ void NLPacket::writeSourceID(LocalID sourceID) const { _sourceID = sourceID; } -void NLPacket::writeVerificationHash(HMACAuth& hmacAuth) const { +void NLPacket::writeVerificationHashGivenSecret(const QUuid& connectionSecret) const { Q_ASSERT(!PacketTypeEnum::getNonSourcedPackets().contains(_type) && !PacketTypeEnum::getNonVerifiedPackets().contains(_type)); auto offset = Packet::totalHeaderSize(isPartOfMessage()) + sizeof(PacketType) + sizeof(PacketVersion) + NUM_BYTES_LOCALID; - QByteArray verificationHash = hashForPacketAndHMAC(*this, hmacAuth); + QByteArray verificationHash = hashForPacketAndSecret(*this, connectionSecret); memcpy(_packet.get() + offset, verificationHash.data(), verificationHash.size()); } diff --git a/libraries/networking/src/NLPacket.h b/libraries/networking/src/NLPacket.h index 4103f2068e..9b07bc6581 100644 --- a/libraries/networking/src/NLPacket.h +++ b/libraries/networking/src/NLPacket.h @@ -18,8 +18,6 @@ #include "udt/Packet.h" -class HMACAuth; - class NLPacket : public udt::Packet { Q_OBJECT public: @@ -71,7 +69,7 @@ public: static LocalID sourceIDInHeader(const udt::Packet& packet); static QByteArray verificationHashInHeader(const udt::Packet& packet); - static QByteArray hashForPacketAndHMAC(const udt::Packet& packet, HMACAuth& hash); + static QByteArray hashForPacketAndSecret(const udt::Packet& packet, const QUuid& connectionSecret); PacketType getType() const { return _type; } void setType(PacketType type); @@ -80,9 +78,9 @@ public: void setVersion(PacketVersion version); LocalID getSourceID() const { return _sourceID; } - + void writeSourceID(LocalID sourceID) const; - void writeVerificationHash(HMACAuth& hmacAuth) const; + void writeVerificationHashGivenSecret(const QUuid& connectionSecret) const; protected: diff --git a/libraries/networking/src/Node.cpp b/libraries/networking/src/Node.cpp index 7979b36e30..73b7c44e7e 100644 --- a/libraries/networking/src/Node.cpp +++ b/libraries/networking/src/Node.cpp @@ -86,10 +86,10 @@ NodeType_t NodeType::fromString(QString type) { Node::Node(const QUuid& uuid, NodeType_t type, const HifiSockAddr& publicSocket, - const HifiSockAddr& localSocket, QObject* parent) : + const HifiSockAddr& localSocket, QObject* parent) : NetworkPeer(uuid, publicSocket, localSocket, parent), _type(type), - _authenticateHash(new HMACAuth), _pingMs(-1), // "Uninitialized" + _pingMs(-1), // "Uninitialized" _clockSkewUsec(0), _mutex(), _clockSkewMovingPercentile(30, 0.8f) // moving 80th percentile of 30 samples @@ -108,7 +108,6 @@ void Node::setType(char type) { _symmetricSocket.setObjectName(typeString); } - void Node::updateClockSkewUsec(qint64 clockSkewSample) { _clockSkewMovingPercentile.updatePercentile(clockSkewSample); _clockSkewUsec = (quint64)_clockSkewMovingPercentile.getValueAtPercentile(); @@ -195,12 +194,3 @@ QDebug operator<<(QDebug debug, const Node& node) { debug.nospace() << node.getPublicSocket() << "/" << node.getLocalSocket(); return debug.nospace(); } - -void Node::setConnectionSecret(const QUuid& connectionSecret) { - if (_connectionSecret == connectionSecret) { - return; - } - - _connectionSecret = connectionSecret; - _authenticateHash->setKey(_connectionSecret); -} diff --git a/libraries/networking/src/Node.h b/libraries/networking/src/Node.h index 5b3b559582..93b6a649d4 100644 --- a/libraries/networking/src/Node.h +++ b/libraries/networking/src/Node.h @@ -33,7 +33,6 @@ #include "SimpleMovingAverage.h" #include "MovingPercentile.h" #include "NodePermissions.h" -#include "HMACAuth.h" class Node : public NetworkPeer { Q_OBJECT @@ -56,8 +55,7 @@ public: void setIsUpstream(bool isUpstream) { _isUpstream = isUpstream; } const QUuid& getConnectionSecret() const { return _connectionSecret; } - void setConnectionSecret(const QUuid& connectionSecret); - HMACAuth& getAuthenticateHash() const { return *_authenticateHash; } + void setConnectionSecret(const QUuid& connectionSecret) { _connectionSecret = connectionSecret; } NodeData* getLinkedData() const { return _linkedData.get(); } void setLinkedData(std::unique_ptr linkedData) { _linkedData = std::move(linkedData); } @@ -99,7 +97,6 @@ private: NodeType_t _type; QUuid _connectionSecret; - std::unique_ptr _authenticateHash; std::unique_ptr _linkedData; bool _isReplicated { false }; int _pingMs; diff --git a/libraries/networking/src/udt/PacketHeaders.cpp b/libraries/networking/src/udt/PacketHeaders.cpp index db466a0403..98b0e1d892 100644 --- a/libraries/networking/src/udt/PacketHeaders.cpp +++ b/libraries/networking/src/udt/PacketHeaders.cpp @@ -91,7 +91,7 @@ PacketVersion versionForPacketType(PacketType packetType) { case PacketType::Ping: return static_cast(PingVersion::IncludeConnectionID); default: - return 19; + return 20; } }