pr comments: safety, receive multiple nst, return status refactor

boring/src/ssl/callbacks.rs

@@ -299,7 +299,7 @@ where
         .ex_data::<F>(SslContext::cached_ex_index::<F>())
         .expect("expected session resumption callback");
 
-    // Safety: the callback guarantees that key_name is 16 bytes
+    // SAFETY: the callback guarantees that key_name is 16 bytes
     let key_name =
         unsafe { slice::from_raw_parts_mut(key_name, ffi::SSL_TICKET_KEY_NAME_LEN as usize) };
     let key_name = <&mut [u8; 16]>::try_from(key_name).expect("boring provides a 16-byte key name");

boring/src/ssl/mod.rs

@@ -810,12 +810,14 @@ pub enum TicketKeyCallbackResult {
     /// Abort the handshake.
     Error,
 
-    /// The peer supplied session ticket was not recognized. Continue with a full handshake.
+    /// Continue with a full handshake.
+    ///
+    /// The peer supplied session ticket was not recognized.
     ///
     /// # Note
     ///
     /// This is a decryption specific status code.
-    DecryptTicketUnrecognized,
+    Noop,
 
     /// Resumption callback was successful.
     ///
@@ -841,7 +843,7 @@ impl From<TicketKeyCallbackResult> for c_int {
     fn from(value: TicketKeyCallbackResult) -> Self {
         match value {
             TicketKeyCallbackResult::Error => -1,
-            TicketKeyCallbackResult::DecryptTicketUnrecognized => 0,
+            TicketKeyCallbackResult::Noop => 0,
             TicketKeyCallbackResult::Success => 1,
             TicketKeyCallbackResult::DecryptSuccessRenew => 2,
         }

boring/src/ssl/test/session_resumption.rs

@@ -15,6 +15,7 @@ static CUSTOM_DECRYPTION_CALLED_BACK: AtomicU8 = AtomicU8::new(0);
 #[test]
 fn resume_session() {
     static SESSION_TICKET: OnceLock<Vec<u8>> = OnceLock::new();
+    static NST_RECIEVED_COUNT: AtomicU8 = AtomicU8::new(0);
 
     let mut server = Server::builder();
     server.expected_connections_count(2);
@@ -25,12 +26,17 @@ fn resume_session() {
         .ctx()
         .set_session_cache_mode(SslSessionCacheMode::CLIENT);
     client.ctx().set_new_session_callback(|_, session| {
-        let _can_receive_multiple_tickets = SESSION_TICKET.set(session.to_der().unwrap());
+        NST_RECIEVED_COUNT.fetch_add(1, Ordering::SeqCst);
+        // The server sends multiple session tickets but we only care to retrieve one.
+        if SESSION_TICKET.get().is_none() {
+            SESSION_TICKET.set(session.to_der().unwrap()).unwrap();
+        }
     });
     let ssl_stream = client.connect();
 
     assert!(!ssl_stream.ssl().session_reused());
     assert!(SESSION_TICKET.get().is_some());
+    assert_eq!(NST_RECIEVED_COUNT.load(Ordering::SeqCst), 2);
 
     // Retrieve the session ticket
     let session_ticket = SslSession::from_der(SESSION_TICKET.get().unwrap()).unwrap();
@@ -47,6 +53,7 @@ fn resume_session() {
 #[test]
 fn custom_callback() {
     static SESSION_TICKET: OnceLock<Vec<u8>> = OnceLock::new();
+    static NST_RECIEVED_COUNT: AtomicU8 = AtomicU8::new(0);
 
     let mut server = Server::builder();
     server.expected_connections_count(2);
@@ -60,7 +67,11 @@ fn custom_callback() {
         .ctx()
         .set_session_cache_mode(SslSessionCacheMode::CLIENT);
     client.ctx().set_new_session_callback(|_, session| {
-        let _can_receive_multiple_tickets = SESSION_TICKET.set(session.to_der().unwrap());
+        NST_RECIEVED_COUNT.fetch_add(1, Ordering::SeqCst);
+        // The server sends multiple session tickets but we only care to retrieve one.
+        if SESSION_TICKET.get().is_none() {
+            SESSION_TICKET.set(session.to_der().unwrap()).unwrap();
+        }
     });
     let ssl_stream = client.connect();
 
@@ -68,6 +79,7 @@ fn custom_callback() {
     assert!(SESSION_TICKET.get().is_some());
     assert_eq!(CUSTOM_ENCRYPTION_CALLED_BACK.load(Ordering::SeqCst), 2);
     assert_eq!(CUSTOM_DECRYPTION_CALLED_BACK.load(Ordering::SeqCst), 0);
+    assert_eq!(NST_RECIEVED_COUNT.load(Ordering::SeqCst), 2);
 
     // Retrieve the session ticket
     let session_ticket = SslSession::from_der(SESSION_TICKET.get().unwrap()).unwrap();