diff --git a/crates/ironrdp-pdu/src/gcc/cluster_data.rs b/crates/ironrdp-pdu/src/gcc/cluster_data.rs index 9e2f499b6d..6f5c6ef4b9 100644 --- a/crates/ironrdp-pdu/src/gcc/cluster_data.rs +++ b/crates/ironrdp-pdu/src/gcc/cluster_data.rs @@ -52,8 +52,11 @@ impl<'de> Decode<'de> for ClientClusterData { let flags_with_version = src.read_u32(); let redirected_session_id = src.read_u32(); - let flags = RedirectionFlags::from_bits(flags_with_version & !REDIRECTION_VERSION_MASK) - .ok_or_else(|| invalid_field_err!("flags", "invalid redirection flags", in: src))?; + // [MS-RDPBCGR] 3.3.5.3.3 asks the server to validate only the Client + // Network Data bounds among the settings blocks, never this bit set; + // retain unknown bits (crate-wide policy since #1144) rather than + // refusing the client at GCC. + let flags = RedirectionFlags::from_bits_retain(flags_with_version & !REDIRECTION_VERSION_MASK); let redirection_version = RedirectionVersion::from_u32((flags_with_version & REDIRECTION_VERSION_MASK) >> 2) .ok_or_else(|| invalid_field_err!("redirVersion", "invalid redirection version", in: src))?; diff --git a/crates/ironrdp-pdu/src/gcc/monitor_data.rs b/crates/ironrdp-pdu/src/gcc/monitor_data.rs index cbb3243a49..d60f6cbdca 100644 --- a/crates/ironrdp-pdu/src/gcc/monitor_data.rs +++ b/crates/ironrdp-pdu/src/gcc/monitor_data.rs @@ -116,8 +116,11 @@ impl<'de> Decode<'de> for Monitor { let top = src.read_i32(); let right = src.read_i32(); let bottom = src.read_i32(); - let flags = MonitorFlags::from_bits(src.read_u32()) - .ok_or_else(|| invalid_field_err!("flags", "invalid monitor flags", in: src))?; + // [MS-RDPBCGR] 2.2.1.3.6.1 defines only TS_MONITOR_PRIMARY here, and + // 3.3.5.3.3 never asks the server to validate the bit set; retain + // unknown bits (crate-wide policy since #1144) rather than refusing a + // multimon client at GCC. + let flags = MonitorFlags::from_bits_retain(src.read_u32()); Ok(Self { left, diff --git a/crates/ironrdp-pdu/src/gcc/multi_transport_channel_data.rs b/crates/ironrdp-pdu/src/gcc/multi_transport_channel_data.rs index 0949c0a04c..c5d5806bb1 100644 --- a/crates/ironrdp-pdu/src/gcc/multi_transport_channel_data.rs +++ b/crates/ironrdp-pdu/src/gcc/multi_transport_channel_data.rs @@ -1,7 +1,5 @@ use bitflags::bitflags; -use ironrdp_core::{ - Decode, DecodeResult, Encode, EncodeResult, ReadCursor, WriteCursor, ensure_fixed_part_size, invalid_field_err, -}; +use ironrdp_core::{Decode, DecodeResult, Encode, EncodeResult, ReadCursor, WriteCursor, ensure_fixed_part_size}; #[derive(Debug, Clone, PartialEq, Eq)] #[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))] @@ -37,8 +35,12 @@ impl<'de> Decode<'de> for MultiTransportChannelData { fn decode(src: &mut ReadCursor<'de>) -> DecodeResult { ensure_fixed_part_size!(in: src); - let flags = MultiTransportFlags::from_bits(src.read_u32()) - .ok_or_else(|| invalid_field_err!("flags", "invalid multitransport flags", in: src))?; + // [MS-RDPBCGR] 2.2.1.3.8's transportFlags list has grown before + // (UDP_PREFERRED and SOFTSYNC_TCP_TO_UDP are later additions), and + // 3.3.5.3.3 never asks the receiver to validate it; retain unknown + // bits (crate-wide policy since #1144) rather than failing the GCC + // exchange. + let flags = MultiTransportFlags::from_bits_retain(src.read_u32()); Ok(Self { flags }) } diff --git a/crates/ironrdp-testsuite-core/tests/pdu/gcc.rs b/crates/ironrdp-testsuite-core/tests/pdu/gcc.rs index e34672339c..acde4242cb 100644 --- a/crates/ironrdp-testsuite-core/tests/pdu/gcc.rs +++ b/crates/ironrdp-testsuite-core/tests/pdu/gcc.rs @@ -248,6 +248,23 @@ fn from_buffer_correctly_parses_client_cluster_data() { assert_eq!(*CLUSTER_DATA, decode(buffer).unwrap()); } +#[test] +fn cluster_data_with_undefined_flag_bits_decodes_and_retains_them() { + // [MS-RDPBCGR] 3.3.5.3.3 never asks the server to validate this bit set; + // an unknown flag must not fail the GCC exchange. The bit is retained so + // re-encoding preserves the wire value. + const UNDEFINED_BIT: u32 = 0x0000_0080; // undefined in 2.2.1.3.5 + + let mut buffer = CLUSTER_DATA_BUFFER.to_vec(); + let flags = u32::from_le_bytes(buffer[0..4].try_into().unwrap()) | UNDEFINED_BIT; + buffer[0..4].copy_from_slice(&flags.to_le_bytes()); + + let data: ClientClusterData = decode(buffer.as_slice()).expect("undefined redirection bits are not fatal"); + + assert_ne!(data.flags.bits() & UNDEFINED_BIT, 0); + assert_eq!(encode_vec(&data).unwrap(), buffer); +} + #[test] fn to_buffer_correctly_serializes_client_cluster_data() { let data = CLUSTER_DATA.clone(); @@ -555,6 +572,25 @@ fn from_buffer_correctly_parses_client_monitor_data_with_monitors() { ); } +#[test] +fn monitor_with_undefined_flag_bits_decodes_and_retains_them() { + // [MS-RDPBCGR] 2.2.1.3.6.1 defines only TS_MONITOR_PRIMARY; a vendor or + // future bit must not refuse a multimon client at GCC. The bit is + // retained so re-encoding preserves the wire value. + const UNDEFINED_BIT: u32 = 0x0000_0002; // undefined in 2.2.1.3.6.1 + + let mut buffer = ironrdp_testsuite_core::monitor_data::MONITOR_DATA_WITH_MONITORS_BUFFER.to_vec(); + // First monitor starts after the 8-byte header; its flags are the fifth + // dword of the 20-byte TS_MONITOR_DEF. + let flags = u32::from_le_bytes(buffer[24..28].try_into().unwrap()) | UNDEFINED_BIT; + buffer[24..28].copy_from_slice(&flags.to_le_bytes()); + + let data: ClientMonitorData = decode(buffer.as_slice()).expect("undefined monitor bits are not fatal"); + + assert_ne!(data.monitors[0].flags.bits() & UNDEFINED_BIT, 0); + assert_eq!(encode_vec(&data).unwrap(), buffer); +} + #[test] fn to_buffer_correctly_serializes_client_monitor_data_without_monitors() { let data = ironrdp_testsuite_core::monitor_data::MONITOR_DATA_WITHOUT_MONITORS.clone(); @@ -666,6 +702,24 @@ fn from_buffer_correctly_parses_server_multi_transport_channel_data() { ); } +#[test] +fn multi_transport_with_undefined_flag_bits_decodes_and_retains_them() { + // [MS-RDPBCGR] 2.2.1.3.8's transportFlags list has grown before; an + // unknown bit must not fail the GCC exchange. The bit is retained so + // re-encoding preserves the wire value. + const UNDEFINED_BIT: u32 = 0x0000_0002; // undefined in 2.2.1.3.8 + + let mut buffer = + ironrdp_testsuite_core::multi_transport_channel_data::SERVER_GCC_MULTI_TRANSPORT_CHANNEL_BLOCK_BUFFER.to_vec(); + let flags = u32::from_le_bytes(buffer[0..4].try_into().unwrap()) | UNDEFINED_BIT; + buffer[0..4].copy_from_slice(&flags.to_le_bytes()); + + let data: MultiTransportChannelData = decode(buffer.as_slice()).expect("undefined transport bits are not fatal"); + + assert_ne!(data.flags.bits() & UNDEFINED_BIT, 0); + assert_eq!(encode_vec(&data).unwrap(), buffer); +} + #[test] fn to_buffer_correctly_serializes_server_multi_transport_channel_data() { let data = ironrdp_testsuite_core::multi_transport_channel_data::SERVER_GCC_MULTI_TRANSPORT_CHANNEL_BLOCK.clone();