Refactor the time handling in neqo-crypto

Removes a couple of TODOs, not that difficult or exciting.

neqo-crypto/src/agent.rs

@@ -17,11 +17,11 @@ use crate::prio;
 use crate::replay::AntiReplay;
 use crate::secrets::SecretHolder;
 use crate::ssl::{self, PRBool};
-use crate::time::{PRTime, Time};
+use crate::time::TimeHolder;
 
 use neqo_common::{matches, qdebug, qinfo, qtrace, qwarn};
 use std::cell::RefCell;
-use std::convert::{TryFrom, TryInto};
+use std::convert::TryFrom;
 use std::ffi::CString;
 use std::mem::{self, MaybeUninit};
 use std::ops::{Deref, DerefMut};
@@ -216,7 +216,7 @@ pub struct SecretAgent {
     /// Records any fatal alert that is sent by the stack.
     alert: Pin<Box<Option<Alert>>>,
     /// The current time.
-    now: Pin<Box<PRTime>>,
+    now: TimeHolder,
 
     extension_handlers: Vec<ExtensionTracker>,
     inf: Option<SecretAgentInfo>,
@@ -238,7 +238,7 @@ impl SecretAgent {
 
             auth_required: Box::pin(false),
             alert: Box::pin(None),
-            now: Box::pin(0),
+            now: TimeHolder::default(),
 
             extension_handlers: Vec::new(),
             inf: None,
@@ -309,12 +309,6 @@ impl SecretAgent {
         }
     }
 
-    // TODO(mt) move to time.rs.
-    unsafe extern "C" fn time_func(arg: *mut c_void) -> PRTime {
-        let p = arg as *mut PRTime as *const PRTime;
-        *p.as_ref().unwrap()
-    }
-
     // Ready this for connecting.
     fn ready(&mut self, is_server: bool) -> Res<()> {
         secstatus_to_res(unsafe {
@@ -333,9 +327,7 @@ impl SecretAgent {
             )
         })?;
 
-        // TODO(mt) move to time.rs so we can remove PRTime definition from nss_ssl bindings.
-        unsafe { ssl::SSL_SetTimeFunc(self.fd, Some(Self::time_func), as_c_void(&mut self.now)) }?;
-
+        self.now.bind(self.fd)?;
         self.configure()?;
         secstatus_to_res(unsafe { ssl::SSL_ResetHandshake(self.fd, is_server as ssl::PRBool) })
     }
@@ -584,7 +576,7 @@ impl SecretAgent {
     /// # Errors
     /// When the handshake fails this returns an error.
     pub fn handshake(&mut self, now: Instant, input: &[u8]) -> Res<Vec<u8>> {
-        *self.now = Time::from(now).try_into()?;
+        self.now.set(now)?;
         self.set_raw(false)?;
 
         let rv = {
@@ -642,7 +634,7 @@ impl SecretAgent {
     /// # Errors
     /// When the handshake fails this returns an error.
     pub fn handshake_raw(&mut self, now: Instant, input: Option<Record>) -> Res<RecordList> {
-        *self.now = Time::from(now).try_into()?;
+        self.now.set(now)?;
         let mut records = self.setup_raw()?;
 
         // Fire off any authentication we might need to complete.
@@ -954,7 +946,7 @@ impl Server {
     /// # Errors
     /// If NSS is unable to send a ticket, or if this agent is incorrectly configured.
     pub fn send_ticket(&mut self, now: Instant, extra: &[u8]) -> Res<RecordList> {
-        *self.agent.now = Time::from(now).try_into()?;
+        self.agent.now.set(now)?;
         let records = self.setup_raw()?;
 
         unsafe {

neqo-crypto/src/ssl.rs

@@ -107,11 +107,6 @@ experimental_api!(SSL_SetResumptionTokenCallback(
     cb: SSLResumptionTokenCallback,
     arg: *mut c_void,
 ));
-experimental_api!(SSL_SetTimeFunc(
-    fd: *mut PRFileDesc,
-    cb: SSLTimeFunc,
-    arg: *mut c_void,
-));
 
 #[cfg(test)]
 mod tests {

neqo-crypto/src/time.rs

@@ -4,16 +4,26 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
+use crate::agentio::as_c_void;
 use crate::err::{Error, Res};
-
 use crate::once::OnceResult;
+use crate::ssl::{PRFileDesc, SSLTimeFunc};
 
+use std::boxed::Box;
 use std::convert::{TryFrom, TryInto};
 use std::ops::Deref;
+use std::os::raw::c_void;
+use std::pin::Pin;
 use std::time::{Duration, Instant};
 
 include!(concat!(env!("OUT_DIR"), "/nspr_time.rs"));
 
+experimental_api!(SSL_SetTimeFunc(
+    fd: *mut PRFileDesc,
+    cb: SSLTimeFunc,
+    arg: *mut c_void,
+));
+
 /// This struct holds the zero time used for converting between `Instant` and `PRTime`.
 #[derive(Debug)]
 struct TimeZero {
@@ -164,6 +174,34 @@ impl TryInto<PRTime> for Interval {
     }
 }
 
+/// `TimeHolder` maintains a `PRTime` value in a form that is accessible to the TLS stack.
+#[derive(Debug)]
+pub struct TimeHolder {
+    t: Pin<Box<PRTime>>,
+}
+
+impl TimeHolder {
+    unsafe extern "C" fn time_func(arg: *mut c_void) -> PRTime {
+        let p = arg as *mut PRTime as *const PRTime;
+        *p.as_ref().unwrap()
+    }
+
+    pub fn bind(&mut self, fd: *mut PRFileDesc) -> Res<()> {
+        unsafe { SSL_SetTimeFunc(fd, Some(Self::time_func), as_c_void(&mut self.t)) }
+    }
+
+    pub fn set(&mut self, t: Instant) -> Res<()> {
+        *self.t = Time::from(t).try_into()?;
+        Ok(())
+    }
+}
+
+impl Default for TimeHolder {
+    fn default() -> Self {
+        TimeHolder { t: Box::pin(0) }
+    }
+}
+
 #[cfg(test)]
 mod test {
     use super::*;