chore: Add tests for Gecko-specific functions (#3280)

Author: larseggert

neqo-crypto/bindings/bindings.toml

@@ -14,6 +14,7 @@ types = [
     "SSLExtensionHandler",
     "SSLExtensionType",
     "SSLExtensionWriter",
+    "SSLExtraServerCertData",
     "SSLHelloRetryRequestAction",
     "SSLHelloRetryRequestCallback",
     "SSLNamedGroup",
@@ -80,12 +81,12 @@ variables = [
 ]
 opaque = [
     "CERTCertificate",
+    "CERTCertificateList",
     "PK11SymKey",
     "PLArenaPool",
     "PRFileDesc",
     "SECKEYPrivateKey",
     "SECKEYPublicKey",
-    "SSLExtraServerCertData",
 ]
 
 [nss_sslopt]

neqo-crypto/src/agent.rs

@@ -13,7 +13,7 @@ use std::{
     cell::RefCell,
     ffi::{CStr, CString},
     fmt::{self, Debug, Display, Formatter},
-    mem::MaybeUninit,
+    mem::{size_of, MaybeUninit},
     ops::{Deref, DerefMut},
     os::raw::{c_uint, c_void},
     pin::Pin,
@@ -414,7 +414,6 @@ impl SecretAgentInfo {
     pub const fn alpn(&self) -> Option<&String> {
         self.alpn.as_ref()
     }
-    // TODO: Not used in neqo, but Gecko calls it. Needs a test to call it.
     #[must_use]
     pub const fn signature_scheme(&self) -> SignatureScheme {
         self.signature_scheme
@@ -1256,6 +1255,17 @@ pub struct Server {
     zero_rtt_check: Option<Pin<Box<ZeroRttCheckState>>>,
 }
 
+fn load_cert_and_key(name: &str) -> Res<(p11::Certificate, PrivateKey)> {
+    let c = CString::new(name)?;
+    let cert = p11::Certificate::from_ptr(unsafe {
+        p11::PK11_FindCertFromNickname(c.as_ptr(), null_mut())
+    })
+    .map_err(|_| Error::CertificateLoading)?;
+    let key = PrivateKey::from_ptr(unsafe { p11::PK11_FindKeyByAnyCert(*cert, null_mut()) })
+        .map_err(|_| Error::CertificateLoading)?;
+    Ok((cert, key))
+}
+
 impl Server {
     /// Create a new server agent.
     ///
@@ -1264,22 +1274,63 @@ impl Server {
     /// Errors returned when NSS fails.
     pub fn new<A: AsRef<str>>(certificates: &[A]) -> Res<Self> {
         let mut agent = SecretAgent::new()?;
+        for n in certificates {
+            let (cert, key) = load_cert_and_key(n.as_ref())?;
+            secstatus_to_res(unsafe {
+                ssl::SSL_ConfigServerCert(agent.fd, (*cert).cast(), (*key).cast(), null(), 0)
+            })?;
+        }
+        agent.ready(true, true)?;
+        Ok(Self {
+            agent,
+            zero_rtt_check: None,
+        })
+    }
 
+    /// Create a server with OCSP responses and SCTs configured.
+    /// Not suitable for multiple certificates, because it configures the same OCSP/SCT for all
+    /// certificates. In other words, this is good for testing that the plumbing works, not for
+    /// a real server.
+    ///
+    /// # Errors
+    ///
+    /// Errors returned when NSS fails.
+    pub fn new_with_ocsp_and_scts<A: AsRef<str>>(
+        certificates: &[A],
+        ocsp_responses: &[&[u8]],
+        scts: &[u8],
+    ) -> Res<Self> {
+        let mut agent = SecretAgent::new()?;
         for n in certificates {
-            let c = CString::new(n.as_ref())?;
-            let cert_ptr = unsafe { p11::PK11_FindCertFromNickname(c.as_ptr(), null_mut()) };
-            let Ok(cert) = p11::Certificate::from_ptr(cert_ptr) else {
-                return Err(Error::CertificateLoading);
+            let (cert, key) = load_cert_and_key(n.as_ref())?;
+            let ocsp_items: Vec<p11::SECItem> = ocsp_responses
+                .iter()
+                .map(|b| p11::Item::wrap(b))
+                .collect::<Res<_>>()?;
+            let ocsp_array = ssl::SECItemArrayStr {
+                items: ocsp_items.as_ptr().cast::<ssl::SECItem>().cast_mut(),
+                len: c_uint::try_from(ocsp_items.len())?,
             };
-            let key_ptr = unsafe { p11::PK11_FindKeyByAnyCert(*cert, null_mut()) };
-            let Ok(key) = PrivateKey::from_ptr(key_ptr) else {
-                return Err(Error::CertificateLoading);
+            let sct_item = p11::Item::wrap(scts)?;
+            let extra = ssl::SSLExtraServerCertDataStr {
+                // ssl_auth_null means "I don't care what sort of certificate this is".
+                authType: ssl::SSLAuthType::ssl_auth_null,
+                certChain: null(),
+                stapledOCSPResponses: &ocsp_array,
+                signedCertTimestamps: std::ptr::from_ref(&sct_item).cast(),
+                delegCred: null(),
+                delegCredPrivKey: null(),
             };
             secstatus_to_res(unsafe {
-                ssl::SSL_ConfigServerCert(agent.fd, (*cert).cast(), (*key).cast(), null(), 0)
+                ssl::SSL_ConfigServerCert(
+                    agent.fd,
+                    (*cert).cast(),
+                    (*key).cast(),
+                    &extra,
+                    c_uint::try_from(size_of::<ssl::SSLExtraServerCertDataStr>())?,
+                )
             })?;
         }
-
         agent.ready(true, true)?;
         Ok(Self {
             agent,

neqo-crypto/src/auth.rs

@@ -50,11 +50,25 @@ impl From<PRErrorCode> for AuthenticationStatus {
     }
 }
 
-#[test]
-fn authentication_status_from_error_code() {
-    assert_eq!(AuthenticationStatus::from(0), AuthenticationStatus::Ok);
-    assert_eq!(
-        AuthenticationStatus::from(12345),
-        AuthenticationStatus::Unknown
-    );
+#[cfg(test)]
+#[cfg_attr(coverage_nightly, coverage(off))]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn authentication_status_from_error_code() {
+        assert_eq!(
+            AuthenticationStatus::from(sec::SEC_ERROR_EXPIRED_CERTIFICATE),
+            AuthenticationStatus::CertExpired
+        );
+        assert_eq!(AuthenticationStatus::from(0), AuthenticationStatus::Ok);
+        assert_eq!(
+            AuthenticationStatus::from(12345),
+            AuthenticationStatus::Unknown
+        );
+        assert_eq!(
+            AuthenticationStatus::from(i32::MIN),
+            AuthenticationStatus::Unknown
+        );
+    }
 }

neqo-crypto/src/cert.rs

@@ -91,13 +91,11 @@ impl CertificateInfo {
         self.certs.into_iter()
     }
 
-    // TODO: Not used in neqo, but Gecko calls it. Needs a test to call it.
     #[must_use]
     pub const fn stapled_ocsp_responses(&self) -> Option<&Vec<Vec<u8>>> {
         self.stapled_ocsp_responses.as_ref()
     }
 
-    // TODO: Not used in neqo, but Gecko calls it. Needs a test to call it.
     #[must_use]
     pub const fn signed_cert_timestamp(&self) -> Option<&Vec<u8>> {
         self.signed_cert_timestamp.as_ref()

neqo-crypto/tests/agent.rs

@@ -10,7 +10,7 @@ use neqo_crypto::{
     agent::CertificateCompressor, generate_ech_keys, AuthenticationStatus, Client, Error,
     HandshakeState, Res, SecretAgentPreInfo, Server, ZeroRttCheckResult, ZeroRttChecker,
     TLS_AES_128_GCM_SHA256, TLS_CHACHA20_POLY1305_SHA256, TLS_GRP_EC_SECP256R1, TLS_GRP_EC_X25519,
-    TLS_VERSION_1_3,
+    TLS_SIG_ECDSA_SECP256R1_SHA256, TLS_VERSION_1_3,
 };
 
 mod handshake;
@@ -66,6 +66,10 @@ fn basic() {
     let client_info = client.info().expect("got info");
     assert_eq!(TLS_VERSION_1_3, client_info.version());
     assert_eq!(TLS_AES_128_GCM_SHA256, client_info.cipher_suite());
+    assert_eq!(
+        TLS_SIG_ECDSA_SECP256R1_SHA256,
+        client_info.signature_scheme()
+    );
 
     let bytes = server.handshake(now(), &bytes[..]).expect("finish");
     assert!(bytes.is_empty());
@@ -74,6 +78,10 @@ fn basic() {
     let server_info = server.info().expect("got info");
     assert_eq!(TLS_VERSION_1_3, server_info.version());
     assert_eq!(TLS_AES_128_GCM_SHA256, server_info.cipher_suite());
+    assert_eq!(
+        TLS_SIG_ECDSA_SECP256R1_SHA256,
+        server_info.signature_scheme()
+    );
 }
 
 fn check_client_preinfo(client_preinfo: &SecretAgentPreInfo) {
@@ -134,11 +142,35 @@ fn raw() {
     // The client should have one certificate for the server.
     let certs = client.peer_certificate().unwrap();
     assert_eq!(1, certs.into_iter().count());
+    assert!(certs.stapled_ocsp_responses().unwrap().is_empty());
+    assert!(certs.signed_cert_timestamp().unwrap().is_empty());
 
     // The server shouldn't have a client certificate.
     assert!(server.peer_certificate().is_none());
 }
 
+#[test]
+fn ocsp_stapling_and_signed_cert_timestamps() {
+    fixture_init();
+    let mut client = Client::new("server.example", true).expect("should create client");
+    client
+        .set_option(neqo_crypto::Opt::SignedCertificateTimestamps, true)
+        .unwrap();
+    let ocsp_response = b"fake ocsp response";
+    let scts = b"fake signed certificate timestamps";
+    let mut server = Server::new_with_ocsp_and_scts(&["key"], &[&ocsp_response[..]], scts)
+        .expect("should create server");
+
+    connect(&mut client, &mut server);
+
+    let certs = client.peer_certificate().unwrap();
+    assert_eq!(1, certs.into_iter().count());
+    let ocsp = certs.stapled_ocsp_responses().unwrap();
+    assert_eq!(ocsp.len(), 1);
+    assert_eq!(ocsp[0], ocsp_response);
+    assert_eq!(certs.signed_cert_timestamp().unwrap(), scts);
+}
+
 #[test]
 fn chacha_client() {
     fixture_init();

neqo-http3/src/conn_params.rs

@@ -137,7 +137,6 @@ impl Http3Parameters {
         self.connect
     }
 
-    // TODO: Not used in neqo, but Gecko calls it. Needs a test to call it.
     #[must_use]
     pub const fn http3_datagram(mut self, http3_datagram: bool) -> Self {
         self.http3_datagram = http3_datagram;
@@ -193,4 +192,12 @@ mod tests {
     fn max_table_size_decoder_rejects_above_limit() {
         _ = Http3Parameters::default().max_table_size_decoder(1 << 30);
     }
+
+    #[test]
+    fn http3_datagram_setting() {
+        let params = Http3Parameters::default()
+            .connection_parameters(ConnectionParameters::default().datagram_size(1200))
+            .http3_datagram(true);
+        assert!(params.get_http3_datagram());
+    }
 }

neqo-http3/src/connection_client.rs

@@ -876,8 +876,6 @@ impl Http3Client {
     /// # Errors
     ///
     /// It may return `InvalidStreamId` if a stream does not exist anymore.
-    //
-    // TODO: Not used in neqo, but Gecko calls it. Needs a test to call it.
     pub fn webtransport_set_sendorder(
         &mut self,
         stream_id: StreamId,

neqo-http3/src/features/extended_connect/tests/webtransport/streams.rs

@@ -1127,3 +1127,16 @@ fn wt_session_close_frame_and_streams_client() {
         )),
     );
 }
+
+#[test]
+fn wt_set_sendorder() {
+    let mut wt = WtTest::new();
+    let wt_session = wt.create_wt_session();
+    let wt_stream = wt.create_wt_stream_client(wt_session.stream_id(), StreamType::UniDi);
+    wt.client
+        .webtransport_set_sendorder(wt_stream, Some(42))
+        .unwrap();
+    wt.client
+        .webtransport_set_sendorder(wt_stream, None)
+        .unwrap();
+}

neqo-transport/src/connection/mod.rs

@@ -490,8 +490,6 @@ impl Connection {
 
     /// # Errors
     /// When the operation fails.
-    //
-    // TODO: Not used in neqo, but Gecko calls it. Needs a test to call it.
     pub fn set_certificate_compression<T: CertificateCompressor>(&mut self) -> Res<()> {
         self.crypto.tls_mut().set_certificate_compression::<T>()?;
         Ok(())

neqo-transport/src/connection/params.rs

@@ -449,7 +449,6 @@ impl ConnectionParameters {
         self.pmtud_iface_mtu
     }
 
-    // TODO: Not used in neqo, but Gecko calls it. Needs a test to call it.
     #[must_use]
     pub const fn pmtud_iface_mtu(mut self, pmtud_iface_mtu: bool) -> Self {
         self.pmtud_iface_mtu = pmtud_iface_mtu;
@@ -556,3 +555,17 @@ impl ConnectionParameters {
         Ok(tps)
     }
 }
+
+#[cfg(test)]
+#[cfg_attr(coverage_nightly, coverage(off))]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn pmtud_iface_mtu() {
+        let params = ConnectionParameters::default().pmtud_iface_mtu(true);
+        assert!(params.pmtud_iface_mtu_enabled());
+        let params = params.pmtud_iface_mtu(false);
+        assert!(!params.pmtud_iface_mtu_enabled());
+    }
+}