Skip to content

feat(pdu): decode the Server Heartbeat PDU on the message channel - #1814

Merged
Marc-André Moreau (mamoreau-devolutions) merged 1 commit into
Devolutions:masterfrom
lamco-admin:feat/message-channel-heartbeat
Aug 28, 2026
Merged

Marc-André Moreau (mamoreau-devolutions) merged 1 commit into
Devolutions:masterfrom
lamco-admin:feat/message-channel-heartbeat

Conversation

@glamberson

Copy link
Copy Markdown
Contributor

Summary

Adds HeartbeatPdu (MS-RDPBCGR 2.2.16.1), decoded on the MCS message channel that #1347/#1348 wired up for Auto-Detect. The Heartbeat PDU is server-to-client only and defines no client response; the client is free to ignore count1/count2.

Fixes a real gap this exposed: the message-channel demux in ironrdp-session unconditionally decoded incoming traffic as an Auto-Detect Request PDU. A Heartbeat PDU arriving on the same channel would fail that decode and error the session. The demux now peeks the security-header flags first (masking SEC_RESET_SEQNO/SEC_IGNORE_SEQNO, which MS-RDPBCGR 2.2.8.1.1.2.1 says MUST be ignored, the same way MultitransportRequestPdu/MultitransportResponsePdu already do) and dispatches by the masked value. Anything it doesn't recognize is logged and ignored rather than treated as session-fatal, matching the forward-safe posture the connect-time demux in ironrdp-connector already has.

Multitransport Bootstrapping, the other optional feature the message channel carries, is already fully implemented (#1098); this PR is the Heartbeat half only.

Validation

cargo xtask check fmt/typos/tests/locks and wasm check all pass. lints has one pre-existing failure on master unrelated to this diff (clippy::result_large_err on ErrorInfoDisconnectHandle::disconnect, fix filed as #1812); this PR's own diff is clean under the same lints run.

New tests: HeartbeatPdu encode/decode round-trip plus the same seqno-flag and cross-PDU-discriminator coverage MultitransportRequestPdu has, in ironrdp-pdu; a Processor::process integration test in ironrdp-testsuite-core covering the full message-channel path (Heartbeat produces no output, Auto-Detect still works alongside it, and an unrecognized PDU is ignored rather than erroring).

@github-actions github-actions Bot added needs-review A human reviewer is the current next actor risk/unknown Risk could not be determined automatically; needs maintainer-level scrutiny scope/core Touches the core architectural tier size/M Size: up to 449 counted lines and 10 files; exceeds S in either measure labels Aug 27, 2026
@mamoreau-devolutions
Marc-André Moreau (mamoreau-devolutions) merged commit 4275b3d into Devolutions:master Aug 28, 2026
43 checks passed
Marc-André Moreau (mamoreau-devolutions) pushed a commit that referenced this pull request Aug 30, 2026
)

The server never sends Server Heartbeat PDUs (MS-RDPBCGR 2.2.16.1), even
though the client half of the feature is in place: HeartbeatPdu
encode/decode landed with #1814 and clients decode and ignore them. This
adds the send half, opt-in.

RdpServer::enable_heartbeat(HeartbeatConfig) arms a fifth joined future
in the client loop, following refresh_auto_reconnect_cookie's shape. It
only runs when three gates hold: the feature is enabled, the client
requested a message channel, and the client advertised
RNS_UD_CS_SUPPORT_HEARTBEAT_PDU in its GCC early capability flags. That
last gate needed the acceptor to expose those flags, so AcceptorResult
gains client_early_capability_flags, populated the same way #1453
exposed multitransport_flags.

Two spec details drove the shape. Heartbeats ride the MCS message
channel framed by a Basic Security Header carrying SEC_HEARTBEAT,
exactly like auto-detect after #1348, so encode_heartbeat mirrors
encode_autodetect_request. And 2.2.16.1 says heartbeats SHOULD only be
sent when no other PDU went out in the interval, so SharedWriter now
counts successful writes and the heartbeat tick skips the send when the
count moved: ordinary traffic doubles as the liveness signal.

The defaults (period 5 seconds, warning after 3 missed heartbeats,
reconnect after 5 more) are this library's own choice; the spec mandates
no values and the client MAY ignore both counts. A zero period is bumped
to one second to keep the timer well-formed.

No new tests: the wire format is already covered by the HeartbeatPdu
round-trip tests #1814 added in ironrdp-pdu, and ironrdp-server builds
no test harness for the client-loop plumbing this touches.

`cargo xtask check fmt/lints/tests/typos/locks` all pass.
@glamberson
Greg Lamberson (glamberson) deleted the feat/message-channel-heartbeat branch August 30, 2026 15:12
holovchenko pushed a commit to holovchenko/IronRDP that referenced this pull request Sep 25, 2026
…volutions#1814)

## Summary

Adds `HeartbeatPdu` (MS-RDPBCGR 2.2.16.1), decoded on the MCS message
channel that Devolutions#1347/Devolutions#1348 wired up for Auto-Detect. The Heartbeat PDU is
server-to-client only and defines no client response; the client is free
to ignore `count1`/`count2`.

Fixes a real gap this exposed: the message-channel demux in
`ironrdp-session` unconditionally decoded incoming traffic as an
Auto-Detect Request PDU. A Heartbeat PDU arriving on the same channel
would fail that decode and error the session. The demux now peeks the
security-header flags first (masking
`SEC_RESET_SEQNO`/`SEC_IGNORE_SEQNO`, which MS-RDPBCGR 2.2.8.1.1.2.1
says MUST be ignored, the same way
`MultitransportRequestPdu`/`MultitransportResponsePdu` already do) and
dispatches by the masked value. Anything it doesn't recognize is logged
and ignored rather than treated as session-fatal, matching the
forward-safe posture the connect-time demux in `ironrdp-connector`
already has.

Multitransport Bootstrapping, the other optional feature the message
channel carries, is already fully implemented (Devolutions#1098); this PR is the
Heartbeat half only.

## Validation

`cargo xtask check fmt/typos/tests/locks` and `wasm check` all pass.
`lints` has one pre-existing failure on master unrelated to this diff
(`clippy::result_large_err` on `ErrorInfoDisconnectHandle::disconnect`,
fix filed as Devolutions#1812); this PR's own diff is clean under the same lints
run.

New tests: `HeartbeatPdu` encode/decode round-trip plus the same
seqno-flag and cross-PDU-discriminator coverage
`MultitransportRequestPdu` has, in `ironrdp-pdu`; a `Processor::process`
integration test in `ironrdp-testsuite-core` covering the full
message-channel path (Heartbeat produces no output, Auto-Detect still
works alongside it, and an unrecognized PDU is ignored rather than
erroring).

(cherry picked from commit 4275b3d)

This branch was previously deployed

1 inactive deployment
llm-providers — d6e406b3 Deployed Aug 28, 2026 by glamberson via Classify pull request #4162
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-review A human reviewer is the current next actor risk/unknown Risk could not be determined automatically; needs maintainer-level scrutiny scope/core Touches the core architectural tier size/M Size: up to 449 counted lines and 10 files; exceeds S in either measure

Development

Successfully merging this pull request may close these issues.

2 participants