Merge pull request #492 from Dliwk/client-addr-verification

add client address checks

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.

src/ballistica/game/connection/connection_set.cc

@@ -452,6 +452,10 @@ auto ConnectionSet::UDPConnectionPacket(const std::vector<uint8_t>& 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)) {
+          Log("VerifyClientAddr() failed");
+          break;
+        }
 
         // Wipe that client out (if it still exists).
         PushClientDisconnectedCall(client_id);
@@ -495,6 +499,11 @@ auto ConnectionSet::UDPConnectionPacket(const std::vector<uint8_t>& data_in,
     case BA_PACKET_CLIENT_GAMEPACKET_COMPRESSED: {
       if (data_size > 2) {
         uint8_t client_id = data[1];
+        if (!VerifyClientAddr(client_id, addr)) {
+          Log("VerifyClientAddr() failed");
+          break;
+        }
+
         auto i = connections_to_clients_.find(client_id);
         if (i != connections_to_clients_.end()) {
           // FIXME: could change HandleGamePacketCompressed to avoid this
@@ -685,6 +694,25 @@ auto ConnectionSet::UDPConnectionPacket(const std::vector<uint8_t>& 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)

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<int, Object::Ref<ConnectionToClient> >

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.

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<uint8_t>& data)
       -> void override;
+  auto addr() { return *addr_; }
 
  private:
   uint8_t request_id_;

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<const sockaddr_in&>(addr_).sin_addr.s_addr
+              == reinterpret_cast<const sockaddr_in&>(other.addr_)
+                     .sin_addr.s_addr)
+             && (reinterpret_cast<const sockaddr_in&>(addr_).sin_port
+                 == reinterpret_cast<const sockaddr_in&>(other.addr_).sin_port);
+    }
+    if (addr_.ss_family == AF_INET6) {
+      return !memcmp(&(reinterpret_cast<const sockaddr_in6&>(addr_).sin6_addr),
+                     &(reinterpret_cast<const sockaddr_in6&>(other.addr_)
+                           .sin6_addr),
+                     sizeof(in6_addr))
+             && (reinterpret_cast<const sockaddr_in6&>(addr_).sin6_port
+                 == reinterpret_cast<const sockaddr_in6&>(other.addr_)
+                        .sin6_port);
+    }
+    throw Exception();
+  }
 
  private:
   sockaddr_storage addr_{};