Skip to content

Commit

Permalink
Handle IPv4 in IPv6, fix regressions (#2829)
Browse files Browse the repository at this point in the history
* Handle IPv4 in IPv6, should help with regressions related to allowing AF_INET6

* ..

* ..

* ..

* ..
  • Loading branch information
aviramha authored Oct 12, 2024
1 parent 6b3b4fe commit 5606f92
Show file tree
Hide file tree
Showing 8 changed files with 34 additions and 18 deletions.
2 changes: 1 addition & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions changelog.d/2827.fixed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Handle IPv4 in IPv6, should help with regressions related to allowing AF_INET6
2 changes: 1 addition & 1 deletion mirrord/layer/src/debugger_ports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
19 changes: 11 additions & 8 deletions mirrord/layer/src/socket.rs
Original file line number Diff line number Diff line change
@@ -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
//! <https://github.com/metalbear-co/mirrord/pull/2829>
use std::{
collections::HashMap,
net::{SocketAddr, ToSocketAddrs},
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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")
Expand All @@ -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),
}
}
Expand Down
2 changes: 1 addition & 1 deletion mirrord/layer/src/socket/dns_selector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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, _) => {
Expand Down
19 changes: 13 additions & 6 deletions mirrord/layer/src/socket/ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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));
}

Expand Down Expand Up @@ -564,7 +566,11 @@ pub(super) fn connect(
) -> Detour<ConnectResult> {
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);
Expand All @@ -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 :()
Expand All @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion mirrord/protocol/Cargo.toml
Original file line number Diff line number Diff line change
@@ -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
Expand Down
5 changes: 5 additions & 0 deletions mirrord/protocol/src/outgoing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,11 @@ impl TryFrom<OsSockAddr> for SocketAddress {

fn try_from(addr: OsSockAddr) -> Result<Self, Self::Error> {
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()
Expand Down

0 comments on commit 5606f92

Please sign in to comment.