From fc1285839227a9ef305f290f34f2f519ae3de5fb Mon Sep 17 00:00:00 2001 From: Greg Lamberson Date: Sun, 13 Sep 2026 10:15:16 -0500 Subject: [PATCH 1/3] fix(rdpeudp): accept version 1/2 SYN offers on the server side too accept() rejected any client offering a protocol version below 3 outright, with a comment noting this crate only implemented the MS-RDPEUDP2 (version 3) data transfer. That implementation gap closed for the client-connects-out role (ConnectionConfig::offer_version, connect()'s SYN, handle_syn_ack()'s negotiation), but accept() itself, the server-accept role, was never updated to match: every real-world Windows client I have observed offers version 1 or 2 in its SYN, so a server built on this crate could never actually negotiate the sideband UDP transport with it, only ever falling back to the main transport. Mirrors handle_syn_ack()'s already-proven version selection exactly: settle on the client's offered version when it's 1 or 2, otherwise settle on our own highest (3), per MS-RDPEUDP 1.7's negotiate-down MUST clause; select WireFormat via UdpVersion::uses_v2_wire_format() the same way. enqueue_syn_ack() now echoes the negotiated version instead of unconditionally claiming 3. Also fixes an adjacent gap in the same code: 3.1.5.1.1 says an invalid cookieHash on a version 3 SYN MUST drop the connection to version 2 rather than refuse it, which the crate could not do until now either; it does that too. Rewrote the two existing tests that encoded the old refuse-on-low- version behavior to assert the new negotiate-down one instead, and added coverage for: settling on version 1, rejecting a genuinely unrecognized low version (there is nothing to negotiate down to below 1), and a full handshake plus bidirectional data exchange with the server on the accept side settling on version 2, exercising the already-existing version 1/2 data path (handle_v1_datagram and its neighbors) from the server-accept role for the first time. --- crates/ironrdp-rdpeudp/src/connection.rs | 71 +++++---- .../tests/rdpeudp/connection.rs | 136 +++++++++++++++++- 2 files changed, 174 insertions(+), 33 deletions(-) diff --git a/crates/ironrdp-rdpeudp/src/connection.rs b/crates/ironrdp-rdpeudp/src/connection.rs index 5051a44e6..adb0db0fd 100644 --- a/crates/ironrdp-rdpeudp/src/connection.rs +++ b/crates/ironrdp-rdpeudp/src/connection.rs @@ -512,36 +512,48 @@ impl RdpeudpConnection { ) })?; - // Only version 3 selects the MS-RDPEUDP2 data transfer (1.3.2.2), and - // that is the only data transfer this crate implements, so a client - // offering version 1 or 2 (asking for the MS-RDPEUDP one) cannot be - // served. A client offering something above version 3 can: MS-RDPEUDP - // 1.7's negotiate-down MUST clause requires settling on our own - // highest supported version rather than refusing the connection, and - // `enqueue_syn_ack` below always answers with version 3 regardless of - // what was offered, which is exactly that settlement. - if syn_data_ex.udp_ver.0 < UdpVersion::V3.0 { + // MS-RDPEUDP 1.7's negotiate-down MUST clause: a remote offering + // anything at or above our own highest supported version (3, + // including a value this crate does not otherwise recognize) settles + // on that highest version rather than being refused. A remote + // offering exactly version 1 or 2 settles there too, now that this + // crate implements the MS-RDPEUDP data transfer alongside MS-RDPEUDP2. + // Anything else (an offer below 3 that is not exactly 1 or 2) is not + // a version this crate can serve. + let mut negotiated_version = if syn_data_ex.udp_ver.0 >= UdpVersion::V3.0 { + UdpVersion::V3 + } else if syn_data_ex.udp_ver == UdpVersion::V1 || syn_data_ex.udp_ver == UdpVersion::V2 { + syn_data_ex.udp_ver + } else { return Err(RdpeudpError::invalid_packet( "accept", - "remote offered a protocol version below 3, whose data transfer is MS-RDPEUDP rather than MS-RDPEUDP2", + "remote offered a protocol version this crate does not implement", )); - } - - // 3.1.5.1.1 asks the server to confirm the hash, and says an invalid - // one MUST drop the connection back to version 2. That version means - // the MS-RDPEUDP data transfer, which this crate does not implement, - // so the only honest outcome is to refuse the connection. - let offered_hash = syn_data_ex - .cookie_hash - .ok_or_else(|| RdpeudpError::invalid_packet("accept", "version 3 SYN carries no cookieHash"))?; + }; - if offered_hash != expected_hash { - return Err(RdpeudpError::invalid_packet( - "accept", - "cookieHash does not match the security cookie for this multitransport request", - )); + // 2.2.2.9: cookieHash accompanies a version 3 SYN and "MUST NOT be + // present in any other case", so a version 1 or 2 offer has none to + // check. 3.1.5.1.1 asks the server to confirm the hash on a version 3 + // SYN and says an invalid one MUST drop the connection to version 2 + // rather than refuse it. + if negotiated_version == UdpVersion::V3 { + let offered_hash = syn_data_ex + .cookie_hash + .ok_or_else(|| RdpeudpError::invalid_packet("accept", "version 3 SYN carries no cookieHash"))?; + + if offered_hash != expected_hash { + negotiated_version = UdpVersion::V2; + } } + let wire = if negotiated_version.uses_v2_wire_format() { + WireFormat::V2 + } else { + WireFormat::V1 { + version: negotiated_version.0, + } + }; + let mut conn = Self::new(Side::Server, config); conn.state = State::SynReceived; @@ -561,10 +573,10 @@ impl RdpeudpConnection { remote_isn, mtu, log_window_size: conn.config.log_window_size, - wire: WireFormat::V2, + wire, }); - conn.enqueue_syn_ack(remote_isn, now); + conn.enqueue_syn_ack(remote_isn, negotiated_version, now); conn.timers.set(Timer::Idle, now + conn.config.idle_timeout); Ok(conn) @@ -1043,7 +1055,7 @@ impl RdpeudpConnection { } /// Build and enqueue the server SYN+ACK datagram. - fn enqueue_syn_ack(&mut self, remote_isn: u32, now: MonotonicInstant) { + fn enqueue_syn_ack(&mut self, remote_isn: u32, negotiated_version: UdpVersion, now: MonotonicInstant) { let datagram = V1Datagram { header: FecHeader { sn_source_ack: remote_isn, @@ -1062,7 +1074,10 @@ impl RdpeudpConnection { correlation_id: None, syn_data_ex: Some(SynDataExPayload { syn_ex_flags: SynExFlags::VERSION_INFO_VALID, - udp_ver: UdpVersion::V3, + // 3.1.5.1.1: "the highest version supported by both + // endpoints", per the negotiate-down decision `accept` above + // already made; not unconditionally our own maximum. + udp_ver: negotiated_version, // 2.2.2.9 puts the hash in the client's SYN and nowhere else: // "It MUST NOT be present in any other case." cookie_hash: None, diff --git a/crates/ironrdp-testsuite-core/tests/rdpeudp/connection.rs b/crates/ironrdp-testsuite-core/tests/rdpeudp/connection.rs index c100c37c4..776d81fb2 100644 --- a/crates/ironrdp-testsuite-core/tests/rdpeudp/connection.rs +++ b/crates/ironrdp-testsuite-core/tests/rdpeudp/connection.rs @@ -150,8 +150,79 @@ fn full_handshake_client_server() { assert_eq!(event, Event::Connected); } +/// End-to-end handshake AND data exchange for the server-accept role +/// settling on version 2, exercising the already-merged MS-RDPEUDP (version +/// 1/2) data path (`handle_v1_datagram` and friends) from the server-accept +/// side rather than only the client-connects-out side those functions were +/// originally tested from. #[test] -fn server_rejects_non_v2_syn() { +fn full_handshake_and_data_exchange_when_the_server_settles_on_version_2() { + let t = now(); + + let mut client_config = default_config(100); + client_config.offer_version = UdpVersion::V2; + client_config.cookie_hash = None; + + let mut client = RdpeudpConnection::connect(client_config, t).expect("connect"); + let syn_transmit = client.poll_transmit(t).expect("client SYN"); + + let syn_datagram: V1Datagram = decode(&syn_transmit.contents).expect("decode SYN"); + let mut server = RdpeudpConnection::accept(default_config(200), &syn_datagram, t).expect("accept"); + let syn_ack_transmit = server.poll_transmit(t).expect("server SYN+ACK"); + + let mut syn_ack_bytes = syn_ack_transmit.contents; + client + .handle_datagram(&mut syn_ack_bytes, later(t, 50)) + .expect("handle SYN+ACK"); + assert!(client.is_established()); + + let ack_transmit = client.poll_transmit(later(t, 50)).expect("client final ACK"); + let mut ack_bytes = ack_transmit.contents; + server + .handle_datagram(&mut ack_bytes, later(t, 100)) + .expect("handle final ACK"); + assert!(server.is_established()); + + // Drain each side's own Connected event before checking for data, since + // poll_event() returns queued events in order. + assert_eq!(client.poll_event().expect("client Connected event"), Event::Connected); + assert_eq!(server.poll_event().expect("server Connected event"), Event::Connected); + + // Both sides settled on the MS-RDPEUDP (version 1/2) data path, not + // MS-RDPEUDP2, confirmed via the same public diagnostic a real caller + // would use to tell which wire format is in effect. + assert_eq!( + client.v1_stats().expect("client on v1/v2 wire").version, + UdpVersion::V2.0 + ); + assert_eq!( + server.v1_stats().expect("server on v1/v2 wire").version, + UdpVersion::V2.0 + ); + + // Data actually flows both ways through the version-1/2 data path with + // the server on the accept side, not just the SYN/SYN+ACK/ACK handshake. + client.send(b"hello from client".to_vec()).expect("client send"); + let client_data_transmit = client.poll_transmit(later(t, 150)).expect("client data"); + let mut client_data_bytes = client_data_transmit.contents; + server + .handle_datagram(&mut client_data_bytes, later(t, 200)) + .expect("server handles client data"); + let received = server.poll_event().expect("server should have data event"); + assert_eq!(received, Event::DataReceived(b"hello from client".to_vec())); + + server.send(b"hello from server".to_vec()).expect("server send"); + let server_data_transmit = server.poll_transmit(later(t, 250)).expect("server data"); + let mut server_data_bytes = server_data_transmit.contents; + client + .handle_datagram(&mut server_data_bytes, later(t, 300)) + .expect("client handles server data"); + let received = client.poll_event().expect("client should have data event"); + assert_eq!(received, Event::DataReceived(b"hello from server".to_vec())); +} + +#[test] +fn server_accepts_and_settles_on_version_1_for_a_syn_offering_it() { let t = now(); let syn = V1Datagram { @@ -170,12 +241,50 @@ fn server_rejects_non_v2_syn() { correlation_id: None, syn_data_ex: Some(SynDataExPayload { syn_ex_flags: SynExFlags::VERSION_INFO_VALID, + // 2.2.2.9: cookieHash MUST NOT be present outside a version 3 SYN. udp_ver: UdpVersion::V1, cookie_hash: None, }), data: None, }; + let mut conn = RdpeudpConnection::accept(default_config(200), &syn, t).expect("version 1 is now implemented"); + + let transmit = conn.poll_transmit(t).expect("should have SYN+ACK"); + let datagram: V1Datagram = decode(&transmit.contents).expect("valid SYN+ACK"); + let syn_data_ex = datagram.syn_data_ex.expect("SYN+ACK carries SynDataEx"); + assert_eq!(syn_data_ex.udp_ver, UdpVersion::V1); + assert_eq!(syn_data_ex.cookie_hash, None); +} + +#[test] +fn server_rejects_a_syn_offering_a_version_below_1() { + let t = now(); + + let syn = V1Datagram { + header: FecHeader { + sn_source_ack: 0xFFFF_FFFF, + receive_window_size: 64, + flags: V1Flags::SYN | V1Flags::SYNEX, + }, + ack_vector: None, + ack_of_acks: None, + syn_data: Some(SynDataPayload { + initial_sequence_number: 100, + upstream_mtu: 1232, + downstream_mtu: 1232, + }), + correlation_id: None, + syn_data_ex: Some(SynDataExPayload { + syn_ex_flags: SynExFlags::VERSION_INFO_VALID, + // 0x0000 is not a version this or any known MS-RDPEUDP + // implementation speaks; there is nothing to negotiate down to. + udp_ver: UdpVersion(0x0000), + cookie_hash: None, + }), + data: None, + }; + let result = RdpeudpConnection::accept(default_config(200), &syn, t); assert!(result.is_err()); } @@ -1126,7 +1235,7 @@ fn connect_refuses_to_build_a_version_3_syn_without_a_cookie_hash() { } #[test] -fn a_server_rejects_a_syn_offering_a_version_below_3() { +fn a_server_accepts_and_settles_on_the_version_a_syn_offers_below_3() { let t = now(); let syn = V1Datagram { @@ -1146,17 +1255,26 @@ fn a_server_rejects_a_syn_offering_a_version_below_3() { syn_data_ex: Some(SynDataExPayload { syn_ex_flags: SynExFlags::VERSION_INFO_VALID, // 0x0002 means the MS-RDPEUDP data transfer, not MS-RDPEUDP2. + // 2.2.2.9: cookieHash MUST NOT be present outside a version 3 SYN. udp_ver: UdpVersion::V2, cookie_hash: None, }), data: None, }; - RdpeudpConnection::accept(default_config(200), &syn, t).expect_err("version 2 is not RDPEUDP2"); + let mut conn = RdpeudpConnection::accept(default_config(200), &syn, t).expect("version 2 is now implemented"); + + // The SYN+ACK echoes the negotiated version back, per 3.1.5.1.1's "highest + // version supported by both endpoints", not unconditionally our maximum. + let transmit = conn.poll_transmit(t).expect("should have SYN+ACK"); + let datagram: V1Datagram = decode(&transmit.contents).expect("valid SYN+ACK"); + let syn_data_ex = datagram.syn_data_ex.expect("SYN+ACK carries SynDataEx"); + assert_eq!(syn_data_ex.udp_ver, UdpVersion::V2); + assert_eq!(syn_data_ex.cookie_hash, None); } #[test] -fn a_server_rejects_a_syn_whose_cookie_hash_does_not_match() { +fn a_server_downgrades_to_version_2_on_a_syn_whose_cookie_hash_does_not_match() { let t = now(); let syn = V1Datagram { @@ -1181,7 +1299,15 @@ fn a_server_rejects_a_syn_whose_cookie_hash_does_not_match() { data: None, }; - RdpeudpConnection::accept(default_config(200), &syn, t).expect_err("the hash is for a different cookie"); + // 3.1.5.1.1: an invalid cookieHash on a version 3 SYN MUST drop the + // connection to version 2, not refuse it outright. + let mut conn = + RdpeudpConnection::accept(default_config(200), &syn, t).expect("a mismatched hash downgrades, not fails"); + + let transmit = conn.poll_transmit(t).expect("should have SYN+ACK"); + let datagram: V1Datagram = decode(&transmit.contents).expect("valid SYN+ACK"); + let syn_data_ex = datagram.syn_data_ex.expect("SYN+ACK carries SynDataEx"); + assert_eq!(syn_data_ex.udp_ver, UdpVersion::V2); } /// A SYN+ACK from a server that only speaks MS-RDPEUDP, answering a client SYN From 4667b680b15b2f5c4cd86dbbbef5da094d6d5585 Mon Sep 17 00:00:00 2001 From: Greg Lamberson Date: Sun, 13 Sep 2026 12:16:53 -0500 Subject: [PATCH 2/3] review: dedupe the version-to-wire-format mapping into one helper accept() and handle_syn_ack() each independently reimplemented the same negotiated-version-to-WireFormat mapping. Both call sites only ever see a version already restricted to V1/V2/V3, so a private WireFormat::for_version helper is total over every reachable value and behavior-preserving. --- crates/ironrdp-rdpeudp/src/connection.rs | 25 ++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/crates/ironrdp-rdpeudp/src/connection.rs b/crates/ironrdp-rdpeudp/src/connection.rs index adb0db0fd..d7de1886c 100644 --- a/crates/ironrdp-rdpeudp/src/connection.rs +++ b/crates/ironrdp-rdpeudp/src/connection.rs @@ -252,6 +252,17 @@ enum WireFormat { V2, } +impl WireFormat { + /// The framing a negotiated `UdpVersion` selects, per `uses_v2_wire_format()`. + fn for_version(version: UdpVersion) -> Self { + if version.uses_v2_wire_format() { + Self::V2 + } else { + Self::V1 { version: version.0 } + } + } +} + #[derive(Debug, Clone)] struct NegotiatedParams { /// Our ISN (from our SYN). @@ -546,13 +557,7 @@ impl RdpeudpConnection { } } - let wire = if negotiated_version.uses_v2_wire_format() { - WireFormat::V2 - } else { - WireFormat::V1 { - version: negotiated_version.0, - } - }; + let wire = WireFormat::for_version(negotiated_version); let mut conn = Self::new(Side::Server, config); conn.state = State::SynReceived; @@ -1181,11 +1186,7 @@ impl RdpeudpConnection { "SYN+ACK selected a protocol version above the one the SYN offered", )); } - let wire = if selected.uses_v2_wire_format() { - WireFormat::V2 - } else { - WireFormat::V1 { version: selected.0 } - }; + let wire = WireFormat::for_version(selected); let local_isn = self.config.initial_sequence_number; let remote_isn = syn_data.initial_sequence_number; From 60cb7d92cd1894c9a3dc09d965a69e13e913b30e Mon Sep 17 00:00:00 2001 From: Greg Lamberson Date: Wed, 16 Sep 2026 19:28:21 -0500 Subject: [PATCH 3/3] test(rdpeudp): dedupe handshake choreography and the hand-built SYN literal full_handshake_and_data_exchange_when_the_server_settles_on_version_2 re-enacted establish_pair()'s SYN -> accept -> SYN+ACK -> final-ACK exchange by hand, differing only in the client config. Extracted handshake(client_config, server_isn), which drives the exchange through the final ACK without draining either side's event queue (poll_event() is a separate queue from poll_transmit(), so callers are free to drain or assert on it however they need). establish_pair() is now a thin wrapper over handshake(default_config(100), 200) that also drains both sides' events, keeping its existing no-argument signature intact for its ~20 existing call sites. Seven tests hand-built the same ~20-line V1Datagram SYN literal, differing only in syn_data_ex's udp_ver and cookie_hash. Extracted syn_datagram(udp_ver, cookie_hash), folding test_syn_data_ex() into a call with UdpVersion::V3 and TEST_COOKIE_HASH. Per-site spec citation comments moved to the call site they annotate. No behavior change: same assertions, same wire bytes, same timing. --- .../tests/rdpeudp/connection.rs | 232 ++++-------------- 1 file changed, 54 insertions(+), 178 deletions(-) diff --git a/crates/ironrdp-testsuite-core/tests/rdpeudp/connection.rs b/crates/ironrdp-testsuite-core/tests/rdpeudp/connection.rs index 776d81fb2..34fb1a562 100644 --- a/crates/ironrdp-testsuite-core/tests/rdpeudp/connection.rs +++ b/crates/ironrdp-testsuite-core/tests/rdpeudp/connection.rs @@ -27,13 +27,29 @@ fn default_config(isn: u32) -> ConnectionConfig { } } -/// A client SYN as `connect` builds it, for tests that drive the server side -/// by hand. -fn test_syn_data_ex() -> SynDataExPayload { - SynDataExPayload { - syn_ex_flags: SynExFlags::VERSION_INFO_VALID, - udp_ver: UdpVersion::V3, - cookie_hash: Some(TEST_COOKIE_HASH), +/// A client SYN offering `udp_ver`. `cookie_hash` is present only alongside +/// version 3 (2.2.2.9: "cookieHash MUST NOT be present in any other case"). +fn syn_datagram(udp_ver: UdpVersion, cookie_hash: Option<[u8; 32]>) -> V1Datagram { + V1Datagram { + header: FecHeader { + sn_source_ack: 0xFFFF_FFFF, + receive_window_size: 64, + flags: V1Flags::SYN | V1Flags::SYNEX, + }, + ack_vector: None, + ack_of_acks: None, + syn_data: Some(SynDataPayload { + initial_sequence_number: 100, + upstream_mtu: 1232, + downstream_mtu: 1232, + }), + correlation_id: None, + syn_data_ex: Some(SynDataExPayload { + syn_ex_flags: SynExFlags::VERSION_INFO_VALID, + udp_ver, + cookie_hash, + }), + data: None, } } @@ -73,23 +89,7 @@ fn server_accept_produces_syn_ack() { let t = now(); // Build a client SYN - let client_syn = V1Datagram { - header: FecHeader { - sn_source_ack: 0xFFFF_FFFF, - receive_window_size: 64, - flags: V1Flags::SYN | V1Flags::SYNEX, - }, - ack_vector: None, - ack_of_acks: None, - syn_data: Some(SynDataPayload { - initial_sequence_number: 100, - upstream_mtu: 1232, - downstream_mtu: 1232, - }), - correlation_id: None, - syn_data_ex: Some(test_syn_data_ex()), - data: None, - }; + let client_syn = syn_datagram(UdpVersion::V3, Some(TEST_COOKIE_HASH)); let mut conn = RdpeudpConnection::accept(default_config(200), &client_syn, t).expect("accept"); @@ -157,30 +157,12 @@ fn full_handshake_client_server() { /// originally tested from. #[test] fn full_handshake_and_data_exchange_when_the_server_settles_on_version_2() { - let t = now(); - let mut client_config = default_config(100); client_config.offer_version = UdpVersion::V2; client_config.cookie_hash = None; - let mut client = RdpeudpConnection::connect(client_config, t).expect("connect"); - let syn_transmit = client.poll_transmit(t).expect("client SYN"); - - let syn_datagram: V1Datagram = decode(&syn_transmit.contents).expect("decode SYN"); - let mut server = RdpeudpConnection::accept(default_config(200), &syn_datagram, t).expect("accept"); - let syn_ack_transmit = server.poll_transmit(t).expect("server SYN+ACK"); - - let mut syn_ack_bytes = syn_ack_transmit.contents; - client - .handle_datagram(&mut syn_ack_bytes, later(t, 50)) - .expect("handle SYN+ACK"); + let (mut client, mut server, t) = handshake(client_config, 200); assert!(client.is_established()); - - let ack_transmit = client.poll_transmit(later(t, 50)).expect("client final ACK"); - let mut ack_bytes = ack_transmit.contents; - server - .handle_datagram(&mut ack_bytes, later(t, 100)) - .expect("handle final ACK"); assert!(server.is_established()); // Drain each side's own Connected event before checking for data, since @@ -225,28 +207,8 @@ fn full_handshake_and_data_exchange_when_the_server_settles_on_version_2() { fn server_accepts_and_settles_on_version_1_for_a_syn_offering_it() { let t = now(); - let syn = V1Datagram { - header: FecHeader { - sn_source_ack: 0xFFFF_FFFF, - receive_window_size: 64, - flags: V1Flags::SYN | V1Flags::SYNEX, - }, - ack_vector: None, - ack_of_acks: None, - syn_data: Some(SynDataPayload { - initial_sequence_number: 100, - upstream_mtu: 1232, - downstream_mtu: 1232, - }), - correlation_id: None, - syn_data_ex: Some(SynDataExPayload { - syn_ex_flags: SynExFlags::VERSION_INFO_VALID, - // 2.2.2.9: cookieHash MUST NOT be present outside a version 3 SYN. - udp_ver: UdpVersion::V1, - cookie_hash: None, - }), - data: None, - }; + // 2.2.2.9: cookieHash MUST NOT be present outside a version 3 SYN. + let syn = syn_datagram(UdpVersion::V1, None); let mut conn = RdpeudpConnection::accept(default_config(200), &syn, t).expect("version 1 is now implemented"); @@ -261,29 +223,9 @@ fn server_accepts_and_settles_on_version_1_for_a_syn_offering_it() { fn server_rejects_a_syn_offering_a_version_below_1() { let t = now(); - let syn = V1Datagram { - header: FecHeader { - sn_source_ack: 0xFFFF_FFFF, - receive_window_size: 64, - flags: V1Flags::SYN | V1Flags::SYNEX, - }, - ack_vector: None, - ack_of_acks: None, - syn_data: Some(SynDataPayload { - initial_sequence_number: 100, - upstream_mtu: 1232, - downstream_mtu: 1232, - }), - correlation_id: None, - syn_data_ex: Some(SynDataExPayload { - syn_ex_flags: SynExFlags::VERSION_INFO_VALID, - // 0x0000 is not a version this or any known MS-RDPEUDP - // implementation speaks; there is nothing to negotiate down to. - udp_ver: UdpVersion(0x0000), - cookie_hash: None, - }), - data: None, - }; + // 0x0000 is not a version this or any known MS-RDPEUDP implementation + // speaks; there is nothing to negotiate down to. + let syn = syn_datagram(UdpVersion(0x0000), None); let result = RdpeudpConnection::accept(default_config(200), &syn, t); assert!(result.is_err()); @@ -297,27 +239,7 @@ fn server_rejects_a_syn_offering_a_version_below_1() { fn server_accepts_and_settles_on_v3_for_an_unrecognized_higher_version() { let t = now(); - let syn = V1Datagram { - header: FecHeader { - sn_source_ack: 0xFFFF_FFFF, - receive_window_size: 64, - flags: V1Flags::SYN | V1Flags::SYNEX, - }, - ack_vector: None, - ack_of_acks: None, - syn_data: Some(SynDataPayload { - initial_sequence_number: 100, - upstream_mtu: 1232, - downstream_mtu: 1232, - }), - correlation_id: None, - syn_data_ex: Some(SynDataExPayload { - syn_ex_flags: SynExFlags::VERSION_INFO_VALID, - udp_ver: UdpVersion(0x0102), - cookie_hash: Some(TEST_COOKIE_HASH), - }), - data: None, - }; + let syn = syn_datagram(UdpVersion(0x0102), Some(TEST_COOKIE_HASH)); let mut conn = RdpeudpConnection::accept(default_config(200), &syn, t).expect("accept"); let transmit = conn.poll_transmit(t).expect("SYN+ACK"); @@ -378,23 +300,7 @@ fn accept_rejects_an_out_of_range_log_window_size() { let mut config = default_config(200); config.log_window_size = 16; - let syn = V1Datagram { - header: FecHeader { - sn_source_ack: 0xFFFF_FFFF, - receive_window_size: 64, - flags: V1Flags::SYN | V1Flags::SYNEX, - }, - ack_vector: None, - ack_of_acks: None, - syn_data: Some(SynDataPayload { - initial_sequence_number: 100, - upstream_mtu: 1232, - downstream_mtu: 1232, - }), - correlation_id: None, - syn_data_ex: Some(test_syn_data_ex()), - data: None, - }; + let syn = syn_datagram(UdpVersion::V3, Some(TEST_COOKIE_HASH)); let result = RdpeudpConnection::accept(config, &syn, t); assert!(matches!(result.unwrap_err().kind(), RdpeudpErrorKind::InvalidState)); @@ -402,15 +308,21 @@ fn accept_rejects_an_out_of_range_log_window_size() { // ── Data transfer tests ── -/// Helper: perform a full handshake and return (client, server, time). -fn establish_pair() -> (RdpeudpConnection, RdpeudpConnection, MonotonicInstant) { +/// Drive a full handshake for `client_config` against a server with +/// `server_isn`, through the final ACK, without draining either side's event +/// queue: `poll_event()` is a separate queue from `poll_transmit()`, so +/// callers are free to drain it (or assert on it) however they need. +fn handshake( + client_config: ConnectionConfig, + server_isn: u32, +) -> (RdpeudpConnection, RdpeudpConnection, MonotonicInstant) { let t = now(); - let mut client = RdpeudpConnection::connect(default_config(100), t).expect("connect"); + let mut client = RdpeudpConnection::connect(client_config, t).expect("connect"); let syn = client.poll_transmit(t).expect("SYN"); let syn_dg: V1Datagram = decode(&syn.contents).expect("decode SYN"); - let mut server = RdpeudpConnection::accept(default_config(200), &syn_dg, t).expect("accept"); + let mut server = RdpeudpConnection::accept(default_config(server_isn), &syn_dg, t).expect("accept"); let syn_ack = server.poll_transmit(t).expect("SYN+ACK"); let mut syn_ack_bytes = syn_ack.contents; @@ -418,19 +330,23 @@ fn establish_pair() -> (RdpeudpConnection, RdpeudpConnection, MonotonicInstant) .handle_datagram(&mut syn_ack_bytes, later(t, 50)) .expect("handle SYN+ACK"); - // Drain client events and final ACK - while client.poll_event().is_some() {} let final_ack = client.poll_transmit(later(t, 50)).expect("final ACK"); - let mut ack_bytes = final_ack.contents; server .handle_datagram(&mut ack_bytes, later(t, 100)) .expect("handle ACK"); - while server.poll_event().is_some() {} (client, server, t) } +/// Helper: perform a full handshake and return (client, server, time). +fn establish_pair() -> (RdpeudpConnection, RdpeudpConnection, MonotonicInstant) { + let (mut client, mut server, t) = handshake(default_config(100), 200); + while client.poll_event().is_some() {} + while server.poll_event().is_some() {} + (client, server, t) +} + /// Each side's handshake round trip seeds its RTT estimate, rather than /// leaving it unset until the first data-phase ACK. #[test] @@ -1238,29 +1154,9 @@ fn connect_refuses_to_build_a_version_3_syn_without_a_cookie_hash() { fn a_server_accepts_and_settles_on_the_version_a_syn_offers_below_3() { let t = now(); - let syn = V1Datagram { - header: FecHeader { - sn_source_ack: 0xFFFF_FFFF, - receive_window_size: 64, - flags: V1Flags::SYN | V1Flags::SYNEX, - }, - ack_vector: None, - ack_of_acks: None, - syn_data: Some(SynDataPayload { - initial_sequence_number: 100, - upstream_mtu: 1232, - downstream_mtu: 1232, - }), - correlation_id: None, - syn_data_ex: Some(SynDataExPayload { - syn_ex_flags: SynExFlags::VERSION_INFO_VALID, - // 0x0002 means the MS-RDPEUDP data transfer, not MS-RDPEUDP2. - // 2.2.2.9: cookieHash MUST NOT be present outside a version 3 SYN. - udp_ver: UdpVersion::V2, - cookie_hash: None, - }), - data: None, - }; + // 0x0002 means the MS-RDPEUDP data transfer, not MS-RDPEUDP2. + // 2.2.2.9: cookieHash MUST NOT be present outside a version 3 SYN. + let syn = syn_datagram(UdpVersion::V2, None); let mut conn = RdpeudpConnection::accept(default_config(200), &syn, t).expect("version 2 is now implemented"); @@ -1277,27 +1173,7 @@ fn a_server_accepts_and_settles_on_the_version_a_syn_offers_below_3() { fn a_server_downgrades_to_version_2_on_a_syn_whose_cookie_hash_does_not_match() { let t = now(); - let syn = V1Datagram { - header: FecHeader { - sn_source_ack: 0xFFFF_FFFF, - receive_window_size: 64, - flags: V1Flags::SYN | V1Flags::SYNEX, - }, - ack_vector: None, - ack_of_acks: None, - syn_data: Some(SynDataPayload { - initial_sequence_number: 100, - upstream_mtu: 1232, - downstream_mtu: 1232, - }), - correlation_id: None, - syn_data_ex: Some(SynDataExPayload { - syn_ex_flags: SynExFlags::VERSION_INFO_VALID, - udp_ver: UdpVersion::V3, - cookie_hash: Some([0xFF; 32]), - }), - data: None, - }; + let syn = syn_datagram(UdpVersion::V3, Some([0xFF; 32])); // 3.1.5.1.1: an invalid cookieHash on a version 3 SYN MUST drop the // connection to version 2, not refuse it outright.