refactor: remove derp meshing (#2079)

## Description Since the advent of the `DerpUrl`, we no longer use any meshing. We are currently on a mission to remove unused and unnecessary code, so the derper meshing has to go! closes #2074 ## Notes We have removed three message types from the protocol: `PeerPresent`, `ClosePeer`, and `WatchConns` These messages were only used to communicate between meshed derpers. Now, if a server recieves on of those messages from a client, the server will error on that connection. This also removes the `PacketForwarder` trait, since derp nodes will no longer act as packet forwarders for each other, they will only ever relay packets between clients. ## Change checklist - [x] Self-review. - [x] Documentation updates if relevant. - [x] Tests if relevant.
n0-computer · Mar 14, 2024 · 29065fd · 29065fd
1 parent 5c70cd2
commit 29065fd
Show file tree

Hide file tree

Showing 14 changed files with 74 additions and 1,810 deletions.
diff --git a/iroh-gossip/src/net.rs b/iroh-gossip/src/net.rs
@@ -854,8 +854,6 @@ mod test {
         pub(crate) async fn run_derp_and_stun(
             stun_ip: IpAddr,
         ) -> Result<(DerpMap, DerpUrl, CleanupDropGuard)> {
-            // TODO: pass a mesh_key?
-
             let server_key = SecretKey::generate();
             let server = iroh_net::derp::http::ServerBuilder::new("127.0.0.1:0".parse().unwrap())
                 .secret_key(Some(server_key))

diff --git a/iroh-net/src/bin/derper.rs b/iroh-net/src/bin/derper.rs
@@ -19,9 +19,9 @@ use hyper::{Method, Request, Response, StatusCode};
 use iroh_metrics::inc;
 use iroh_net::defaults::{DEFAULT_DERP_STUN_PORT, NA_DERP_HOSTNAME};
 use iroh_net::derp::http::{
-    MeshAddrs, ServerBuilder as DerpServerBuilder, TlsAcceptor, TlsConfig as DerpTlsConfig,
+    ServerBuilder as DerpServerBuilder, TlsAcceptor, TlsConfig as DerpTlsConfig,
 };
-use iroh_net::derp::{self, DerpUrl};
+use iroh_net::derp::{self};
 use iroh_net::key::SecretKey;
 use iroh_net::stun;
 use serde::{Deserialize, Serialize};
@@ -196,22 +196,11 @@ struct Config {
     tls: Option<TlsConfig>,
     /// Rate limiting configuration
     limits: Option<Limits>,
-    /// Mesh network configuration
-    mesh: Option<MeshConfig>,
     #[cfg(feature = "metrics")]
     /// Metrics serve address. If not set, metrics are not served.
     metrics_addr: Option<SocketAddr>,
 }
 
-#[derive(Serialize, Deserialize)]
-struct MeshConfig {
-    /// Path to file containing the mesh pre-shared key file. It should contain some hex string; whitespace is trimmed.
-    mesh_psk_file: PathBuf,
-    /// Comma-separated list of urls to mesh with. Must also include the scheme ('http' or
-    /// 'https').
-    mesh_with: Vec<DerpUrl>,
-}
-
 #[derive(Serialize, Deserialize)]
 struct TlsConfig {
     /// Mode for getting a cert. possible options: 'Manual', 'LetsEncrypt'
@@ -258,7 +247,6 @@ impl Default for Config {
             enable_derp: true,
             tls: None,
             limits: None,
-            mesh: None,
             #[cfg(feature = "metrics")]
             metrics_addr: None,
         }
@@ -402,26 +390,10 @@ async fn run(
     }
 
     // set up derp configuration details
-    let (secret_key, mesh_key, mesh_derpers) = match cfg.enable_derp {
-        true => {
-            let (mesh_key, mesh_derpers) = if let Some(mesh_config) = cfg.mesh {
-                let raw = tokio::fs::read_to_string(mesh_config.mesh_psk_file)
-                    .await
-                    .context("reading mesh-pks file")?;
-                let mut mesh_key = [0u8; 32];
-                hex::decode_to_slice(raw.trim(), &mut mesh_key)
-                    .context("invalid mesh-pks content")?;
-                info!("DERP mesh key configured");
-                (
-                    Some(mesh_key),
-                    Some(MeshAddrs::Addrs(mesh_config.mesh_with)),
-                )
-            } else {
-                (None, None)
-            };
-            (Some(cfg.secret_key), mesh_key, mesh_derpers)
-        }
-        false => (None, None, None),
+    let secret_key = if cfg.enable_derp {
+        Some(cfg.secret_key)
+    } else {
+        None
     };
 
     // run stun
@@ -463,11 +435,9 @@ async fn run(
 
     let mut builder = DerpServerBuilder::new(addr)
         .secret_key(secret_key.map(Into::into))
-        .mesh_key(mesh_key)
         .headers(headers)
         .tls_config(tls_config.clone())
         .derp_override(Box::new(derp_disabled_handler))
-        .mesh_derpers(mesh_derpers)
         .request_handler(Method::GET, "/", Box::new(root_handler))
         .request_handler(Method::GET, "/index.html", Box::new(root_handler))
         .request_handler(Method::GET, "/derp/probe", Box::new(probe_handler))
@@ -775,19 +745,6 @@ async fn server_stun_listener(sock: UdpSocket) {
 // 	return errors.New("invalid hostname")
 // }
 
-// func defaultMeshPSKFile() string {
-// 	try := []string{
-// 		"/home/derp/keys/derp-mesh.key",
-// 		filepath.Join(os.Getenv("HOME"), "keys", "derp-mesh.key"),
-// 	}
-// 	for _, p := range try {
-// 		if _, err := os.Stat(p); err == nil {
-// 			return p
-// 		}
-// 	}
-// 	return ""
-// }
-
 // func rateLimitedListenAndServeTLS(srv *http.Server) error {
 // 	addr := srv.Addr
 // 	if addr == "" {
@@ -906,6 +863,7 @@ mod tests {
     use anyhow::Result;
     use bytes::Bytes;
     use http_body_util::BodyExt;
+    use iroh_base::node_addr::DerpUrl;
     use iroh_net::derp::http::ClientBuilder;
     use iroh_net::derp::ReceivedMessage;
     use iroh_net::key::SecretKey;

diff --git a/iroh-net/src/derp.rs b/iroh-net/src/derp.rs
@@ -25,8 +25,5 @@ pub use self::codec::MAX_PACKET_SIZE;
 pub use self::http::Client as HttpClient;
 pub use self::map::{DerpMap, DerpMode, DerpNode};
 pub use self::metrics::Metrics;
-pub use self::server::{
-    ClientConnHandler, MaybeTlsStream as MaybeTlsStreamServer, PacketForwarderHandler, Server,
-};
-pub use self::types::{MeshKey, PacketForwarder};
+pub use self::server::{ClientConnHandler, MaybeTlsStream as MaybeTlsStreamServer, Server};
 pub use iroh_base::node_addr::DerpUrl;
diff --git a/iroh-net/src/derp/client.rs b/iroh-net/src/derp/client.rs
@@ -18,7 +18,7 @@ use super::{
         recv_frame, write_frame, DerpCodec, Frame, FrameType, MAX_PACKET_SIZE,
         PER_CLIENT_SEND_QUEUE_DEPTH, PROTOCOL_VERSION,
     },
-    types::{ClientInfo, MeshKey, RateLimiter, ServerInfo},
+    types::{ClientInfo, RateLimiter, ServerInfo},
 };
 
 use crate::key::{PublicKey, SecretKey};
@@ -92,23 +92,6 @@ impl Client {
         Ok(())
     }
 
-    /// Used by mesh peers to forward packets.
-    ///
-    // TODO: this is the only method with a timeout, why? Why does it have a timeout and no rate
-    // limiter?
-    pub async fn forward_packet(
-        &self,
-        srckey: PublicKey,
-        dstkey: PublicKey,
-        packet: Bytes,
-    ) -> Result<()> {
-        self.inner
-            .writer_channel
-            .send(ClientWriterMessage::FwdPacket((srckey, dstkey, packet)))
-            .await?;
-        Ok(())
-    }
-
     /// Send a ping with 8 bytes of random data.
     pub async fn send_ping(&self, data: [u8; 8]) -> Result<()> {
         self.inner
@@ -139,27 +122,6 @@ impl Client {
         Ok(())
     }
 
-    /// Sends a request to subscribe to the peer's connection list.
-    /// It's a fatal error if the client wasn't created using [`MeshKey`].
-    pub async fn watch_connection_changes(&self) -> Result<()> {
-        self.inner
-            .writer_channel
-            .send(ClientWriterMessage::WatchConnectionChanges)
-            .await?;
-        Ok(())
-    }
-
-    /// Asks the server to close the target's TCP connection.
-    ///
-    /// It's a fatal error if the client wasn't created using [`MeshKey`]
-    pub async fn close_peer(&self, target: PublicKey) -> Result<()> {
-        self.inner
-            .writer_channel
-            .send(ClientWriterMessage::ClosePeer(target))
-            .await?;
-        Ok(())
-    }
-
     /// The local address that the [`Client`] is listening on.
     pub fn local_addr(&self) -> Result<SocketAddr> {
         Ok(self.inner.local_addr)
@@ -203,7 +165,6 @@ fn process_incoming_frame(frame: Frame) -> Result<ReceivedMessage> {
             Ok(ReceivedMessage::KeepAlive)
         }
         Frame::PeerGone { peer } => Ok(ReceivedMessage::PeerGone(peer)),
-        Frame::PeerPresent { peer } => Ok(ReceivedMessage::PeerPresent(peer)),
         Frame::RecvPacket { src_key, content } => {
             let packet = ReceivedMessage::ReceivedPacket {
                 source: src_key,
@@ -238,21 +199,12 @@ fn process_incoming_frame(frame: Frame) -> Result<ReceivedMessage> {
 enum ClientWriterMessage {
     /// Send a packet (addressed to the [`PublicKey`]) to the server
     Packet((PublicKey, Bytes)),
-    /// Forward a packet from the src [`PublicKey`] to the dst [`PublicKey`] to the server
-    /// Should only be used for mesh clients.
-    FwdPacket((PublicKey, PublicKey, Bytes)),
     /// Send a pong to the server
     Pong([u8; 8]),
     /// Send a ping to the server
     Ping([u8; 8]),
     /// Tell the server whether or not this client is the user's preferred client
     NotePreferred(bool),
-    /// Subscribe to the server's connection list.
-    /// Should only be used for mesh clients.
-    WatchConnectionChanges,
-    /// Asks the server to close the target's connection.
-    /// Should only be used for mesh clients.
-    ClosePeer(PublicKey),
     /// Shutdown the writer
     Shutdown,
 }
@@ -273,18 +225,8 @@ impl<W: AsyncWrite + Unpin + Send + 'static> ClientWriter<W> {
         while let Some(msg) = self.recv_msgs.recv().await {
             match msg {
                 ClientWriterMessage::Packet((key, bytes)) => {
-                    // TODO: the rate limiter is only used on this method, is it because it's the only method that
-                    // theoretically sends a bunch of data, or is it an oversight? For example,
-                    // the `forward_packet` method does not have a rate limiter, but _does_ have a timeout.
                     send_packet(&mut self.writer, &self.rate_limiter, key, bytes).await?;
                 }
-                ClientWriterMessage::FwdPacket((srckey, dstkey, bytes)) => {
-                    tokio::time::timeout(
-                        Duration::from_secs(5),
-                        forward_packet(&mut self.writer, srckey, dstkey, bytes),
-                    )
-                    .await??;
-                }
                 ClientWriterMessage::Pong(data) => {
                     write_frame(&mut self.writer, Frame::Pong { data }, None).await?;
                     self.writer.flush().await?;
@@ -297,14 +239,6 @@ impl<W: AsyncWrite + Unpin + Send + 'static> ClientWriter<W> {
                     write_frame(&mut self.writer, Frame::NotePreferred { preferred }, None).await?;
                     self.writer.flush().await?;
                 }
-                ClientWriterMessage::WatchConnectionChanges => {
-                    write_frame(&mut self.writer, Frame::WatchConns, None).await?;
-                    self.writer.flush().await?;
-                }
-                ClientWriterMessage::ClosePeer(peer) => {
-                    write_frame(&mut self.writer, Frame::ClosePeer { peer }, None).await?;
-                    self.writer.flush().await?;
-                }
                 ClientWriterMessage::Shutdown => {
                     return Ok(());
                 }
@@ -321,7 +255,6 @@ pub struct ClientBuilder {
     reader: DerpReader,
     writer: FramedWrite<Box<dyn AsyncWrite + Unpin + Send + Sync + 'static>, DerpCodec>,
     local_addr: SocketAddr,
-    mesh_key: Option<MeshKey>,
     is_prober: bool,
     server_public_key: Option<PublicKey>,
     can_ack_pings: bool,
@@ -339,18 +272,12 @@ impl ClientBuilder {
             reader: FramedRead::new(reader, DerpCodec),
             writer: FramedWrite::new(writer, DerpCodec),
             local_addr,
-            mesh_key: None,
             is_prober: false,
             server_public_key: None,
             can_ack_pings: false,
         }
     }
 
-    pub fn mesh_key(mut self, mesh_key: Option<MeshKey>) -> Self {
-        self.mesh_key = mesh_key;
-        self
-    }
-
     pub fn prober(mut self, is_prober: bool) -> Self {
         self.is_prober = is_prober;
         self
@@ -383,7 +310,6 @@ impl ClientBuilder {
         }
         let client_info = ClientInfo {
             version: PROTOCOL_VERSION,
-            mesh_key: self.mesh_key,
             can_ack_pings: self.can_ack_pings,
             is_prober: self.is_prober,
         };
@@ -520,8 +446,6 @@ pub enum ReceivedMessage {
     /// Indicates that the client identified by the underlying public key had previously sent you a
     /// packet but has now disconnected from the server.
     PeerGone(PublicKey),
-    /// Indicates that the client is connected to the server. (Only used by trusted mesh clients)
-    PeerPresent(PublicKey),
     /// Sent by the server upon first connect.
     ServerInfo {
         /// How many bytes per second the server says it will accept, including all framing bytes.
@@ -591,29 +515,3 @@ pub(crate) async fn send_packet<S: Sink<Frame, Error = std::io::Error> + Unpin>(
 
     Ok(())
 }
-
-pub(crate) async fn forward_packet<S: Sink<Frame, Error = std::io::Error> + Unpin>(
-    mut writer: S,
-    src_key: PublicKey,
-    dst_key: PublicKey,
-    packet: Bytes,
-) -> Result<()> {
-    ensure!(
-        packet.len() <= MAX_PACKET_SIZE,
-        "packet too big: {}",
-        packet.len()
-    );
-
-    write_frame(
-        &mut writer,
-        Frame::ForwardPacket {
-            src_key,
-            dst_key,
-            packet,
-        },
-        None,
-    )
-    .await?;
-    writer.flush().await?;
-    Ok(())
-}