fix(connection): set a Connections remote-socket from WG activity (#3245)

It appears that sometimes, the dialer already considers the connection
as connected whilst the other party is still finishing the ICE
handshake. In that case, the dialer will start wireguard activity. Once
the tunnel is fully established, the dialer will then start to send data
which can lead to a `NotConnected` error in case the listener hasn't yet
finished the handshake and updated the state. This is only a local
inconsistency which we can fix by also updating the `remote_socket`
field based on activity on the wireguard tunnel.

For future debugging, we also raise the log level of `str0m` to see the
STUN messages that are being exchanged.

Fixes: #3178.

rust/connection-tests/src/main.rs

@@ -26,7 +26,7 @@ async fn main() -> Result<()> {
     tokio::time::sleep(Duration::from_secs(1)).await; // Until redis is up.
 
     tracing_subscriber::fmt()
-        .with_env_filter(EnvFilter::builder().parse("info,boringtun=debug")?)
+        .with_env_filter(EnvFilter::builder().parse("info,boringtun=debug,str0m=debug")?)
         .init();
 
     let role = std::env::var("ROLE")

rust/connlib/connection/src/pool.rs

@@ -182,12 +182,16 @@ where
                 // In our API, we parse the packets directly as an IpPacket.
                 // Thus, the caller can query whatever data they'd like, not just the source IP so we don't return it in addition.
                 TunnResult::WriteToTunnelV4(packet, ip) => {
+                    conn.set_remote_from_wg_activity(from);
+
                     let ipv4_packet = Ipv4Packet::new(packet).expect("boringtun verifies validity");
                     debug_assert_eq!(ipv4_packet.get_source(), ip);
 
                     Ok(Some((*id, ipv4_packet.into())))
                 }
                 TunnResult::WriteToTunnelV6(packet, ip) => {
+                    conn.set_remote_from_wg_activity(from);
+
                     let ipv6_packet = Ipv6Packet::new(packet).expect("boringtun verifies validity");
                     debug_assert_eq!(ipv6_packet.get_source(), ip);
 
@@ -199,6 +203,8 @@ where
                 // This should be fairly rare which is why we just allocate these and return them from `poll_transmit` instead.
                 // Overall, this results in a much nicer API for our caller and should not affect performance.
                 TunnResult::WriteToNetwork(bytes) => {
+                    conn.set_remote_from_wg_activity(from);
+
                     self.buffered_transmits.push_back(Transmit {
                         dst: from,
                         payload: bytes.to_vec(),
@@ -579,6 +585,20 @@ impl Connection {
         from_connected_remote || from_possible_remote
     }
 
+    fn set_remote_from_wg_activity(&mut self, remote: SocketAddr) {
+        match self.remote_socket {
+            Some(current) if current != remote => {
+                tracing::info!(%current, new = %remote, "Setting new remote socket from WG activity");
+                self.remote_socket = Some(remote);
+            }
+            None => {
+                tracing::info!(new = %remote, "Setting remote socket from WG activity");
+                self.remote_socket = Some(remote);
+            }
+            _ => {}
+        }
+    }
+
     fn poll_timeout(&mut self) -> Option<Instant> {
         let agent_timeout = self.agent.poll_timeout();
         let next_wg_timer = self.next_timer_update;