Improve TLS testing infrastructure and reliability

This change consolidates duplicate certificate generation code and improves
test infrastructure reliability based on code review feedback.

Key improvements:
1. Consolidate Certificate Generation Utilities:
   - Removed three separate and nearly identical certificate generation
     helper functions from https_client.rs, registration.rs, and
     registrar_client.rs (261 lines of duplicate code eliminated)
   - Introduced a single, comprehensive utility function
     crypto::testing::generate_tls_certs_for_test that creates CA,
     server, and client certificate sets for testing purposes
   - All relevant tests refactored to use this new shared utility,
     reducing code duplication and improving maintainability

2. Strengthen Mockoon TLS Test Assertion:
   - Fixed weak assertion assert!(result.is_err() || result.is_ok()) in
     test_mockoon_registrar_https_registration test
   - Changed to assert!(result.is_err()) to specifically verify that
     connection fails as expected when mock server lacks TLS configuration

3. Replace Brittle Sleep with Polling Mechanism:
   - Replaced sleep 3 command in tests/mockoon_registrar_tests.sh with
     robust polling loop using curl to check server responsiveness
   - Added 15-second timeout with proper error handling
   - Makes tests less prone to failures on slower or busier CI runners

Co-Authored-By: Gemini <[email protected]>
Co-Authored-By: Claude <[email protected]>
Signed-off-by: Sergio Arroutbi <[email protected]>

Author: 3 people

keylime-push-model-agent/src/registration.rs

@@ -540,83 +540,20 @@ mod tests {
         assert_eq!(timeout, None);
     }
 
-    // Helper function to generate test certificates
-    #[cfg(test)]
-    fn generate_test_tls_certificates(
-        temp_dir: &std::path::Path,
-    ) -> (String, String, String) {
-        use keylime::crypto;
-        use std::fs::File;
-        use std::io::Write;
-
-        let ca_path = temp_dir.join("ca.pem");
-        let cert_path = temp_dir.join("cert.pem");
-        let key_path = temp_dir.join("key.pem");
-
-        // Generate CA certificate
-        let ca_key = crypto::testing::rsa_generate(2048)
-            .expect("Failed to generate CA key");
-        let ca_cert = crypto::x509::CertificateBuilder::new()
-            .private_key(&ca_key)
-            .common_name("Test Registrar CA")
-            .build()
-            .expect("Failed to build CA cert");
-
-        // Generate client certificate
-        let client_key = crypto::testing::rsa_generate(2048)
-            .expect("Failed to generate client key");
-        let client_cert = crypto::x509::CertificateBuilder::new()
-            .private_key(&client_key)
-            .common_name("test-agent")
-            .build()
-            .expect("Failed to build client cert");
-
-        // Write certificates to files
-        let mut ca_file =
-            File::create(&ca_path).expect("Failed to create CA file");
-        ca_file
-            .write_all(
-                &ca_cert.to_pem().expect("Failed to convert CA to PEM"),
-            )
-            .expect("Failed to write CA cert");
-
-        let mut cert_file =
-            File::create(&cert_path).expect("Failed to create cert file");
-        cert_file
-            .write_all(
-                &client_cert.to_pem().expect("Failed to convert cert to PEM"),
-            )
-            .expect("Failed to write cert");
-
-        let mut key_file =
-            File::create(&key_path).expect("Failed to create key file");
-        key_file
-            .write_all(
-                &client_key
-                    .private_key_to_pem_pkcs8()
-                    .expect("Failed to convert key to PEM"),
-            )
-            .expect("Failed to write key");
-
-        (
-            ca_path.to_string_lossy().to_string(),
-            cert_path.to_string_lossy().to_string(),
-            key_path.to_string_lossy().to_string(),
-        )
-    }
-
     #[tokio::test]
     async fn test_register_agent_with_real_tls_certs() {
         let _mutex = testing::lock_tests().await;
 
         let tmpdir = tempfile::tempdir().expect("failed to create tmpdir");
-        let (ca_path, cert_path, key_path) =
-            generate_test_tls_certificates(tmpdir.path());
+        let (ca_path, _server_cert, _server_key, cert_path, key_path) =
+            keylime::crypto::testing::generate_tls_certs_for_test(
+                tmpdir.path(),
+            );
 
         // Verify files were created
-        assert!(std::path::Path::new(&ca_path).exists());
-        assert!(std::path::Path::new(&cert_path).exists());
-        assert!(std::path::Path::new(&key_path).exists());
+        assert!(ca_path.exists());
+        assert!(cert_path.exists());
+        assert!(key_path.exists());
 
         let mut config = get_testing_config(tmpdir.path(), None);
         let alg_config = AlgorithmConfigurationString {
@@ -638,9 +575,9 @@ mod tests {
             .expect("Failed to create context info from string");
 
         let tls_config = RegistrarTlsConfig {
-            ca_cert: Some(ca_path),
-            client_cert: Some(cert_path),
-            client_key: Some(key_path),
+            ca_cert: Some(ca_path.to_string_lossy().to_string()),
+            client_cert: Some(cert_path.to_string_lossy().to_string()),
+            client_key: Some(key_path.to_string_lossy().to_string()),
             insecure: Some(false),
             timeout: Some(5000),
         };
@@ -695,21 +632,32 @@ mod tests {
     #[actix_rt::test]
     async fn test_tls_config_all_fields_set() {
         let tmpdir = tempfile::tempdir().expect("failed to create tmpdir");
-        let (ca_path, cert_path, key_path) =
-            generate_test_tls_certificates(tmpdir.path());
+        let (ca_path, _server_cert, _server_key, cert_path, key_path) =
+            keylime::crypto::testing::generate_tls_certs_for_test(
+                tmpdir.path(),
+            );
 
         let tls_config = RegistrarTlsConfig {
-            ca_cert: Some(ca_path.clone()),
-            client_cert: Some(cert_path.clone()),
-            client_key: Some(key_path.clone()),
+            ca_cert: Some(ca_path.to_string_lossy().to_string()),
+            client_cert: Some(cert_path.to_string_lossy().to_string()),
+            client_key: Some(key_path.to_string_lossy().to_string()),
             insecure: Some(false),
             timeout: Some(10000),
         };
 
         // Verify all fields are set correctly
-        assert_eq!(tls_config.ca_cert, Some(ca_path));
-        assert_eq!(tls_config.client_cert, Some(cert_path));
-        assert_eq!(tls_config.client_key, Some(key_path));
+        assert_eq!(
+            tls_config.ca_cert,
+            Some(ca_path.to_string_lossy().to_string())
+        );
+        assert_eq!(
+            tls_config.client_cert,
+            Some(cert_path.to_string_lossy().to_string())
+        );
+        assert_eq!(
+            tls_config.client_key,
+            Some(key_path.to_string_lossy().to_string())
+        );
         assert_eq!(tls_config.insecure, Some(false));
         assert_eq!(tls_config.timeout, Some(10000));
     }

keylime/src/crypto.rs

@@ -1130,7 +1130,7 @@ pub fn decrypt_aead(key: &[u8], data: &[u8]) -> Result<Vec<u8>, CryptoError> {
 pub mod testing {
     use super::*;
     use openssl::encrypt::Encrypter;
-    use std::path::Path;
+    use std::path::{Path, PathBuf};
 
     #[derive(Error, Debug)]
     pub enum CryptoTestError {
@@ -1249,6 +1249,112 @@ pub mod testing {
             .map_err(CryptoError::IOWriteError)?;
         Ok(())
     }
+
+    /// Generates a full set of TLS certificates (CA, server, client) for testing purposes.
+    ///
+    /// # Arguments
+    /// * `temp_dir` - The temporary directory to write the certificate files into.
+    ///
+    /// # Returns
+    /// A tuple containing the paths to the generated files:
+    /// (ca_cert, server_cert, server_key, client_cert, client_key)
+    pub fn generate_tls_certs_for_test(
+        temp_dir: &Path,
+    ) -> (PathBuf, PathBuf, PathBuf, PathBuf, PathBuf) {
+        use std::fs::File;
+        use std::io::Write;
+
+        // Define paths
+        let ca_path = temp_dir.join("ca.pem");
+        let server_cert_path = temp_dir.join("server_cert.pem");
+        let server_key_path = temp_dir.join("server_key.pem");
+        let client_cert_path = temp_dir.join("client_cert.pem");
+        let client_key_path = temp_dir.join("client_key.pem");
+
+        // Generate CA
+        let ca_key = rsa_generate(2048).expect("Failed to generate CA key");
+        let ca_cert = x509::CertificateBuilder::new()
+            .private_key(&ca_key)
+            .common_name("Test CA")
+            .build()
+            .expect("Failed to build CA cert");
+
+        // Generate server certificate
+        let server_key =
+            rsa_generate(2048).expect("Failed to generate server key");
+        let server_cert = x509::CertificateBuilder::new()
+            .private_key(&server_key)
+            .common_name("localhost")
+            .add_ips(vec!["127.0.0.1"])
+            .build()
+            .expect("Failed to build server cert");
+
+        // Generate client certificate
+        let client_key =
+            rsa_generate(2048).expect("Failed to generate client key");
+        let client_cert = x509::CertificateBuilder::new()
+            .private_key(&client_key)
+            .common_name("test-client")
+            .build()
+            .expect("Failed to build client cert");
+
+        // Write files
+        let mut ca_file =
+            File::create(&ca_path).expect("Failed to create CA file");
+        ca_file
+            .write_all(
+                &ca_cert.to_pem().expect("Failed to convert CA to PEM"),
+            )
+            .expect("Failed to write CA cert");
+
+        let mut server_cert_file = File::create(&server_cert_path)
+            .expect("Failed to create server cert file");
+        server_cert_file
+            .write_all(
+                &server_cert
+                    .to_pem()
+                    .expect("Failed to convert server cert to PEM"),
+            )
+            .expect("Failed to write server cert");
+
+        let mut server_key_file = File::create(&server_key_path)
+            .expect("Failed to create server key file");
+        server_key_file
+            .write_all(
+                &server_key
+                    .private_key_to_pem_pkcs8()
+                    .expect("Failed to convert key to PEM"),
+            )
+            .expect("Failed to write server key");
+
+        let mut client_cert_file = File::create(&client_cert_path)
+            .expect("Failed to create client cert file");
+        client_cert_file
+            .write_all(
+                &client_cert
+                    .to_pem()
+                    .expect("Failed to convert client cert to PEM"),
+            )
+            .expect("Failed to write client cert");
+
+        let mut client_key_file = File::create(&client_key_path)
+            .expect("Failed to create client key file");
+        client_key_file
+            .write_all(
+                &client_key
+                    .private_key_to_pem_pkcs8()
+                    .expect("Failed to convert key to PEM"),
+            )
+            .expect("Failed to write client key");
+
+        (
+            ca_path,
+            server_cert_path,
+            server_key_path,
+            client_cert_path,
+            client_key_path,
+        )
+    }
 }
 
 // Unit Testing