diff --git a/Cargo.lock b/Cargo.lock index 6d865cf63..ca4c0ea84 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2531,7 +2531,6 @@ dependencies = [ name = "ironrdp-connector" version = "0.8.0" dependencies = [ - "arbitrary", "ironrdp-core", "ironrdp-error", "ironrdp-pdu", diff --git a/crates/ironrdp-connector/Cargo.toml b/crates/ironrdp-connector/Cargo.toml index e2cd7c659..de4f39e36 100644 --- a/crates/ironrdp-connector/Cargo.toml +++ b/crates/ironrdp-connector/Cargo.toml @@ -18,7 +18,6 @@ test = false [features] default = [] -arbitrary = ["dep:arbitrary", "ironrdp-pdu/arbitrary"] qoi = ["ironrdp-pdu/qoi"] qoiz = ["ironrdp-pdu/qoiz"] @@ -27,7 +26,6 @@ ironrdp-svc = { path = "../ironrdp-svc", version = "0.6" } # public ironrdp-core = { path = "../ironrdp-core", version = "0.1" } # public ironrdp-error = { path = "../ironrdp-error", version = "0.1" } # public ironrdp-pdu = { path = "../ironrdp-pdu", version = "0.7", features = ["std"] } # public -arbitrary = { version = "1", features = ["derive"], optional = true } # public sspi = { version = "0.19", features = ["scard"] } url = "2.5" # public rand = { version = "0.9", features = ["std"] } # TODO: dependency injection? diff --git a/crates/ironrdp-connector/src/channel_connection.rs b/crates/ironrdp-connector/src/channel_connection.rs index 249a3d352..199946aed 100644 --- a/crates/ironrdp-connector/src/channel_connection.rs +++ b/crates/ironrdp-connector/src/channel_connection.rs @@ -12,7 +12,6 @@ use crate::{ #[derive(Default, Debug)] #[non_exhaustive] -#[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))] pub enum ChannelConnectionState { #[default] Consumed, @@ -56,7 +55,6 @@ impl State for ChannelConnectionState { } #[derive(Debug)] -#[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))] pub struct ChannelConnectionSequence { pub state: ChannelConnectionState, pub channel_ids: Option>, diff --git a/crates/ironrdp-connector/src/connection_finalization.rs b/crates/ironrdp-connector/src/connection_finalization.rs index 56f28e764..7eccbcac3 100644 --- a/crates/ironrdp-connector/src/connection_finalization.rs +++ b/crates/ironrdp-connector/src/connection_finalization.rs @@ -11,7 +11,6 @@ use crate::{ConnectorResult, Sequence, State, Written, general_err, legacy, reas #[derive(Default, Debug, Copy, Clone)] #[non_exhaustive] -#[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))] pub enum ConnectionFinalizationState { #[default] Consumed, @@ -49,7 +48,6 @@ impl State for ConnectionFinalizationState { } #[derive(Debug, Copy, Clone)] -#[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))] pub struct ConnectionFinalizationSequence { pub state: ConnectionFinalizationState, pub io_channel_id: u16, diff --git a/crates/ironrdp-connector/src/lib.rs b/crates/ironrdp-connector/src/lib.rs index 2d76df8c9..d81e14410 100644 --- a/crates/ironrdp-connector/src/lib.rs +++ b/crates/ironrdp-connector/src/lib.rs @@ -34,7 +34,6 @@ pub use crate::license_exchange::LicenseCache; /// Provides user-friendly error messages for RDP negotiation failures #[derive(Clone, Copy, Debug, PartialEq, Eq)] -#[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))] pub struct NegotiationFailure(ironrdp_pdu::nego::FailureCode); impl NegotiationFailure { @@ -83,14 +82,12 @@ impl fmt::Display for NegotiationFailure { } #[derive(Debug, Clone, Copy, PartialEq, Eq)] -#[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))] pub struct DesktopSize { pub width: u16, pub height: u16, } #[derive(Debug, Clone)] -#[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))] pub struct BitmapConfig { pub lossy_compression: bool, pub color_depth: u32, @@ -98,7 +95,6 @@ pub struct BitmapConfig { } #[derive(Debug, Clone)] -#[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))] pub struct SmartCardIdentity { /// DER-encoded X509 certificate pub certificate: Vec, @@ -113,7 +109,6 @@ pub struct SmartCardIdentity { } #[derive(Debug, Clone)] -#[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))] pub enum Credentials { UsernamePassword { username: String, @@ -142,7 +137,6 @@ impl Credentials { } #[derive(Debug, Clone)] -#[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))] pub struct Config { /// The initial desktop size to request pub desktop_size: DesktopSize, @@ -236,11 +230,6 @@ pub struct Config { pub enable_audio_playback: bool, pub performance_flags: PerformanceFlags, - // `Arc` cannot be generated by `arbitrary`. Skipped via - // `arbitrary(default)` which yields `None`. License-cache-dependent paths - // are not exercised under fuzz; a fuzz-mode `LicenseCache` impl is a - // possible follow-up. - #[cfg_attr(feature = "arbitrary", arbitrary(default))] pub license_cache: Option>, // For Timezone Redirection to sync the server's timezone with the client's. @@ -304,7 +293,6 @@ impl State for () { } #[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] -#[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))] pub enum Written { Nothing, Size(core::num::NonZeroUsize), @@ -351,8 +339,6 @@ pub type ConnectorResult = Result; #[non_exhaustive] #[derive(Debug)] -// ConnectorErrorKind carries foreign error types (sspi::Error, ironrdp_error::Error) -// that do not implement Arbitrary, so this enum is intentionally not annotated. pub enum ConnectorErrorKind { Encode(ironrdp_core::EncodeError), Decode(ironrdp_core::DecodeError), diff --git a/crates/ironrdp-connector/src/license_exchange.rs b/crates/ironrdp-connector/src/license_exchange.rs index 4e3f5094c..c226cc088 100644 --- a/crates/ironrdp-connector/src/license_exchange.rs +++ b/crates/ironrdp-connector/src/license_exchange.rs @@ -15,7 +15,6 @@ use crate::{ConnectorResult, ConnectorResultExt as _, Sequence, State, Written, #[derive(Default, Debug)] #[non_exhaustive] -#[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))] pub enum LicenseExchangeState { #[default] Consumed, @@ -65,29 +64,6 @@ pub struct LicenseExchangeSequence { pub license_cache: Arc, } -// `license_cache` is `Arc` which cannot be derived. The hand-rolled impl -// hardcodes `NoopLicenseCache`, so license-cache-dependent paths are not exercised under fuzz. -// Reaching the license sequence is still useful coverage (MS-RDPELE is a historically -// interesting target). A fuzz-mode `LicenseCache` impl is a possible follow-up. -#[cfg(feature = "arbitrary")] -impl<'a> arbitrary::Arbitrary<'a> for LicenseExchangeSequence { - fn arbitrary(u: &mut arbitrary::Unstructured<'a>) -> arbitrary::Result { - Ok(Self { - state: LicenseExchangeState::arbitrary(u)?, - io_channel_id: u16::arbitrary(u)?, - username: String::arbitrary(u)?, - domain: Option::::arbitrary(u)?, - hardware_id: [ - u32::arbitrary(u)?, - u32::arbitrary(u)?, - u32::arbitrary(u)?, - u32::arbitrary(u)?, - ], - license_cache: Arc::new(NoopLicenseCache), - }) - } -} - // Use RefUnwindSafe so that types that embed LicenseCache remain UnwindSafe pub trait LicenseCache: Sync + Send + Debug + RefUnwindSafe { fn get_license(&self, license_info: LicenseInformation) -> ConnectorResult>>; diff --git a/fuzz/README.md b/fuzz/README.md index edf7c469e..0afe7c4d4 100644 --- a/fuzz/README.md +++ b/fuzz/README.md @@ -46,7 +46,6 @@ To verify the feature compiles cleanly for a single crate: ```shell cargo check -p ironrdp-pdu --features arbitrary -cargo check -p ironrdp-connector --features arbitrary ``` The feature is also compatible with the `no_std + alloc` build path: @@ -57,15 +56,13 @@ cargo check -p ironrdp-pdu --no-default-features --features arbitrary,alloc A handful of PDU types do not implement `Arbitrary`. They fall into two categories: -- **Types with non-derivable fields** (e.g., `Arc`, - `StaticChannelSet` keyed by `TypeId`). These are either skipped via - `#[arbitrary(default)]` on the containing struct's field, or hand-rolled - with a placeholder. +- **Types with non-derivable fields** (e.g., `StaticChannelSet` keyed by `TypeId`). + These are either skipped via `#[arbitrary(default)]` on the containing struct's + field, or hand-rolled with a placeholder. - **Error types that are not part of the wire-protocol surface** (e.g., - `ServerLicenseError`, `ConnectorErrorKind`). Most error enums fall here: - they are constructed locally rather than decoded from the wire, so the - fuzzer has no reason to generate them. The exception is wire-protocol - error PDUs such as `DisconnectProviderUltimatum` in `mcs.rs`, which do - implement `Arbitrary` because they are decoded from the wire. + `ServerLicenseError`). Most error enums fall here: they are constructed locally + rather than decoded from the wire, so the fuzzer has no reason to generate them. + The exception is wire-protocol error PDUs such as `DisconnectProviderUltimatum` + in `mcs.rs`, which do implement `Arbitrary` because they are decoded from the wire. Inline source comments mark each skip with the rationale.