Core/Networking: Fixed unsafe access to _worldSession member in WorldSocket

(cherry picked from commit bed88e0dd4)

Core/Networking: Fixed deadlock in HandlePing if the client is about to be kicked for overspeed pings

(cherry picked from commit 3da0f7e409)

Core/Networking: Cleanup CloseSocket calls from read failures in WorldSocket

(cherry picked from commit 18343a7309)

Conflicts:
	src/server/game/Server/WorldSocket.cpp

Ref #14474
This commit is contained in:
Shauren
2015-04-04 23:07:16 +01:00
committed by DDuarte
parent 0df5cb972d
commit 548aa119ac
4 changed files with 85 additions and 41 deletions

View File

@@ -225,6 +225,7 @@ void WorldSession::SendPacket(WorldPacket* packet)
sScriptMgr->OnPacketSend(this, *packet);
TC_LOG_TRACE("network.opcode", "S->C: %s %s", GetPlayerInfo().c_str(), GetOpcodeNameForLogging(packet->GetOpcode()).c_str());
m_Socket->SendPacket(*packet);
}

View File

@@ -28,7 +28,7 @@
using boost::asio::ip::tcp;
WorldSocket::WorldSocket(tcp::socket&& socket)
: Socket(std::move(socket)), _authSeed(rand32()), _OverSpeedPings(0), _worldSession(nullptr)
: Socket(std::move(socket)), _authSeed(rand32()), _OverSpeedPings(0), _worldSession(nullptr), _authed(false)
{
_headerBuffer.Resize(sizeof(ClientPktHeader));
}
@@ -53,7 +53,15 @@ void WorldSocket::HandleSendAuthSession()
seed2.SetRand(16 * 8);
packet.append(seed2.AsByteArray(16).get(), 16); // new encryption seeds
SendPacket(packet);
SendPacketAndLogOpcode(packet);
}
void WorldSocket::OnClose()
{
{
std::lock_guard<std::mutex> sessionGuard(_worldSessionLock);
_worldSession = nullptr;
}
}
void WorldSocket::ReadHandler()
@@ -80,7 +88,10 @@ void WorldSocket::ReadHandler()
// We just received nice new header
if (!ReadHeaderHandler())
{
CloseSocket();
return;
}
}
// We have full read header, now check the data payload
@@ -101,7 +112,10 @@ void WorldSocket::ReadHandler()
// just received fresh new payload
if (!ReadDataHandler())
{
CloseSocket();
return;
}
_headerBuffer.Reset();
}
@@ -121,17 +135,8 @@ bool WorldSocket::ReadHeaderHandler()
if (!header->IsValidSize() || !header->IsValidOpcode())
{
if (_worldSession)
{
Player* player = _worldSession->GetPlayer();
TC_LOG_ERROR("network", "WorldSocket::ReadHeaderHandler(): client (account: %u, char [GUID: %u, name: %s]) sent malformed packet (size: %hu, cmd: %u)",
_worldSession->GetAccountId(), player ? player->GetGUIDLow() : 0, player ? player->GetName().c_str() : "<none>", header->size, header->cmd);
}
else
TC_LOG_ERROR("network", "WorldSocket::ReadHeaderHandler(): client %s sent malformed packet (size: %hu, cmd: %u)",
GetRemoteIpAddress().to_string().c_str(), header->size, header->cmd);
CloseSocket();
TC_LOG_ERROR("network", "WorldSocket::ReadHeaderHandler(): client %s sent malformed packet (size: %hu, cmd: %u)",
GetRemoteIpAddress().to_string().c_str(), header->size, header->cmd);
return false;
}
@@ -151,28 +156,32 @@ bool WorldSocket::ReadDataHandler()
if (sPacketLog->CanLogPacket())
sPacketLog->LogPacket(packet, CLIENT_TO_SERVER, GetRemoteIpAddress(), GetRemotePort());
TC_LOG_TRACE("network.opcode", "C->S: %s %s", (_worldSession ? _worldSession->GetPlayerInfo() : GetRemoteIpAddress().to_string()).c_str(), GetOpcodeNameForLogging(opcode).c_str());
std::unique_lock<std::mutex> sessionGuard(_worldSessionLock, std::defer_lock);
switch (opcode)
{
case CMSG_PING:
HandlePing(packet);
break;
LogOpcodeText(opcode, sessionGuard);
return HandlePing(packet);
case CMSG_AUTH_SESSION:
if (_worldSession)
LogOpcodeText(opcode, sessionGuard);
if (_authed)
{
TC_LOG_ERROR("network", "WorldSocket::ProcessIncoming: received duplicate CMSG_AUTH_SESSION from %s", _worldSession->GetPlayerInfo().c_str());
break;
// locking just to safely log offending user is probably overkill but we are disconnecting him anyway
if (sessionGuard.try_lock())
TC_LOG_ERROR("network", "WorldSocket::ProcessIncoming: received duplicate CMSG_AUTH_SESSION from %s", _worldSession->GetPlayerInfo().c_str());
return false;
}
HandleAuthSession(packet);
break;
case CMSG_KEEP_ALIVE:
TC_LOG_DEBUG("network", "%s", GetOpcodeNameForLogging(opcode).c_str());
sScriptMgr->OnPacketReceive(_worldSession, packet);
LogOpcodeText(opcode, sessionGuard);
break;
default:
{
sessionGuard.lock();
LogOpcodeText(opcode, sessionGuard);
if (!_worldSession)
{
TC_LOG_ERROR("network.opcode", "ProcessIncoming: Client not authed opcode = %u", uint32(opcode));
@@ -193,7 +202,26 @@ bool WorldSocket::ReadDataHandler()
return true;
}
void WorldSocket::SendPacket(WorldPacket& packet)
void WorldSocket::LogOpcodeText(uint16 opcode, std::unique_lock<std::mutex> const& guard) const
{
if (!guard)
{
TC_LOG_TRACE("network.opcode", "C->S: %s %s", GetRemoteIpAddress().to_string().c_str(), GetOpcodeNameForLogging(opcode).c_str());
}
else
{
TC_LOG_TRACE("network.opcode", "C->S: %s %s", (_worldSession ? _worldSession->GetPlayerInfo() : GetRemoteIpAddress().to_string()).c_str(),
GetOpcodeNameForLogging(opcode).c_str());
}
}
void WorldSocket::SendPacketAndLogOpcode(WorldPacket const& packet)
{
TC_LOG_TRACE("network.opcode", "S->C: %s %s", GetRemoteIpAddress().to_string().c_str(), GetOpcodeNameForLogging(packet.GetOpcode()).c_str());
SendPacket(packet);
}
void WorldSocket::SendPacket(WorldPacket const& packet)
{
if (!IsOpen())
return;
@@ -201,8 +229,6 @@ void WorldSocket::SendPacket(WorldPacket& packet)
if (sPacketLog->CanLogPacket())
sPacketLog->LogPacket(packet, SERVER_TO_CLIENT, GetRemoteIpAddress(), GetRemotePort());
TC_LOG_TRACE("network.opcode", "S->C: %s %s", (_worldSession ? _worldSession->GetPlayerInfo() : GetRemoteIpAddress().to_string()).c_str(), GetOpcodeNameForLogging(packet.GetOpcode()).c_str());
ServerPktHeader header(packet.size() + 2, packet.GetOpcode());
std::unique_lock<std::mutex> guard(_writeLock);
@@ -459,6 +485,7 @@ void WorldSocket::HandleAuthSession(WorldPacket& recvPacket)
// At this point, we can safely hook a successful login
sScriptMgr->OnAccountLogin(id);
_authed = true;
_worldSession = new WorldSession(id, shared_from_this(), AccountTypes(security), expansion, mutetime, locale, recruiter, isRecruiter);
_worldSession->LoadGlobalAccountData();
_worldSession->LoadTutorialsData();
@@ -477,10 +504,10 @@ void WorldSocket::SendAuthResponseError(uint8 code)
WorldPacket packet(SMSG_AUTH_RESPONSE, 1);
packet << uint8(code);
SendPacket(packet);
SendPacketAndLogOpcode(packet);
}
void WorldSocket::HandlePing(WorldPacket& recvPacket)
bool WorldSocket::HandlePing(WorldPacket& recvPacket)
{
uint32 ping;
uint32 latency;
@@ -509,13 +536,14 @@ void WorldSocket::HandlePing(WorldPacket& recvPacket)
if (maxAllowed && _OverSpeedPings > maxAllowed)
{
std::unique_lock<std::mutex> sessionGuard(_worldSessionLock);
if (_worldSession && !_worldSession->HasPermission(rbac::RBAC_PERM_SKIP_CHECK_OVERSPEED_PING))
{
TC_LOG_ERROR("network", "WorldSocket::HandlePing: %s kicked for over-speed pings (address: %s)",
_worldSession->GetPlayerInfo().c_str(), GetRemoteIpAddress().to_string().c_str());
CloseSocket();
return;
return false;
}
}
}
@@ -523,20 +551,23 @@ void WorldSocket::HandlePing(WorldPacket& recvPacket)
_OverSpeedPings = 0;
}
if (_worldSession)
{
_worldSession->SetLatency(latency);
_worldSession->ResetClientTimeDelay();
}
else
{
TC_LOG_ERROR("network", "WorldSocket::HandlePing: peer sent CMSG_PING, but is not authenticated or got recently kicked, address = %s", GetRemoteIpAddress().to_string().c_str());
std::lock_guard<std::mutex> sessionGuard(_worldSessionLock);
CloseSocket();
return;
if (_worldSession)
{
_worldSession->SetLatency(latency);
_worldSession->ResetClientTimeDelay();
}
else
{
TC_LOG_ERROR("network", "WorldSocket::HandlePing: peer sent CMSG_PING, but is not authenticated or got recently kicked, address = %s", GetRemoteIpAddress().to_string().c_str());
return false;
}
}
WorldPacket packet(SMSG_PONG, 4);
packet << ping;
return SendPacket(packet);
SendPacketAndLogOpcode(packet);
return true;
}

View File

@@ -55,19 +55,25 @@ public:
void Start() override;
void SendPacket(WorldPacket& packet);
void SendPacket(WorldPacket const& packet);
protected:
void OnClose() override;
void ReadHandler() override;
bool ReadHeaderHandler();
bool ReadDataHandler();
private:
/// writes network.opcode log
/// accessing WorldSession is not threadsafe, only do it when holding _worldSessionLock
void LogOpcodeText(uint16 opcode, std::unique_lock<std::mutex> const& guard) const;
/// sends and logs network.opcode without accessing WorldSession
void SendPacketAndLogOpcode(WorldPacket const& packet);
void HandleSendAuthSession();
void HandleAuthSession(WorldPacket& recvPacket);
void SendAuthResponseError(uint8 code);
void HandlePing(WorldPacket& recvPacket);
bool HandlePing(WorldPacket& recvPacket);
uint32 _authSeed;
AuthCrypt _authCrypt;
@@ -75,7 +81,9 @@ private:
std::chrono::steady_clock::time_point _LastPingTime;
uint32 _OverSpeedPings;
std::mutex _worldSessionLock;
WorldSession* _worldSession;
bool _authed;
MessageBuffer _headerBuffer;
MessageBuffer _packetBuffer;

View File

@@ -62,7 +62,7 @@ public:
return false;
#ifndef TC_SOCKET_USE_IOCP
std::unique_lock<std::mutex> guard(_writeLock, std::try_to_lock);
std::unique_lock<std::mutex> guard(_writeLock);
if (!guard)
return true;
@@ -140,6 +140,8 @@ public:
if (shutdownError)
TC_LOG_DEBUG("network", "Socket::CloseSocket: %s errored when shutting down socket: %i (%s)", GetRemoteIpAddress().to_string().c_str(),
shutdownError.value(), shutdownError.message().c_str());
OnClose();
}
/// Marks the socket for closing after write buffer becomes empty
@@ -148,6 +150,8 @@ public:
MessageBuffer& GetReadBuffer() { return _readBuffer; }
protected:
virtual void OnClose() { }
virtual void ReadHandler() = 0;
bool AsyncProcessQueue(std::unique_lock<std::mutex>&)