net/quic: Fix inconsistencies around secure connection handling

This set of changes implements the below fixes:

- Allow certificates to be specified for client/quicsink
- Secure connection being true on server/quicsrc and false on
  client/quicsink still resulted in a successful connection
  instead of server rejecting the connection
- Using secure connection with ALPN was not working

Part-of: <https://gitlab.freedesktop.org/gstreamer/gst-plugins-rs/-/merge_requests/1036>

net/quic/src/quinnquicsink/imp.rs

@@ -16,6 +16,7 @@ use gst::{glib, prelude::*, subclass::prelude::*};
 use gst_base::subclass::prelude::*;
 use once_cell::sync::Lazy;
 use quinn::{Connection, SendStream};
+use std::path::PathBuf;
 use std::sync::Mutex;
 
 static DEFAULT_SERVER_NAME: &str = "localhost";
@@ -67,6 +68,7 @@ struct Settings {
     timeout: u32,
     secure_conn: bool,
     use_datagram: bool,
+    certificate_path: Option<PathBuf>,
 }
 
 impl Default for Settings {
@@ -81,6 +83,7 @@ impl Default for Settings {
             timeout: DEFAULT_TIMEOUT,
             secure_conn: DEFAULT_SECURE_CONNECTION,
             use_datagram: false,
+            certificate_path: None,
         }
     }
 }
@@ -131,6 +134,30 @@ impl ElementImpl for QuinnQuicSink {
 
         PAD_TEMPLATES.as_ref()
     }
+
+    fn change_state(
+        &self,
+        transition: gst::StateChange,
+    ) -> Result<gst::StateChangeSuccess, gst::StateChangeError> {
+        if transition == gst::StateChange::NullToReady {
+            let settings = self.settings.lock().unwrap();
+
+            /*
+             * Fail the state change if a secure connection was requested but
+             * no certificate path was provided.
+             */
+            if settings.secure_conn && settings.certificate_path.is_none() {
+                gst::error!(
+                    CAT,
+                    imp: self,
+                    "Certificate path not provided for secure connection"
+                );
+                return Err(gst::StateChangeError);
+            }
+        }
+
+        self.parent_change_state(transition)
+    }
 }
 
 impl ObjectImpl for QuinnQuicSink {
@@ -184,6 +211,10 @@ impl ObjectImpl for QuinnQuicSink {
                     .blurb("Use certificates for QUIC connection. False: Insecure connection, True: Secure connection.")
                     .default_value(DEFAULT_SECURE_CONNECTION)
                     .build(),
+                glib::ParamSpecString::builder("certificate-path")
+                    .nick("Certificate Path")
+                    .blurb("Path where the certificate files cert.pem and privkey.pem are stored")
+                    .build(),
                 glib::ParamSpecBoolean::builder("use-datagram")
                     .nick("Use datagram")
                     .blurb("Use datagram for lower latency, unreliable messaging")
@@ -233,6 +264,10 @@ impl ObjectImpl for QuinnQuicSink {
             "secure-connection" => {
                 settings.secure_conn = value.get().expect("type checked upstream");
             }
+            "certificate-path" => {
+                let value: String = value.get().unwrap();
+                settings.certificate_path = Some(value.into());
+            }
             "use-datagram" => {
                 settings.use_datagram = value.get().expect("type checked upstream");
             }
@@ -261,6 +296,10 @@ impl ObjectImpl for QuinnQuicSink {
             }
             "timeout" => settings.timeout.to_value(),
             "secure-connection" => settings.secure_conn.to_value(),
+            "certificate-path" => {
+                let certpath = settings.certificate_path.as_ref();
+                certpath.and_then(|file| file.to_str()).to_value()
+            }
             "use-datagram" => settings.use_datagram.to_value(),
             _ => unimplemented!(),
         }
@@ -456,6 +495,7 @@ impl QuinnQuicSink {
         let alpns;
         let use_datagram;
         let secure_conn;
+        let cert_path;
 
         {
             let settings = self.settings.lock().unwrap();
@@ -471,14 +511,16 @@ impl QuinnQuicSink {
             alpns = settings.alpns.clone();
             use_datagram = settings.use_datagram;
             secure_conn = settings.secure_conn;
+            cert_path = settings.certificate_path.clone();
         }
 
-        let endpoint = client_endpoint(client_addr, secure_conn, alpns).map_err(|err| {
-            WaitError::FutureError(gst::error_msg!(
-                gst::ResourceError::Failed,
-                ["Failed to configure endpoint: {}", err]
-            ))
-        })?;
+        let endpoint =
+            client_endpoint(client_addr, secure_conn, alpns, cert_path).map_err(|err| {
+                WaitError::FutureError(gst::error_msg!(
+                    gst::ResourceError::Failed,
+                    ["Failed to configure endpoint: {}", err]
+                ))
+            })?;
 
         let connection = endpoint
             .connect(server_addr, &server_name)

net/quic/src/utils.rs

@@ -136,23 +136,38 @@ impl rustls::client::ServerCertVerifier for SkipServerVerification {
     }
 }
 
-fn configure_client(secure_conn: bool, alpns: Vec<String>) -> Result<ClientConfig, Box<dyn Error>> {
-    if secure_conn {
-        Ok(ClientConfig::with_native_roots())
+fn configure_client(
+    secure_conn: bool,
+    certificate_path: Option<PathBuf>,
+    alpns: Vec<String>,
+) -> Result<ClientConfig, Box<dyn Error>> {
+    let mut crypto = if secure_conn {
+        let (certs, key) = read_certs_from_file(certificate_path)?;
+        let mut cert_store = rustls::RootCertStore::empty();
+
+        for cert in &certs {
+            cert_store.add(cert)?;
+        }
+
+        rustls::ClientConfig::builder()
+            .with_safe_defaults()
+            .with_root_certificates(Arc::new(cert_store))
+            .with_client_auth_cert(certs, key)?
     } else {
-        let mut crypto = rustls::ClientConfig::builder()
+        rustls::ClientConfig::builder()
             .with_safe_defaults()
             .with_custom_certificate_verifier(SkipServerVerification::new())
-            .with_no_client_auth();
-        let alpn_protocols: Vec<Vec<u8>> = alpns
-            .iter()
-            .map(|x| x.as_bytes().to_vec())
-            .collect::<Vec<_>>();
-        crypto.alpn_protocols = alpn_protocols;
-        crypto.key_log = Arc::new(rustls::KeyLogFile::new());
+            .with_no_client_auth()
+    };
 
-        Ok(ClientConfig::new(Arc::new(crypto)))
-    }
+    let alpn_protocols: Vec<Vec<u8>> = alpns
+        .iter()
+        .map(|x| x.as_bytes().to_vec())
+        .collect::<Vec<_>>();
+    crypto.alpn_protocols = alpn_protocols;
+    crypto.key_log = Arc::new(rustls::KeyLogFile::new());
+
+    Ok(ClientConfig::new(Arc::new(crypto)))
 }
 
 fn read_certs_from_file(
@@ -213,8 +228,8 @@ fn configure_server(
     certificate_path: Option<PathBuf>,
     alpns: Vec<String>,
 ) -> Result<(ServerConfig, Vec<rustls::Certificate>), Box<dyn Error>> {
-    let (cert, key) = if secure_conn {
-        read_certs_from_file(certificate_path).unwrap()
+    let (certs, key) = if secure_conn {
+        read_certs_from_file(certificate_path)?
     } else {
         let cert = rcgen::generate_simple_self_signed(vec![server_name.into()]).unwrap();
         let cert_der = cert.serialize_der().unwrap();
@@ -225,13 +240,24 @@ fn configure_server(
         (cert_chain, priv_key)
     };
 
-    let mut crypto = rustls::ServerConfig::builder()
-        .with_safe_default_cipher_suites()
-        .with_safe_default_kx_groups()
-        .with_protocol_versions(&[&rustls::version::TLS13])
-        .unwrap()
-        .with_no_client_auth()
-        .with_single_cert(cert.clone(), key)?;
+    let mut crypto = if secure_conn {
+        let mut cert_store = rustls::RootCertStore::empty();
+        for cert in &certs {
+            cert_store.add(cert)?;
+        }
+
+        let auth_client = rustls::server::AllowAnyAuthenticatedClient::new(cert_store);
+        rustls::ServerConfig::builder()
+            .with_safe_defaults()
+            .with_client_cert_verifier(Arc::new(auth_client))
+            .with_single_cert(certs.clone(), key)
+    } else {
+        rustls::ServerConfig::builder()
+            .with_safe_defaults()
+            .with_no_client_auth()
+            .with_single_cert(certs.clone(), key)
+    }?;
+
     let alpn_protocols: Vec<Vec<u8>> = alpns
         .iter()
         .map(|x| x.as_bytes().to_vec())
@@ -245,7 +271,7 @@ fn configure_server(
         .max_concurrent_bidi_streams(0_u8.into())
         .max_concurrent_uni_streams(1_u8.into());
 
-    Ok((server_config, cert))
+    Ok((server_config, certs))
 }
 
 pub fn server_endpoint(
@@ -264,9 +290,10 @@ pub fn server_endpoint(
 pub fn client_endpoint(
     client_addr: SocketAddr,
     secure_conn: bool,
-    alpn: Vec<String>,
+    alpns: Vec<String>,
+    certificate_path: Option<PathBuf>,
 ) -> Result<Endpoint, Box<dyn Error>> {
-    let client_cfg = configure_client(secure_conn, alpn)?;
+    let client_cfg = configure_client(secure_conn, certificate_path, alpns)?;
     let mut endpoint = Endpoint::client(client_addr)?;
 
     endpoint.set_default_client_config(client_cfg);