diff --git a/assignment-client/src/avatars/AvatarMixer.cpp b/assignment-client/src/avatars/AvatarMixer.cpp index 2174d7ff05..68ef30be33 100644 --- a/assignment-client/src/avatars/AvatarMixer.cpp +++ b/assignment-client/src/avatars/AvatarMixer.cpp @@ -83,7 +83,7 @@ AvatarMixer::AvatarMixer(ReceivedMessage& message) : packetReceiver.registerListener(PacketType::BulkAvatarTraitsAck, this, "queueIncomingPacket"); packetReceiver.registerListenerForTypes({ PacketType::OctreeStats, PacketType::EntityData, PacketType::EntityErase }, this, "handleOctreePacket"); - packetReceiver.registerListener(PacketType::ChallengeOwnership, this, "handleChallengeOwnership"); + packetReceiver.registerListener(PacketType::ChallengeOwnership, this, "queueIncomingPacket"); packetReceiver.registerListenerForTypes({ PacketType::ReplicatedAvatarIdentity, @@ -1141,7 +1141,7 @@ void AvatarMixer::handleChallengeOwnership(QSharedPointer messa auto clientData = static_cast(senderNode->getLinkedData()); auto avatar = clientData->getAvatarSharedPointer(); if (avatar) { - avatar->handleChallengeResponse(message.data()); + //avatar->handleChallengeResponse(message.data()); } } } diff --git a/assignment-client/src/avatars/AvatarMixerClientData.cpp b/assignment-client/src/avatars/AvatarMixerClientData.cpp index 8081c77ee8..7171f7a38e 100644 --- a/assignment-client/src/avatars/AvatarMixerClientData.cpp +++ b/assignment-client/src/avatars/AvatarMixerClientData.cpp @@ -74,6 +74,9 @@ int AvatarMixerClientData::processPackets(const SlaveSharedData& slaveSharedData case PacketType::BulkAvatarTraitsAck: processBulkAvatarTraitsAckMessage(*packet); break; + case PacketType::ChallengeOwnership: + _avatar->handleChallengeResponse(*packet); + break; default: Q_UNREACHABLE(); } diff --git a/assignment-client/src/avatars/MixerAvatar.cpp b/assignment-client/src/avatars/MixerAvatar.cpp index 6d6358ddf5..bb201e4dc5 100644 --- a/assignment-client/src/avatars/MixerAvatar.cpp +++ b/assignment-client/src/avatars/MixerAvatar.cpp @@ -33,7 +33,15 @@ MixerAvatar::~MixerAvatar() { } } +const char* MixerAvatar::stateToName(VerifyState state) { + return QMetaEnum::fromType().valueToKey(state); +} + void MixerAvatar::fetchAvatarFST() { + volatile auto enumName = stateToName(_verifyState); + if (_verifyState >= requestingFST && _verifyState <= challengeClient) { + qCDebug(avatars) << "WARNING: Avatar verification restarted; old state:" << stateToName(_verifyState); + } _verifyState = nonCertified; _pendingEvent = false; @@ -80,7 +88,7 @@ void MixerAvatar::fetchAvatarFST() { void MixerAvatar::fstRequestComplete() { ResourceRequest* fstRequest = static_cast(QObject::sender()); QMutexLocker certifyLocker(&_avatarCertifyLock); - if (fstRequest == _avatarRequest) { + if (_verifyState == requestingFST && fstRequest == _avatarRequest) { auto result = fstRequest->getResult(); if (result != ResourceRequest::Success) { _verifyState = error; @@ -90,11 +98,11 @@ void MixerAvatar::fstRequestComplete() { _verifyState = receivedFST; _pendingEvent = true; } - _avatarRequest->deleteLater(); _avatarRequest = nullptr; } else { - qCDebug(avatars) << "Incorrect request for" << getDisplayName(); + qCDebug(avatars) << "Incorrect or outdated FST request for" << getDisplayName(); } + fstRequest->deleteLater(); } bool MixerAvatar::generateFSTHash() { @@ -108,7 +116,7 @@ bool MixerAvatar::generateFSTHash() { return true; } -bool MixerAvatar::validateFSTHash(const QString& publicKey) { +bool MixerAvatar::validateFSTHash(const QString& publicKey) const { // Guess we should refactor this stuff into a Authorization namespace ... return EntityItemProperties::verifySignature(publicKey, _certificateHash, QByteArray::fromBase64(_certificateIdFromFST.toUtf8())); @@ -171,7 +179,9 @@ void MixerAvatar::ownerRequestComplete() { QMutexLocker certifyLocker(&_avatarCertifyLock); QNetworkReply* networkReply = static_cast(QObject::sender()); - if (networkReply->error() == QNetworkReply::NoError) { + if (_verifyState != requestingOwner) { + qCDebug(avatars) << "WARNING: outdated avatar-owner information received in state" << stateToName(_verifyState); + } else if (networkReply->error() == QNetworkReply::NoError) { _dynamicMarketResponse = networkReply->readAll(); _verifyState = ownerResponse; _pendingEvent = true; @@ -259,7 +269,6 @@ void MixerAvatar::processCertifyEvents() { } sendOwnerChallenge(); _verifyState = challengeClient; - _pendingEvent = true; } else { _verifyState = error; qCDebug(avatars) << "Get owner status - couldn't parse response for" << getSessionUUID() @@ -273,37 +282,6 @@ void MixerAvatar::processCertifyEvents() { break; } - case challengeResponse: - { - if (_challengeResponse.length() < 8) { - _verifyState = error; - _pendingEvent = false; - break; - } - - int avatarIDLength; - int signedNonceLength; - { - QDataStream responseStream(_challengeResponse); - responseStream.setByteOrder(QDataStream::LittleEndian); - responseStream >> avatarIDLength >> signedNonceLength; - } - QByteArray avatarID(_challengeResponse.data() + 2 * sizeof(int), avatarIDLength); - QByteArray signedNonce(_challengeResponse.data() + 2 * sizeof(int) + avatarIDLength, signedNonceLength); - - bool challengeResult = EntityItemProperties::verifySignature(_ownerPublicKey, _challengeNonceHash, - QByteArray::fromBase64(signedNonce)); - _verifyState = challengeResult ? verificationSucceeded : verificationFailed; - _needsIdentityUpdate = true; - if (_verifyState == verificationFailed) { - qCDebug(avatars) << "Dynamic verification FAILED for " << getDisplayName() << getSessionUUID(); - } else { - qCDebug(avatars) << "Dynamic verification SUCCEEDED for " << getDisplayName() << getSessionUUID(); - } - _pendingEvent = false; - break; - } - case requestingOwner: case challengeClient: { // Qt networking done on this thread: @@ -312,7 +290,7 @@ void MixerAvatar::processCertifyEvents() { } default: - qCDebug(avatars) << "Unexpected verify state" << _verifyState; + qCDebug(avatars) << "Unexpected verify state" << stateToName(_verifyState); break; } // close switch @@ -334,6 +312,7 @@ void MixerAvatar::sendOwnerChallenge() { QCryptographicHash nonceHash(QCryptographicHash::Sha256); nonceHash.addData(nonce); _challengeNonceHash = nonceHash.result(); + _pendingEvent = false; static constexpr int CHALLENGE_TIMEOUT_MS = 10 * 1000; // 10 s if (_challengeTimeout) { @@ -348,18 +327,45 @@ void MixerAvatar::sendOwnerChallenge() { _verifyState = verificationFailed; _needsIdentityUpdate = true; qCDebug(avatars) << "Dynamic verification TIMED-OUT for " << getDisplayName() << getSessionUUID(); + } else { + qCDebug(avatars) << "Ignoring timeout of avatar challenge"; } }); _challengeTimeout->start(); } -void MixerAvatar::handleChallengeResponse(ReceivedMessage* response) { +void MixerAvatar::handleChallengeResponse(ReceivedMessage& response) { QByteArray avatarID; - QByteArray encryptedNonce; QMutexLocker certifyLocker(&_avatarCertifyLock); if (_verifyState == challengeClient) { - _challengeResponse = response->readAll(); - _verifyState = challengeResponse; - _pendingEvent = true; + QByteArray responseData = response.readAll(); + if (responseData.length() < 8) { + _verifyState = error; + qCDebug(avatars) << "Avatar challenge response packet too small, length:" << responseData.length(); + return; + } + + int avatarIDLength; + int signedNonceLength; + { + QDataStream responseStream(responseData); + responseStream.setByteOrder(QDataStream::LittleEndian); + responseStream >> avatarIDLength >> signedNonceLength; + } + QByteArray avatarID(responseData.data() + 2 * sizeof(int), avatarIDLength); + QByteArray signedNonce(responseData.data() + 2 * sizeof(int) + avatarIDLength, signedNonceLength); + + bool challengeResult = EntityItemProperties::verifySignature(_ownerPublicKey, _challengeNonceHash, + QByteArray::fromBase64(signedNonce)); + _verifyState = challengeResult ? verificationSucceeded : verificationFailed; + _needsIdentityUpdate = true; + if (_verifyState == verificationFailed) { + qCDebug(avatars) << "Dynamic verification FAILED for " << getDisplayName() << getSessionUUID(); + } else { + qCDebug(avatars) << "Dynamic verification SUCCEEDED for" << getDisplayName() << getSessionUUID(); + } + + } else { + qCDebug(avatars) << "WARNING: Unexpected avatar challenge-response in state" << stateToName(_verifyState); } } diff --git a/assignment-client/src/avatars/MixerAvatar.h b/assignment-client/src/avatars/MixerAvatar.h index e8d9c959db..bbddf874b9 100644 --- a/assignment-client/src/avatars/MixerAvatar.h +++ b/assignment-client/src/avatars/MixerAvatar.h @@ -20,6 +20,7 @@ class ResourceRequest; class MixerAvatar : public AvatarData { + Q_OBJECT public: ~MixerAvatar(); bool getNeedsHeroCheck() const { return _needsHeroCheck; } @@ -31,15 +32,18 @@ public: void setNeedsIdentityUpdate(bool value = true) { _needsIdentityUpdate = value; } void processCertifyEvents(); - void handleChallengeResponse(ReceivedMessage* response); + void handleChallengeResponse(ReceivedMessage& response); + + // Avatar certification/verification: + enum VerifyState { + nonCertified, requestingFST, receivedFST, staticValidation, requestingOwner, ownerResponse, + challengeClient, verified, verificationFailed, verificationSucceeded, error + }; + Q_ENUM(VerifyState) private: bool _needsHeroCheck { false }; - - // Avatar certification/verification: - enum VerifyState { nonCertified, requestingFST, receivedFST, staticValidation, requestingOwner, ownerResponse, - challengeClient, challengeResponse, verified, verificationFailed, verificationSucceeded, error }; - Q_ENUM(VerifyState); + static const char* stateToName(VerifyState state); VerifyState _verifyState { nonCertified }; std::atomic _pendingEvent { false }; QMutex _avatarCertifyLock; @@ -53,12 +57,11 @@ private: QString _dynamicMarketResponse; QString _ownerPublicKey; QByteArray _challengeNonceHash; - QByteArray _challengeResponse; QTimer* _challengeTimeout { nullptr }; bool _needsIdentityUpdate { false }; bool generateFSTHash(); - bool validateFSTHash(const QString& publicKey); + bool validateFSTHash(const QString& publicKey) const; QByteArray canonicalJson(const QString fstFile); void sendOwnerChallenge();