feat(s2n-quic-tls, s2n-quic-rustls): emit server_name and application_protocol events sooner in the handshake

quic/s2n-quic-core/src/crypto/tls.rs

@@ -19,8 +19,6 @@ pub mod testing;
 pub struct ApplicationParameters<'a> {
     /// The negotiated Application Layer Protocol
     pub application_protocol: &'a [u8],
-    /// Server Name Indication
-    pub server_name: Option<crate::application::ServerName>,
     /// Encoded transport parameters
     pub transport_parameters: &'a [u8],
 }
@@ -53,6 +51,11 @@ pub trait Context<Crypto: CryptoSuite> {
         application_parameters: ApplicationParameters,
     ) -> Result<(), transport::Error>;
 
+    fn on_server_name(
+        &mut self,
+        server_name: Option<crate::application::ServerName>,
+    ) -> Result<(), transport::Error>;
+
     //= https://www.rfc-editor.org/rfc/rfc9001#section-4.1.1
     //# The TLS handshake is considered complete when the
     //# TLS stack has reported that the handshake is complete.  This happens

quic/s2n-quic-core/src/crypto/tls/testing.rs

@@ -323,7 +323,6 @@ impl<C: CryptoSuite> Context<C> {
 
     fn on_application_params(&mut self, params: tls::ApplicationParameters) {
         self.application_protocol = Some(Bytes::copy_from_slice(params.application_protocol));
-        self.server_name = params.server_name.map(|sni| sni.into_bytes());
         self.transport_parameters = Some(Bytes::copy_from_slice(params.transport_parameters));
     }
 
@@ -507,6 +506,20 @@ impl<C: CryptoSuite> tls::Context<C> for Context<C> {
         Ok(())
     }
 
+    fn on_server_name(
+        &mut self,
+        server_name: Option<crate::application::ServerName>,
+    ) -> Result<(), transport::Error> {
+        assert!(
+            self.application.crypto.is_none(),
+            "1-rtt keys emitted before server name parsing"
+        );
+        self.log("server name");
+        self.server_name = server_name.map(|sni| sni.into_bytes());
+
+        Ok(())
+    }
+
     fn on_handshake_complete(&mut self) -> Result<(), transport::Error> {
         assert!(
             !self.handshake_complete,

quic/s2n-quic-rustls/src/session.rs

@@ -100,11 +100,8 @@ impl Session {
                 CryptoError::MISSING_EXTENSION.with_reason("Missing QUIC transport parameters")
             })?;
 
-        let server_name = self.server_name();
-
         Ok(tls::ApplicationParameters {
             application_protocol,
-            server_name,
             transport_parameters,
         })
     }
@@ -257,6 +254,7 @@ impl tls::Session for Session {
                             let (key, header_key) = OneRttKey::new(keys, next, cipher_suite);
                             let application_parameters = self.application_parameters()?;
 
+                            context.on_server_name(self.server_name())?;
                             context.on_one_rtt_keys(key, header_key, application_parameters)?;
 
                             // Transition the tx_phase to Application

quic/s2n-quic-tls/src/callback.rs

@@ -215,6 +215,8 @@ where
                             OneRttKey::new(self.endpoint, aead_algo, pair).expect("invalid cipher");
 
                         let params = unsafe {
+                            self.context.on_server_name(get_server_name(conn))?;
+
                             // Safety: conn needs to outlive params
                             get_application_params(conn)?
                         };
@@ -445,11 +447,8 @@ unsafe fn get_application_params<'a>(
     let transport_parameters =
         get_transport_parameters(connection).ok_or(CryptoError::MISSING_EXTENSION)?;
 
-    let server_name = get_server_name(connection);
-
     Ok(tls::ApplicationParameters {
         application_protocol,
-        server_name,
         transport_parameters,
     })
 }

quic/s2n-quic-transport/src/connection/connection_impl.rs

@@ -1717,8 +1717,7 @@ impl<Config: endpoint::Config> connection::Trait for ConnectionImpl<Config> {
     }
 
     fn server_name(&self) -> Option<ServerName> {
-        // TODO move SNI to connection
-        self.space_manager.application()?.server_name.clone()
+        self.space_manager.server_name.clone()
     }
 
     fn application_protocol(&self) -> Bytes {

quic/s2n-quic-transport/src/space/application.rs

@@ -20,7 +20,6 @@ use core::{convert::TryInto, fmt, marker::PhantomData};
 use once_cell::sync::OnceCell;
 use s2n_codec::EncoderBuffer;
 use s2n_quic_core::{
-    application::ServerName,
     crypto::{application::KeySet, limited, tls, CryptoSuite},
     event::{self, ConnectionPublisher as _, IntoEvent},
     frame::{
@@ -70,7 +69,6 @@ pub struct ApplicationSpace<Config: endpoint::Config> {
     //# another mechanism is used for agreeing on an application protocol,
     //# endpoints MUST use ALPN for this purpose.
     pub application_protocol: Bytes,
-    pub server_name: Option<ServerName>,
     ping: flag::Ping,
     keep_alive: KeepAlive,
     processed_packet_numbers: SlidingWindow,
@@ -85,7 +83,6 @@ impl<Config: endpoint::Config> fmt::Debug for ApplicationSpace<Config> {
             .field("ping", &self.ping)
             .field("processed_packet_numbers", &self.processed_packet_numbers)
             .field("recovery_manager", &self.recovery_manager)
-            .field("server_name", &self.server_name)
             .field("stream_manager", &self.stream_manager)
             .field("tx_packet_numbers", &self.tx_packet_numbers)
             .finish()
@@ -101,7 +98,6 @@ impl<Config: endpoint::Config> ApplicationSpace<Config> {
         stream_manager: AbstractStreamManager<Config::Stream>,
         ack_manager: AckManager,
         keep_alive: KeepAlive,
-        server_name: Option<ServerName>,
         application_protocol: Bytes,
         max_mtu: MaxMtu,
     ) -> Self {
@@ -114,7 +110,6 @@ impl<Config: endpoint::Config> ApplicationSpace<Config> {
             stream_manager,
             key_set,
             header_key,
-            server_name,
             application_protocol,
             ping: flag::Ping::default(),
             keep_alive,

quic/s2n-quic-transport/src/space/mod.rs

@@ -16,6 +16,7 @@ use core::{
 use s2n_codec::DecoderBufferMut;
 use s2n_quic_core::{
     ack,
+    application::ServerName,
     connection::{limits::Limits, InitialId, PeerId},
     crypto::{tls, tls::Session, CryptoSuite, Key},
     event::{self, IntoEvent},
@@ -63,6 +64,8 @@ pub struct PacketSpaceManager<Config: endpoint::Config> {
     zero_rtt_crypto:
         Option<Box<<<Config::TLSEndpoint as tls::Endpoint>::Session as CryptoSuite>::ZeroRttKey>>,
     handshake_status: HandshakeStatus,
+    /// Server Name Indication
+    pub server_name: Option<ServerName>,
 }
 
 impl<Config: endpoint::Config> fmt::Debug for PacketSpaceManager<Config> {
@@ -155,6 +158,7 @@ impl<Config: endpoint::Config> PacketSpaceManager<Config> {
             application: None,
             zero_rtt_crypto: None,
             handshake_status: HandshakeStatus::default(),
+            server_name: None,
         }
     }
 
@@ -203,6 +207,7 @@ impl<Config: endpoint::Config> PacketSpaceManager<Config> {
                 handshake_status: &mut self.handshake_status,
                 local_id_registry,
                 limits,
+                server_name: &mut self.server_name,
                 waker,
                 publisher,
             };

quic/s2n-quic-transport/src/space/session_context.rs

@@ -15,6 +15,7 @@ use core::{ops::Not, task::Waker};
 use s2n_codec::{DecoderBuffer, DecoderValue};
 use s2n_quic_core::{
     ack,
+    application::ServerName,
     connection::{InitialId, PeerId},
     crypto::{tls, CryptoSuite, Key},
     ct::ConstantTimeEq,
@@ -45,6 +46,7 @@ pub struct SessionContext<'a, Config: endpoint::Config, Pub: event::ConnectionPu
     pub handshake_status: &'a mut HandshakeStatus,
     pub local_id_registry: &'a mut connection::LocalIdRegistry,
     pub limits: &'a mut Limits,
+    pub server_name: &'a mut Option<ServerName>,
     pub waker: &'a Waker,
     pub publisher: &'a mut Pub,
 }
@@ -364,7 +366,6 @@ impl<'a, Config: endpoint::Config, Pub: event::ConnectionPublisher>
 
         // TODO use interning for these values
         // issue: https://github.com/aws/s2n-quic/issues/248
-        let server_name = application_parameters.server_name;
         let application_protocol =
             Bytes::copy_from_slice(application_parameters.application_protocol);
 
@@ -373,12 +374,6 @@ impl<'a, Config: endpoint::Config, Pub: event::ConnectionPublisher>
                 chosen_application_protocol: &application_protocol,
             },
         );
-        if let Some(chosen_server_name) = &server_name {
-            self.publisher
-                .on_server_name_information(event::builder::ServerNameInformation {
-                    chosen_server_name,
-                });
-        };
 
         let cipher_suite = key.cipher_suite().into_event();
         let max_mtu = self.path_manager.max_mtu();
@@ -389,7 +384,6 @@ impl<'a, Config: endpoint::Config, Pub: event::ConnectionPublisher>
             stream_manager,
             ack_manager,
             keep_alive,
-            server_name,
             application_protocol,
             max_mtu,
         )));
@@ -401,6 +395,20 @@ impl<'a, Config: endpoint::Config, Pub: event::ConnectionPublisher>
         Ok(())
     }
 
+    fn on_server_name(&mut self, server_name: Option<ServerName>) -> Result<(), transport::Error> {
+        // TODO use interning for these values
+        // issue: https://github.com/aws/s2n-quic/issues/248
+        if let Some(server_name) = &server_name {
+            self.publisher
+                .on_server_name_information(event::builder::ServerNameInformation {
+                    chosen_server_name: server_name,
+                });
+        }
+        *self.server_name = server_name;
+
+        Ok(())
+    }
+
     fn on_handshake_complete(&mut self) -> Result<(), transport::Error> {
         // After the handshake is complete, the handshake crypto stream should be completely
         // finished