Apply suggestions from code review

Co-authored-by: Lion <development@kortlepel.com>

.github/workflows/cmake-windows.yml

@@ -20,7 +20,7 @@ jobs:
         with:
             vcpkgArguments: 'lua zlib rapidjson openssl websocketpp curl'
             vcpkgDirectory: '${{ runner.workspace }}/b/vcpkg'
-            vcpkgGitCommitId: "16ee2ecb31788c336ace8bb14c21801efb6836e4"
+            vcpkgGitCommitId: "06b5f4a769d848d1a20fa0acd556019728b56273"
             vcpkgTriplet: 'x64-windows-static'
 
       - name: Create Build Environment

.github/workflows/release-build.yml

@@ -85,7 +85,7 @@ jobs:
         with:
             vcpkgArguments: 'lua zlib rapidjson openssl websocketpp curl'
             vcpkgDirectory: '${{ runner.workspace }}/b/vcpkg'
-            vcpkgGitCommitId: '16ee2ecb31788c336ace8bb14c21801efb6836e4'
+            vcpkgGitCommitId: '06b5f4a769d848d1a20fa0acd556019728b56273'
             vcpkgTriplet: 'x64-windows-static'
 
       - name: Create Build Environment

README.md

@@ -117,7 +117,7 @@ Ubuntu 22.04
 git
 libz-dev
 rapidjson-dev
-liblua5.3
+liblua5.3-dev
 libssl-dev
 libwebsocketpp-dev
 libcurl4-openssl-dev
@@ -142,7 +142,6 @@ curl
 git
 cmake
 g++
-cmake
 zlib
 boost
 websocketpp

include/Common.h

@@ -49,6 +49,7 @@ public:
         std::string Resource { "Resources" };
         std::string MapName { "/levels/gridmap_v2/info.json" };
         std::string Key {};
+        std::string Password{};
         std::string SSLKeyPath { "./.ssl/HttpServer/key.pem" };
         std::string SSLCertPath { "./.ssl/HttpServer/cert.pem" };
         bool HTTPServerEnabled { false };
@@ -137,7 +138,7 @@ private:
     static inline std::mutex mShutdownHandlersMutex {};
     static inline std::deque<TShutdownHandler> mShutdownHandlers {};
 
-    static inline Version mVersion { 3, 1, 2 };
+    static inline Version mVersion { 3, 1, 1 };
 };
 
 std::string ThreadName(bool DebugModeOverride = false);

include/TNetwork.h

@@ -52,4 +52,5 @@ private:
     static const uint8_t* SendSplit(TClient& c, ip::tcp::socket& Socket, const uint8_t* DataPtr, size_t Size);
 };
 
+std::string HashPassword(const std::string& str);
 std::vector<uint8_t> StringToVector(const std::string& Str);

src/TConfig.cpp

@@ -18,6 +18,7 @@ static constexpr std::string_view StrDescription = "Description";
 static constexpr std::string_view StrResourceFolder = "ResourceFolder";
 static constexpr std::string_view StrAuthKey = "AuthKey";
 static constexpr std::string_view StrLogChat = "LogChat";
+static constexpr std::string_view StrPassword = "Password";
 
 // Misc
 static constexpr std::string_view StrSendErrors = "SendErrors";
@@ -106,6 +107,8 @@ void TConfig::FlushToFile() {
     data["General"][StrMap.data()] = Application::Settings.MapName;
     data["General"][StrDescription.data()] = Application::Settings.ServerDesc;
     data["General"][StrResourceFolder.data()] = Application::Settings.Resource;
+    data["General"][StrPassword.data()] = Application::Settings.Password;
+    SetComment(data["General"][StrPassword.data()].comments(), " Sets a password on this server, which restricts people from joining. To join, a player must enter this exact password. Leave empty ("") to disable the password.");
     // Misc
     data["Misc"][StrHideUpdateMessages.data()] = Application::Settings.HideUpdateMessages;
     SetComment(data["Misc"][StrHideUpdateMessages.data()].comments(), " Hides the periodic update message which notifies you of a new server version. You should really keep this on and always update as soon as possible. For more information visit https://wiki.beammp.com/en/home/server-maintenance#updating-the-server. An update message will always appear at startup regardless.");
@@ -189,6 +192,7 @@ void TConfig::ParseFromFile(std::string_view name) {
         TryReadValue(data, "General", StrResourceFolder, Application::Settings.Resource);
         TryReadValue(data, "General", StrAuthKey, Application::Settings.Key);
         TryReadValue(data, "General", StrLogChat, Application::Settings.LogChat);
+        TryReadValue(data, "General", StrPassword, Application::Settings.Password);
         // Misc
         TryReadValue(data, "Misc", StrSendErrors, Application::Settings.SendErrors);
         TryReadValue(data, "Misc", StrHideUpdateMessages, Application::Settings.HideUpdateMessages);
@@ -240,6 +244,7 @@ void TConfig::PrintDebug() {
     beammp_debug(std::string(StrHTTPServerIP) + ": \"" + Application::Settings.HTTPServerIP + "\"");
     // special!
     beammp_debug("Key Length: " + std::to_string(Application::Settings.Key.length()) + "");
+    beammp_debug("Password Protected: " + std::string(Application::Settings.Password.empty() ? "false" : "true"));
 }
 
 void TConfig::ParseOldFormat() {

src/THeartbeatThread.cpp

@@ -149,7 +149,8 @@ std::string THeartbeatThread::GenerateCall() {
         << "&modstotalsize=" << mResourceManager.MaxModSize()
         << "&modstotal=" << mResourceManager.ModsLoaded()
         << "&playerslist=" << GetPlayers()
-        << "&desc=" << Application::Settings.ServerDesc;
+        << "&desc=" << Application::Settings.ServerDesc
+        << "&pass=" << (Application::Settings.Password.empty() ? "false" : "true");
     return Ret.str();
 }
 THeartbeatThread::THeartbeatThread(TResourceManager& ResourceManager, TServer& Server)

src/TNetwork.cpp

@@ -11,6 +11,8 @@
 #include <boost/asio/ip/address_v4.hpp>
 #include <cstring>
 
+typedef boost::asio::detail::socket_option::integer<SOL_SOCKET, SO_RCVTIMEO> rcv_timeout_option;
+
 std::vector<uint8_t> StringToVector(const std::string& Str) {
     return std::vector<uint8_t>(Str.data(), Str.data() + Str.size());
 }
@@ -151,6 +153,7 @@ void TNetwork::TCPServerMain() {
             if (ec) {
                 beammp_errorf("failed to accept: {}", ec.message());
             }
+            ClientSocket.set_option(rcv_timeout_option{ 120000 }); //timeout of 120seconds
             TConnection Conn { std::move(ClientSocket), ClientEp };
             std::thread ID(&TNetwork::Identify, this, std::move(Conn));
             ID.detach(); // TODO: Add to a queue and attempt to join periodically
@@ -177,29 +180,16 @@ void TNetwork::Identify(TConnection&& RawConnection) {
         return;
     }
     std::shared_ptr<TClient> Client { nullptr };
-    try {
-        if (Code == 'C') {
-            Client = Authentication(std::move(RawConnection));
-        } else if (Code == 'D') {
-            HandleDownload(std::move(RawConnection));
-        } else if (Code == 'P') {
-            boost::system::error_code ec;
-            write(RawConnection.Socket, buffer("P"), ec);
-            return;
-        } else {
-            beammp_errorf("Invalid code got in Identify: '{}'", Code);
-        }
-    } catch(const std::exception& e) {
-        beammp_errorf("Error during handling of code {} - client left in invalid state, closing socket", Code);
+    if (Code == 'C') {
+        Client = Authentication(std::move(RawConnection));
+    } else if (Code == 'D') {
+        HandleDownload(std::move(RawConnection));
+    } else if (Code == 'P') {
         boost::system::error_code ec;
-        RawConnection.Socket.shutdown(socket_base::shutdown_both, ec);
-        if (ec) {
-            beammp_debugf("Failed to shutdown client socket: {}", ec.message());
-        }
-        RawConnection.Socket.close(ec);
-        if (ec) {
-            beammp_debugf("Failed to close client socket: {}", ec.message());
-        }
+        write(RawConnection.Socket, buffer("P"), ec);
+        return;
+    } else {
+        beammp_errorf("Invalid code got in Identify: '{}'", Code);
     }
 }
 
@@ -225,6 +215,15 @@ void TNetwork::HandleDownload(TConnection&& Conn) {
     });
 }
 
+std::string HashPassword(const std::string& str) {
+    std::stringstream ret;
+    unsigned char* hash = SHA256(reinterpret_cast<const unsigned char*>(str.c_str()), str.length(), nullptr);
+    for (int i = 0; i < 32; i++) {
+        ret << std::hex << static_cast<int>(hash[i]);
+    }
+    return ret.str();
+}
+
 std::shared_ptr<TClient> TNetwork::Authentication(TConnection&& RawConnection) {
     auto Client = CreateClient(std::move(RawConnection.Socket));
     Client->SetIdentifier("ip", RawConnection.SockAddr.address().to_string());
@@ -248,7 +247,8 @@ std::shared_ptr<TClient> TNetwork::Authentication(TConnection&& RawConnection) {
         ClientKick(*Client, fmt::format("Invalid version header: '{}' ({})", std::string(reinterpret_cast<const char*>(Data.data()), Data.size()), Data.size()));
         return nullptr;
     }
-    if (!TCPSend(*Client, StringToVector("S"))) {
+
+    if (!TCPSend(*Client, StringToVector("A"))) { //changed to A for Accepted version
         // TODO: handle
     }
 
@@ -259,24 +259,12 @@ std::shared_ptr<TClient> TNetwork::Authentication(TConnection&& RawConnection) {
         return nullptr;
     }
 
-    std::string key(reinterpret_cast<const char*>(Data.data()), Data.size());
-
-    nlohmann::json AuthReq{};
-    std::string AuthResStr{};
-    try {
-        AuthReq = nlohmann::json {
-            { "key", key }
-        };
-
-        auto Target = "/pkToUser";
-        unsigned int ResponseCode = 0;
-        AuthResStr = Http::POST(Application::GetBackendUrlForAuth(), 443, Target, AuthReq.dump(), "application/json", &ResponseCode);
-
-    } catch (const std::exception& e) {
-        beammp_debugf("Invalid json sent by client, kicking: {}", e.what());
-        ClientKick(*Client, "Invalid Key (invalid UTF8 string)!");
-        return nullptr;
-    }
+    nlohmann::json AuthReq {
+        { "key", std::string(reinterpret_cast<const char*>(Data.data()), Data.size()) }
+    };
+    auto Target = "/pkToUser";
+    unsigned int ResponseCode = 0;
+    const auto AuthResStr = Http::POST(Application::GetBackendUrlForAuth(), 443, Target, AuthReq.dump(), "application/json", &ResponseCode);
 
     try {
         nlohmann::json AuthRes = nlohmann::json::parse(AuthResStr);
@@ -304,6 +292,22 @@ std::shared_ptr<TClient> TNetwork::Authentication(TConnection&& RawConnection) {
         return nullptr;
     }
 
+    if(!Application::Settings.Password.empty()) { // ask password
+        if(!TCPSend(*Client, StringToVector("S"))) {
+            // TODO: handle
+        }
+        beammp_info("Waiting for password");
+        Data = TCPRcv(*Client);
+        std::string Pass = std::string(reinterpret_cast<const char*>(Data.data()), Data.size());
+        if(Pass != HashPassword(Application::Settings.Password)) {
+            beammp_debug(Client->GetName() + " attempted to connect with a wrong password");
+            ClientKick(*Client, "Wrong password!");
+            return {};
+        } else {
+            beammp_debug(Client->GetName() + " used the correct password");
+        }
+    }
+
     beammp_debug("Name -> " + Client->GetName() + ", Guest -> " + std::to_string(Client->IsGuest()) + ", Roles -> " + Client->GetRoles());
     mServer.ForEachClient([&](const std::weak_ptr<TClient>& ClientPtr) -> bool {
         std::shared_ptr<TClient> Cl;
@@ -353,6 +357,7 @@ std::shared_ptr<TClient> TNetwork::Authentication(TConnection&& RawConnection) {
     } else {
         ClientKick(*Client, "Server full!");
     }
+
     return Client;
 }