From 3c1a35ea00ee5a328f35e9565db7e38d08c30a71 Mon Sep 17 00:00:00 2001 From: ddidderr Date: Fri, 22 May 2026 04:57:10 +0200 Subject: [PATCH] fix(client): enforce virtual MAC before relay send The relay already rejects client frames whose source MAC does not match the announced virtual MAC. The Windows bridge can still see those frames from TAP, though, and sending them to the relay wastes datagram budget and makes the client-side counters less useful during manual tests. Carry the configured virtual MAC into ClientRelayIo and drop invalid or unauthorized TAP source MACs before QUIC DATAGRAM encoding. The relay keeps the same checks as the trust boundary, but client diagnostics now account for these drops locally. Document the local source-MAC check and list InvalidSourceMac as a suspicious manual-test drop reason. Test Plan: - cargo fmt --check - cargo test -p lanparty-client-core connects_to_relay_control_stream_as_client - cargo test --workspace - cargo clippy --workspace --all-targets -- -D warnings - git diff --check Refs: PLAN.md source-MAC authorization and safety filters --- README.md | 14 +++++--- TESTING.md | 1 + crates/lanparty-client-core/src/lib.rs | 48 ++++++++++++++++++++++++-- 3 files changed, 55 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index 1c7665e..7745fd2 100644 --- a/README.md +++ b/README.md @@ -61,8 +61,8 @@ Platform-neutral remote client relay session: - QUIC DATAGRAM support and negotiated datagram budget diagnostics - relay RTT diagnostics from the active QUIC connection - reliable relay control-event reads for peer lifecycle messages -- Ethernet frame send/receive helpers over QUIC DATAGRAM with budget checks and - local drop outcomes for malformed or oversized sends +- Ethernet frame send/receive helpers over QUIC DATAGRAM with budget and source + MAC checks plus local drop outcomes for malformed or oversized sends - client tunnel statistics for frame/datagram rx/tx and drops - reliable client stats snapshot sends for relay diagnostics - best-effort graceful disconnect messages before QUIC close @@ -220,6 +220,9 @@ the first TAP-Windows6 adapter until shutdown. `--relay` accepts a DNS name or socket address; bare hosts default to UDP/443. Before opening the adapter for bridging, it writes the generated tunnel MAC to the TAP driver's `NetworkAddress` registry setting. +TAP frames whose source MAC does not match that generated tunnel MAC are +dropped locally before they can consume relay bandwidth; the relay still +enforces the same source-MAC rule. If the exact relay host route already exists, the client uses it and leaves it alone on exit. The startup status reports whether the relay already has a LAN gateway for the room. @@ -248,9 +251,10 @@ after bridging starts become visible in later status lines. Each snapshot also emits short user-facing lines such as relay/gateway connection status, relay-route and TAP readiness warnings, DHCP address presence, relay RTT, and broadcast-flow confirmation when those signals are observed. Malformed frames -read from TAP, jumbo frames, and TAP frames whose encoded datagrams exceed the -negotiated QUIC budget are counted and dropped before relay send without -stopping the bridge; TAP device read/write errors still stop the bridge. +read from TAP, invalid or unauthorized source-MAC frames, jumbo frames, and TAP +frames whose encoded datagrams exceed the negotiated QUIC budget are counted and +dropped before relay send without stopping the bridge; TAP device read/write +errors still stop the bridge. Relay lifecycle events are logged as they arrive, including gateway joins and peer leaves. The client remembers peer identities from join and catch-up events so later leave logs can identify a disconnected LAN gateway or client MAC when diff --git a/TESTING.md b/TESTING.md index 3ff356f..043a067 100644 --- a/TESTING.md +++ b/TESTING.md @@ -211,6 +211,7 @@ Drops that should be investigated if they dominate: ```text drop_reason=Malformed +drop_reason=InvalidSourceMac drop_reason=UnauthorizedSourceMac drop_reason=ControlPlaneEtherType ``` diff --git a/crates/lanparty-client-core/src/lib.rs b/crates/lanparty-client-core/src/lib.rs index 403120c..516079b 100644 --- a/crates/lanparty-client-core/src/lib.rs +++ b/crates/lanparty-client-core/src/lib.rs @@ -276,6 +276,7 @@ impl ClientSession { self.connection.clone(), self.welcome.clone(), self.quic_max_datagram_size, + self.config.virtual_mac(), Arc::clone(&self.stats), ) } @@ -320,6 +321,7 @@ pub struct ClientRelayIo { connection: quinn::Connection, welcome: ServerWelcome, quic_max_datagram_size: u16, + virtual_mac: MacAddr, stats: Arc, } @@ -329,12 +331,14 @@ impl ClientRelayIo { connection: quinn::Connection, welcome: ServerWelcome, quic_max_datagram_size: u16, + virtual_mac: MacAddr, stats: Arc, ) -> Self { Self { connection, welcome, quic_max_datagram_size, + virtual_mac, stats, } } @@ -349,6 +353,11 @@ impl ClientRelayIo { self.quic_max_datagram_size } + #[must_use] + pub const fn virtual_mac(&self) -> MacAddr { + self.virtual_mac + } + pub fn send_ethernet(&self, frame: &[u8]) -> Result<()> { match self.send_ethernet_with_outcome(frame)? { ClientSendOutcome::Sent => Ok(()), @@ -376,6 +385,16 @@ impl ClientRelayIo { self.stats.record_dropped_frame(); return Ok(ClientSendOutcome::Dropped(DropReason::JumboFrame)); } + if !ethernet_frame.source().is_valid_unicast() { + self.stats.record_dropped_frame(); + return Ok(ClientSendOutcome::Dropped(DropReason::InvalidSourceMac)); + } + if ethernet_frame.source() != self.virtual_mac { + self.stats.record_dropped_frame(); + return Ok(ClientSendOutcome::Dropped( + DropReason::UnauthorizedSourceMac, + )); + } let datagram = encode_datagram( FrameType::Ethernet, @@ -805,7 +824,7 @@ mod tests { let ControlMessage::Stats(stats) = stats_message else { panic!("expected client stats event"); }; - assert_eq!(stats, TunnelStats::new(1, 1, 1, 1, 2, 1)); + assert_eq!(stats, TunnelStats::new(1, 1, 1, 1, 4, 1)); stats_received_tx.send(()).unwrap(); let mut disconnect_recv = connection.accept_uni().await.unwrap(); @@ -856,6 +875,7 @@ mod tests { relay_io.quic_max_datagram_size(), client.quic_max_datagram_size() ); + assert_eq!(relay_io.virtual_mac(), client.config().virtual_mac()); assert_eq!( relay_io @@ -891,6 +911,24 @@ mod tests { relay_io.send_ethernet_with_outcome(&[0; 4]).unwrap(), ClientSendOutcome::Dropped(DropReason::Malformed) ); + assert_eq!( + relay_io + .send_ethernet_with_outcome(ðernet_frame_from( + MacAddr::new([0x03, 0, 0, 0, 0, 9]), + b"invalid source" + )) + .unwrap(), + ClientSendOutcome::Dropped(DropReason::InvalidSourceMac) + ); + assert_eq!( + relay_io + .send_ethernet_with_outcome(ðernet_frame_from( + MacAddr::new([0x02, 0, 0, 0, 0, 9]), + b"forged source" + )) + .unwrap(), + ClientSendOutcome::Dropped(DropReason::UnauthorizedSourceMac) + ); let stats = relay_io.stats_snapshot(); assert_eq!(stats.ethernet_frames_tx(), 1); assert_eq!(stats.ethernet_frames_rx(), 1); @@ -898,7 +936,7 @@ mod tests { assert_eq!(stats.broadcast_frames_rx(), 0); assert_eq!(stats.datagrams_tx(), 1); assert_eq!(stats.datagrams_rx(), 1); - assert_eq!(stats.dropped_frames(), 2); + assert_eq!(stats.dropped_frames(), 4); assert_eq!(stats.malformed_frames(), 1); assert_eq!(client.stats_snapshot(), stats); @@ -970,9 +1008,13 @@ mod tests { } fn ethernet_frame(payload: &[u8]) -> Vec { + ethernet_frame_from(MacAddr::new([0x02, 0, 0, 0, 0, 1]), payload) + } + + fn ethernet_frame_from(source: MacAddr, payload: &[u8]) -> Vec { let mut frame = Vec::new(); frame.extend_from_slice(&[0x02, 0, 0, 0, 0, 2]); - frame.extend_from_slice(&[0x02, 0, 0, 0, 0, 1]); + frame.extend_from_slice(&source.octets()); frame.extend_from_slice(&0x0800_u16.to_be_bytes()); frame.extend_from_slice(payload); frame