Suggestions from @martinthomson

neqo-crypto/src/constants.rs

@@ -8,14 +8,14 @@
 
 use enum_map::Enum;
 
-use crate::ssl;
+use crate::{ssl, Error};
 
 // Ideally all of these would be enums, but size matters and we need to allow
 // for values outside of those that are defined here.
 
 pub type Alert = u8;
 
-#[derive(Default, Debug, Enum)]
+#[derive(Default, Debug, Enum, Clone, Copy, PartialEq, Eq)]
 pub enum Epoch {
     // TLS doesn't really have an "initial" concept that maps to QUIC so directly,
     // but this should be clear enough.
@@ -28,15 +28,15 @@ pub enum Epoch {
 }
 
 impl TryFrom<u16> for Epoch {
-    type Error = ();
+    type Error = Error;
 
     fn try_from(value: u16) -> Result<Self, Self::Error> {
         match value {
             0 => Ok(Self::Initial),
             1 => Ok(Self::ZeroRtt),
             2 => Ok(Self::Handshake),
             3 => Ok(Self::ApplicationData),
-            _ => Err(()),
+            _ => Err(Error::InvalidEpoch),
         }
     }
 }

neqo-crypto/src/p11.rs

@@ -16,7 +16,7 @@ use std::{
     cell::RefCell,
     ops::{Deref, DerefMut},
     os::raw::c_uint,
-    ptr::null_mut,
+    ptr::{self, null_mut},
     slice::Iter as SliceIter,
 };
 
@@ -229,6 +229,14 @@ impl std::fmt::Debug for SymKey {
     }
 }
 
+impl Default for SymKey {
+    fn default() -> Self {
+        Self {
+            ptr: ptr::null_mut(),
+        }
+    }
+}
+
 unsafe fn destroy_pk11_context(ctxt: *mut PK11Context) {
     PK11_DestroyContext(ctxt, PRBool::from(true));
 }

neqo-crypto/src/secrets.rs

@@ -6,7 +6,7 @@
 
 #![allow(clippy::unwrap_used)] // Let's assume the use of `unwrap` was checked when the use of `unsafe` was reviewed.
 
-use std::{os::raw::c_void, pin::Pin};
+use std::{mem, os::raw::c_void, pin::Pin};
 
 use enum_map::EnumMap;
 use neqo_common::qdebug;
@@ -44,17 +44,21 @@ impl From<SSLSecretDirection::Type> for SecretDirection {
 
 #[derive(Debug, Default)]
 pub struct DirectionalSecrets {
-    // We only need to maintain 3 secrets for the epochs used during the handshake.
-    secrets: EnumMap<Epoch, Option<SymKey>>,
+    secrets: EnumMap<Epoch, SymKey>,
 }
 
 impl DirectionalSecrets {
     fn put(&mut self, epoch: Epoch, key: SymKey) {
-        self.secrets[epoch] = Some(key);
+        debug_assert!(epoch != Epoch::Initial);
+        self.secrets[epoch] = key;
     }
 
     pub fn take(&mut self, epoch: Epoch) -> Option<SymKey> {
-        self.secrets[epoch].take()
+        if self.secrets[epoch].is_null() {
+            None
+        } else {
+            Some(mem::take(&mut self.secrets[epoch]))
+        }
     }
 }
 
@@ -73,10 +77,12 @@ impl Secrets {
         arg: *mut c_void,
     ) {
         let Ok(epoch) = Epoch::try_from(epoch) else {
+            debug_assert!(false, "Invalid epoch");
             // Don't touch secrets.
             return;
         };
         let Some(secrets) = arg.cast::<Self>().as_mut() else {
+            debug_assert!(false, "No secrets");
             return;
         };
         secrets.put_raw(epoch, dir, secret);