Set HMAC key once and reuse OpenSSL context

Store the HMAC wrapper in Node. Unfortunately requires a
lot of plumbing down to NLPacket. Added a mutex to the
wrapper since suspicious crashes occurred.
Authentication times seem to be comparable to existing MD5.

libraries/networking/src/LimitedNodeList.cpp

@@ -36,6 +36,7 @@
 #include "HifiSockAddr.h"
 #include "NetworkLogging.h"
 #include "udt/Packet.h"
+#include "HmacAuth.h"
 
 static Setting::Handle<quint16> LIMITED_NODELIST_LOCAL_PORT("LimitedNodeList.LocalPort", 0);
 
@@ -319,7 +320,7 @@ bool LimitedNodeList::packetSourceAndHashMatchAndTrackBandwidth(const udt::Packe
             if (verifiedPacket && !ignoreVerification) {
 
                 QByteArray packetHeaderHash = NLPacket::verificationHashInHeader(packet);
-                QByteArray expectedHash = NLPacket::hashForPacketAndSecret(packet, sourceNode->getConnectionSecret());
+                QByteArray expectedHash = NLPacket::hashForPacketAndSecret(packet, sourceNode->getConnectionSecret(), sourceNode->getAuthenticateHash());
 
                 // check if the md5 hash in the header matches the hash we would expect
                 if (packetHeaderHash != expectedHash) {
@@ -363,7 +364,7 @@ void LimitedNodeList::collectPacketStats(const NLPacket& packet) {
     _numCollectedBytes += packet.getDataSize();
 }
 
-void LimitedNodeList::fillPacketHeader(const NLPacket& packet, const QUuid& connectionSecret) {
+void LimitedNodeList::fillPacketHeader(const NLPacket& packet, HmacAuth& hmacAuth, const QUuid& connectionSecret) {
     if (!PacketTypeEnum::getNonSourcedPackets().contains(packet.getType())) {
         packet.writeSourceID(getSessionUUID());
     }
@@ -371,7 +372,7 @@ void LimitedNodeList::fillPacketHeader(const NLPacket& packet, const QUuid& conn
     if (!connectionSecret.isNull()
         && !PacketTypeEnum::getNonSourcedPackets().contains(packet.getType())
         && !PacketTypeEnum::getNonVerifiedPackets().contains(packet.getType())) {
-        packet.writeVerificationHashGivenSecret(connectionSecret);
+        packet.writeVerificationHashGivenSecret(hmacAuth, connectionSecret);
     }
 }
 
@@ -387,17 +388,18 @@ qint64 LimitedNodeList::sendUnreliablePacket(const NLPacket& packet, const Node&
     emit dataSent(destinationNode.getType(), packet.getDataSize());
     destinationNode.recordBytesSent(packet.getDataSize());
 
-    return sendUnreliablePacket(packet, *destinationNode.getActiveSocket(), destinationNode.getConnectionSecret());
+    return sendUnreliablePacket(packet, *destinationNode.getActiveSocket(), destinationNode.getAuthenticateHash(),
+        destinationNode.getConnectionSecret());
 }
 
 qint64 LimitedNodeList::sendUnreliablePacket(const NLPacket& packet, const HifiSockAddr& sockAddr,
-                                             const QUuid& connectionSecret) {
+        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, connectionSecret);
+    fillPacketHeader(packet, hmacAuth, connectionSecret);
 
     return _nodeSocket.writePacket(packet, sockAddr);
 }
@@ -410,7 +412,8 @@ qint64 LimitedNodeList::sendPacket(std::unique_ptr<NLPacket> packet, const Node&
         emit dataSent(destinationNode.getType(), packet->getDataSize());
         destinationNode.recordBytesSent(packet->getDataSize());
 
-        return sendPacket(std::move(packet), *activeSocket, destinationNode.getConnectionSecret());
+        return sendPacket(std::move(packet), *activeSocket, destinationNode.getAuthenticateHash(),
+            destinationNode.getConnectionSecret());
     } else {
         qCDebug(networking) << "LimitedNodeList::sendPacket called without active socket for node" << destinationNode << "- not sending";
         return ERROR_SENDING_PACKET_BYTES;
@@ -418,18 +421,18 @@ qint64 LimitedNodeList::sendPacket(std::unique_ptr<NLPacket> packet, const Node&
 }
 
 qint64 LimitedNodeList::sendPacket(std::unique_ptr<NLPacket> packet, const HifiSockAddr& sockAddr,
-                                   const QUuid& connectionSecret) {
+                                   HmacAuth& hmacAuth, const QUuid& connectionSecret) {
     Q_ASSERT(!packet->isPartOfMessage());
     if (packet->isReliable()) {
         collectPacketStats(*packet);
-        fillPacketHeader(*packet, connectionSecret);
+        fillPacketHeader(*packet, hmacAuth, connectionSecret);
 
         auto size = packet->getDataSize();
         _nodeSocket.writePacket(std::move(packet), sockAddr);
 
         return size;
     } else {
-        return sendUnreliablePacket(*packet, sockAddr, connectionSecret);
+        return sendUnreliablePacket(*packet, sockAddr, hmacAuth, connectionSecret);
     }
 }
 
@@ -444,7 +447,8 @@ qint64 LimitedNodeList::sendUnreliableUnorderedPacketList(NLPacketList& packetLi
         packetList.closeCurrentPacket();
 
         while (!packetList._packets.empty()) {
-            bytesSent += sendPacket(packetList.takeFront<NLPacket>(), *activeSocket, connectionSecret);
+            bytesSent += sendPacket(packetList.takeFront<NLPacket>(), *activeSocket,
+                destinationNode.getAuthenticateHash(), connectionSecret);
         }
 
         emit dataSent(destinationNode.getType(), bytesSent);
@@ -457,14 +461,14 @@ qint64 LimitedNodeList::sendUnreliableUnorderedPacketList(NLPacketList& packetLi
 }
 
 qint64 LimitedNodeList::sendUnreliableUnorderedPacketList(NLPacketList& packetList, const HifiSockAddr& sockAddr,
-                                                          const QUuid& connectionSecret) {
+                                                          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<NLPacket>(), sockAddr, connectionSecret);
+        bytesSent += sendPacket(packetList.takeFront<NLPacket>(), sockAddr, hmacAuth, connectionSecret);
     }
 
     return bytesSent;
@@ -474,10 +478,11 @@ qint64 LimitedNodeList::sendPacketList(std::unique_ptr<NLPacketList> packetList,
     // close the last packet in the list
     packetList->closeCurrentPacket();
 
+    HmacAuth unusedHmac;
     for (std::unique_ptr<udt::Packet>& packet : packetList->_packets) {
         NLPacket* nlPacket = static_cast<NLPacket*>(packet.get());
         collectPacketStats(*nlPacket);
-        fillPacketHeader(*nlPacket);
+        fillPacketHeader(*nlPacket, unusedHmac);
     }
 
     return _nodeSocket.writePacketList(std::move(packetList), sockAddr);
@@ -492,7 +497,7 @@ qint64 LimitedNodeList::sendPacketList(std::unique_ptr<NLPacketList> packetList,
         for (std::unique_ptr<udt::Packet>& packet : packetList->_packets) {
             NLPacket* nlPacket = static_cast<NLPacket*>(packet.get());
             collectPacketStats(*nlPacket);
-            fillPacketHeader(*nlPacket, destinationNode.getConnectionSecret());
+            fillPacketHeader(*nlPacket, destinationNode.getAuthenticateHash(), destinationNode.getConnectionSecret());
         }
 
         return _nodeSocket.writePacketList(std::move(packetList), *activeSocket);
@@ -515,7 +520,8 @@ qint64 LimitedNodeList::sendPacket(std::unique_ptr<NLPacket> packet, const Node&
     auto& destinationSockAddr = (overridenSockAddr.isNull()) ? *destinationNode.getActiveSocket()
                                                              : overridenSockAddr;
 
-    return sendPacket(std::move(packet), destinationSockAddr, destinationNode.getConnectionSecret());
+    return sendPacket(std::move(packet), destinationSockAddr, destinationNode.getAuthenticateHash(),
+        destinationNode.getConnectionSecret());
 }
 
 int LimitedNodeList::updateNodeWithDataFromPacket(QSharedPointer<ReceivedMessage> message, SharedNodePointer sendingNode) {

libraries/networking/src/LimitedNodeList.h

@@ -132,18 +132,18 @@ public:
     // 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,
-                                const QUuid& connectionSecret = QUuid());
+                                HmacAuth& hmacAuth = HmacAuth(), 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<NLPacket> packet, const Node& destinationNode);
     qint64 sendPacket(std::unique_ptr<NLPacket> packet, const HifiSockAddr& sockAddr,
-                      const QUuid& connectionSecret = QUuid());
+                      HmacAuth& hmacAuth = HmacAuth(), 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,
-                          const QUuid& connectionSecret = QUuid());
+                                             HmacAuth& hmacAuth = HmacAuth(), 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
@@ -364,7 +364,7 @@ protected:
     qint64 writePacket(const NLPacket& packet, const HifiSockAddr& destinationSockAddr,
                        const QUuid& connectionSecret = QUuid());
     void collectPacketStats(const NLPacket& packet);
-    void fillPacketHeader(const NLPacket& packet, const QUuid& connectionSecret = QUuid());
+    void fillPacketHeader(const NLPacket& packet, HmacAuth& hmacAuth, const QUuid& connectionSecret = QUuid());
 
     void setLocalSocket(const HifiSockAddr& sockAddr);
 

libraries/networking/src/NLPacket.cpp

@@ -157,15 +157,15 @@ 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) {
+QByteArray NLPacket::hashForPacketAndSecret(const udt::Packet& packet, const QUuid& connectionSecret, HmacAuth& hash) {
-    HmacAuth hash;
+#define HIFI_USE_HMAC
+#ifdef HIFI_USE_HMAC
     int offset = Packet::totalHeaderSize(packet.isPartOfMessage()) + sizeof(PacketType) + sizeof(PacketVersion)
         + NUM_BYTES_RFC4122_UUID + NUM_BYTES_MD5_HASH;
-    hash.setKey(connectionSecret);
     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)
@@ -177,8 +177,7 @@ QByteArray NLPacket::hashForPacketAndSecret(const udt::Packet& packet, const QUu
     
     // return the hash
     return hash.result();
-    */
+#endif
-
 }
 
 void NLPacket::writeTypeAndVersion() {
@@ -230,7 +229,7 @@ void NLPacket::writeSourceID(const QUuid& sourceID) const {
     _sourceID = sourceID;
 }
 
-void NLPacket::writeVerificationHashGivenSecret(const QUuid& connectionSecret) const {
+void NLPacket::writeVerificationHashGivenSecret(HmacAuth& hmacAuth, const QUuid& connectionSecret) const {
     Q_ASSERT(!PacketTypeEnum::getNonSourcedPackets().contains(_type) &&
              !PacketTypeEnum::getNonVerifiedPackets().contains(_type));
     
@@ -241,7 +240,7 @@ void NLPacket::writeVerificationHashGivenSecret(const QUuid& connectionSecret) c
     static int timedHashes = 0;
     quint64 startTime = usecTimestampNow();
 #endif
-    QByteArray verificationHash = hashForPacketAndSecret(*this, connectionSecret);
+    QByteArray verificationHash = hashForPacketAndSecret(*this, connectionSecret, hmacAuth);
 #ifdef HIFI_HASH_TIMINGS
     quint64 endTime = usecTimestampNow();
     totalTime += endTime - startTime;

libraries/networking/src/NLPacket.h

@@ -18,6 +18,8 @@
 
 #include "udt/Packet.h"
 
+class HmacAuth;
+
 class NLPacket : public udt::Packet {
     Q_OBJECT
 public:
@@ -71,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);
+    static QByteArray hashForPacketAndSecret(const udt::Packet& packet, const QUuid& connectionSecret, HmacAuth& hash);
     
     PacketType getType() const { return _type; }
     void setType(PacketType type);
@@ -82,7 +84,7 @@ public:
     const QUuid& getSourceID() const { return _sourceID; }
     
     void writeSourceID(const QUuid& sourceID) const;
-    void writeVerificationHashGivenSecret(const QUuid& connectionSecret) const;
+    void writeVerificationHashGivenSecret(HmacAuth& hmacAuth, const QUuid& connectionSecret) const;
 
 protected:
     

libraries/networking/src/Node.cpp

@@ -86,9 +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"
     _clockSkewUsec(0),
     _mutex(),
@@ -192,3 +193,7 @@ QDebug operator<<(QDebug debug, const Node& node) {
     debug.nospace() << node.getPublicSocket() << "/" << node.getLocalSocket();
     return debug.nospace();
 }
+
+void Node::_updateAuthenticateHash() {
+    _authenticateHash->setKey(_connectionSecret);
+}

libraries/networking/src/Node.h

@@ -33,6 +33,7 @@
 #include "SimpleMovingAverage.h"
 #include "MovingPercentile.h"
 #include "NodePermissions.h"
+#include "HmacAuth.h"
 
 class Node : public NetworkPeer {
     Q_OBJECT
@@ -55,7 +56,8 @@ public:
     void setIsUpstream(bool isUpstream) { _isUpstream = isUpstream; }
 
     const QUuid& getConnectionSecret() const { return _connectionSecret; }
-    void setConnectionSecret(const QUuid& connectionSecret) { _connectionSecret = connectionSecret; }
+    void setConnectionSecret(const QUuid& connectionSecret) { _connectionSecret = connectionSecret; _updateAuthenticateHash(); }
+    HmacAuth& getAuthenticateHash() const { return *_authenticateHash; }
 
     NodeData* getLinkedData() const { return _linkedData.get(); }
     void setLinkedData(std::unique_ptr<NodeData> linkedData) { _linkedData = std::move(linkedData); }
@@ -94,9 +96,12 @@ private:
     Node(const Node &otherNode);
     Node& operator=(Node otherNode);
 
+    void _updateAuthenticateHash();
+
     NodeType_t _type;
 
     QUuid _connectionSecret;
+    std::unique_ptr<HmacAuth> _authenticateHash;
     std::unique_ptr<NodeData> _linkedData;
     bool _isReplicated { false };
     int _pingMs;

libraries/shared/src/HmacAuth.cpp

@@ -46,6 +46,7 @@ bool HmacAuth::setKey(const char * keyValue, int keyLen) {
         return false;
     }
 
+    QMutexLocker lock(&_lock);
     return (bool) HMAC_Init(_hmacContext.get(), keyValue, keyLen, sslStruct);
 }
 
@@ -55,13 +56,17 @@ bool HmacAuth::setKey(const QUuid& uidKey) {
 }
 
 bool HmacAuth::addData(const char * data, int dataLen) {
+    QMutexLocker lock(&_lock);
     return (bool) HMAC_Update(_hmacContext.get(), reinterpret_cast<const unsigned char*>(data), dataLen);
 }
 
 HmacAuth::HmacHash HmacAuth::result() {
     HmacHash hashValue(EVP_MAX_MD_SIZE);
     unsigned int  hashLen;
+    QMutexLocker lock(&_lock);
     HMAC_Final(_hmacContext.get(), &hashValue[0], &hashLen);
     hashValue.resize((size_t) hashLen);
+    // Clear state for possible reuse.
+    HMAC_Init(_hmacContext.get(), nullptr, 0, nullptr);
     return hashValue;
 }

libraries/shared/src/HmacAuth.h

@@ -7,6 +7,7 @@
 
 #include <vector>
 #include <memory>
+#include <QtCore/QMutex>
 
 struct hmac_ctx_st;
 class QUuid;
@@ -25,6 +26,7 @@ public:
     HmacHash result();
 
 private:
+    QMutex _lock;
     std::unique_ptr<hmac_ctx_st> _hmacContext;
     AuthMethod _authMethod { MD5 };
 };