Bind address in ResolverConfig does not take effort for AsyncResolver

[hainesc]

Describe the bug
A clear and concise description of what the bug is.

When I set bind_addr in ResolverConfig when make AsyncResolver, I expect that the UDP packets of DNS query would bind this SocketAddr, But it does not work!

To Reproduce
Steps to reproduce the behavior:

Source Code

use std::net::{IpAddr, Ipv4Addr, SocketAddr};
use trust_dns_resolver::name_server::TokioConnectionProvider;

#[tokio::main]
async fn main() -> Result<(), Box<dyn Error>> {
    // Config trace here.
    let subscriber = tracing_subscriber::fmt()
        .compact()
        .with_file(true)
        .with_line_number(true)
        .with_max_level(tracing::Level::TRACE)
        .finish();
    tracing::subscriber::set_global_default(subscriber)?;

    // Get a config and option by read_system_conf, we don't use the config directly. 
    let (config, options) = trust_dns_resolver::system_conf::read_system_conf()?;

    // Make a new ResolverConfig and copy from the config above.
    let mut c = trust_dns_resolver::config::ResolverConfig::new();
    for s in config.name_servers() {
        let mut cloned = s.clone();
        cloned.bind_addr = Some(SocketAddr::new(
            // Set bind_addr here, you can change to what your network iface is. Also you can keep it, it takes no effort at all.
            IpAddr::V4(Ipv4Addr::new(192, 168, 1, 109)),
            0,
        ));
        c.add_name_server(cloned);
    }

    // make a resolver and query.
    let resolver = trust_dns_resolver::AsyncResolver::new_with_conn(
        c,
        options,
        TokioConnectionProvider::default(),
    );

    resolver.ipv4_lookup("wwww.google.com").await;
    Ok(())
}

Dependencies of Cargo.toml

[dependencies]
tokio = { version = "1.0", features = ["full"] }
tracing = "0.1"
tracing-subscriber = "0.3"
# local path here, so we can add log to see the detail
trust-dns-resolver = { path = "../trust-dns-resolver-0.23.2" }

cp -r ~/.cargo/registry/src/index.crates.io-6f17d22bba15001f/trust-dns-resolver-0.23.2 ../
cp -r ~/.cargo/registry/src/index.crates.io-6f17d22bba15001f/trust-dns-proto-0.23.2  ../ 
# Change the Cargo.toml of resolver, point it to { path = "../trust-dns-proto-0.23.2" }
# Modify the source code at  some_path/trust-dns-proto-0.23.2/src/udp/udp_stream.rs L310 to debug!("created socket successfully, bind_addr: {}", bind_addr);

Build and run

2024-04-22T03:49:05.608551Z DEBUG trust_dns_resolver::name_server::name_server_pool: /Users/coder/workspace/rust/trust-dns-resolver-0.23.2/src/name_server/name_server_pool.rs:258: sending request: [Query { name: Name("wwww.google.com"), query_type: A, query_class: IN }]
2024-04-22T03:49:05.608677Z DEBUG trust_dns_resolver::name_server::name_server: /Users/coder/workspace/rust/trust-dns-resolver-0.23.2/src/name_server/name_server.rs:105: reconnecting: NameServerConfig { socket_addr: 223.6.6.6:53, protocol: Udp, tls_dns_name: None, trust_negative_responses: false, bind_addr: Some(**192.168.1.109:0**) }
...

2024-04-22T03:49:05.609384Z DEBUG trust_dns_proto::udp::udp_stream: /Users/coder/workspace/rust/trust-dns-proto-0.23.2/src/udp/udp_stream.rs:310: created socket successfully, bind_addr: **0.0.0.0:64665**

Expected behavior
A clear and concise description of what you expected to happen.

The last line of log should use bind_addr: 192.168.1.109:64665

System:

• OS: [e.g. macOS]
• Architecture: [e.g. x86_64]
• Version [e.g. 22]
• rustc version: [e.g. 1.28]

Version:
Crate: [e.g. client, server, resolver]
Version: [e.g. 0.9.1]

Additional context
Add any other context about the problem here.

[djc]

IIRC a UDP DNS client doesn't bind to a single address (it uses ~one address per query) because otherwise it would be to easy to spoof responses.

Why is it important that UDP messages come from a single address? What's the use case for this?

[hainesc]

IIRC a UDP DNS client doesn't bind to a single address (it uses ~one address per query) because otherwise it would be to easy to spoof responses.

Why is it important that UDP messages come from a single address? What's the use case for this?

I am agree with you that most of time, we don't need to bind a specific iface to do dns query. But sometimes we need, when I have more than one ifaces in my machine, which connect different networks.

And also, it is a feature that we can bind a specific iface, that means we can bind a specific iface if we want. For most network library for example tokio, socket2, as you see, they all support this feature.

Thanks.

[djc]

@bluejekyll maybe bind_addr should take an IpAddr instead of a SocketAddr?

[hainesc]

@bluejekyll maybe bind_addr should take an IpAddr instead of a SocketAddr?

I think it should take a SocketAddr, because most of other library take a SocketAddr. for example, glibc, tokio.

https://www.gnu.org/software/libc/manual/html_node/Setting-Address.html

And more, in hickory dns, we have used SocketAddr for bind_addr, see:
https://github.com/hickory-dns/hickory-dns/blob/main/crates/proto/src/udp/udp_stream.rs#L245

Thanks.

[djc]

IMO using a SocketAddr for binding an UDP socket is surprising since, AIUI, our UDP client sockets don't want to stick to one port as I mentioned in a previous comment. In general I feel like the notion of a single bind_addr is a little confusing since we could potentially bind any number of clients (UDP, TCP, DoT, DoH, DoQ) which can't all bind to the same port.

[hainesc]

Most of cases, the users specific the port to 0 which means random port by system.

See: https://github.com/hickory-dns/hickory-dns/blob/main/crates/proto/src/udp/udp_stream.rs#L291

[djc]

It doesn't feel like you're actually reading my comments, so I'm going to stop engaging with this issue now.

[bluejekyll]

Looking at this more, I think @djc, is making a very good point that for this interface, we want a bind_addr that is just the IP address, and not the port. The port should be randomly chosen based on our logic which works to enforce a random port is always used for each connection. For that reason, I think we want the PR to continue to use the random port logic that we have, but allow for the bind address to be set.

[hainesc]

1. First and fotmost, IP_BIND_ADDRESS_NO_PORT SocketAddr with port 0 which means the port will later be automatically chosen when connect. This behavior has been documented in here https://man7.org/linux/man-pages/man7/ip.7.html. So if the user wants specific IP(for most of cases), he can use port 0.
2. IP_BIND_ADDRESS_NO_PORT aka SocketAddr with port 0 is widely used, connect(2) operating-system function in Linux, let socket = tokio::net::TcpSocket::new_v4().unwrap(); socket.connect(socketaddr) in rust. For some user at least for me, it is a inertial thinking when I want specific a bind_addr.

[bluejekyll]

Not sure why you wanted to close this? My sense is that allowing the bind address is a good thing. We don’t trust the OS to distribute the port addresses in general, which is why the library has a random function to ensure it’s somewhat randomly distributed across the port space.

I see that you believe we should accept SocketAddr and use the port as an indication of using random selection logic, and I get your reasoning, but in this case we will be issuing multiple requests from this interface. In order to issue multiple requests to the same remote address, those must be on separate ports, otherwise we run afoul of the response spoofing that the random port selection is intended to prevent.

so it leads me to believe that we want to guide people in the proper direction, and only take IpAdrr as the bind address, and always randomize the port. Do you have a particular use case where you want the port to be static and non-zero ever?