Merge pull request #718 from omf2097/adt/netplay-logging

Log connection IPs, fix netplay leaks
omf2097 · Nov 3, 2024 · 6dd0bdb · 6dd0bdb
2 parents bef8ac8 + 6d915ad
commit 6dd0bdb
Show file tree

Hide file tree

Showing 4 changed files with 45 additions and 43 deletions.
diff --git a/src/game/scenes/lobby.c b/src/game/scenes/lobby.c
@@ -101,10 +101,9 @@ void lobby_free(scene *scene) {
     guiframe_free(local->frame);
     list_free(&local->users);
     list_free(&local->log);
-    // TODO only destroy this if we're going back to the main menu
-    /*if(local->client) {
+    if(local->client) {
         enet_host_destroy(local->client);
-    }*/
+    }
     if(local->dialog) {
         dialog_free(local->dialog);
         omf_free(local->dialog);
@@ -629,7 +628,9 @@ void lobby_try_connect(void *scenedata, void *userdata) {
     scene *s = scenedata;
     lobby_local *local = scene_get_userdata(s);
     if(!local->opponent_peer) {
-        DEBUG("doing scheduled outbound connection");
+        DEBUG("doing scheduled outbound connection to %d.%d.%d.%d port %d", local->opponent->address.host & 0xFF,
+              (local->opponent->address.host >> 8) & 0xFF, (local->opponent->address.host >> 16) & 0xF,
+              (local->opponent->address.host >> 24) & 0xFF, local->opponent->address.port);
         local->opponent_peer = enet_host_connect(local->client, &local->opponent->address, 2, 0);
         enet_peer_timeout(local->opponent_peer, 4, 1000, 1000);
     }
@@ -681,7 +682,7 @@ component *lobby_exit_create(scene *s) {
 
     menu_attach(menu, label_create(&tconf, "Exit the Challenge Arena?"));
     menu_attach(menu, textbutton_create(&tconf, "Yes", NULL, COM_ENABLED, lobby_do_exit, s));
-    menu_attach(menu, textbutton_create(&tconf, "No", NULL, COM_ENABLED, lobby_refuse_exit, NULL));
+    menu_attach(menu, textbutton_create(&tconf, "No", NULL, COM_ENABLED, lobby_refuse_exit, s));
 
     return menu;
 }
@@ -982,6 +983,11 @@ void lobby_tick(scene *scene, int paused) {
                                 // try to connect immediately
                                 local->opponent_peer =
                                     enet_host_connect(local->client, &local->opponent->address, 2, 0);
+
+                                DEBUG("doing immediate outbound connection to %d.%d.%d.%d port %d",
+                                      local->opponent->address.host & 0xFF, (local->opponent->address.host >> 8) & 0xFF,
+                                      (local->opponent->address.host >> 16) & 0xF,
+                                      (local->opponent->address.host >> 24) & 0xFF, local->opponent->address.port);
                                 enet_peer_timeout(local->opponent_peer, 4, 1000, 1000);
                                 local->connection_count = 0;
 
@@ -1083,13 +1089,15 @@ void lobby_tick(scene *scene, int paused) {
     controller *c1 = game_player_get_ctrl(p1);
     if(c1->type == CTRL_TYPE_NETWORK && net_controller_ready(c1)) {
         DEBUG("network peer is ready, tick offset is %d and rtt is %d", net_controller_tick_offset(c1), c1->rtt);
+        local->client = NULL;
         game_state_set_next(gs, SCENE_MELEE);
     }
 
     game_player *p2 = game_state_get_player(gs, 1);
     controller *c2 = game_player_get_ctrl(p2);
     if(c2->type == CTRL_TYPE_NETWORK && net_controller_ready(c2) == 1) {
         DEBUG("network peer is ready, tick offset is %d and rtt is %d", net_controller_tick_offset(c2), c2->rtt);
+        local->client = NULL;
         game_state_set_next(gs, SCENE_MELEE);
     }
 }

diff --git a/src/game/scenes/mainmenu/menu_listen.c b/src/game/scenes/mainmenu/menu_listen.c
@@ -171,6 +171,7 @@ component *menu_listen_create(scene *s) {
         while(!nat_create_mapping(&local->nat, address.port, ext_port)) {
             if(settings_get()->net.net_ext_port_start == 0) {
                 ext_port = rand_int(65525 - 1024) + 1024;
+                randtries++;
                 if(randtries > 10) {
                     ext_port = 0;
                     break;

diff --git a/src/game/utils/nat.c b/src/game/utils/nat.c
@@ -14,41 +14,31 @@ bool nat_create_upnp_mapping(nat_ctx *ctx, uint16_t int_port, uint16_t ext_port)
     char ext_portstr[6];
     snprintf(ext_portstr, sizeof(ext_portstr), "%d", ext_port);
 
-    char lan_address[64];
-#if(MINIUPNPC_API_VERSION >= 18)
-    int status =
-        UPNP_GetValidIGD(ctx->upnp_dev, &ctx->upnp_urls, &ctx->upnp_data, lan_address, sizeof(lan_address), NULL, 0);
-#else
-    int status = UPNP_GetValidIGD(ctx->upnp_dev, &ctx->upnp_urls, &ctx->upnp_data, lan_address, sizeof(lan_address));
-#endif
+    // get the external (WAN) IP address
+    // char wan_address[64];
+    // UPNP_GetExternalIPAddress(ctx->upnp_urls.controlURL, ctx->upnp_data.first.servicetype, wan_address);
 
-    if(status == 1) {
-        // get the external (WAN) IP address
-        char wan_address[64];
-        UPNP_GetExternalIPAddress(ctx->upnp_urls.controlURL, ctx->upnp_data.first.servicetype, wan_address);
-
-        // add a new UDP port mapping from WAN port 12345 to local host port 24680
-        int error = UPNP_AddPortMapping(
-            ctx->upnp_urls.controlURL, ctx->upnp_data.first.servicetype,
-            ext_portstr, // external (WAN) port requested
-            int_portstr, // internal (LAN) port to which packets will be redirected
-            lan_address, // internal (LAN) address to which packets will be redirected
-            "OpenOMF",   // text description to indicate why or who is responsible for the port mapping
-            "UDP",       // protocol must be either TCP or UDP
-            NULL,        // remote (peer) host address or nullptr for no restriction
-            "86400");    // port map lease duration (in seconds) or zero for "as long as possible"
-        if(error == 0) {
-            DEBUG("NAT-uPNP Port map successfully created from %d to %d!", int_port, ext_port);
-
-            ctx->int_port = int_port;
-            ctx->ext_port = ext_port;
-            return true;
-        } else {
-            DEBUG("NAT-uPNP port mapping failed with %d", error);
-            // TODO there are some errors we can work around here
-            // like overly short lifetimes
-            return false;
-        }
+    // add a new UDP port mapping from WAN port 12345 to local host port 24680
+    int error =
+        UPNP_AddPortMapping(ctx->upnp_urls.controlURL, ctx->upnp_data.first.servicetype,
+                            ext_portstr,      // external (WAN) port requested
+                            int_portstr,      // internal (LAN) port to which packets will be redirected
+                            ctx->lan_address, // internal (LAN) address to which packets will be redirected
+                            "OpenOMF", // text description to indicate why or who is responsible for the port mapping
+                            "UDP",     // protocol must be either TCP or UDP
+                            NULL,      // remote (peer) host address or nullptr for no restriction
+                            "86400");  // port map lease duration (in seconds) or zero for "as long as possible"
+    if(error == 0) {
+        DEBUG("NAT-uPNP Port map successfully created from %d to %d!", int_port, ext_port);
+
+        ctx->int_port = int_port;
+        ctx->ext_port = ext_port;
+        return true;
+    } else {
+        DEBUG("NAT-uPNP port %d -> %d mapping failed with %d", int_port, ext_port, error);
+        // TODO there are some errors we can work around here
+        // like overly short lifetimes
+        return false;
     }
 #endif
     return false;
@@ -109,12 +99,12 @@ void nat_try_upnp(nat_ctx *ctx) {
                                  &error); // error condition
     // TODO check error here?
     // try to look up our lan address, to test it
-    char lan_address[64];
 #if(MINIUPNPC_API_VERSION >= 18)
-    int status =
-        UPNP_GetValidIGD(ctx->upnp_dev, &ctx->upnp_urls, &ctx->upnp_data, lan_address, sizeof(lan_address), NULL, 0);
+    int status = UPNP_GetValidIGD(ctx->upnp_dev, &ctx->upnp_urls, &ctx->upnp_data, ctx->lan_address,
+                                  sizeof(ctx->lan_address), NULL, 0);
 #else
-    int status = UPNP_GetValidIGD(ctx->upnp_dev, &ctx->upnp_urls, &ctx->upnp_data, lan_address, sizeof(lan_address));
+    int status =
+        UPNP_GetValidIGD(ctx->upnp_dev, &ctx->upnp_urls, &ctx->upnp_data, ctx->lan_address, sizeof(ctx->lan_address));
 #endif
     // look up possible "status" values, the number "1" indicates a valid IGD was found
 
@@ -153,6 +143,7 @@ void nat_release_upnp(nat_ctx *ctx) {
         DEBUG("failed to remove port mapping with %d", error);
     }
     FreeUPNPUrls(&ctx->upnp_urls);
+    freeUPNPDevlist(ctx->upnp_dev);
 #endif
 }
 
@@ -210,4 +201,5 @@ void nat_free(nat_ctx *ctx) {
         default:
             break;
     }
+    ctx->type = NAT_TYPE_NONE;
 }
diff --git a/src/game/utils/nat.h b/src/game/utils/nat.h
@@ -20,6 +20,7 @@ typedef struct nat_ctx_t {
     uint16_t int_port;
     uint16_t ext_port;
 #ifdef MINIUPNPC_FOUND
+    char lan_address[64];
     struct UPNPUrls upnp_urls;
     struct IGDdatas upnp_data;
     struct UPNPDev *upnp_dev;