Disallow using IPv6 sockets with mirrord (metalbear-co#2837)

* Revert "4f82caadbf6b00a6c51e36289021aff4526dfa6c"

* Typo

* Revert "5606f9226b520b19533308dbda0b2df64d375991"

changelog.d/2836.fixed.md

@@ -0,0 +1 @@
+Disallow using IPv6 sockets with mirrord.

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().to_canonical(),
+            addr.ip(),
             IpAddr::V4(Ipv4Addr::LOCALHOST) | IpAddr::V6(Ipv6Addr::LOCALHOST)
         );
         if !is_localhost {

mirrord/layer/src/error.rs

@@ -75,6 +75,9 @@ pub(crate) enum HookError {
     #[error("mirrord-layer: SIP patch failed with error `{0}`!")]
     FailedSipPatch(#[from] SipError),
 
+    #[error("mirrord-layer: IPv6 can't be used with mirrord")]
+    SocketUnsuportedIpv6,
+
     // `From` implemented below, not with `#[from]` so that when new variants of
     // `SerializationError` are added, they are mapped into different variants of
     // `LayerError`.
@@ -218,6 +221,9 @@ impl From<HookError> for i64 {
             HookError::FileNotFound => {
                 info!("mirrord file not found triggered")
             }
+            HookError::SocketUnsuportedIpv6 => {
+                info!("{fail}")
+            }
             HookError::ProxyError(ref err) => {
                 graceful_exit!(
                     r"Proxy error, connectivity issue or a bug.
@@ -286,6 +292,7 @@ impl From<HookError> for i64 {
             HookError::LocalFileCreation(_) => libc::EINVAL,
             #[cfg(target_os = "macos")]
             HookError::FailedSipPatch(_) => libc::EACCES,
+            HookError::SocketUnsuportedIpv6 => libc::EAFNOSUPPORT,
             HookError::UnsupportedSocketType => libc::EAFNOSUPPORT,
             HookError::BadPointer => libc::EFAULT,
             HookError::AddressAlreadyBound(_) => libc::EADDRINUSE,

mirrord/layer/src/socket.rs

@@ -1,7 +1,5 @@
 //! 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},
@@ -384,13 +382,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().to_canonical().is_loopback() {
+        if address.ip().is_loopback() {
             return Ok(address);
         }
 
         let cached = REMOTE_DNS_REVERSE_MAPPING
             .lock()?
-            .get(&address.ip().to_canonical())
+            .get(&address.ip())
             .cloned();
         let Some(hostname) = cached else {
             return Ok(address);
@@ -460,7 +458,7 @@ impl ProtocolAndAddressFilterExt for ProtocolAndAddressFilter {
                     let _guard = DetourGuard::new();
 
                     match (name.as_str(), *port).to_socket_addrs() {
-                        Ok(addresses) => addresses.map(|addr| addr.ip().to_canonical()).collect(),
+                        Ok(addresses) => addresses.map(|addr| addr.ip()).collect(),
                         Err(e) => {
                             let as_string = e.to_string();
                             if as_string.contains("Temporary failure in name resolution")
@@ -479,13 +477,12 @@ impl ProtocolAndAddressFilterExt for ProtocolAndAddressFilter {
                     }
                 };
 
-                Ok(resolved_ips
-                    .into_iter()
-                    .any(|ip| ip == address.ip().to_canonical()))
+                Ok(resolved_ips.into_iter().any(|ip| ip == 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::Socket(addr) => {
+                Ok(addr.ip().is_unspecified() || addr.ip() == address.ip())
+            }
+            AddressFilter::Subnet(net, _) => Ok(net.contains(&address.ip())),
             AddressFilter::Port(..) => Ok(true),
         }
     }

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().to_canonical().is_unspecified()
+                    filter_socket.ip().is_unspecified()
                         || Some(filter_socket.ip()) == node.parse().ok()
                 }
                 AddressFilter::Subnet(filter_subnet, _) => {

mirrord/layer/src/socket/ops.rs

@@ -129,6 +129,10 @@ pub(super) fn socket(domain: c_int, type_: c_int, protocol: c_int) -> Detour<Raw
         Ok(())
     }?;
 
+    if domain == libc::AF_INET6 {
+        return Detour::Error(HookError::SocketUnsuportedIpv6);
+    }
+
     let socket_result = unsafe { FN_SOCKET(domain, type_, protocol) };
 
     let socket_fd = if socket_result == -1 {
@@ -154,9 +158,7 @@ fn bind_similar_address(sockfd: c_int, requested_address: &SocketAddr) -> Detour
     let addr = requested_address.ip();
     let port = requested_address.port();
 
-    let canonical_address = addr.to_canonical();
-
-    let address = if canonical_address.is_loopback() || canonical_address.is_unspecified() {
+    let address = if addr.is_loopback() || addr.is_unspecified() {
         *requested_address
     } else if addr.is_ipv4() {
         SocketAddr::new(IpAddr::V4(Ipv4Addr::LOCALHOST), port)
@@ -255,7 +257,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().to_canonical().is_loopback() {
+    if incoming_config.ignore_localhost && requested_address.ip().is_loopback() {
         return Detour::Bypass(Bypass::IgnoreLocalhost(requested_port));
     }
 
@@ -566,11 +568,7 @@ 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);
@@ -587,7 +585,7 @@ pub(super) fn connect(
                 .family()
                 .map(|family| family as i32)
                 .unwrap_or(-1);
-            if domain != libc::AF_INET && domain != libc::AF_UNIX && !is_ipv4_in_ipv6 {
+            if domain != libc::AF_INET && domain != libc::AF_UNIX {
                 return Detour::Bypass(Bypass::Domain(domain));
             }
             // I really hate it, but nix seems to really make this API bad :()
@@ -609,9 +607,8 @@ pub(super) fn connect(
             return Detour::Success(connect_result);
         }
 
-        let canonical_ip = ip_address.ip().to_canonical();
-
-        if canonical_ip.is_loopback() || canonical_ip.is_unspecified() {
+        let ip = ip_address.ip();
+        if ip.is_loopback() || 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.

mirrord/protocol/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "mirrord-protocol"
-version = "1.11.1"
+version = "1.11.2"
 authors.workspace = true
 description.workspace = true
 documentation.workspace = true

mirrord/protocol/src/outgoing.rs

@@ -83,11 +83,6 @@ 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()