refactor(iroh, iroh-net-report)!: Make ports more private (#3207)

## Description

I find this whole port business of these mapped addresses very
confusing.  This attempts to document them better, make it clearer
that these SocketAddrs are made up and are not routable.

## Breaking Changes

### iroh-net-report

- `MAPPED_ADDR_PORT` is removed.
- `IpMappedAddr::socket_addr` -> `IpMappedAddr::private_socket_addr`

## Notes & open questions

<!-- Any notes, remarks or open questions you have to make about the PR.
-->

## Change checklist
<!-- Remove any that are not relevant. -->
- [x] Self-review.
- [x] Documentation updates following the [style
guide](https://rust-lang.github.io/rfcs/1574-more-api-documentation-conventions.html#appendix-a-full-conventions-text),
if relevant.
- [x] All breaking changes documented.
- [x] List all breaking changes in the above "Breaking Changes" section.

Author: flub

iroh-net-report/src/ip_mapped_addrs.rs

@@ -7,9 +7,6 @@ use std::{
     },
 };
 
-/// The dummy port used for all mapped addresses
-pub const MAPPED_ADDR_PORT: u16 = 12345;
-
 /// Can occur when converting a [`SocketAddr`] to an [`IpMappedAddr`]
 #[derive(Debug, thiserror::Error)]
 #[error("Failed to convert")]
@@ -32,6 +29,9 @@ impl IpMappedAddr {
     /// The Subnet ID used in our Unique Local Addresses.
     const ADDR_SUBNET: [u8; 2] = [0, 1];
 
+    /// The dummy port used for all mapped addresses.
+    const MAPPED_ADDR_PORT: u16 = 12345;
+
     /// Generates a globally unique fake UDP address.
     ///
     /// This generates a new IPv6 address in the Unique Local Address range (RFC 4193)
@@ -48,9 +48,15 @@ impl IpMappedAddr {
         Self(Ipv6Addr::from(addr))
     }
 
-    /// Return a [`SocketAddr`] from the [`IpMappedAddr`].
-    pub fn socket_addr(&self) -> SocketAddr {
-        SocketAddr::new(IpAddr::from(self.0), MAPPED_ADDR_PORT)
+    /// Returns a consistent [`SocketAddr`] for the [`IpMappedAddr`].
+    ///
+    /// This does not have a routable IP address.
+    ///
+    /// This uses a made-up, but fixed port number.  The [IpMappedAddresses`] map this is
+    /// made for creates a unique [`IpMappedAddr`] for each IP+port and thus does not use
+    /// the port to map back to the original [`SocketAddr`].
+    pub fn private_socket_addr(&self) -> SocketAddr {
+        SocketAddr::new(IpAddr::from(self.0), Self::MAPPED_ADDR_PORT)
     }
 }
 

iroh-net-report/src/lib.rs

@@ -64,7 +64,7 @@ pub mod portmapper {
     }
 }
 
-pub use ip_mapped_addrs::{IpMappedAddr, IpMappedAddrError, IpMappedAddresses, MAPPED_ADDR_PORT};
+pub use ip_mapped_addrs::{IpMappedAddr, IpMappedAddrError, IpMappedAddresses};
 pub use metrics::Metrics;
 pub use options::Options;
 pub use reportgen::QuicConfig;

iroh-net-report/src/reportgen.rs

@@ -968,7 +968,7 @@ fn maybe_to_mapped_addr(
     addr: SocketAddr,
 ) -> SocketAddr {
     if let Some(ip_mapped_addrs) = ip_mapped_addrs.as_ref() {
-        return ip_mapped_addrs.get_or_register(addr).socket_addr();
+        return ip_mapped_addrs.get_or_register(addr).private_socket_addr();
     }
     addr
 }

iroh/src/endpoint.rs

@@ -754,7 +754,7 @@ impl Endpoint {
         let server_name = &format!("{}.iroh.invalid", BASE32_DNSSEC.encode(node_id.as_bytes()));
         let connect = self.msock.endpoint().connect_with(
             client_config,
-            mapped_addr.socket_addr(),
+            mapped_addr.private_socket_addr(),
             server_name,
         )?;
 

iroh/src/magicsock.rs

@@ -958,7 +958,7 @@ impl MagicSock {
                                 "UDP recv QUIC address discovery packets",
                             );
                             quic_packets_total += quic_datagram_count;
-                            meta.addr = ip_mapped_addr.socket_addr();
+                            meta.addr = ip_mapped_addr.private_socket_addr();
                         } else {
                             warn!(
                                 src = ?meta.addr,
@@ -980,7 +980,7 @@ impl MagicSock {
                             "UDP recv quic packets",
                         );
                         quic_packets_total += quic_datagram_count;
-                        meta.addr = quic_mapped_addr.socket_addr();
+                        meta.addr = quic_mapped_addr.private_socket_addr();
                     }
                 }
             } else {
@@ -1094,7 +1094,7 @@ impl MagicSock {
         let meta = quinn_udp::RecvMeta {
             len: dm.buf.len(),
             stride: dm.buf.len(),
-            addr: quic_mapped_addr.socket_addr(),
+            addr: quic_mapped_addr.private_socket_addr(),
             dst_ip,
             ecn: None,
         };
@@ -3211,9 +3211,6 @@ fn split_packets(transmit: &quinn_udp::Transmit) -> RelayContents {
 #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
 pub(crate) struct NodeIdMappedAddr(Ipv6Addr);
 
-/// The dummy port used for all [`NodeIdMappedAddr`]s
-pub const NODE_ID_MAPPED_PORT: u16 = 12345;
-
 /// Can occur when converting a [`SocketAddr`] to an [`NodeIdMappedAddr`]
 #[derive(Debug, thiserror::Error)]
 #[error("Failed to convert")]
@@ -3230,6 +3227,9 @@ impl NodeIdMappedAddr {
     /// The Subnet ID used in our Unique Local Addresses.
     const ADDR_SUBNET: [u8; 2] = [0; 2];
 
+    /// The dummy port used for all [`NodeIdMappedAddr`]s.
+    const NODE_ID_MAPPED_PORT: u16 = 12345;
+
     /// Generates a globally unique fake UDP address.
     ///
     /// This generates and IPv6 Unique Local Address according to RFC 4193.
@@ -3245,9 +3245,15 @@ impl NodeIdMappedAddr {
         Self(Ipv6Addr::from(addr))
     }
 
-    /// Return the [`SocketAddr`] from the [`NodeIdMappedAddr`]
-    pub(crate) fn socket_addr(&self) -> SocketAddr {
-        SocketAddr::new(IpAddr::from(self.0), NODE_ID_MAPPED_PORT)
+    /// Returns a consistent [`SocketAddr`] for the [`NodeIdMappedAddr`].
+    ///
+    /// This socket address does not have a routable IP address.
+    ///
+    /// This uses a made-up port number, since the port does not play a role in looking up
+    /// the node in the [`NodeMap`].  This socket address is only to be used to pass into
+    /// Quinn.
+    pub(crate) fn private_socket_addr(&self) -> SocketAddr {
+        SocketAddr::new(IpAddr::from(self.0), Self::NODE_ID_MAPPED_PORT)
     }
 }
 
@@ -4372,7 +4378,11 @@ mod tests {
             tls::make_client_config(&ep_secret_key, Some(node_id), alpns, None, true)?;
         let mut client_config = quinn::ClientConfig::new(Arc::new(quic_client_config));
         client_config.transport_config(transport_config);
-        let connect = ep.connect_with(client_config, mapped_addr.socket_addr(), "localhost")?;
+        let connect = ep.connect_with(
+            client_config,
+            mapped_addr.private_socket_addr(),
+            "localhost",
+        )?;
         let connection = connect.await?;
         Ok(connection)
     }