From 7b0872aeccc2be460e8af5cd4a14b0660a83c1ed Mon Sep 17 00:00:00 2001 From: Ethan Jones Date: Thu, 23 Sep 2021 14:09:52 -0600 Subject: BungeeGuard style proxy security and OnlyAllowBungee config (#5291) --- src/ClientHandle.cpp | 75 +++++++++++++++++++++++++++++++++++++++++++ src/ClientHandle.h | 9 ++++++ src/Protocol/Protocol_1_8.cpp | 6 ++-- src/Server.cpp | 8 +++++ src/Server.h | 10 ++++++ 5 files changed, 104 insertions(+), 4 deletions(-) diff --git a/src/ClientHandle.cpp b/src/ClientHandle.cpp index cf70f870e..bbf018587 100644 --- a/src/ClientHandle.cpp +++ b/src/ClientHandle.cpp @@ -222,6 +222,28 @@ bool cClientHandle::IsUUIDOnline(const cUUID & a_UUID) +void cClientHandle::ProxyInit(const AString & a_IPString, const cUUID & a_UUID) +{ + this->SetIPString(a_IPString); + this->SetUUID(a_UUID); + + this->m_ProxyConnection = true; +} + + + + + +void cClientHandle::ProxyInit(const AString & a_IPString, const cUUID & a_UUID, const Json::Value & a_Properties) +{ + this->SetProperties(a_Properties); + this->ProxyInit(a_IPString, a_UUID); +} + + + + + void cClientHandle::ProcessProtocolOut() { decltype(m_OutgoingData) OutgoingData; @@ -264,6 +286,54 @@ void cClientHandle::Kick(const AString & a_Reason) +bool cClientHandle::BungeeAuthenticate() +{ + if (!m_ProxyConnection && cRoot::Get()->GetServer()->OnlyAllowBungeeCord()) + { + Kick("You can only connect to this server using a Proxy."); + + return false; + } + + cServer * Server = cRoot::Get()->GetServer(); + + // Proxy Shared Secret Check (BungeeGuard) + const AString & ForwardSecret = Server->GetProxySharedSecret(); + const bool AllowBungee = Server->ShouldAllowBungeeCord(); + const bool RequireForwardSecret = AllowBungee && !ForwardSecret.empty(); + + if (RequireForwardSecret) + { + for (auto & Node : GetProperties()) + { + if (Node.get("name", "").asString() == "bungeeguard-token") + { + AString SentToken = Node.get("value", "").asString(); + + if (ForwardSecret.compare(SentToken) == 0) + { + return true; + } + + break; + } + } + + Kick("Unable to authenticate."); + return false; + } + else if (m_ProxyConnection) + { + LOG("A player connected through a proxy without requiring a forwarding secret. If open to the internet, this is very insecure!"); + } + + return true; +} + + + + + void cClientHandle::Authenticate(const AString & a_Name, const cUUID & a_UUID, const Json::Value & a_Properties) { { @@ -281,6 +351,11 @@ void cClientHandle::Authenticate(const AString & a_Name, const cUUID & a_UUID, c ASSERT(m_Player == nullptr); + if (!BungeeAuthenticate()) + { + return; + } + m_Username = a_Name; // Only assign UUID and properties if not already pre-assigned (BungeeCord sends those in the Handshake packet): diff --git a/src/ClientHandle.h b/src/ClientHandle.h index 1a48e0458..132cc7225 100644 --- a/src/ClientHandle.h +++ b/src/ClientHandle.h @@ -101,6 +101,10 @@ public: // tolua_export We use Version-3 UUIDs for offline UUIDs, online UUIDs are Version-4, thus we can tell them apart. */ static bool IsUUIDOnline(const cUUID & a_UUID); // Exported in ManualBindings.cpp + /** Function to mark bungee / proxy connection on this client, and to add proxy-related data */ + void ProxyInit(const AString & a_IPString, const cUUID & a_UUID); + void ProxyInit(const AString & a_IPString, const cUUID & a_UUID, const Json::Value & a_Properties); + /** Flushes all buffered outgoing data to the network. */ void ProcessProtocolOut(); @@ -114,6 +118,9 @@ public: // tolua_export void Kick(const AString & a_Reason); // 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); @@ -470,6 +477,8 @@ private: Otherwise, this contains an arbitrary value which should not be used. */ cChunkCoords m_CachedSentChunk; + bool m_ProxyConnection; ///< True if player connected from a proxy (Bungee / Velocity) + bool m_HasSentDC; ///< True if a Disconnect packet has been sent in either direction // Chunk position when the last StreamChunks() was called; used to avoid re-streaming while in the same chunk diff --git a/src/Protocol/Protocol_1_8.cpp b/src/Protocol/Protocol_1_8.cpp index e7e110296..c34e95117 100644 --- a/src/Protocol/Protocol_1_8.cpp +++ b/src/Protocol/Protocol_1_8.cpp @@ -123,20 +123,18 @@ cProtocol_1_8_0::cProtocol_1_8_0(cClientHandle * a_Client, const AString & a_Ser LOGD("Player at %s connected via BungeeCord", Params[1].c_str()); - m_Client->SetIPString(Params[1]); - cUUID UUID; UUID.FromString(Params[2]); - m_Client->SetUUID(UUID); Json::Value root; if (!JsonUtils::ParseString(Params[3], root)) { LOGERROR("Unable to parse player properties: '%s'", Params[3]); + m_Client->ProxyInit(Params[1], UUID); } else { - m_Client->SetProperties(root); + m_Client->ProxyInit(Params[1], UUID, root); } } else diff --git a/src/Server.cpp b/src/Server.cpp index f70c0d092..8be63c083 100644 --- a/src/Server.cpp +++ b/src/Server.cpp @@ -192,11 +192,19 @@ bool cServer::InitServer(cSettingsRepositoryInterface & a_Settings, bool a_Shoul // Check if both BungeeCord and online mode are on, if so, warn the admin: m_ShouldAllowBungeeCord = a_Settings.GetValueSetB("Authentication", "AllowBungeeCord", false); + m_OnlyAllowBungeeCord = a_Settings.GetValueSetB("Authentication", "OnlyAllowBungeeCord", false); + m_ProxySharedSecret = a_Settings.GetValueSet("Authentication", "ProxySharedSecret", ""); + if (m_ShouldAllowBungeeCord && m_ShouldAuthenticate) { LOGWARNING("WARNING: BungeeCord is allowed and server set to online mode. This is unsafe and will not work properly. Disable either authentication or BungeeCord in settings.ini."); } + if (m_ShouldAllowBungeeCord && m_ProxySharedSecret.empty()) + { + LOGWARNING("WARNING: There is not a Proxy Forward Secret set up, and any proxy server can forward a player to this server unless closed from the internet."); + } + m_ShouldAllowMultiWorldTabCompletion = a_Settings.GetValueSetB("Server", "AllowMultiWorldTabCompletion", true); m_ShouldLimitPlayerBlockChanges = a_Settings.GetValueSetB("AntiCheat", "LimitPlayerBlockChanges", true); diff --git a/src/Server.h b/src/Server.h index 7701faa69..6bdab3e85 100644 --- a/src/Server.h +++ b/src/Server.h @@ -149,6 +149,10 @@ public: it makes the server vulnerable to identity theft through direct connections. */ bool ShouldAllowBungeeCord(void) const { return m_ShouldAllowBungeeCord; } + bool OnlyAllowBungeeCord(void) const { return m_OnlyAllowBungeeCord; } + + const AString & GetProxySharedSecret(void) const { return m_ProxySharedSecret; } + /** Returns true if usernames should be completed across worlds. This is read from the settings. */ bool ShouldAllowMultiWorldTabCompletion(void) const { return m_ShouldAllowMultiWorldTabCompletion; } @@ -240,6 +244,12 @@ private: /** True if BungeeCord handshake packets (with player UUID) should be accepted. */ bool m_ShouldAllowBungeeCord; + /** True if BungeeCord handshake packets should be the only ones accepted. */ + bool m_OnlyAllowBungeeCord; + + /** Security string that the proxy server should send, compatible with BungeeGuard */ + AString m_ProxySharedSecret; + /** True if usernames should be completed across worlds. */ bool m_ShouldAllowMultiWorldTabCompletion; -- cgit v1.2.3