diff --git a/Cargo.lock b/Cargo.lock index ba51684820c..495ef9a69ae 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4352,7 +4352,7 @@ dependencies = [ [[package]] name = "mirrord-protocol" -version = "1.11.0" +version = "1.11.1" dependencies = [ "actix-codec", "bincode", diff --git a/changelog.d/2827.fixed.md b/changelog.d/2827.fixed.md new file mode 100644 index 00000000000..392db10e2a2 --- /dev/null +++ b/changelog.d/2827.fixed.md @@ -0,0 +1 @@ +Handle IPv4 in IPv6, should help with regressions related to allowing AF_INET6 \ No newline at end of file diff --git a/mirrord/layer/src/debugger_ports.rs b/mirrord/layer/src/debugger_ports.rs index 2b2feb0aed6..fdabac20859 100644 --- a/mirrord/layer/src/debugger_ports.rs +++ b/mirrord/layer/src/debugger_ports.rs @@ -248,7 +248,7 @@ impl DebuggerPorts { /// Return whether the given [SocketAddr] is used by the debugger. pub fn contains(&self, addr: &SocketAddr) -> bool { let is_localhost = matches!( - addr.ip(), + addr.ip().to_canonical(), IpAddr::V4(Ipv4Addr::LOCALHOST) | IpAddr::V6(Ipv6Addr::LOCALHOST) ); if !is_localhost { diff --git a/mirrord/layer/src/socket.rs b/mirrord/layer/src/socket.rs index 945391c9bc2..189c71e2152 100644 --- a/mirrord/layer/src/socket.rs +++ b/mirrord/layer/src/socket.rs @@ -1,5 +1,7 @@ //! We implement each hook function in a safe function as much as possible, having the unsafe do the //! absolute minimum +//! Note the case of IPv6 in IPv4 which requires special care to do right +//! use std::{ collections::HashMap, net::{SocketAddr, ToSocketAddrs}, @@ -382,13 +384,13 @@ impl OutgoingSelector { // https://github.com/metalbear-co/mirrord/issues/2389 fixed and I don't have time to // fully understand or refactor, and the logic is sound (if it's loopback, just connect to // it) - if address.ip().is_loopback() { + if address.ip().to_canonical().is_loopback() { return Ok(address); } let cached = REMOTE_DNS_REVERSE_MAPPING .lock()? - .get(&address.ip()) + .get(&address.ip().to_canonical()) .cloned(); let Some(hostname) = cached else { return Ok(address); @@ -458,7 +460,7 @@ impl ProtocolAndAddressFilterExt for ProtocolAndAddressFilter { let _guard = DetourGuard::new(); match (name.as_str(), *port).to_socket_addrs() { - Ok(addresses) => addresses.map(|addr| addr.ip()).collect(), + Ok(addresses) => addresses.map(|addr| addr.ip().to_canonical()).collect(), Err(e) => { let as_string = e.to_string(); if as_string.contains("Temporary failure in name resolution") @@ -477,12 +479,13 @@ impl ProtocolAndAddressFilterExt for ProtocolAndAddressFilter { } }; - Ok(resolved_ips.into_iter().any(|ip| ip == address.ip())) + Ok(resolved_ips + .into_iter() + .any(|ip| ip == address.ip().to_canonical())) } - AddressFilter::Socket(addr) => { - Ok(addr.ip().is_unspecified() || addr.ip() == address.ip()) - } - AddressFilter::Subnet(net, _) => Ok(net.contains(&address.ip())), + AddressFilter::Socket(addr) => Ok(addr.ip().to_canonical().is_unspecified() + || addr.ip().to_canonical() == address.ip().to_canonical()), + AddressFilter::Subnet(net, _) => Ok(net.contains(&address.ip().to_canonical())), AddressFilter::Port(..) => Ok(true), } } diff --git a/mirrord/layer/src/socket/dns_selector.rs b/mirrord/layer/src/socket/dns_selector.rs index e83dbba2d9f..20c202ec8ae 100644 --- a/mirrord/layer/src/socket/dns_selector.rs +++ b/mirrord/layer/src/socket/dns_selector.rs @@ -33,7 +33,7 @@ impl DnsSelector { AddressFilter::Port(..) => true, AddressFilter::Name(filter_name, _) => filter_name == node, AddressFilter::Socket(filter_socket) => { - filter_socket.ip().is_unspecified() + filter_socket.ip().to_canonical().is_unspecified() || Some(filter_socket.ip()) == node.parse().ok() } AddressFilter::Subnet(filter_subnet, _) => { diff --git a/mirrord/layer/src/socket/ops.rs b/mirrord/layer/src/socket/ops.rs index e03569b102c..e6f92ff492d 100644 --- a/mirrord/layer/src/socket/ops.rs +++ b/mirrord/layer/src/socket/ops.rs @@ -154,7 +154,9 @@ fn bind_similar_address(sockfd: c_int, requested_address: &SocketAddr) -> Detour let addr = requested_address.ip(); let port = requested_address.port(); - let address = if addr.is_loopback() || addr.is_unspecified() { + let canonical_address = addr.to_canonical(); + + let address = if canonical_address.is_loopback() || canonical_address.is_unspecified() { *requested_address } else if addr.is_ipv4() { SocketAddr::new(IpAddr::V4(Ipv4Addr::LOCALHOST), port) @@ -253,7 +255,7 @@ pub(super) fn bind( // we don't use `is_localhost` here since unspecified means to listen // on all IPs. - if incoming_config.ignore_localhost && requested_address.ip().is_loopback() { + if incoming_config.ignore_localhost && requested_address.ip().to_canonical().is_loopback() { return Detour::Bypass(Bypass::IgnoreLocalhost(requested_port)); } @@ -564,7 +566,11 @@ pub(super) fn connect( ) -> Detour { let remote_address = SockAddr::try_from_raw(raw_address, address_length)?; let optional_ip_address = remote_address.as_socket(); - + let is_ipv4_in_ipv6 = remote_address + .as_socket() + .as_ref() + .map(|addr| addr.ip().to_canonical().is_ipv6()) + .unwrap_or(false); let unix_streams = crate::setup().remote_unix_streams(); trace!("in connect {:#?}", SOCKETS); @@ -581,7 +587,7 @@ pub(super) fn connect( .family() .map(|family| family as i32) .unwrap_or(-1); - if domain != libc::AF_INET && domain != libc::AF_UNIX { + if domain != libc::AF_INET && domain != libc::AF_UNIX && !is_ipv4_in_ipv6 { return Detour::Bypass(Bypass::Domain(domain)); } // I really hate it, but nix seems to really make this API bad :() @@ -603,8 +609,9 @@ pub(super) fn connect( return Detour::Success(connect_result); } - let ip = ip_address.ip(); - if ip.is_loopback() || ip.is_unspecified() { + let canonical_ip = ip_address.ip().to_canonical(); + + if canonical_ip.is_loopback() || canonical_ip.is_unspecified() { if let Some(result) = connect_to_local_address(sockfd, &user_socket_info, ip_address)? { // `result` here is always a success, as error and bypass are returned on the `?` // above. diff --git a/mirrord/protocol/Cargo.toml b/mirrord/protocol/Cargo.toml index b577a8fbe5b..c6e954f0f38 100644 --- a/mirrord/protocol/Cargo.toml +++ b/mirrord/protocol/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "mirrord-protocol" -version = "1.11.0" +version = "1.11.1" authors.workspace = true description.workspace = true documentation.workspace = true diff --git a/mirrord/protocol/src/outgoing.rs b/mirrord/protocol/src/outgoing.rs index dedfa099b7b..2614aac0052 100644 --- a/mirrord/protocol/src/outgoing.rs +++ b/mirrord/protocol/src/outgoing.rs @@ -83,6 +83,11 @@ impl TryFrom for SocketAddress { fn try_from(addr: OsSockAddr) -> Result { addr.as_socket() + .map(|mut socket_addr| { + // convert ipv4 in ipv6 to ipv4 + socket_addr.set_ip(socket_addr.ip().to_canonical()); + socket_addr + }) .map(SocketAddress::Ip) .or_else(|| { addr.as_pathname()