From 79a24ca9cfc66b476482e4c8bab6c535dc041e4c Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Fri, 18 Aug 2023 11:44:41 +0200 Subject: [PATCH] feat(relay): remove `LISTEN_IPX_ADDR` parameters (#1922) Previously, we required the user to specify a `LISTEN_IP4_ADDR` and/or a `LISTEN_IP6_ADDR` parameter. This is cumbersome because dynamically fetching the address of the local interface is not trivial in all environments. We remove this parameter in exchange for listening on all interfaces. This is a trade-off. The relay will now listen on all interfaces, even the ones not exposed to the public internet. This is true for the main socket on port 3478 and for all created allocations. Actually relaying data relies on the 4-tuple of a "connection", i.e. the source and destination address and port. Technically, I think it is possible with this change to send traffic to a relay via an interface that was not intended to be used for that. I think this will still require spoofing the source address which is a known and accepted problem. It is still recommended that operators put appropriate firewall rules in place to not allow ingress traffic on any interface other than the one intended for relaying. I've tested locally that we are correctly using the `IPV6_ONLY` flag. In other words, a relay listening on the `0.0.0.0` wildcard interface will not accept IPv6 traffic and vice versa. Resolves #1886. --- docker-compose.yml | 2 -- rust/docker-init.sh | 5 ---- rust/relay/run_smoke_test.sh | 1 - rust/relay/src/allocation.rs | 13 ++++----- rust/relay/src/main.rs | 51 ++++++++---------------------------- rust/relay/src/udp_socket.rs | 25 ++++++++++-------- 6 files changed, 32 insertions(+), 65 deletions(-) diff --git a/docker-compose.yml b/docker-compose.yml index ac4f37fc0..2afd0b33f 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -180,9 +180,7 @@ services: relay: environment: PUBLIC_IP4_ADDR: 172.28.0.101 - LISTEN_IP4_ADDR: 172.28.0.101 PUBLIC_IP6_ADDR: fcff:3990:3990::101 - LISTEN_IP6_ADDR: fcff:3990:3990::101 PORTAL_WS_URL: "ws://api:8081/" PORTAL_TOKEN: "SFMyNTY.g2gDaAJtAAAAJDcyODZiNTNkLTA3M2UtNGM0MS05ZmYxLWNjODQ1MWRhZDI5OW0AAABARVg3N0dhMEhLSlVWTGdjcE1yTjZIYXRkR25mdkFEWVFyUmpVV1d5VHFxdDdCYVVkRVUzbzktRmJCbFJkSU5JS24GAFSzb0KJAWIAAVGA.waeGE26tbgkgIcMrWyck0ysv9SHIoHr0zqoM3wao84M" RUST_LOG: "debug" diff --git a/rust/docker-init.sh b/rust/docker-init.sh index 7747bbd3e..6a4e2e308 100755 --- a/rust/docker-init.sh +++ b/rust/docker-init.sh @@ -17,11 +17,6 @@ if [[ "${LISTEN_ADDRESS_DISCOVERY_METHOD}" == "gce_metadata" ]]; then export PUBLIC_IP4_ADDR=$(curl "http://metadata.google.internal/computeMetadata/v1/instance/network-interfaces/0/access-configs/0/external-ip" -H "Metadata-Flavor: Google" -s) echo "Discovered PUBLIC_IP4_ADDR: ${PUBLIC_IP4_ADDR}" fi; - - if [[ "${LISTEN_IP4_ADDR:-}" == "" ]]; then - export LISTEN_IP4_ADDR=$(curl "http://metadata.google.internal/computeMetadata/v1/instance/network-interfaces/0/ip" -H "Metadata-Flavor: Google" -s) - echo "Discovered LISTEN_IP4_ADDR: ${LISTEN_IP4_ADDR}" - fi; fi # if first arg looks like a flag, assume we want to run postgres server diff --git a/rust/relay/run_smoke_test.sh b/rust/relay/run_smoke_test.sh index 2f0eda891..05c777639 100755 --- a/rust/relay/run_smoke_test.sh +++ b/rust/relay/run_smoke_test.sh @@ -22,7 +22,6 @@ gateway="$target_directory/debug/examples/gateway" relay="$target_directory/debug/relay" export PUBLIC_IP4_ADDR=127.0.0.1; -export LISTEN_IP4_ADDR=127.0.0.1; export RNG_SEED=0; export RUST_LOG=relay=debug; diff --git a/rust/relay/src/allocation.rs b/rust/relay/src/allocation.rs index bb527ac22..a041bcf15 100644 --- a/rust/relay/src/allocation.rs +++ b/rust/relay/src/allocation.rs @@ -1,10 +1,11 @@ use crate::server::AllocationId; use crate::udp_socket::UdpSocket; +use crate::AddressFamily; use anyhow::{bail, Result}; use futures::channel::mpsc; use futures::{SinkExt, StreamExt}; use std::convert::Infallible; -use std::net::{IpAddr, SocketAddr}; +use std::net::SocketAddr; use tokio::task; /// The maximum amount of items that can be buffered in the channel to the allocation task. @@ -24,17 +25,17 @@ impl Allocation { pub fn new( relay_data_sender: mpsc::Sender<(Vec, SocketAddr, AllocationId)>, id: AllocationId, - listen_addr: IpAddr, + family: AddressFamily, port: u16, ) -> Self { let (client_to_peer_sender, client_to_peer_receiver) = mpsc::channel(MAX_BUFFERED_ITEMS); let task = tokio::spawn(async move { - let Err(e) = forward_incoming_relay_data(relay_data_sender, client_to_peer_receiver, id, listen_addr, port).await else { + let Err(e) = forward_incoming_relay_data(relay_data_sender, client_to_peer_receiver, id, family, port).await else { unreachable!() }; - tracing::warn!(allocation = %id, %listen_addr, "Allocation task failed: {e:#}"); + tracing::warn!(allocation = %id, %family, "Allocation task failed: {e:#}"); // With the task stopping, the channel will be closed and any attempt to send data to it will fail. }); @@ -83,10 +84,10 @@ async fn forward_incoming_relay_data( mut relayed_data_sender: mpsc::Sender<(Vec, SocketAddr, AllocationId)>, mut client_to_peer_receiver: mpsc::Receiver<(Vec, SocketAddr)>, id: AllocationId, - listen_addr: IpAddr, + family: AddressFamily, port: u16, ) -> Result { - let mut socket = UdpSocket::bind((listen_addr, port))?; + let mut socket = UdpSocket::bind(family, port)?; loop { tokio::select! { diff --git a/rust/relay/src/main.rs b/rust/relay/src/main.rs index 65ca19db0..0c0b5d8c1 100644 --- a/rust/relay/src/main.rs +++ b/rust/relay/src/main.rs @@ -13,7 +13,7 @@ use relay::{ use std::collections::hash_map::Entry; use std::collections::HashMap; use std::convert::Infallible; -use std::net::{IpAddr, Ipv4Addr, Ipv6Addr, SocketAddr}; +use std::net::{Ipv4Addr, Ipv6Addr, SocketAddr}; use std::pin::Pin; use std::task::Poll; use std::time::SystemTime; @@ -26,25 +26,11 @@ use url::Url; #[derive(Parser, Debug)] struct Args { /// The public (i.e. internet-reachable) IPv4 address of the relay server. - /// - /// Must route to the local IPv4 interface we listen on. #[arg(long, env)] public_ip4_addr: Option, - /// The address of the local IPv4 interface we should listen on. - /// - /// Must not be a wildcard-address. - #[arg(long, env)] - listen_ip4_addr: Option, /// The public (i.e. internet-reachable) IPv6 address of the relay server. - /// - /// Must route to the local IP6 interface we listen on. #[arg(long, env)] public_ip6_addr: Option, - /// The address of the local IP6 interface we should listen on. - /// - /// Must not be a wildcard-address. - #[arg(long, env)] - listen_ip6_addr: Option, /// The address of the local interface where we should serve the prometheus metrics. /// /// The metrics will be available at `http:///metrics`. @@ -84,7 +70,7 @@ async fn main() -> Result<()> { tracing_subscriber::fmt().with_env_filter(env_filter).init() } - let listen_addr = match (args.listen_ip4_addr, args.listen_ip6_addr) { + let public_addr = match (args.public_ip4_addr, args.public_ip6_addr) { (Some(ip4), Some(ip6)) => IpStack::Dual { ip4, ip6 }, (Some(ip4), None) => IpStack::Ip4(ip4), (None, Some(ip6)) => IpStack::Ip6(ip6), @@ -92,14 +78,6 @@ async fn main() -> Result<()> { bail!("Must listen on at least one of IPv4 or IPv6") } }; - let public_addr = match (args.public_ip4_addr, args.public_ip6_addr, listen_addr) { - (Some(ip4), Some(ip6), IpStack::Dual { .. }) => IpStack::Dual { ip4, ip6 }, - (Some(ip4), None, IpStack::Ip4(_)) => IpStack::Ip4(ip4), - (None, Some(ip6), IpStack::Ip6(_)) => IpStack::Ip6(ip6), - _ => { - bail!("Must specify a public address for each listen address") - } - }; let mut metric_registry = Registry::with_prefix("relay"); let server = Server::new(public_addr, make_rng(args.rng_seed), &mut metric_registry); @@ -164,7 +142,7 @@ async fn main() -> Result<()> { None }; - let mut eventloop = Eventloop::new(server, channel, listen_addr, &mut metric_registry)?; + let mut eventloop = Eventloop::new(server, channel, public_addr, &mut metric_registry)?; if let Some(metrics_addr) = args.metrics_addr { tokio::spawn(relay::metrics::serve(metrics_addr, metric_registry)); @@ -214,7 +192,6 @@ struct Eventloop { inbound_data_receiver: mpsc::Receiver<(Vec, SocketAddr)>, outbound_ip4_data_sender: mpsc::Sender<(Vec, SocketAddr)>, outbound_ip6_data_sender: mpsc::Sender<(Vec, SocketAddr)>, - listen_address: IpStack, server: Server, channel: Option>, allocations: HashMap<(AllocationId, AddressFamily), Allocation>, @@ -230,7 +207,7 @@ where fn new( server: Server, channel: Option>, - listen_address: IpStack, + public_address: IpStack, _: &mut Registry, ) -> Result { let (relay_data_sender, relay_data_receiver) = mpsc::channel(1); @@ -240,16 +217,16 @@ where let (outbound_ip6_data_sender, outbound_ip6_data_receiver) = mpsc::channel::<(Vec, SocketAddr)>(10); - if let Some(ip4) = listen_address.as_v4() { + if public_address.as_v4().is_some() { tokio::spawn(main_udp_socket_task( - (*ip4).into(), + AddressFamily::V4, inbound_data_sender.clone(), outbound_ip4_data_receiver, )); } - if let Some(ip6) = listen_address.as_v6() { + if public_address.as_v6().is_some() { tokio::spawn(main_udp_socket_task( - (*ip6).into(), + AddressFamily::V6, inbound_data_sender, outbound_ip6_data_receiver, )); @@ -259,7 +236,6 @@ where inbound_data_receiver, outbound_ip4_data_sender, outbound_ip6_data_sender, - listen_address, server, channel, allocations: Default::default(), @@ -293,14 +269,9 @@ where } } Command::CreateAllocation { id, family, port } => { - let listen_addr = match family { - AddressFamily::V4 => (*self.listen_address.as_v4().expect("to have listen address for IP4 if we are creating an IP4 allocation")).into(), - AddressFamily::V6 => (*self.listen_address.as_v6().expect("to have listen address for IP6 if we are creating an IP6 allocation")).into(), - }; - self.allocations.insert( (id, family), - Allocation::new(self.relay_data_sender.clone(), id, listen_addr, port), + Allocation::new(self.relay_data_sender.clone(), id, family, port), ); } Command::FreeAllocation { id, family } => { @@ -406,11 +377,11 @@ where } async fn main_udp_socket_task( - listen_address: IpAddr, + family: AddressFamily, mut inbound_data_sender: mpsc::Sender<(Vec, SocketAddr)>, mut outbound_data_receiver: mpsc::Receiver<(Vec, SocketAddr)>, ) -> Result { - let mut socket = UdpSocket::bind((listen_address, 3478))?; + let mut socket = UdpSocket::bind(family, 3478)?; loop { tokio::select! { diff --git a/rust/relay/src/udp_socket.rs b/rust/relay/src/udp_socket.rs index 92cd74070..168035b22 100644 --- a/rust/relay/src/udp_socket.rs +++ b/rust/relay/src/udp_socket.rs @@ -1,6 +1,6 @@ -use crate::{AddressFamily, SocketAddrExt}; +use crate::AddressFamily; use anyhow::{Context as _, Result}; -use std::net::SocketAddr; +use std::net::{IpAddr, Ipv4Addr, Ipv6Addr, SocketAddr}; use std::task::{ready, Context, Poll}; use tokio::io::ReadBuf; @@ -13,10 +13,9 @@ pub struct UdpSocket { } impl UdpSocket { - pub fn bind(addr: impl Into) -> Result { - let addr = addr.into(); - let std_socket = make_std_socket(addr) - .with_context(|| format!("Failed to bind UDP socket to {addr}"))?; + pub fn bind(family: AddressFamily, port: u16) -> Result { + let std_socket = make_wildcard_socket(family, port) + .with_context(|| format!("Failed to bind UDP socket for {family}"))?; Ok(Self { inner: tokio::net::UdpSocket::from_std(std_socket)?, @@ -61,21 +60,25 @@ impl UdpSocket { /// Creates an [std::net::UdpSocket] via the [socket2] library that is configured for our needs. /// /// Most importantly, this sets the `IPV6_V6ONLY` flag to ensure we disallow IP4-mapped IPv6 addresses and can bind to IP4 and IP6 addresses on the same port. -fn make_std_socket(socket_addr: SocketAddr) -> Result { +fn make_wildcard_socket(family: AddressFamily, port: u16) -> Result { use socket2::*; - let domain = match socket_addr.family() { + let domain = match family { AddressFamily::V4 => Domain::IPV4, AddressFamily::V6 => Domain::IPV6, }; - let socket = Socket::new(domain, Type::DGRAM, Some(Protocol::UDP))?; + let address = match family { + AddressFamily::V4 => IpAddr::from(Ipv4Addr::UNSPECIFIED), + AddressFamily::V6 => IpAddr::from(Ipv6Addr::UNSPECIFIED), + }; - if socket_addr.is_ipv6() { + let socket = Socket::new(domain, Type::DGRAM, Some(Protocol::UDP))?; + if family == AddressFamily::V6 { socket.set_only_v6(true)?; } socket.set_nonblocking(true)?; - socket.bind(&socket_addr.into())?; + socket.bind(&SockAddr::from(SocketAddr::new(address, port)))?; Ok(socket.into()) }