feat(rdpdr): LockControl handling (#313)

- Add a new `UnsupportedPdu` kind to `PduError`, enhancing error handling capabilities in ironrdp-pdu.
  This new error kind allows for more specific error messages when encountering unsupported PDUs.
- Implement a macro `unsupported_pdu_err` for conveniently creating `PduError` instances with the `UnsupportedPdu` kind.
- Implement handling for `ServerDriveLockControlRequest` in ironrdp-rdpdr, including the definition of
  the struct and its decoding method. This adds support for processing lock control requests in the RDPDR protocol.
- Update `Rdpdr`'s `process` method to utilize the new `unsupported_pdu_err` when encountering unknown packet IDs, replacing the previous handling of unimplemented PDUs.
- Remove the `Unimplemented` variant from `RdpdrPdu` and `Component` enums, replacing it with `EmptyResponse` in `RdpdrPdu`.
  This change streamlines the handling of unimplemented or unexpected PDUs.
- Enhance `FileInformationClass` and `FileInformationClassLevel` with `Display` implementations, providing better logging and error message capabilities.

crates/ironrdp-pdu/src/lib.rs

@@ -52,6 +52,7 @@ pub enum PduErrorKind {
     InvalidMessage { field: &'static str, reason: &'static str },
     UnexpectedMessageType { got: u8 },
     UnsupportedVersion { got: u8 },
+    UnsupportedPdu { name: &'static str, value: String },
     Other { description: &'static str },
     Custom,
 }
@@ -74,6 +75,9 @@ impl fmt::Display for PduErrorKind {
             Self::UnsupportedVersion { got } => {
                 write!(f, "unsupported version ({got})")
             }
+            Self::UnsupportedPdu { name, value } => {
+                write!(f, "unsupported {name} ({value})")
+            }
             Self::Other { description } => {
                 write!(f, "{description}")
             }
@@ -89,6 +93,7 @@ pub trait PduErrorExt {
     fn invalid_message(context: &'static str, field: &'static str, reason: &'static str) -> Self;
     fn unexpected_message_type(context: &'static str, got: u8) -> Self;
     fn unsupported_version(context: &'static str, got: u8) -> Self;
+    fn unsupported_pdu(context: &'static str, name: &'static str, value: String) -> Self;
     fn other(context: &'static str, description: &'static str) -> Self;
     fn custom<E>(context: &'static str, e: E) -> Self
     where
@@ -112,6 +117,10 @@ impl PduErrorExt for PduError {
         Self::new(context, PduErrorKind::UnsupportedVersion { got })
     }
 
+    fn unsupported_pdu(context: &'static str, name: &'static str, value: String) -> Self {
+        Self::new(context, PduErrorKind::UnsupportedPdu { name, value })
+    }
+
     fn other(context: &'static str, description: &'static str) -> Self {
         Self::new(context, PduErrorKind::Other { description })
     }

crates/ironrdp-pdu/src/macros.rs

@@ -82,6 +82,25 @@ macro_rules! unsupported_version_err {
     }};
 }
 
+/// Creates a `PduError` with `UnsupportedPdu` kind
+/// Shorthand for
+/// ```rust
+/// <ironrdp_pdu::PduError as ironrdp_pdu::PduErrorExt>::unsupported_pdu(context, name, value)
+/// ```
+/// and
+/// ```rust
+/// <ironrdp_pdu::PduError as ironrdp_pdu::PduErrorExt>::unsupported_pdu(Self::NAME, name, value)
+/// ```
+#[macro_export]
+macro_rules! unsupported_pdu_err {
+    ( $context:expr, $name:expr, $value:expr $(,)? ) => {{
+        <$crate::PduError as $crate::PduErrorExt>::unsupported_pdu($context, $name, $value)
+    }};
+    ( $name:expr, $value:expr $(,)? ) => {{
+        unsupported_pdu_err!(Self::NAME, $name, $value)
+    }};
+}
+
 /// Creates a `PduError` with `Other` kind
 ///
 /// Shorthand for

crates/ironrdp-rdpdr/src/lib.rs

@@ -187,9 +187,9 @@ impl StaticVirtualChannelProcessor for Rdpdr {
         CompressionCondition::WhenRdpDataIsCompressed
     }
 
-    fn process(&mut self, payload: &[u8]) -> PduResult<Vec<SvcMessage>> {
-        let mut payload = ReadCursor::new(payload);
-        let pdu = decode_cursor::<RdpdrPdu>(&mut payload)?;
+    fn process(&mut self, src: &[u8]) -> PduResult<Vec<SvcMessage>> {
+        let mut src = ReadCursor::new(src);
+        let pdu = decode_cursor::<RdpdrPdu>(&mut src)?;
         debug!("received {:?}", pdu);
 
         match pdu {
@@ -203,11 +203,7 @@ impl StaticVirtualChannelProcessor for Rdpdr {
                 self.handle_client_id_confirm()
             }
             RdpdrPdu::ServerDeviceAnnounceResponse(pdu) => self.handle_server_device_announce_response(pdu),
-            RdpdrPdu::DeviceIoRequest(pdu) => self.handle_device_io_request(pdu, &mut payload),
-            RdpdrPdu::Unimplemented => {
-                warn!(?pdu, "received unimplemented packet");
-                Ok(Vec::new())
-            }
+            RdpdrPdu::DeviceIoRequest(pdu) => self.handle_device_io_request(pdu, &mut src),
             // TODO: This can eventually become a `_ => {}` block, but being explicit for now
             // to make sure we don't miss handling new RdpdrPdu variants here during active development.
             RdpdrPdu::ClientNameRequest(_)
@@ -222,7 +218,8 @@ impl StaticVirtualChannelProcessor for Rdpdr {
             | RdpdrPdu::ClientDriveQueryVolumeInformationResponse(_)
             | RdpdrPdu::DeviceReadResponse(_)
             | RdpdrPdu::DeviceWriteResponse(_)
-            | RdpdrPdu::ClientDriveSetInformationResponse(_) => Err(other_err!("Rdpdr", "received unexpected packet")),
+            | RdpdrPdu::ClientDriveSetInformationResponse(_)
+            | RdpdrPdu::EmptyResponse => Err(other_err!("Rdpdr", "received unexpected packet")),
         }
     }
 }

crates/ironrdp-rdpdr/src/pdu/efs.rs

@@ -3,15 +3,15 @@
 //! [\[MS-RDPEFS\]: Remote Desktop Protocol: File System Virtual Channel Extension]: https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-rdpefs/34d9de58-b2b5-40b6-b970-f82d4603bdb5
 
 use core::fmt;
-use std::fmt::Debug;
+use std::fmt::{Debug, Display};
 use std::mem::size_of;
 
 use bitflags::bitflags;
 use ironrdp_pdu::cursor::{ReadCursor, WriteCursor};
 use ironrdp_pdu::utils::{decode_string, encoded_str_len, from_utf16_bytes, write_string_to_cursor, CharacterSet};
 use ironrdp_pdu::{
-    cast_length, ensure_fixed_part_size, ensure_size, invalid_message_err, read_padding, write_padding, PduError,
-    PduResult,
+    cast_length, ensure_fixed_part_size, ensure_size, invalid_message_err, read_padding, unsupported_pdu_err,
+    write_padding, PduError, PduResult,
 };
 
 use super::esc::rpce;
@@ -1390,6 +1390,7 @@ pub enum ServerDriveIoRequest {
     DeviceReadRequest(DeviceReadRequest),
     DeviceWriteRequest(DeviceWriteRequest),
     ServerDriveSetInformationRequest(ServerDriveSetInformationRequest),
+    ServerDriveLockControlRequest(ServerDriveLockControlRequest),
 }
 
 impl ServerDriveIoRequest {
@@ -1403,7 +1404,11 @@ impl ServerDriveIoRequest {
             MajorFunction::QueryVolumeInformation => {
                 Ok(ServerDriveQueryVolumeInformationRequest::decode(dev_io_req, src)?.into())
             }
-            MajorFunction::SetVolumeInformation => todo!(),
+            MajorFunction::SetVolumeInformation => Err(unsupported_pdu_err!(
+                "ServerDriveIoRequest::decode",
+                "MajorFunction",
+                "SetVolumeInformation".to_owned()
+            )), // FreeRDP doesn't implement this
             MajorFunction::QueryInformation => Ok(ServerDriveQueryInformationRequest::decode(dev_io_req, src)?.into()),
             MajorFunction::SetInformation => Ok(ServerDriveSetInformationRequest::decode(dev_io_req, src)?.into()),
             MajorFunction::DirectoryControl => match dev_io_req.minor_function {
@@ -1420,7 +1425,7 @@ impl ServerDriveIoRequest {
                     "invalid value"
                 )),
             },
-            MajorFunction::LockControl => todo!(),
+            MajorFunction::LockControl => Ok(ServerDriveLockControlRequest::decode(dev_io_req, src)?.into()),
         }
     }
 }
@@ -1485,6 +1490,12 @@ impl From<ServerDriveSetInformationRequest> for ServerDriveIoRequest {
     }
 }
 
+impl From<ServerDriveLockControlRequest> for ServerDriveIoRequest {
+    fn from(req: ServerDriveLockControlRequest) -> Self {
+        Self::ServerDriveLockControlRequest(req)
+    }
+}
+
 /// [2.2.3.3.1] Server Create Drive Request (DR_DRIVE_CREATE_REQ)
 /// and [2.2.1.4.1] Device Create Request (DR_CREATE_REQ)
 ///
@@ -1795,6 +1806,25 @@ impl FileInformationClassLevel {
     pub const FILE_ALLOCATION_INFORMATION: Self = Self(19);
 }
 
+impl Display for FileInformationClassLevel {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match *self {
+            FileInformationClassLevel::FILE_BASIC_INFORMATION => write!(f, "FileBasicInformation"),
+            FileInformationClassLevel::FILE_STANDARD_INFORMATION => write!(f, "FileStandardInformation"),
+            FileInformationClassLevel::FILE_ATTRIBUTE_TAG_INFORMATION => write!(f, "FileAttributeTagInformation"),
+            FileInformationClassLevel::FILE_DIRECTORY_INFORMATION => write!(f, "FileDirectoryInformation"),
+            FileInformationClassLevel::FILE_FULL_DIRECTORY_INFORMATION => write!(f, "FileFullDirectoryInformation"),
+            FileInformationClassLevel::FILE_BOTH_DIRECTORY_INFORMATION => write!(f, "FileBothDirectoryInformation"),
+            FileInformationClassLevel::FILE_NAMES_INFORMATION => write!(f, "FileNamesInformation"),
+            FileInformationClassLevel::FILE_END_OF_FILE_INFORMATION => write!(f, "FileEndOfFileInformation"),
+            FileInformationClassLevel::FILE_DISPOSITION_INFORMATION => write!(f, "FileDispositionInformation"),
+            FileInformationClassLevel::FILE_RENAME_INFORMATION => write!(f, "FileRenameInformation"),
+            FileInformationClassLevel::FILE_ALLOCATION_INFORMATION => write!(f, "FileAllocationInformation"),
+            _ => write!(f, "FileInformationClassLevel({})", self.0),
+        }
+    }
+}
+
 impl Debug for FileInformationClassLevel {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match *self {
@@ -1898,7 +1928,11 @@ impl FileInformationClass {
             Self::FullDirectory(f) => f.encode(dst),
             Self::Names(f) => f.encode(dst),
             Self::Directory(f) => f.encode(dst),
-            _ => unimplemented!("FileInformationClass::encode: {:?}", self),
+            _ => Err(unsupported_pdu_err!(
+                "FileInformationClass::encode",
+                "FileInformationClass",
+                self.to_string()
+            )),
         }
     }
 
@@ -1919,7 +1953,11 @@ impl FileInformationClass {
             FileInformationClassLevel::FILE_ALLOCATION_INFORMATION => {
                 Ok(FileAllocationInformation::decode(src)?.into())
             }
-            _ => unimplemented!("FileInformationClass::decode: {:?}", file_info_class_level),
+            _ => Err(unsupported_pdu_err!(
+                "FileInformationClass::decode",
+                "FileInformationClassLevel",
+                file_info_class_level.to_string()
+            )),
         }
     }
 
@@ -1940,6 +1978,24 @@ impl FileInformationClass {
     }
 }
 
+impl Display for FileInformationClass {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match self {
+            Self::Basic(_) => write!(f, "FileBasicInformation"),
+            Self::Standard(_) => write!(f, "FileStandardInformation"),
+            Self::AttributeTag(_) => write!(f, "FileAttributeTagInformation"),
+            Self::BothDirectory(_) => write!(f, "FileBothDirectoryInformation"),
+            Self::FullDirectory(_) => write!(f, "FileFullDirectoryInformation"),
+            Self::Names(_) => write!(f, "FileNamesInformation"),
+            Self::Directory(_) => write!(f, "FileDirectoryInformation"),
+            Self::EndOfFile(_) => write!(f, "FileEndOfFileInformation"),
+            Self::Disposition(_) => write!(f, "FileDispositionInformation"),
+            Self::Rename(_) => write!(f, "FileRenameInformation"),
+            Self::Allocation(_) => write!(f, "FileAllocationInformation"),
+        }
+    }
+}
+
 impl From<FileBasicInformation> for FileInformationClass {
     fn from(f: FileBasicInformation) -> Self {
         Self::Basic(f)
@@ -3278,9 +3334,13 @@ impl FileDispositionInformation {
     const FIXED_PART_SIZE: usize = 1; // DeletePending
 
     fn decode(src: &mut ReadCursor<'_>, length: usize) -> PduResult<Self> {
-        ensure_fixed_part_size!(in: src);
         // https://github.com/FreeRDP/FreeRDP/blob/dfa231c0a55b005af775b833f92f6bcd30363d77/channels/drive/client/drive_file.c#L684-L692
-        let delete_pending = if length != 0 { src.read_u8() } else { 1 };
+        let delete_pending = if length != 0 {
+            ensure_fixed_part_size!(in: src);
+            src.read_u8()
+        } else {
+            1
+        };
         Ok(Self { delete_pending })
     }
 
@@ -3382,3 +3442,25 @@ impl ClientDriveSetInformationResponse {
         + 4 // Length
     }
 }
+
+/// 2.2.3.3.12 Server Drive Lock Control Request (DR_DRIVE_LOCK_REQ)
+///
+/// [2.2.3.3.12]: https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-rdpefs/a96fe85c-620c-40ce-8858-a6bc38609b0a
+#[derive(Debug, Clone)]
+pub struct ServerDriveLockControlRequest {
+    pub device_io_request: DeviceIoRequest,
+}
+
+impl ServerDriveLockControlRequest {
+    const NAME: &str = "DR_DRIVE_LOCK_REQ";
+
+    fn decode(dev_io_req: DeviceIoRequest, src: &mut ReadCursor<'_>) -> PduResult<Self> {
+        // It's not quite clear why this is done this way, but it's what FreeRDP does:
+        // https://github.com/FreeRDP/FreeRDP/blob/dfa231c0a55b005af775b833f92f6bcd30363d77/channels/drive/client/drive_main.c#L600
+        ensure_size!(in: src, size: 4);
+        let _ = src.read_u32();
+        Ok(Self {
+            device_io_request: dev_io_req,
+        })
+    }
+}