[GH-ISSUE #1074] [resolver] custom socket connect calls #597

Closed
opened 2026-03-15 23:21:45 +03:00 by kerem · 12 comments
Owner

Originally created by @balboah on GitHub (Apr 14, 2020).
Original GitHub issue: https://github.com/hickory-dns/hickory-dns/issues/1074

I'm looking for a way to be able to customize how the connections are created in the https/tls and possibly later clear text resolvers (I need to bind it to a specific network interface).

What do you think if I try to implement a new dialer param like for example in https_client_stream,

enum HttpsClientConnectState {
    ConnectTcp {
        name_server: SocketAddr,
        tls: Option<TlsConfig>,
        dialer: Option<Box<dyn Fn(SocketAddr) -> TokioTcpStream>>,
    },
}

which would then be checked for to be replacing the currently existing let connect = Box::pin(TokioTcpStream::connect(name_server));

Or would you reject this type of flexibility for trust-dns itself?

Originally created by @balboah on GitHub (Apr 14, 2020). Original GitHub issue: https://github.com/hickory-dns/hickory-dns/issues/1074 I'm looking for a way to be able to customize how the connections are created in the https/tls and possibly later clear text resolvers (I need to bind it to a specific network interface). What do you think if I try to implement a new **dialer** param like for example in https_client_stream, ```rust enum HttpsClientConnectState { ConnectTcp { name_server: SocketAddr, tls: Option<TlsConfig>, dialer: Option<Box<dyn Fn(SocketAddr) -> TokioTcpStream>>, }, } ``` which would then be checked for to be replacing the currently existing `let connect = Box::pin(TokioTcpStream::connect(name_server));` Or would you reject this type of flexibility for trust-dns itself?
kerem 2026-03-15 23:21:45 +03:00
Author
Owner

@balboah commented on GitHub (Apr 14, 2020):

some work in progress if you want to see what I'm heading towards:

diff --git a/crates/client/src/https_client_connection.rs b/crates/client/src/https_client_connection.rs
index cee7e668..de4e94cc 100644
--- a/crates/client/src/https_client_connection.rs
+++ b/crates/client/src/https_client_connection.rs
@@ -53,9 +53,9 @@ impl ClientConnection for HttpsClientConnection {
         _signer: Option<Arc<Signer>>,
     ) -> Self::SenderFuture {
         // TODO: maybe signer needs to be applied in https...
-        let https_builder =
-            HttpsClientStreamBuilder::with_client_config(Arc::new(self.client_config.clone()));
-        https_builder.build(self.name_server, self.dns_name.clone())
+        HttpsClientStreamBuilder::new()
+            .with_client_config(Arc::new(self.client_config.clone()))
+            .build(self.name_server, self.dns_name.clone())
     }
 }
 
diff --git a/crates/https/src/https_client_stream.rs b/crates/https/src/https_client_stream.rs
index 58611e5b..94fe839a 100644
--- a/crates/https/src/https_client_stream.rs
+++ b/crates/https/src/https_client_stream.rs
@@ -5,6 +5,7 @@
 // http://opensource.org/licenses/MIT>, at your option. This file may not be
 // copied, modified, or distributed except according to those terms.
 
+use std::fmt::Debug;
 use std::fmt::{self, Display};
 use std::io;
 use std::mem;
@@ -284,26 +285,45 @@ impl Stream for HttpsClientStream {
     }
 }
 
+// Dialer can be used to customize how new streams are made.
+pub trait Dialer: Send + Sync + Debug {
+    fn dial(&self, addr: SocketAddr) -> Pin<Box<dyn Future<Output = TokioTcpStream>>>;
+}
+
 /// A HTTPS connection builder for DNS-over-HTTPS
 #[derive(Clone)]
 pub struct HttpsClientStreamBuilder {
     client_config: Arc<ClientConfig>,
+    dialer: Option<Arc<dyn Dialer>>,
 }
 
 impl HttpsClientStreamBuilder {
-    /// Return a new builder for DNS-over-HTTPS
+    /// Return a new builder for DNS-over-HTTPS with default config
     pub fn new() -> HttpsClientStreamBuilder {
         let mut client_config = ClientConfig::new();
         client_config.alpn_protocols.push(ALPN_H2.to_vec());
 
         HttpsClientStreamBuilder {
             client_config: Arc::new(client_config),
+            dialer: None,
         }
     }
 
-    /// Constructs a new TlsStreamBuilder with the associated ClientConfig
-    pub fn with_client_config(client_config: Arc<ClientConfig>) -> Self {
-        HttpsClientStreamBuilder { client_config }
+    /// Override the default ClientConfig
+    pub fn with_client_config(mut self, client_config: Arc<ClientConfig>) -> Self {
+        assert!(client_config
+            .alpn_protocols
+            .iter()
+            .any(|protocol| *protocol == ALPN_H2.to_vec()));
+
+        self.client_config = client_config;
+        self
+    }
+
+    // Customize the dialer used for making new connections
+    pub fn with_dialer(mut self, dialer: Arc<dyn Dialer>) -> Self {
+        self.dialer = Some(dialer);
+        self
     }
 
     /// Creates a new HttpsStream to the specified name_server
@@ -314,12 +334,6 @@ impl HttpsClientStreamBuilder {
     /// * `dns_name` - The DNS name, Subject Public Key Info (SPKI) name, as associated to a certificate
     /// * `loop_handle` - The reactor Core handle
     pub fn build(self, name_server: SocketAddr, dns_name: String) -> HttpsClientConnect {
-        assert!(self
-            .client_config
-            .alpn_protocols
-            .iter()
-            .any(|protocol| *protocol == ALPN_H2.to_vec()));
-
         let tls = TlsConfig {
             client_config: self.client_config,
             dns_name: Arc::new(dns_name),
@@ -328,6 +342,7 @@ impl HttpsClientStreamBuilder {
         HttpsClientConnect(HttpsClientConnectState::ConnectTcp {
             name_server,
             tls: Some(tls),
+            dialer: self.dialer,
         })
     }
 }
@@ -360,6 +375,7 @@ enum HttpsClientConnectState {
     ConnectTcp {
         name_server: SocketAddr,
         tls: Option<TlsConfig>,
+        dialer: Option<Arc<dyn Dialer>>,
     },
     TcpConnecting {
         connect: Pin<Box<dyn Future<Output = io::Result<TokioTcpStream>> + Send>>,
@@ -402,8 +418,10 @@ impl Future for HttpsClientConnectState {
                 HttpsClientConnectState::ConnectTcp {
                     name_server,
                     ref mut tls,
+                    ref dialer,
                 } => {
                     debug!("tcp connecting to: {}", name_server);
+                    // TODO: match dialer
                     let connect = Box::pin(TokioTcpStream::connect(name_server));
                     HttpsClientConnectState::TcpConnecting {
                         connect,
@@ -546,7 +564,8 @@ mod tests {
         client_config.versions = versions;
         client_config.alpn_protocols.push(ALPN_H2.to_vec());
 
-        let https_builder = HttpsClientStreamBuilder::with_client_config(Arc::new(client_config));
+        let https_builder =
+            HttpsClientStreamBuilder::new().with_client_config(Arc::new(client_config));
         let connect = https_builder.build(google, "dns.google".to_string());
 
         // tokio runtime stuff...
diff --git a/crates/https/src/lib.rs b/crates/https/src/lib.rs
index 68601cf9..14e9ab48 100644
--- a/crates/https/src/lib.rs
+++ b/crates/https/src/lib.rs
@@ -32,6 +32,6 @@ pub use self::error::{Error as HttpsError, Result as HttpsResult};
 
 //pub use self::https_client_connection::{HttpsClientConnection, HttpsClientConnectionBuilder};
 pub use self::https_client_stream::{
-    HttpsClientConnect, HttpsClientResponse, HttpsClientStream, HttpsClientStreamBuilder,
+    Dialer, HttpsClientConnect, HttpsClientResponse, HttpsClientStream, HttpsClientStreamBuilder,
 };
 //pub use self::https_stream::{HttpsStream, HttpsStreamBuilder};
diff --git a/crates/resolver/src/config.rs b/crates/resolver/src/config.rs
index 9af87a05..2ecbb65a 100644
--- a/crates/resolver/src/config.rs
+++ b/crates/resolver/src/config.rs
@@ -10,10 +10,8 @@
 use std::fmt;
 use std::net::{IpAddr, Ipv4Addr, Ipv6Addr, SocketAddr};
 use std::ops::{Deref, DerefMut};
-use std::time::Duration;
-
-#[cfg(feature = "dns-over-rustls")]
 use std::sync::Arc;
+use std::time::Duration;
 
 use proto::rr::Name;
 #[cfg(feature = "dns-over-rustls")]
diff --git a/crates/resolver/src/https.rs b/crates/resolver/src/https.rs
index ae427a9e..cdc40e43 100644
--- a/crates/resolver/src/https.rs
+++ b/crates/resolver/src/https.rs
@@ -2,13 +2,14 @@ extern crate rustls;
 extern crate webpki_roots;
 
 use std::net::SocketAddr;
+use std::sync::Arc;
 
 use crate::tls::CLIENT_CONFIG;
 
 use proto::xfer::{DnsExchange, DnsExchangeConnect};
 use proto::TokioTime;
 use trust_dns_https::{
-    HttpsClientConnect, HttpsClientResponse, HttpsClientStream, HttpsClientStreamBuilder,
+    Dialer, HttpsClientConnect, HttpsClientResponse, HttpsClientStream, HttpsClientStreamBuilder,
 };
 
 use crate::config::TlsClientConfig;
@@ -18,13 +19,14 @@ pub(crate) fn new_https_stream(
     socket_addr: SocketAddr,
     dns_name: String,
     client_config: Option<TlsClientConfig>,
+    dialer: Option<Arc<dyn Dialer>>,
 ) -> DnsExchangeConnect<HttpsClientConnect, HttpsClientStream, HttpsClientResponse, TokioTime> {
     let client_config = client_config.map_or_else(
         || CLIENT_CONFIG.clone(),
         |TlsClientConfig(client_config)| client_config,
     );
 
-    let https_builder = HttpsClientStreamBuilder::with_client_config(client_config);
+    let https_builder = HttpsClientStreamBuilder::new().with_client_config(client_config);
     DnsExchange::connect(https_builder.build(socket_addr, dns_name))
 }
 
diff --git a/crates/resolver/src/name_server/connection_provider.rs b/crates/resolver/src/name_server/connection_provider.rs
index 621716e0..877fe36f 100644
--- a/crates/resolver/src/name_server/connection_provider.rs
+++ b/crates/resolver/src/name_server/connection_provider.rs
@@ -170,9 +170,15 @@ where
                 let tls_dns_name = config.tls_dns_name.clone().unwrap_or_default();
                 #[cfg(feature = "dns-over-rustls")]
                 let client_config = config.tls_config.clone();
-
-                let exchange =
-                    crate::https::new_https_stream(socket_addr, tls_dns_name, client_config);
+                // TODO: where do we get the dialer from?
+                // Config objects require serialization and copy traits.
+                let dialer = None;
+                let exchange = crate::https::new_https_stream(
+                    socket_addr,
+                    tls_dns_name,
+                    client_config,
+                    dialer,
+                );
                 ConnectionConnect::Https(exchange)
             }
             #[cfg(feature = "mdns")]
<!-- gh-comment-id:613454221 --> @balboah commented on GitHub (Apr 14, 2020): some work in progress if you want to see what I'm heading towards: <details> ```diff diff --git a/crates/client/src/https_client_connection.rs b/crates/client/src/https_client_connection.rs index cee7e668..de4e94cc 100644 --- a/crates/client/src/https_client_connection.rs +++ b/crates/client/src/https_client_connection.rs @@ -53,9 +53,9 @@ impl ClientConnection for HttpsClientConnection { _signer: Option<Arc<Signer>>, ) -> Self::SenderFuture { // TODO: maybe signer needs to be applied in https... - let https_builder = - HttpsClientStreamBuilder::with_client_config(Arc::new(self.client_config.clone())); - https_builder.build(self.name_server, self.dns_name.clone()) + HttpsClientStreamBuilder::new() + .with_client_config(Arc::new(self.client_config.clone())) + .build(self.name_server, self.dns_name.clone()) } } diff --git a/crates/https/src/https_client_stream.rs b/crates/https/src/https_client_stream.rs index 58611e5b..94fe839a 100644 --- a/crates/https/src/https_client_stream.rs +++ b/crates/https/src/https_client_stream.rs @@ -5,6 +5,7 @@ // http://opensource.org/licenses/MIT>, at your option. This file may not be // copied, modified, or distributed except according to those terms. +use std::fmt::Debug; use std::fmt::{self, Display}; use std::io; use std::mem; @@ -284,26 +285,45 @@ impl Stream for HttpsClientStream { } } +// Dialer can be used to customize how new streams are made. +pub trait Dialer: Send + Sync + Debug { + fn dial(&self, addr: SocketAddr) -> Pin<Box<dyn Future<Output = TokioTcpStream>>>; +} + /// A HTTPS connection builder for DNS-over-HTTPS #[derive(Clone)] pub struct HttpsClientStreamBuilder { client_config: Arc<ClientConfig>, + dialer: Option<Arc<dyn Dialer>>, } impl HttpsClientStreamBuilder { - /// Return a new builder for DNS-over-HTTPS + /// Return a new builder for DNS-over-HTTPS with default config pub fn new() -> HttpsClientStreamBuilder { let mut client_config = ClientConfig::new(); client_config.alpn_protocols.push(ALPN_H2.to_vec()); HttpsClientStreamBuilder { client_config: Arc::new(client_config), + dialer: None, } } - /// Constructs a new TlsStreamBuilder with the associated ClientConfig - pub fn with_client_config(client_config: Arc<ClientConfig>) -> Self { - HttpsClientStreamBuilder { client_config } + /// Override the default ClientConfig + pub fn with_client_config(mut self, client_config: Arc<ClientConfig>) -> Self { + assert!(client_config + .alpn_protocols + .iter() + .any(|protocol| *protocol == ALPN_H2.to_vec())); + + self.client_config = client_config; + self + } + + // Customize the dialer used for making new connections + pub fn with_dialer(mut self, dialer: Arc<dyn Dialer>) -> Self { + self.dialer = Some(dialer); + self } /// Creates a new HttpsStream to the specified name_server @@ -314,12 +334,6 @@ impl HttpsClientStreamBuilder { /// * `dns_name` - The DNS name, Subject Public Key Info (SPKI) name, as associated to a certificate /// * `loop_handle` - The reactor Core handle pub fn build(self, name_server: SocketAddr, dns_name: String) -> HttpsClientConnect { - assert!(self - .client_config - .alpn_protocols - .iter() - .any(|protocol| *protocol == ALPN_H2.to_vec())); - let tls = TlsConfig { client_config: self.client_config, dns_name: Arc::new(dns_name), @@ -328,6 +342,7 @@ impl HttpsClientStreamBuilder { HttpsClientConnect(HttpsClientConnectState::ConnectTcp { name_server, tls: Some(tls), + dialer: self.dialer, }) } } @@ -360,6 +375,7 @@ enum HttpsClientConnectState { ConnectTcp { name_server: SocketAddr, tls: Option<TlsConfig>, + dialer: Option<Arc<dyn Dialer>>, }, TcpConnecting { connect: Pin<Box<dyn Future<Output = io::Result<TokioTcpStream>> + Send>>, @@ -402,8 +418,10 @@ impl Future for HttpsClientConnectState { HttpsClientConnectState::ConnectTcp { name_server, ref mut tls, + ref dialer, } => { debug!("tcp connecting to: {}", name_server); + // TODO: match dialer let connect = Box::pin(TokioTcpStream::connect(name_server)); HttpsClientConnectState::TcpConnecting { connect, @@ -546,7 +564,8 @@ mod tests { client_config.versions = versions; client_config.alpn_protocols.push(ALPN_H2.to_vec()); - let https_builder = HttpsClientStreamBuilder::with_client_config(Arc::new(client_config)); + let https_builder = + HttpsClientStreamBuilder::new().with_client_config(Arc::new(client_config)); let connect = https_builder.build(google, "dns.google".to_string()); // tokio runtime stuff... diff --git a/crates/https/src/lib.rs b/crates/https/src/lib.rs index 68601cf9..14e9ab48 100644 --- a/crates/https/src/lib.rs +++ b/crates/https/src/lib.rs @@ -32,6 +32,6 @@ pub use self::error::{Error as HttpsError, Result as HttpsResult}; //pub use self::https_client_connection::{HttpsClientConnection, HttpsClientConnectionBuilder}; pub use self::https_client_stream::{ - HttpsClientConnect, HttpsClientResponse, HttpsClientStream, HttpsClientStreamBuilder, + Dialer, HttpsClientConnect, HttpsClientResponse, HttpsClientStream, HttpsClientStreamBuilder, }; //pub use self::https_stream::{HttpsStream, HttpsStreamBuilder}; diff --git a/crates/resolver/src/config.rs b/crates/resolver/src/config.rs index 9af87a05..2ecbb65a 100644 --- a/crates/resolver/src/config.rs +++ b/crates/resolver/src/config.rs @@ -10,10 +10,8 @@ use std::fmt; use std::net::{IpAddr, Ipv4Addr, Ipv6Addr, SocketAddr}; use std::ops::{Deref, DerefMut}; -use std::time::Duration; - -#[cfg(feature = "dns-over-rustls")] use std::sync::Arc; +use std::time::Duration; use proto::rr::Name; #[cfg(feature = "dns-over-rustls")] diff --git a/crates/resolver/src/https.rs b/crates/resolver/src/https.rs index ae427a9e..cdc40e43 100644 --- a/crates/resolver/src/https.rs +++ b/crates/resolver/src/https.rs @@ -2,13 +2,14 @@ extern crate rustls; extern crate webpki_roots; use std::net::SocketAddr; +use std::sync::Arc; use crate::tls::CLIENT_CONFIG; use proto::xfer::{DnsExchange, DnsExchangeConnect}; use proto::TokioTime; use trust_dns_https::{ - HttpsClientConnect, HttpsClientResponse, HttpsClientStream, HttpsClientStreamBuilder, + Dialer, HttpsClientConnect, HttpsClientResponse, HttpsClientStream, HttpsClientStreamBuilder, }; use crate::config::TlsClientConfig; @@ -18,13 +19,14 @@ pub(crate) fn new_https_stream( socket_addr: SocketAddr, dns_name: String, client_config: Option<TlsClientConfig>, + dialer: Option<Arc<dyn Dialer>>, ) -> DnsExchangeConnect<HttpsClientConnect, HttpsClientStream, HttpsClientResponse, TokioTime> { let client_config = client_config.map_or_else( || CLIENT_CONFIG.clone(), |TlsClientConfig(client_config)| client_config, ); - let https_builder = HttpsClientStreamBuilder::with_client_config(client_config); + let https_builder = HttpsClientStreamBuilder::new().with_client_config(client_config); DnsExchange::connect(https_builder.build(socket_addr, dns_name)) } diff --git a/crates/resolver/src/name_server/connection_provider.rs b/crates/resolver/src/name_server/connection_provider.rs index 621716e0..877fe36f 100644 --- a/crates/resolver/src/name_server/connection_provider.rs +++ b/crates/resolver/src/name_server/connection_provider.rs @@ -170,9 +170,15 @@ where let tls_dns_name = config.tls_dns_name.clone().unwrap_or_default(); #[cfg(feature = "dns-over-rustls")] let client_config = config.tls_config.clone(); - - let exchange = - crate::https::new_https_stream(socket_addr, tls_dns_name, client_config); + // TODO: where do we get the dialer from? + // Config objects require serialization and copy traits. + let dialer = None; + let exchange = crate::https::new_https_stream( + socket_addr, + tls_dns_name, + client_config, + dialer, + ); ConnectionConnect::Https(exchange) } #[cfg(feature = "mdns")] ``` </details>
Author
Owner

@bluejekyll commented on GitHub (Apr 14, 2020):

Ah, this is great. I've been wanting to get to doing this for a while, so thank you for tackling the work.

While I see the direction your going it, I'm trying to decide between the impl your heading towards, vs. just a new bind address for the connection being passed in? I think it would be a little cheaper to pass the address (default would be 0.0.0.0). So I'm curious if you have other reasons to want to pass in the function pointer?

Each connection type would benefit from having this ability, and I think as a generic configuration parameter too.

<!-- gh-comment-id:613527067 --> @bluejekyll commented on GitHub (Apr 14, 2020): Ah, this is great. I've been wanting to get to doing this for a while, so thank you for tackling the work. While I see the direction your going it, I'm trying to decide between the impl your heading towards, vs. just a new bind address for the connection being passed in? I think it would be a little cheaper to pass the address (default would be `0.0.0.0`). So I'm curious if you have other reasons to want to pass in the function pointer? Each connection type would benefit from having this ability, and I think as a generic configuration parameter too.
Author
Owner

@balboah commented on GitHub (Apr 14, 2020):

Are you able to bind to a specific IP while doing tcp connect in rust without unsafe code?
One reason why I do it as a function pointer is so that I can keep the unsafe code to myself.

It would be simpler if it could pass a listener address config wise, as I can’t put this in config/options when it’s not serializable like the rest of the fields.

<!-- gh-comment-id:613556971 --> @balboah commented on GitHub (Apr 14, 2020): Are you able to bind to a specific IP while doing tcp connect in rust without unsafe code? One reason why I do it as a function pointer is so that I can keep the unsafe code to myself. It would be simpler if it could pass a listener address config wise, as I can’t put this in config/options when it’s not serializable like the rest of the fields.
Author
Owner

@bluejekyll commented on GitHub (Apr 14, 2020):

Are you able to bind to a specific IP while doing tcp connect in rust without unsafe code?

Yes. Here's where the connection is coming from: https://github.com/bluejekyll/trust-dns/blob/master/crates/proto/src/tcp/tcp_client_stream.rs#L142

It would require us to pass in a new socket addr for that creation with the bind. It looks like we might need to switch over to using this function for the TokioStream creation: https://docs.rs/tokio/0.2.18/tokio/net/struct.TcpStream.html#method.from_std

To bind you'd need to follow an example like this: github.com/bluejekyll/trust-dns@3eaf3043d7/crates/proto/src/multicast/mdns_stream.rs (L183)

Which uses socket2 library for configuring the socket. I hope that helps. This definitely feels like something we should make sure the Tokio networking modules help make this more straightforward.

<!-- gh-comment-id:613585544 --> @bluejekyll commented on GitHub (Apr 14, 2020): > Are you able to bind to a specific IP while doing tcp connect in rust without unsafe code? Yes. Here's where the connection is coming from: https://github.com/bluejekyll/trust-dns/blob/master/crates/proto/src/tcp/tcp_client_stream.rs#L142 It would require us to pass in a new socket addr for that creation with the bind. It looks like we might need to switch over to using this function for the TokioStream creation: https://docs.rs/tokio/0.2.18/tokio/net/struct.TcpStream.html#method.from_std To bind you'd need to follow an example like this: https://github.com/bluejekyll/trust-dns/blob/3eaf3043d78bea843f823958e4228fc004085650/crates/proto/src/multicast/mdns_stream.rs#L183 Which uses `socket2` library for configuring the socket. I hope that helps. This definitely feels like something we should make sure the Tokio networking modules help make this more straightforward.
Author
Owner

@balboah commented on GitHub (Apr 15, 2020):

Thanks for the detailed pointers, I'll take a stab at this later this week

<!-- gh-comment-id:613880934 --> @balboah commented on GitHub (Apr 15, 2020): Thanks for the detailed pointers, I'll take a stab at this later this week
Author
Owner

@balboah commented on GitHub (Apr 17, 2020):

I just verified that:

async fn connect_bind(bind: SocketAddr, connect: SocketAddr) -> Result<TokioTcpStream, io::Error> {
    tokio::task::spawn_blocking(move || {
        let socket = Socket::new(Domain::ipv4(), Type::stream(), None)?;
        socket.bind(&bind.into())?;
        socket.connect(&connect.into())?;
        TokioTcpStream::from_std(socket.into_tcp_stream())
    })
    .await?
}

works for implementing source port selections. However this will not work in my use case.
In my case I need to bind the socket to a specific interface or it won't work. Do you still think it makes sense to have an option for interface name or a callback function?

<!-- gh-comment-id:615152268 --> @balboah commented on GitHub (Apr 17, 2020): I just verified that: ```rust async fn connect_bind(bind: SocketAddr, connect: SocketAddr) -> Result<TokioTcpStream, io::Error> { tokio::task::spawn_blocking(move || { let socket = Socket::new(Domain::ipv4(), Type::stream(), None)?; socket.bind(&bind.into())?; socket.connect(&connect.into())?; TokioTcpStream::from_std(socket.into_tcp_stream()) }) .await? } ``` works for implementing source port selections. However this will not work in my use case. In my case I need to bind the socket to a specific interface or it won't work. Do you still think it makes sense to have an option for interface name or a callback function?
Author
Owner

@bluejekyll commented on GitHub (Apr 17, 2020):

Could you explain why it won't work for your use case in a little more detail? For some reason I thought all you needed was the local bind address, but it sounds like you need more than that.

While my preference is to have just the bind address, I think we could do the bind function. I wonder though if the existing genericized Runtime stuff would be enough for your use case, or has the correct pattern. Right now we only have TCP and UDP abstracted here, but we can finish this abstraction up by adding TLS and HTTPS: github.com/bluejekyll/trust-dns@224def9c92/crates/resolver/src/name_server/connection_provider.rs (L74-L86)

Once that is in place, then you could define your own custom Runtime that would use a derivation of the TokioRuntime to perform the custom logic you need. You could even publish your own crate if you wanted, that would look like the AsyncStd crate: https://github.com/bluejekyll/trust-dns/tree/master/crates/async-std-resolver

To be clear, about all of this, the libraries are already fairly complex, and I would like to try and reduce that complexity. My concern with adding the callback is that it will add more complexity and indirection. So if we could avoid that, I think that's my preference. Though, I'm happy to be convinced otherwise.

<!-- gh-comment-id:615305094 --> @bluejekyll commented on GitHub (Apr 17, 2020): Could you explain why it won't work for your use case in a little more detail? For some reason I thought all you needed was the local bind address, but it sounds like you need more than that. While my preference is to have just the bind address, I think we could do the bind function. I wonder though if the existing genericized Runtime stuff would be enough for your use case, or has the correct pattern. Right now we only have TCP and UDP abstracted here, but we can finish this abstraction up by adding TLS and HTTPS: https://github.com/bluejekyll/trust-dns/blob/224def9c928f41e5ca4040e58f750a0b59b27a7e/crates/resolver/src/name_server/connection_provider.rs#L74-L86 Once that is in place, then you could define your own custom Runtime that would use a derivation of the TokioRuntime to perform the custom logic you need. You could even publish your own crate if you wanted, that would look like the AsyncStd crate: https://github.com/bluejekyll/trust-dns/tree/master/crates/async-std-resolver To be clear, about all of this, the libraries are already fairly complex, and I would like to try and reduce that complexity. My concern with adding the callback is that it will add more complexity and indirection. So if we could avoid that, I think that's my preference. Though, I'm happy to be convinced otherwise.
Author
Owner

@balboah commented on GitHub (Apr 20, 2020):

My use case is pretty narrow as I'm using the resolver on iOS, where I need to set it up so that the resolving happens through the VPN interface. Seems the only way to do that in native code is with the IP_BOUND_IF socket flag. When I ran a test on an iPhone to only use the connect_bind that I pasted earlier, the lookups would just stall.

Using the RuntimeProvider trait is a great suggestion, I had missed this interface.
If I could use it to hook into the HttpsClientStreamBuilder when it creates a new TCP socket, it would be enough for my case. I don't think I necessarily need an "HTTPS" type as it's already using TCP as transport, unless that's how you want to abstract it. Might be confusing to mix transport layer and application layer types on this trait?

I saw that you just split out the async runtime crate recently, were you already considering implementing this for the TLS and HTTPS resolvers in the near term?

<!-- gh-comment-id:616573273 --> @balboah commented on GitHub (Apr 20, 2020): My use case is pretty narrow as I'm using the resolver on iOS, where I need to set it up so that the resolving happens through the VPN interface. Seems the only way to do that in native code is with the IP_BOUND_IF socket flag. When I ran a test on an iPhone to only use the `connect_bind` that I pasted earlier, the lookups would just stall. Using the RuntimeProvider trait is a great suggestion, I had missed this interface. If I could use it to hook into the HttpsClientStreamBuilder when it creates a new TCP socket, it would be enough for my case. I don't think I necessarily need an "HTTPS" type as it's already using TCP as transport, unless that's how you want to abstract it. Might be confusing to mix transport layer and application layer types on this trait? I saw that you just split out the async runtime crate recently, were you already considering implementing this for the TLS and HTTPS resolvers in the near term?
Author
Owner

@balboah commented on GitHub (Apr 21, 2020):

Ah I see now why you need to provide a type for TLS as well in the trait. BTW, noticed that the futures AsyncRead/Write does not match the tokio ones.

oh right, that's why AsyncIo02As03 exists :)

<!-- gh-comment-id:617037316 --> @balboah commented on GitHub (Apr 21, 2020): Ah I see now why you need to provide a type for TLS as well in the trait. BTW, noticed that the futures AsyncRead/Write does not match the tokio ones. oh right, that's why `AsyncIo02As03` exists :)
Author
Owner

@bluejekyll commented on GitHub (Apr 21, 2020):

Sorry for the late response on this. This comment gave me pause:

I'm using the resolver on iOS, where I need to set it up so that the resolving happens through the VPN interface. Seems the only way to do that in native code is with the IP_BOUND_IF socket flag.

Is it possible that we need to add a configuration option to iOS specifically for this? I see that you're already making a bunch of progress, so maybe we could start with something external initially, but it might be something we really do want to support directly in the library if it allows us to broaden the potential installation base to another platform. Just a thought.

But, yes, it looks like you've understood my suggestion properly and are making good progress.

<!-- gh-comment-id:617449481 --> @bluejekyll commented on GitHub (Apr 21, 2020): Sorry for the late response on this. This comment gave me pause: > I'm using the resolver on iOS, where I need to set it up so that the resolving happens through the VPN interface. Seems the only way to do that in native code is with the IP_BOUND_IF socket flag. Is it possible that we need to add a configuration option to iOS specifically for this? I see that you're already making a bunch of progress, so maybe we could start with something external initially, but it might be something we really do want to support directly in the library if it allows us to broaden the potential installation base to another platform. Just a thought. But, yes, it looks like you've understood my suggestion properly and are making good progress.
Author
Owner

@balboah commented on GitHub (Apr 22, 2020):

I think it will be OK to do it like you suggested with RuntimeProvider. Because it’s only when you run the resolver inside the VPN service process on iOS that this matters. A user who run the resolver in their non vpn app will use the correct route without having to think about it, even if a VPN is already established by a different app.

<!-- gh-comment-id:617551652 --> @balboah commented on GitHub (Apr 22, 2020): I think it will be OK to do it like you suggested with RuntimeProvider. Because it’s only when you run the resolver inside the VPN service process on iOS that this matters. A user who run the resolver in their non vpn app will use the correct route without having to think about it, even if a VPN is already established by a different app.
Author
Owner

@balboah commented on GitHub (Apr 22, 2020):

one issue with this approach when you want to do something dynamic like when I'm not looking at how to toggle different interface names for the TCP connect, is that the Tokio TcpStream::connect() that we're mimicking is a static method. So I'll need to toggle different runtimes with a static interface name at compile time.

However I finally got it to work all the way in my project :)

<!-- gh-comment-id:617694125 --> @balboah commented on GitHub (Apr 22, 2020): one issue with this approach when you want to do something dynamic like when I'm not looking at how to toggle different interface names for the TCP connect, is that the Tokio TcpStream::connect() that we're mimicking is a static method. So I'll need to toggle different runtimes with a static interface name at compile time. However I finally got it to work all the way in my project :)
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
starred/hickory-dns#597
No description provided.