fix: Don't panic in neqo_crypto::init()

neqo-bin/src/bin/client/main.rs

@@ -45,6 +45,7 @@ pub enum ClientError {
     IoError(io::Error),
     QlogError,
     TransportError(neqo_transport::Error),
+    InternalError,
 }
 
 impl From<io::Error> for ClientError {
@@ -451,7 +452,7 @@ async fn main() -> Res<()> {
     neqo_common::log::init(Some(args.verbose.log_level_filter()));
     args.update_for_tests();
 
-    init();
+    init().map_err(|_| ClientError::InternalError)?;
 
     let urls_by_origin = args
         .urls

neqo-bin/src/bin/server/main.rs

@@ -579,7 +579,7 @@ async fn main() -> Result<(), io::Error> {
     neqo_common::log::init(Some(args.verbose.log_level_filter()));
     assert!(!args.key.is_empty(), "Need at least one key");
 
-    init_db(args.db.clone());
+    init_db(args.db.clone()).map_err(|_| io::Error::other("init_db failure"))?;
 
     if let Some(testcase) = args.shared.qns_test.as_ref() {
         if args.shared.quic_parameters.quic_version.is_empty() {

neqo-crypto/src/lib.rs

@@ -89,7 +89,7 @@ impl Drop for NssLoaded {
     }
 }
 
-static INITIALIZED: OnceLock<NssLoaded> = OnceLock::new();
+static INITIALIZED: OnceLock<Res<NssLoaded>> = OnceLock::new();
 
 fn already_initialized() -> bool {
     unsafe { nss::NSS_IsInitialized() != 0 }
@@ -107,24 +107,27 @@ fn version_check() {
 /// Initialize NSS.  This only executes the initialization routines once, so if there is any chance
 /// that
 ///
-/// # Panics
+/// # Errors
 ///
 /// When NSS initialization fails.
-pub fn init() {
+pub fn init() -> Res<()> {
     // Set time zero.
     time::init();
-    _ = INITIALIZED.get_or_init(|| {
+    let res = INITIALIZED.get_or_init(|| {
         version_check();
         if already_initialized() {
-            return NssLoaded::External;
+            return Ok(NssLoaded::External);
         }
 
-        secstatus_to_res(unsafe { nss::NSS_NoDB_Init(null()) }).expect("NSS_NoDB_Init failed");
-        secstatus_to_res(unsafe { nss::NSS_SetDomesticPolicy() })
-            .expect("NSS_SetDomesticPolicy failed");
+        secstatus_to_res(unsafe { nss::NSS_NoDB_Init(null()) })?;
+        secstatus_to_res(unsafe { nss::NSS_SetDomesticPolicy() })?;
 
-        NssLoaded::NoDb
+        Ok(NssLoaded::NoDb)
     });
+    match res {
+        Ok(_) => Ok(()),
+        Err(e) => Err(e.clone()),
+    }
 }
 
 /// This enables SSLTRACE by calling a simple, harmless function to trigger its
@@ -141,22 +144,24 @@ fn enable_ssl_trace() {
 
 /// Initialize with a database.
 ///
-/// # Panics
+/// # Errors
 ///
 /// If NSS cannot be initialized.
-pub fn init_db<P: Into<PathBuf>>(dir: P) {
+pub fn init_db<P: Into<PathBuf>>(dir: P) -> Res<()> {
     time::init();
-    _ = INITIALIZED.get_or_init(|| {
+    let res = INITIALIZED.get_or_init(|| {
         version_check();
         if already_initialized() {
-            return NssLoaded::External;
+            return Ok(NssLoaded::External);
         }
 
         let path = dir.into();
-        assert!(path.is_dir());
-        let pathstr = path.to_str().expect("path converts to string").to_string();
-        let dircstr = CString::new(pathstr).unwrap();
-        let empty = CString::new("").unwrap();
+        if !path.is_dir() {
+            return Err(Error::InternalError);
+        }
+        let pathstr = path.to_str().ok_or(Error::InternalError)?;
+        let dircstr = CString::new(pathstr)?;
+        let empty = CString::new("")?;
         secstatus_to_res(unsafe {
             nss::NSS_Initialize(
                 dircstr.as_ptr(),
@@ -165,21 +170,22 @@ pub fn init_db<P: Into<PathBuf>>(dir: P) {
                 nss::SECMOD_DB.as_ptr().cast(),
                 nss::NSS_INIT_READONLY,
             )
-        })
-        .expect("NSS_Initialize failed");
+        })?;
 
-        secstatus_to_res(unsafe { nss::NSS_SetDomesticPolicy() })
-            .expect("NSS_SetDomesticPolicy failed");
+        secstatus_to_res(unsafe { nss::NSS_SetDomesticPolicy() })?;
         secstatus_to_res(unsafe {
             ssl::SSL_ConfigServerSessionIDCache(1024, 0, 0, dircstr.as_ptr())
-        })
-        .expect("SSL_ConfigServerSessionIDCache failed");
+        })?;
 
         #[cfg(debug_assertions)]
         enable_ssl_trace();
 
-        NssLoaded::Db
+        Ok(NssLoaded::Db)
     });
+    match res {
+        Ok(_) => Ok(()),
+        Err(e) => Err(e.clone()),
+    }
 }
 
 /// # Panics

neqo-crypto/tests/init.rs

@@ -39,7 +39,7 @@ fn init_nodb() {
 #[cfg(not(nss_nodb))]
 #[test]
 fn init_withdb() {
-    init_db(::test_fixture::NSS_DB_PATH);
+    let _ = init_db(::test_fixture::NSS_DB_PATH);
     assert_initialized();
     unsafe {
         assert!(nss::NSS_IsInitialized() != 0);

neqo-crypto/tests/selfencrypt.rs

@@ -15,15 +15,15 @@ use neqo_crypto::{
 
 #[test]
 fn se_create() {
-    init();
+    let _ = init();
     SelfEncrypt::new(TLS_VERSION_1_3, TLS_AES_128_GCM_SHA256).expect("constructor works");
 }
 
 const PLAINTEXT: &[u8] = b"PLAINTEXT";
 const AAD: &[u8] = b"AAD";
 
 fn sealed() -> (SelfEncrypt, Vec<u8>) {
-    init();
+    let _ = init();
     let se = SelfEncrypt::new(TLS_VERSION_1_3, TLS_AES_128_GCM_SHA256).unwrap();
     let sealed = se.seal(AAD, PLAINTEXT).expect("sealing works");
     (se, sealed)

test-fixture/src/lib.rs

@@ -42,7 +42,7 @@ pub const NSS_DB_PATH: &str = concat!(env!("CARGO_MANIFEST_DIR"), "/db");
 /// fixture function that depends on setup.  Other functions in the fixture
 /// that depend on this setup call the function for you.
 pub fn fixture_init() {
-    init_db(NSS_DB_PATH);
+    let _ = init_db(NSS_DB_PATH);
 }
 
 // This needs to be > 2ms to avoid it being rounded to zero.