[ironrdp-dvc] Support multiple DVC channel instances with the same name

[CBenoit]

MS-RDPEUSB (and potentially other protocols) requires opening multiple DVC channels that share the same channel name but have distinct channel IDs. The current DynamicChannelSet in ironrdp-dvc uses channel name as its primary key, which makes this impossible.

Depends on: nothing

Tasks

• Add a DvcChannelListener trait (factory) to lib.rs that produces a new DvcClientProcessor instance per DYNVC_CREATE_REQ:

pub trait DvcChannelListener: Send {
  fn channel_name(&self) -> &str;

  /// Called for each new DYNVC_CREATE_REQ for this name.
  fn on_channel_creation(&mut self, channel_id: u32) -> Box<dyn DvcClientProcessor + Send>;
}

• Refactor DynamicChannelSet to store entries keyed by DynamicChannelId rather than DynamicChannelName; maintain a name → listener map for creation
• Update DrdynvcClient::process() to call the listener factory on DYNVC_CREATE_REQ instead of looking up a pre-registered processor by name
• Add DrdynvcClient::with_listener() / attach_listener() alongside the existing with_dynamic_channel() / attach_dynamic_channel() (keep the old API for single-instance channels)
• Add a take_new_channels() drain hook to DvcServerProcessor in server.rs; update DrdynvcServer::process() to drain it after each process() call and emit the corresponding DYNVC_CREATE_REQ PDUs

// Add a post-process drain hook on DvcServerProcessor:

pub trait DvcServerProcessor: DvcProcessor {
    /// Called by DrdynvcServer after each process().
    /// Return new processor instances to be opened as new DVC channels.
    fn take_new_channels(&mut self) -> Vec<Box<dyn DvcServerProcessor + Send>> {
        Vec::new()
    }
}

// Then in DrdynvcServer::process(), after calling c.processor.process(...):

// Drain any new channels the processor wants to open.
for new_proc in c.processor.take_new_channels() {
    let id = self.dynamic_channels.insert(DynamicChannel::new_boxed(new_proc));
    let channel_id = u32::try_from(id).expect("...");
    let channel = &self.dynamic_channels[id];
    let req = DrdynvcServerPdu::Create(CreateRequestPdu::new(
        channel_id,
        channel.processor.channel_name().into(),
    ));
    self.dynamic_channels[id].state = ChannelState::Creation;
    resp.push(as_svc_msg_with_flag(req)?);
}

// The URBDRC control processor would then:

fn process(&mut self, channel_id: u32, payload: &[u8]) -> PduResult<Vec<DvcMessage>> {
    // ...
    self.pending_new_channels.push(Box::new(UsbDeviceChannel::new()));
    Ok(vec![])
}

fn take_new_channels(&mut self) -> Vec<Box<dyn DvcServerProcessor + Send>> {
    self.pending_new_channels.drain(..).collect()
}

EDIT: See this comment: #1135 (comment)

Acceptance criteria

• A channel name can have multiple simultaneous open instances, each with its own independent processor and fragmentation buffer
• Existing single-instance channels (displaycontrol, cliprdr, etc.) continue to work unchanged via with_dynamic_channel()
• Server can open new DVC channels mid-session in response to processor requests
• Unit tests covering multi-instance creation, data routing by channel ID, and close handling

[uchouT]

Hi, I've implemented client-side, and I'm going to implement the server side, but my approach differs from the proposed take_new_channels() design.

My proposal for server-side

In ironrdp-dvc, simply add a public method on DrdynvcServer:

/// Creates a new DVC, returns CreateRequest PDU to send to client.
pub fn create_channel(&mut self, processor: impl DvcServerProcessor + 'static) -> PduResult<Vec<SvcMessage>>;

The application layer (ironrdp-server) calls this when needed, e.g. from dispatch_server_events():

ServerEvent::Usb(...) => {
    let Some(drdynvc) = self.get_svc_processor::<DrdynvcServer>() else { ... };
    let msgs = drdynvc.create_channel(UsbDeviceProcessor::new(...))?;
    // composite final msgs and write to stream
}

Explanation

Add a take_new_channels() drain hook to DvcServerProcessor in server.rs; update DrdynvcServer::process() to drain it after each process() call and emit the corresponding DYNVC_CREATE_REQ PDUs

Since RDPEDYC mandates that DVC creation is always server-initiated, I was wondering whether take_new_channels() might not be the best fit here — it seems to shift the timing control to the client side. Two concerns I had:

1. Passive-only trigger: It is drained inside process(), which is only called when client data arrives.

2. Responsibility inversion: It requires the processor to hold a pending channel queue, mixing protocol handling with channel lifecycle management — a concern that belongs to the application layer, not a Core Tier processor.

I think my proposal would avoid both — the application layer decides when to create channels, and DvcServerProcessor stays a pure protocol handler. But I'd love to hear your thoughts on this!

[uchouT]

A little more explanation.

It is convenient for cases where the client sends an RDP message requesting the server to issue a DYNVC_CREATE_REQ (which is exactly how RDPEUSB works). However, semantically, RDPEDYC defines DVC creation as always server-initiated. The take_new_channels() pattern only helps in that specific case — if another protocol requires the server to initiate creation on its own, the pattern would not apply.

More importantly, ironrdp-dvc is an RDPEDYC implementation. I don't think it should take responsibility for handling situations defined by upper-layer protocols, such as RDPEUSB's ADD_VIRTUAL_CHANNEL (which is sent from the client side). That should be the responsibility of the protocol implementor ( ironrdp-rdpeusb::server ).

For example, when the RDPEUSB server processor receives an ADD_VIRTUAL_CHANNEL request in process(), it should delegate the channel creation to the application layer via a backend trait, rather than pushing to an ironrdp-dvc-managed pending queue. It's a bit more wiring, but I think the layering is cleaner.

Benoît Cortier (@CBenoit) Marc-Andre Lureau (@elmarco)