fix: inject socket local address for the client addr (#759)

We used to inject the resolved target server address, but that is not
what is expected. Server typically ignores this field so this was not a
problem up until now.

Author: CBenoit

Cargo.lock

@@ -77,6 +77,7 @@ semver = "1"
 raw-window-handle = "0.6"
 uuid = { version = "1.16" }
 x509-cert = { version = "0.2", default-features = false, features = ["std"] }
+url = "2"
 
 [target.'cfg(windows)'.dependencies]
 windows = { version = "0.61", features = ["Win32_Foundation"] }

crates/ironrdp-client/Cargo.toml

@@ -8,6 +8,7 @@ use ironrdp::connector::{self, Credentials};
 use ironrdp::pdu::rdp::capability_sets::MajorPlatformType;
 use ironrdp::pdu::rdp::client_info::PerformanceFlags;
 use tap::prelude::*;
+use url::Url;
 
 const DEFAULT_WIDTH: u16 = 1920;
 const DEFAULT_HEIGHT: u16 = 1080;
@@ -131,7 +132,7 @@ impl From<&Destination> for connector::ServerName {
 
 #[derive(Clone, Debug)]
 pub struct RDCleanPathConfig {
-    pub url: String,
+    pub url: Url,
     pub auth_token: String,
 }
 
@@ -161,7 +162,7 @@ struct Args {
 
     /// Proxy URL to connect to for the RDCleanPath
     #[clap(long, requires("rdcleanpath_token"))]
-    rdcleanpath_url: Option<String>,
+    rdcleanpath_url: Option<Url>,
 
     /// Authentication token to insert in the RDCleanPath packet
     #[clap(long, requires("rdcleanpath_url"))]

crates/ironrdp-client/src/config.rs

@@ -121,18 +121,17 @@ async fn connect(
 ) -> ConnectorResult<(ConnectionResult, UpgradedFramed)> {
     let dest = format!("{}:{}", config.destination.name(), config.destination.port());
 
-    let stream = TcpStream::connect(dest)
+    let socket = TcpStream::connect(dest)
         .await
         .map_err(|e| connector::custom_err!("TCP connect", e))?;
 
-    let server_addr = stream
-        .peer_addr()
-        .map_err(|e| connector::custom_err!("Peer address", e))?;
+    let client_addr = socket
+        .local_addr()
+        .map_err(|e| connector::custom_err!("get socket local address", e))?;
 
-    let mut framed = ironrdp_tokio::TokioFramed::new(stream);
+    let mut framed = ironrdp_tokio::TokioFramed::new(socket);
 
-    let mut connector = connector::ClientConnector::new(config.connector.clone())
-        .with_client_addr(server_addr)
+    let mut connector = connector::ClientConnector::new(config.connector.clone(), client_addr)
         .with_static_channel(
             ironrdp::dvc::DrdynvcClient::new().with_dynamic_channel(DisplayControlClient::new(|_| Ok(Vec::new()))),
         )
@@ -184,15 +183,34 @@ async fn connect_ws(
     rdcleanpath: &RDCleanPathConfig,
     cliprdr_factory: Option<&(dyn CliprdrBackendFactory + Send)>,
 ) -> ConnectorResult<(ConnectionResult, UpgradedFramed)> {
-    let (ws, _) = tokio_tungstenite::connect_async(&rdcleanpath.url)
+    let hostname = rdcleanpath
+        .url
+        .host_str()
+        .ok_or_else(|| connector::general_err!("host missing from the URL"))?;
+
+    let port = rdcleanpath.url.port_or_known_default().unwrap_or(443);
+
+    let socket = TcpStream::connect((hostname, port))
+        .await
+        .map_err(|e| connector::custom_err!("TCP connect", e))?;
+
+    socket
+        .set_nodelay(true)
+        .map_err(|e| connector::custom_err!("set TCP_NODELAY", e))?;
+
+    let client_addr = socket
+        .local_addr()
+        .map_err(|e| connector::custom_err!("get socket local address", e))?;
+
+    let (ws, _) = tokio_tungstenite::client_async_tls(rdcleanpath.url.as_str(), socket)
         .await
         .map_err(|e| connector::custom_err!("WS connect", e))?;
 
     let ws = crate::ws::websocket_compat(ws);
 
     let mut framed = ironrdp_tokio::TokioFramed::new(ws);
 
-    let mut connector = connector::ClientConnector::new(config.connector.clone())
+    let mut connector = connector::ClientConnector::new(config.connector.clone(), client_addr)
         .with_static_channel(
             ironrdp::dvc::DrdynvcClient::new().with_dynamic_channel(DisplayControlClient::new(|_| Ok(Vec::new()))),
         )
@@ -312,7 +330,7 @@ where
 
         debug!(message = ?rdcleanpath_res, "Received RDCleanPath PDU");
 
-        let (x224_connection_response, server_cert_chain, server_addr) = match rdcleanpath_res
+        let (x224_connection_response, server_cert_chain) = match rdcleanpath_res
             .into_enum()
             .map_err(|e| connector::custom_err!("invalid RDCleanPath PDU", e))?
         {
@@ -324,19 +342,13 @@ where
             ironrdp_rdcleanpath::RDCleanPath::Response {
                 x224_connection_response,
                 server_cert_chain,
-                server_addr,
-            } => (x224_connection_response, server_cert_chain, server_addr),
+                server_addr: _,
+            } => (x224_connection_response, server_cert_chain),
             ironrdp_rdcleanpath::RDCleanPath::Err(error) => {
                 return Err(connector::custom_err!("received an RDCleanPath error", error));
             }
         };
 
-        let server_addr = server_addr
-            .parse()
-            .map_err(|e| connector::custom_err!("failed to parse server address sent by proxy", e))?;
-
-        connector.attach_client_addr(server_addr);
-
         let connector::ClientConnectorState::ConnectionInitiationWaitConfirm { .. } = connector.state else {
             return Err(connector::general_err!("invalid connector state (wait confirm)"));
         };

crates/ironrdp-client/src/rdp.rs

@@ -123,32 +123,21 @@ impl State for ClientConnectorState {
 pub struct ClientConnector {
     pub config: Config,
     pub state: ClientConnectorState,
-    pub client_addr: Option<SocketAddr>,
+    /// The client address to be used in the Client Info PDU.
+    pub client_addr: SocketAddr,
     pub static_channels: StaticChannelSet,
 }
 
 impl ClientConnector {
-    pub fn new(config: Config) -> Self {
+    pub fn new(config: Config, client_addr: SocketAddr) -> Self {
         Self {
             config,
             state: ClientConnectorState::ConnectionInitiationSendRequest,
-            client_addr: None,
+            client_addr,
             static_channels: StaticChannelSet::new(),
         }
     }
 
-    /// Sets the client address to be used in the Client Info PDU.
-    #[must_use]
-    pub fn with_client_addr(mut self, addr: SocketAddr) -> Self {
-        self.client_addr = Some(addr);
-        self
-    }
-
-    /// Sets the client address to be used in the Client Info PDU.
-    pub fn attach_client_addr(&mut self, addr: SocketAddr) {
-        self.client_addr = Some(addr);
-    }
-
     #[must_use]
     pub fn with_static_channel<T>(mut self, channel: T) -> Self
     where
@@ -448,12 +437,7 @@ impl Sequence for ClientConnector {
             } => {
                 debug!("Secure Settings Exchange");
 
-                let client_addr = self
-                    .client_addr
-                    .as_ref()
-                    .ok_or_else(|| general_err!("client address is missing"))?;
-
-                let client_info = create_client_info_pdu(&self.config, client_addr);
+                let client_info = create_client_info_pdu(&self.config, &self.client_addr);
 
                 debug!(message = ?client_info, "Send");
 

crates/ironrdp-connector/src/connection.rs

@@ -195,10 +195,11 @@ where
             let client = tokio::task::spawn_local(async move {
                 let (tx, rx) = oneshot::channel();
                 ev.send(ServerEvent::GetLocalAddr(tx)).unwrap();
-                let addr = rx.await.unwrap().unwrap();
-                let tcp_stream = TcpStream::connect(addr).await.expect("TCP connect");
+                let server_addr = rx.await.unwrap().unwrap();
+                let tcp_stream = TcpStream::connect(server_addr).await.expect("TCP connect");
+                let client_addr = tcp_stream.local_addr().expect("local_addr");
                 let mut framed = ironrdp_tokio::TokioFramed::new(tcp_stream);
-                let mut connector = connector::ClientConnector::new(client_config).with_client_addr(addr);
+                let mut connector = connector::ClientConnector::new(client_config, client_addr);
                 let should_upgrade = ironrdp_async::connect_begin(&mut framed, &mut connector)
                     .await
                     .expect("begin connection");

crates/ironrdp-testsuite-extra/tests/tests.rs

@@ -5,6 +5,7 @@ use core::cell::RefCell;
 use core::num::NonZeroU32;
 use core::time::Duration;
 use std::borrow::Cow;
+use std::net::{Ipv4Addr, SocketAddrV4};
 use std::rc::Rc;
 
 use anyhow::Context as _;
@@ -919,7 +920,10 @@ async fn connect(
 ) -> Result<(connector::ConnectionResult, WebSocket), IronError> {
     let mut framed = ironrdp_futures::LocalFuturesFramed::new(ws);
 
-    let mut connector = ClientConnector::new(config);
+    // In web browser environments, we do not have an easy access to the local address of the socket.
+    let dummy_client_addr = std::net::SocketAddr::V4(SocketAddrV4::new(Ipv4Addr::LOCALHOST, 33899));
+
+    let mut connector = ClientConnector::new(config, dummy_client_addr);
 
     if let Some(clipboard_backend) = clipboard_backend {
         connector.attach_static_channel(CliprdrClient::new(Box::new(clipboard_backend)));
@@ -1031,16 +1035,16 @@ where
 
         debug!(message = ?rdcleanpath_res, "Received RDCleanPath PDU");
 
-        let (x224_connection_response, server_cert_chain, server_addr) =
+        let (x224_connection_response, server_cert_chain) =
             match rdcleanpath_res.into_enum().context("invalid RDCleanPath PDU")? {
                 ironrdp_rdcleanpath::RDCleanPath::Request { .. } => {
                     return Err(anyhow::Error::msg("received an unexpected RDCleanPath type (request)").into());
                 }
                 ironrdp_rdcleanpath::RDCleanPath::Response {
                     x224_connection_response,
                     server_cert_chain,
-                    server_addr,
-                } => (x224_connection_response, server_cert_chain, server_addr),
+                    server_addr: _,
+                } => (x224_connection_response, server_cert_chain),
                 ironrdp_rdcleanpath::RDCleanPath::Err(error) => {
                     return Err(
                         IronError::from(anyhow::Error::new(error).context("received an RDCleanPath error"))
@@ -1049,12 +1053,6 @@ where
                 }
             };
 
-        let server_addr = server_addr
-            .parse()
-            .context("failed to parse server address sent by proxy")?;
-
-        connector.attach_client_addr(server_addr);
-
         let connector::ClientConnectorState::ConnectionInitiationWaitConfirm { .. } = connector.state else {
             return Err(anyhow::Error::msg("invalid connector state (wait confirm)").into());
         };

crates/ironrdp-web/src/session.rs

@@ -237,9 +237,11 @@ fn connect(
         .set_read_timeout(Some(Duration::from_secs(3)))
         .expect("set_read_timeout call failed");
 
+    let client_addr = tcp_stream.local_addr().context("get socket local address")?;
+
     let mut framed = ironrdp_blocking::Framed::new(tcp_stream);
 
-    let mut connector = connector::ClientConnector::new(config).with_client_addr(server_addr);
+    let mut connector = connector::ClientConnector::new(config, client_addr);
 
     let should_upgrade = ironrdp_blocking::connect_begin(&mut framed, &mut connector).context("begin connection")?;
 

crates/ironrdp/examples/screenshot.rs

@@ -32,7 +32,7 @@ static async Task Main(string[] args)
                     var readPduTask = framed.ReadPdu();
                     Action? action = null;
                     byte[]? payload = null;
-                    if (readPduTask == await Task.WhenAny(readPduTask, Task.Delay(1000)))
+                    if (readPduTask == await Task.WhenAny(readPduTask, Task.Delay(2000)))
                     {
                         var pduReadTask = await readPduTask;
                         action = pduReadTask.Item1;

ffi/dotnet/Devolutions.IronRdp.ConnectExample/Program.cs

@@ -37,45 +37,31 @@ public unsafe ClientConnector(Raw.ClientConnector* handle)
         _inner = handle;
     }
 
+    /// <exception cref="IronRdpException"></exception>
     /// <returns>
     /// A <c>ClientConnector</c> allocated on Rust side.
     /// </returns>
-    public static ClientConnector New(Config config)
+    public static ClientConnector New(Config config, string clientAddr)
     {
         unsafe
         {
+            byte[] clientAddrBuf = DiplomatUtils.StringToUtf8(clientAddr);
+            nuint clientAddrBufLength = (nuint)clientAddrBuf.Length;
             Raw.Config* configRaw;
             configRaw = config.AsFFI();
             if (configRaw == null)
             {
                 throw new ObjectDisposedException("Config");
             }
-            Raw.ClientConnector* retVal = Raw.ClientConnector.New(configRaw);
-            return new ClientConnector(retVal);
-        }
-    }
-
-    /// <summary>
-    /// Must use
-    /// </summary>
-    /// <exception cref="IronRdpException"></exception>
-    public void WithClientAddr(string clientAddr)
-    {
-        unsafe
-        {
-            if (_inner == null)
-            {
-                throw new ObjectDisposedException("ClientConnector");
-            }
-            byte[] clientAddrBuf = DiplomatUtils.StringToUtf8(clientAddr);
-            nuint clientAddrBufLength = (nuint)clientAddrBuf.Length;
             fixed (byte* clientAddrBufPtr = clientAddrBuf)
             {
-                Raw.ConnectorFfiResultVoidBoxIronRdpError result = Raw.ClientConnector.WithClientAddr(_inner, clientAddrBufPtr, clientAddrBufLength);
+                Raw.ConnectorFfiResultBoxClientConnectorBoxIronRdpError result = Raw.ClientConnector.New(configRaw, clientAddrBufPtr, clientAddrBufLength);
                 if (!result.isOk)
                 {
                     throw new IronRdpException(new IronRdpError(result.Err));
                 }
+                Raw.ClientConnector* retVal = result.Ok;
+                return new ClientConnector(retVal);
             }
         }
     }