-
Notifications
You must be signed in to change notification settings - Fork 246
feat(rdpeudp): add the RDP-UDP crate and the v1 handshake PDUs #1627
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| [package] | ||
| name = "ironrdp-rdpeudp" | ||
| version = "0.1.0" | ||
| readme = "README.md" | ||
| description = "RDP-UDP transport ([MS-RDPEUDP] and [MS-RDPEUDP2]) implementation for IronRDP" | ||
| edition.workspace = true | ||
| rust-version = "1.94" | ||
| license.workspace = true | ||
| homepage.workspace = true | ||
| repository.workspace = true | ||
| authors.workspace = true | ||
| keywords.workspace = true | ||
| categories.workspace = true | ||
|
|
||
| [lib] | ||
| doctest = false | ||
| # test = false # FIXME: turn off and keep tests in testsuite crates | ||
|
|
||
| [features] | ||
| # Matches `ironrdp-pdu` and `ironrdp-rdpeusb`: `no_std` unless a consumer asks | ||
| # for `std`. The `alloc` crate is not optional here, because the variable-length | ||
| # payloads this protocol carries are heap-allocated, so there is no useful build | ||
| # without it and no `alloc` feature is offered. | ||
| default = [] | ||
| std = ["ironrdp-core/std", "ironrdp-error/std"] | ||
| # Structure-aware fuzzing support, mirroring `ironrdp-pdu`. | ||
| # `arbitrary` needs the standard library, so structure-aware fuzzing implies `std`. | ||
| arbitrary = ["dep:arbitrary", "std", "bitflags/arbitrary"] | ||
|
|
||
| [dependencies] | ||
| arbitrary = { version = "1", features = ["derive"], optional = true } | ||
| ironrdp-core = { path = "../ironrdp-core", version = "0.2", features = ["alloc"] } # public | ||
| ironrdp-error = { path = "../ironrdp-error", version = "0.2", features = ["alloc"] } # public | ||
| bitflags = "2.11" # public | ||
|
|
||
| [lints] | ||
| workspace = true | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please, can you update this file using the following skills (in the .agents folder):
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| # IronRDP RDP-UDP | ||
|
|
||
| Reliable UDP transport implemented as described in [MS-RDPEUDP] and | ||
| [MS-RDPEUDP2]. | ||
|
|
||
| The two documents divide the work. [MS-RDPEUDP] defines the handshake that | ||
| opens a connection and negotiates a protocol version; from version 3 onward | ||
| that handshake leads into the data transfer defined by [MS-RDPEUDP2], which is | ||
| the one this crate implements. Note that the documents take opposite byte | ||
| orders, and number the bits in their diagrams in opposite directions. | ||
|
|
||
| Sans-I/O: the state machine is driven by datagrams and by a caller-supplied | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This crate currently exposes only PDU codecs; it has no state machine, caller-supplied instant, or transmit API. Reword this as planned design or scope the README to the implemented handshake-PDU layer so users are not promised unavailable functionality. Note LLM-assisted content (no human feedback). |
||
| instant, and returns the datagrams it wants sent. It performs no I/O and reads | ||
| no clock, so it can be driven by any runtime. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. blocking / medium: The README states that the MS-RDPEUDP2 data transfer is "the one this crate implements" and that the crate is sans-I/O with a state machine "driven by datagrams and by a caller-supplied instant" that "returns the datagrams it wants sent". Neither exists: the crate ships only v1 handshake PDUs, with no MS-RDPEUDP2 structures, no state machine, and no instant or transmit API. `lib.rs` pulls this file in via `#![cfg_attr(doc, doc = include_str!("../README.md"))]`, so it becomes the crate-level documentation and the published docs.rs page will advertise absent functionality. Describe what the crate contains today. |
||
|
|
||
| This crate is part of the [IronRDP] project. | ||
|
|
||
| [IronRDP]: https://github.com/Devolutions/IronRDP | ||
| [MS-RDPEUDP]: https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-rdpeudp/ | ||
| [MS-RDPEUDP2]: https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-rdpeudp2/ | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,110 @@ | ||
| //! Error types for the RDP-UDP connection state machine. | ||
| //! | ||
| //! These surface at the public API boundary of [`RdpeudpConnection`], and carry | ||
| //! both wire-level decode and encode failures from `ironrdp-core` and | ||
| //! protocol-level failures from the state machine itself. | ||
| //! | ||
| //! [`RdpeudpConnection`]: crate::RdpeudpConnection | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Note LLM-assisted content (no human feedback). |
||
|
|
||
| use core::fmt; | ||
|
|
||
| use ironrdp_core::{DecodeError, EncodeError}; | ||
|
|
||
| pub type RdpeudpResult<T> = Result<T, RdpeudpError>; | ||
|
|
||
| pub type RdpeudpError = ironrdp_error::Error<RdpeudpErrorKind>; | ||
|
|
||
| #[non_exhaustive] | ||
| #[derive(Debug)] | ||
| pub enum RdpeudpErrorKind { | ||
| /// A datagram could not be decoded. | ||
| Decode(DecodeError), | ||
|
|
||
| /// A datagram could not be encoded. | ||
| Encode(EncodeError), | ||
|
|
||
| /// The connection is not in a state where this operation is meaningful. | ||
| /// | ||
| /// Sending before the handshake completes, or handling a datagram after the | ||
| /// connection is closed, both land here. | ||
| InvalidState, | ||
|
|
||
| /// The send window is full. | ||
| /// | ||
| /// The caller should drain [`poll_transmit`] and wait for acknowledgements | ||
| /// to open the window before retrying. | ||
| /// | ||
| /// [`poll_transmit`]: crate::RdpeudpConnection::poll_transmit | ||
| SendBufferFull, | ||
|
|
||
| /// The connection has been closed, locally or by the idle timeout. | ||
| ConnectionClosed, | ||
|
|
||
| /// A datagram decoded cleanly but is not valid for the current state. | ||
| InvalidPacket { reason: &'static str }, | ||
| } | ||
|
|
||
| impl fmt::Display for RdpeudpErrorKind { | ||
| fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { | ||
| match self { | ||
| Self::Decode(_) => write!(f, "decode error"), | ||
| Self::Encode(_) => write!(f, "encode error"), | ||
| Self::InvalidState => write!(f, "connection is in the wrong state for this operation"), | ||
| Self::SendBufferFull => write!(f, "send buffer is full"), | ||
| Self::ConnectionClosed => write!(f, "connection is closed"), | ||
| Self::InvalidPacket { reason } => write!(f, "invalid packet: {reason}"), | ||
| } | ||
| } | ||
| } | ||
|
|
||
| #[cfg(feature = "std")] | ||
| impl core::error::Error for RdpeudpErrorKind { | ||
| fn source(&self) -> Option<&(dyn core::error::Error + 'static)> { | ||
| match self { | ||
| Self::Decode(error) => Some(error), | ||
| Self::Encode(error) => Some(error), | ||
| Self::InvalidState | Self::SendBufferFull | Self::ConnectionClosed | Self::InvalidPacket { .. } => None, | ||
| } | ||
| } | ||
| } | ||
|
|
||
| pub trait RdpeudpErrorExt { | ||
| fn decode(error: DecodeError) -> Self; | ||
| fn encode(error: EncodeError) -> Self; | ||
| fn invalid_state(context: &'static str) -> Self; | ||
| fn send_buffer_full(context: &'static str) -> Self; | ||
| fn connection_closed(context: &'static str) -> Self; | ||
| fn invalid_packet(context: &'static str, reason: &'static str) -> Self; | ||
| } | ||
|
|
||
| impl RdpeudpErrorExt for RdpeudpError { | ||
| #[track_caller] | ||
| fn decode(error: DecodeError) -> Self { | ||
| Self::new("decode error", RdpeudpErrorKind::Decode(error)) | ||
| } | ||
|
|
||
| #[track_caller] | ||
| fn encode(error: EncodeError) -> Self { | ||
| Self::new("encode error", RdpeudpErrorKind::Encode(error)) | ||
| } | ||
|
|
||
| #[track_caller] | ||
| fn invalid_state(context: &'static str) -> Self { | ||
| Self::new(context, RdpeudpErrorKind::InvalidState) | ||
| } | ||
|
|
||
| #[track_caller] | ||
| fn send_buffer_full(context: &'static str) -> Self { | ||
| Self::new(context, RdpeudpErrorKind::SendBufferFull) | ||
| } | ||
|
|
||
| #[track_caller] | ||
| fn connection_closed(context: &'static str) -> Self { | ||
| Self::new(context, RdpeudpErrorKind::ConnectionClosed) | ||
| } | ||
|
|
||
| #[track_caller] | ||
| fn invalid_packet(context: &'static str, reason: &'static str) -> Self { | ||
| Self::new(context, RdpeudpErrorKind::InvalidPacket { reason }) | ||
| } | ||
| } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. blocking / medium: The module is a public error API for a state machine this PR does not contain. Nothing in the crate returns `RdpeudpResult` or constructs any variant; `InvalidState`, `SendBufferFull`, `ConnectionClosed` and `InvalidPacket` describe a connection lifecycle that does not exist, and the six `RdpeudpErrorExt` constructors have no callers. Its docs link `crate::RdpeudpConnection` and `crate::RdpeudpConnection::poll_transmit`, neither of which exists, so rustdoc reports broken intra-doc links and readers are pointed at an absent type. This is a compatibility-bearing surface shaped with no consumer to validate it; it belongs in the PR that adds the state machine. |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| #![cfg_attr(doc, doc = include_str!("../README.md"))] | ||
| #![cfg_attr(not(feature = "std"), no_std)] | ||
| #![forbid(unsafe_code)] | ||
|
|
||
| extern crate alloc; | ||
|
|
||
| pub mod error; | ||
| pub mod pdu; | ||
|
|
||
| pub use self::error::{RdpeudpError, RdpeudpErrorExt, RdpeudpErrorKind, RdpeudpResult}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
blocking / medium: The `arbitrary` feature and the eight `derive(arbitrary::Arbitrary)` attributes across the crate have no consumer. `crates/ironrdp-fuzzing` is not updated, `fuzz/Cargo.toml` gains no target, and no workspace build enables the feature (clippy runs `--features helper,__bench`, and feature unification only turns on `std`), so nothing ever compiles this configuration and breakage is silent. The convention it cites is the opposite: ironrdp-pdu and ironrdp-egfx expose `arbitrary` and are wired into ironrdp-fuzzing with a fuzz target. A test doc comment in tests/rdpeudp/pdu_v1_syn.rs:231 also credits a `rdpeudp_pdu_round_trip` fuzz oracle that is not in the tree. Either land the fuzz target with the feature or drop both.