From 22cffa39fe930f499be3e7bc9c4ac50237b68287 Mon Sep 17 00:00:00 2001 From: Roman Trapeznikov Date: Sat, 30 Jul 2022 16:18:40 +0300 Subject: [PATCH 1/3] add client address verification --- .../game/connection/connection_set.cc | 22 +++++++++++++++++++ .../game/connection/connection_set.h | 2 ++ .../game/connection/connection_to_client.h | 1 - .../connection/connection_to_client_udp.h | 2 ++ src/ballistica/networking/sockaddr.h | 20 +++++++++++++++++ 5 files changed, 46 insertions(+), 1 deletion(-) diff --git a/src/ballistica/game/connection/connection_set.cc b/src/ballistica/game/connection/connection_set.cc index fcd96868..fa314032 100644 --- a/src/ballistica/game/connection/connection_set.cc +++ b/src/ballistica/game/connection/connection_set.cc @@ -452,6 +452,7 @@ auto ConnectionSet::UDPConnectionPacket(const std::vector& data_in, if (data_size == 2) { // Client is telling us (host) that it wants to disconnect. uint8_t client_id = data[1]; + if (!VerifyClientAddr(client_id, addr)) break; // Wipe that client out (if it still exists). PushClientDisconnectedCall(client_id); @@ -495,6 +496,8 @@ auto ConnectionSet::UDPConnectionPacket(const std::vector& data_in, case BA_PACKET_CLIENT_GAMEPACKET_COMPRESSED: { if (data_size > 2) { uint8_t client_id = data[1]; + if (!VerifyClientAddr(client_id, addr)) break; + auto i = connections_to_clients_.find(client_id); if (i != connections_to_clients_.end()) { // FIXME: could change HandleGamePacketCompressed to avoid this @@ -685,6 +688,25 @@ auto ConnectionSet::UDPConnectionPacket(const std::vector& data_in, } } +auto ConnectionSet::VerifyClientAddr(uint8_t client_id, const SockAddr& addr) + -> bool { + auto connection_to_client = connections_to_clients_.find(client_id); + + if (connection_to_client != connections_to_clients_.end()) { + auto connection_to_client_udp = connection_to_client->second->GetAsUDP(); + if (connection_to_client_udp == nullptr) { + // We can't get client's SockAddr in that case. + return true; + } + if (addr == connection_to_client_udp->addr()) { + return true; + } + return false; + } + + return false; +} + void ConnectionSet::SetClientInfoFromMasterServer( const std::string& client_token, PyObject* info_obj) { // NOLINTNEXTLINE (python doing bitwise math on signed int) diff --git a/src/ballistica/game/connection/connection_set.h b/src/ballistica/game/connection/connection_set.h index 29db4fc2..e44b97c7 100644 --- a/src/ballistica/game/connection/connection_set.h +++ b/src/ballistica/game/connection/connection_set.h @@ -99,6 +99,8 @@ class ConnectionSet { auto PushClientDisconnectedCall(int id) -> void; private: + auto VerifyClientAddr(uint8_t client_id, const SockAddr& addr) -> bool; + // Try to minimize the chance a garbage packet will have this id. int next_connection_to_client_id_{113}; std::unordered_map > diff --git a/src/ballistica/game/connection/connection_to_client.h b/src/ballistica/game/connection/connection_to_client.h index bb58023d..5269e1c5 100644 --- a/src/ballistica/game/connection/connection_to_client.h +++ b/src/ballistica/game/connection/connection_to_client.h @@ -49,7 +49,6 @@ class ConnectionToClient : public Connection { next_kick_vote_allow_time_ = val; } auto next_kick_vote_allow_time() const { return next_kick_vote_allow_time_; } - auto public_device_id() const { return public_device_id_; } // Returns a spec for this client that incorporates their player names // or their peer name if they have no players. diff --git a/src/ballistica/game/connection/connection_to_client_udp.h b/src/ballistica/game/connection/connection_to_client_udp.h index 58c3d045..cac1b30a 100644 --- a/src/ballistica/game/connection/connection_to_client_udp.h +++ b/src/ballistica/game/connection/connection_to_client_udp.h @@ -9,6 +9,7 @@ #include "ballistica/game/connection/connection_to_client.h" #include "ballistica/networking/networking.h" +#include "ballistica/networking/sockaddr.h" namespace ballistica { @@ -27,6 +28,7 @@ class ConnectionToClientUDP : public ConnectionToClient { void SendDisconnectRequest(); auto SendGamePacketCompressed(const std::vector& data) -> void override; + auto addr() { return *addr_; } private: uint8_t request_id_; diff --git a/src/ballistica/networking/sockaddr.h b/src/ballistica/networking/sockaddr.h index 25916e00..ff112459 100644 --- a/src/ballistica/networking/sockaddr.h +++ b/src/ballistica/networking/sockaddr.h @@ -45,6 +45,26 @@ class SockAddr { throw Exception(); } } + auto operator==(const SockAddr& other) const -> bool { + if (addr_.ss_family != other.addr_.ss_family) return false; + if (addr_.ss_family == AF_INET) { + return (reinterpret_cast(addr_).sin_addr.s_addr + == reinterpret_cast(other.addr_) + .sin_addr.s_addr) + && (reinterpret_cast(addr_).sin_port + == reinterpret_cast(other.addr_).sin_port); + } + if (addr_.ss_family == AF_INET6) { + return !memcmp(&(reinterpret_cast(addr_).sin6_addr), + &(reinterpret_cast(other.addr_) + .sin6_addr), + sizeof(in6_addr)) + && (reinterpret_cast(addr_).sin6_port + == reinterpret_cast(other.addr_) + .sin6_port); + } + throw Exception(); + } private: sockaddr_storage addr_{}; From 54dd0a6a12362b73787f0ff1e0d3dbee6d943db9 Mon Sep 17 00:00:00 2001 From: Roman Trapeznikov Date: Sun, 21 Aug 2022 21:22:33 +0300 Subject: [PATCH 2/3] add log message --- src/ballistica/game/connection/connection_set.cc | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/ballistica/game/connection/connection_set.cc b/src/ballistica/game/connection/connection_set.cc index fa314032..8c32d0ce 100644 --- a/src/ballistica/game/connection/connection_set.cc +++ b/src/ballistica/game/connection/connection_set.cc @@ -452,7 +452,10 @@ auto ConnectionSet::UDPConnectionPacket(const std::vector& data_in, if (data_size == 2) { // Client is telling us (host) that it wants to disconnect. uint8_t client_id = data[1]; - if (!VerifyClientAddr(client_id, addr)) break; + if (!VerifyClientAddr(client_id, addr)) { + Log("VerifyClientAddr() failed"); + break; + } // Wipe that client out (if it still exists). PushClientDisconnectedCall(client_id); @@ -496,7 +499,10 @@ auto ConnectionSet::UDPConnectionPacket(const std::vector& data_in, case BA_PACKET_CLIENT_GAMEPACKET_COMPRESSED: { if (data_size > 2) { uint8_t client_id = data[1]; - if (!VerifyClientAddr(client_id, addr)) break; + if (!VerifyClientAddr(client_id, addr)) { + Log("VerifyClientAddr() failed"); + break; + } auto i = connections_to_clients_.find(client_id); if (i != connections_to_clients_.end()) { From 41cae90a12e8bf3c2145564d7555b653752f21ce Mon Sep 17 00:00:00 2001 From: Roman Trapeznikov Date: Sun, 21 Aug 2022 21:59:05 +0300 Subject: [PATCH 3/3] update changelog --- CHANGELOG.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2e0e62bf..aa148714 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,12 +1,12 @@ ### 1.7.7 (build 20698, api 7, 2022-08-21) -- added `ba.app.meta.load_exported_classes()` for loading classes discovered by the meta subsystem cleanly in a background thread -- improved logging of missing playlist game types -- some ba.Lstr functionality can now be used in background threads - +- Added `ba.app.meta.load_exported_classes()` for loading classes discovered by the meta subsystem cleanly in a background thread. +- Improved logging of missing playlist game types. +- Some ba.Lstr functionality can now be used in background threads. +- Added simple check for incoming packets (should increase security level a bit). ### 1.7.6 (build 20687, api 7, 2022-08-11) - Cleaned up da MetaSubsystem code. -- It is now possible to tell the meta system about arbitrary classes (ba_meta export foo.bar.Class) instead of just the preset types 'plugin', 'game', etc. +- It is now possible to tell the meta system about arbitrary classes (ba_meta export foo.bar.Class) instead of just the preset types 'plugin', 'game', etc. - Newly discovered plugins are now activated immediately instead of requiring a restart. ### 1.7.5 (build 20672, api 7, 2022-07-25) @@ -49,7 +49,7 @@ ### 1.7.2 (20620, 2022-06-25) - Minor fixes in some minigames (Thanks Droopy!) -- Fixed a bug preventing 'clients' arg from working in _ba.chatmessage (Thanks imayushsaini!) +- Fixed a bug preventing 'clients' arg from working in `_ba.chatmessage` (Thanks imayushsaini!) - Fixed a bug where ba.Player.getdelegate(doraise=True) could return None instead of raising a ba.DelegateNotFoundError (thanks Dliwk!) - Lots of Romanian language improvements (Thanks Meryu!) - Workspaces are now functional. They require signing in with a V2 account, which currently is limited to explicitly created email/password logins. See ballistica.net to create such an account or create/edit a workspace. This is bleeding edge stuff so please holler with any bugs you come across or if anything seems unintuitive.