fix: actually allow to disable DERP (#1560)

## Description

`node::Builder::disable_derp` is broken. We do not call
`MagicEndpointBuilder::disable_derp` for that, and at some point we
changed the magic endpoint to default to the default, n0-supplied derp
servers.

This fixes this through a `DerpMode` enum that is much more explicit I
think.

Fixes #1558 

## Notes & open questions

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

## Change checklist

- [x] Self-review.
- [x] Documentation updates if relevant.
- [x] Tests if relevant.

iroh-gossip/examples/chat.rs

@@ -9,8 +9,7 @@ use iroh_gossip::{
     proto::{util::base32, Event, TopicId},
 };
 use iroh_net::{
-    defaults::default_derp_map,
-    derp::DerpMap,
+    derp::{DerpMap, DerpMode},
     key::{PublicKey, SecretKey},
     magic_endpoint::accept_conn,
     MagicEndpoint, PeerAddr,
@@ -95,13 +94,13 @@ async fn main() -> anyhow::Result<()> {
     println!("> our secret key: {}", base32::fmt(secret_key.to_bytes()));
 
     // configure our derp map
-    let derp_map = match (args.no_derp, args.derp) {
-        (false, None) => Some(default_derp_map()),
-        (false, Some(url)) => Some(DerpMap::from_url(url, 0)),
-        (true, None) => None,
+    let derp_mode = match (args.no_derp, args.derp) {
+        (false, None) => DerpMode::Default,
+        (false, Some(url)) => DerpMode::Custom(DerpMap::from_url(url, 0)),
+        (true, None) => DerpMode::Disabled,
         (true, Some(_)) => bail!("You cannot set --no-derp and --derp at the same time"),
     };
-    println!("> using DERP servers: {}", fmt_derp_map(&derp_map));
+    println!("> using DERP servers: {}", fmt_derp_mode(&derp_mode));
 
     // init a cell that will hold our gossip handle to be used in endpoint callbacks
     let gossip_cell: OnceCell<Gossip> = OnceCell::new();
@@ -113,6 +112,7 @@ async fn main() -> anyhow::Result<()> {
     let endpoint = MagicEndpoint::builder()
         .secret_key(secret_key)
         .alpns(vec![GOSSIP_ALPN.to_vec()])
+        .derp_mode(derp_mode)
         .on_endpoints({
             let gossip_cell = gossip_cell.clone();
             let notify = notify.clone();
@@ -124,12 +124,9 @@ async fn main() -> anyhow::Result<()> {
                 // notify the outer task of the initial endpoint update (later updates are not interesting)
                 notify.notify_one();
             })
-        });
-    let endpoint = match derp_map {
-        Some(derp_map) => endpoint.enable_derp(derp_map),
-        None => endpoint,
-    };
-    let endpoint = endpoint.bind(args.bind_port).await?;
+        })
+        .bind(args.bind_port)
+        .await?;
     println!("> our peer id: {}", endpoint.peer_id());
 
     // create the gossip protocol
@@ -328,10 +325,11 @@ fn parse_secret_key(secret: &str) -> anyhow::Result<SecretKey> {
     Ok(SecretKey::from(bytes))
 }
 
-fn fmt_derp_map(derp_map: &Option<DerpMap>) -> String {
-    match derp_map {
-        None => "None".to_string(),
-        Some(map) => map
+fn fmt_derp_mode(derp_mode: &DerpMode) -> String {
+    match derp_mode {
+        DerpMode::Disabled => "None".to_string(),
+        DerpMode::Default => "Default Derp servers".to_string(),
+        DerpMode::Custom(map) => map
             .regions()
             .flat_map(|region| region.nodes.iter().map(|node| node.url.to_string()))
             .collect::<Vec<_>>()

iroh-gossip/src/net.rs

@@ -613,7 +613,10 @@ mod test {
     use std::time::Duration;
 
     use iroh_net::PeerAddr;
-    use iroh_net::{derp::DerpMap, MagicEndpoint};
+    use iroh_net::{
+        derp::{DerpMap, DerpMode},
+        MagicEndpoint,
+    };
     use tokio::spawn;
     use tokio::time::timeout;
     use tokio_util::sync::CancellationToken;
@@ -624,7 +627,7 @@ mod test {
     async fn create_endpoint(derp_map: DerpMap) -> anyhow::Result<MagicEndpoint> {
         MagicEndpoint::builder()
             .alpns(vec![GOSSIP_ALPN.to_vec()])
-            .enable_derp(derp_map)
+            .derp_mode(DerpMode::Custom(derp_map))
             .bind(0)
             .await
     }

iroh-net/examples/magic.rs

@@ -2,8 +2,8 @@ use std::net::SocketAddr;
 
 use clap::Parser;
 use iroh_net::{
-    defaults::{default_derp_map, TEST_REGION_ID},
-    derp::DerpMap,
+    defaults::TEST_REGION_ID,
+    derp::{DerpMap, DerpMode},
     key::SecretKey,
     magic_endpoint::accept_conn,
     MagicEndpoint, PeerAddr,
@@ -50,16 +50,16 @@ async fn main() -> anyhow::Result<()> {
         Some(key) => parse_secret(&key)?,
     };
 
-    let derp_map = match args.derp_url {
-        None => default_derp_map(),
+    let derp_mode = match args.derp_url {
+        None => DerpMode::Default,
         // use `region_id` 65535, which is reserved for testing and experiments
-        Some(url) => DerpMap::from_url(url, TEST_REGION_ID),
+        Some(url) => DerpMode::Custom(DerpMap::from_url(url, TEST_REGION_ID)),
     };
 
     let endpoint = MagicEndpoint::builder()
         .secret_key(secret_key)
         .alpns(vec![args.alpn.to_string().into_bytes()])
-        .enable_derp(derp_map)
+        .derp_mode(derp_mode)
         .bind(args.bind_port)
         .await?;
 

iroh-net/src/derp.rs

@@ -22,7 +22,7 @@ pub(crate) mod types;
 pub use self::client::{Client as DerpClient, ReceivedMessage};
 pub use self::codec::MAX_PACKET_SIZE;
 pub use self::http::Client as HttpClient;
-pub use self::map::{DerpMap, DerpNode, DerpRegion, UseIpv4, UseIpv6};
+pub use self::map::{DerpMap, DerpMode, DerpNode, DerpRegion, UseIpv4, UseIpv6};
 pub use self::metrics::Metrics;
 pub use self::server::{
     ClientConnHandler, MaybeTlsStream as MaybeTlsStreamServer, PacketForwarderHandler, Server,

iroh-net/src/derp/map.rs

@@ -13,8 +13,19 @@ use url::Url;
 
 use crate::defaults::DEFAULT_DERP_STUN_PORT;
 
+/// Configuration options for the Derp servers of the magic endpoint.
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub enum DerpMode {
+    /// Disable Derp servers completely.
+    Disabled,
+    /// Use the default Derp map, with Derp servers from n0.
+    Default,
+    /// Use a custom Derp map.
+    Custom(DerpMap),
+}
+
 /// Configuration of all the Derp servers that can be used.
-#[derive(Debug, Default, Clone, PartialEq, Eq)]
+#[derive(Debug, Clone, PartialEq, Eq)]
 pub struct DerpMap {
     /// A map of the different region IDs to the [`DerpRegion`] information
     regions: Arc<HashMap<u16, DerpRegion>>,
@@ -28,6 +39,13 @@ impl DerpMap {
         ids
     }
 
+    /// Create an empty Derp map.
+    pub fn empty() -> Self {
+        Self {
+            regions: Default::default(),
+        }
+    }
+
     /// Returns an `Iterator` over all known regions.
     pub fn regions(&self) -> impl Iterator<Item = &DerpRegion> {
         self.regions.values()

iroh-net/src/magic_endpoint.rs

@@ -10,7 +10,7 @@ use tracing::{debug, trace};
 use crate::{
     config,
     defaults::default_derp_map,
-    derp::DerpMap,
+    derp::{DerpMap, DerpMode},
     key::{PublicKey, SecretKey},
     magicsock::{self, Callbacks, MagicSock},
     tls,
@@ -112,7 +112,7 @@ impl PeerAddr {
 #[derive(Debug)]
 pub struct MagicEndpointBuilder {
     secret_key: Option<SecretKey>,
-    derp_map: Option<DerpMap>,
+    derp_mode: DerpMode,
     alpn_protocols: Vec<Vec<u8>>,
     transport_config: Option<quinn::TransportConfig>,
     concurrent_connections: Option<u32>,
@@ -126,7 +126,7 @@ impl Default for MagicEndpointBuilder {
     fn default() -> Self {
         Self {
             secret_key: Default::default(),
-            derp_map: Some(default_derp_map()),
+            derp_mode: DerpMode::Default,
             alpn_protocols: Default::default(),
             transport_config: Default::default(),
             concurrent_connections: Default::default(),
@@ -162,32 +162,19 @@ impl MagicEndpointBuilder {
         self
     }
 
-    /// Enables using DERP servers to assist in establishing connectivity.
+    /// Sets the DERP servers to assist in establishing connectivity.
     ///
     /// DERP servers are used to discover other peers by [`PublicKey`] and also help
     /// establish connections between peers by being an initial relay for traffic while
     /// assisting in holepunching to establish a direct connection between peers.
     ///
-    /// The provided `derp_map` must contain at least one region with a configured derp
-    /// node.  If an invalid [`DerpMap`] is provided [`bind`] will result in an error.
-    ///
-    /// When calling neither this, nor [`disable_derp`] the builder uses the
-    /// [`default_derp_map`] containing number0's global derp servers.
+    /// When using [DerpMode::Custom], the provided `derp_map` must contain at least one
+    /// region with a configured derp node.  If an invalid [`DerpMap`] is provided [`bind`]
+    /// will result in an error.
     ///
     /// [`bind`]: MagicEndpointBuilder::bind
-    /// [`disable_derp`]: MagicEndpointBuilder::disable_derp
-    pub fn enable_derp(mut self, derp_map: DerpMap) -> Self {
-        self.derp_map = Some(derp_map);
-        self
-    }
-
-    /// Disables using DERP servers.
-    ///
-    /// See [`enable_derp`] for details.
-    ///
-    /// [`enable_derp`]: MagicEndpointBuilder::enable_derp
-    pub fn disable_derp(mut self) -> Self {
-        self.derp_map = None;
+    pub fn derp_mode(mut self, derp_mode: DerpMode) -> Self {
+        self.derp_mode = derp_mode;
         self
     }
 
@@ -253,13 +240,14 @@ impl MagicEndpointBuilder {
     /// You can pass `0` to let the operating system choose a free port for you.
     /// NOTE: This will be improved soon to add support for binding on specific addresses.
     pub async fn bind(self, bind_port: u16) -> Result<MagicEndpoint> {
-        ensure!(
-            self.derp_map
-                .as_ref()
-                .map(|m| !m.is_empty())
-                .unwrap_or(true),
-            "Derp server enabled but DerpMap is empty",
-        );
+        let derp_map = match self.derp_mode {
+            DerpMode::Disabled => DerpMap::empty(),
+            DerpMode::Default => default_derp_map(),
+            DerpMode::Custom(derp_map) => {
+                ensure!(!derp_map.is_empty(), "Empty custom Derp server map",);
+                derp_map
+            }
+        };
         let secret_key = self.secret_key.unwrap_or_else(SecretKey::generate);
         let mut server_config = make_server_config(
             &secret_key,
@@ -273,7 +261,7 @@ impl MagicEndpointBuilder {
         let msock_opts = magicsock::Options {
             port: bind_port,
             secret_key,
-            derp_map: self.derp_map.unwrap_or_default(),
+            derp_map,
             callbacks: self.callbacks,
             peers_path: self.peers_path,
         };
@@ -579,7 +567,7 @@ mod tests {
                     let ep = MagicEndpoint::builder()
                         .secret_key(server_secret_key)
                         .alpns(vec![TEST_ALPN.to_vec()])
-                        .enable_derp(derp_map)
+                        .derp_mode(DerpMode::Custom(derp_map))
                         .bind(0)
                         .await
                         .unwrap();
@@ -612,7 +600,7 @@ mod tests {
             async move {
                 let ep = MagicEndpoint::builder()
                     .alpns(vec![TEST_ALPN.to_vec()])
-                    .enable_derp(derp_map)
+                    .derp_mode(DerpMode::Custom(derp_map))
                     .bind(0)
                     .await
                     .unwrap();

iroh-net/src/magicsock.rs

@@ -174,7 +174,7 @@ impl Default for Options {
         Options {
             port: 0,
             secret_key: SecretKey::generate(),
-            derp_map: Default::default(),
+            derp_map: DerpMap::empty(),
             callbacks: Default::default(),
             peers_path: None,
         }
@@ -2648,7 +2648,7 @@ pub(crate) mod tests {
     use tracing_subscriber::{prelude::*, EnvFilter};
 
     use super::*;
-    use crate::{test_utils::run_derper, tls, MagicEndpoint};
+    use crate::{derp::DerpMode, test_utils::run_derper, tls, MagicEndpoint};
 
     fn make_transmit(destination: SocketAddr) -> quinn_udp::Transmit {
         quinn_udp::Transmit {
@@ -2797,7 +2797,7 @@ pub(crate) mod tests {
                     on_derp_s.try_send(()).ok();
                 }))
                 .transport_config(transport_config)
-                .enable_derp(derp_map)
+                .derp_mode(DerpMode::Custom(derp_map))
                 .alpns(vec![ALPN.to_vec()])
                 .bind(0)
                 .await?;

iroh/examples/sync.rs

@@ -37,7 +37,9 @@ use iroh_gossip::{
 };
 use iroh_io::AsyncSliceReaderExt;
 use iroh_net::{
-    defaults::default_derp_map, derp::DerpMap, key::SecretKey, magic_endpoint::get_alpn,
+    derp::{DerpMap, DerpMode},
+    key::SecretKey,
+    magic_endpoint::get_alpn,
     MagicEndpoint, PeerAddr,
 };
 use iroh_sync::{
@@ -130,13 +132,13 @@ async fn run(args: Args) -> anyhow::Result<()> {
     println!("> our secret key: {}", secret_key);
 
     // configure our derp map
-    let derp_map = match (args.no_derp, args.derp) {
-        (false, None) => Some(default_derp_map()),
-        (false, Some(url)) => Some(DerpMap::from_url(url, 0)),
-        (true, None) => None,
+    let derp_mode = match (args.no_derp, args.derp) {
+        (false, None) => DerpMode::Default,
+        (false, Some(url)) => DerpMode::Custom(DerpMap::from_url(url, 0)),
+        (true, None) => DerpMode::Disabled,
         (true, Some(_)) => bail!("You cannot set --no-derp and --derp at the same time"),
     };
-    println!("> using DERP servers: {}", fmt_derp_map(&derp_map));
+    println!("> using DERP servers: {}", fmt_derp_mode(&derp_mode));
 
     // build our magic endpoint and the gossip protocol
     let (endpoint, gossip) = {
@@ -152,6 +154,7 @@ async fn run(args: Args) -> anyhow::Result<()> {
                 SYNC_ALPN.to_vec(),
                 iroh_bytes::protocol::ALPN.to_vec(),
             ])
+            .derp_mode(derp_mode)
             .on_endpoints({
                 let gossip_cell = gossip_cell.clone();
                 Box::new(move |endpoints| {
@@ -162,12 +165,9 @@ async fn run(args: Args) -> anyhow::Result<()> {
                     // trigger oneshot on the first endpoint update
                     initial_endpoints_tx.try_send(endpoints.to_vec()).ok();
                 })
-            });
-        let endpoint = match derp_map {
-            Some(derp_map) => endpoint.enable_derp(derp_map),
-            None => endpoint,
-        };
-        let endpoint = endpoint.bind(args.bind_port).await?;
+            })
+            .bind(args.bind_port)
+            .await?;
 
         // initialize the gossip protocol
         let gossip = Gossip::from_endpoint(endpoint.clone(), Default::default());
@@ -956,10 +956,11 @@ fn fmt_hash(hash: impl AsRef<[u8]>) -> String {
     text.make_ascii_lowercase();
     format!("{}…{}", &text[..5], &text[(text.len() - 2)..])
 }
-fn fmt_derp_map(derp_map: &Option<DerpMap>) -> String {
-    match derp_map {
-        None => "None".to_string(),
-        Some(map) => map
+fn fmt_derp_mode(derp_mode: &DerpMode) -> String {
+    match derp_mode {
+        DerpMode::Disabled => "None".to_string(),
+        DerpMode::Default => "Default Derp servers".to_string(),
+        DerpMode::Custom(map) => map
             .regions()
             .flat_map(|region| region.nodes.iter().map(|node| node.url.to_string()))
             .collect::<Vec<_>>()