Merge 3b36544 into a62a2bd

Author: matheus23

Cargo.lock

@@ -9,6 +9,7 @@ allow = [
     "BSD-2-Clause",
     "BSD-3-Clause",
     "BSL-1.0",                        # BOSL license
+    "CDLA-Permissive-2.0",            # Community Data License Agreement Permissive 2.0 due to webpki-root-certs
     "ISC",
     "MIT",
     "Zlib",

deny.toml

@@ -70,7 +70,7 @@ async fn start_accept_side() -> Result<Router> {
     let endpoint = Endpoint::builder().discovery_n0().bind().await?;
 
     // Build our protocol handler and add our protocol, identified by its ALPN, and spawn the node.
-    let router = Router::builder(endpoint).accept(ALPN, Echo).spawn().await?;
+    let router = Router::builder(endpoint).accept(ALPN, Echo).spawn();
 
     Ok(router)
 }

iroh/examples/echo.rs

@@ -85,7 +85,7 @@ async fn main() -> Result<()> {
     let builder = Router::builder(endpoint);
 
     // Add our protocol, identified by our ALPN, to the node, and spawn the node.
-    let router = builder.accept(ALPN, proto.clone()).spawn().await?;
+    let router = builder.accept(ALPN, proto.clone()).spawn();
 
     match args.command {
         Command::Listen { text } => {

iroh/examples/search.rs

@@ -22,7 +22,6 @@ use std::{
 };
 
 use anyhow::{bail, Context, Result};
-use data_encoding::BASE32_DNSSEC;
 use ed25519_dalek::{pkcs8::DecodePublicKey, VerifyingKey};
 use iroh_base::{NodeAddr, NodeId, RelayUrl, SecretKey};
 use iroh_relay::RelayMap;
@@ -78,16 +77,6 @@ pub use super::magicsock::{
 /// is still no connection the configured [`Discovery`] will be used however.
 const DISCOVERY_WAIT_PERIOD: Duration = Duration::from_millis(500);
 
-/// Maximum amount of TLS tickets we will cache (by default) for 0-RTT connection
-/// establishment.
-///
-/// 8 tickets per remote endpoint, 32 different endpoints would max out the required storage:
-/// ~200 bytes per session + certificates (which are ~387 bytes)
-/// So 8 * 32 * (200 + 387) = 150.272 bytes, assuming pointers to certificates
-/// are never aliased pointers (they're Arc'ed).
-/// I think 150KB is an acceptable default upper limit for such a cache.
-const MAX_TLS_TICKETS: usize = 8 * 32;
-
 type DiscoveryBuilder = Box<dyn FnOnce(&SecretKey) -> Option<Box<dyn Discovery>> + Send + Sync>;
 
 /// Defines the mode of path selection for all traffic flowing through
@@ -175,9 +164,8 @@ impl Builder {
             .unwrap_or_else(|| SecretKey::generate(rand::rngs::OsRng));
         let static_config = StaticConfig {
             transport_config: Arc::new(self.transport_config),
-            tls_auth: self.tls_auth,
+            tls_config: tls::TlsConfig::new(self.tls_auth, secret_key.clone()),
             keylog: self.keylog,
-            secret_key: secret_key.clone(),
         };
         #[cfg(not(wasm_browser))]
         let dns_resolver = self.dns_resolver.unwrap_or_default();
@@ -191,7 +179,7 @@ impl Builder {
             1 => Some(discovery.into_iter().next().expect("checked length")),
             _ => Some(Box::new(ConcurrentDiscovery::from_services(discovery))),
         };
-        let server_config = static_config.create_server_config(self.alpn_protocols)?;
+        let server_config = static_config.create_server_config(self.alpn_protocols);
 
         let metrics = EndpointMetrics::default();
 
@@ -544,22 +532,21 @@ impl Builder {
 /// Configuration for a [`quinn::Endpoint`] that cannot be changed at runtime.
 #[derive(Debug)]
 struct StaticConfig {
-    tls_auth: tls::Authentication,
-    secret_key: SecretKey,
+    tls_config: tls::TlsConfig,
     transport_config: Arc<quinn::TransportConfig>,
     keylog: bool,
 }
 
 impl StaticConfig {
     /// Create a [`quinn::ServerConfig`] with the specified ALPN protocols.
-    fn create_server_config(&self, alpn_protocols: Vec<Vec<u8>>) -> Result<ServerConfig> {
-        let quic_server_config =
-            self.tls_auth
-                .make_server_config(&self.secret_key, alpn_protocols, self.keylog)?;
+    fn create_server_config(&self, alpn_protocols: Vec<Vec<u8>>) -> ServerConfig {
+        let quic_server_config = self
+            .tls_config
+            .make_server_config(alpn_protocols, self.keylog);
         let mut server_config = ServerConfig::with_crypto(Arc::new(quic_server_config));
         server_config.transport_config(self.transport_config.clone());
 
-        Ok(server_config)
+        server_config
     }
 }
 
@@ -596,8 +583,6 @@ pub struct Endpoint {
     rtt_actor: Arc<rtt_actor::RttHandle>,
     /// Configuration structs for quinn, holds the transport config, certificate setup, secret key etc.
     static_config: Arc<StaticConfig>,
-    /// Cache for TLS session keys we receive.
-    session_store: Arc<dyn rustls::client::ClientSessionStore>,
 }
 
 impl Endpoint {
@@ -616,7 +601,7 @@ impl Endpoint {
     ///
     /// This is for internal use, the public interface is the [`Builder`] obtained from
     /// [Self::builder]. See the methods on the builder for documentation of the parameters.
-    #[instrument("ep", skip_all, fields(me = %static_config.secret_key.public().fmt_short()))]
+    #[instrument("ep", skip_all, fields(me = %static_config.tls_config.secret_key.public().fmt_short()))]
     async fn bind(static_config: StaticConfig, msock_opts: magicsock::Options) -> Result<Self> {
         let msock = magicsock::MagicSock::spawn(msock_opts).await?;
         trace!("created magicsock");
@@ -626,9 +611,6 @@ impl Endpoint {
             msock: msock.clone(),
             rtt_actor: Arc::new(rtt_actor::RttHandle::new(msock.metrics.magicsock.clone())),
             static_config: Arc::new(static_config),
-            session_store: Arc::new(rustls::client::ClientSessionMemoryCache::new(
-                MAX_TLS_TICKETS,
-            )),
         };
         Ok(ep)
     }
@@ -637,10 +619,9 @@ impl Endpoint {
     ///
     /// This will only affect new incoming connections.
     /// Note that this *overrides* the current list of ALPNs.
-    pub fn set_alpns(&self, alpns: Vec<Vec<u8>>) -> Result<()> {
-        let server_config = self.static_config.create_server_config(alpns)?;
+    pub fn set_alpns(&self, alpns: Vec<Vec<u8>>) {
+        let server_config = self.static_config.create_server_config(alpns);
         self.msock.endpoint().set_server_config(Some(server_config));
-        Ok(())
     }
 
     // # Methods for establishing connectivity.
@@ -758,28 +739,16 @@ impl Endpoint {
         let client_config = {
             let mut alpn_protocols = vec![alpn.to_vec()];
             alpn_protocols.extend(options.additional_alpns);
-            let quic_client_config = self.static_config.tls_auth.make_client_config(
-                &self.static_config.secret_key,
-                node_id,
-                alpn_protocols,
-                Some(self.session_store.clone()),
-                self.static_config.keylog,
-            )?;
+            let quic_client_config = self
+                .static_config
+                .tls_config
+                .make_client_config(alpn_protocols, self.static_config.keylog);
             let mut client_config = quinn::ClientConfig::new(Arc::new(quic_client_config));
             client_config.transport_config(transport_config);
             client_config
         };
 
-        // We used to use a constant "localhost" for this - however, that would put all of
-        // the TLS session tickets we receive into the same bucket in the TLS session ticket cache.
-        // So we choose something that'd dependent on the NodeId.
-        // We cannot use hex to encode the NodeId, as that'd encode to 64 characters, but we only
-        // have 63 maximum per DNS subdomain. Base32 is the next best alternative.
-        // We use the `.invalid` TLD, as that's specified (in RFC 2606) to never actually resolve
-        // "for real", unlike `.localhost` which is allowed to resolve to `127.0.0.1`.
-        // We also add "iroh" as a subdomain, although those 5 bytes might not be necessary.
-        // We *could* decide to remove that indicator in the future likely without breakage.
-        let server_name = &format!("{}.iroh.invalid", BASE32_DNSSEC.encode(node_id.as_bytes()));
+        let server_name = &tls::name::encode(node_id);
         let connect = self.msock.endpoint().connect_with(
             client_config,
             mapped_addr.private_socket_addr(),
@@ -883,15 +852,15 @@ impl Endpoint {
 
     /// Returns the secret_key of this endpoint.
     pub fn secret_key(&self) -> &SecretKey {
-        &self.static_config.secret_key
+        &self.static_config.tls_config.secret_key
     }
 
     /// Returns the node id of this endpoint.
     ///
     /// This ID is the unique addressing information of this node and other peers must know
     /// it to be able to connect to this node.
     pub fn node_id(&self) -> NodeId {
-        self.static_config.secret_key.public()
+        self.static_config.tls_config.secret_key.public()
     }
 
     /// Returns the current [`NodeAddr`] for this endpoint.
@@ -1571,7 +1540,7 @@ impl Future for IncomingFuture {
             Poll::Ready(Ok(inner)) => {
                 let conn = Connection {
                     inner,
-                    tls_auth: this.ep.static_config.tls_auth,
+                    tls_auth: this.ep.static_config.tls_config.auth,
                 };
                 try_send_rtt_msg(&conn, this.ep, None);
                 Poll::Ready(Ok(conn))
@@ -1644,7 +1613,7 @@ impl Connecting {
             Ok((inner, zrtt_accepted)) => {
                 let conn = Connection {
                     inner,
-                    tls_auth: self.ep.static_config.tls_auth,
+                    tls_auth: self.ep.static_config.tls_config.auth,
                 };
                 let zrtt_accepted = ZeroRttAccepted {
                     inner: zrtt_accepted,
@@ -1699,7 +1668,7 @@ impl Future for Connecting {
             Poll::Ready(Ok(inner)) => {
                 let conn = Connection {
                     inner,
-                    tls_auth: this.ep.static_config.tls_auth,
+                    tls_auth: this.ep.static_config.tls_config.auth,
                 };
                 try_send_rtt_msg(&conn, this.ep, *this.remote_node_id);
                 Poll::Ready(Ok(conn))

iroh/src/endpoint.rs

@@ -3460,9 +3460,8 @@ mod tests {
         secret_key: &SecretKey,
         tls_auth: crate::tls::Authentication,
     ) -> ServerConfig {
-        let quic_server_config = tls_auth
-            .make_server_config(secret_key, vec![], false)
-            .expect("should generate valid config");
+        let quic_server_config = crate::tls::TlsConfig::new(tls_auth, secret_key.clone())
+            .make_server_config(vec![], false);
         let mut server_config = ServerConfig::with_crypto(Arc::new(quic_server_config));
         server_config.transport_config(Arc::new(quinn::TransportConfig::default()));
         server_config
@@ -4042,8 +4041,8 @@ mod tests {
         secret_key: SecretKey,
         tls_auth: tls::Authentication,
     ) -> anyhow::Result<Handle> {
-        let quic_server_config =
-            tls_auth.make_server_config(&secret_key, vec![ALPN.to_vec()], true)?;
+        let quic_server_config = tls::TlsConfig::new(tls_auth, secret_key.clone())
+            .make_server_config(vec![ALPN.to_vec()], true);
         let mut server_config = ServerConfig::with_crypto(Arc::new(quic_server_config));
         server_config.transport_config(Arc::new(quinn::TransportConfig::default()));
 
@@ -4110,13 +4109,13 @@ mod tests {
     ) -> Result<quinn::Connection> {
         let alpns = vec![ALPN.to_vec()];
         let quic_client_config =
-            tls_auth.make_client_config(&ep_secret_key, node_id, alpns, None, true)?;
+            tls::TlsConfig::new(tls_auth, ep_secret_key.clone()).make_client_config(alpns, 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.private_socket_addr(),
-            "localhost",
+            &tls::name::encode(node_id),
         )?;
         let connection = connect.await?;
         Ok(connection)

iroh/src/magicsock.rs

@@ -12,8 +12,7 @@
 //!
 //! let router = Router::builder(endpoint)
 //!     .accept(b"/my/alpn", Echo)
-//!     .spawn()
-//!     .await?;
+//!     .spawn();
 //! # Ok(())
 //! # }
 //!
@@ -78,8 +77,7 @@ use crate::{
 ///
 /// let router = Router::builder(endpoint)
 ///     // .accept(&ALPN, <something>)
-///     .spawn()
-///     .await?;
+///     .spawn();
 ///
 /// // wait until the user wants to
 /// tokio::signal::ctrl_c().await?;
@@ -255,7 +253,7 @@ impl RouterBuilder {
     }
 
     /// Spawns an accept loop and returns a handle to it encapsulated as the [`Router`].
-    pub async fn spawn(self) -> Result<Router> {
+    pub fn spawn(self) -> Router {
         // Update the endpoint with our alpns.
         let alpns = self
             .protocols
@@ -264,10 +262,7 @@ impl RouterBuilder {
             .collect::<Vec<_>>();
 
         let protocols = Arc::new(self.protocols);
-        if let Err(err) = self.endpoint.set_alpns(alpns) {
-            shutdown(&self.endpoint, protocols.clone()).await;
-            return Err(err);
-        }
+        self.endpoint.set_alpns(alpns);
 
         let mut join_set = JoinSet::new();
         let endpoint = self.endpoint.clone();
@@ -333,11 +328,11 @@ impl RouterBuilder {
         let task = task::spawn(run_loop_fut);
         let task = AbortOnDropHandle::new(task);
 
-        Ok(Router {
+        Router {
             endpoint: self.endpoint,
             task: Arc::new(Mutex::new(Some(task))),
             cancel_token: cancel,
-        })
+        }
     }
 }
 
@@ -441,7 +436,7 @@ mod tests {
     #[tokio::test]
     async fn test_shutdown() -> Result<()> {
         let endpoint = Endpoint::builder().bind().await?;
-        let router = Router::builder(endpoint.clone()).spawn().await?;
+        let router = Router::builder(endpoint.clone()).spawn();
 
         assert!(!router.is_shutdown());
         assert!(!endpoint.is_closed());
@@ -481,10 +476,7 @@ mod tests {
         let e1 = Endpoint::builder().bind().await?;
         // deny all access
         let proto = AccessLimit::new(Echo, |_node_id| false);
-        let r1 = Router::builder(e1.clone())
-            .accept(ECHO_ALPN, proto)
-            .spawn()
-            .await?;
+        let r1 = Router::builder(e1.clone()).accept(ECHO_ALPN, proto).spawn();
 
         let addr1 = r1.endpoint().node_addr().await?;