Skip to content

Commit

Permalink
fix(pdu-rust): fixes/improvements for NowProto 2nd iteration
Browse files Browse the repository at this point in the history
  • Loading branch information
pacmancoder committed Jan 22, 2025
1 parent 050fb03 commit b9ee6dc
Show file tree
Hide file tree
Showing 8 changed files with 36 additions and 14 deletions.
1 change: 1 addition & 0 deletions crates/now-proto-pdu/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 = []
9 changes: 6 additions & 3 deletions crates/now-proto-pdu/src/channel/capset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Expand Down Expand Up @@ -145,10 +145,13 @@ impl NowChannelCapsetMsg {
}

pub fn with_heartbeat_interval(mut self, interval: time::Duration) -> EncodeResult<Self> {
// 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"));
}

Expand Down
4 changes: 4 additions & 0 deletions crates/now-proto-pdu/src/exec/pwsh.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Option<ComApartmentStateKind>> {
ComApartmentStateKind::from_flags(self.flags)
}
Expand Down
4 changes: 4 additions & 0 deletions crates/now-proto-pdu/src/exec/win_ps.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Option<ComApartmentStateKind>> {
ComApartmentStateKind::from_flags(self.flags)
}
Expand Down
12 changes: 9 additions & 3 deletions crates/now-proto-pdu/src/session/msg_box_rsp.rs
Original file line number Diff line number Diff line change
@@ -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::{
Expand Down Expand Up @@ -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<Self> {
ensure_fixed_part_size!(in: src);

Expand All @@ -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,
Expand All @@ -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()
}
}

Expand Down
8 changes: 8 additions & 0 deletions crates/now-proto-testsuite/tests/proto/channel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 2 additions & 6 deletions crates/now-proto-testsuite/tests/proto/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]"
],
);

Expand All @@ -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 {
Expand All @@ -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()
);
}
4 changes: 2 additions & 2 deletions docs/NOW-spec.md
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,7 @@ closed if protocol versions are not compatible.

| Flag | Meaning |
|-------|---------|
| NOW_CHANNEL_SET_HEATBEAT<br>0x0001 | Set if `heartbeat` specify channel heartbeat interval. |
| NOW_CHANNEL_SET_HEARTBEAT<br>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.
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit b9ee6dc

Please sign in to comment.