diff --git a/crates/ironrdp-client/src/config.rs b/crates/ironrdp-client/src/config.rs index 820c9462c..ffc040b7c 100644 --- a/crates/ironrdp-client/src/config.rs +++ b/crates/ironrdp-client/src/config.rs @@ -5,7 +5,7 @@ use anyhow::Context as _; use clap::clap_derive::ValueEnum; use clap::Parser; use ironrdp::connector::{self, Credentials}; -use ironrdp::pdu::rdp::capability_sets::MajorPlatformType; +use ironrdp::pdu::rdp::capability_sets::{client_codecs_capabilities, MajorPlatformType}; use ironrdp::pdu::rdp::client_info::PerformanceFlags; use tap::prelude::*; use url::Url; @@ -271,6 +271,7 @@ impl Config { Some(connector::BitmapConfig { color_depth, lossy_compression: true, + codecs: client_codecs_capabilities(), }) } else { None diff --git a/crates/ironrdp-connector/src/connection_activation.rs b/crates/ironrdp-connector/src/connection_activation.rs index 8e501e3a9..c3ecb0a3b 100644 --- a/crates/ironrdp-connector/src/connection_activation.rs +++ b/crates/ironrdp-connector/src/connection_activation.rs @@ -355,16 +355,13 @@ fn create_client_confirm_active( CapabilitySet::SurfaceCommands(SurfaceCommands { flags: CmdFlags::SET_SURFACE_BITS | CmdFlags::STREAM_SURFACE_BITS | CmdFlags::FRAME_MARKER, }), - CapabilitySet::BitmapCodecs(BitmapCodecs(vec![Codec { - id: 0x03, // RemoteFX - property: CodecProperty::RemoteFx(RemoteFxContainer::ClientContainer(RfxClientCapsContainer { - capture_flags: CaptureFlags::empty(), - caps_data: RfxCaps(RfxCapset(vec![RfxICap { - flags: RfxICapFlags::empty(), - entropy_bits: EntropyBits::Rlgr3, - }])), - })), - }])), + CapabilitySet::BitmapCodecs( + config + .bitmap + .as_ref() + .map(|b| b.codecs.clone()) + .unwrap_or_else(client_codecs_capabilities), + ), CapabilitySet::FrameAcknowledge(FrameAcknowledge { // FIXME(#447): Revert this to 2 per FreeRDP. // This is a temporary hack to fix a resize bug, see: diff --git a/crates/ironrdp-connector/src/lib.rs b/crates/ironrdp-connector/src/lib.rs index 66679be86..8a4558df4 100644 --- a/crates/ironrdp-connector/src/lib.rs +++ b/crates/ironrdp-connector/src/lib.rs @@ -23,7 +23,7 @@ use std::sync::Arc; use ironrdp_core::{encode_buf, encode_vec, Encode, WriteBuf}; use ironrdp_pdu::nego::NegoRequestData; -use ironrdp_pdu::rdp::capability_sets; +use ironrdp_pdu::rdp::capability_sets::{self, BitmapCodecs}; use ironrdp_pdu::rdp::client_info::PerformanceFlags; use ironrdp_pdu::x224::X224; use ironrdp_pdu::{gcc, x224, PduHint}; @@ -43,11 +43,12 @@ pub struct DesktopSize { pub height: u16, } -#[derive(Debug, Clone, Copy)] +#[derive(Debug, Clone)] #[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))] pub struct BitmapConfig { pub lossy_compression: bool, pub color_depth: u32, + pub codecs: BitmapCodecs, } #[derive(Debug, Clone)] diff --git a/crates/ironrdp-pdu/src/rdp/capability_sets.rs b/crates/ironrdp-pdu/src/rdp/capability_sets.rs index e2aef9b6c..38bb5a39e 100644 --- a/crates/ironrdp-pdu/src/rdp/capability_sets.rs +++ b/crates/ironrdp-pdu/src/rdp/capability_sets.rs @@ -32,8 +32,9 @@ pub use self::bitmap_cache::{ BitmapCache, BitmapCacheRev2, CacheEntry, CacheFlags, CellInfo, BITMAP_CACHE_ENTRIES_NUM, }; pub use self::bitmap_codecs::{ - BitmapCodecs, CaptureFlags, Codec, CodecProperty, EntropyBits, Guid, NsCodec, RemoteFxContainer, RfxCaps, - RfxCapset, RfxClientCapsContainer, RfxICap, RfxICapFlags, + client_codecs_capabilities, BitmapCodecs, CaptureFlags, Codec, CodecId, CodecProperty, EntropyBits, Guid, NsCodec, + RemoteFxContainer, RfxCaps, RfxCapset, RfxClientCapsContainer, RfxICap, RfxICapFlags, CODEC_ID_NONE, + CODEC_ID_REMOTEFX, }; pub use self::brush::{Brush, SupportLevel}; pub use self::frame_acknowledge::FrameAcknowledge; diff --git a/crates/ironrdp-pdu/src/rdp/capability_sets/bitmap_codecs.rs b/crates/ironrdp-pdu/src/rdp/capability_sets/bitmap_codecs.rs index 26b22e0ff..ad2e8e74e 100644 --- a/crates/ironrdp-pdu/src/rdp/capability_sets/bitmap_codecs.rs +++ b/crates/ironrdp-pdu/src/rdp/capability_sets/bitmap_codecs.rs @@ -1,6 +1,8 @@ #[cfg(test)] mod tests; +use core::fmt::{self, Debug}; + use bitflags::bitflags; use ironrdp_core::{ cast_length, decode, ensure_fixed_part_size, ensure_size, invalid_field_err, other_err, Decode, DecodeResult, @@ -97,7 +99,7 @@ impl<'de> Decode<'de> for Guid { } } -#[derive(Debug, PartialEq, Eq, Clone)] +#[derive(Debug, PartialEq, Eq, Clone, Default)] pub struct BitmapCodecs(pub Vec); impl BitmapCodecs { @@ -239,39 +241,29 @@ impl<'de> Decode<'de> for Codec { let id = src.read_u8(); let codec_properties_len = usize::from(src.read_u16()); - let property = if codec_properties_len != 0 { - ensure_size!(in: src, size: codec_properties_len); - let property_buffer = src.read_slice(codec_properties_len); - - match guid { - GUID_NSCODEC => CodecProperty::NsCodec(decode(property_buffer)?), - GUID_REMOTEFX | GUID_IMAGE_REMOTEFX => { - let property = if property_buffer[0] == 0 { - RemoteFxContainer::ServerContainer(codec_properties_len) - } else { - RemoteFxContainer::ClientContainer(decode(property_buffer)?) - }; - - match guid { - GUID_REMOTEFX => CodecProperty::RemoteFx(property), - GUID_IMAGE_REMOTEFX => CodecProperty::ImageRemoteFx(property), - _ => unreachable!(), - } - } - GUID_IGNORE => CodecProperty::Ignore, - _ => CodecProperty::None, - } - } else { - match guid { - GUID_NSCODEC | GUID_REMOTEFX | GUID_IMAGE_REMOTEFX => { - return Err(invalid_field_err!( - "codecPropertiesLen", - "invalid codec property length" - )); + ensure_size!(in: src, size: codec_properties_len); + let property_buffer = src.read_slice(codec_properties_len); + + let property = match guid { + GUID_NSCODEC => CodecProperty::NsCodec(decode(property_buffer)?), + GUID_REMOTEFX | GUID_IMAGE_REMOTEFX => { + let byte = property_buffer + .first() + .ok_or_else(|| invalid_field_err!("remotefx property", "must not be empty"))?; + let property = if *byte == 0 { + RemoteFxContainer::ServerContainer(codec_properties_len) + } else { + RemoteFxContainer::ClientContainer(decode(property_buffer)?) + }; + + match guid { + GUID_REMOTEFX => CodecProperty::RemoteFx(property), + GUID_IMAGE_REMOTEFX => CodecProperty::ImageRemoteFx(property), + _ => unreachable!(), } - GUID_IGNORE => CodecProperty::Ignore, - _ => CodecProperty::None, } + GUID_IGNORE => CodecProperty::Ignore, + _ => CodecProperty::None, }; Ok(Self { id, property }) @@ -391,6 +383,8 @@ impl Encode for RfxClientCapsContainer { impl<'de> Decode<'de> for RfxClientCapsContainer { fn decode(src: &mut ReadCursor<'de>) -> DecodeResult { + ensure_fixed_part_size!(in: src); + let _length = src.read_u32(); let capture_flags = CaptureFlags::from_bits_truncate(src.read_u32()); let _caps_length = src.read_u32(); @@ -617,3 +611,47 @@ bitflags! { const CODEC_MODE = 2; } } + +// Those IDs are hard-coded for practical reasons, they are implementation +// details of the IronRDP client. The server should respect the client IDs. +#[derive(Copy, Clone, PartialEq, Eq)] +pub struct CodecId(u8); + +pub const CODEC_ID_NONE: CodecId = CodecId(0); +pub const CODEC_ID_REMOTEFX: CodecId = CodecId(3); + +impl Debug for CodecId { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let name = match self.0 { + 0 => "None", + 3 => "RemoteFx", + _ => "unknown", + }; + write!(f, "CodecId({})", name) + } +} + +impl CodecId { + pub const fn from_u8(value: u8) -> Option { + match value { + 0 => Some(CODEC_ID_NONE), + 3 => Some(CODEC_ID_REMOTEFX), + _ => None, + } + } +} + +pub fn client_codecs_capabilities() -> BitmapCodecs { + let codecs = vec![Codec { + id: CODEC_ID_REMOTEFX.0, + property: CodecProperty::RemoteFx(RemoteFxContainer::ClientContainer(RfxClientCapsContainer { + capture_flags: CaptureFlags::empty(), + caps_data: RfxCaps(RfxCapset(vec![RfxICap { + flags: RfxICapFlags::empty(), + entropy_bits: EntropyBits::Rlgr3, + }])), + })), + }]; + + BitmapCodecs(codecs) +} diff --git a/crates/ironrdp-session/src/fast_path.rs b/crates/ironrdp-session/src/fast_path.rs index fb0e1ea23..d0a25da0b 100644 --- a/crates/ironrdp-session/src/fast_path.rs +++ b/crates/ironrdp-session/src/fast_path.rs @@ -9,12 +9,12 @@ use ironrdp_pdu::codecs::rfx::FrameAcknowledgePdu; use ironrdp_pdu::fast_path::{FastPathHeader, FastPathUpdate, FastPathUpdatePdu, Fragmentation}; use ironrdp_pdu::geometry::{InclusiveRectangle, Rectangle as _}; use ironrdp_pdu::pointer::PointerUpdateData; +use ironrdp_pdu::rdp::capability_sets::{CodecId, CODEC_ID_NONE, CODEC_ID_REMOTEFX}; use ironrdp_pdu::rdp::headers::ShareDataPdu; use ironrdp_pdu::surface_commands::{FrameAction, FrameMarkerPdu, SurfaceCommand}; use crate::image::DecodedImage; use crate::pointer::PointerCache; -use crate::utils::CodecId; use crate::{rfx, SessionError, SessionErrorExt, SessionResult}; #[derive(Debug)] @@ -337,7 +337,7 @@ impl Processor { bottom: destination.bottom - 1, }; match codec_id { - CodecId::None => { + CODEC_ID_NONE => { let ext_data = bits.extended_bitmap_data; match ext_data.bpp { 32 => { @@ -352,7 +352,7 @@ impl Processor { } } } - CodecId::RemoteFx => { + CODEC_ID_REMOTEFX => { let mut data = ReadCursor::new(bits.extended_bitmap_data.data); while !data.is_empty() { let (_frame_id, rectangle) = self.rfx_handler.decode(image, &destination, &mut data)?; @@ -361,6 +361,9 @@ impl Processor { .or(Some(rectangle)); } } + _ => { + warn!("Unsupported codec ID: {}", bits.extended_bitmap_data.codec_id); + } } } SurfaceCommand::FrameMarker(marker) => { diff --git a/crates/ironrdp-session/src/lib.rs b/crates/ironrdp-session/src/lib.rs index d82badd41..eecd0b296 100644 --- a/crates/ironrdp-session/src/lib.rs +++ b/crates/ironrdp-session/src/lib.rs @@ -13,7 +13,6 @@ pub mod image; pub mod legacy; pub mod pointer; pub mod rfx; // FIXME: maybe this module should not be in this crate -pub mod utils; pub mod x224; mod active_stage; diff --git a/crates/ironrdp-session/src/utils.rs b/crates/ironrdp-session/src/utils.rs deleted file mode 100644 index f18338071..000000000 --- a/crates/ironrdp-session/src/utils.rs +++ /dev/null @@ -1,16 +0,0 @@ -#[derive(Debug, Copy, Clone, PartialEq, Eq)] -#[repr(u8)] -pub enum CodecId { - None = 0x0, - RemoteFx = 0x3, -} - -impl CodecId { - pub const fn from_u8(value: u8) -> Option { - match value { - 0 => Some(Self::None), - 3 => Some(Self::RemoteFx), - _ => None, - } - } -} diff --git a/crates/ironrdp-web/src/session.rs b/crates/ironrdp-web/src/session.rs index 385d5b1bb..78fd331ba 100644 --- a/crates/ironrdp-web/src/session.rs +++ b/crates/ironrdp-web/src/session.rs @@ -22,6 +22,7 @@ use ironrdp::displaycontrol::client::DisplayControlClient; use ironrdp::dvc::DrdynvcClient; use ironrdp::graphics::image_processing::PixelFormat; use ironrdp::pdu::input::fast_path::FastPathInputEvent; +use ironrdp::pdu::rdp::capability_sets::client_codecs_capabilities; use ironrdp::pdu::rdp::client_info::PerformanceFlags; use ironrdp::session::image::DecodedImage; use ironrdp::session::{fast_path, ActiveStage, ActiveStageOutput, GracefulDisconnectReason}; @@ -838,6 +839,7 @@ fn build_config( bitmap: Some(connector::BitmapConfig { color_depth: 16, lossy_compression: true, + codecs: client_codecs_capabilities(), }), #[allow(clippy::arithmetic_side_effects)] // fine unless we end up with an insanely big version client_build: semver::Version::parse(env!("CARGO_PKG_VERSION")) diff --git a/ffi/src/connector/config.rs b/ffi/src/connector/config.rs index 3c2113a12..ac49ae3c6 100644 --- a/ffi/src/connector/config.rs +++ b/ffi/src/connector/config.rs @@ -125,7 +125,7 @@ pub mod ffi { } pub fn set_bitmap_config(&mut self, bitmap: &BitmapConfig) { - self.bitmap = Some(bitmap.0); + self.bitmap = Some(bitmap.0.clone()); } pub fn set_client_build(&mut self, client_build: u32) {