enable_quic_addr_discovery is false by default

iroh-base/src/relay_map.rs

@@ -16,7 +16,7 @@ pub const DEFAULT_STUN_PORT: u16 = 3478;
 /// for QUIC address discovery
 ///
 /// The port is "QUIC" typed on a phone keypad.
-pub const DEFAULT_QUIC_PORT: u16 = 7842;
+pub const DEFAULT_RELAY_QUIC_PORT: u16 = 7842;
 
 /// Configuration of all the relay servers that can be used.
 #[derive(Debug, Clone, PartialEq, Eq)]
@@ -68,7 +68,7 @@ impl RelayMap {
     /// Allows to set a custom STUN port and different IP addresses for IPv4 and IPv6.
     /// If IP addresses are provided, no DNS lookup will be performed.
     ///
-    /// Sets the port to the default [`DEFAULT_QUIC_PORT`].
+    /// Sets the port to the default [`DEFAULT_RELAY_QUIC_PORT`].
     pub fn default_from_node(url: RelayUrl, stun_port: u16) -> Self {
         let mut nodes = BTreeMap::new();
         nodes.insert(
@@ -155,7 +155,7 @@ pub struct QuicConfig {
 impl Default for QuicConfig {
     fn default() -> Self {
         Self {
-            port: DEFAULT_QUIC_PORT,
+            port: DEFAULT_RELAY_QUIC_PORT,
         }
     }
 }

iroh-relay/README.md

@@ -60,7 +60,7 @@ Then run the server with the `--dev-quic` flag:
 
 The relay server will run over http on port 3340, as it does using the `--dev` flag, but it will also run a QUIC server on port 7824.
 
-The relay will use the configured TLS certificates for the QUIC connection, but use http (rather than https) for the server; the local certificates that we generate using `rcgen` may not work properly for the TCP relay connections.
+The relay will use the configured TLS certificates for the QUIC connection, but use http (rather than https) for the server.
 
 # License
 

iroh-relay/src/client.rs

@@ -377,7 +377,7 @@ impl ClientBuilder {
     }
 }
 
-#[cfg(test)]
+#[cfg(any(test))]
 /// Creates a client config that trusts any servers without verifying their TLS certificate.
 ///
 /// Should be used for testing local relay setups only.

iroh-relay/src/defaults.rs

@@ -1,6 +1,6 @@
 //! Default values used in the relay.
 
-pub use iroh_base::relay_map::{DEFAULT_QUIC_PORT, DEFAULT_STUN_PORT};
+pub use iroh_base::relay_map::{DEFAULT_RELAY_QUIC_PORT, DEFAULT_STUN_PORT};
 
 /// The default HTTP port used by the Relay server.
 pub const DEFAULT_HTTP_PORT: u16 = 80;

iroh-relay/src/main.rs

@@ -10,9 +10,11 @@ use std::{
 
 use anyhow::{bail, Context as _, Result};
 use clap::Parser;
-use iroh_base::relay_map::DEFAULT_QUIC_PORT;
 use iroh_relay::{
-    defaults::{DEFAULT_HTTPS_PORT, DEFAULT_HTTP_PORT, DEFAULT_METRICS_PORT, DEFAULT_STUN_PORT},
+    defaults::{
+        DEFAULT_HTTPS_PORT, DEFAULT_HTTP_PORT, DEFAULT_METRICS_PORT, DEFAULT_RELAY_QUIC_PORT,
+        DEFAULT_STUN_PORT,
+    },
     server::{self as relay, ClientConnRateLimit, QuicConfig},
 };
 use serde::{Deserialize, Serialize};
@@ -124,12 +126,6 @@ struct Config {
     /// `https_bind_addr` of the `tls` configuration section and only the captive portal
     /// will be served from the HTTP socket.
     http_bind_addr: Option<SocketAddr>,
-    /// When `true`, will only bind the relay to http.
-    /// Default is `false`.
-    ///
-    /// When the `--dev` flag is used on the CLI, it will set `http_only` to `true`.
-    #[serde(default = "cfg_defaults::http_only")]
-    http_only: bool,
     /// TLS specific configuration.
     ///
     /// TLS is disabled if not present and the Relay server will serve all services over
@@ -152,7 +148,7 @@ struct Config {
     ///
     /// If no `tls` is set, this will error.
     ///
-    /// Defaults to `true`
+    /// Defaults to `false`
     #[serde(default = "cfg_defaults::enable_quic_addr_discovery")]
     enable_quic_addr_discovery: bool,
     /// Rate limiting configuration.
@@ -191,21 +187,20 @@ impl Config {
 impl Default for Config {
     fn default() -> Self {
         Self {
-            enable_relay: true,
+            enable_relay: cfg_defaults::enable_relay(),
             http_bind_addr: None,
-            http_only: false,
             tls: None,
-            enable_stun: true,
+            enable_stun: cfg_defaults::enable_stun(),
             stun_bind_addr: None,
-            enable_quic_addr_discovery: true,
+            enable_quic_addr_discovery: cfg_defaults::enable_quic_addr_discovery(),
             limits: None,
-            enable_metrics: true,
+            enable_metrics: cfg_defaults::enable_metrics(),
             metrics_bind_addr: None,
         }
     }
 }
 
-/// Defaults for fields from [`Config`].
+/// Defaults for fields from [`Config`] [`TlsConfig`].
 ///
 /// These are the defaults that serde will fill in.  Other defaults depends on each other
 /// and can not immediately be substituted by serde.
@@ -214,16 +209,12 @@ mod cfg_defaults {
         true
     }
 
-    pub(crate) fn http_only() -> bool {
-        false
-    }
-
     pub(crate) fn enable_stun() -> bool {
         true
     }
 
     pub(crate) fn enable_quic_addr_discovery() -> bool {
-        true
+        false
     }
 
     pub(crate) fn enable_metrics() -> bool {
@@ -234,6 +225,10 @@ mod cfg_defaults {
         pub(crate) fn prod_tls() -> bool {
             true
         }
+
+        pub(crate) fn dangerous_http_only() -> bool {
+            false
+        }
     }
 }
 
@@ -245,12 +240,11 @@ struct TlsConfig {
     https_bind_addr: Option<SocketAddr>,
     /// The socket address to bind the QUIC server one.
     ///
-    /// Defaults to the `https_bind_addr` with the port set to [`iroh_base::relay_map::DEFAULT_QUIC_PORT`].
+    /// Defaults to the `https_bind_addr` with the port set to [`iroh_relay::defaults::DEFAULT_RELAY_QUIC_PORT`].
     ///
     /// If `https_bind_addr` is not set, defaults to `http_bind_addr` with the
-    /// port set to [`iroh_base::relay_map::DEFAULT_QUIC_PORT`]
+    /// port set to [`iroh_relay::defaults::DEFAULT_RELAY_QUIC_PORT`]
     quic_bind_addr: Option<SocketAddr>,
-    ///
     /// Certificate hostname when using LetsEncrypt.
     hostname: Option<String>,
     /// Mode for getting a cert.
@@ -292,6 +286,14 @@ struct TlsConfig {
     ///
     /// Used when `cert_mode` is `LetsEncrypt`.
     contact: Option<String>,
+    /// **This field should never be manually set**
+    ///
+    /// When `true`, it will force the relay to ignore binding to https. It is only
+    /// ever used internally when the `--dev` flag is used on the CLI.
+    ///
+    /// Default is `false`.
+    #[serde(default = "cfg_defaults::tls_config::dangerous_http_only")]
+    dangerous_http_only: bool,
 }
 
 impl TlsConfig {
@@ -301,8 +303,9 @@ impl TlsConfig {
     }
 
     fn quic_bind_addr(&self, cfg: &Config) -> SocketAddr {
-        self.quic_bind_addr
-            .unwrap_or_else(|| SocketAddr::new(self.https_bind_addr(cfg).ip(), DEFAULT_QUIC_PORT))
+        self.quic_bind_addr.unwrap_or_else(|| {
+            SocketAddr::new(self.https_bind_addr(cfg).ip(), DEFAULT_RELAY_QUIC_PORT)
+        })
     }
 
     fn cert_dir(&self) -> PathBuf {
@@ -392,15 +395,18 @@ async fn main() -> Result<()> {
 
     let cli = Cli::parse();
     let mut cfg = Config::load(&cli).await?;
-    if cli.dev || cli.dev_quic {
-        cfg.http_only = true;
+    if cfg.enable_quic_addr_discovery && cfg.tls.is_none() {
+        bail!("TLS must be configured in order to spawn a QUIC endpoint");
+    }
+    if cli.dev {
+        // When in `--dev` mode, do not use https, even when tls is configured.
+        if let Some(ref mut tls) = cfg.tls {
+            tls.dangerous_http_only = true;
+        }
         if cfg.http_bind_addr.is_none() {
             cfg.http_bind_addr = Some((Ipv6Addr::UNSPECIFIED, DEV_MODE_HTTP_PORT).into());
         }
     }
-    if cli.dev {
-        cfg.enable_quic_addr_discovery = false;
-    }
     if cfg.tls.is_none() && cfg.enable_quic_addr_discovery {
         bail!("If QUIC address discovery is enabled, TLS must also be configured");
     };
@@ -422,7 +428,6 @@ async fn maybe_load_tls(
     cfg: &Config,
 ) -> Result<Option<relay::TlsConfig<std::io::Error, std::io::Error>>> {
     let Some(ref tls) = cfg.tls else {
-        println!("no tls, returning none");
         return Ok(None);
     };
     let server_config = rustls::ServerConfig::builder_with_provider(std::sync::Arc::new(
@@ -474,6 +479,10 @@ async fn maybe_load_tls(
 
 /// Convert the TOML-loaded config to the [`relay::RelayConfig`] format.
 async fn build_relay_config(cfg: Config) -> Result<relay::ServerConfig<std::io::Error>> {
+    // Don't bind to https, even if tls configuration is available.
+    // Is really only relevant if we are in `--dev` mode & we also have TLS configuration
+    // enabled to use QUIC address discovery locally.
+    let dangerous_http_only = cfg.tls.as_ref().is_some_and(|tls| tls.dangerous_http_only);
     let relay_tls = maybe_load_tls(&cfg).await?;
 
     let mut quic_config = None;
@@ -522,8 +531,8 @@ async fn build_relay_config(cfg: Config) -> Result<relay::ServerConfig<std::io::
 
     let relay_config = relay::RelayConfig {
         http_bind_addr: cfg.http_bind_addr(),
-        // if `http_only` is set, do not pass in any tls configuration
-        tls: relay_tls.and_then(|tls| if cfg.http_only { None } else { Some(tls) }),
+        // if `dangerous_http_only` is set, do not pass in any tls configuration
+        tls: relay_tls.and_then(|tls| if dangerous_http_only { None } else { Some(tls) }),
         limits,
     };
     let stun_config = relay::StunConfig {
@@ -597,8 +606,6 @@ mod tests {
     #[tokio::test]
     async fn test_rate_limit_config() -> TestResult {
         let config = "
-            enable_quic_addr_discovery = false
-
             [limits.client.rx]
             bytes_per_second = 400
             max_burst_bytes = 800
@@ -621,7 +628,7 @@ mod tests {
 
     #[tokio::test]
     async fn test_rate_limit_default() -> TestResult {
-        let config = Config::from_str("enable_quic_addr_discovery = false")?;
+        let config = Config::from_str("")?;
         let relay_config = build_relay_config(config).await?;
 
         let relay = relay_config.relay.expect("no relay config");

iroh-relay/src/quic.rs

@@ -6,7 +6,7 @@ use anyhow::Result;
 use quinn::{crypto::rustls::QuicClientConfig, VarInt};
 
 /// ALPN for our quic addr discovery
-pub const ALPN_QUIC_ADDR_DISC: &[u8] = b"n0/qad";
+pub const ALPN_QUIC_ADDR_DISC: &[u8] = b"/iroh-qad/0";
 /// Endpoint close error code
 pub const QUIC_ADDR_DISC_CLOSE_CODE: VarInt = VarInt::from_u32(1);
 /// Endpoint close reason
@@ -80,7 +80,7 @@ pub(crate) mod server {
             let endpoint = quinn::Endpoint::server(server_config, quic_config.bind_addr)?;
             let bind_addr = endpoint.local_addr()?;
 
-            info!(?bind_addr, "QUIC server bound");
+            info!(?bind_addr, "QUIC server listening on");
 
             let cancel = CancellationToken::new();
             let cancel_accept_loop = cancel.clone();
@@ -97,21 +97,20 @@ pub(crate) mod server {
                             }
                             Some(res) = set.join_next(), if !set.is_empty() => {
                                 if let Err(err) = res {
-                                    // panic if necessary, otherwise, this error has already
-                                    // been logged in `handle_connection`
                                     if err.is_panic() {
-                                        panic!("task panicked: {:#?}", err);
+                                        panic!("task panicked: {err:#?}");
+                                    } else {
+                                        debug!("error accepting incoming connection: {err:#?}");
                                     }
                                 }
                             }
                             res = endpoint.accept() => match res {
                                 Some(conn) => {
                                      debug!("accepting connection");
                                      let remote_addr = conn.remote_address();
-                                     set.spawn(async move {
-                                         handle_connection(conn).await
-                                     }.instrument(info_span!("qad-conn", %remote_addr)));
-                                }
+                                     set.spawn(
+                                         handle_connection(conn).instrument(info_span!("qad-conn", %remote_addr))
+                                     );                                }
                                 None => {
                                     debug!("endpoint closed");
                                     break;
@@ -169,14 +168,10 @@ pub(crate) mod server {
     }
 
     /// Handle the connection from the client.
-    ///
-    /// Any errors that happen during this connection do not need to be handled,
-    /// and will be logged at the debug level in this function.
     async fn handle_connection(incoming: quinn::Incoming) -> Result<()> {
         let connection = match incoming.await {
             Ok(conn) => conn,
             Err(e) => {
-                debug!("error accepting incoming connection: {e:#?}");
                 return Err(e.into());
             }
         };
@@ -189,10 +184,7 @@ pub(crate) mod server {
             {
                 Ok(())
             }
-            _ => {
-                debug!("error closing connection {connection_err:#?}",);
-                Err(connection_err.into())
-            }
+            _ => Err(connection_err.into()),
         }
     }
 }
@@ -265,9 +257,8 @@ impl QuicClient {
             }
         };
         let mut observed_addr = res.expect("checked");
-        // if we've sent an to an ipv4 address, but
-        // received an observed address that is ivp6
-        // then the address is an [IPv4-Mapped IPv6 Addresses](https://doc.rust-lang.org/beta/std/net/struct.Ipv6Addr.html#ipv4-mapped-ipv6-addresses)
+        // if we've sent to an ipv4 address, but received an observed address
+        // that is ivp6 then the address is an [IPv4-Mapped IPv6 Addresses](https://doc.rust-lang.org/beta/std/net/struct.Ipv6Addr.html#ipv4-mapped-ipv6-addresses)
         if server_addr.is_ipv4() && observed_addr.is_ipv6() {
             observed_addr =
                 SocketAddr::new(observed_addr.ip().to_canonical(), observed_addr.port());

iroh-relay/src/server.rs

@@ -134,7 +134,7 @@ pub struct StunConfig {
 pub struct QuicConfig {
     /// The socket address on which the QUIC server should bind.
     ///
-    /// Normally you'd chose port `7842`, see [`crate::defaults::DEFAULT_QUIC_PORT`].
+    /// Normally you'd chose port `7842`, see [`crate::defaults::DEFAULT_RELAY_QUIC_PORT`].
     pub bind_addr: SocketAddr,
     /// The TLS server configuration for the QUIC server.
     ///
@@ -263,7 +263,7 @@ impl Server {
                 match UdpSocket::bind(stun.bind_addr).await {
                     Ok(sock) => {
                         let addr = sock.local_addr()?;
-                        info!("STUN server bound on {addr}");
+                        info!("STUN server listening on {addr}");
                         tasks.spawn(
                             server_stun_listener(sock).instrument(info_span!("stun-server", %addr)),
                         );

iroh/src/defaults.rs

@@ -5,7 +5,7 @@ use iroh_base::relay_map::QuicConfig;
 /// for QUIC address discovery
 ///
 /// The port is "QUIC" typed on a phone keypad.
-pub use iroh_base::relay_map::DEFAULT_QUIC_PORT as DEFAULT_RELAY_QUIC_PORT;
+pub use iroh_base::relay_map::DEFAULT_RELAY_QUIC_PORT;
 /// The default STUN port used by the Relay server.
 ///
 /// The STUN port as defined by [RFC

iroh/src/endpoint.rs

@@ -1636,7 +1636,7 @@ mod tests {
                         .await
                         .unwrap();
                     let eps = ep.bound_sockets();
-                    info!(me = %ep.node_id().fmt_short(), ipv4=%eps.0, ipv6=?eps.1, "server bound");
+                    info!(me = %ep.node_id().fmt_short(), ipv4=%eps.0, ipv6=?eps.1, "server listening on");
                     for i in 0..n_clients {
                         let now = Instant::now();
                         println!("[server] round {}", i + 1);