From 1a0d9450eaa0f3c2ff475175f5d45932fd4dd7fa Mon Sep 17 00:00:00 2001 From: Tiger Wang Date: Sat, 2 Oct 2021 21:27:32 +0100 Subject: [PATCH] Authentication flow: move construction, slim down classes (#5312) - Remove extra members in ForgeHandshake --- src/ClientHandle.cpp | 23 +++++++------- src/ClientHandle.h | 14 ++++----- src/Protocol/Authenticator.cpp | 25 +++++++-------- src/Protocol/Authenticator.h | 2 +- src/Protocol/ForgeHandshake.cpp | 55 +++++++++++++++------------------ src/Protocol/ForgeHandshake.h | 34 ++++++++++---------- src/Protocol/Protocol_1_8.cpp | 3 +- src/Root.cpp | 9 ------ src/Root.h | 3 -- src/Server.cpp | 10 +++--- src/Server.h | 4 +-- 11 files changed, 80 insertions(+), 102 deletions(-) diff --git a/src/ClientHandle.cpp b/src/ClientHandle.cpp index 4c73f060f..efe4c9d48 100644 --- a/src/ClientHandle.cpp +++ b/src/ClientHandle.cpp @@ -67,7 +67,6 @@ float cClientHandle::FASTBREAK_PERCENTAGE; // cClientHandle: cClientHandle::cClientHandle(const AString & a_IPString, int a_ViewDistance) : - m_ForgeHandshake(this), m_CurrentViewDistance(a_ViewDistance), m_RequestedViewDistance(a_ViewDistance), m_IPString(a_IPString), @@ -334,7 +333,7 @@ bool cClientHandle::BungeeAuthenticate() -void cClientHandle::Authenticate(const AString & a_Name, const cUUID & a_UUID, const Json::Value & a_Properties) +void cClientHandle::Authenticate(AString && a_Name, const cUUID & a_UUID, Json::Value && a_Properties) { { cCSLock lock(m_CSState); @@ -356,7 +355,7 @@ void cClientHandle::Authenticate(const AString & a_Name, const cUUID & a_UUID, c return; } - m_Username = a_Name; + m_Username = std::move(a_Name); // Only assign UUID and properties if not already pre-assigned (BungeeCord sends those in the Handshake packet): if (m_UUID.IsNil()) @@ -365,19 +364,19 @@ void cClientHandle::Authenticate(const AString & a_Name, const cUUID & a_UUID, c } if (m_Properties.empty()) { - m_Properties = a_Properties; + m_Properties = std::move(a_Properties); } // Send login success (if the protocol supports it): m_Protocol->SendLoginSuccess(); - if (m_ForgeHandshake.m_IsForgeClient) + if (m_ForgeHandshake.IsForgeClient) { - m_ForgeHandshake.BeginForgeHandshake(a_Name, a_UUID, a_Properties); + m_ForgeHandshake.BeginForgeHandshake(*this); } else { - FinishAuthenticate(a_Name, a_UUID, a_Properties); + FinishAuthenticate(); } } } @@ -386,7 +385,7 @@ void cClientHandle::Authenticate(const AString & a_Name, const cUUID & a_UUID, c -void cClientHandle::FinishAuthenticate(const AString & a_Name, const cUUID & a_UUID, const Json::Value & a_Properties) +void cClientHandle::FinishAuthenticate() { // Serverside spawned player (so data are loaded). std::unique_ptr Player; @@ -422,8 +421,8 @@ void cClientHandle::FinishAuthenticate(const AString & a_Name, const cUUID & a_U if (!cRoot::Get()->GetPluginManager()->CallHookPlayerJoined(*m_Player)) { - cRoot::Get()->BroadcastChatJoin(Printf("%s has joined the game", a_Name.c_str())); - LOGINFO("Player %s has joined the game", a_Name.c_str()); + cRoot::Get()->BroadcastChatJoin(Printf("%s has joined the game", m_Username.c_str())); + LOGINFO("Player %s has joined the game", m_Username.c_str()); } // TODO: this accesses the world spawn from the authenticator thread @@ -747,7 +746,7 @@ bool cClientHandle::HandleLogin() } // lock(m_CSState) // Schedule for authentication; until then, let the player wait (but do not block) - cRoot::Get()->GetAuthenticator().Authenticate(GetUniqueID(), GetUsername(), m_Protocol->GetAuthServerID()); + cRoot::Get()->GetAuthenticator().Authenticate(GetUniqueID(), std::move(m_Username), m_Protocol->GetAuthServerID()); return true; } @@ -887,7 +886,7 @@ void cClientHandle::HandlePluginMessage(const AString & a_Channel, const Contigu } else if (a_Channel == "FML|HS") { - m_ForgeHandshake.DataReceived(this, a_Message); + m_ForgeHandshake.DataReceived(*this, a_Message); } else if (!HasPluginChannel(a_Channel)) { diff --git a/src/ClientHandle.h b/src/ClientHandle.h index 132cc7225..f454e53bd 100644 --- a/src/ClientHandle.h +++ b/src/ClientHandle.h @@ -121,8 +121,8 @@ public: // tolua_export /** Authenticates the specified user with the bungee proxy server */ bool BungeeAuthenticate(); - /** Authenticates the specified user, called by cAuthenticator */ - void Authenticate(const AString & a_Name, const cUUID & a_UUID, const Json::Value & a_Properties); + /** Authenticates ourselves, called by cAuthenticator supplying player details from Mojang. */ + void Authenticate(AString && a_Name, const cUUID & a_UUID, Json::Value && a_Properties); /** Sends a set number of new chunks to the player on every invocation, until all chunks in the view distance have been sent. */ void StreamNextChunks(); @@ -276,20 +276,20 @@ public: // tolua_export const AStringMap & GetForgeMods(void) const { return m_ForgeMods; } /** Returns true if the client is modded with Forge. */ - bool IsForgeClient(void) const { return m_ForgeHandshake.m_IsForgeClient; } + bool IsForgeClient(void) const { return m_ForgeHandshake.IsForgeClient; } // tolua_end /** Add the Forge mod list to the server ping response. */ void ForgeAugmentServerListPing(Json::Value & a_Response) { - m_ForgeHandshake.AugmentServerListPing(a_Response); + m_ForgeHandshake.AugmentServerListPing(*this, a_Response); } /** Mark a client connection as using Forge. Set by the protocol. */ void SetIsForgeClient() { - m_ForgeHandshake.m_IsForgeClient = true; + m_ForgeHandshake.IsForgeClient = true; } /** Returns true if the client wants the chunk specified to be sent (in m_ChunksToSend) */ @@ -418,8 +418,6 @@ public: // tolua_export private: - friend class cServer; // Needs access to SetSelf() - friend class cForgeHandshake; // Needs access to FinishAuthenticate() /** The type used for storing the names of registered plugin channels. */ @@ -573,7 +571,7 @@ private: float m_BreakProgress; /** Finish logging the user in after authenticating. */ - void FinishAuthenticate(const AString & a_Name, const cUUID & a_UUID, const Json::Value & a_Properties); + void FinishAuthenticate(); /** Returns true if the rate block interactions is within a reasonable limit (bot protection) */ bool CheckBlockInteractionsRate(void); diff --git a/src/Protocol/Authenticator.cpp b/src/Protocol/Authenticator.cpp index 6233ddb32..15a246f6e 100644 --- a/src/Protocol/Authenticator.cpp +++ b/src/Protocol/Authenticator.cpp @@ -57,17 +57,17 @@ void cAuthenticator::ReadSettings(cSettingsRepositoryInterface & a_Settings) -void cAuthenticator::Authenticate(int a_ClientID, const AString & a_UserName, const AString & a_ServerHash) +void cAuthenticator::Authenticate(int a_ClientID, AString && a_Username, const AString & a_ServerHash) { if (!m_ShouldAuthenticate) { - Json::Value Value; - cRoot::Get()->AuthenticateUser(a_ClientID, a_UserName, cClientHandle::GenerateOfflineUUID(a_UserName), Value); + const auto UUID = cClientHandle::GenerateOfflineUUID(a_Username); + cRoot::Get()->GetServer()->AuthenticateUser(a_ClientID, std::move(a_Username), UUID, Json::Value{}); return; } - cCSLock LOCK(m_CS); - m_Queue.emplace_back(a_ClientID, a_UserName, a_ServerHash); + cCSLock Lock(m_CS); + m_Queue.emplace_back(a_ClientID, std::move(a_Username), a_ServerHash); m_QueueNonempty.Set(); } @@ -112,20 +112,19 @@ void cAuthenticator::Execute(void) } ASSERT(!m_Queue.empty()); - cAuthenticator::cUser & User = m_Queue.front(); - int ClientID = User.m_ClientID; - AString UserName = User.m_Name; - AString ServerID = User.m_ServerID; + cAuthenticator::cUser User = std::move(m_Queue.front()); + int & ClientID = User.m_ClientID; + AString & UserName = User.m_Name; + AString & ServerID = User.m_ServerID; m_Queue.pop_front(); Lock.Unlock(); - AString NewUserName = UserName; cUUID UUID; Json::Value Properties; - if (AuthWithYggdrasil(NewUserName, ServerID, UUID, Properties)) + if (AuthWithYggdrasil(UserName, ServerID, UUID, Properties)) { - LOGINFO("User %s authenticated with UUID %s", NewUserName.c_str(), UUID.ToShortString().c_str()); - cRoot::Get()->AuthenticateUser(ClientID, NewUserName, UUID, Properties); + LOGINFO("User %s authenticated with UUID %s", UserName.c_str(), UUID.ToShortString().c_str()); + cRoot::Get()->GetServer()->AuthenticateUser(ClientID, std::move(UserName), UUID, std::move(Properties)); } else { diff --git a/src/Protocol/Authenticator.h b/src/Protocol/Authenticator.h index 5fd8e5434..39a50ac47 100644 --- a/src/Protocol/Authenticator.h +++ b/src/Protocol/Authenticator.h @@ -41,7 +41,7 @@ public: void ReadSettings(cSettingsRepositoryInterface & a_Settings); /** Queues a request for authenticating a user. If the auth fails, the user will be kicked */ - void Authenticate(int a_ClientID, const AString & a_UserName, const AString & a_ServerHash); + void Authenticate(int a_ClientID, AString && a_Username, const AString & a_ServerHash); /** Starts the authenticator thread. The thread may be started and stopped repeatedly */ void Start(cSettingsRepositoryInterface & a_Settings); diff --git a/src/Protocol/ForgeHandshake.cpp b/src/Protocol/ForgeHandshake.cpp index 99b585894..34e044fbc 100644 --- a/src/Protocol/ForgeHandshake.cpp +++ b/src/Protocol/ForgeHandshake.cpp @@ -44,10 +44,9 @@ namespace ServerPhase -cForgeHandshake::cForgeHandshake(cClientHandle *a_Client) : - m_IsForgeClient(false), - m_Errored(false), - m_Client(a_Client) +cForgeHandshake::cForgeHandshake() : + IsForgeClient(false), + m_Errored(false) { } @@ -65,9 +64,9 @@ void cForgeHandshake::SetError(const AString & message) -void cForgeHandshake::AugmentServerListPing(Json::Value & a_ResponseValue) +void cForgeHandshake::AugmentServerListPing(cClientHandle & a_Client, Json::Value & a_ResponseValue) { - auto ProtocolVersion = m_Client->GetProtocolVersion(); + auto ProtocolVersion = a_Client.GetProtocolVersion(); auto & Mods = cRoot::Get()->GetServer()->GetRegisteredForgeMods(ProtocolVersion); if (Mods.empty()) @@ -97,13 +96,9 @@ void cForgeHandshake::AugmentServerListPing(Json::Value & a_ResponseValue) -void cForgeHandshake::BeginForgeHandshake(const AString & a_Name, const cUUID & a_UUID, const Json::Value & a_Properties) +void cForgeHandshake::BeginForgeHandshake(cClientHandle & a_Client) { - ASSERT(m_IsForgeClient); - - m_Name = a_Name; - m_UUID = a_UUID; - m_Properties = a_Properties; + ASSERT(IsForgeClient); static const std::array Channels{{ "FML|HS", "FML", "FML|MP", "FML", "FORGE" }}; ContiguousByteBuffer ChannelsString; @@ -113,15 +108,15 @@ void cForgeHandshake::BeginForgeHandshake(const AString & a_Name, const cUUID & ChannelsString.push_back(std::byte(0)); } - m_Client->SendPluginMessage("REGISTER", ChannelsString); - SendServerHello(); + a_Client.SendPluginMessage("REGISTER", ChannelsString); + SendServerHello(a_Client); } -void cForgeHandshake::SendServerHello() +void cForgeHandshake::SendServerHello(cClientHandle & a_Client) { cByteBuffer Buf(6); // Discriminator | Byte | Always 0 for ServerHello @@ -134,7 +129,7 @@ void cForgeHandshake::SendServerHello() ContiguousByteBuffer Message; Buf.ReadAll(Message); - m_Client->SendPluginMessage("FML|HS", Message); + a_Client.SendPluginMessage("FML|HS", Message); } @@ -183,7 +178,7 @@ AStringMap cForgeHandshake::ParseModList(const ContiguousByteBufferView a_Data) -void cForgeHandshake::HandleClientHello(cClientHandle * a_Client, const ContiguousByteBufferView a_Data) +void cForgeHandshake::HandleClientHello(cClientHandle & a_Client, const ContiguousByteBufferView a_Data) { if (a_Data.size() == 2) { @@ -204,7 +199,7 @@ void cForgeHandshake::HandleClientHello(cClientHandle * a_Client, const Contiguo -void cForgeHandshake::HandleModList(cClientHandle * a_Client, const ContiguousByteBufferView a_Data) +void cForgeHandshake::HandleModList(cClientHandle & a_Client, const ContiguousByteBufferView a_Data) { LOGD("Received ModList"); @@ -217,10 +212,10 @@ void cForgeHandshake::HandleModList(cClientHandle * a_Client, const ContiguousBy LOG("Client connected with %zu mods: %s", ClientMods.size(), ClientModsString.c_str()); - m_Client->m_ForgeMods = ClientMods; + a_Client.m_ForgeMods = ClientMods; // Let the plugins know about this event, they may refuse the player: - if (cRoot::Get()->GetPluginManager()->CallHookLoginForge(*a_Client, ClientMods)) + if (cRoot::Get()->GetPluginManager()->CallHookLoginForge(a_Client, ClientMods)) { SetError("Modded client refused by plugin"); return; @@ -229,7 +224,7 @@ void cForgeHandshake::HandleModList(cClientHandle * a_Client, const ContiguousBy // Send server ModList // Send server-side Forge mods registered by plugins - const auto & ServerMods = m_Client->GetForgeMods(); + const auto & ServerMods = a_Client.GetForgeMods(); const auto ModCount = ServerMods.size(); @@ -246,14 +241,14 @@ void cForgeHandshake::HandleModList(cClientHandle * a_Client, const ContiguousBy ContiguousByteBuffer ServerModList; Buf.ReadAll(ServerModList); - m_Client->SendPluginMessage("FML|HS", ServerModList); + a_Client.SendPluginMessage("FML|HS", ServerModList); } -void cForgeHandshake::HandleHandshakeAck(cClientHandle * a_Client, const ContiguousByteBufferView a_Data) +void cForgeHandshake::HandleHandshakeAck(cClientHandle & a_Client, const ContiguousByteBufferView a_Data) { if (a_Data.size() != 2) { @@ -286,7 +281,7 @@ void cForgeHandshake::HandleHandshakeAck(cClientHandle * a_Client, const Contigu ContiguousByteBuffer RegistryData; Buf.ReadAll(RegistryData); - m_Client->SendPluginMessage("FML|HS", RegistryData); + a_Client.SendPluginMessage("FML|HS", RegistryData); break; } @@ -297,7 +292,7 @@ void cForgeHandshake::HandleHandshakeAck(cClientHandle * a_Client, const Contigu ContiguousByteBuffer Ack; Ack.push_back(std::byte(Discriminator::HandshakeAck)); Ack.push_back(ServerPhase::WAITINGCACK); - m_Client->SendPluginMessage("FML|HS", Ack); + a_Client.SendPluginMessage("FML|HS", Ack); break; } @@ -308,15 +303,15 @@ void cForgeHandshake::HandleHandshakeAck(cClientHandle * a_Client, const Contigu ContiguousByteBuffer Ack; Ack.push_back(std::byte(Discriminator::HandshakeAck)); Ack.push_back(ServerPhase::COMPLETE); - m_Client->SendPluginMessage("FML|HS", Ack); + a_Client.SendPluginMessage("FML|HS", Ack); break; } case ClientPhase::COMPLETE: { - // Now finish logging in - m_Client->FinishAuthenticate(m_Name, m_UUID, m_Properties); + // Now finish logging in: + a_Client.FinishAuthenticate(); break; } @@ -332,9 +327,9 @@ void cForgeHandshake::HandleHandshakeAck(cClientHandle * a_Client, const Contigu -void cForgeHandshake::DataReceived(cClientHandle * a_Client, const ContiguousByteBufferView a_Data) +void cForgeHandshake::DataReceived(cClientHandle & a_Client, const ContiguousByteBufferView a_Data) { - if (!m_IsForgeClient) + if (!IsForgeClient) { SetError(Printf("Received unexpected Forge data from non-Forge client (%zu bytes)", a_Data.size())); return; diff --git a/src/Protocol/ForgeHandshake.h b/src/Protocol/ForgeHandshake.h index 061369c15..7c3969b96 100644 --- a/src/Protocol/ForgeHandshake.h +++ b/src/Protocol/ForgeHandshake.h @@ -9,6 +9,10 @@ #include "../UUID.h" #include "json/json.h" + + + + // fwd: class cClientHandle; @@ -19,38 +23,32 @@ class cClientHandle; class cForgeHandshake { public: - /** True if the client advertised itself as a Forge client. */ - bool m_IsForgeClient; - cForgeHandshake(cClientHandle * client); + /** True if the client advertised itself as a Forge client. */ + bool IsForgeClient; + + cForgeHandshake(); /** Add the registered Forge mods to the server ping list packet. */ - void AugmentServerListPing(Json::Value & ResponseValue); + void AugmentServerListPing(cClientHandle & a_Client, Json::Value & ResponseValue); /** Begin the Forge Modloader Handshake (FML|HS) sequence. */ - void BeginForgeHandshake(const AString & a_Name, const cUUID & a_UUID, const Json::Value & a_Properties); + void BeginForgeHandshake(cClientHandle & a_Client); /** Send the ServerHello packet in the Forge handshake. */ - void SendServerHello(); + void SendServerHello(cClientHandle & a_Client); /** Process received data from the client advancing the Forge handshake. */ - void DataReceived(cClientHandle * a_Client, ContiguousByteBufferView a_Data); + void DataReceived(cClientHandle & a_Client, ContiguousByteBufferView a_Data); private: + /** True if the Forge handshake is in an errored state. */ bool m_Errored; - /** The client handle undergoing this Forge handshake. */ - cClientHandle * m_Client; - - /** Values saved from BeginForgeHandshake() for continuing the normal handshake after Forge completes. */ - AString m_Name; - cUUID m_UUID; - Json::Value m_Properties; - - void HandleClientHello(cClientHandle * a_Client, ContiguousByteBufferView a_Data); - void HandleModList(cClientHandle * a_Client, ContiguousByteBufferView a_Data); - void HandleHandshakeAck(cClientHandle * a_Client, ContiguousByteBufferView a_Data); + void HandleClientHello(cClientHandle & a_Client, ContiguousByteBufferView a_Data); + void HandleModList(cClientHandle & a_Client, ContiguousByteBufferView a_Data); + void HandleHandshakeAck(cClientHandle & a_Client, ContiguousByteBufferView a_Data); /** Set errored state to prevent further handshake message processing. */ void SetError(const AString & message); diff --git a/src/Protocol/Protocol_1_8.cpp b/src/Protocol/Protocol_1_8.cpp index c34e95117..d17b65e2d 100644 --- a/src/Protocol/Protocol_1_8.cpp +++ b/src/Protocol/Protocol_1_8.cpp @@ -2161,6 +2161,7 @@ void cProtocol_1_8_0::HandlePacketStatusRequest(cByteBuffer & a_ByteBuffer) auto NumPlayers = static_cast(Server->GetNumPlayers()); auto MaxPlayers = static_cast(Server->GetMaxPlayers()); AString Favicon = Server->GetFaviconData(); + cRoot::Get()->GetPluginManager()->CallHookServerPing(*m_Client, ServerDescription, NumPlayers, MaxPlayers, Favicon); // Version: @@ -2177,7 +2178,7 @@ void cProtocol_1_8_0::HandlePacketStatusRequest(cByteBuffer & a_ByteBuffer) // Description: Json::Value Description; - Description["text"] = ServerDescription.c_str(); + Description["text"] = std::move(ServerDescription); // Create the response: Json::Value ResponseValue; diff --git a/src/Root.cpp b/src/Root.cpp index f100a242a..462544ed2 100644 --- a/src/Root.cpp +++ b/src/Root.cpp @@ -558,15 +558,6 @@ void cRoot::KickUser(int a_ClientID, const AString & a_Reason) -void cRoot::AuthenticateUser(int a_ClientID, const AString & a_Name, const cUUID & a_UUID, const Json::Value & a_Properties) -{ - m_Server->AuthenticateUser(a_ClientID, a_Name, a_UUID, a_Properties); -} - - - - - size_t cRoot::GetTotalChunkCount(void) { size_t Count = 0; diff --git a/src/Root.h b/src/Root.h index 74a0acbb4..1db13a70f 100644 --- a/src/Root.h +++ b/src/Root.h @@ -130,9 +130,6 @@ public: /** Kicks the user, no matter in what world they are. Used from cAuthenticator */ void KickUser(int a_ClientID, const AString & a_Reason); - /** Called by cAuthenticator to auth the specified user */ - void AuthenticateUser(int a_ClientID, const AString & a_Name, const cUUID & a_UUID, const Json::Value & a_Properties); - /** Returns the number of chunks loaded */ size_t GetTotalChunkCount(void); // tolua_export diff --git a/src/Server.cpp b/src/Server.cpp index 8be63c083..08142b008 100644 --- a/src/Server.cpp +++ b/src/Server.cpp @@ -709,7 +709,7 @@ void cServer::KickUser(int a_ClientID, const AString & a_Reason) -void cServer::AuthenticateUser(int a_ClientID, const AString & a_Name, const cUUID & a_UUID, const Json::Value & a_Properties) +void cServer::AuthenticateUser(int a_ClientID, AString && a_Username, const cUUID & a_UUID, Json::Value && a_Properties) { cCSLock Lock(m_CSClients); @@ -720,14 +720,14 @@ void cServer::AuthenticateUser(int a_ClientID, const AString & a_Name, const cUU return; } - for (auto itr = m_Clients.begin(); itr != m_Clients.end(); ++itr) + for (const auto & Client : m_Clients) { - if ((*itr)->GetUniqueID() == a_ClientID) + if (Client->GetUniqueID() == a_ClientID) { - (*itr)->Authenticate(a_Name, a_UUID, a_Properties); + Client->Authenticate(std::move(a_Username), a_UUID, std::move(a_Properties)); return; } - } // for itr - m_Clients[] + } } diff --git a/src/Server.h b/src/Server.h index 6bdab3e85..6fa2002e4 100644 --- a/src/Server.h +++ b/src/Server.h @@ -118,8 +118,8 @@ public: void KickUser(int a_ClientID, const AString & a_Reason); - /** Authenticates the specified user, called by cAuthenticator */ - void AuthenticateUser(int a_ClientID, const AString & a_Name, const cUUID & a_UUID, const Json::Value & a_Properties); + /** Authenticates the specified user, called by cAuthenticator supplying player details from Mojang. */ + void AuthenticateUser(int a_ClientID, AString && a_Username, const cUUID & a_UUID, Json::Value && a_Properties); const AString & GetServerID(void) const { return m_ServerID; } // tolua_export