From b9ee6dc28c08ecb90ae2ec55328f7bc8bd00ccd5 Mon Sep 17 00:00:00 2001 From: Vladyslav Nikonov Date: Wed, 15 Jan 2025 16:45:08 +0200 Subject: [PATCH] fix(pdu-rust): fixes/improvements for NowProto 2nd iteration --- crates/now-proto-pdu/Cargo.toml | 1 + crates/now-proto-pdu/src/channel/capset.rs | 9 ++++++--- crates/now-proto-pdu/src/exec/pwsh.rs | 4 ++++ crates/now-proto-pdu/src/exec/win_ps.rs | 4 ++++ crates/now-proto-pdu/src/session/msg_box_rsp.rs | 12 +++++++++--- crates/now-proto-testsuite/tests/proto/channel.rs | 8 ++++++++ crates/now-proto-testsuite/tests/proto/session.rs | 8 ++------ docs/NOW-spec.md | 4 ++-- 8 files changed, 36 insertions(+), 14 deletions(-) diff --git a/crates/now-proto-pdu/Cargo.toml b/crates/now-proto-pdu/Cargo.toml index e6e3068..3618289 100644 --- a/crates/now-proto-pdu/Cargo.toml +++ b/crates/now-proto-pdu/Cargo.toml @@ -25,4 +25,5 @@ ironrdp-core = { version = "0.1", features = ["alloc"] } ironrdp-error = { version = "0.1", features = ["alloc"] } [features] +std = ["ironrdp-core/std", "ironrdp-error/std"] default = [] diff --git a/crates/now-proto-pdu/src/channel/capset.rs b/crates/now-proto-pdu/src/channel/capset.rs index f5ba9f7..c6be25d 100644 --- a/crates/now-proto-pdu/src/channel/capset.rs +++ b/crates/now-proto-pdu/src/channel/capset.rs @@ -13,7 +13,7 @@ bitflags! { pub struct NowChannelCapsetFlags: u16 { /// Set if heartbeat specify channel heartbeat interval. /// - /// NOW-PROTO: NOW_CHANNEL_SET_HEATBEAT + /// NOW-PROTO: NOW_CHANNEL_SET_HEARTBEAT const SET_HEARTBEAT = 0x0001; } } @@ -145,10 +145,13 @@ impl NowChannelCapsetMsg { } pub fn with_heartbeat_interval(mut self, interval: time::Duration) -> EncodeResult { - // Sanity check: Limit heartbeat interval to 24 hours. + // Sanity check: Limit min heartbeat interval to 5 seconds. + const MIN_HEARTBEAT_INTERVAL: time::Duration = time::Duration::from_secs(5); + + // Sanity check: Limit max heartbeat interval to 24 hours. const MAX_HEARTBEAT_INTERVAL: time::Duration = time::Duration::from_secs(60 * 60 * 24); - if interval > MAX_HEARTBEAT_INTERVAL { + if interval < MIN_HEARTBEAT_INTERVAL || interval > MAX_HEARTBEAT_INTERVAL { return Err(invalid_field_err!("heartbeat_timeout", "too big heartbeat interval")); } diff --git a/crates/now-proto-pdu/src/exec/pwsh.rs b/crates/now-proto-pdu/src/exec/pwsh.rs index 2cb9ca0..60c6f75 100644 --- a/crates/now-proto-pdu/src/exec/pwsh.rs +++ b/crates/now-proto-pdu/src/exec/pwsh.rs @@ -165,6 +165,10 @@ impl<'a> NowExecPwshMsg<'a> { self.flags.contains(NowExecWinPsFlags::NO_PROFILE) } + pub fn is_non_interactive(&self) -> bool { + self.flags.contains(NowExecWinPsFlags::NON_INTERACTIVE) + } + pub fn apartment_state(&self) -> DecodeResult> { ComApartmentStateKind::from_flags(self.flags) } diff --git a/crates/now-proto-pdu/src/exec/win_ps.rs b/crates/now-proto-pdu/src/exec/win_ps.rs index 3db616a..b730d4b 100644 --- a/crates/now-proto-pdu/src/exec/win_ps.rs +++ b/crates/now-proto-pdu/src/exec/win_ps.rs @@ -244,6 +244,10 @@ impl<'a> NowExecWinPsMsg<'a> { self.flags.contains(NowExecWinPsFlags::NO_PROFILE) } + pub fn is_non_interactive(&self) -> bool { + self.flags.contains(NowExecWinPsFlags::NON_INTERACTIVE) + } + pub fn apartment_state(&self) -> DecodeResult> { ComApartmentStateKind::from_flags(self.flags) } diff --git a/crates/now-proto-pdu/src/session/msg_box_rsp.rs b/crates/now-proto-pdu/src/session/msg_box_rsp.rs index 695b3eb..af777a3 100644 --- a/crates/now-proto-pdu/src/session/msg_box_rsp.rs +++ b/crates/now-proto-pdu/src/session/msg_box_rsp.rs @@ -1,5 +1,5 @@ use ironrdp_core::{ - ensure_fixed_part_size, Decode, DecodeResult, Encode, EncodeResult, IntoOwned, ReadCursor, WriteCursor, + cast_length, ensure_fixed_part_size, Decode, DecodeResult, Encode, EncodeResult, IntoOwned, ReadCursor, WriteCursor }; use crate::{ @@ -121,6 +121,12 @@ impl<'a> NowSessionMsgBoxRspMsg<'a> { self.status.to_result().map(|_| self.response) } + // LINTS: Overall message size is validated in the constructor/decode method + #[allow(clippy::arithmetic_side_effects)] + fn body_size(&self) -> usize { + Self::FIXED_PART_SIZE + self.status.size() + } + pub(super) fn decode_from_body(_header: NowHeader, src: &mut ReadCursor<'a>) -> DecodeResult { ensure_fixed_part_size!(in: src); @@ -140,7 +146,7 @@ impl<'a> NowSessionMsgBoxRspMsg<'a> { impl Encode for NowSessionMsgBoxRspMsg<'_> { fn encode(&self, dst: &mut WriteCursor<'_>) -> EncodeResult<()> { let header = NowHeader { - size: u32::try_from(Self::FIXED_PART_SIZE).expect("always fits in u32"), + size: cast_length!("size", self.body_size())?, class: NowMessageClass::SESSION, kind: NowSessionMessageKind::MSGBOX_RSP.0, flags: 0, @@ -162,7 +168,7 @@ impl Encode for NowSessionMsgBoxRspMsg<'_> { } fn size(&self) -> usize { - NowHeader::FIXED_PART_SIZE + Self::FIXED_PART_SIZE + self.status.size() + NowHeader::FIXED_PART_SIZE + self.body_size() } } diff --git a/crates/now-proto-testsuite/tests/proto/channel.rs b/crates/now-proto-testsuite/tests/proto/channel.rs index f3ae7d0..f1e1bb9 100644 --- a/crates/now-proto-testsuite/tests/proto/channel.rs +++ b/crates/now-proto-testsuite/tests/proto/channel.rs @@ -54,6 +54,14 @@ fn roundtrip_channel_capset_simple() { assert!(actual.heartbeat_interval().is_none()); } +#[test] +fn roundtrip_channel_capset_too_small_heartbeat_interval() { + // Sanity check should fail + NowChannelCapsetMsg::default() + .with_heartbeat_interval(time::Duration::from_secs(3)) + .unwrap_err(); +} + #[test] fn roundtrip_channel_capset_too_big_heartbeat_interval() { // Sanity check should fail diff --git a/crates/now-proto-testsuite/tests/proto/session.rs b/crates/now-proto-testsuite/tests/proto/session.rs index 4232ed0..eefaa8c 100644 --- a/crates/now-proto-testsuite/tests/proto/session.rs +++ b/crates/now-proto-testsuite/tests/proto/session.rs @@ -76,7 +76,7 @@ fn roundtrip_session_msgbox_rsp() { let decoded = now_msg_roundtrip( msg, expect![ - "[08, 00, 00, 00, 12, 04, 00, 00, 67, 45, 23, 01, 04, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00]" + "[12, 00, 00, 00, 12, 04, 00, 00, 67, 45, 23, 01, 04, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00]" ], ); @@ -94,14 +94,12 @@ fn roundtrip_session_msgbox_rsp_error() { let msg = NowSessionMsgBoxRspMsg::new_error( 0x01234567, NowStatusError::from(NowStatusErrorKind::Now(NowProtoError::NotImplemented)) - .with_message("err") - .unwrap(), ) .unwrap(); let decoded = now_msg_roundtrip( msg, - expect!["[08, 00, 00, 00, 12, 04, 00, 00, 67, 45, 23, 01, 00, 00, 00, 00, 03, 00, 01, 00, 07, 00, 00, 00, 03, 65, 72, 72, 00]"], + expect!["[12, 00, 00, 00, 12, 04, 00, 00, 67, 45, 23, 01, 00, 00, 00, 00, 01, 00, 01, 00, 07, 00, 00, 00, 00, 00]"], ); let actual = match decoded { @@ -113,7 +111,5 @@ fn roundtrip_session_msgbox_rsp_error() { assert_eq!( actual.to_result().unwrap_err(), NowStatusError::from(NowStatusErrorKind::Now(NowProtoError::NotImplemented)) - .with_message("err") - .unwrap() ); } diff --git a/docs/NOW-spec.md b/docs/NOW-spec.md index 53b0eb3..55d5f30 100644 --- a/docs/NOW-spec.md +++ b/docs/NOW-spec.md @@ -316,7 +316,7 @@ closed if protocol versions are not compatible. | Flag | Meaning | |-------|---------| -| NOW_CHANNEL_SET_HEATBEAT
0x0001 | Set if `heartbeat` specify channel heartbeat interval. | +| NOW_CHANNEL_SET_HEARTBEAT
0x0001 | Set if `heartbeat` specify channel heartbeat interval. | **versionMajor (1 byte)**: Major protocol version. Breaking changes in protocol should increment major version; Protocol implementations with different major version are not compatible. @@ -352,7 +352,7 @@ increment major version; Protocol implementations with different major version a **heartbeatInterval (4 bytes, optional)**: A 32-bit unsigned integer, which represents periodic heartbeat interval *hint* for a server (60 seconds by default). -Disables periodic heartbeat if set to `0`. Ignored if `NOW_CHANNEL_SET_HEATBEAT` is not set. +Disables periodic heartbeat if set to `0`. Ignored if `NOW_CHANNEL_SET_HEARTBEAT` is not set. #### NOW_CHANNEL_HEARTBEAT_MSG