From 604e4ace0f9e2a455947f89b11f0a2c678280c5b Mon Sep 17 00:00:00 2001 From: DcruBro Date: Tue, 17 Mar 2026 17:00:20 +0100 Subject: [PATCH 1/6] Addressing some bugs regarding lifetimes and callbacks that could trigger random-ish crashed - wip --- CMakeLists.txt | 2 +- .../columnlynx/client/net/tcp/tcp_client.hpp | 5 +- .../common/net/tcp/tcp_message_handler.hpp | 2 +- src/client/net/tcp/tcp_client.cpp | 46 +++++++++++-------- src/common/session_registry.cpp | 14 ++++-- src/common/tcp_message_handler.cpp | 15 +++--- src/common/utils.cpp | 2 +- src/server/main.cpp | 20 +++++++- src/server/net/tcp/tcp_connection.cpp | 37 +++++++++------ 9 files changed, 92 insertions(+), 51 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 0e48f08..6007c54 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -6,7 +6,7 @@ cmake_minimum_required(VERSION 3.16) # If MAJOR is 0, and MINOR > 0, Version is BETA project(ColumnLynx - VERSION 1.1.0 + VERSION 1.1.1 LANGUAGES CXX ) diff --git a/include/columnlynx/client/net/tcp/tcp_client.hpp b/include/columnlynx/client/net/tcp/tcp_client.hpp index 2faf3db..d39b135 100644 --- a/include/columnlynx/client/net/tcp/tcp_client.hpp +++ b/include/columnlynx/client/net/tcp/tcp_client.hpp @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include @@ -89,8 +90,8 @@ namespace ColumnLynx::Net::TCP { // TODO: Move ptrs to smart ptrs - bool mConnected = false; - bool mHandshakeComplete = false; + std::atomic mConnected{false}; + std::atomic mHandshakeComplete{false}; tcp::resolver mResolver; tcp::socket mSocket; std::shared_ptr mHandler; diff --git a/include/columnlynx/common/net/tcp/tcp_message_handler.hpp b/include/columnlynx/common/net/tcp/tcp_message_handler.hpp index 8bd1260..f855f09 100644 --- a/include/columnlynx/common/net/tcp/tcp_message_handler.hpp +++ b/include/columnlynx/common/net/tcp/tcp_message_handler.hpp @@ -39,6 +39,6 @@ namespace ColumnLynx::Net::TCP { std::array mHeader{}; // [type][lenHigh][lenLow] std::vector mBody; std::function mOnMessage; - std::function mOnDisconnect; + std::function mOnDisconnect; }; } \ No newline at end of file diff --git a/src/client/net/tcp/tcp_client.cpp b/src/client/net/tcp/tcp_client.cpp index c0e4da9..aac19c8 100644 --- a/src/client/net/tcp/tcp_client.cpp +++ b/src/client/net/tcp/tcp_client.cpp @@ -14,19 +14,25 @@ namespace ColumnLynx::Net::TCP { asio::async_connect(mSocket, endpoints, [this, self](asio::error_code ec, const tcp::endpoint&) { if (!ec) { - mConnected = true; + mConnected.store(true, std::memory_order_relaxed); Utils::log("Client connected."); mHandler = std::make_shared(std::move(mSocket)); - mHandler->onMessage([this](AnyMessageType type, const std::string& data) { - mHandleMessage(static_cast(MessageHandler::toUint8(type)), data); + mHandler->onMessage([weakSelf = weak_from_this()](AnyMessageType type, const std::string& data) { + if (auto self = weakSelf.lock()) { + self->mHandleMessage(static_cast(MessageHandler::toUint8(type)), data); + } }); // Close only after peer FIN to avoid RSTs - mHandler->onDisconnect([this](const asio::error_code& ec) { - asio::error_code ec2; - if (mHandler) { - mHandler->socket().close(ec2); + mHandler->onDisconnect([weakSelf = weak_from_this()](const asio::error_code& ec) { + auto self = weakSelf.lock(); + if (!self) { + return; } - mConnected = false; + asio::error_code ec2; + if (self->mHandler) { + self->mHandler->socket().close(ec2); + } + self->mConnected.store(false, std::memory_order_relaxed); Utils::log(std::string("Server disconnected: ") + ec.message()); }); mHandler->start(); @@ -74,7 +80,7 @@ namespace ColumnLynx::Net::TCP { } void TCPClient::sendMessage(ClientMessageType type, const std::string& data) { - if (!mConnected) { + if (!mConnected.load(std::memory_order_relaxed)) { Utils::error("Cannot send message, client not connected."); return; } @@ -87,7 +93,7 @@ namespace ColumnLynx::Net::TCP { } void TCPClient::disconnect(bool echo) { - if (mConnected && mHandler) { + if (mConnected.load(std::memory_order_relaxed) && mHandler) { if (echo) { mHandler->sendMessage(ClientMessageType::GRACEFUL_DISCONNECT, "Goodbye"); } @@ -107,17 +113,17 @@ namespace ColumnLynx::Net::TCP { } bool TCPClient::isHandshakeComplete() const { - return mHandshakeComplete; + return mHandshakeComplete.load(std::memory_order_relaxed); } bool TCPClient::isConnected() const { - return mConnected; + return mConnected.load(std::memory_order_relaxed); } void TCPClient::mStartHeartbeat() { auto self = shared_from_this(); mHeartbeatTimer.expires_after(std::chrono::seconds(5)); - mHeartbeatTimer.async_wait([this, self](const asio::error_code& ec) { + mHeartbeatTimer.async_wait([self](const asio::error_code& ec) { if (ec == asio::error::operation_aborted) { return; // Timer was cancelled } @@ -130,9 +136,11 @@ namespace ColumnLynx::Net::TCP { // Close sockets forcefully, server is dead asio::error_code ec; - mHandler->socket().shutdown(tcp::socket::shutdown_both, ec); - mHandler->socket().close(ec); - mConnected = false; + if (self->mHandler) { + self->mHandler->socket().shutdown(tcp::socket::shutdown_both, ec); + self->mHandler->socket().close(ec); + } + self->mConnected.store(false, std::memory_order_relaxed); ClientSession::getInstance().setAESKey({}); // Clear AES key with all zeros ClientSession::getInstance().setSessionID(0); @@ -261,7 +269,7 @@ namespace ColumnLynx::Net::TCP { mTun->configureIP(clientIP, serverIP, prefixLen, mtu); } - mHandshakeComplete = true; + mHandshakeComplete.store(true, std::memory_order_relaxed); } break; @@ -276,13 +284,13 @@ namespace ColumnLynx::Net::TCP { break; case ServerMessageType::GRACEFUL_DISCONNECT: Utils::log("Server is disconnecting: " + data); - if (mConnected) { // Prevent Recursion + if (mConnected.load(std::memory_order_relaxed)) { // Prevent Recursion disconnect(false); } break; case ServerMessageType::KILL_CONNECTION: Utils::warn("Server is killing the connection: " + data); - if (mConnected) { + if (mConnected.load(std::memory_order_relaxed)) { disconnect(false); } break; diff --git a/src/common/session_registry.cpp b/src/common/session_registry.cpp index ff3c065..7511081 100644 --- a/src/common/session_registry.cpp +++ b/src/common/session_registry.cpp @@ -85,11 +85,15 @@ namespace ColumnLynx::Net { void SessionRegistry::lockIP(uint32_t sessionID, uint32_t ip) { std::unique_lock lock(mMutex); mSessionIPs[sessionID] = ip; - - /*if (mIPSessions.find(sessionID) == mIPSessions.end()) { - Utils::debug("yikes"); - }*/ - mIPSessions[ip] = mSessions.find(sessionID)->second; + + auto it = mSessions.find(sessionID); + if (it == mSessions.end() || !it->second) { + Utils::warn("SessionRegistry::lockIP called for unknown session " + std::to_string(sessionID)); + mSessionIPs.erase(sessionID); + return; + } + + mIPSessions[ip] = it->second; } void SessionRegistry::deallocIP(uint32_t sessionID) { diff --git a/src/common/tcp_message_handler.cpp b/src/common/tcp_message_handler.cpp index f5a6b15..527d668 100644 --- a/src/common/tcp_message_handler.cpp +++ b/src/common/tcp_message_handler.cpp @@ -5,6 +5,7 @@ #include #include #include +#include namespace ColumnLynx::Net::TCP { void MessageHandler::start() { @@ -17,17 +18,17 @@ namespace ColumnLynx::Net::TCP { return static_cast(type); }, type); - std::vector data; - data.push_back(typeByte); + auto data = std::make_shared>(); + data->push_back(typeByte); uint16_t length = payload.size(); - data.push_back(length >> 8); - data.push_back(length & 0xFF); + data->push_back(length >> 8); + data->push_back(length & 0xFF); - data.insert(data.end(), payload.begin(), payload.end()); + data->insert(data->end(), payload.begin(), payload.end()); auto self = shared_from_this(); - asio::async_write(mSocket, asio::buffer(data), - [self](asio::error_code ec, std::size_t) { + asio::async_write(mSocket, asio::buffer(*data), + [self, data](asio::error_code ec, std::size_t) { if (ec) { Utils::error("Send failed: " + ec.message()); } diff --git a/src/common/utils.cpp b/src/common/utils.cpp index a2429fd..209e0f3 100644 --- a/src/common/utils.cpp +++ b/src/common/utils.cpp @@ -85,7 +85,7 @@ namespace ColumnLynx::Utils { } std::string getVersion() { - return "1.1.0"; + return "1.1.1"; } unsigned short serverPort() { diff --git a/src/server/main.cpp b/src/server/main.cpp index 35b8188..c1ebf0e 100644 --- a/src/server/main.cpp +++ b/src/server/main.cpp @@ -6,6 +6,7 @@ #include #include #include +#include #include #include #include @@ -172,9 +173,24 @@ int main(int argc, char** argv) { continue; } + if (packet.size() < 20) { + Utils::warn("TUN: Dropping packet smaller than IPv4 header (" + std::to_string(packet.size()) + " bytes)"); + continue; + } + const uint8_t* ip = packet.data(); - uint32_t srcIP = ntohl(*(uint32_t*)(ip + 12)); // IPv4 source address offset - uint32_t dstIP = ntohl(*(uint32_t*)(ip + 16)); // IPv4 destination address offset + uint8_t ipVersion = (ip[0] >> 4); + if (ipVersion != 4) { + Utils::debug("TUN: Non-IPv4 packet received (version=" + std::to_string(ipVersion) + "), skipping server IPv4 routing path."); + continue; + } + + uint32_t srcIPNet = 0; + uint32_t dstIPNet = 0; + std::memcpy(&srcIPNet, ip + 12, sizeof(srcIPNet)); // IPv4 source address offset + std::memcpy(&dstIPNet, ip + 16, sizeof(dstIPNet)); // IPv4 destination address offset + uint32_t srcIP = ntohl(srcIPNet); + uint32_t dstIP = ntohl(dstIPNet); // First, check if destination IP is a registered client (e.g., server responding to client or client-to-client) auto dstSession = SessionRegistry::getInstance().getByIP(dstIP); diff --git a/src/server/net/tcp/tcp_connection.cpp b/src/server/net/tcp/tcp_connection.cpp index d5792f1..0ab4982 100644 --- a/src/server/net/tcp/tcp_connection.cpp +++ b/src/server/net/tcp/tcp_connection.cpp @@ -14,23 +14,31 @@ namespace ColumnLynx::Net::TCP { Utils::warn("Failed to get remote endpoint: " + std::string(e.what())); } - mHandler->onMessage([this](AnyMessageType type, const std::string& data) { - mHandleMessage(static_cast(MessageHandler::toUint8(type)), data); + mHandler->onMessage([weakSelf = weak_from_this()](AnyMessageType type, const std::string& data) { + if (auto self = weakSelf.lock()) { + self->mHandleMessage(static_cast(MessageHandler::toUint8(type)), data); + } }); - mHandler->onDisconnect([this](const asio::error_code& ec) { + mHandler->onDisconnect([weakSelf = weak_from_this()](const asio::error_code& ec) { + auto self = weakSelf.lock(); + if (!self) { + return; + } // Peer has closed; finalize locally without sending RST - Utils::log("Client disconnected: " + mRemoteIP + " - " + ec.message()); + Utils::log("Client disconnected: " + self->mRemoteIP + " - " + ec.message()); asio::error_code ec2; - mHandler->socket().close(ec2); + if (self->mHandler) { + self->mHandler->socket().close(ec2); + } - SessionRegistry::getInstance().erase(mConnectionSessionID); - SessionRegistry::getInstance().deallocIP(mConnectionSessionID); + SessionRegistry::getInstance().erase(self->mConnectionSessionID); + SessionRegistry::getInstance().deallocIP(self->mConnectionSessionID); - Utils::log("Closed connection to " + mRemoteIP); + Utils::log("Closed connection to " + self->mRemoteIP); - if (mOnDisconnect) { - mOnDisconnect(shared_from_this()); + if (self->mOnDisconnect) { + self->mOnDisconnect(self); } }); @@ -77,7 +85,7 @@ namespace ColumnLynx::Net::TCP { void TCPConnection::mStartHeartbeat() { auto self = shared_from_this(); mHeartbeatTimer.expires_after(std::chrono::seconds(5)); - mHeartbeatTimer.async_wait([this, self](const asio::error_code& ec) { + mHeartbeatTimer.async_wait([self](const asio::error_code& ec) { if (ec == asio::error::operation_aborted) { return; // Timer was cancelled } @@ -90,10 +98,13 @@ namespace ColumnLynx::Net::TCP { // Remove socket forcefully, client is dead asio::error_code ec; - mHandler->socket().shutdown(asio::ip::tcp::socket::shutdown_both, ec); - mHandler->socket().close(ec); + if (self->mHandler) { + self->mHandler->socket().shutdown(asio::ip::tcp::socket::shutdown_both, ec); + self->mHandler->socket().close(ec); + } SessionRegistry::getInstance().erase(self->mConnectionSessionID); + SessionRegistry::getInstance().deallocIP(self->mConnectionSessionID); return; } From b64d9c4498866f78fa0719ce1afceaecccd6b1c9 Mon Sep 17 00:00:00 2001 From: DcruBro Date: Mon, 25 May 2026 12:19:24 +0200 Subject: [PATCH 2/6] High priority and critical issues --- .gitignore | 1 + include/columnlynx/common/utils.hpp | 2 +- src/client/main.cpp | 16 +++- src/client/net/tcp/tcp_client.cpp | 16 +++- src/client/net/udp/udp_client.cpp | 21 ++++- src/common/session_registry.cpp | 21 ++++- src/common/tcp_message_handler.cpp | 12 ++- src/common/utils.cpp | 62 +++++++++++++-- src/common/virtual_interface.cpp | 108 ++++++++++++-------------- src/server/main.cpp | 18 +++++ src/server/net/tcp/tcp_connection.cpp | 16 ++-- 11 files changed, 213 insertions(+), 80 deletions(-) diff --git a/.gitignore b/.gitignore index 290af9a..92448fb 100644 --- a/.gitignore +++ b/.gitignore @@ -12,5 +12,6 @@ _deps CMakeUserPresets.json build/ +build*/ .vscode/ .DS_Store diff --git a/include/columnlynx/common/utils.hpp b/include/columnlynx/common/utils.hpp index 51d6f69..5ad2605 100644 --- a/include/columnlynx/common/utils.hpp +++ b/include/columnlynx/common/utils.hpp @@ -15,7 +15,7 @@ #include #include #include -#include + #ifdef _WIN32 #include diff --git a/src/client/main.cpp b/src/client/main.cpp index f29724d..be27d56 100644 --- a/src/client/main.cpp +++ b/src/client/main.cpp @@ -5,6 +5,7 @@ #include #include #include +#include #include #include #include @@ -90,7 +91,20 @@ int main(int argc, char** argv) { std::string configPath = optionsObj["config-dir"].as(); const char* envConfigPath = std::getenv("COLUMNLYNX_CONFIG_DIR"); if (envConfigPath != nullptr) { - configPath = std::string(envConfigPath); + // Validate and canonicalize environment-provided path + try { + namespace fs = std::filesystem; + std::error_code ec; + fs::path candidate(envConfigPath); + fs::path abs = fs::absolute(candidate, ec); + if (!ec) { + configPath = abs.string(); + } else { + warn(std::string("Invalid COLUMNLYNX_CONFIG_DIR value: ") + envConfigPath + " - using default"); + } + } catch (const std::exception& e) { + warn(std::string("Failed to canonicalize COLUMNLYNX_CONFIG_DIR: ") + e.what()); + } } if (configPath.back() != '/' && configPath.back() != '\\') { diff --git a/src/client/net/tcp/tcp_client.cpp b/src/client/net/tcp/tcp_client.cpp index aac19c8..9766d86 100644 --- a/src/client/net/tcp/tcp_client.cpp +++ b/src/client/net/tcp/tcp_client.cpp @@ -159,7 +159,13 @@ namespace ColumnLynx::Net::TCP { void TCPClient::mHandleMessage(ServerMessageType type, const std::string& data) { switch (type) { case ServerMessageType::HANDSHAKE_IDENTIFY: { - std::memcpy(mServerPublicKey, data.data(), std::min(data.size(), sizeof(mServerPublicKey))); + if (data.size() != sizeof(mServerPublicKey)) { + Utils::warn("HANDSHAKE_IDENTIFY has invalid size: " + std::to_string(data.size())); + disconnect(); + return; + } + + std::memcpy(mServerPublicKey, data.data(), sizeof(mServerPublicKey)); std::string hexServerPub = Utils::bytesToHexString(mServerPublicKey, 32); Utils::log("Received server identity. Public Key: " + hexServerPub); @@ -188,7 +194,13 @@ namespace ColumnLynx::Net::TCP { { // Verify the signature Signature sig{}; - std::memcpy(sig.data(), data.data(), std::min(data.size(), sig.size())); + if (data.size() != sig.size()) { + Utils::warn("HANDSHAKE_CHALLENGE_RESPONSE has invalid size: " + std::to_string(data.size())); + disconnect(); + return; + } + + std::memcpy(sig.data(), data.data(), sig.size()); if (Utils::LibSodiumWrapper::verifyMessage(mSubmittedChallenge.data(), mSubmittedChallenge.size(), sig, mServerPublicKey)) { Utils::log("Challenge response verified successfully."); diff --git a/src/client/net/udp/udp_client.cpp b/src/client/net/udp/udp_client.cpp index 9c12c8a..dc755f7 100644 --- a/src/client/net/udp/udp_client.cpp +++ b/src/client/net/udp/udp_client.cpp @@ -73,7 +73,7 @@ namespace ColumnLynx::Net::UDP { reinterpret_cast(&hdr) + sizeof(UDPPacketHeader) ); uint32_t sessionID = static_cast(ClientSession::getInstance().getSessionID()); - uint32_t sessionIDNet = sessionID; + uint32_t sessionIDNet = htonl(sessionID); packet.insert(packet.end(), reinterpret_cast(&sessionIDNet), reinterpret_cast(&sessionIDNet) + sizeof(uint32_t) @@ -136,9 +136,24 @@ namespace ColumnLynx::Net::UDP { } // Decrypt payload + // Extract ciphertext safely + size_t headerLen = sizeof(UDPPacketHeader) + sizeof(uint32_t); + if (bytes < headerLen) { + Utils::warn("UDP Client received packet too small after header check."); + return; + } + + size_t ciphertextLen = bytes - headerLen; + // Enforce reasonable maximum (UDP payload practical limit) + const size_t MAX_UDP_PAYLOAD = 65507; // 65535 - UDP/IP headers + if (ciphertextLen > MAX_UDP_PAYLOAD) { + Utils::warn("UDP Client received packet with excessive payload size: " + std::to_string(ciphertextLen)); + return; + } + std::vector ciphertext( - mRecvBuffer.begin() + sizeof(UDPPacketHeader) + sizeof(uint32_t), - mRecvBuffer.begin() + bytes + mRecvBuffer.begin() + headerLen, + mRecvBuffer.begin() + headerLen + ciphertextLen ); if (ClientSession::getInstance().getAESKey().empty()) { diff --git a/src/common/session_registry.cpp b/src/common/session_registry.cpp index 7511081..6df8019 100644 --- a/src/common/session_registry.cpp +++ b/src/common/session_registry.cpp @@ -32,7 +32,26 @@ namespace ColumnLynx::Net { void SessionRegistry::erase(uint32_t sessionID) { std::unique_lock lock(mMutex); - mSessions.erase(sessionID); + auto it = mSessions.find(sessionID); + if (it != mSessions.end()) { + // If the session has a client IP mapping, remove it to avoid stale entries + if (it->second) { + uint32_t ip = it->second->clientTunIP; + auto ipIt = mIPSessions.find(ip); + if (ipIt != mIPSessions.end()) { + // Only erase if it points to the same session + if (ipIt->second == it->second) { + mIPSessions.erase(ipIt); + } + } + } + + // Remove any session->ip bookkeeping + mSessionIPs.erase(sessionID); + + // Finally erase the session + mSessions.erase(it); + } } void SessionRegistry::cleanupExpired() { diff --git a/src/common/tcp_message_handler.cpp b/src/common/tcp_message_handler.cpp index 527d668..66e2fe3 100644 --- a/src/common/tcp_message_handler.cpp +++ b/src/common/tcp_message_handler.cpp @@ -20,10 +20,16 @@ namespace ColumnLynx::Net::TCP { auto data = std::make_shared>(); data->push_back(typeByte); - uint16_t length = payload.size(); + // Ensure payload fits into protocol's 16-bit length field + if (payload.size() > static_cast(std::numeric_limits::max())) { + Utils::error("sendMessage(): payload too large (>65535 bytes)"); + return; + } - data->push_back(length >> 8); - data->push_back(length & 0xFF); + uint16_t length = static_cast(payload.size()); + + data->push_back(static_cast(length >> 8)); + data->push_back(static_cast(length & 0xFF)); data->insert(data->end(), payload.begin(), payload.end()); auto self = shared_from_this(); diff --git a/src/common/utils.cpp b/src/common/utils.cpp index 209e0f3..9ba8d40 100644 --- a/src/common/utils.cpp +++ b/src/common/utils.cpp @@ -3,6 +3,7 @@ // Distributed under the terms of the GNU General Public License, either version 2 only or version 3. See LICENSES/ for details. #include +#include namespace ColumnLynx::Utils { std::string unixMillisToISO8601(uint64_t unixMillis, bool local) { @@ -144,19 +145,49 @@ namespace ColumnLynx::Utils { std::vector out; - std::ifstream file(basePath + "whitelisted_keys"); + namespace fs = std::filesystem; + std::error_code ec; + + fs::path base(basePath); + fs::path absBase = fs::absolute(base, ec); + if (ec) { + warn("getWhitelistedKeys(): failed to resolve base path: " + basePath + " - " + ec.message()); + return out; + } + + fs::path whitelist = absBase / "whitelisted_keys"; + if (!fs::exists(whitelist, ec) || ec) { + warn("getWhitelistedKeys(): whitelist file not found: " + whitelist.string()); + return out; + } + + // Canonicalize to avoid symlink tricks + fs::path canon = fs::canonical(whitelist, ec); + if (ec) { + warn("getWhitelistedKeys(): failed to canonicalize path: " + whitelist.string()); + return out; + } + + std::ifstream file(canon); if (!file.is_open()) { - warn("Failed to open whitelisted_keys file at path: " + basePath + "whitelisted_keys"); + warn("getWhitelistedKeys(): failed to open whitelist file: " + canon.string()); return out; } std::string line; while (std::getline(file, line)) { + // Trim whitespace + while (!line.empty() && isspace(static_cast(line.back()))) line.pop_back(); + size_t start = 0; + while (start < line.size() && isspace(static_cast(line[start]))) ++start; + if (start >= line.size()) continue; + std::string key = line.substr(start); + // Convert to upper case to align with the bytesToHexString() output - for (int i = 0; i < line.length(); i++) { - line[i] = toupper(line[i]); + for (size_t i = 0; i < key.length(); ++i) { + key[i] = static_cast(toupper(static_cast(key[i]))); } - out.push_back(line); + out.push_back(key); } return out; @@ -166,9 +197,26 @@ namespace ColumnLynx::Utils { // TODO: Currently re-reads every time. std::vector readLines; - std::ifstream file(path); + namespace fs = std::filesystem; + std::error_code ec; + fs::path p(path); + fs::path abs = fs::absolute(p, ec); + if (ec) { + throw std::runtime_error("getConfigMap(): failed to resolve path: " + path + " - " + ec.message()); + } + + if (!fs::exists(abs, ec) || ec) { + throw std::runtime_error("getConfigMap(): config file does not exist: " + abs.string()); + } + + fs::path canon = fs::canonical(abs, ec); + if (ec) { + throw std::runtime_error("getConfigMap(): failed to canonicalize config path: " + abs.string()); + } + + std::ifstream file(canon); if (!file.is_open()) { - throw std::runtime_error("Failed to open config file at path: " + path); + throw std::runtime_error("Failed to open config file at path: " + canon.string()); } std::string line; diff --git a/src/common/virtual_interface.cpp b/src/common/virtual_interface.cpp index 1807eec..c18b2dd 100644 --- a/src/common/virtual_interface.cpp +++ b/src/common/virtual_interface.cpp @@ -4,6 +4,11 @@ #include +#include +#include + +extern char **environ; + // This is all fucking voodoo dark magic. #if defined(_WIN32) @@ -56,6 +61,33 @@ static void InitializeWintun() #endif // _WIN32 namespace ColumnLynx::Net { + +// Run a command without invoking a shell. Arguments are passed directly +// to the underlying process to avoid shell injection vulnerabilities. +static bool runCommand(const std::vector& args) { + if (args.empty()) return false; + + std::vector argv; + argv.reserve(args.size() + 1); + for (const auto &s : args) { + argv.push_back(const_cast(s.c_str())); + } + argv.push_back(nullptr); + + pid_t pid; + int rc = posix_spawnp(&pid, argv[0], nullptr, nullptr, argv.data(), environ); + if (rc != 0) { + return false; + } + + int status = 0; + if (waitpid(pid, &status, 0) == -1) { + return false; + } + + return WIFEXITED(status) && WEXITSTATUS(status) == 0; +} + // ------------------------------ Constructor ------------------------------ VirtualInterface::VirtualInterface(const std::string& ifName) : mIfName(ifName), mFd(-1) @@ -307,25 +339,10 @@ namespace ColumnLynx::Net { void VirtualInterface::resetIP() { #if defined(__linux__) - char cmd[512]; - snprintf(cmd, sizeof(cmd), - "ip addr flush dev %s", - mIfName.c_str() - ); - system(cmd); + runCommand({"ip", "addr", "flush", "dev", mIfName}); #elif defined(__APPLE__) - char cmd[512]; - snprintf(cmd, sizeof(cmd), - "ifconfig %s inet 0.0.0.0 delete", - mIfName.c_str() - ); - system(cmd); - - snprintf(cmd, sizeof(cmd), - "ifconfig %s inet6 :: delete", - mIfName.c_str() - ); - system(cmd); + runCommand({"ifconfig", mIfName, "inet", "0.0.0.0", "delete"}); + runCommand({"ifconfig", mIfName, "inet6", "::", "delete"}); // Wipe old routes //snprintf(cmd, sizeof(cmd), @@ -357,26 +374,19 @@ namespace ColumnLynx::Net { bool VirtualInterface::mApplyLinuxIP(uint32_t clientIP, uint32_t serverIP, uint8_t prefixLen, uint16_t mtu) { - char cmd[512]; std::string ipStr = ipv4ToString(clientIP); std::string peerStr = ipv4ToString(serverIP); // Wipe the current config - snprintf(cmd, sizeof(cmd), - "ip addr flush dev %s", - mIfName.c_str() - ); - system(cmd); + runCommand({"ip", "addr", "flush", "dev", mIfName}); - snprintf(cmd, sizeof(cmd), - "ip addr add %s/%d peer %s dev %s", - ipStr.c_str(), prefixLen, peerStr.c_str(), mIfName.c_str()); - system(cmd); - - snprintf(cmd, sizeof(cmd), - "ip link set dev %s up mtu %d", mIfName.c_str(), mtu); - system(cmd); + // Add address with peer + std::string addrArg = ipStr + "/" + std::to_string(prefixLen); + runCommand({"ip", "addr", "add", addrArg, "peer", peerStr, "dev", mIfName}); + + // Bring link up and set MTU + runCommand({"ip", "link", "set", "dev", mIfName, "up", "mtu", std::to_string(mtu)}); return true; } @@ -387,39 +397,23 @@ namespace ColumnLynx::Net { bool VirtualInterface::mApplyMacOSIP(uint32_t clientIP, uint32_t serverIP, uint8_t prefixLen, uint16_t mtu) { - char cmd[512]; std::string ipStr = ipv4ToString(clientIP); std::string peerStr = ipv4ToString(serverIP); std::string prefixStr = ipv4ToString(prefixLengthToNetmask(prefixLen), false); Utils::debug("Prefix string: " + prefixStr); - // Reset - snprintf(cmd, sizeof(cmd), - "ifconfig %s inet 0.0.0.0 delete", - mIfName.c_str() - ); - system(cmd); + // Reset IPv4 and IPv6 addresses + runCommand({"ifconfig", mIfName, "inet", "0.0.0.0", "delete"}); + runCommand({"ifconfig", mIfName, "inet6", "::", "delete"}); - snprintf(cmd, sizeof(cmd), - "ifconfig %s inet6 :: delete", - mIfName.c_str() - ); - system(cmd); + // Set address and netmask + std::string netArg = ipStr + " " + peerStr; // ifconfig expects ip peer + runCommand({"ifconfig", mIfName, "inet", ipStr, peerStr, "mtu", std::to_string(mtu), "netmask", prefixStr, "up"}); - // Set - snprintf(cmd, sizeof(cmd), - "ifconfig %s inet %s %s mtu %d netmask %s up", - mIfName.c_str(), ipStr.c_str(), peerStr.c_str(), mtu, prefixStr.c_str()); - system(cmd); - - // Host bits are auto-normalized by the kernel on macOS, so we don't need to worry about them not being zeroed out. - snprintf(cmd, sizeof(cmd), - "route -n add -net %s/%d -interface %s", - ipStr.c_str(), prefixLen, mIfName.c_str()); - system(cmd); - - Utils::log("Executed command: " + std::string(cmd)); + // Add route for the network + std::string networkArg = ipStr + "/" + std::to_string(prefixLen); + runCommand({"route", "-n", "add", "-net", networkArg, "-interface", mIfName}); return true; } diff --git a/src/server/main.cpp b/src/server/main.cpp index c1ebf0e..441546c 100644 --- a/src/server/main.cpp +++ b/src/server/main.cpp @@ -145,6 +145,22 @@ int main(int argc, char** argv) { auto server = std::make_shared(io, serverPort()); auto udpServer = std::make_shared(io, serverPort()); + // Schedule periodic cleanup of expired sessions every 5 minutes + auto cleanupTimer = std::make_shared(io); + auto cleanupHandler = std::make_shared>(); + *cleanupHandler = [cleanupTimer, cleanupHandler](const asio::error_code& ec) { + if (ec == asio::error::operation_aborted) return; // Timer cancelled + try { + SessionRegistry::getInstance().cleanupExpired(); + } catch (const std::exception& e) { + Utils::warn(std::string("SessionRegistry::cleanupExpired() threw: ") + e.what()); + } + cleanupTimer->expires_after(std::chrono::minutes(5)); + cleanupTimer->async_wait(*cleanupHandler); + }; + cleanupTimer->expires_after(std::chrono::minutes(5)); + cleanupTimer->async_wait(*cleanupHandler); + asio::signal_set signals(io, SIGINT, SIGTERM); signals.async_wait([&](const std::error_code&, int) { log("Received termination signal. Shutting down server gracefully."); @@ -153,6 +169,8 @@ int main(int argc, char** argv) { ServerSession::getInstance().setHostRunning(false); server->stop(); udpServer->stop(); + // Cancel cleanup timer + cleanupTimer->cancel(); }); }); diff --git a/src/server/net/tcp/tcp_connection.cpp b/src/server/net/tcp/tcp_connection.cpp index 0ab4982..bcfd37d 100644 --- a/src/server/net/tcp/tcp_connection.cpp +++ b/src/server/net/tcp/tcp_connection.cpp @@ -124,8 +124,8 @@ namespace ColumnLynx::Net::TCP { case ClientMessageType::HANDSHAKE_INIT: { Utils::log("Received HANDSHAKE_INIT from " + reqAddr); - if (data.size() < 1 + crypto_box_PUBLICKEYBYTES) { - Utils::warn("HANDSHAKE_INIT from " + reqAddr + " is too short."); + if (data.size() != 1 + crypto_sign_PUBLICKEYBYTES) { + Utils::warn("HANDSHAKE_INIT from " + reqAddr + " has invalid size: " + std::to_string(data.size())); disconnect(); return; } @@ -141,7 +141,7 @@ namespace ColumnLynx::Net::TCP { Utils::log("Client protocol version " + std::to_string(clientProtoVer) + " accepted from " + reqAddr + "."); PublicKey signPk; - std::memcpy(signPk.data(), data.data() + 1, std::min(data.size() - 1, sizeof(signPk))); + std::memcpy(signPk.data(), data.data() + 1, sizeof(signPk)); // We can safely store this without further checking, the client will need to send the encrypted AES key in a way where they must possess the corresponding private key anyways. int r = crypto_sign_ed25519_pk_to_curve25519(mConnectionPublicKey.data(), signPk.data()); // Store the client's public encryption key key (for identification) @@ -173,9 +173,15 @@ namespace ColumnLynx::Net::TCP { case ClientMessageType::HANDSHAKE_CHALLENGE: { Utils::log("Received HANDSHAKE_CHALLENGE from " + reqAddr); - // Convert to byte array + // Convert to byte array - require exact size + if (data.size() != 32) { + Utils::warn("HANDSHAKE_CHALLENGE has invalid size: " + std::to_string(data.size())); + disconnect(); + return; + } + uint8_t challengeData[32]; - std::memcpy(challengeData, data.data(), std::min(data.size(), sizeof(challengeData))); + std::memcpy(challengeData, data.data(), sizeof(challengeData)); // Sign the challenge Signature sig = Utils::LibSodiumWrapper::signMessage( From 60795c60d8f4b93007e0ddd6c08b2c8ca1b73106 Mon Sep 17 00:00:00 2001 From: DcruBro Date: Mon, 25 May 2026 12:22:33 +0200 Subject: [PATCH 3/6] minor fixes relating to nonce overflows, size checks, etc. --- include/columnlynx/client/client_session.hpp | 4 ++++ src/client/net/udp/udp_client.cpp | 9 ++++++++- src/common/virtual_interface.cpp | 4 ++-- src/server/main.cpp | 9 +++++++-- src/server/net/udp/udp_server.cpp | 18 +++++++++++++++++- 5 files changed, 38 insertions(+), 6 deletions(-) diff --git a/include/columnlynx/client/client_session.hpp b/include/columnlynx/client/client_session.hpp index 1cb3188..bd4db0f 100644 --- a/include/columnlynx/client/client_session.hpp +++ b/include/columnlynx/client/client_session.hpp @@ -87,6 +87,10 @@ namespace ColumnLynx { void incrementSendCount() { std::unique_lock lock(mMutex); + if (mClientState->send_cnt == std::numeric_limits::max()) { + Utils::error("ClientSession: send counter overflow detected"); + return; + } mClientState->send_cnt++; } diff --git a/src/client/net/udp/udp_client.cpp b/src/client/net/udp/udp_client.cpp index dc755f7..fb55162 100644 --- a/src/client/net/udp/udp_client.cpp +++ b/src/client/net/udp/udp_client.cpp @@ -3,6 +3,8 @@ // Distributed under the terms of the GNU General Public License, either version 2 only or version 3. See LICENSES/ for details. #include +#include +#include namespace ColumnLynx::Net::UDP { void UDPClient::start() { @@ -102,7 +104,12 @@ namespace ColumnLynx::Net::UDP { if (ec) { if (ec == asio::error::operation_aborted) return; // Socket closed // Other recv error - mStartReceive(); + Utils::warn("UDPClient receive error: " + ec.message()); + // Back off briefly before restarting receive to avoid busy error loops + asio::post(mSocket.get_executor(), [this]() { + std::this_thread::sleep_for(std::chrono::milliseconds(50)); + mStartReceive(); + }); return; } diff --git a/src/common/virtual_interface.cpp b/src/common/virtual_interface.cpp index c18b2dd..6fb2ad7 100644 --- a/src/common/virtual_interface.cpp +++ b/src/common/virtual_interface.cpp @@ -213,8 +213,8 @@ static bool runCommand(const std::vector& args) { pfd.fd = mFd; pfd.events = POLLIN; - // timeout in ms; keep it small so shutdown is responsive - int ret = poll(&pfd, 1, 200); + // timeout in ms; keep it small so shutdown is responsive. Reduced for lower latency. + int ret = poll(&pfd, 1, 50); if (ret == 0) { // No data yet diff --git a/src/server/main.cpp b/src/server/main.cpp index 441546c..4f20de6 100644 --- a/src/server/main.cpp +++ b/src/server/main.cpp @@ -213,8 +213,13 @@ int main(int argc, char** argv) { // First, check if destination IP is a registered client (e.g., server responding to client or client-to-client) auto dstSession = SessionRegistry::getInstance().getByIP(dstIP); if (dstSession) { - // Destination is a registered client, forward to that client's session - udpServer->sendData(dstSession->sessionID, std::string(packet.begin(), packet.end())); + // Destination is a registered client, enforce MTU and forward to that client's session + const size_t MTU = 1420; // Enforce configured MTU; TODO: read from server config + if (packet.size() > MTU) { + Utils::warn("TUN: Dropping oversized packet (" + std::to_string(packet.size()) + " > MTU " + std::to_string(MTU) + ")"); + } else { + udpServer->sendData(dstSession->sessionID, std::string(packet.begin(), packet.end())); + } continue; } diff --git a/src/server/net/udp/udp_server.cpp b/src/server/net/udp/udp_server.cpp index d99ee1e..e786324 100644 --- a/src/server/net/udp/udp_server.cpp +++ b/src/server/net/udp/udp_server.cpp @@ -95,7 +95,23 @@ namespace ColumnLynx::Net::UDP { UDPPacketHeader hdr{}; uint8_t nonce[12]; uint32_t prefix = session->noncePrefix; - uint64_t sendCount = const_cast(session.get())->send_ctr.fetch_add(1, std::memory_order_relaxed); + // Increment send counter with overflow protection + uint64_t sendCount = 0; + { + auto ptr = const_cast(session.get()); + uint64_t old = ptr->send_ctr.load(std::memory_order_relaxed); + for (;;) { + if (old == std::numeric_limits::max()) { + Utils::error("UDP: send counter overflow for session " + std::to_string(sessionID)); + return; + } + if (ptr->send_ctr.compare_exchange_weak(old, old + 1, std::memory_order_relaxed)) { + sendCount = old; + break; + } + // old updated by compare_exchange_weak, loop + } + } memcpy(nonce, &prefix, sizeof(uint32_t)); // Prefix nonce memcpy(nonce + sizeof(uint32_t), &sendCount, sizeof(uint64_t)); // Use send count as nonce suffix to ensure uniqueness std::copy_n(nonce, 12, hdr.nonce.data()); From afe10bbb6eeb16001cde9bea320393fc1cd0bd61 Mon Sep 17 00:00:00 2001 From: DcruBro Date: Mon, 25 May 2026 12:29:19 +0200 Subject: [PATCH 4/6] Context fill-in and CI tests This commit adds common units tests and CI sanitasion. Additional context for commit b64d9c4498866f78fa0719ce1afceaecccd6b1c9: - Fixed macOS/Linux non-portable and unsafe shell usage by adding a posix_spawn helper and replacing system() calls in virtual_interface.cpp. - Fixed SessionRegistry::erase() to remove mIPSessions and mSessionIPs entries in session_registry.cpp. - Prevented message-length truncation in tcp_message_handler.cpp by rejecting payloads > 65535 bytes. - Validated handshake message sizes and removed silent truncation in: - tcp_connection.cpp - tcp_client.cpp - Canonicalized and validated config and whitelist paths in utils.cpp using std::filesystem. - Hardened environment-provided config path handling in main.cpp. - Validated UDP ciphertext lengths and fixed session ID endianness in udp_client.cpp. - Scheduled periodic SessionRegistry::cleanupExpired() in main.cpp (every 5 minutes). --- .github/workflows/sanitizers.yml | 39 ++++++++++++++++++ CMakeLists.txt | 25 ++++++++++++ tests/test_libsodium_wrapper.cpp | 42 +++++++++++++++++++ tests/test_session_registry.cpp | 49 +++++++++++++++++++++++ tests/test_session_registry_ip.cpp | 43 ++++++++++++++++++++ tests/test_tcp_message_handler_static.cpp | 30 ++++++++++++++ 6 files changed, 228 insertions(+) create mode 100644 .github/workflows/sanitizers.yml create mode 100644 tests/test_libsodium_wrapper.cpp create mode 100644 tests/test_session_registry.cpp create mode 100644 tests/test_session_registry_ip.cpp create mode 100644 tests/test_tcp_message_handler_static.cpp diff --git a/.github/workflows/sanitizers.yml b/.github/workflows/sanitizers.yml new file mode 100644 index 0000000..589b5e5 --- /dev/null +++ b/.github/workflows/sanitizers.yml @@ -0,0 +1,39 @@ +name: Sanitizers + +on: + push: + branches: [ main, master ] + pull_request: + branches: [ main, master ] + +jobs: + build-and-test: + runs-on: ubuntu-latest + env: + SANITIZERS: "-fsanitize=address,undefined -fno-omit-frame-pointer -g -O1" + ASAN_OPTIONS: "detect_leaks=1:abort_on_error=1" + UBSAN_OPTIONS: "print_stacktrace=1" + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: Install dependencies + run: | + sudo apt-get update + sudo apt-get install -y cmake build-essential clang + + - name: Configure (CMake) + run: | + mkdir -p build-sanitizers + cd build-sanitizers + CC=clang CXX=clang++ cmake -DCMAKE_BUILD_TYPE=Debug -DCMAKE_CXX_FLAGS="$SANITIZERS" -DCMAKE_EXE_LINKER_FLAGS="$SANITIZERS" .. + + - name: Build + run: | + cd build-sanitizers + cmake --build . -- -j + + - name: Run tests + run: | + cd build-sanitizers + ctest --output-on-failure || (echo "ctest failed"; exit 1) diff --git a/CMakeLists.txt b/CMakeLists.txt index 6007c54..416267e 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -173,3 +173,28 @@ install(FILES LICENSES/GPL-2.0-only.txt LICENSES/GPL-3.0.txt DESTINATION ${CMAKE_INSTALL_DATAROOTDIR}/licenses/${PROJECT_NAME}) + +# --------------------------------------------------------- +# Unit tests +# --------------------------------------------------------- +option(BUILD_TESTS "Build unit tests" ON) + +if(BUILD_TESTS) + enable_testing() + file(GLOB_RECURSE TEST_SRC CONFIGURE_DEPENDS tests/*.cpp) + if(TEST_SRC) + foreach(TEST_FILE IN LISTS TEST_SRC) + get_filename_component(TEST_NAME ${TEST_FILE} NAME_WE) + add_executable(${TEST_NAME} ${TEST_FILE}) + target_link_libraries(${TEST_NAME} PRIVATE common sodium) + target_include_directories(${TEST_NAME} PRIVATE + ${PROJECT_SOURCE_DIR}/include + ${asio_SOURCE_DIR}/asio/include + ${sodium_SOURCE_DIR}/src/libsodium/include + ${sodium_BINARY_DIR}/src/libsodium/include + ) + add_test(NAME ${TEST_NAME} COMMAND ${TEST_NAME}) + set_target_properties(${TEST_NAME} PROPERTIES OUTPUT_NAME "columnlynx_${TEST_NAME}") + endforeach() + endif() +endif() diff --git a/tests/test_libsodium_wrapper.cpp b/tests/test_libsodium_wrapper.cpp new file mode 100644 index 0000000..944f127 --- /dev/null +++ b/tests/test_libsodium_wrapper.cpp @@ -0,0 +1,42 @@ +// Tests for LibSodiumWrapper: random, symmetric encrypt/decrypt, sign/verify +#include +#include + +#include + +int main() { + using namespace ColumnLynx::Utils; + + // Random bytes uniqueness + auto a = LibSodiumWrapper::generateRandom256Bit(); + auto b = LibSodiumWrapper::generateRandom256Bit(); + assert(a != b && "generateRandom256Bit() should produce different outputs (very likely)"); + + // Symmetric encrypt/decrypt roundtrip + ColumnLynx::SymmetricKey key = {}; + for (size_t i = 0; i < key.size(); ++i) key[i] = static_cast(i); + auto nonce = LibSodiumWrapper::generateNonce(); + + std::string plaintext = "The quick brown fox jumps over the lazy dog"; + auto ct = LibSodiumWrapper::encryptMessage(reinterpret_cast(plaintext.data()), plaintext.size(), key, nonce, "aad"); + auto pt = LibSodiumWrapper::decryptMessage(ct.data(), ct.size(), key, nonce, "aad"); + std::string recovered(pt.begin(), pt.end()); + assert(recovered == plaintext && "decrypt should recover original plaintext"); + + // Sign and verify + ColumnLynx::PrivateKey sk{}; ColumnLynx::PublicKey pk{}; + randombytes_buf(sk.data(), sk.size()); + // naive keypair generation for test purposes: use libsodium functions via wrapper + // generate a real keypair using crypto_sign + if (crypto_sign_keypair(pk.data(), sk.data()) != 0) { + std::cerr << "Failed to generate keypair\n"; + return 2; + } + + auto sig = LibSodiumWrapper::signMessage(plaintext, sk); + bool ok = LibSodiumWrapper::verifyMessage(plaintext, sig, pk); + assert(ok && "Signature should verify"); + + std::cout << "LibSodiumWrapper tests passed\n"; + return 0; +} diff --git a/tests/test_session_registry.cpp b/tests/test_session_registry.cpp new file mode 100644 index 0000000..2d8bcf2 --- /dev/null +++ b/tests/test_session_registry.cpp @@ -0,0 +1,49 @@ +// Simple unit tests for SessionRegistry +#include +#include +#include + +#include +#include + +int main() { + using namespace ColumnLynx::Net; + using namespace ColumnLynx::Utils; + + auto ® = SessionRegistry::getInstance(); + + // Use a unique session id to avoid colliding with any running instance + const uint32_t sid = 0xDEADBEEF; + // ensure clean state + reg.erase(sid); + + auto key = LibSodiumWrapper::generateRandom256Bit(); + auto state = std::make_shared(key, std::chrono::hours(24), 0xC0A80101 /*192.168.1.1*/, 0, sid); + + reg.put(sid, state); + + assert(reg.exists(sid) && "Session should exist after put()"); + + auto got = reg.get(sid); + assert(got && got->sessionID == sid && "get() should return stored session"); + + auto byip = reg.getByIP(0xC0A80101); + assert(byip && byip->sessionID == sid && "getByIP() should find session by client IP"); + + // Erase and verify removed + reg.erase(sid); + assert(!reg.exists(sid) && "Session should not exist after erase()"); + assert(reg.getByIP(0xC0A80101) == nullptr && "getByIP() should return nullptr after erase"); + + // Test cleanupExpired: insert an already-expired session + const uint32_t sid2 = 0xFEEDBEEF; + reg.erase(sid2); + auto expiredState = std::make_shared(key, std::chrono::seconds(0), 0xC0A80102, 0, sid2); + reg.put(sid2, expiredState); + // Force cleanup + reg.cleanupExpired(); + assert(!reg.exists(sid2) && "Expired session should be removed by cleanupExpired()"); + + std::cout << "SessionRegistry tests passed\n"; + return 0; +} diff --git a/tests/test_session_registry_ip.cpp b/tests/test_session_registry_ip.cpp new file mode 100644 index 0000000..83f8ed8 --- /dev/null +++ b/tests/test_session_registry_ip.cpp @@ -0,0 +1,43 @@ +// Tests for SessionRegistry IP allocation and lock/dealloc +#include +#include + +#include +#include + +int main() { + using namespace ColumnLynx::Net; + using namespace ColumnLynx::Utils; + + auto ® = SessionRegistry::getInstance(); + + const uint32_t sid = 0xABCDEF01; + reg.erase(sid); + + auto key = LibSodiumWrapper::generateRandom256Bit(); + auto state = std::make_shared(key, std::chrono::hours(24), 0, 0, sid); + reg.put(sid, state); + + // Lock IP + uint32_t ip = 0xC0A80201; // 192.168.2.1 + reg.lockIP(sid, ip); + + auto byip = reg.getByIP(ip); + assert(byip && byip->sessionID == sid && "lockIP should populate mIPSessions"); + + // deallocIP + reg.deallocIP(sid); + assert(reg.getByIP(ip) == nullptr && "deallocIP should remove mapping"); + + // getFirstAvailableIP: choose a small /30 range to limit hosts + uint32_t base = 0x0A000000; // 10.0.0.0 + uint8_t mask = 30; // 2 usable hosts + uint32_t first = reg.getFirstAvailableIP(base, mask); + assert(first != 0 && "Should find available IP in empty registry"); + + // cleanup + reg.erase(sid); + + std::cout << "SessionRegistry IP tests passed\n"; + return 0; +} diff --git a/tests/test_tcp_message_handler_static.cpp b/tests/test_tcp_message_handler_static.cpp new file mode 100644 index 0000000..5f5c08e --- /dev/null +++ b/tests/test_tcp_message_handler_static.cpp @@ -0,0 +1,30 @@ +// Tests for TCP MessageHandler static helpers +#include +#include + +#include +#include + +int main() { + using namespace ColumnLynx::Net::TCP; + + // server message special codes + auto t1 = MessageHandler::decodeMessageType(0xFE); + // Expect GRACEFUL_DISCONNECT mapped + // Compare by converting back to uint8 + assert(MessageHandler::toUint8(t1) == 0xFE); + + auto t2 = MessageHandler::decodeMessageType(0xFF); + assert(MessageHandler::toUint8(t2) == 0xFF); + + // Client message range (>= 0xA0) + auto t3 = MessageHandler::decodeMessageType(0xA5); + assert(MessageHandler::toUint8(t3) == 0xA5); + + // Server message range (< 0xA0) and not special + auto t4 = MessageHandler::decodeMessageType(0x10); + assert(MessageHandler::toUint8(t4) == 0x10); + + std::cout << "TCP MessageHandler static helpers tests passed\n"; + return 0; +} From 05febee79e1406c7557f4e23a869113416a448d4 Mon Sep 17 00:00:00 2001 From: DcruBro Date: Fri, 29 May 2026 10:45:20 +0200 Subject: [PATCH 5/6] Key loading from files --- CMakeLists.txt | 2 +- README.md | 36 +++++---- .../columnlynx/client/net/tcp/tcp_client.hpp | 45 +---------- include/columnlynx/common/utils.hpp | 11 +++ src/client/main.cpp | 22 ++++++ src/client/net/tcp/tcp_client.cpp | 14 ++++ src/common/utils.cpp | 77 ++++++++++++++++++- src/server/main.cpp | 36 ++++----- tests/test_key_file_loading.cpp | 62 +++++++++++++++ 9 files changed, 225 insertions(+), 80 deletions(-) create mode 100644 tests/test_key_file_loading.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 416267e..7d0f7d1 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -6,7 +6,7 @@ cmake_minimum_required(VERSION 3.16) # If MAJOR is 0, and MINOR > 0, Version is BETA project(ColumnLynx - VERSION 1.1.1 + VERSION 1.2.0 LANGUAGES CXX ) diff --git a/README.md b/README.md index d3ad9d7..7297c0a 100644 --- a/README.md +++ b/README.md @@ -55,7 +55,22 @@ openssl pkey -in key.pem -pubout -outform DER | tail -c 32 | xxd -p -c 32 # Output example: 1c9d4f7a3b2e8a6d0f5c9b1e4d8a7f3c6e2b1a9d5f4c8e0a7b3d6c9f2e ``` -You can then set these keys accordingly in the **server_config** and **client_config** files. +Write the hex output into two separate files in the matching config directory: + +- `public.key` for the public key +- `private.key` for the private key seed + +The files should contain only the hex ASCII characters, optionally followed by a trailing newline. Hex parsing is case-insensitive. + +For example: + +```bash +printf '%s' '1c9d4f7a3b2e8a6d0f5c9b1e4d8a7f3c6e2b1a9d5f4c8e0a7b3d6c9f2e' > /etc/columnlynx/public.key +printf '%s' '9f3a2b6c0f8e4d1a7c3e9a4b5d2f8c6e1a9d0b7e3f4c2a8e6d5b1f0a3c4e' > /etc/columnlynx/private.key +chmod 600 /etc/columnlynx/private.key +``` + +On Unix-like systems, the software will warn if the private key file is too permissive and recommend tightening it with `chmod 600`. ### Server Setup (Linux Server ONLY) @@ -157,20 +172,19 @@ sudo nft add rule nat postroute ip saddr 10.10.0.0/24 oifname "eth0" masquerade "**server_config**" is a file that contains the server configuration, **one variable per line**. These are the current configuration available variables: -- **SERVER_PUBLIC_KEY** (Hex String): The public key to be used - Used for verification -- **SERVER_PRIVATE_KEY** (Hex String): The private key seed to be used - **NETWORK** (IPv4 Format): The network IPv4 to be used (Server Interface still needs to be configured manually) - **SUBNET_MASK** (Integer): The subnet mask to be used (ensure proper length, it will not be checked) **Example:** ``` -SERVER_PUBLIC_KEY=1c9d4f7a3b2e8a6d0f5c9b1e4d8a7f3c6e2b1a9d5f4c8e0a7b3d6c9f2e -SERVER_PRIVATE_KEY=9f3a2b6c0f8e4d1a7c3e9a4b5d2f8c6e1a9d0b7e3f4c2a8e6d5b1f0a3c4e NETWORK=10.10.0.0 SUBNET_MASK=24 ``` +The server keypair must now live in the same directory as `server_config`, stored in `public.key` and `private.key`. +`server_config` no longer stores key material. +
"**whitelisted_keys**" is a file that **public keys of clients that are allowed to connect to the server, one key per line**. @@ -184,17 +198,9 @@ SUBNET_MASK=24 ### Client -"**client_config**" is a file that contains the client configuration, **one variable per line**. These are the current configuration available variables: +"**client_config**" is a file that contains the client configuration, **one variable per line**. Key material is no longer stored here; if you do not have any client-only settings yet, this file can stay empty. -- **CLIENT_PUBLIC_KEY** (Hex String): The public key to be used - Used for verification -- **CLIENT_PRIVATE_KEY** (Hex String): The private key seed to be used - -**Example:** - -``` -CLIENT_PUBLIC_KEY=1c9d4f7a3b2e8a6d0f5c9b1e4d8a7f3c6e2b1a9d5f4c8e0a7b3d6c9f2e -CLIENT_PRIVATE_KEY=9f3a2b6c0f8e4d1a7c3e9a4b5d2f8c6e1a9d0b7e3f4c2a8e6d5b1f0a3c4e -``` +The client keypair must now live in the same directory as `client_config`, stored in `public.key` and `private.key`.
diff --git a/include/columnlynx/client/net/tcp/tcp_client.hpp b/include/columnlynx/client/net/tcp/tcp_client.hpp index d39b135..fa850f7 100644 --- a/include/columnlynx/client/net/tcp/tcp_client.hpp +++ b/include/columnlynx/client/net/tcp/tcp_client.hpp @@ -14,7 +14,6 @@ #include #include #include -#include #include #include #include @@ -27,48 +26,7 @@ namespace ColumnLynx::Net::TCP { public: TCPClient(asio::io_context& ioContext, const std::string& host, - const std::string& port) - : - mResolver(ioContext), - mSocket(ioContext), - mHost(host), - mPort(port), - mHeartbeatTimer(mSocket.get_executor()), - mLastHeartbeatReceived(std::chrono::steady_clock::now()), - mLastHeartbeatSent(std::chrono::steady_clock::now()) - { - // Get initial client config - std::string configPath = ClientSession::getInstance().getConfigPath(); - std::shared_ptr mLibSodiumWrapper = ClientSession::getInstance().getSodiumWrapper(); - - // Preload the config map - mRawClientConfig = Utils::getConfigMap(configPath + "client_config"); - - auto itPubkey = mRawClientConfig.find("CLIENT_PUBLIC_KEY"); - auto itPrivkey = mRawClientConfig.find("CLIENT_PRIVATE_KEY"); - - if (itPubkey != mRawClientConfig.end() && itPrivkey != mRawClientConfig.end()) { - Utils::log("Loading keypair from config file."); - - PublicKey pk; - PrivateSeed seed; - - std::copy_n(Utils::hexStringToBytes(itPrivkey->second).begin(), seed.size(), seed.begin()); // This is extremely stupid, but the C++ compiler has forced my hand (I would've just used to_array, but fucking asio decls) - std::copy_n(Utils::hexStringToBytes(itPubkey->second).begin(), pk.size(), pk.begin()); - - if (!mLibSodiumWrapper->recomputeKeys(seed, pk)) { - throw std::runtime_error("Failed to recompute keypair from config file values!"); - } - - Utils::debug("Newly-Loaded Public Key: " + Utils::bytesToHexString(mLibSodiumWrapper->getPublicKey(), 32)); - } else { - #if defined(DEBUG) - Utils::warn("No keypair found in config file! Using random key."); - #else - throw std::runtime_error("No keypair found in config file! Cannot start client without keys."); - #endif - } - } + const std::string& port); // Starts the TCP Client and initiaties the handshake void start(); @@ -106,6 +64,5 @@ namespace ColumnLynx::Net::TCP { int mMissedHeartbeats = 0; bool mIsHostDomain; Protocol::TunConfig mTunConfig; - std::unordered_map mRawClientConfig; }; } \ No newline at end of file diff --git a/include/columnlynx/common/utils.hpp b/include/columnlynx/common/utils.hpp index 5ad2605..626804a 100644 --- a/include/columnlynx/common/utils.hpp +++ b/include/columnlynx/common/utils.hpp @@ -79,4 +79,15 @@ namespace ColumnLynx::Utils { // Returns the config file in an unordered_map format. This purely reads the config file, you still need to parse it manually. std::unordered_map getConfigMap(std::string path, std::vector requiredKeys = {}); + + // Load a hex-encoded file, validate its byte length, and return the decoded bytes. + std::vector loadHexBytesFromFile(const std::string& path, size_t expectedBytes, const std::string& description = "key", bool warnOnInsecurePermissions = false); + + template + inline std::array loadHexArrayFromFile(const std::string& path, const std::string& description = "key", bool warnOnInsecurePermissions = false) { + auto bytes = loadHexBytesFromFile(path, N, description, warnOnInsecurePermissions); + std::array out{}; + std::copy_n(bytes.begin(), N, out.begin()); + return out; + } }; \ No newline at end of file diff --git a/src/client/main.cpp b/src/client/main.cpp index be27d56..1e99a1e 100644 --- a/src/client/main.cpp +++ b/src/client/main.cpp @@ -127,6 +127,28 @@ int main(int argc, char** argv) { initialState.virtualInterface = tun; std::shared_ptr sodiumWrapper = std::make_shared(); + const std::string clientPublicKeyPath = configPath + "public.key"; + const std::string clientPrivateKeyPath = configPath + "private.key"; + + namespace fs = std::filesystem; + bool clientKeyFilesPresent = fs::exists(clientPublicKeyPath) && fs::exists(clientPrivateKeyPath); + if (clientKeyFilesPresent) { + Utils::log("Loading client keypair from key files."); + + PublicKey pk = Utils::loadHexArrayFromFile(clientPublicKeyPath, "client public key"); + PrivateSeed seed = Utils::loadHexArrayFromFile(clientPrivateKeyPath, "client private key", true); + + if (!sodiumWrapper->recomputeKeys(seed, pk)) { + throw std::runtime_error("Failed to recompute client keypair from key files!"); + } + } else { +#if defined(DEBUG) + Utils::warn("No client keypair files found! Using random key."); +#else + throw std::runtime_error("No client keypair files found! Cannot start client without keys."); +#endif + } + debug("Public Key: " + Utils::bytesToHexString(sodiumWrapper->getPublicKey(), 32)); debug("Private Key: " + Utils::bytesToHexString(sodiumWrapper->getPrivateKey(), 64)); initialState.sodiumWrapper = sodiumWrapper; diff --git a/src/client/net/tcp/tcp_client.cpp b/src/client/net/tcp/tcp_client.cpp index 9766d86..ab74ef6 100644 --- a/src/client/net/tcp/tcp_client.cpp +++ b/src/client/net/tcp/tcp_client.cpp @@ -6,6 +6,20 @@ //#include namespace ColumnLynx::Net::TCP { + TCPClient::TCPClient(asio::io_context& ioContext, const std::string& host, const std::string& port) + : mResolver(ioContext), + mSocket(ioContext), + mHost(host), + mPort(port), + mHeartbeatTimer(mSocket.get_executor()), + mLastHeartbeatReceived(std::chrono::steady_clock::now()), + mLastHeartbeatSent(std::chrono::steady_clock::now()) + { + if (!ClientSession::getInstance().getSodiumWrapper()) { + throw std::runtime_error("ClientSession sodium wrapper is not initialized"); + } + } + void TCPClient::start() { auto self = shared_from_this(); mResolver.async_resolve(mHost, mPort, diff --git a/src/common/utils.cpp b/src/common/utils.cpp index 9ba8d40..d11784e 100644 --- a/src/common/utils.cpp +++ b/src/common/utils.cpp @@ -4,6 +4,7 @@ #include #include +#include namespace ColumnLynx::Utils { std::string unixMillisToISO8601(uint64_t unixMillis, bool local) { @@ -86,7 +87,7 @@ namespace ColumnLynx::Utils { } std::string getVersion() { - return "1.1.1"; + return "1.2.0"; } unsigned short serverPort() { @@ -251,4 +252,78 @@ namespace ColumnLynx::Utils { return config; } + + std::vector loadHexBytesFromFile(const std::string& path, size_t expectedBytes, const std::string& description, bool warnOnInsecurePermissions) { + namespace fs = std::filesystem; + std::error_code ec; + + fs::path p(path); + fs::path abs = fs::absolute(p, ec); + if (ec) { + throw std::runtime_error("loadHexBytesFromFile(): failed to resolve path: " + path + " - " + ec.message()); + } + + if (!fs::exists(abs, ec) || ec) { + throw std::runtime_error("loadHexBytesFromFile(): file does not exist: " + abs.string()); + } + + fs::path canon = fs::canonical(abs, ec); + if (ec) { + throw std::runtime_error("loadHexBytesFromFile(): failed to canonicalize path: " + abs.string()); + } + +#ifndef _WIN32 + if (warnOnInsecurePermissions) { + ec.clear(); + fs::file_status status = fs::status(canon, ec); + if (ec) { + warn("loadHexBytesFromFile(): failed to inspect permissions for " + canon.string() + " - " + ec.message()); + } else { + auto perms = status.permissions(); + if ((perms & (fs::perms::group_all | fs::perms::others_all)) != fs::perms::none) { + warn(description + " file permissions are too permissive: " + canon.string() + " (recommend chmod 600)"); + } + + if (!fs::is_regular_file(status)) { + warn(description + " path is not a regular file: " + canon.string()); + } + } + } +#endif + + std::ifstream file(canon); + if (!file.is_open()) { + throw std::runtime_error("Failed to open " + description + " file at path: " + canon.string()); + } + + std::string hex((std::istreambuf_iterator(file)), std::istreambuf_iterator()); + + auto trim = [](std::string& s) { + while (!s.empty() && std::isspace(static_cast(s.back()))) { + s.pop_back(); + } + + size_t start = 0; + while (start < s.size() && std::isspace(static_cast(s[start]))) { + ++start; + } + + if (start > 0) { + s.erase(0, start); + } + }; + + trim(hex); + + if (hex.empty()) { + throw std::runtime_error(description + " file is empty: " + canon.string()); + } + + std::vector bytes = hexStringToBytes(hex); + if (bytes.size() != expectedBytes) { + throw std::runtime_error(description + " file must contain exactly " + std::to_string(expectedBytes * 2) + " hex characters (" + std::to_string(expectedBytes) + " bytes), got " + std::to_string(bytes.size()) + " bytes"); + } + + return bytes; + } } \ No newline at end of file diff --git a/src/server/main.cpp b/src/server/main.cpp index 4f20de6..89f611d 100644 --- a/src/server/main.cpp +++ b/src/server/main.cpp @@ -6,6 +6,7 @@ #include #include #include +#include #include #include #include @@ -91,10 +92,9 @@ int main(int argc, char** argv) { serverState.configPath = configPath; #if defined(DEBUG) - std::unordered_map config = Utils::getConfigMap(configPath + "server_config", { "NETWORK", "SUBNET_MASK" }); + std::unordered_map config = Utils::getConfigMap(configPath + "server_config", { "NETWORK", "SUBNET_MASK" }); #else - // A production server should never use random keys. If the config file cannot be read or does not contain keys, the server will fail to start. - std::unordered_map config = Utils::getConfigMap(configPath + "server_config", { "NETWORK", "SUBNET_MASK", "SERVER_PUBLIC_KEY", "SERVER_PRIVATE_KEY" }); + std::unordered_map config = Utils::getConfigMap(configPath + "server_config", { "NETWORK", "SUBNET_MASK" }); #endif serverState.serverConfig = config; @@ -105,30 +105,28 @@ int main(int argc, char** argv) { // Store a reference to the tun in the serverState, it will increment and keep a safe reference (we love shared_ptrs) serverState.virtualInterface = tun; - // Generate a temporary keypair, replace with actual CA signed keys later (Note, these are stored in memory) std::shared_ptr sodiumWrapper = std::make_shared(); - auto itPubkey = config.find("SERVER_PUBLIC_KEY"); - auto itPrivkey = config.find("SERVER_PRIVATE_KEY"); + const std::string serverPublicKeyPath = configPath + "public.key"; + const std::string serverPrivateKeyPath = configPath + "private.key"; - if (itPubkey != config.end() && itPrivkey != config.end()) { - log("Loading keypair from config file."); + namespace fs = std::filesystem; + bool serverKeyFilesPresent = fs::exists(serverPublicKeyPath) && fs::exists(serverPrivateKeyPath); + if (serverKeyFilesPresent) { + log("Loading server keypair from key files."); - PublicKey pk; - PrivateSeed seed; - - std::copy_n(Utils::hexStringToBytes(itPrivkey->second).begin(), seed.size(), seed.begin()); - std::copy_n(Utils::hexStringToBytes(itPubkey->second).begin(), pk.size(), pk.begin()); + PublicKey pk = Utils::loadHexArrayFromFile(serverPublicKeyPath, "server public key"); + PrivateSeed seed = Utils::loadHexArrayFromFile(serverPrivateKeyPath, "server private key", true); if (!sodiumWrapper->recomputeKeys(seed, pk)) { - throw std::runtime_error("Failed to recompute keypair from config file values!"); + throw std::runtime_error("Failed to recompute keypair from key files!"); } } else { - #if defined(DEBUG) - warn("No keypair found in config file! Using random key."); - #else - throw std::runtime_error("No keypair found in config file! Cannot start server without keys."); - #endif +#if defined(DEBUG) + warn("No server keypair files found! Using random key."); +#else + throw std::runtime_error("No server keypair files found! Cannot start server without keys."); +#endif } log("Server public key: " + bytesToHexString(sodiumWrapper->getPublicKey(), crypto_sign_PUBLICKEYBYTES)); diff --git a/tests/test_key_file_loading.cpp b/tests/test_key_file_loading.cpp new file mode 100644 index 0000000..6929ba2 --- /dev/null +++ b/tests/test_key_file_loading.cpp @@ -0,0 +1,62 @@ +// Tests for hex key file loading helpers +#include +#include +#include +#include + +#include +#include + +int main() { + namespace fs = std::filesystem; + using namespace ColumnLynx::Utils; + + fs::path tempDir = fs::temp_directory_path() / "columnlynx_key_loader_test"; + fs::remove_all(tempDir); + fs::create_directories(tempDir); + + auto publicKeyPath = tempDir / "public.key"; + auto privateKeyPath = tempDir / "private.key"; + + { + std::ofstream pub(publicKeyPath); + pub << "00112233445566778899aabbccddeeff00112233445566778899AABBCCDDEEFF"; + } + + { + std::ofstream priv(privateKeyPath); + priv << "ffeeddccbbaa99887766554433221100ffeeddccbbaa99887766554433221100\n"; + } + + auto pk = loadHexArrayFromFile(publicKeyPath.string(), "public key"); + auto sk = loadHexArrayFromFile(privateKeyPath.string(), "private key", true); + + assert(pk[0] == 0x00 && pk[1] == 0x11 && pk.back() == 0xFF); + assert(sk[0] == 0xFF && sk[1] == 0xEE && sk.back() == 0x00); + + bool threwMissing = false; + try { + (void)loadHexArrayFromFile((tempDir / "missing.key").string(), "missing key"); + } catch (const std::exception&) { + threwMissing = true; + } + assert(threwMissing && "Missing key file should throw"); + + auto badLengthPath = tempDir / "bad-length.key"; + { + std::ofstream bad(badLengthPath); + bad << "abcd"; + } + + bool threwBadLength = false; + try { + (void)loadHexArrayFromFile(badLengthPath.string(), "bad length key"); + } catch (const std::exception&) { + threwBadLength = true; + } + assert(threwBadLength && "Wrong-length key file should throw"); + + std::cout << "Key file loader tests passed\n"; + fs::remove_all(tempDir); + return 0; +} \ No newline at end of file From 1d9e1e616516b2b47ea16e933f9073022b077834 Mon Sep 17 00:00:00 2001 From: DcruBro Date: Sun, 14 Jun 2026 20:02:50 +0200 Subject: [PATCH 6/6] Version 1.2.1 - Bug Fixes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Critical (4) C1 — udp_client.hpp / udp_client.cpp: Moved mStartReceive() out of the constructor into start(), after the socket is opened. C2 — client/main.cpp:187: || → && in the main loop condition, so it exits when the connection drops instead of spinning forever. C3 — tcp_connection.hpp: Added mConnectionAESKey{}, mConnectionSessionID(0), and mConnectionPublicKey{} to the constructor initializer list. C4 — tcp_server.hpp / tcp_server.cpp: Added std::mutex mClientsMutex and locked it at all three mClients access sites (insert, erase callback, stop snapshot). High (5) H1 — tcp_connection.cpp:284: Replaced hardcoded htonl(0x0A0A0001) with htonl(baseIP + 1). H2 — session_registry.hpp / session_registry.cpp / udp_server.cpp: Changed get() and getByIP() to return shared_ptr (non-const); removed all four const_cast usages. H3 — session_registry.cpp:90: Guarded against mask == 0 || mask >= 32 (shift UB) and hostCount < 3 (unsigned underflow). H4 — virtual_interface.cpp: Added runCommandWin32() using CreateProcess (no shell); replaced both system() calls in resetIP(). H5 — virtual_interface.cpp: Added close(mFd); mFd = -1; before both throw sites in the macOS constructor (ioctl failure and connect failure). Medium (4) M1 — tcp_connection.cpp:241: Added explicit find() + end-iterator check for NETWORK and SUBNET_MASK before dereferencing. M2 — tcp_message_handler.cpp: Added an 8 KB (MAX_MSG_BODY = 8192) cap on incoming message body length; oversized messages close the connection. M3 — tcp_connection.cpp:232: Capped the session ID generation loop at 16 attempts before disconnecting. M4 — session_registry.cpp:8: put() now evicts the old IP→session mapping before replacing an entry, preventing stale mIPSessions entries. --- .../columnlynx/client/net/udp/udp_client.hpp | 1 - .../common/net/session_registry.hpp | 4 +- .../server/net/tcp/tcp_connection.hpp | 3 + .../columnlynx/server/net/tcp/tcp_server.hpp | 2 + src/client/main.cpp | 2 +- src/client/net/udp/udp_client.cpp | 1 + src/common/session_registry.cpp | 11 +++- src/common/tcp_message_handler.cpp | 6 ++ src/common/utils.cpp | 2 +- src/common/virtual_interface.cpp | 62 ++++++++++++++----- src/server/net/tcp/tcp_connection.cpp | 27 ++++++-- src/server/net/tcp/tcp_server.cpp | 12 +++- src/server/net/udp/udp_server.cpp | 13 ++-- 13 files changed, 109 insertions(+), 37 deletions(-) diff --git a/include/columnlynx/client/net/udp/udp_client.hpp b/include/columnlynx/client/net/udp/udp_client.hpp index af613d9..04f1240 100644 --- a/include/columnlynx/client/net/udp/udp_client.hpp +++ b/include/columnlynx/client/net/udp/udp_client.hpp @@ -20,7 +20,6 @@ namespace ColumnLynx::Net::UDP { const std::string& port) : mSocket(ioContext), mResolver(ioContext), mHost(host), mPort(port) { - mStartReceive(); } // Start the UDP client diff --git a/include/columnlynx/common/net/session_registry.hpp b/include/columnlynx/common/net/session_registry.hpp index e18f11e..c7f04b8 100644 --- a/include/columnlynx/common/net/session_registry.hpp +++ b/include/columnlynx/common/net/session_registry.hpp @@ -61,10 +61,10 @@ namespace ColumnLynx::Net { void put(uint32_t sessionID, std::shared_ptr state); // Lookup a session entry by session ID - std::shared_ptr get(uint32_t sessionID) const; + std::shared_ptr get(uint32_t sessionID) const; // Lookup a session entry by IPv4 - std::shared_ptr getByIP(uint32_t ip) const; + std::shared_ptr getByIP(uint32_t ip) const; // Get a snapshot of the Session Registry std::unordered_map> snapshot() const; diff --git a/include/columnlynx/server/net/tcp/tcp_connection.hpp b/include/columnlynx/server/net/tcp/tcp_connection.hpp index 0afdcdf..0463a98 100644 --- a/include/columnlynx/server/net/tcp/tcp_connection.hpp +++ b/include/columnlynx/server/net/tcp/tcp_connection.hpp @@ -53,6 +53,9 @@ namespace ColumnLynx::Net::TCP { : mHandler(std::make_shared(std::move(socket))), mHeartbeatTimer(mHandler->socket().get_executor()), + mConnectionAESKey{}, + mConnectionSessionID(0), + mConnectionPublicKey{}, mLastHeartbeatReceived(std::chrono::steady_clock::now()), mLastHeartbeatSent(std::chrono::steady_clock::now()) {} diff --git a/include/columnlynx/server/net/tcp/tcp_server.hpp b/include/columnlynx/server/net/tcp/tcp_server.hpp index 224993e..7c5159d 100644 --- a/include/columnlynx/server/net/tcp/tcp_server.hpp +++ b/include/columnlynx/server/net/tcp/tcp_server.hpp @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include @@ -77,6 +78,7 @@ namespace ColumnLynx::Net::TCP { asio::io_context &mIoContext; asio::ip::tcp::acceptor mAcceptor; + std::mutex mClientsMutex; std::unordered_set mClients; }; diff --git a/src/client/main.cpp b/src/client/main.cpp index 1e99a1e..73efaf9 100644 --- a/src/client/main.cpp +++ b/src/client/main.cpp @@ -184,7 +184,7 @@ int main(int argc, char** argv) { debug("isDone flag: " + std::to_string(done)); // Client is running - while ((client->isConnected() || !client->isHandshakeComplete()) && !done) { + while (client->isConnected() && !done) { //debug("Client connection flag: " + std::to_string(client->isConnected())); auto packet = tun->readPacket(); /*if (!client->isConnected() || done) { diff --git a/src/client/net/udp/udp_client.cpp b/src/client/net/udp/udp_client.cpp index fb55162..da60b24 100644 --- a/src/client/net/udp/udp_client.cpp +++ b/src/client/net/udp/udp_client.cpp @@ -44,6 +44,7 @@ namespace ColumnLynx::Net::UDP { } Utils::log("UDP Client ready to send to " + mRemoteEndpoint.address().to_string() + ":" + std::to_string(mRemoteEndpoint.port())); + mStartReceive(); } void UDPClient::sendMessage(const std::string& data) { diff --git a/src/common/session_registry.cpp b/src/common/session_registry.cpp index 6df8019..3bb0fb7 100644 --- a/src/common/session_registry.cpp +++ b/src/common/session_registry.cpp @@ -7,17 +7,22 @@ namespace ColumnLynx::Net { void SessionRegistry::put(uint32_t sessionID, std::shared_ptr state) { std::unique_lock lock(mMutex); + // If replacing an existing entry, evict its old IP mapping first. + auto existing = mSessions.find(sessionID); + if (existing != mSessions.end() && existing->second) { + mIPSessions.erase(existing->second->clientTunIP); + } mSessions[sessionID] = std::move(state); mIPSessions[mSessions[sessionID]->clientTunIP] = mSessions[sessionID]; } - std::shared_ptr SessionRegistry::get(uint32_t sessionID) const { + std::shared_ptr SessionRegistry::get(uint32_t sessionID) const { std::shared_lock lock(mMutex); auto it = mSessions.find(sessionID); return (it == mSessions.end()) ? nullptr : it->second; } - std::shared_ptr SessionRegistry::getByIP(uint32_t ip) const { + std::shared_ptr SessionRegistry::getByIP(uint32_t ip) const { std::shared_lock lock(mMutex); auto it = mIPSessions.find(ip); return (it == mIPSessions.end()) ? nullptr : it->second; @@ -87,7 +92,9 @@ namespace ColumnLynx::Net { uint32_t SessionRegistry::getFirstAvailableIP(uint32_t baseIP, uint8_t mask) const { std::shared_lock lock(mMutex); + if (mask == 0 || mask >= 32) return 0; // degenerate subnet uint32_t hostCount = (1u << (32 - mask)); + if (hostCount < 3) return 0; // no assignable host addresses uint32_t firstHost = 2; uint32_t lastHost = hostCount - 2; diff --git a/src/common/tcp_message_handler.cpp b/src/common/tcp_message_handler.cpp index 66e2fe3..a873b17 100644 --- a/src/common/tcp_message_handler.cpp +++ b/src/common/tcp_message_handler.cpp @@ -54,6 +54,12 @@ namespace ColumnLynx::Net::TCP { mCurrentType = decodeMessageType(mHeader[0]); uint16_t len = (mHeader[1] << 8) | mHeader[2]; + constexpr uint16_t MAX_MSG_BODY = 8192; + if (len > MAX_MSG_BODY) { + Utils::error("Incoming message body too large (" + std::to_string(len) + " > " + std::to_string(MAX_MSG_BODY) + "), closing connection."); + if (mOnDisconnect) mOnDisconnect(asio::error::message_size); + return; + } mReadBody(len); } else { if (!NetHelper::isExpectedDisconnect(ec)) { diff --git a/src/common/utils.cpp b/src/common/utils.cpp index d11784e..a968515 100644 --- a/src/common/utils.cpp +++ b/src/common/utils.cpp @@ -87,7 +87,7 @@ namespace ColumnLynx::Utils { } std::string getVersion() { - return "1.2.0"; + return "1.2.1"; } unsigned short serverPort() { diff --git a/src/common/virtual_interface.cpp b/src/common/virtual_interface.cpp index 6fb2ad7..d8a3dc7 100644 --- a/src/common/virtual_interface.cpp +++ b/src/common/virtual_interface.cpp @@ -58,6 +58,44 @@ static void InitializeWintun() #undef RESOLVE } +// Run a command on Windows via CreateProcess (no shell — shell metacharacters in +// arguments are not interpreted by cmd.exe). +static bool runCommandWin32(const std::vector& args) { + if (args.empty()) return false; + + // Build a properly-quoted command line using Windows quoting rules. + std::string cmdLine; + for (size_t i = 0; i < args.size(); i++) { + if (i > 0) cmdLine += ' '; + bool needsQuote = args[i].find_first_of(" \t\"") != std::string::npos || args[i].empty(); + if (needsQuote) { + cmdLine += '"'; + for (char c : args[i]) { + if (c == '"') cmdLine += '\\'; + cmdLine += c; + } + cmdLine += '"'; + } else { + cmdLine += args[i]; + } + } + + STARTUPINFOA si{}; + si.cb = sizeof(si); + PROCESS_INFORMATION pi{}; + + if (!CreateProcessA(nullptr, cmdLine.data(), nullptr, nullptr, + FALSE, CREATE_NO_WINDOW, nullptr, nullptr, &si, &pi)) + return false; + + WaitForSingleObject(pi.hProcess, INFINITE); + DWORD exitCode = 1; + GetExitCodeProcess(pi.hProcess, &exitCode); + CloseHandle(pi.hProcess); + CloseHandle(pi.hThread); + return exitCode == 0; +} + #endif // _WIN32 namespace ColumnLynx::Net { @@ -116,8 +154,10 @@ static bool runCommand(const std::vector& args) { struct ctl_info ctlInfo {}; std::strncpy(ctlInfo.ctl_name, UTUN_CONTROL_NAME, sizeof(ctlInfo.ctl_name)); - if (ioctl(mFd, CTLIOCGINFO, &ctlInfo) == -1) + if (ioctl(mFd, CTLIOCGINFO, &ctlInfo) == -1) { + close(mFd); mFd = -1; throw std::runtime_error("ioctl(CTLIOCGINFO) failed: " + std::string(strerror(errno))); + } struct sockaddr_ctl sc {}; sc.sc_len = sizeof(sc); @@ -127,9 +167,11 @@ static bool runCommand(const std::vector& args) { sc.sc_unit = 0; // 0 = auto-assign next utunX if (connect(mFd, (struct sockaddr*)&sc, sizeof(sc)) < 0) { - if (errno == EPERM) + int savedErrno = errno; + close(mFd); mFd = -1; + if (savedErrno == EPERM) throw std::runtime_error("connect(AF_SYS_CONTROL) failed: Insufficient permissions (try running with sudo)"); - throw std::runtime_error("connect(AF_SYS_CONTROL) failed: " + std::string(strerror(errno))); + throw std::runtime_error("connect(AF_SYS_CONTROL) failed: " + std::string(strerror(savedErrno))); } // Retrieve actual utun device name via UTUN_OPT_IFNAME @@ -351,20 +393,10 @@ static bool runCommand(const std::vector& args) { //); //system(cmd); #elif defined(_WIN32) - char cmd[512]; // Remove any persistent routes associated with this interface - snprintf(cmd, sizeof(cmd), - "netsh routing ip delete persistentroute all name=\"%s\"", - mIfName.c_str() - ); - system(cmd); - + runCommandWin32({"netsh", "routing", "ip", "delete", "persistentroute", "all", "name=" + mIfName}); // Reset to DHCP - snprintf(cmd, sizeof(cmd), - "netsh interface ip set address name=\"%s\" dhcp", - mIfName.c_str() - ); - system(cmd); + runCommandWin32({"netsh", "interface", "ip", "set", "address", "name=" + mIfName, "dhcp"}); #endif } diff --git a/src/server/net/tcp/tcp_connection.cpp b/src/server/net/tcp/tcp_connection.cpp index bcfd37d..5ab0a44 100644 --- a/src/server/net/tcp/tcp_connection.cpp +++ b/src/server/net/tcp/tcp_connection.cpp @@ -229,17 +229,32 @@ namespace ColumnLynx::Net::TCP { std::memcpy(mConnectionAESKey.data(), decrypted.data(), decrypted.size()); // Make a Session ID - unique and not zero (zero is reserved for invalid sessions) - do { - randombytes_buf(&mConnectionSessionID, sizeof(mConnectionSessionID)); - } while (SessionRegistry::getInstance().exists(mConnectionSessionID) || mConnectionSessionID == 0); // Regenerate if it already exists or is zero (zero is reserved for invalid sessions) + { + int idAttempts = 0; + do { + if (++idAttempts > 16) { + Utils::error("Failed to generate unique session ID after 16 attempts. Killing connection from " + reqAddr); + disconnect(); + return; + } + randombytes_buf(&mConnectionSessionID, sizeof(mConnectionSessionID)); + } while (SessionRegistry::getInstance().exists(mConnectionSessionID) || mConnectionSessionID == 0); + } // Encrypt the Session ID with the established AES key (using symmetric encryption, nonce can be all zeros for this purpose) Nonce symNonce{}; // All zeros const auto& serverConfig = ServerSession::getInstance().getRawServerConfig(); - std::string networkString = serverConfig.find("NETWORK")->second; // The load check guarantees that this value exists - uint8_t configMask = std::stoi(serverConfig.find("SUBNET_MASK")->second); // Same deal here + auto netIt = serverConfig.find("NETWORK"); + auto maskIt = serverConfig.find("SUBNET_MASK"); + if (netIt == serverConfig.end() || maskIt == serverConfig.end()) { + Utils::error("Server config is missing NETWORK or SUBNET_MASK. Killing connection from " + reqAddr); + disconnect(); + return; + } + std::string networkString = netIt->second; + uint8_t configMask = static_cast(std::stoi(maskIt->second)); uint32_t baseIP = Net::VirtualInterface::stringToIpv4(networkString); @@ -281,7 +296,7 @@ namespace ColumnLynx::Net::TCP { // Add to session registry Utils::log("Handshake with " + reqAddr + " completed successfully. Session ID assigned (" + std::to_string(mConnectionSessionID) + ")."); - auto session = std::make_shared(mConnectionAESKey, std::chrono::hours(12), clientIP, htonl(0x0A0A0001), mConnectionSessionID); + auto session = std::make_shared(mConnectionAESKey, std::chrono::hours(12), clientIP, htonl(baseIP + 1), mConnectionSessionID); session->setBaseNonce(); // Set it SessionRegistry::getInstance().put(mConnectionSessionID, std::move(session)); SessionRegistry::getInstance().lockIP(mConnectionSessionID, clientIP); diff --git a/src/server/net/tcp/tcp_server.cpp b/src/server/net/tcp/tcp_server.cpp index 77c30b7..aa7bf2b 100644 --- a/src/server/net/tcp/tcp_server.cpp +++ b/src/server/net/tcp/tcp_server.cpp @@ -35,11 +35,15 @@ namespace ColumnLynx::Net::TCP { auto client = TCPConnection::create( std::move(socket), [this](std::shared_ptr c) { + std::lock_guard lock(mClientsMutex); mClients.erase(c); Utils::log("Client removed."); } ); - mClients.insert(client); + { + std::lock_guard lock(mClientsMutex); + mClients.insert(client); + } client->start(); Utils::log("Accepted new client connection."); @@ -59,7 +63,11 @@ namespace ColumnLynx::Net::TCP { } // Snapshot to avoid iterator invalidation while callbacks erase() - std::vector> snapshot(mClients.begin(), mClients.end()); + std::vector> snapshot; + { + std::lock_guard lock(mClientsMutex); + snapshot.assign(mClients.begin(), mClients.end()); + } for (auto &client : snapshot) { try { client->disconnect(); // should shutdown+close the socket diff --git a/src/server/net/udp/udp_server.cpp b/src/server/net/udp/udp_server.cpp index e786324..71f8766 100644 --- a/src/server/net/udp/udp_server.cpp +++ b/src/server/net/udp/udp_server.cpp @@ -40,7 +40,7 @@ namespace ColumnLynx::Net::UDP { std::vector encryptedPayload(it, mRecvBuffer.begin() + bytes); // Get associated session state - std::shared_ptr session = SessionRegistry::getInstance().get(sessionID); + std::shared_ptr session = SessionRegistry::getInstance().get(sessionID); if (!session) { Utils::warn("UDP: Unknown or invalid session from " + mRemoteEndpoint.address().to_string()); @@ -60,9 +60,9 @@ namespace ColumnLynx::Net::UDP { Utils::debug("Passed decryption"); - const_cast(session.get())->setUDPEndpoint(mRemoteEndpoint); // Update endpoint after confirming decryption + session->setUDPEndpoint(mRemoteEndpoint); // Update endpoint after confirming decryption // Update recv counter - const_cast(session.get())->recv_ctr.fetch_add(1, std::memory_order_relaxed); + session->recv_ctr.fetch_add(1, std::memory_order_relaxed); // For now, just log the decrypted payload std::string payloadStr(plaintext.begin(), plaintext.end()); @@ -79,7 +79,7 @@ namespace ColumnLynx::Net::UDP { void UDPServer::sendData(uint32_t sessionID, const std::string& data) { // Find the IPv4/IPv6 endpoint for the session - std::shared_ptr session = SessionRegistry::getInstance().get(sessionID); + std::shared_ptr session = SessionRegistry::getInstance().get(sessionID); if (!session) { Utils::warn("UDP: Cannot send data, unknown session ID " + std::to_string(sessionID)); return; @@ -98,14 +98,13 @@ namespace ColumnLynx::Net::UDP { // Increment send counter with overflow protection uint64_t sendCount = 0; { - auto ptr = const_cast(session.get()); - uint64_t old = ptr->send_ctr.load(std::memory_order_relaxed); + uint64_t old = session->send_ctr.load(std::memory_order_relaxed); for (;;) { if (old == std::numeric_limits::max()) { Utils::error("UDP: send counter overflow for session " + std::to_string(sessionID)); return; } - if (ptr->send_ctr.compare_exchange_weak(old, old + 1, std::memory_order_relaxed)) { + if (session->send_ctr.compare_exchange_weak(old, old + 1, std::memory_order_relaxed)) { sendCount = old; break; }