From ca4061ef7e1feb483214bdf207caf19c92c79309 Mon Sep 17 00:00:00 2001 From: Hani Hawari Date: Mon, 8 Jun 2026 22:29:14 -0400 Subject: [PATCH] fix(pdu): tolerate unknown GCC user-data blocks instead of failing As an RDP server, IronRDP fails to negotiate a connection with the current Windows App (the msrdc client): ClientGccBlocks/ServerGccBlocks return an error on the first GCC user-data block whose blockType is not one of the variants IronRDP models, which aborts the whole decode and drops the connection during the Conference Create exchange. The Windows App trips this in two ways: - It sends a TS_UD_CS_UNUSED1 block (blockType 0xC00C). This block is documented and MS-RDPBCGR 2.2.1.3.9 states it "SHOULD be ignored by the server if sent by the client", but IronRDP did not model it (the ClientGccType set stopped at 0xC00A), so it was rejected rather than ignored. - It also sends blockType 0xC00D, which is undocumented: it is in neither the User Data Header type list in MS-RDPBCGR 2.2.1.3.1 nor FreeRDP's parser, mirroring the historical 0xC009 block sent by the Lync client. GCC user-data blocks are a length-prefixed TLV (TS_UD_HEADER is type + length + value), so a reader can seek past an unknown block and continue; FreeRDP logs and skips unknown blocks rather than aborting. Add UserDataHeader::decode_raw, which returns the raw u16 blockType and the block body without mapping the type to an enum. The block decoders use it and skip user-data blocks they do not recognise, advancing past the body via the block length. The existing generic decode is retained (now a thin wrapper over decode_raw) so its API and behavior are unchanged. Also model TS_UD_CS_UNUSED1 (0xC00C) as an explicit no-op so the one documented ignorable block is handled by name. Add tests covering a skipped unknown block, an ignored CS_UNUSED1 block, and decode_raw returning a raw unrecognised blockType. --- crates/ironrdp-pdu/src/gcc/mod.rs | 53 ++++++++++++++++--- .../ironrdp-testsuite-core/tests/pdu/gcc.rs | 38 +++++++++++++ 2 files changed, 83 insertions(+), 8 deletions(-) diff --git a/crates/ironrdp-pdu/src/gcc/mod.rs b/crates/ironrdp-pdu/src/gcc/mod.rs index 2156a613b..9229b1dcc 100644 --- a/crates/ironrdp-pdu/src/gcc/mod.rs +++ b/crates/ironrdp-pdu/src/gcc/mod.rs @@ -149,7 +149,17 @@ impl<'de> Decode<'de> for ClientGccBlocks { let mut monitor_extended = None; loop { - let (ty, cur) = user_header_try!(UserDataHeader::decode(src)); + let (raw_ty, cur) = user_header_try!(UserDataHeader::decode_raw(src)); + let Some(ty) = ClientGccType::from_u16(raw_ty) else { + // Forward-compat: a user-data block with a type IronRDP does not + // model is skipped rather than rejected. The block table in + // MS-RDPBCGR 2.2.1.3.1 is not closed: current Microsoft clients + // emit blocks absent from the public spec (e.g. 0xC00D from + // recent mstsc/Windows App, mirroring the historical 0xC009 sent + // by the Lync client), and rejecting them would break the + // connection. The block length lets us advance past the body. + continue; + }; match ty { ClientGccType::CoreData => core = Some(decode(cur)?), @@ -160,6 +170,8 @@ impl<'de> Decode<'de> for ClientGccBlocks { ClientGccType::MessageChannelData => message_channel = Some(decode(cur)?), ClientGccType::MonitorExtendedData => monitor_extended = Some(decode(cur)?), ClientGccType::MultiTransportChannelData => multi_transport_channel = Some(decode(cur)?), + // Padding-only block the server is told to ignore (2.2.1.3.9). + ClientGccType::Unused1 => {} }; } @@ -244,7 +256,13 @@ impl<'de> Decode<'de> for ServerGccBlocks { let mut multi_transport_channel = None; loop { - let (ty, cur) = user_header_try!(UserDataHeader::decode(src)); + let (raw_ty, cur) = user_header_try!(UserDataHeader::decode_raw(src)); + let Some(ty) = ServerGccType::from_u16(raw_ty) else { + // Forward-compat: skip server user-data blocks IronRDP does not + // model rather than rejecting the connection (see the matching + // note on `ClientGccBlocks::decode`). + continue; + }; match ty { ServerGccType::CoreData => core = Some(decode(cur)?), @@ -277,6 +295,13 @@ pub enum ClientGccType { MessageChannelData = 0xC006, MonitorExtendedData = 0xC008, MultiTransportChannelData = 0xC00A, + /// TS_UD_CS_UNUSED1, a padding-only block that, per [MS-RDPBCGR 2.2.1.3.9], + /// "SHOULD be ignored by the server if sent by the client". It carries no + /// data IronRDP consumes, so it is recognised purely so it can be skipped + /// explicitly rather than treated as an unknown type. + /// + /// [MS-RDPBCGR 2.2.1.3.9]: https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-rdpbcgr/bd4c2741-8842-4a43-8770-791b50dd2207 + Unused1 = 0xC00C, } impl ClientGccType { @@ -331,14 +356,16 @@ impl UserDataHeader { Ok(()) } - pub fn decode<'de, T>(src: &mut ReadCursor<'de>) -> DecodeResult<(T, &'de [u8])> - where - T: FromPrimitive, - { + /// Decodes a GCC user-data block header, returning the raw `blockType` and + /// the block body without mapping the type to an enum. + /// + /// Unlike [`decode`](Self::decode), an unrecognised `blockType` is not an + /// error: the raw value is returned so the caller can skip blocks it does + /// not recognise, as MS-RDPBCGR requires for forward-compatibility. + pub fn decode_raw<'de>(src: &mut ReadCursor<'de>) -> DecodeResult<(u16, &'de [u8])> { ensure_fixed_part_size!(in: src); - let block_type = - T::from_u16(src.read_u16()).ok_or_else(|| invalid_field_err!("blockType", "invalid GCC type"))?; + let block_type = src.read_u16(); let block_length: usize = cast_length!("blockLen", src.read_u16())?; if block_length <= USER_DATA_HEADER_SIZE { @@ -350,4 +377,14 @@ impl UserDataHeader { Ok((block_type, src.read_slice(len))) } + + pub fn decode<'de, T>(src: &mut ReadCursor<'de>) -> DecodeResult<(T, &'de [u8])> + where + T: FromPrimitive, + { + let (raw_type, body) = Self::decode_raw(src)?; + let block_type = T::from_u16(raw_type).ok_or_else(|| invalid_field_err!("blockType", "invalid GCC type"))?; + + Ok((block_type, body)) + } } diff --git a/crates/ironrdp-testsuite-core/tests/pdu/gcc.rs b/crates/ironrdp-testsuite-core/tests/pdu/gcc.rs index a080939a6..ecc791530 100644 --- a/crates/ironrdp-testsuite-core/tests/pdu/gcc.rs +++ b/crates/ironrdp-testsuite-core/tests/pdu/gcc.rs @@ -183,6 +183,44 @@ fn from_buffer_correctly_handles_invalid_lengths_in_user_data_header() { assert!(UserDataHeader::decode::(&mut cur).is_err()); } +#[test] +fn decode_raw_returns_raw_block_type_for_unknown_type() { + // blockType = 0xC00D (an undocumented type not modelled by IronRDP), + // blockLen = 8, then 4 body bytes. decode_raw returns the raw type instead + // of rejecting it, so block decoders can skip unrecognised blocks + // (MS-RDPBCGR forward-compat). + let buffer: [u8; 8] = [0x0d, 0xc0, 0x08, 0x00, 0xde, 0xad, 0xbe, 0xef]; + let mut cur = ReadCursor::new(&buffer); + + let (block_type, body) = UserDataHeader::decode_raw(&mut cur).unwrap(); + + assert_eq!(block_type, 0xc00d); + assert_eq!(body, [0xde, 0xad, 0xbe, 0xef]); +} + +#[test] +fn from_buffer_skips_unknown_client_gcc_block_and_parses_known_blocks() { + // An undocumented block (type 0xC00D, blockLen 8, 4 body bytes) trailing an + // otherwise valid set must be skipped without affecting the known blocks. + // Recent Microsoft clients emit such blocks; rejecting them would break the + // connection (see ClientGccBlocks::decode). + let mut buffer = CLIENT_GCC_WITHOUT_OPTIONAL_FIELDS_BUFFER.to_vec(); + buffer.extend_from_slice(&[0x0d, 0xc0, 0x08, 0x00, 0xde, 0xad, 0xbe, 0xef]); + + assert_eq!(*CLIENT_GCC_WITHOUT_OPTIONAL_FIELDS, decode(buffer.as_slice()).unwrap()); +} + +#[test] +fn from_buffer_ignores_cs_unused1_client_gcc_block() { + // TS_UD_CS_UNUSED1 (type 0xC00C, blockLen 6, 2 pad octets) is a documented + // padding block the server "SHOULD ignore" (MS-RDPBCGR 2.2.1.3.9). It is + // recognised and dropped, leaving the parsed blocks unchanged. + let mut buffer = CLIENT_GCC_WITHOUT_OPTIONAL_FIELDS_BUFFER.to_vec(); + buffer.extend_from_slice(&[0x0c, 0xc0, 0x06, 0x00, 0x00, 0x00]); + + assert_eq!(*CLIENT_GCC_WITHOUT_OPTIONAL_FIELDS, decode(buffer.as_slice()).unwrap()); +} + #[test] fn from_buffer_correctly_parses_client_cluster_data() { let buffer = CLUSTER_DATA_BUFFER.as_ref();