Open Zoxc opened 3 months ago
@Zoxc For clarity in my own mind, for the example to be more realistic, you would want the first socket to be open as the "server" and bind to all interfaces, probably on a specified port:
let addr = env::args() .nth(1) .unwrap_or_else(|| "0.0.0.0:6555".to_string());
and then for the client, you would open a socket on another port, use UdpSocket.connect()
to specify the destination (using any of the server IP addresses) and then send the PINGS and listen for the PONGS. Replies would be guaranteed to be from the correct IP by virtue of the filtering that .connect()
does. Does that sound right? Did I miss anything?
That would run into problems since it reuses the port and seems inefficient. A better way would be following the Sourcing packets from a wildcard socket
section in https://blog.cloudflare.com/everything-you-ever-wanted-to-know-about-udp-sockets-but-were-afraid-to-ask-part-1 however that likely would need the addition of suitable APIs in tokio
and possibly mio
.
Ok that’s what I thought you meant at first but then I saw the thing about the client not even checking if the response is from the same server and thought it might be an easier problem to solve than I first thought.
I read that section of the blog, thanks. In terms of implementing new APIs, specifically it would be an API in Tokio/Mio to be able to get the CSMG Metadata from the underlying Linux/BSD/(Windows?) API?
Is there an argument to iterate all interfaces and bind to each one individually in the example and then noting in the comments that this isn’t ideal for all use cases? That would at least open up the idea and leaves further reading up to the user. Or did you intend this issue to point out this as a limitation of Tokio and then track updating the example as a good “definition of done”.
In terms of implementing new APIs, specifically it would be an API in Tokio/Mio to be able to get the CSMG Metadata from the underlying Linux/BSD/(Windows?) API?
Yeah I think some replacement for recv_from
and send_to
on UdpSocket
which allows the right IP to be passed along would be needed.
Or did you intend this issue to point out this as a limitation of Tokio and then track updating the example as a good “definition of done”.
I was wondering if there was an easier way that I missed, but it doesn't appear so.
I also noticed that there's a similar example on UdpSocket
itself which uses wildcard IPs, so that's straight up incorrect.
Yeah, I see the other example now and the potential problems with it. After a little digging it sounds like the "replacement" would be recvmsg(2)
. It looks like exposing that in mio has been discussed and softly rejected in the past: MIO #200 and MIO #550
If we got an implementation of recvmsg
, then it's just a matter* of passing some parameters for extra information following the example here: https://blog.powerdns.com/2012/10/08/on-binding-datagram-udp-sockets-to-the-any-addresses
*just a matter - it's still messy in IPv4 but better in IPv6, and the example in the blog doesn't include Windows implementation
I could imagine a few paths forward. Not sure if I love any of them:
recvmsg
straight from nix::sys::socket
in the example, and not update or change any APIs to support the example. Assuming it plays well enough with the Tokio structs to still be a useful example (I haven't dug in enough to see), this could be the most feasible solution that actually uses a wildcard approach.At the very minimum, if it was decided to do nothing else, the example that uses wildcards should be changed, since it could fail silently.
What about using the Rust implementation of Wireguard as a precedent for how to implement this? Basically make a bind4
and bind6
for the special cases of binding to the INADDR_ANY addresses.
Inside those functions you could do the CMSG work and when the UDPSocket is operating in that mode, it attaches that information to each incoming packet so that you can read it off the packet when sending a response. I guess then all that would be left is to implement a send_to
that can take a source IP parameter, like you originally suggested.
*I see the issue you opened in the Rust code base, feel free to carry this over there if you think it's a more appropriate issue to have the discussion.
Here's some code demonstrating the issue, for anyone else coming along later:
add a file in the examples/
directory called broken-udp.rs
add network-interface = "2.0.0"
to the examples/Cargo.toml
file
run it with cargo run --example broken-udp
#![warn(rust_2018_idioms)]
use std::error::Error;
use std::io;
use std::net::{IpAddr, SocketAddr};
use network_interface::{NetworkInterface, NetworkInterfaceConfig};
use tokio::net::UdpSocket;
struct Server {
socket: UdpSocket,
buf: Vec<u8>,
to_send: Option<(usize, SocketAddr)>,
}
impl Server {
async fn run(self) -> Result<(), io::Error> {
let Server {
socket,
mut buf,
mut to_send,
} = self;
loop {
// First we check to see if there's a message we need to echo back.
// If so then we try to send it back to the original source, waiting
// until it's writable and we're able to do so.
if let Some((size, peer)) = to_send {
let amt = socket.send_to(&buf[..size], &peer).await?;
println!("Echoed {}/{} bytes to {}", amt, size, peer);
}
// If we're here then `to_send` is `None`, so we take a look for the
// next message we're going to echo back.
to_send = Some(socket.recv_from(&mut buf).await?);
}
}
}
#[tokio::main]
async fn main() -> Result<(), Box<dyn Error>> {
let serv_addr = "0.0.0.0:0";
let socket = UdpSocket::bind(&serv_addr).await.expect("Failed to open socket");
// server will echo the bytes
let server = Server {
socket,
buf: vec![0; 1024],
to_send: None,
};
let server_address = server.socket.local_addr().unwrap().clone();
let port = server_address.port();
let networks = NetworkInterface::show().unwrap();
let addresses = networks.iter().fold(Vec::new(), |mut carry, next| -> Vec<IpAddr> {
carry.push(next.addr.first().unwrap().ip());
return carry.clone(); // not very efficient
});
let socket_addresses = addresses.iter().map(|addr| {
SocketAddr::new(*addr, port)
}).collect::<Vec<SocketAddr>>();
// This starts the server task.
tokio::spawn(async { server.run().await });
// set up the client socket
let client_addr = "127.0.0.1:0";
let client = UdpSocket::bind(&client_addr).await.expect("Failed to open client socket");
// iterate through the server socket addresses (one per IP) and see check what IP the server responds from
for ip in socket_addresses {
client.send_to(&b"PING"[..], ip).await.expect(format!("Failed to send bytes to {:?}", ip).as_str());
println!("Sent some bytes to {:?}", ip);
let mut recv_buf = [0u8; 32];
let (len, addr) = client.recv_from(&mut recv_buf).await.unwrap();
println!("Received some bytes! {:?} from {:?}", String::from_utf8_lossy(&recv_buf[..len]), addr);
}
Ok(())
}
Example output:
Sent some bytes to 172.21.108.37:35308 Echoed 4/4 bytes to 127.0.0.1:51832 Received some bytes! "PING" from 127.0.0.1:35308 Sent some bytes to 127.0.0.1:35308 Echoed 4/4 bytes to 127.0.0.1:51832 Received some bytes! "PING" from 127.0.0.1:35308
We loop through all the IP addresses the server is hosting from but it always responds with the same default IP.
It would be nice if the UDP echo server example was more realistic and allowed binding to any interface.
The current example doesn't constrain the reply to occur from the same IP it was received on which means it only works when bound to a specific IP.