From eec615eddbc1ff96ee08fdb964d21c4c836bc55c Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Tue, 25 Jun 2024 13:53:00 +1000 Subject: [PATCH] refactor(connlib): drop all connections when roaming (#5308) Currently, `snownet` tries to be very clever in how it roams connections. This is/was necessary because we associated DNS-specific state with a connection. More specifically, the assigned proxy IPs for a DNS resource are stored as part of a connection with the gateway. As a result, DNS resources would always break if the underlying connection in `snownet` failed. This is quite error prone and means, `snownet` must be very careful to never-ever fail a connection erroneously. With #5049, we no longer store any important state with a connection and thus, can implement roaming in much simpler way: Drop all connections and let the incoming packets create new ones. This is much more robust as we don't have to "patch" existing state in `snownet` as part of roaming. We test this new functionality by adding a `RoamClient` transition to `tunnel_test`. This ensures roaming works in a lot of scenarios, including relayed and non-relayed situations as well as roaming between either of them. As a result, we can delete several of the more specific test cases of `snownet`. Depends-On: #5049. Replaces: #5060. Resolves: #5080. --- rust/connlib/snownet/src/node.rs | 33 ++- rust/connlib/snownet/tests/lib.rs | 192 +----------------- .../tunnel/proptest-regressions/tests.txt | 2 + rust/connlib/tunnel/src/client.rs | 14 +- rust/connlib/tunnel/src/gateway.rs | 3 +- rust/connlib/tunnel/src/tests/reference.rs | 13 ++ rust/connlib/tunnel/src/tests/sim_node.rs | 30 +++ rust/connlib/tunnel/src/tests/sut.rs | 9 + rust/connlib/tunnel/src/tests/transition.rs | 27 ++- rust/snownet-tests/src/main.rs | 6 +- .../tests/direct-download-roaming-network.sh | 10 +- 11 files changed, 138 insertions(+), 201 deletions(-) diff --git a/rust/connlib/snownet/src/node.rs b/rust/connlib/snownet/src/node.rs index eae12ce9f..9addfefe0 100644 --- a/rust/connlib/snownet/src/node.rs +++ b/rust/connlib/snownet/src/node.rs @@ -148,15 +148,26 @@ where binding.refresh(now); } + // FIXME(tech-debt): A refresh here is unnecessary. + // We always rebind the sockets one layer up, which changes our outgoing port and thus means we are a "new" client from TURN's perspective (TURN operates on 3-tuples). + // Thus, a "reset" operation that just clears the local state would be sufficient here and would likely simplify the internals of `Allocation`. for allocation in self.allocations.values_mut() { allocation.refresh(now); } - for candidate in self.host_candidates.drain() { - for (id, agent) in self.connections.agents_mut() { - remove_local_candidate(id, agent, &candidate, &mut self.pending_events) - } - } + self.pending_events.clear(); + + let connections = self.connections.iter_ids().collect::>(); + let num_connections = connections.len(); + + self.pending_events + .push_back(Event::ConnectionsCleared(connections)); + + self.host_candidates.clear(); + self.connections.clear(); + self.buffered_transmits.clear(); + + tracing::debug!("Cleared {num_connections} connections"); } pub fn public_key(&self) -> PublicKey { @@ -1097,6 +1108,15 @@ where fn len(&self) -> usize { self.initial.len() + self.established.len() } + + fn clear(&mut self) { + self.initial.clear(); + self.established.clear(); + } + + fn iter_ids(&self) -> impl Iterator + '_ { + self.initial.keys().chain(self.established.keys()).copied() + } } /// Wraps the message as a channel data message via the relay, iff: @@ -1247,6 +1267,9 @@ pub enum Event { /// /// All state associated with the connection has been cleared. ConnectionFailed(TId), + + /// The referenced connections had their state cleared. + ConnectionsCleared(Vec), } #[derive(Clone, PartialEq)] diff --git a/rust/connlib/snownet/tests/lib.rs b/rust/connlib/snownet/tests/lib.rs index 430eb0c9d..921daa3e9 100644 --- a/rust/connlib/snownet/tests/lib.rs +++ b/rust/connlib/snownet/tests/lib.rs @@ -11,156 +11,9 @@ use std::{ vec, }; use str0m::{net::Protocol, Candidate}; -use tracing::{debug_span, info_span, Span}; +use tracing::{debug_span, Span}; use tracing_subscriber::util::SubscriberInitExt; -#[test] -fn smoke_direct() { - let _guard = setup_tracing(); - let firewall = Firewall::default(); - let mut clock = Clock::new(); - - let (alice, bob) = alice_and_bob(); - - let mut alice = - TestNode::new(info_span!("Alice"), alice, "1.1.1.1:80").with_primary_as_host_candidate(); - let mut bob = - TestNode::new(info_span!("Bob"), bob, "1.1.1.2:80").with_primary_as_host_candidate(); - - handshake(&mut alice, &mut bob, &clock); - - loop { - if alice.is_connected_to(&bob) && bob.is_connected_to(&alice) { - break; - } - - progress(&mut alice, &mut bob, &mut [], &firewall, &mut clock); - } - - alice.ping(ip("9.9.9.9"), ip("8.8.8.8"), &bob, clock.now); - progress(&mut alice, &mut bob, &mut [], &firewall, &mut clock); - assert_eq!(bob.packets_from(ip("9.9.9.9")).count(), 1); - - bob.ping(ip("8.8.8.8"), ip("9.9.9.9"), &alice, clock.now); - progress(&mut alice, &mut bob, &mut [], &firewall, &mut clock); - assert_eq!(alice.packets_from(ip("8.8.8.8")).count(), 1); -} - -#[test] -fn smoke_relayed() { - let _guard = setup_tracing(); - let mut clock = Clock::new(); - - let (alice, bob) = alice_and_bob(); - - let mut relays = [( - 1, - TestRelay::new( - SocketAddrV4::new(Ipv4Addr::LOCALHOST, 3478), - debug_span!("Roger"), - ), - )]; - let mut alice = TestNode::new(debug_span!("Alice"), alice, "1.1.1.1:80").with_relays( - "alice", - HashSet::default(), - &mut relays, - clock.now, - ); - let mut bob = TestNode::new(debug_span!("Bob"), bob, "2.2.2.2:80").with_relays( - "bob", - HashSet::default(), - &mut relays, - clock.now, - ); - let firewall = Firewall::default() - .with_block_rule(&alice, &bob) - .with_block_rule(&bob, &alice); - - handshake(&mut alice, &mut bob, &clock); - - loop { - if alice.is_connected_to(&bob) && bob.is_connected_to(&alice) { - break; - } - - progress(&mut alice, &mut bob, &mut relays, &firewall, &mut clock); - } - - alice.ping(ip("9.9.9.9"), ip("8.8.8.8"), &bob, clock.now); - progress(&mut alice, &mut bob, &mut relays, &firewall, &mut clock); - assert_eq!(bob.packets_from(ip("9.9.9.9")).count(), 1); - - bob.ping(ip("8.8.8.8"), ip("9.9.9.9"), &alice, clock.now); - progress(&mut alice, &mut bob, &mut relays, &firewall, &mut clock); - assert_eq!(alice.packets_from(ip("8.8.8.8")).count(), 1); -} - -#[test] -fn reconnect_discovers_new_interface() { - let _guard = setup_tracing(); - let mut clock = Clock::new(); - let firewall = Firewall::default(); - - let (alice, bob) = alice_and_bob(); - - let mut relays = [( - 1, - TestRelay::new( - SocketAddrV4::new(Ipv4Addr::LOCALHOST, 3478), - debug_span!("Roger"), - ), - )]; - let mut alice = TestNode::new(debug_span!("Alice"), alice, "1.1.1.1:80").with_relays( - "alice", - HashSet::default(), - &mut relays, - clock.now, - ); - let mut bob = TestNode::new(debug_span!("Bob"), bob, "2.2.2.2:80").with_relays( - "bob", - HashSet::default(), - &mut relays, - clock.now, - ); - - handshake(&mut alice, &mut bob, &clock); - - loop { - if alice.is_connected_to(&bob) && bob.is_connected_to(&alice) { - break; - } - - progress(&mut alice, &mut bob, &mut relays, &firewall, &mut clock); - } - - // To ensure that switching networks really works, block all traffic from the old IP. - let firewall = firewall - .with_block_rule(&alice, &bob) - .with_block_rule(&bob, &alice); - - alice.switch_network("10.0.0.1:80"); - alice.span.in_scope(|| alice.node.reconnect(clock.now)); - - // Make some progress. - for _ in 0..10 { - progress(&mut alice, &mut bob, &mut relays, &firewall, &mut clock); - } - - alice.ping(ip("9.9.9.9"), ip("8.8.8.8"), &bob, clock.now); - progress(&mut alice, &mut bob, &mut relays, &firewall, &mut clock); - assert_eq!(bob.packets_from(ip("9.9.9.9")).count(), 1); - - bob.ping(ip("8.8.8.8"), ip("9.9.9.9"), &alice, clock.now); - progress(&mut alice, &mut bob, &mut relays, &firewall, &mut clock); - assert_eq!(alice.packets_from(ip("8.8.8.8")).count(), 1); - - assert!(alice - .signalled_candidates() - .any(|(_, c, _)| c.addr().to_string() == "10.0.0.1:80")); - assert_eq!(alice.failed_connections().count(), 0); - assert_eq!(bob.failed_connections().count(), 0); -} - #[test] fn migrate_connection_to_new_relay() { let _guard = setup_tracing(); @@ -717,11 +570,6 @@ impl TestNode { self } - fn switch_network(&mut self, new_primary: &str) { - self.primary = new_primary.parse().unwrap(); - self.local.push(self.primary); - } - fn is_connected_to(&self, other: &TestNode) -> bool { self.node.connection_id(other.node.public_key()).is_some() } @@ -748,37 +596,12 @@ impl TestNode { self.transmits.push_back(transmit); } - fn signalled_candidates(&self) -> impl Iterator + '_ { - self.events.iter().filter_map(|(e, instant)| match e { - Event::NewIceCandidate { - connection, - candidate, - } => Some(( - *connection, - Candidate::from_sdp_string(candidate).unwrap(), - *instant, - )), - Event::InvalidateIceCandidate { .. } - | Event::ConnectionEstablished(_) - | Event::ConnectionFailed(_) => None, - }) - } - fn packets_from(&self, src: IpAddr) -> impl Iterator> { self.received_packets .iter() .filter(move |p| p.source() == src) } - fn failed_connections(&self) -> impl Iterator + '_ { - self.events.iter().filter_map(|(e, instant)| match e { - Event::ConnectionFailed(id) => Some((*id, *instant)), - Event::NewIceCandidate { .. } => None, - Event::InvalidateIceCandidate { .. } => None, - Event::ConnectionEstablished(_) => None, - }) - } - fn receive(&mut self, local: SocketAddr, from: SocketAddr, packet: &[u8], now: Instant) { if let Some((_, packet)) = self .span @@ -809,8 +632,9 @@ impl TestNode { } => other .span .in_scope(|| other.node.remove_remote_candidate(connection, candidate)), - Event::ConnectionEstablished(_) => {} - Event::ConnectionFailed(_) => {} + Event::ConnectionEstablished(_) + | Event::ConnectionFailed(_) + | Event::ConnectionsCleared(_) => {} }; } } @@ -851,14 +675,6 @@ impl TestNode { other.receive(dst, src, payload, now); } } - - fn with_primary_as_host_candidate(mut self) -> Self { - self.span - .in_scope(|| self.node.add_local_host_candidate(self.primary)) - .unwrap(); - - self - } } fn handshake(client: &mut TestNode, server: &mut TestNode, clock: &Clock) { diff --git a/rust/connlib/tunnel/proptest-regressions/tests.txt b/rust/connlib/tunnel/proptest-regressions/tests.txt index 0f5543781..41861d435 100644 --- a/rust/connlib/tunnel/proptest-regressions/tests.txt +++ b/rust/connlib/tunnel/proptest-regressions/tests.txt @@ -40,3 +40,5 @@ cc bebf2584fa9a47eafd09973073038358ade7f06d46200b5c4beaee1ff2a05988 cc 4e887961e6763061483a7dd29700e66f8d4cd217e9ae26710615da0527949438 # shrinks to (initial_state, transitions, seen_counter) = (ReferenceState { now: Instant { tv_sec: 30118, tv_nsec: 190553482 }, utc_now: 2024-06-05T06:05:55.088202180Z, client: SimNode { id: ClientId(00000000-0000-0000-0000-000000000000), state: PrivateKey("0000000000000000000000000000000000000000000000000000000000000000"), ip4_socket: Some(127.0.0.1:1), ip6_socket: None, tunnel_ip4: 100.64.0.1, tunnel_ip6: fd00:2021:1111:: }, gateway: SimNode { id: GatewayId(00000000-0000-0000-0000-000000000000), state: PrivateKey("0000000000000000000000000000000000000000000000000000000000000001"), ip4_socket: None, ip6_socket: Some([::ffff:92.167.158.82]:54712), tunnel_ip4: 100.81.139.146, tunnel_ip6: fd00:2021:1111::16:9540 }, relay: SimRelay { id: RelayId(fe771606-7d14-196d-2162-22eebbea169b), ip_stack: Dual { ip4: 25.201.169.214, ip6: ::ffff:144.204.179.91 }, allocations: {} }, system_dns_resolvers: [c3c4:8a8f:53da:c629:5236:730f:91c5:f15], upstream_dns_resolvers: [IpPort(IpDnsServer { address: [::ffff:127.0.0.1]:53 }), IpPort(IpDnsServer { address: 51.122.145.158:53 }), IpPort(IpDnsServer { address: [::ffff:39.181.232.19]:53 })], client_cidr_resources: {}, client_dns_resources: {}, client_dns_records: {}, client_connected_cidr_resources: {}, global_dns_records: {Name(vmut.jky.ligln.): {::ffff:217.23.242.189, 9f66:8d45:d7:f18d:a421:b430:3acc:51d7, ::ffff:231.124.94.200}, Name(wrykdl.cgs.zvixy.): {cadc:13bd:c595:e38d:7edd:78b0:834d:2547, ::ffff:77.5.113.155, 62ba:e078:d78:2637:ae90:be5c:14ed:5df8, 66.215.26.40, 202.224.245.215}, Name(kdh.fratyq.): {127.0.0.1, 16.4.156.68, 44.35.176.74, c43b:9954:1bdf:c859:461b:a89c:cae2:8e57}, Name(narigr.lkge.): {209.31.162.162, ::ffff:41.19.133.230, ::ffff:183.121.211.92, ::ffff:22.165.223.248, 2000:8af9:ba17:4f86:83c7:c9f3:d4:d67e}, Name(rgbp.yvgeue.femho.): {::ffff:197.22.223.230, 229.242.167.188, 7c99:52fa:e26d:fc63:671c:1d8a:8a3f:f5d3}, Name(qkybry.rvm.qobdn.): {220.42.249.229, ::ffff:126.109.54.130, ad59:36b4:a30c:67a1:acac:4640:12df:6b2}, Name(bvc.fdqmvs.): {88.72.154.13, f1c9:be99:fa9c:5264:b4e3:cdea:e72f:6fae, 97.19.159.117, ::ffff:49.191.156.213, 76.81.237.142}, Name(woib.hnvwng.mlsh.): {1.79.11.217, 103.36.117.81, 127.0.0.1, 90.88.31.136, ::ffff:47.215.26.213}, Name(uafu.xynv.dbyj.): {::ffff:194.255.207.96, 3fb:3712:52b4:76f1:fe65:19ad:f32c:7db2, 22.54.137.69}, Name(ptzw.xgjs.): {218.146.70.246, cae2:9018:d23e:adf2:6b9a:54:cd32:6e9e, 160.57.249.196, 127.0.0.1, 321:13a8:41f9:a59a:e2c:14ab:591a:8de9}, Name(nlf.cbovqn.jlerys.): {::ffff:197.85.159.78, 157.127.143.152, ::ffff:79.190.192.196, ::ffff:2.139.13.245}, Name(jskrx.vajo.): {1155:8b71:b6ea:ed9a:a51f:55c:ffd:fb4e}, Name(yirfsp.zbe.): {154.111.46.199, 10.226.130.205, ::ffff:195.164.181.164, 127.0.0.1}, Name(udamv.okbnu.): {127.0.0.1, 235.181.223.120}}, expected_icmp_handshakes: [], expected_dns_handshakes: [] }, [AddCidrResource(ResourceDescriptionCidr { id: ResourceId(00000000-0000-0000-0000-000000000000), address: V4(Ipv4Network { network_address: 0.0.0.0, netmask: 32 }), name: "aaaa", address_description: "qlmtkrkthh", sites: [Site { name: "oxpw", id: SiteId(a8f9c1d1-93fd-1f18-57c8-027bd41a9d31) }, Site { name: "fsqg", id: SiteId(295fb094-0889-daca-1e67-235ea6e89fdd) }, Site { name: "qnntuwywo", id: SiteId(ee755002-0d03-a0b1-1e93-25faba25deae) }] }), UpdateUpstreamDnsServers { servers: [IpPort(IpDnsServer { address: 0.0.0.0:53 }), IpPort(IpDnsServer { address: [::ffff:0.0.0.0]:53 })] }, SendICMPPacketToResource { idx: Index(0), seq: 0, identifier: 0, src: TunnelIp4 }, SendDnsQuery { r_idx: Index(0), r_type: A, query_id: 0, dns_server_idx: Index(0) }], None) cc 425de9c1549daa24850b76eb8b80d37677bb8f334af5ca2fb676933a6957d0ba # shrinks to (initial_state, transitions, seen_counter) = (ReferenceState { now: Instant { tv_sec: 28370, tv_nsec: 556450729 }, utc_now: 2024-06-12T05:51:51.032598569Z, client: SimNode { id: ClientId(00000000-0000-0000-0000-000000000000), state: PrivateKey("0000000000000000000000000000000000000000000000000000000000000000"), ip4_socket: None, ip6_socket: Some([::ffff:0.0.0.0]:1), tunnel_ip4: 100.64.0.1, tunnel_ip6: fd00:2021:1111:: }, gateway: SimNode { id: GatewayId(00000000-0000-0000-0000-000000000000), state: PrivateKey("0000000000000000000000000000000000000000000000000000000000000001"), ip4_socket: Some(127.0.0.1:1), ip6_socket: None, tunnel_ip4: 100.64.0.1, tunnel_ip6: fd00:2021:1111:: }, relay: SimRelay { id: RelayId(00000000-0000-0000-0000-000000000000), ip_stack: Dual { ip4: 0.0.0.1, ip6: ::ffff:127.0.0.1 }, allocations: {} }, system_dns_resolvers: [0.0.0.0], upstream_dns_resolvers: [IpPort(IpDnsServer { address: 0.0.0.0:53 }), IpPort(IpDnsServer { address: [::ffff:0.0.0.0]:53 })], client_cidr_resources: {}, client_dns_resources: {}, client_dns_records: {}, client_connected_cidr_resources: {}, global_dns_records: {}, expected_icmp_handshakes: [], expected_dns_handshakes: [] }, [SendICMPPacketToDnsResource { src: TunnelIp4, dst: Name(ouybla.zolmy.ngm.omtew.), resolved_ip: Selector { rng: TestRng { rng: ChaCha(ChaCha20Rng { rng: BlockRng { core: ChaChaXCore {}, result_len: 64, index: 64 } }) }, bias_increment: 18446744073709551615 }, seq: 0, identifier: 0 }], None) cc f8bcb3a0a75efb0c38ec9ea1f9efd520899f4be4eeb284fc65e4a83a8844d7b3 # shrinks to (initial_state, transitions, seen_counter) = (ReferenceState { now: Instant { tv_sec: 17358, tv_nsec: 527869944 }, utc_now: 2024-06-06T21:27:16.648845453Z, client: SimNode { id: ClientId(00000000-0000-0000-0000-000000000000), state: PrivateKey("0000000000000000000000000000000000000000000000000000000000000000"), ip4_socket: Some(127.0.0.1:1), ip6_socket: None, tunnel_ip4: 100.64.0.1, tunnel_ip6: fd00:2021:1111:: }, gateway: SimNode { id: GatewayId(00000000-0000-0000-0000-000000000000), state: PrivateKey("0000000000000000000000000000000000000000000000000000000000000001"), ip4_socket: None, ip6_socket: Some([::ffff:0.0.0.0]:1), tunnel_ip4: 100.64.0.1, tunnel_ip6: fd00:2021:1111:: }, relay: SimRelay { id: RelayId(00000000-0000-0000-0000-000000000000), ip_stack: Dual { ip4: 0.0.0.1, ip6: ::ffff:127.0.0.1 }, allocations: {} }, system_dns_resolvers: [0.0.0.0], upstream_dns_resolvers: [], client_cidr_resources: {}, client_dns_resources: {}, client_dns_records: {}, client_connected_cidr_resources: {}, client_connected_dns_resources: {}, global_dns_records: {}, expected_icmp_handshakes: [], expected_dns_handshakes: [] }, [AddDnsResource { resource: ResourceDescriptionDns { id: ResourceId(00000000-0000-0000-0000-000000000000), address: "aaa.bro", name: "aaaa", address_description: "txuskjcbl", sites: [Site { name: "ymhutnt", id: SiteId(7ca05481-9c03-d9e8-1286-dbba1947acc6) }, Site { name: "qlhsw", id: SiteId(ef1bb3da-e883-0f6a-48f3-7c6610eeae5c) }] }, records: {Name(aaa.bro.): {::ffff:127.0.0.1, 127.0.0.1}} }, SendDnsQuery { r_idx: Index(15802363307536118505), r_type: A, query_id: 39521, dns_server_idx: Index(16823264331511040100) }, SendICMPPacketToResource { idx: Index(0), seq: 0, identifier: 0, src: TunnelIp4 }, SendICMPPacketToResource { idx: Index(0), seq: 0, identifier: 0, src: Other(0.0.0.0) }], None) +cc 83703ff8437242d56e8abec8d4ffcaf32d85b7153b419879125c85cc09cfc265 # shrinks to (initial_state, transitions, seen_counter) = (ReferenceState { now: Instant { tv_sec: 22429, tv_nsec: 464161289 }, utc_now: 2024-06-11T05:10:44.360588236Z, client: SimNode { id: ClientId(00000000-0000-0000-0000-000000000000), state: PrivateKey("0000000000000000000000000000000000000000000000000000000000000000"), ip4_socket: None, ip6_socket: Some([::ffff:127.0.0.1]:1), tunnel_ip4: 100.64.0.1, tunnel_ip6: fd00:2021:1111:: }, gateway: SimNode { id: GatewayId(00000000-0000-0000-0000-000000000000), state: PrivateKey("0000000000000000000000000000000000000000000000000000000000000001"), ip4_socket: None, ip6_socket: Some([::ffff:0.0.0.0]:61663), tunnel_ip4: 100.81.187.212, tunnel_ip6: fd00:2021:1111::f:d81e }, relay: SimRelay { id: RelayId(2ba35b8a-768a-3d67-9695-c408ca7601ed), ip_stack: Dual { ip4: 160.146.67.126, ip6: ::ffff:13.28.200.65 }, allocations: {} }, system_dns_resolvers: [183.78.150.83], upstream_dns_resolvers: [IpPort(IpDnsServer { address: 127.0.0.1:53 }), IpPort(IpDnsServer { address: 97.136.151.155:53 }), IpPort(IpDnsServer { address: 127.0.0.1:53 }), IpPort(IpDnsServer { address: [9c5:3fb6:1a48:a994:2176:deb9:fbc8:8892]:53 }), IpPort(IpDnsServer { address: [::ffff:203.178.186.72]:53 }), IpPort(IpDnsServer { address: [::ffff:127.0.0.1]:53 })], client_cidr_resources: {}, client_dns_resources: {}, client_dns_records: {}, client_connected_cidr_resources: {}, global_dns_records: {Name(itwl.ztqvp.): {::ffff:250.129.7.170, 152.91.0.13, ::ffff:0.0.0.0}, Name(artfkh.nzogs.): {4430:a99:c73e:c2f0:b470:3d9:8c69:8138, 229.4.195.238, 144.131.80.205, 3f49:7c88:edf1:aa85:1f5d:f1be:4a0d:95ef, 194.218.159.177}, Name(wxmhq.oweypu.): {94.163.223.160, 234.247.5.103}, Name(jcwg.oat.etbd.): {93.105.73.141, ::ffff:127.0.0.1, 197.76.65.151}}, expected_icmp_handshakes: [], expected_dns_handshakes: [] }, [AddCidrResource(ResourceDescriptionCidr { id: ResourceId(00000000-0000-0000-0000-000000000000), address: V6(Ipv6Network { network_address: ::ffff:0.0.0.32, netmask: 123 }), name: "aaaa", address_description: "cufzrsghzi", sites: [Site { name: "ohhzezo", id: SiteId(7479b0e4-3f89-1235-4264-6f61cd64265d) }] }), AddCidrResource(ResourceDescriptionCidr { id: ResourceId(00000000-0000-0000-0000-000000000000), address: V4(Ipv4Network { network_address: 0.0.0.0, netmask: 32 }), name: "aaaa", address_description: "kiodmg", sites: [Site { name: "nsvy", id: SiteId(b629efc5-3490-c009-f713-d6fbbefcde14) }] }), RoamClient { ip4_socket: Some(127.0.0.1:1), ip6_socket: None }, SendICMPPacketToNonResourceIp { dst: 0.0.0.1, seq: 53260, identifier: 3682 }, SendICMPPacketToResource { idx: Index(0), seq: 0, identifier: 0, src: TunnelIp6 }, SendICMPPacketToResource { idx: Index(0), seq: 0, identifier: 0, src: TunnelIp4 }, SendICMPPacketToResource { idx: Index(0), seq: 1, identifier: 51615, src: TunnelIp4 }], None) +cc 4c15aa7f37217bce3154f4f6ca1a1579b461eb937c388d64831c66f663882f0c diff --git a/rust/connlib/tunnel/src/client.rs b/rust/connlib/tunnel/src/client.rs index 851352ec0..04432cacc 100644 --- a/rust/connlib/tunnel/src/client.rs +++ b/rust/connlib/tunnel/src/client.rs @@ -818,6 +818,15 @@ impl ClientState { resources: self.resources(), }); } + snownet::Event::ConnectionsCleared(ids) => { + for id in ids { + self.cleanup_connected_gateway(&id); + } + self.buffered_events + .push_back(ClientEvent::ResourcesChanged { + resources: self.resources(), + }); + } snownet::Event::NewIceCandidate { connection, candidate, @@ -863,8 +872,9 @@ impl ClientState { } pub(crate) fn reconnect(&mut self, now: Instant) { - tracing::info!("Network change detected, refreshing connections"); - self.node.reconnect(now) + tracing::info!("Network change detected"); + self.node.reconnect(now); + self.handle_timeout(now); // Ensure we process all events. } pub(crate) fn poll_transmit(&mut self) -> Option> { diff --git a/rust/connlib/tunnel/src/gateway.rs b/rust/connlib/tunnel/src/gateway.rs index e44e23818..da6323227 100644 --- a/rust/connlib/tunnel/src/gateway.rs +++ b/rust/connlib/tunnel/src/gateway.rs @@ -378,7 +378,8 @@ impl GatewayState { candidate, }); } - _ => {} + snownet::Event::ConnectionEstablished(_) + | snownet::Event::ConnectionsCleared(_) => {} } } } diff --git a/rust/connlib/tunnel/src/tests/reference.rs b/rust/connlib/tunnel/src/tests/reference.rs index 301f3a2e5..e94a95cb5 100644 --- a/rust/connlib/tunnel/src/tests/reference.rs +++ b/rust/connlib/tunnel/src/tests/reference.rs @@ -195,6 +195,7 @@ impl ReferenceStateMachine for ReferenceState { .prop_map(|servers| Transition::UpdateUpstreamDnsServers { servers }), ) .with(1, cidr_resource(8).prop_map(Transition::AddCidrResource)) + .with(1, roam_client()) .with( 1, prop_oneof![ @@ -363,6 +364,17 @@ impl ReferenceStateMachine for ReferenceState { Transition::UpdateUpstreamDnsServers { servers } => { state.upstream_dns_resolvers.clone_from(servers); } + Transition::RoamClient { + ip4_socket, + ip6_socket, + } => { + state.client.ip4_socket.clone_from(ip4_socket); + state.client.ip6_socket.clone_from(ip6_socket); + + // When roaming, we are not connected to any resource and wait for the next packet to re-establish a connection. + state.client_connected_cidr_resources.clear(); + state.client_connected_dns_resources.clear(); + } }; state @@ -480,6 +492,7 @@ impl ReferenceStateMachine for ReferenceState { state.client_cidr_resources.iter().any(|(_, r)| &r.id == id) || state.client_dns_resources.contains_key(id) } + Transition::RoamClient { .. } => true, } } } diff --git a/rust/connlib/tunnel/src/tests/sim_node.rs b/rust/connlib/tunnel/src/tests/sim_node.rs index 4a89a3e41..c41ae78ef 100644 --- a/rust/connlib/tunnel/src/tests/sim_node.rs +++ b/rust/connlib/tunnel/src/tests/sim_node.rs @@ -25,6 +25,8 @@ pub(crate) struct SimNode { pub(crate) ip4_socket: Option, pub(crate) ip6_socket: Option, + pub(crate) old_sockets: Vec, + pub(crate) tunnel_ip4: Ipv4Addr, pub(crate) tunnel_ip6: Ipv6Addr, @@ -48,6 +50,7 @@ impl SimNode { tunnel_ip4, tunnel_ip6, span: Span::none(), + old_sockets: Default::default(), } } } @@ -65,6 +68,7 @@ where ip6_socket: self.ip6_socket, tunnel_ip4: self.tunnel_ip4, tunnel_ip6: self.tunnel_ip6, + old_sockets: self.old_sockets.clone(), span, } } @@ -112,6 +116,31 @@ impl SimNode { self.state.remove_resources(&[resource]); }) } + + pub(crate) fn roam( + &mut self, + ip4_socket: Option, + ip6_socket: Option, + now: Instant, + ) { + // 1. Remember what the current sockets were. + self.old_sockets.extend(self.ip4_socket.map(SocketAddr::V4)); + self.old_sockets.extend(self.ip6_socket.map(SocketAddr::V6)); + + // 2. Update to the new sockets. + self.ip4_socket = ip4_socket; + self.ip6_socket = ip6_socket; + + // 3. Ensure our new sockets aren't present in old sockets (a client should be able to roam "back" to a previous network interface). + if let Some(s4) = self.ip4_socket.map(SocketAddr::V4) { + self.old_sockets.retain(|s| s != &s4); + } + if let Some(s6) = self.ip6_socket.map(SocketAddr::V6) { + self.old_sockets.retain(|s| s != &s6); + } + + self.state.reconnect(now); + } } impl SimNode { @@ -164,6 +193,7 @@ impl fmt::Debug for SimNode { .field("ip6_socket", &self.ip6_socket) .field("tunnel_ip4", &self.tunnel_ip4) .field("tunnel_ip6", &self.tunnel_ip6) + .field("old_sockets", &self.old_sockets) .finish() } } diff --git a/rust/connlib/tunnel/src/tests/sut.rs b/rust/connlib/tunnel/src/tests/sut.rs index 6d1004f77..93d3b7cfa 100644 --- a/rust/connlib/tunnel/src/tests/sut.rs +++ b/rust/connlib/tunnel/src/tests/sut.rs @@ -210,6 +210,10 @@ impl StateMachineTest for TunnelTest { Transition::UpdateUpstreamDnsServers { servers } => { state.client.update_upstream_dns(servers); } + Transition::RoamClient { + ip4_socket, + ip6_socket, + } => state.client.roam(ip4_socket, ip6_socket, state.now), }; state.advance(ref_state, &mut buffered_transmits); assert!(buffered_transmits.is_empty()); // Sanity check to ensure we handled all packets. @@ -503,6 +507,11 @@ impl TunnelTest { ) -> ControlFlow<()> { let mut buffer = [0u8; 2000]; + if self.client.old_sockets.contains(&dst) { + tracing::debug!("Dropping packet to {dst} because the client roamed away from this network interface"); + return ControlFlow::Break(()); + } + if !self.client.wants(dst) { return ControlFlow::Continue(()); } diff --git a/rust/connlib/tunnel/src/tests/transition.rs b/rust/connlib/tunnel/src/tests/transition.rs index cd1010d50..29f387ea6 100644 --- a/rust/connlib/tunnel/src/tests/transition.rs +++ b/rust/connlib/tunnel/src/tests/transition.rs @@ -11,7 +11,7 @@ use hickory_proto::rr::RecordType; use proptest::{prelude::*, sample}; use std::{ collections::{HashMap, HashSet}, - net::{IpAddr, SocketAddr}, + net::{IpAddr, SocketAddr, SocketAddrV4, SocketAddrV6}, }; /// The possible transitions of the state machine. @@ -72,6 +72,12 @@ pub(crate) enum Transition { /// Remove a resource from the client. RemoveResource(ResourceId), + + /// Roam the client to a new pair of sockets. + RoamClient { + ip4_socket: Option, + ip6_socket: Option, + }, } pub(crate) fn ping_random_ip( @@ -206,3 +212,22 @@ pub(crate) fn question_mark_wildcard_dns_resource() -> impl Strategy impl Strategy { + ( + firezone_relay::proptest::any_ip_stack(), // We are re-using the strategy here because it is exactly what we need although we are generating a node here and not a relay. + any::().prop_filter("port must not be 0", |p| *p != 0), + any::().prop_filter("port must not be 0", |p| *p != 0), + ) + .prop_map(move |(ip_stack, v4_port, v6_port)| { + let ip4_socket = ip_stack.as_v4().map(|ip| SocketAddrV4::new(*ip, v4_port)); + let ip6_socket = ip_stack + .as_v6() + .map(|ip| SocketAddrV6::new(*ip, v6_port, 0, 0)); + + Transition::RoamClient { + ip4_socket, + ip6_socket, + } + }) +} diff --git a/rust/snownet-tests/src/main.rs b/rust/snownet-tests/src/main.rs index ec4d39424..6ffb9029e 100644 --- a/rust/snownet-tests/src/main.rs +++ b/rust/snownet-tests/src/main.rs @@ -398,7 +398,11 @@ impl Eventloop { Some(snownet::Event::ConnectionFailed(conn)) => { return Poll::Ready(Ok(Event::ConnectionFailed { conn })) } - Some(snownet::Event::InvalidateIceCandidate { .. }) | None => {} + Some( + snownet::Event::InvalidateIceCandidate { .. } + | snownet::Event::ConnectionsCleared { .. }, + ) + | None => {} } if let Poll::Ready(Some(wire::Candidate { conn, candidate })) = diff --git a/scripts/tests/direct-download-roaming-network.sh b/scripts/tests/direct-download-roaming-network.sh index 10c283d6a..292d2c2e4 100755 --- a/scripts/tests/direct-download-roaming-network.sh +++ b/scripts/tests/direct-download-roaming-network.sh @@ -3,11 +3,15 @@ source "./scripts/tests/lib.sh" # Download 10MB at a max rate of 1MB/s. Shouldn't take longer than 12 seconds (allows for 2s of restablishing) -docker compose exec -it client sh -c \ +client sh -c \ "curl \ --fail \ --max-time 12 \ - --limit-rate 1M http://download.httpbin/bytes?num=10000000" > download.file & + --retry 1 \ + --continue-at - \ + --limit-rate 1M \ + --output download.file \ + http://download.httpbin/bytes?num=10000000" & DOWNLOAD_PID=$! @@ -27,7 +31,7 @@ wait $DOWNLOAD_PID || { } known_checksum="f5e02aa71e67f41d79023a128ca35bad86cf7b6656967bfe0884b3a3c4325eaf" -computed_checksum=$(sha256sum download.file | awk '{ print $1 }') +computed_checksum=$(client sha256sum download.file | awk '{ print $1 }') if [[ "$computed_checksum" != "$known_checksum" ]]; then echo "Checksum of downloaded file does not match"