From 82b68fce8d61627a223c7a79359b64c9f756dff1 Mon Sep 17 00:00:00 2001
From: Simon Walton <simon@highfidelity.io>
Date: Thu, 15 Mar 2018 18:32:48 -0700
Subject: [PATCH 01/22] Quick attempt at using openssl HMAC in NLPacket
---
libraries/networking/src/NLPacket.cpp | 12 ++++++
libraries/shared/src/HmacAuth.cpp | 55 +++++++++++++++++++++++++++
libraries/shared/src/HmacAuth.h | 32 ++++++++++++++++
3 files changed, 99 insertions(+)
create mode 100644 libraries/shared/src/HmacAuth.cpp
create mode 100644 libraries/shared/src/HmacAuth.h
diff --git a/libraries/networking/src/NLPacket.cpp b/libraries/networking/src/NLPacket.cpp
index 5c5077691b..9df84e6abc 100644
--- a/libraries/networking/src/NLPacket.cpp
+++ b/libraries/networking/src/NLPacket.cpp
@@ -11,6 +11,8 @@
#include "NLPacket.h"
+#include "HmacAuth.h"
+
int NLPacket::localHeaderSize(PacketType type) {
bool nonSourced = PacketTypeEnum::getNonSourcedPackets().contains(type);
bool nonVerified = PacketTypeEnum::getNonVerifiedPackets().contains(type);
@@ -150,6 +152,14 @@ QByteArray NLPacket::verificationHashInHeader(const udt::Packet& packet) {
}
QByteArray NLPacket::hashForPacketAndSecret(const udt::Packet& packet, const QUuid& connectionSecret) {
+ HmacAuth hash;
+ 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());
+ /*
QCryptographicHash hash(QCryptographicHash::Md5);
int offset = Packet::totalHeaderSize(packet.isPartOfMessage()) + sizeof(PacketType) + sizeof(PacketVersion)
@@ -161,6 +171,8 @@ QByteArray NLPacket::hashForPacketAndSecret(const udt::Packet& packet, const QUu
// return the hash
return hash.result();
+ */
+
}
void NLPacket::writeTypeAndVersion() {
diff --git a/libraries/shared/src/HmacAuth.cpp b/libraries/shared/src/HmacAuth.cpp
new file mode 100644
index 0000000000..6cc6835329
--- /dev/null
+++ b/libraries/shared/src/HmacAuth.cpp
@@ -0,0 +1,55 @@
+//
+// HmacAuth.cpp
+
+#include <openssl/hmac.h>
+
+#include "HmacAuth.h"
+
+#include <QUuid>
+
+HmacAuth::HmacAuth(AuthMethod authMethod)
+ : _hmacContext(new(HMAC_CTX))
+ , _authMethod(authMethod) {
+ HMAC_CTX_init(_hmacContext.get());
+}
+
+HmacAuth::~HmacAuth() {
+ HMAC_CTX_cleanup(_hmacContext.get());
+}
+
+bool HmacAuth::setKey(const char * keyValue, int keyLen) {
+ const EVP_MD * sslStruct = nullptr;
+
+ switch (_authMethod)
+ {
+ case SHA1:
+ sslStruct = EVP_sha1();
+ break;
+
+ case RIPEMD160:
+ sslStruct = EVP_ripemd160();
+ break;
+
+ default:
+ return false;
+ }
+
+ return (bool) HMAC_Init(_hmacContext.get(), keyValue, keyLen, sslStruct);
+}
+
+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) {
+ 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;
+ HMAC_Final(_hmacContext.get(), &hashValue[0], &hashLen);
+ hashValue.resize((size_t) hashLen);
+ return hashValue;
+}
diff --git a/libraries/shared/src/HmacAuth.h b/libraries/shared/src/HmacAuth.h
new file mode 100644
index 0000000000..9d90f5fb4d
--- /dev/null
+++ b/libraries/shared/src/HmacAuth.h
@@ -0,0 +1,32 @@
+//
+// HmacAuth.h
+// libraries/shared/src
+
+#ifndef hifi_HmacAuth_h
+#define hifi_HmacAuth_h
+
+#include <vector>
+#include <memory>
+
+struct hmac_ctx_st;
+class QUuid;
+
+class HmacAuth {
+public:
+ enum AuthMethod { SHA1, RIPEMD160 };
+ typedef std::vector<unsigned char> HmacHash;
+
+ HmacAuth(AuthMethod authMethod = SHA1);
+ ~HmacAuth();
+
+ bool setKey(const char * keyValue, int keyLen);
+ bool setKey(const QUuid& uidKey);
+ bool addData(const char * data, int dataLen);
+ HmacHash result();
+
+private:
+ std::unique_ptr<hmac_ctx_st> _hmacContext;
+ AuthMethod _authMethod { SHA1 };
+};
+
+#endif // hifi_HmacAuth_h
From 480f76c21aaa92f358691fa9fc296bd65f308523 Mon Sep 17 00:00:00 2001
From: Simon Walton <simon@highfidelity.io>
Date: Fri, 16 Mar 2018 11:50:03 -0700
Subject: [PATCH 02/22] Quick trial of HMAC-MD5 auth + timings
---
libraries/networking/src/NLPacket.cpp | 18 ++++++++++++++++++
libraries/shared/src/HmacAuth.cpp | 12 ++++++++++++
libraries/shared/src/HmacAuth.h | 6 +++---
3 files changed, 33 insertions(+), 3 deletions(-)
diff --git a/libraries/networking/src/NLPacket.cpp b/libraries/networking/src/NLPacket.cpp
index 9df84e6abc..988e86afc2 100644
--- a/libraries/networking/src/NLPacket.cpp
+++ b/libraries/networking/src/NLPacket.cpp
@@ -13,6 +13,12 @@
#include "HmacAuth.h"
+#define HIFI_HASH_TIMINGS
+#ifdef HIFI_HASH_TIMINGS
+#include "NetworkLogging.h"
+#include "SharedUtil.h"
+#endif
+
int NLPacket::localHeaderSize(PacketType type) {
bool nonSourced = PacketTypeEnum::getNonSourcedPackets().contains(type);
bool nonVerified = PacketTypeEnum::getNonVerifiedPackets().contains(type);
@@ -230,7 +236,19 @@ void NLPacket::writeVerificationHashGivenSecret(const QUuid& connectionSecret) c
auto offset = Packet::totalHeaderSize(isPartOfMessage()) + sizeof(PacketType) + sizeof(PacketVersion)
+ NUM_BYTES_RFC4122_UUID;
+#ifdef HIFI_HASH_TIMINGS
+ static quint64 totalTime = 0;
+ static int timedHashes = 0;
+ quint64 startTime = usecTimestampNow();
+#endif
QByteArray verificationHash = hashForPacketAndSecret(*this, connectionSecret);
+#ifdef HIFI_HASH_TIMINGS
+ quint64 endTime = usecTimestampNow();
+ totalTime += endTime - startTime;
+ if ((++timedHashes % 20) == 0) {
+ qCDebug(networking) << "Average packet hash time " << (totalTime / timedHashes / 1000.0f) << " ms";
+ }
+#endif
memcpy(_packet.get() + offset, verificationHash.data(), verificationHash.size());
}
diff --git a/libraries/shared/src/HmacAuth.cpp b/libraries/shared/src/HmacAuth.cpp
index 6cc6835329..469d77c624 100644
--- a/libraries/shared/src/HmacAuth.cpp
+++ b/libraries/shared/src/HmacAuth.cpp
@@ -22,10 +22,22 @@ bool HmacAuth::setKey(const char * keyValue, int keyLen) {
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;
diff --git a/libraries/shared/src/HmacAuth.h b/libraries/shared/src/HmacAuth.h
index 9d90f5fb4d..1ed6be0eb0 100644
--- a/libraries/shared/src/HmacAuth.h
+++ b/libraries/shared/src/HmacAuth.h
@@ -13,10 +13,10 @@ class QUuid;
class HmacAuth {
public:
- enum AuthMethod { SHA1, RIPEMD160 };
+ enum AuthMethod { MD5, SHA1, SHA224, SHA256, RIPEMD160 };
typedef std::vector<unsigned char> HmacHash;
- HmacAuth(AuthMethod authMethod = SHA1);
+ explicit HmacAuth(AuthMethod authMethod = MD5);
~HmacAuth();
bool setKey(const char * keyValue, int keyLen);
@@ -26,7 +26,7 @@ public:
private:
std::unique_ptr<hmac_ctx_st> _hmacContext;
- AuthMethod _authMethod { SHA1 };
+ AuthMethod _authMethod { MD5 };
};
#endif // hifi_HmacAuth_h
From db8a1ccb3e16a8f561d1eae4daa29f12d0b2878a Mon Sep 17 00:00:00 2001
From: Simon Walton <simon@highfidelity.io>
Date: Fri, 16 Mar 2018 18:03:13 -0700
Subject: [PATCH 03/22] 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 | 38 +++++++++++---------
libraries/networking/src/LimitedNodeList.h | 8 ++---
libraries/networking/src/NLPacket.cpp | 15 ++++----
libraries/networking/src/NLPacket.h | 6 ++--
libraries/networking/src/Node.cpp | 7 +++-
libraries/networking/src/Node.h | 7 +++-
libraries/shared/src/HmacAuth.cpp | 5 +++
libraries/shared/src/HmacAuth.h | 2 ++
8 files changed, 56 insertions(+), 32 deletions(-)
diff --git a/libraries/networking/src/LimitedNodeList.cpp b/libraries/networking/src/LimitedNodeList.cpp
index 0803e380f2..861629fd72 100644
--- a/libraries/networking/src/LimitedNodeList.cpp
+++ b/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) {
diff --git a/libraries/networking/src/LimitedNodeList.h b/libraries/networking/src/LimitedNodeList.h
index 7165b3dd63..8e73440f5b 100644
--- a/libraries/networking/src/LimitedNodeList.h
+++ b/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);
diff --git a/libraries/networking/src/NLPacket.cpp b/libraries/networking/src/NLPacket.cpp
index 988e86afc2..b32c1f1f7f 100644
--- a/libraries/networking/src/NLPacket.cpp
+++ b/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) {
- HmacAuth hash;
+QByteArray NLPacket::hashForPacketAndSecret(const udt::Packet& packet, const QUuid& connectionSecret, 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;
diff --git a/libraries/networking/src/NLPacket.h b/libraries/networking/src/NLPacket.h
index f49cc47645..f38f29ec36 100644
--- a/libraries/networking/src/NLPacket.h
+++ b/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:
diff --git a/libraries/networking/src/Node.cpp b/libraries/networking/src/Node.cpp
index bd895c8ef1..6669c68a2e 100644
--- a/libraries/networking/src/Node.cpp
+++ b/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);
+}
diff --git a/libraries/networking/src/Node.h b/libraries/networking/src/Node.h
index 93b6a649d4..80d51202d5 100644
--- a/libraries/networking/src/Node.h
+++ b/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;
diff --git a/libraries/shared/src/HmacAuth.cpp b/libraries/shared/src/HmacAuth.cpp
index 469d77c624..ca0ec39b94 100644
--- a/libraries/shared/src/HmacAuth.cpp
+++ b/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;
}
diff --git a/libraries/shared/src/HmacAuth.h b/libraries/shared/src/HmacAuth.h
index 1ed6be0eb0..305e1a36ed 100644
--- a/libraries/shared/src/HmacAuth.h
+++ b/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 };
};
From d889384d946f8558f261084fc417cb6b2cc958da Mon Sep 17 00:00:00 2001
From: Simon Walton <simon@highfidelity.io>
Date: Mon, 19 Mar 2018 09:41:57 -0700
Subject: [PATCH 04/22] Use elaborated type-specifier for openssl internal
class
---
libraries/shared/src/HmacAuth.h | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/libraries/shared/src/HmacAuth.h b/libraries/shared/src/HmacAuth.h
index 305e1a36ed..b39423de95 100644
--- a/libraries/shared/src/HmacAuth.h
+++ b/libraries/shared/src/HmacAuth.h
@@ -9,7 +9,6 @@
#include <memory>
#include <QtCore/QMutex>
-struct hmac_ctx_st;
class QUuid;
class HmacAuth {
@@ -27,7 +26,7 @@ public:
private:
QMutex _lock;
- std::unique_ptr<hmac_ctx_st> _hmacContext;
+ std::unique_ptr<struct hmac_ctx_st> _hmacContext;
AuthMethod _authMethod { MD5 };
};
From af21cac0c2a3587c2b3d6f12613d2b84ef6f0aa7 Mon Sep 17 00:00:00 2001
From: Simon Walton <simon@highfidelity.io>
Date: Mon, 19 Mar 2018 12:53:16 -0700
Subject: [PATCH 05/22] Fixes for gcc
---
libraries/networking/src/LimitedNodeList.h | 6 +++---
libraries/shared/src/HmacAuth.cpp | 2 ++
libraries/shared/src/HmacAuth.h | 2 ++
3 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/libraries/networking/src/LimitedNodeList.h b/libraries/networking/src/LimitedNodeList.h
index 8e73440f5b..638f3efefc 100644
--- a/libraries/networking/src/LimitedNodeList.h
+++ b/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,
- HmacAuth& hmacAuth = HmacAuth(), const QUuid& connectionSecret = QUuid());
+ HmacAuth& hmacAuth = HmacAuth::nullHmacAuth, 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,
- HmacAuth& hmacAuth = HmacAuth(), const QUuid& connectionSecret = QUuid());
+ HmacAuth& hmacAuth = HmacAuth::nullHmacAuth, 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 = HmacAuth(), const QUuid& connectionSecret = QUuid());
+ HmacAuth& hmacAuth = HmacAuth::nullHmacAuth, 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
diff --git a/libraries/shared/src/HmacAuth.cpp b/libraries/shared/src/HmacAuth.cpp
index ca0ec39b94..47f0e4d224 100644
--- a/libraries/shared/src/HmacAuth.cpp
+++ b/libraries/shared/src/HmacAuth.cpp
@@ -7,6 +7,8 @@
#include <QUuid>
+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 b39423de95..4970f08ca6 100644
--- a/libraries/shared/src/HmacAuth.h
+++ b/libraries/shared/src/HmacAuth.h
@@ -24,6 +24,8 @@ public:
bool addData(const char * data, int dataLen);
HmacHash result();
+ static HmacAuth nullHmacAuth;
+
private:
QMutex _lock;
std::unique_ptr<struct hmac_ctx_st> _hmacContext;
From da7298b8bde004947d4a748d723f074b1f95d29b Mon Sep 17 00:00:00 2001
From: Simon Walton <simon@highfidelity.io>
Date: Mon, 19 Mar 2018 15:28:44 -0700
Subject: [PATCH 06/22] 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<NLPacket> 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<NLPacket> packet, const Node&
}
qint64 LimitedNodeList::sendPacket(std::unique_ptr<NLPacket> 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<NLPacket>(), *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<NLPacket>(), sockAddr, hmacAuth, connectionSecret);
+ bytesSent += sendPacket(packetList.takeFront<NLPacket>(), sockAddr, hmacAuth);
}
return bytesSent;
@@ -478,11 +476,10 @@ 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, unusedHmac);
+ fillPacketHeader(*nlPacket, nullptr);
}
return _nodeSocket.writePacketList(std::move(packetList), sockAddr);
@@ -497,7 +494,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.getAuthenticateHash(), destinationNode.getConnectionSecret());
+ fillPacketHeader(*nlPacket, &destinationNode.getAuthenticateHash());
}
return _nodeSocket.writePacketList(std::move(packetList), *activeSocket);
@@ -520,8 +517,7 @@ qint64 LimitedNodeList::sendPacket(std::unique_ptr<NLPacket> 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<ReceivedMessage> 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<NLPacket> packet, const Node& destinationNode);
- qint64 sendPacket(std::unique_ptr<NLPacket> packet, const HifiSockAddr& sockAddr,
- HmacAuth& hmacAuth = HmacAuth::nullHmacAuth, const QUuid& connectionSecret = QUuid());
+ qint64 sendPacket(std::unique_ptr<NLPacket> 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 <openssl/hmac.h>
@@ -7,8 +15,6 @@
#include <QUuid>
-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<unsigned char> HmacHash;
+ using HmacHash = std::vector<unsigned char>;
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<struct hmac_ctx_st> _hmacContext;
From 020a6a65852c7d56d4520199388895e9aa288248 Mon Sep 17 00:00:00 2001
From: Simon Walton <simon@highfidelity.io>
Date: Mon, 19 Mar 2018 15:57:32 -0700
Subject: [PATCH 07/22] Take out hash timing code
---
libraries/networking/src/NLPacket.cpp | 19 +------------------
1 file changed, 1 insertion(+), 18 deletions(-)
diff --git a/libraries/networking/src/NLPacket.cpp b/libraries/networking/src/NLPacket.cpp
index 99313247e9..37bb465ca9 100644
--- a/libraries/networking/src/NLPacket.cpp
+++ b/libraries/networking/src/NLPacket.cpp
@@ -13,12 +13,6 @@
#include "HmacAuth.h"
-#define HIFI_HASH_TIMINGS
-#ifdef HIFI_HASH_TIMINGS
-#include "NetworkLogging.h"
-#include "SharedUtil.h"
-#endif
-
int NLPacket::localHeaderSize(PacketType type) {
bool nonSourced = PacketTypeEnum::getNonSourcedPackets().contains(type);
bool nonVerified = PacketTypeEnum::getNonVerifiedPackets().contains(type);
@@ -220,19 +214,8 @@ void NLPacket::writeVerificationHashGivenSecret(HmacAuth& hmacAuth) const {
auto offset = Packet::totalHeaderSize(isPartOfMessage()) + sizeof(PacketType) + sizeof(PacketVersion)
+ NUM_BYTES_RFC4122_UUID;
-#ifdef HIFI_HASH_TIMINGS
- static quint64 totalTime = 0;
- static int timedHashes = 0;
- quint64 startTime = usecTimestampNow();
-#endif
+
QByteArray verificationHash = hashForPacketAndSecret(*this, hmacAuth);
-#ifdef HIFI_HASH_TIMINGS
- quint64 endTime = usecTimestampNow();
- totalTime += endTime - startTime;
- if ((++timedHashes % 20) == 0) {
- qCDebug(networking) << "Average packet hash time " << (totalTime / timedHashes / 1000.0f) << " ms";
- }
-#endif
memcpy(_packet.get() + offset, verificationHash.data(), verificationHash.size());
}
From 2a486a4c1419f974c1a11b55fdb266d8c42e84c1 Mon Sep 17 00:00:00 2001
From: Simon Walton <simon@highfidelity.io>
Date: Mon, 19 Mar 2018 17:48:11 -0700
Subject: [PATCH 08/22] Bump packet version numbers
---
libraries/networking/src/LimitedNodeList.h | 3 ++-
.../networking/src/udt/PacketHeaders.cpp | 21 ++++++++--------
libraries/networking/src/udt/PacketHeaders.h | 25 +++++++++++++------
3 files changed, 29 insertions(+), 20 deletions(-)
diff --git a/libraries/networking/src/LimitedNodeList.h b/libraries/networking/src/LimitedNodeList.h
index 6d546d4d65..612a6ce947 100644
--- a/libraries/networking/src/LimitedNodeList.h
+++ b/libraries/networking/src/LimitedNodeList.h
@@ -140,7 +140,8 @@ public:
// 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);
+ qint64 sendUnreliableUnorderedPacketList(NLPacketList & packetList, const HifiSockAddr & sockAddr,
+ HmacAuth * hmacAuth = nullptr);
// use sendPacketList to send reliable packet lists (ordered or unordered) to a node's active socket
// or to a manual sock addr
diff --git a/libraries/networking/src/udt/PacketHeaders.cpp b/libraries/networking/src/udt/PacketHeaders.cpp
index a83924ee58..d6b59d59f9 100644
--- a/libraries/networking/src/udt/PacketHeaders.cpp
+++ b/libraries/networking/src/udt/PacketHeaders.cpp
@@ -25,30 +25,29 @@ int packetTypeMetaTypeId = qRegisterMetaType<PacketType>();
PacketVersion versionForPacketType(PacketType packetType) {
switch (packetType) {
case PacketType::DomainList:
- return static_cast<PacketVersion>(DomainListVersion::GetMachineFingerprintFromUUIDSupport);
+ return static_cast<PacketVersion>(DomainListVersion::UseHmacAuthentication);
case PacketType::EntityAdd:
case PacketType::EntityEdit:
case PacketType::EntityData:
case PacketType::EntityPhysics:
- return static_cast<PacketVersion>(EntityVersion::ShadowControl);
+ return static_cast<PacketVersion>(EntityVersion::UseHmacAuthentication);
case PacketType::EntityQuery:
- return static_cast<PacketVersion>(EntityQueryPacketVersion::RemovedJurisdictions);
+ return static_cast<PacketVersion>(EntityQueryPacketVersion::UseHmacAuthentication);
case PacketType::AvatarIdentity:
case PacketType::AvatarData:
case PacketType::BulkAvatarData:
case PacketType::KillAvatar:
- return static_cast<PacketVersion>(AvatarMixerPacketVersion::FBXReaderNodeReparenting);
+ return static_cast<PacketVersion>(AvatarMixerPacketVersion::UseHmacAuthentication);
case PacketType::MessagesData:
- return static_cast<PacketVersion>(MessageDataVersion::TextOrBinaryData);
+ return static_cast<PacketVersion>(MessageDataVersion::UseHmacAuthentication);
case PacketType::ICEServerHeartbeat:
return 18; // ICE Server Heartbeat signing
case PacketType::AssetMappingOperation:
case PacketType::AssetMappingOperationReply:
- return static_cast<PacketVersion>(AssetServerPacketVersion::RedirectedMappings);
case PacketType::AssetGetInfo:
case PacketType::AssetGet:
case PacketType::AssetUpload:
- return static_cast<PacketVersion>(AssetServerPacketVersion::RangeRequestSupport);
+ return static_cast<PacketVersion>(AssetServerPacketVersion::UseHmacAuthentication);
case PacketType::NodeIgnoreRequest:
return 18; // Introduction of node ignore request (which replaced an unused packet tpye)
@@ -59,10 +58,10 @@ PacketVersion versionForPacketType(PacketType packetType) {
return static_cast<PacketVersion>(DomainConnectRequestVersion::AlwaysHasMachineFingerprint);
case PacketType::DomainServerAddedNode:
- return static_cast<PacketVersion>(DomainServerAddedNodeVersion::PermissionsGrid);
+ return static_cast<PacketVersion>(DomainServerAddedNodeVersion::UseHmacAuthentication);
case PacketType::EntityScriptCallMethod:
- return static_cast<PacketVersion>(EntityScriptCallMethodVersion::ClientCallable);
+ return static_cast<PacketVersion>(EntityScriptCallMethodVersion::UseHmacAuthentication);
case PacketType::MixedAudio:
case PacketType::SilentAudioFrame:
@@ -70,13 +69,13 @@ PacketVersion versionForPacketType(PacketType packetType) {
case PacketType::MicrophoneAudioNoEcho:
case PacketType::MicrophoneAudioWithEcho:
case PacketType::AudioStreamStats:
- return static_cast<PacketVersion>(AudioVersion::HighDynamicRangeVolume);
+ return static_cast<PacketVersion>(AudioVersion::UseHmacAuthentication);
case PacketType::ICEPing:
return static_cast<PacketVersion>(IcePingVersion::SendICEPeerID);
case PacketType::DomainSettings:
return 18; // replace min_avatar_scale and max_avatar_scale with min_avatar_height and max_avatar_height
default:
- return 17;
+ return 18;
}
}
diff --git a/libraries/networking/src/udt/PacketHeaders.h b/libraries/networking/src/udt/PacketHeaders.h
index 98a9087d37..9dc3f2befd 100644
--- a/libraries/networking/src/udt/PacketHeaders.h
+++ b/libraries/networking/src/udt/PacketHeaders.h
@@ -231,25 +231,29 @@ enum class EntityVersion : PacketVersion {
ZoneStageRemoved,
SoftEntities,
MaterialEntities,
- ShadowControl
+ ShadowControl,
+ UseHmacAuthentication
};
enum class EntityScriptCallMethodVersion : PacketVersion {
ServerCallable = 18,
- ClientCallable = 19
+ ClientCallable = 19,
+ UseHmacAuthentication = 20
};
enum class EntityQueryPacketVersion: PacketVersion {
JSONFilter = 18,
JSONFilterWithFamilyTree = 19,
ConnectionIdentifier = 20,
- RemovedJurisdictions = 21
+ RemovedJurisdictions = 21,
+ UseHmacAuthentication = 22
};
enum class AssetServerPacketVersion: PacketVersion {
VegasCongestionControl = 19,
RangeRequestSupport,
- RedirectedMappings
+ RedirectedMappings,
+ UseHmacAuthentication
};
enum class AvatarMixerPacketVersion : PacketVersion {
@@ -274,7 +278,8 @@ enum class AvatarMixerPacketVersion : PacketVersion {
AvatarIdentityLookAtSnapping,
UpdatedMannequinDefaultAvatar,
AvatarJointDefaultPoseFlags,
- FBXReaderNodeReparenting
+ FBXReaderNodeReparenting,
+ UseHmacAuthentication
};
enum class DomainConnectRequestVersion : PacketVersion {
@@ -294,14 +299,16 @@ enum class DomainConnectionDeniedVersion : PacketVersion {
enum class DomainServerAddedNodeVersion : PacketVersion {
PrePermissionsGrid = 17,
- PermissionsGrid
+ PermissionsGrid,
+ UseHmacAuthentication
};
enum class DomainListVersion : PacketVersion {
PrePermissionsGrid = 18,
PermissionsGrid,
GetUsernameFromUUIDSupport,
- GetMachineFingerprintFromUUIDSupport
+ GetMachineFingerprintFromUUIDSupport,
+ UseHmacAuthentication
};
enum class AudioVersion : PacketVersion {
@@ -312,10 +319,12 @@ enum class AudioVersion : PacketVersion {
SpaceBubbleChanges,
HasPersonalMute,
HighDynamicRangeVolume,
+ UseHmacAuthentication,
};
enum class MessageDataVersion : PacketVersion {
- TextOrBinaryData = 18
+ TextOrBinaryData = 18,
+ UseHmacAuthentication
};
enum class IcePingVersion : PacketVersion {
From 8ce03d65b76493ddc4c8d8a92bd3eea6abfc938a Mon Sep 17 00:00:00 2001
From: Simon Walton <simon@highfidelity.io>
Date: Tue, 20 Mar 2018 18:16:45 -0700
Subject: [PATCH 09/22] Only update the connection secret UUID if it changes
Rekeying the openssl HMAC context occasionally causes
hash generation errors. It is not clear why. The Node
secret never seems to change to check for this before
rekeying. Also other clean-up for PR.
---
libraries/networking/src/LimitedNodeList.cpp | 4 ++--
libraries/networking/src/NLPacket.h | 2 +-
libraries/networking/src/Node.cpp | 8 +++++++-
libraries/networking/src/Node.h | 4 +---
libraries/shared/src/HmacAuth.cpp | 2 +-
5 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/libraries/networking/src/LimitedNodeList.cpp b/libraries/networking/src/LimitedNodeList.cpp
index d09e379909..7d4ac574da 100644
--- a/libraries/networking/src/LimitedNodeList.cpp
+++ b/libraries/networking/src/LimitedNodeList.cpp
@@ -439,14 +439,14 @@ qint64 LimitedNodeList::sendUnreliableUnorderedPacketList(NLPacketList& packetLi
if (activeSocket) {
qint64 bytesSent = 0;
- auto connectionSecret = destinationNode.getConnectionSecret();
+ auto& connectionHash = destinationNode.getAuthenticateHash();
// close the last packet in the list
packetList.closeCurrentPacket();
while (!packetList._packets.empty()) {
bytesSent += sendPacket(packetList.takeFront<NLPacket>(), *activeSocket,
- &destinationNode.getAuthenticateHash());
+ &connectionHash);
}
emit dataSent(destinationNode.getType(), bytesSent);
diff --git a/libraries/networking/src/NLPacket.h b/libraries/networking/src/NLPacket.h
index 8f73475530..88b856cfda 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, HmacAuth & hash);
+ static QByteArray hashForPacketAndSecret(const udt::Packet& packet, HmacAuth& hash);
PacketType getType() const { return _type; }
void setType(PacketType type);
diff --git a/libraries/networking/src/Node.cpp b/libraries/networking/src/Node.cpp
index 6669c68a2e..5af5172580 100644
--- a/libraries/networking/src/Node.cpp
+++ b/libraries/networking/src/Node.cpp
@@ -109,6 +109,7 @@ void Node::setType(char type) {
_symmetricSocket.setObjectName(typeString);
}
+
void Node::updateClockSkewUsec(qint64 clockSkewSample) {
_clockSkewMovingPercentile.updatePercentile(clockSkewSample);
_clockSkewUsec = (quint64)_clockSkewMovingPercentile.getValueAtPercentile();
@@ -194,6 +195,11 @@ QDebug operator<<(QDebug debug, const Node& node) {
return debug.nospace();
}
-void Node::_updateAuthenticateHash() {
+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 80d51202d5..fe99e9c1ca 100644
--- a/libraries/networking/src/Node.h
+++ b/libraries/networking/src/Node.h
@@ -56,7 +56,7 @@ public:
void setIsUpstream(bool isUpstream) { _isUpstream = isUpstream; }
const QUuid& getConnectionSecret() const { return _connectionSecret; }
- void setConnectionSecret(const QUuid& connectionSecret) { _connectionSecret = connectionSecret; _updateAuthenticateHash(); }
+ void setConnectionSecret(const QUuid& connectionSecret);
HmacAuth& getAuthenticateHash() const { return *_authenticateHash; }
NodeData* getLinkedData() const { return _linkedData.get(); }
@@ -96,8 +96,6 @@ private:
Node(const Node &otherNode);
Node& operator=(Node otherNode);
- void _updateAuthenticateHash();
-
NodeType_t _type;
QUuid _connectionSecret;
diff --git a/libraries/shared/src/HmacAuth.cpp b/libraries/shared/src/HmacAuth.cpp
index 5d04bb96a4..f3ffec2c05 100644
--- a/libraries/shared/src/HmacAuth.cpp
+++ b/libraries/shared/src/HmacAuth.cpp
@@ -55,7 +55,7 @@ bool HmacAuth::setKey(const char * keyValue, int keyLen) {
}
QMutexLocker lock(&_lock);
- return (bool) HMAC_Init(_hmacContext.get(), keyValue, keyLen, sslStruct);
+ return (bool) HMAC_Init_ex(_hmacContext.get(), keyValue, keyLen, sslStruct, nullptr);
}
bool HmacAuth::setKey(const QUuid& uidKey) {
From eb04f77c3dc71990ea6e4fb21a0b156c234dab98 Mon Sep 17 00:00:00 2001
From: Simon Walton <simon@highfidelity.io>
Date: Wed, 21 Mar 2018 14:04:34 -0700
Subject: [PATCH 10/22] HMAC Auth - code modifications requested by reviewer
---
libraries/networking/src/LimitedNodeList.cpp | 16 ++++++-------
libraries/networking/src/LimitedNodeList.h | 8 +++----
libraries/networking/src/NLPacket.cpp | 10 ++++----
libraries/networking/src/NLPacket.h | 6 ++---
libraries/networking/src/Node.cpp | 5 ++--
libraries/networking/src/Node.h | 6 ++---
.../shared/src/{HmacAuth.cpp => HMACAuth.cpp} | 23 +++++++++----------
.../shared/src/{HmacAuth.h => HMACAuth.h} | 16 ++++++-------
8 files changed, 44 insertions(+), 46 deletions(-)
rename libraries/shared/src/{HmacAuth.cpp => HMACAuth.cpp} (78%)
rename libraries/shared/src/{HmacAuth.h => HMACAuth.h} (74%)
diff --git a/libraries/networking/src/LimitedNodeList.cpp b/libraries/networking/src/LimitedNodeList.cpp
index 7d4ac574da..ab66b7ae92 100644
--- a/libraries/networking/src/LimitedNodeList.cpp
+++ b/libraries/networking/src/LimitedNodeList.cpp
@@ -36,7 +36,7 @@
#include "HifiSockAddr.h"
#include "NetworkLogging.h"
#include "udt/Packet.h"
-#include "HmacAuth.h"
+#include "HMACAuth.h"
static Setting::Handle<quint16> LIMITED_NODELIST_LOCAL_PORT("LimitedNodeList.LocalPort", 0);
@@ -320,7 +320,7 @@ bool LimitedNodeList::packetSourceAndHashMatchAndTrackBandwidth(const udt::Packe
if (verifiedPacket && !ignoreVerification) {
QByteArray packetHeaderHash = NLPacket::verificationHashInHeader(packet);
- QByteArray expectedHash = NLPacket::hashForPacketAndSecret(packet, sourceNode->getAuthenticateHash());
+ QByteArray expectedHash = NLPacket::hashForPacketAndHMAC(packet, sourceNode->getAuthenticateHash());
// check if the md5 hash in the header matches the hash we would expect
if (packetHeaderHash != expectedHash) {
@@ -364,7 +364,7 @@ void LimitedNodeList::collectPacketStats(const NLPacket& packet) {
_numCollectedBytes += packet.getDataSize();
}
-void LimitedNodeList::fillPacketHeader(const NLPacket& packet, HmacAuth * hmacAuth) {
+void LimitedNodeList::fillPacketHeader(const NLPacket& packet, HMACAuth* hmacAuth) {
if (!PacketTypeEnum::getNonSourcedPackets().contains(packet.getType())) {
packet.writeSourceID(getSessionUUID());
}
@@ -372,7 +372,7 @@ void LimitedNodeList::fillPacketHeader(const NLPacket& packet, HmacAuth * hmacAu
if (hmacAuth
&& !PacketTypeEnum::getNonSourcedPackets().contains(packet.getType())
&& !PacketTypeEnum::getNonVerifiedPackets().contains(packet.getType())) {
- packet.writeVerificationHashGivenSecret(*hmacAuth);
+ packet.writeVerificationHash(*hmacAuth);
}
}
@@ -392,7 +392,7 @@ qint64 LimitedNodeList::sendUnreliablePacket(const NLPacket& packet, const Node&
}
qint64 LimitedNodeList::sendUnreliablePacket(const NLPacket& packet, const HifiSockAddr& sockAddr,
- HmacAuth * hmacAuth) {
+ HMACAuth * hmacAuth) {
Q_ASSERT(!packet.isPartOfMessage());
Q_ASSERT_X(!packet.isReliable(), "LimitedNodeList::sendUnreliablePacket",
"Trying to send a reliable packet unreliably.");
@@ -419,7 +419,7 @@ qint64 LimitedNodeList::sendPacket(std::unique_ptr<NLPacket> packet, const Node&
}
qint64 LimitedNodeList::sendPacket(std::unique_ptr<NLPacket> packet, const HifiSockAddr& sockAddr,
- HmacAuth * hmacAuth) {
+ HMACAuth* hmacAuth) {
Q_ASSERT(!packet->isPartOfMessage());
if (packet->isReliable()) {
collectPacketStats(*packet);
@@ -459,7 +459,7 @@ qint64 LimitedNodeList::sendUnreliableUnorderedPacketList(NLPacketList& packetLi
}
qint64 LimitedNodeList::sendUnreliableUnorderedPacketList(NLPacketList& packetList, const HifiSockAddr& sockAddr,
- HmacAuth * hmacAuth) {
+ HMACAuth* hmacAuth) {
qint64 bytesSent = 0;
// close the last packet in the list
@@ -479,7 +479,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, nullptr);
+ fillPacketHeader(*nlPacket);
}
return _nodeSocket.writePacketList(std::move(packetList), sockAddr);
diff --git a/libraries/networking/src/LimitedNodeList.h b/libraries/networking/src/LimitedNodeList.h
index 612a6ce947..eb70fbcbdf 100644
--- a/libraries/networking/src/LimitedNodeList.h
+++ b/libraries/networking/src/LimitedNodeList.h
@@ -131,17 +131,17 @@ 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, 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<NLPacket> packet, const Node& destinationNode);
- qint64 sendPacket(std::unique_ptr<NLPacket> packet, const HifiSockAddr & sockAddr, HmacAuth * hmacAuth = nullptr);
+ qint64 sendPacket(std::unique_ptr<NLPacket> 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 = nullptr);
+ HMACAuth * hmacAuth = nullptr);
// use sendPacketList to send reliable packet lists (ordered or unordered) to a node's active socket
// or to a manual sock addr
@@ -362,7 +362,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);
+ void fillPacketHeader(const NLPacket& packet, HMACAuth* hmacAuth = nullptr);
void setLocalSocket(const HifiSockAddr& sockAddr);
diff --git a/libraries/networking/src/NLPacket.cpp b/libraries/networking/src/NLPacket.cpp
index 37bb465ca9..93274843a6 100644
--- a/libraries/networking/src/NLPacket.cpp
+++ b/libraries/networking/src/NLPacket.cpp
@@ -11,7 +11,7 @@
#include "NLPacket.h"
-#include "HmacAuth.h"
+#include "HMACAuth.h"
int NLPacket::localHeaderSize(PacketType type) {
bool nonSourced = PacketTypeEnum::getNonSourcedPackets().contains(type);
@@ -151,11 +151,11 @@ QByteArray NLPacket::verificationHashInHeader(const udt::Packet& packet) {
return QByteArray(packet.getData() + offset, NUM_BYTES_MD5_HASH);
}
-QByteArray NLPacket::hashForPacketAndSecret(const udt::Packet& packet, HmacAuth& hash) {
+QByteArray NLPacket::hashForPacketAndHMAC(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());
+ auto hashResult { hash.result() };
return QByteArray((const char*) hashResult.data(), (int) hashResult.size());
}
@@ -208,14 +208,14 @@ void NLPacket::writeSourceID(const QUuid& sourceID) const {
_sourceID = sourceID;
}
-void NLPacket::writeVerificationHashGivenSecret(HmacAuth& hmacAuth) const {
+void NLPacket::writeVerificationHash(HMACAuth& hmacAuth) const {
Q_ASSERT(!PacketTypeEnum::getNonSourcedPackets().contains(_type) &&
!PacketTypeEnum::getNonVerifiedPackets().contains(_type));
auto offset = Packet::totalHeaderSize(isPartOfMessage()) + sizeof(PacketType) + sizeof(PacketVersion)
+ NUM_BYTES_RFC4122_UUID;
- QByteArray verificationHash = hashForPacketAndSecret(*this, hmacAuth);
+ QByteArray verificationHash = hashForPacketAndHMAC(*this, hmacAuth);
memcpy(_packet.get() + offset, verificationHash.data(), verificationHash.size());
}
diff --git a/libraries/networking/src/NLPacket.h b/libraries/networking/src/NLPacket.h
index 88b856cfda..302598f77c 100644
--- a/libraries/networking/src/NLPacket.h
+++ b/libraries/networking/src/NLPacket.h
@@ -18,7 +18,7 @@
#include "udt/Packet.h"
-class HmacAuth;
+class HMACAuth;
class NLPacket : public udt::Packet {
Q_OBJECT
@@ -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, HmacAuth& hash);
+ static QByteArray hashForPacketAndHMAC(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;
+ void writeVerificationHash(HMACAuth& hmacAuth) const;
protected:
diff --git a/libraries/networking/src/Node.cpp b/libraries/networking/src/Node.cpp
index 5af5172580..132d27d311 100644
--- a/libraries/networking/src/Node.cpp
+++ b/libraries/networking/src/Node.cpp
@@ -89,8 +89,7 @@ Node::Node(const QUuid& uuid, NodeType_t type, const HifiSockAddr& publicSocket,
const HifiSockAddr& localSocket, QObject* parent) :
NetworkPeer(uuid, publicSocket, localSocket, parent),
_type(type),
- _authenticateHash(new HmacAuth),
- _pingMs(-1), // "Uninitialized"
+ _authenticateHash(new HMACAuth), _pingMs(-1), // "Uninitialized"
_clockSkewUsec(0),
_mutex(),
_clockSkewMovingPercentile(30, 0.8f) // moving 80th percentile of 30 samples
@@ -195,7 +194,7 @@ QDebug operator<<(QDebug debug, const Node& node) {
return debug.nospace();
}
-void Node::setConnectionSecret(const QUuid & connectionSecret) {
+void Node::setConnectionSecret(const QUuid& connectionSecret) {
if (_connectionSecret == connectionSecret) {
return;
}
diff --git a/libraries/networking/src/Node.h b/libraries/networking/src/Node.h
index fe99e9c1ca..5b3b559582 100644
--- a/libraries/networking/src/Node.h
+++ b/libraries/networking/src/Node.h
@@ -33,7 +33,7 @@
#include "SimpleMovingAverage.h"
#include "MovingPercentile.h"
#include "NodePermissions.h"
-#include "HmacAuth.h"
+#include "HMACAuth.h"
class Node : public NetworkPeer {
Q_OBJECT
@@ -57,7 +57,7 @@ public:
const QUuid& getConnectionSecret() const { return _connectionSecret; }
void setConnectionSecret(const QUuid& connectionSecret);
- HmacAuth& getAuthenticateHash() const { return *_authenticateHash; }
+ HMACAuth& getAuthenticateHash() const { return *_authenticateHash; }
NodeData* getLinkedData() const { return _linkedData.get(); }
void setLinkedData(std::unique_ptr<NodeData> linkedData) { _linkedData = std::move(linkedData); }
@@ -99,7 +99,7 @@ private:
NodeType_t _type;
QUuid _connectionSecret;
- std::unique_ptr<HmacAuth> _authenticateHash;
+ std::unique_ptr<HMACAuth> _authenticateHash;
std::unique_ptr<NodeData> _linkedData;
bool _isReplicated { false };
int _pingMs;
diff --git a/libraries/shared/src/HmacAuth.cpp b/libraries/shared/src/HMACAuth.cpp
similarity index 78%
rename from libraries/shared/src/HmacAuth.cpp
rename to libraries/shared/src/HMACAuth.cpp
index f3ffec2c05..9abce7b954 100644
--- a/libraries/shared/src/HmacAuth.cpp
+++ b/libraries/shared/src/HMACAuth.cpp
@@ -1,5 +1,5 @@
//
-// HmacAuth.cpp
+// HMACAuth.cpp
// libraries/shared/src
//
// Created by Simon Walton on 3/19/2018.
@@ -11,25 +11,24 @@
#include <openssl/hmac.h>
-#include "HmacAuth.h"
+#include "HMACAuth.h"
#include <QUuid>
-HmacAuth::HmacAuth(AuthMethod authMethod)
+HMACAuth::HMACAuth(AuthMethod authMethod)
: _hmacContext(new(HMAC_CTX))
, _authMethod(authMethod) {
HMAC_CTX_init(_hmacContext.get());
}
-HmacAuth::~HmacAuth() {
+HMACAuth::~HMACAuth() {
HMAC_CTX_cleanup(_hmacContext.get());
}
-bool HmacAuth::setKey(const char * keyValue, int keyLen) {
- const EVP_MD * sslStruct = nullptr;
+bool HMACAuth::setKey(const char * keyValue, int keyLen) {
+ const EVP_MD* sslStruct = nullptr;
- switch (_authMethod)
- {
+ switch (_authMethod) {
case MD5:
sslStruct = EVP_md5();
break;
@@ -58,18 +57,18 @@ bool HmacAuth::setKey(const char * keyValue, int keyLen) {
return (bool) HMAC_Init_ex(_hmacContext.get(), keyValue, keyLen, sslStruct, nullptr);
}
-bool HmacAuth::setKey(const QUuid& uidKey) {
+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) {
+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);
+HMACAuth::HMACHash HMACAuth::result() {
+ HMACHash hashValue(EVP_MAX_MD_SIZE);
unsigned int hashLen;
QMutexLocker lock(&_lock);
HMAC_Final(_hmacContext.get(), &hashValue[0], &hashLen);
diff --git a/libraries/shared/src/HmacAuth.h b/libraries/shared/src/HMACAuth.h
similarity index 74%
rename from libraries/shared/src/HmacAuth.h
rename to libraries/shared/src/HMACAuth.h
index dfc79e8e47..4bb20a6464 100644
--- a/libraries/shared/src/HmacAuth.h
+++ b/libraries/shared/src/HMACAuth.h
@@ -1,6 +1,6 @@
//
-// HmacAuth.h
-// libraries/shared/src
+// HMACAuth.h
+// libraries/shared/src
//
// Created by Simon Walton on 3/19/2018.
// Copyright 2018 High Fidelity, Inc.
@@ -18,23 +18,23 @@
class QUuid;
-class HmacAuth {
+class HMACAuth {
public:
enum AuthMethod { MD5, SHA1, SHA224, SHA256, RIPEMD160 };
- using HmacHash = std::vector<unsigned char>;
+ using HMACHash = std::vector<unsigned char>;
- explicit HmacAuth(AuthMethod authMethod = MD5);
- ~HmacAuth();
+ 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();
+ HMACHash result();
private:
QMutex _lock;
std::unique_ptr<struct hmac_ctx_st> _hmacContext;
- AuthMethod _authMethod { MD5 };
+ AuthMethod _authMethod;
};
#endif // hifi_HmacAuth_h
From adbb2400ab5c01ca2da0a6b47e286f73b63b8eff Mon Sep 17 00:00:00 2001
From: Simon Walton <simon@highfidelity.io>
Date: Wed, 21 Mar 2018 14:58:14 -0700
Subject: [PATCH 11/22] HMAC Auth - add openssl to cmake file for lib shared
---
libraries/shared/CMakeLists.txt | 1 +
1 file changed, 1 insertion(+)
diff --git a/libraries/shared/CMakeLists.txt b/libraries/shared/CMakeLists.txt
index 713501aa77..bff842fdd8 100644
--- a/libraries/shared/CMakeLists.txt
+++ b/libraries/shared/CMakeLists.txt
@@ -2,6 +2,7 @@ set(TARGET_NAME shared)
# TODO: there isn't really a good reason to have Script linked here - let's get what is requiring it out (RegisteredMetaTypes.cpp)
setup_hifi_library(Gui Network Script)
+include_directories(SYSTEM "${OPENSSL_INCLUDE_DIR}")
if (WIN32)
target_link_libraries(${TARGET_NAME} Wbemuuid.lib)
From fb16e772ba45f84b1b7dbdfdd3024caa13784e62 Mon Sep 17 00:00:00 2001
From: Simon Walton <simon@highfidelity.io>
Date: Wed, 21 Mar 2018 15:25:50 -0700
Subject: [PATCH 12/22] Move HMACAuth class to networking lib
Also reverts addition of openssl headers to shared lib.
Commit will require rerunning cmake.
---
libraries/{shared => networking}/src/HMACAuth.cpp | 0
libraries/{shared => networking}/src/HMACAuth.h | 0
libraries/shared/CMakeLists.txt | 1 -
3 files changed, 1 deletion(-)
rename libraries/{shared => networking}/src/HMACAuth.cpp (100%)
rename libraries/{shared => networking}/src/HMACAuth.h (100%)
diff --git a/libraries/shared/src/HMACAuth.cpp b/libraries/networking/src/HMACAuth.cpp
similarity index 100%
rename from libraries/shared/src/HMACAuth.cpp
rename to libraries/networking/src/HMACAuth.cpp
diff --git a/libraries/shared/src/HMACAuth.h b/libraries/networking/src/HMACAuth.h
similarity index 100%
rename from libraries/shared/src/HMACAuth.h
rename to libraries/networking/src/HMACAuth.h
diff --git a/libraries/shared/CMakeLists.txt b/libraries/shared/CMakeLists.txt
index bff842fdd8..713501aa77 100644
--- a/libraries/shared/CMakeLists.txt
+++ b/libraries/shared/CMakeLists.txt
@@ -2,7 +2,6 @@ set(TARGET_NAME shared)
# TODO: there isn't really a good reason to have Script linked here - let's get what is requiring it out (RegisteredMetaTypes.cpp)
setup_hifi_library(Gui Network Script)
-include_directories(SYSTEM "${OPENSSL_INCLUDE_DIR}")
if (WIN32)
target_link_libraries(${TARGET_NAME} Wbemuuid.lib)
From 64973aa334f5f6582ea183c5e026f6dd19b5db31 Mon Sep 17 00:00:00 2001
From: Simon Walton <simon@highfidelity.io>
Date: Wed, 21 Mar 2018 17:06:15 -0700
Subject: [PATCH 13/22] OpenSSL HMAC changes for Android
Looks like Android uses OpenSSL 1.1.0, which provides
an allocator for its HMAC context.
---
libraries/networking/src/HMACAuth.cpp | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/libraries/networking/src/HMACAuth.cpp b/libraries/networking/src/HMACAuth.cpp
index 9abce7b954..52c43fe574 100644
--- a/libraries/networking/src/HMACAuth.cpp
+++ b/libraries/networking/src/HMACAuth.cpp
@@ -9,6 +9,7 @@
// See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html
//
+#include <openssl/opensslv.h>
#include <openssl/hmac.h>
#include "HMACAuth.h"
@@ -16,7 +17,11 @@
#include <QUuid>
HMACAuth::HMACAuth(AuthMethod authMethod)
+#if OPENSSL_VERSION_NUMBER >= 0x10100000
+ : _hmacContext(HMAC_CTX_new())
+#else
: _hmacContext(new(HMAC_CTX))
+#endif
, _authMethod(authMethod) {
HMAC_CTX_init(_hmacContext.get());
}
From 755d89464fb326e74311cd68ba648cc4a17e0dbe Mon Sep 17 00:00:00 2001
From: Simon Walton <simon@highfidelity.io>
Date: Wed, 21 Mar 2018 17:15:49 -0700
Subject: [PATCH 14/22] HMAC Auth - reviewer-requested changes
---
libraries/networking/src/HMACAuth.cpp | 2 +-
libraries/networking/src/HMACAuth.h | 8 ++++----
libraries/networking/src/LimitedNodeList.cpp | 2 +-
3 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/libraries/networking/src/HMACAuth.cpp b/libraries/networking/src/HMACAuth.cpp
index 52c43fe574..dc6d790425 100644
--- a/libraries/networking/src/HMACAuth.cpp
+++ b/libraries/networking/src/HMACAuth.cpp
@@ -30,7 +30,7 @@ HMACAuth::~HMACAuth() {
HMAC_CTX_cleanup(_hmacContext.get());
}
-bool HMACAuth::setKey(const char * keyValue, int keyLen) {
+bool HMACAuth::setKey(const char* keyValue, int keyLen) {
const EVP_MD* sslStruct = nullptr;
switch (_authMethod) {
diff --git a/libraries/networking/src/HMACAuth.h b/libraries/networking/src/HMACAuth.h
index 4bb20a6464..57f9dd64b8 100644
--- a/libraries/networking/src/HMACAuth.h
+++ b/libraries/networking/src/HMACAuth.h
@@ -1,6 +1,6 @@
//
// HMACAuth.h
-// libraries/shared/src
+// libraries/networking/src
//
// Created by Simon Walton on 3/19/2018.
// Copyright 2018 High Fidelity, Inc.
@@ -9,8 +9,8 @@
// See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html
//
-#ifndef hifi_HmacAuth_h
-#define hifi_HmacAuth_h
+#ifndef hifi_HMACAuth_h
+#define hifi_HMACAuth_h
#include <vector>
#include <memory>
@@ -37,4 +37,4 @@ private:
AuthMethod _authMethod;
};
-#endif // hifi_HmacAuth_h
+#endif // hifi_HMACAuth_h
diff --git a/libraries/networking/src/LimitedNodeList.cpp b/libraries/networking/src/LimitedNodeList.cpp
index ab66b7ae92..d2de034d0e 100644
--- a/libraries/networking/src/LimitedNodeList.cpp
+++ b/libraries/networking/src/LimitedNodeList.cpp
@@ -392,7 +392,7 @@ qint64 LimitedNodeList::sendUnreliablePacket(const NLPacket& packet, const Node&
}
qint64 LimitedNodeList::sendUnreliablePacket(const NLPacket& packet, const HifiSockAddr& sockAddr,
- HMACAuth * hmacAuth) {
+ HMACAuth* hmacAuth) {
Q_ASSERT(!packet.isPartOfMessage());
Q_ASSERT_X(!packet.isReliable(), "LimitedNodeList::sendUnreliablePacket",
"Trying to send a reliable packet unreliably.");
From ef087702352784848b93bd60e6a2adcc76dd9121 Mon Sep 17 00:00:00 2001
From: Simon Walton <simon@highfidelity.io>
Date: Wed, 21 Mar 2018 17:17:24 -0700
Subject: [PATCH 15/22] Missed reviewer change
---
libraries/networking/src/HMACAuth.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libraries/networking/src/HMACAuth.cpp b/libraries/networking/src/HMACAuth.cpp
index dc6d790425..1096098cdd 100644
--- a/libraries/networking/src/HMACAuth.cpp
+++ b/libraries/networking/src/HMACAuth.cpp
@@ -67,7 +67,7 @@ bool HMACAuth::setKey(const QUuid& uidKey) {
return setKey(rfcBytes.constData(), rfcBytes.length());
}
-bool HMACAuth::addData(const char * data, int dataLen) {
+bool HMACAuth::addData(const char* data, int dataLen) {
QMutexLocker lock(&_lock);
return (bool) HMAC_Update(_hmacContext.get(), reinterpret_cast<const unsigned char*>(data), dataLen);
}
From 3ced1c89237b5706b5365b06f4d2810e8d8aa565 Mon Sep 17 00:00:00 2001
From: Simon Walton <simon@highfidelity.io>
Date: Wed, 21 Mar 2018 17:31:54 -0700
Subject: [PATCH 16/22] More Openssl 1.1 (Android) fixes
---
libraries/networking/src/HMACAuth.cpp | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/libraries/networking/src/HMACAuth.cpp b/libraries/networking/src/HMACAuth.cpp
index 1096098cdd..77b4fb67b2 100644
--- a/libraries/networking/src/HMACAuth.cpp
+++ b/libraries/networking/src/HMACAuth.cpp
@@ -16,12 +16,17 @@
#include <QUuid>
-HMACAuth::HMACAuth(AuthMethod authMethod)
#if OPENSSL_VERSION_NUMBER >= 0x10100000
+HMACAuth::HMACAuth(AuthMethod authMethod)
: _hmacContext(HMAC_CTX_new())
+ , _authMethod(authMethod) { }
+
+HMACAuth::~HMACAuth() { }
+
#else
+
+HMACAuth::HMACAuth(AuthMethod authMethod)
: _hmacContext(new(HMAC_CTX))
-#endif
, _authMethod(authMethod) {
HMAC_CTX_init(_hmacContext.get());
}
@@ -29,6 +34,7 @@ HMACAuth::HMACAuth(AuthMethod authMethod)
HMACAuth::~HMACAuth() {
HMAC_CTX_cleanup(_hmacContext.get());
}
+#endif
bool HMACAuth::setKey(const char* keyValue, int keyLen) {
const EVP_MD* sslStruct = nullptr;
@@ -79,6 +85,6 @@ HMACAuth::HMACHash HMACAuth::result() {
HMAC_Final(_hmacContext.get(), &hashValue[0], &hashLen);
hashValue.resize((size_t) hashLen);
// Clear state for possible reuse.
- HMAC_Init(_hmacContext.get(), nullptr, 0, nullptr);
+ HMAC_Init_ex(_hmacContext.get(), nullptr, 0, nullptr, nullptr);
return hashValue;
}
From d58b2acc8ce5c7ee8fd3211d41e8937656052e79 Mon Sep 17 00:00:00 2001
From: Simon Walton <simon@highfidelity.io>
Date: Wed, 21 Mar 2018 18:25:26 -0700
Subject: [PATCH 17/22] Use raw pointer for possibly-opaque openssl context
type
---
libraries/networking/src/HMACAuth.cpp | 18 +++++++++++-------
libraries/networking/src/HMACAuth.h | 6 +++---
2 files changed, 14 insertions(+), 10 deletions(-)
diff --git a/libraries/networking/src/HMACAuth.cpp b/libraries/networking/src/HMACAuth.cpp
index 77b4fb67b2..baeffeadeb 100644
--- a/libraries/networking/src/HMACAuth.cpp
+++ b/libraries/networking/src/HMACAuth.cpp
@@ -21,18 +21,22 @@ HMACAuth::HMACAuth(AuthMethod authMethod)
: _hmacContext(HMAC_CTX_new())
, _authMethod(authMethod) { }
-HMACAuth::~HMACAuth() { }
+HMACAuth::~HMACAuth()
+{
+ HMAC_CTX_free(_hmacContext);
+}
#else
HMACAuth::HMACAuth(AuthMethod authMethod)
: _hmacContext(new(HMAC_CTX))
, _authMethod(authMethod) {
- HMAC_CTX_init(_hmacContext.get());
+ HMAC_CTX_init(_hmacContext);
}
HMACAuth::~HMACAuth() {
- HMAC_CTX_cleanup(_hmacContext.get());
+ HMAC_CTX_cleanup(_hmacContext);
+ delete _hmacContext;
}
#endif
@@ -65,7 +69,7 @@ bool HMACAuth::setKey(const char* keyValue, int keyLen) {
}
QMutexLocker lock(&_lock);
- return (bool) HMAC_Init_ex(_hmacContext.get(), keyValue, keyLen, sslStruct, nullptr);
+ return (bool) HMAC_Init_ex(_hmacContext, keyValue, keyLen, sslStruct, nullptr);
}
bool HMACAuth::setKey(const QUuid& uidKey) {
@@ -75,16 +79,16 @@ 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);
+ return (bool) HMAC_Update(_hmacContext, 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);
+ HMAC_Final(_hmacContext, &hashValue[0], &hashLen);
hashValue.resize((size_t) hashLen);
// Clear state for possible reuse.
- HMAC_Init_ex(_hmacContext.get(), nullptr, 0, nullptr, nullptr);
+ HMAC_Init_ex(_hmacContext, nullptr, 0, nullptr, nullptr);
return hashValue;
}
diff --git a/libraries/networking/src/HMACAuth.h b/libraries/networking/src/HMACAuth.h
index 57f9dd64b8..89c20a3906 100644
--- a/libraries/networking/src/HMACAuth.h
+++ b/libraries/networking/src/HMACAuth.h
@@ -26,14 +26,14 @@ public:
explicit HMACAuth(AuthMethod authMethod = MD5);
~HMACAuth();
- bool setKey(const char * keyValue, int keyLen);
+ bool setKey(const char* keyValue, int keyLen);
bool setKey(const QUuid& uidKey);
- bool addData(const char * data, int dataLen);
+ bool addData(const char* data, int dataLen);
HMACHash result();
private:
QMutex _lock;
- std::unique_ptr<struct hmac_ctx_st> _hmacContext;
+ struct hmac_ctx_st * _hmacContext;
AuthMethod _authMethod;
};
From 29b4353397bdfa168d0d212738d333c8091e7392 Mon Sep 17 00:00:00 2001
From: Simon Walton <simon@highfidelity.io>
Date: Wed, 21 Mar 2018 18:26:32 -0700
Subject: [PATCH 18/22] Spacing clean-up
---
libraries/networking/src/HMACAuth.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libraries/networking/src/HMACAuth.h b/libraries/networking/src/HMACAuth.h
index 89c20a3906..0bf7a86ec1 100644
--- a/libraries/networking/src/HMACAuth.h
+++ b/libraries/networking/src/HMACAuth.h
@@ -33,7 +33,7 @@ public:
private:
QMutex _lock;
- struct hmac_ctx_st * _hmacContext;
+ struct hmac_ctx_st* _hmacContext;
AuthMethod _authMethod;
};
From 16b0c48b73bb5d5afb31c2efc904328497dc84b8 Mon Sep 17 00:00:00 2001
From: Simon Walton <simon@highfidelity.io>
Date: Wed, 21 Mar 2018 18:32:25 -0700
Subject: [PATCH 19/22] HMACAuth - improved syntax for new
---
libraries/networking/src/HMACAuth.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libraries/networking/src/HMACAuth.cpp b/libraries/networking/src/HMACAuth.cpp
index baeffeadeb..fdc2588f62 100644
--- a/libraries/networking/src/HMACAuth.cpp
+++ b/libraries/networking/src/HMACAuth.cpp
@@ -29,7 +29,7 @@ HMACAuth::~HMACAuth()
#else
HMACAuth::HMACAuth(AuthMethod authMethod)
- : _hmacContext(new(HMAC_CTX))
+ : _hmacContext(new HMAC_CTX())
, _authMethod(authMethod) {
HMAC_CTX_init(_hmacContext);
}
From 3e1a33377615fa097605125d6eea0ac1c22d3a2a Mon Sep 17 00:00:00 2001
From: Simon Walton <simon@highfidelity.io>
Date: Wed, 21 Mar 2018 18:40:54 -0700
Subject: [PATCH 20/22] Just bump default packet version
---
.../networking/src/udt/PacketHeaders.cpp | 19 +++++++-------
libraries/networking/src/udt/PacketHeaders.h | 25 ++++++-------------
2 files changed, 18 insertions(+), 26 deletions(-)
diff --git a/libraries/networking/src/udt/PacketHeaders.cpp b/libraries/networking/src/udt/PacketHeaders.cpp
index d6b59d59f9..f09a049fc4 100644
--- a/libraries/networking/src/udt/PacketHeaders.cpp
+++ b/libraries/networking/src/udt/PacketHeaders.cpp
@@ -25,29 +25,30 @@ int packetTypeMetaTypeId = qRegisterMetaType<PacketType>();
PacketVersion versionForPacketType(PacketType packetType) {
switch (packetType) {
case PacketType::DomainList:
- return static_cast<PacketVersion>(DomainListVersion::UseHmacAuthentication);
+ return static_cast<PacketVersion>(DomainListVersion::GetMachineFingerprintFromUUIDSupport);
case PacketType::EntityAdd:
case PacketType::EntityEdit:
case PacketType::EntityData:
case PacketType::EntityPhysics:
- return static_cast<PacketVersion>(EntityVersion::UseHmacAuthentication);
+ return static_cast<PacketVersion>(EntityVersion::ShadowControl);
case PacketType::EntityQuery:
- return static_cast<PacketVersion>(EntityQueryPacketVersion::UseHmacAuthentication);
+ return static_cast<PacketVersion>(EntityQueryPacketVersion::RemovedJurisdictions);
case PacketType::AvatarIdentity:
case PacketType::AvatarData:
case PacketType::BulkAvatarData:
case PacketType::KillAvatar:
- return static_cast<PacketVersion>(AvatarMixerPacketVersion::UseHmacAuthentication);
+ return static_cast<PacketVersion>(AvatarMixerPacketVersion::FBXReaderNodeReparenting);
case PacketType::MessagesData:
- return static_cast<PacketVersion>(MessageDataVersion::UseHmacAuthentication);
+ return static_cast<PacketVersion>(MessageDataVersion::TextOrBinaryData);
case PacketType::ICEServerHeartbeat:
return 18; // ICE Server Heartbeat signing
case PacketType::AssetMappingOperation:
case PacketType::AssetMappingOperationReply:
+ return static_cast<PacketVersion>(AssetServerPacketVersion::RedirectedMappings);
case PacketType::AssetGetInfo:
case PacketType::AssetGet:
case PacketType::AssetUpload:
- return static_cast<PacketVersion>(AssetServerPacketVersion::UseHmacAuthentication);
+ return static_cast<PacketVersion>(AssetServerPacketVersion::RangeRequestSupport);
case PacketType::NodeIgnoreRequest:
return 18; // Introduction of node ignore request (which replaced an unused packet tpye)
@@ -58,10 +59,10 @@ PacketVersion versionForPacketType(PacketType packetType) {
return static_cast<PacketVersion>(DomainConnectRequestVersion::AlwaysHasMachineFingerprint);
case PacketType::DomainServerAddedNode:
- return static_cast<PacketVersion>(DomainServerAddedNodeVersion::UseHmacAuthentication);
+ return static_cast<PacketVersion>(DomainServerAddedNodeVersion::PermissionsGrid);
case PacketType::EntityScriptCallMethod:
- return static_cast<PacketVersion>(EntityScriptCallMethodVersion::UseHmacAuthentication);
+ return static_cast<PacketVersion>(EntityScriptCallMethodVersion::ClientCallable);
case PacketType::MixedAudio:
case PacketType::SilentAudioFrame:
@@ -69,7 +70,7 @@ PacketVersion versionForPacketType(PacketType packetType) {
case PacketType::MicrophoneAudioNoEcho:
case PacketType::MicrophoneAudioWithEcho:
case PacketType::AudioStreamStats:
- return static_cast<PacketVersion>(AudioVersion::UseHmacAuthentication);
+ return static_cast<PacketVersion>(AudioVersion::HighDynamicRangeVolume);
case PacketType::ICEPing:
return static_cast<PacketVersion>(IcePingVersion::SendICEPeerID);
case PacketType::DomainSettings:
diff --git a/libraries/networking/src/udt/PacketHeaders.h b/libraries/networking/src/udt/PacketHeaders.h
index 9dc3f2befd..98a9087d37 100644
--- a/libraries/networking/src/udt/PacketHeaders.h
+++ b/libraries/networking/src/udt/PacketHeaders.h
@@ -231,29 +231,25 @@ enum class EntityVersion : PacketVersion {
ZoneStageRemoved,
SoftEntities,
MaterialEntities,
- ShadowControl,
- UseHmacAuthentication
+ ShadowControl
};
enum class EntityScriptCallMethodVersion : PacketVersion {
ServerCallable = 18,
- ClientCallable = 19,
- UseHmacAuthentication = 20
+ ClientCallable = 19
};
enum class EntityQueryPacketVersion: PacketVersion {
JSONFilter = 18,
JSONFilterWithFamilyTree = 19,
ConnectionIdentifier = 20,
- RemovedJurisdictions = 21,
- UseHmacAuthentication = 22
+ RemovedJurisdictions = 21
};
enum class AssetServerPacketVersion: PacketVersion {
VegasCongestionControl = 19,
RangeRequestSupport,
- RedirectedMappings,
- UseHmacAuthentication
+ RedirectedMappings
};
enum class AvatarMixerPacketVersion : PacketVersion {
@@ -278,8 +274,7 @@ enum class AvatarMixerPacketVersion : PacketVersion {
AvatarIdentityLookAtSnapping,
UpdatedMannequinDefaultAvatar,
AvatarJointDefaultPoseFlags,
- FBXReaderNodeReparenting,
- UseHmacAuthentication
+ FBXReaderNodeReparenting
};
enum class DomainConnectRequestVersion : PacketVersion {
@@ -299,16 +294,14 @@ enum class DomainConnectionDeniedVersion : PacketVersion {
enum class DomainServerAddedNodeVersion : PacketVersion {
PrePermissionsGrid = 17,
- PermissionsGrid,
- UseHmacAuthentication
+ PermissionsGrid
};
enum class DomainListVersion : PacketVersion {
PrePermissionsGrid = 18,
PermissionsGrid,
GetUsernameFromUUIDSupport,
- GetMachineFingerprintFromUUIDSupport,
- UseHmacAuthentication
+ GetMachineFingerprintFromUUIDSupport
};
enum class AudioVersion : PacketVersion {
@@ -319,12 +312,10 @@ enum class AudioVersion : PacketVersion {
SpaceBubbleChanges,
HasPersonalMute,
HighDynamicRangeVolume,
- UseHmacAuthentication,
};
enum class MessageDataVersion : PacketVersion {
- TextOrBinaryData = 18,
- UseHmacAuthentication
+ TextOrBinaryData = 18
};
enum class IcePingVersion : PacketVersion {
From 68ab0eed68dfe8b8b2b76472e8c0891d52e477ee Mon Sep 17 00:00:00 2001
From: Simon Walton <simon@highfidelity.io>
Date: Thu, 22 Mar 2018 14:44:25 -0700
Subject: [PATCH 21/22] HMACAuth - fix some more spacing issue
---
libraries/networking/src/HMACAuth.cpp | 4 ++--
libraries/networking/src/LimitedNodeList.h | 8 ++++----
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/libraries/networking/src/HMACAuth.cpp b/libraries/networking/src/HMACAuth.cpp
index fdc2588f62..42b5c48d93 100644
--- a/libraries/networking/src/HMACAuth.cpp
+++ b/libraries/networking/src/HMACAuth.cpp
@@ -1,6 +1,6 @@
//
-// HMACAuth.cpp
-// libraries/shared/src
+// HMACAuth.cpp
+// libraries/networking/src
//
// Created by Simon Walton on 3/19/2018.
// Copyright 2018 High Fidelity, Inc.
diff --git a/libraries/networking/src/LimitedNodeList.h b/libraries/networking/src/LimitedNodeList.h
index eb70fbcbdf..64969862ee 100644
--- a/libraries/networking/src/LimitedNodeList.h
+++ b/libraries/networking/src/LimitedNodeList.h
@@ -131,17 +131,17 @@ 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, 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<NLPacket> packet, const Node& destinationNode);
- qint64 sendPacket(std::unique_ptr<NLPacket> packet, const HifiSockAddr & sockAddr, HMACAuth * hmacAuth = nullptr);
+ qint64 sendPacket(std::unique_ptr<NLPacket> 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 = nullptr);
+ qint64 sendUnreliableUnorderedPacketList(NLPacketList& packetList, const HifiSockAddr& sockAddr,
+ HMACAuth* hmacAuth = nullptr);
// use sendPacketList to send reliable packet lists (ordered or unordered) to a node's active socket
// or to a manual sock addr
From 377fc6d6c6d8041550d4b2c9919a54d42f6865cb Mon Sep 17 00:00:00 2001
From: Simon Walton <simon@highfidelity.io>
Date: Thu, 19 Apr 2018 16:55:32 -0700
Subject: [PATCH 22/22] Revert to old packet version for ICE packets
Some ICE (& STUN) packets were using the default version,
which we bumped for the HMAC change. This commit breaks
out the switch for them and reverts to 17.
---
libraries/networking/src/udt/PacketHeaders.cpp | 17 +++++++++++++++--
1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/libraries/networking/src/udt/PacketHeaders.cpp b/libraries/networking/src/udt/PacketHeaders.cpp
index f09a049fc4..0dbeb1e92e 100644
--- a/libraries/networking/src/udt/PacketHeaders.cpp
+++ b/libraries/networking/src/udt/PacketHeaders.cpp
@@ -24,6 +24,8 @@ int packetTypeMetaTypeId = qRegisterMetaType<PacketType>();
PacketVersion versionForPacketType(PacketType packetType) {
switch (packetType) {
+ case PacketType::StunResponse:
+ return 17;
case PacketType::DomainList:
return static_cast<PacketVersion>(DomainListVersion::GetMachineFingerprintFromUUIDSupport);
case PacketType::EntityAdd:
@@ -40,8 +42,21 @@ PacketVersion versionForPacketType(PacketType packetType) {
return static_cast<PacketVersion>(AvatarMixerPacketVersion::FBXReaderNodeReparenting);
case PacketType::MessagesData:
return static_cast<PacketVersion>(MessageDataVersion::TextOrBinaryData);
+ // ICE packets
+ case PacketType::ICEServerPeerInformation:
+ return 17;
+ case PacketType::ICEServerHeartbeatACK:
+ return 17;
+ case PacketType::ICEServerQuery:
+ return 17;
case PacketType::ICEServerHeartbeat:
return 18; // ICE Server Heartbeat signing
+ case PacketType::ICEPing:
+ return static_cast<PacketVersion>(IcePingVersion::SendICEPeerID);
+ case PacketType::ICEPingReply:
+ return 17;
+ case PacketType::ICEServerHeartbeatDenied:
+ return 17;
case PacketType::AssetMappingOperation:
case PacketType::AssetMappingOperationReply:
return static_cast<PacketVersion>(AssetServerPacketVersion::RedirectedMappings);
@@ -71,8 +86,6 @@ PacketVersion versionForPacketType(PacketType packetType) {
case PacketType::MicrophoneAudioWithEcho:
case PacketType::AudioStreamStats:
return static_cast<PacketVersion>(AudioVersion::HighDynamicRangeVolume);
- case PacketType::ICEPing:
- return static_cast<PacketVersion>(IcePingVersion::SendICEPeerID);
case PacketType::DomainSettings:
return 18; // replace min_avatar_scale and max_avatar_scale with min_avatar_height and max_avatar_height
default: