mirrors/IronRDP
1
0
Fork 0
mirror of https://github.com/Devolutions/IronRDP.git synced 2025-08-03 22:58:17 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 1

fix: GCC client, 16/32bpp, RemoteFX rectangles

Browse source 
Co-authored-by: Benoît CORTIER <bcortier@proton.me>
This commit is contained in:
Marc-André Moreau 2023-05-13 13:33:23 -04:00 • committed by Benoît Cortier
parent cf2287739d
commit 9c3ec78588
15 changed files with 110 additions and 59 deletions
Download patch file Download diff file Expand all files Collapse all files

13
crates/ironrdp-client/src/config.rs
View file

@ -168,7 +168,7 @@ struct Args {
password: Option<String>,
/// Specify the security protocols to use
#[clap(long, value_enum, value_parser, default_value_t = SecurityProtocol::HybridEx)]
#[clap(long, value_enum, value_parser, default_value_t = SecurityProtocol::Hybrid)]
security_protocol: SecurityProtocol,
/// The keyboard type
@ -207,11 +207,6 @@ struct Args {
#[clap(long)]
small_cache: bool,
/// Enable RDP6 lossy bitmap compression algorithm. Please note that lossy compression
/// only works with 32 bit color depth
#[clap(long)]
lossy_bitmap_compression: bool,
/// Set required color depth. Currently only 32 and 16 bit color depths are supported
#[clap(long)]
color_depth: Option<u32>,
@ -255,13 +250,9 @@ impl Config {
anyhow::bail!("Invalid color depth. Only 16 and 32 bit color depths are supported.");
}
if color_depth != 32 && args.lossy_bitmap_compression {
anyhow::bail!("Lossy bitmap compression only works with 32 bit color depth.");
}
Some(connector::BitmapConfig {
color_depth,
lossy_compression: args.lossy_bitmap_compression,
lossy_compression: true,
})
} else {
None

51
crates/ironrdp-connector/src/connection.rs
View file

@ -793,24 +793,22 @@ impl Sequence for ClientConnector {
fn create_gcc_blocks(config: &Config, selected_protocol: nego::SecurityProtocol) -> gcc::ClientGccBlocks {
use ironrdp_pdu::gcc::*;
let color_depth = config
.bitmap
.as_ref()
.map(|bitmap| match bitmap.color_depth {
15 => SupportedColorDepths::BPP15,
16 => SupportedColorDepths::BPP16,
24 => SupportedColorDepths::BPP24,
32 => SupportedColorDepths::BPP32,
_ => panic!("Unsupported color depth: {}", bitmap.color_depth),
})
.unwrap_or(SupportedColorDepths::BPP16);
let max_color_depth = config.bitmap.as_ref().map(|bitmap| bitmap.color_depth).unwrap_or(32);
let supported_color_depths = match max_color_depth {
15 => SupportedColorDepths::BPP15,
16 => SupportedColorDepths::BPP16,
24 => SupportedColorDepths::BPP24,
32 => SupportedColorDepths::BPP32 | SupportedColorDepths::BPP16,
_ => panic!("Unsupported color depth: {}", max_color_depth),
};
ClientGccBlocks {
core: ClientCoreData {
version: RdpVersion::V5_PLUS,
desktop_width: config.desktop_size.width,
desktop_height: config.desktop_size.height,
color_depth: ColorDepth::Bpp4, // ignored because we use the optional core data below
color_depth: ColorDepth::Bpp8, // ignored because we use the optional core data below
sec_access_sequence: SecureAccessSequence::Del,
keyboard_layout: 0, // the server SHOULD use the default active input locale identifier
client_build: config.client_build,
@ -820,19 +818,25 @@ fn create_gcc_blocks(config: &Config, selected_protocol: nego::SecurityProtocol)
keyboard_functional_keys_count: config.keyboard_functional_keys_count,
ime_file_name: config.ime_file_name.clone(),
optional_data: ClientCoreOptionalData {
post_beta2_color_depth: Some(ColorDepth::Bpp4), // ignored because we set high_color_depth
post_beta2_color_depth: Some(ColorDepth::Bpp8), // ignored because we set high_color_depth
client_product_id: Some(1),
serial_number: Some(0),
high_color_depth: Some(HighColorDepth::Bpp24),
supported_color_depths: Some(color_depth),
supported_color_depths: Some(supported_color_depths),
early_capability_flags: {
let mut early_capability_flags = ClientEarlyCapabilityFlags::VALID_CONNECTION_TYPE
| ClientEarlyCapabilityFlags::SUPPORT_ERR_INFO_PDU;
| ClientEarlyCapabilityFlags::SUPPORT_ERR_INFO_PDU
| ClientEarlyCapabilityFlags::SUPPORT_STATUS_INFO_PDU
| ClientEarlyCapabilityFlags::STRONG_ASYMMETRIC_KEYS;
if config.graphics.is_some() {
early_capability_flags |= ClientEarlyCapabilityFlags::SUPPORT_DYN_VC_GFX_PROTOCOL;
}
if max_color_depth == 32 {
early_capability_flags |= ClientEarlyCapabilityFlags::WANT_32_BPP_SESSION;
}
Some(early_capability_flags)
},
dig_product_id: Some(config.dig_product_id.clone()),
@ -845,7 +849,10 @@ fn create_gcc_blocks(config: &Config, selected_protocol: nego::SecurityProtocol)
device_scale_factor: None,
},
},
security: ClientSecurityData::no_security(),
security: ClientSecurityData {
encryption_methods: EncryptionMethod::empty(),
ext_encryption_methods: 0,
},
network: if config.graphics.is_some() {
Some(ClientNetworkData {
channels: vec![Channel {
@ -856,10 +863,16 @@ fn create_gcc_blocks(config: &Config, selected_protocol: nego::SecurityProtocol)
} else {
Some(ClientNetworkData { channels: Vec::new() })
},
cluster: None,
cluster: Some(ClientClusterData {
flags: RedirectionFlags::REDIRECTION_SUPPORTED,
redirection_version: RedirectionVersion::V4,
redirected_session_id: 0,
}),
monitor: None,
message_channel: None,
multi_transport_channel: None,
message_channel: Some(ClientMessageChannelData {}),
multi_transport_channel: Some(MultiTransportChannelData {
flags: MultiTransportFlags::empty(),
}),
monitor_extended: None,
}
}

2
crates/ironrdp-graphics/src/image_processing.rs
View file

@ -121,7 +121,7 @@ impl PixelFormat {
if buffer.len() < 4 {
Err(io::Error::new(
io::ErrorKind::InvalidInput,
"The input buffer is not large enough",
"input buffer is not large enough (this is a bug)",
))
} else {
let color = &buffer[..4];

2
crates/ironrdp-pdu/src/basic_output/bitmap/rdp6.rs
View file

@ -269,7 +269,7 @@ mod tests {
context: "Rdp6BitmapStream",
kind: InvalidMessage {
field: "padding",
reason: "missing padding byte from zero-size Non-RLE bitmap data",
reason: "missing padding byte from zero-sized non-RLE bitmap data",
},
source: None,
}

6
crates/ironrdp-pdu/src/basic_output/surface_commands.rs
View file

@ -11,7 +11,7 @@ use thiserror::Error;
use crate::geometry::Rectangle;
use crate::utils::SplitTo;
use crate::{PduBufferParsing, PduParsing};
use crate::PduBufferParsing;
pub const SURFACE_COMMAND_HEADER_SIZE: usize = 2;
@ -70,7 +70,7 @@ impl<'a> PduBufferParsing<'a> for SurfaceBitsPdu<'a> {
type Error = SurfaceCommandsError;
fn from_buffer_consume(mut buffer: &mut &'a [u8]) -> Result<Self, Self::Error> {
let destination = Rectangle::from_buffer(&mut buffer)?;
let destination = Rectangle::from_buffer_exclusive(&mut buffer)?;
let extended_bitmap_data = ExtendedBitmapDataPdu::from_buffer_consume(buffer)?;
Ok(Self {
@ -80,7 +80,7 @@ impl<'a> PduBufferParsing<'a> for SurfaceBitsPdu<'a> {
}
fn to_buffer_consume(&self, mut buffer: &mut &mut [u8]) -> Result<(), Self::Error> {
self.destination.to_buffer(&mut buffer)?;
self.destination.to_buffer_exclusive(&mut buffer)?;
self.extended_bitmap_data.to_buffer_consume(buffer)?;
Ok(())

2
crates/ironrdp-pdu/src/basic_output/surface_commands/tests.rs
View file

@ -5,7 +5,7 @@ use super::*;
const FRAME_MARKER_BUFFER: [u8; 8] = [0x4, 0x0, 0x0, 0x0, 0x5, 0x0, 0x0, 0x0];
const SURFACE_BITS_BUFFER: [u8; 1217] = [
0x6, 0x0, 0x0, 0x0, 0x0, 0x0, 0x7F, 0x7, 0x37, 0x4, 0x20, 0x0, 0x0, 0x3, 0x80, 0x7, 0x38, 0x4, 0xab, 0x4, 0x0, 0x0,
0x6, 0x0, 0x0, 0x0, 0x0, 0x0, 0x80, 0x7, 0x38, 0x4, 0x20, 0x0, 0x0, 0x3, 0x80, 0x7, 0x38, 0x4, 0xab, 0x4, 0x0, 0x0,
0xc4, 0xcc, 0xe, 0x0, 0x0, 0x0, 0x1, 0x0, 0x4, 0x0, 0x0, 0x0, 0x1, 0x0, 0xc6, 0xcc, 0x17, 0x0, 0x0, 0x0, 0x1, 0x0,
0x1, 0x1, 0x0, 0x70, 0x7, 0xa0, 0x0, 0x10, 0x0, 0xc0, 0x0, 0xc1, 0xca, 0x1, 0x0, 0xc7, 0xcc, 0x7e, 0x4, 0x0, 0x0,
0x1, 0x0, 0xc2, 0xca, 0x0, 0x0, 0x51, 0x50, 0x1, 0x40, 0x4, 0x0, 0x63, 0x4, 0x0, 0x0, 0x66, 0x66, 0x77, 0x88, 0x98,

41
crates/ironrdp-pdu/src/geometry.rs
View file

@ -3,6 +3,10 @@ use std::io;
use byteorder::{LittleEndian, ReadBytesExt, WriteBytesExt};
/// An **inclusive** rectangle.
///
/// This struct is defined as an **inclusive** rectangle.
/// That is, the pixel at coordinate (right, bottom) is included in the rectangle.
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct Rectangle {
pub left: u16,
@ -61,6 +65,43 @@ impl Rectangle {
bottom: max(self.bottom, other.bottom),
}
}
// TODO: clarify code related to rectangles (inclusive vs exclusive bounds, …)
// See for instance:
// https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-rdpbcgr/84a3d4d2-5523-4e49-9a48-33952c559485
// https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-rdpbcgr/776dbdaf-7619-45fd-9a90-ebfd07802b24
// Rename / new structs "InclusiveRect" / "ExclusiveRect"…
// Also, avoid / audit mixed usage of "Rectangle" with "RfxRectangle"
// We should be careful when manipulating structs with slight nuances like this.
pub fn from_buffer_exclusive(mut stream: impl io::Read) -> Result<Self, io::Error> {
let left = stream.read_u16::<LittleEndian>()?;
let top = stream.read_u16::<LittleEndian>()?;
let right = stream
.read_u16::<LittleEndian>()?
.checked_sub(1)
.ok_or_else(|| io::Error::new(io::ErrorKind::Other, "invalid exclusive right bound"))?;
let bottom = stream
.read_u16::<LittleEndian>()?
.checked_sub(1)
.ok_or_else(|| io::Error::new(io::ErrorKind::Other, "invalid exclusive bottom bound"))?;
Ok(Self {
left,
top,
right,
bottom,
})
}
pub fn to_buffer_exclusive(&self, mut stream: impl io::Write) -> Result<(), io::Error> {
stream.write_u16::<LittleEndian>(self.left)?;
stream.write_u16::<LittleEndian>(self.top)?;
stream.write_u16::<LittleEndian>(self.right + 1)?;
stream.write_u16::<LittleEndian>(self.bottom + 1)?;
Ok(())
}
}
impl crate::PduParsing for Rectangle {

2
crates/ironrdp-pdu/src/mcs.rs
View file

@ -905,7 +905,7 @@ impl DomainParameters {
pub fn max() -> Self {
Self {
max_channel_ids: 65535,
max_user_ids: 65535,
max_user_ids: 64535,
max_token_ids: 65535,
num_priorities: 1,
min_throughput: 0,

4
crates/ironrdp-pdu/src/pcb.rs
View file

@ -156,10 +156,10 @@ impl PduEncode for PreconnectionBlob {
#[cfg(test)]
mod tests {
use super::*;
use expect_test::expect;
use super::*;
const PRECONNECTION_PDU_V1_NULL_SIZE_BUF: [u8; 16] = [
0x00, 0x00, 0x00, 0x00, // -> RDP_PRECONNECTION_PDU_V1::cbSize = 0x00 = 0 bytes
0x00, 0x00, 0x00, 0x00, // -> RDP_PRECONNECTION_PDU_V1::Flags = 0

16
crates/ironrdp-pdu/src/per.rs
View file

@ -547,10 +547,10 @@ pub(crate) mod legacy {
#[cfg(test)]
mod tests {
use super::*;
use expect_test::expect;
use super::*;
#[test]
fn read_length_is_correct_length() {
let mut src = ReadCursor::new(&[0x05]);
@ -706,7 +706,8 @@ mod tests {
expect![[r#"
Overflow
"#]].assert_debug_eq(&e)
"#]]
.assert_debug_eq(&e)
}
#[test]
@ -730,7 +731,8 @@ mod tests {
expect![[r#"
Underflow
"#]].assert_debug_eq(&e);
"#]]
.assert_debug_eq(&e);
}
#[test]
@ -761,7 +763,8 @@ mod tests {
expect![[r#"
UnexpectedEnumVariant
"#]].assert_debug_eq(&e);
"#]]
.assert_debug_eq(&e);
}
#[test]
@ -781,7 +784,8 @@ mod tests {
expect![[r#"
UnexpectedEnumVariant
"#]].assert_debug_eq(&e);
"#]]
.assert_debug_eq(&e);
}
#[test]

4
crates/ironrdp-session/src/image.rs
View file

@ -60,8 +60,8 @@ impl DecodedImage {
region: Rectangle {
left: source_x,
top: source_y,
right: source_x + region_rectangle.width(),
bottom: source_y + region_rectangle.height(),
right: source_x + region_rectangle.width() - 1,
bottom: source_y + region_rectangle.height() - 1,
},
step: SOURCE_STRIDE,
pixel_format: SOURCE_PIXEL_FORMAT,

12
crates/ironrdp-session/src/rfx.rs
View file

@ -107,8 +107,8 @@ impl DecodingContext {
region.rectangles = vec![RfxRectangle {
x: 0,
y: 0,
width: channel.width as u16,
height: channel.height as u16,
width,
height,
}];
}
let region = region;
@ -211,8 +211,8 @@ fn clipping_rectangles(rectangles: &[RfxRectangle], destination: &Rectangle, wid
.map(|r| Rectangle {
left: min(destination.left + r.x, width - 1),
top: min(destination.top + r.y, height - 1),
right: min(destination.left + r.x + r.width, width - 1),
bottom: min(destination.top + r.y + r.height, height - 1),
right: min(destination.left + r.x + r.width - 1, width - 1),
bottom: min(destination.top + r.y + r.height - 1, height - 1),
})
.for_each(|r| clipping_rectangles.union_rectangle(r));
@ -223,8 +223,8 @@ fn tiles_to_rectangles<'a>(tiles: &'a [Tile<'_>], destination: &'a Rectangle) ->
tiles.iter().map(|t| Rectangle {
left: destination.left + t.x * TILE_SIZE,
top: destination.top + t.y * TILE_SIZE,
right: destination.left + t.x * TILE_SIZE + TILE_SIZE,
bottom: destination.top + t.y * TILE_SIZE + TILE_SIZE,
right: destination.left + t.x * TILE_SIZE + TILE_SIZE - 1,
bottom: destination.top + t.y * TILE_SIZE + TILE_SIZE - 1,
})
}

9
crates/ironrdp-testsuite-core/tests/pdu/x224.rs
View file

@ -225,7 +225,8 @@ fn nego_request_unexpected_rdp_msg_type() {
},
source: None,
}
"#]].assert_debug_eq(&e);
"#]]
.assert_debug_eq(&e);
}
#[test]
@ -258,7 +259,8 @@ fn nego_confirm_unexpected_rdp_msg_type() {
},
source: None,
}
"#]].assert_debug_eq(&e);
"#]]
.assert_debug_eq(&e);
}
#[test]
@ -320,5 +322,6 @@ fn cookie_without_cr_lf_error_decode() {
},
source: None,
}
"#]].assert_debug_eq(&e);
"#]]
.assert_debug_eq(&e);
}

3
crates/ironrdp-web/src/error.rs
View file

@ -1,5 +1,4 @@
use ironrdp::connector::sspi;
use ironrdp::connector::{self, ConnectorErrorKind};
use ironrdp::connector::{self, sspi, ConnectorErrorKind};
use wasm_bindgen::prelude::*;
#[wasm_bindgen]

2
crates/ironrdp-web/src/session.rs
View file

@ -333,7 +333,7 @@ fn build_config(
username,
password,
domain,
security_protocol: ironrdp::pdu::nego::SecurityProtocol::HYBRID_EX,
security_protocol: ironrdp::pdu::nego::SecurityProtocol::HYBRID,
keyboard_type: ironrdp::pdu::gcc::KeyboardType::IbmEnhanced,
keyboard_subtype: 0,
keyboard_functional_keys_count: 12,