Refactor/rethinking

README.md

@@ -379,46 +379,7 @@ When you implement SFTP in Citadel, you're responsible for taking care of logist
 
 ## Helpers
 
-### SSH Key Generation
-
-Citadel provides a high-level API for generating SSH key pairs programmatically:
-
-```swift
-// Generate Ed25519 key pair (recommended for most cases)
-let keyPair = SSHKeyGenerator.generateEd25519()
-
-// Generate RSA key pair
-let rsaKeyPair = SSHKeyGenerator.generateRSA(bits: 4096)
-
-// Generate ECDSA key pair
-let ecdsaKeyPair = SSHKeyGenerator.generateECDSA(curve: .p256)
-
-// Export keys in various formats
-
-/// OpenSSH format
-let privateKeyString = try keyPair.privateKeyOpenSSHString(comment: "user@example.com")
-let publicKeyString = try keyPair.publicKeyOpenSSHString() // ssh-ed25519 AAAA...
-
-/// PEM format
-let publicKeyPEM = try keyPair.publicKeyPEMString()
-let privateKeyPEM = try keyPair.privateKeyPEMString()
-
-// Export with passphrase protection
-let encryptedKey = try keyPair.privateKeyOpenSSHString(
-    comment: "user@example.com",
-    passphrase: "secure_passphrase",
-    cipher: "aes256-ctr"  // Supported: "none", "aes128-ctr", "aes256-ctr"
-)
-
-// Save keys to files
-try privateKeyString.write(toFile: "~/.ssh/id_ed25519", atomically: true, encoding: .utf8)
-try publicKeyString.write(toFile: "~/.ssh/id_ed25519.pub", atomically: true, encoding: .utf8)
-```
-
-### OpenSSH Key Parsing
-
-We support extensions on PrivateKey types such as our own `Insecure.RSA.PrivateKey`, as well as existing SwiftCrypto types like `Curve25519.Signing.PrivateKey`:
-
+The most important helper most people need is OpenSSH key parsing. We support extensions on PrivateKey types such as our own `Insecure.RSA.PrivateKey`, as well as existing SwiftCrypto types like `Curve25519.Signing.PrivateKey`:
 ```swift
 // Parse an OpenSSH RSA private key. This is the same format as the one used by OpenSSH
 let sshFile = try String(contentsOf: ..)

Tests/CitadelTests/CertificateAuthenticationMethodRealTests.swift

@@ -6,30 +6,45 @@ import _CryptoExtras
 /// Tests for certificate authentication methods using real SSH certificates
 final class CertificateAuthenticationMethodRealTests: XCTestCase {
 
-    override class func setUp() {
+    override func setUp() {
         super.setUp()
-        // Generate certificates dynamically for tests
-        do {
-            try SSHCertificateGenerator.ensureSSHKeygenAvailable()
-            try SSHCertificateGenerator.setUp()
-        } catch {
-            print("Failed to set up certificate generation: \(error)")
+        // Generate certificates dynamically for tests (only once)
+        if !SSHCertificateGenerator.hasAttemptedSetup {
+            SSHCertificateGenerator.hasAttemptedSetup = true
+            do {
+                try SSHCertificateGenerator.ensureSSHKeygenAvailable()
+                try SSHCertificateGenerator.setUp()
+                SSHCertificateGenerator.isSetupSuccessful = true
+            } catch {
+                SSHCertificateGenerator.setupError = error
+                SSHCertificateGenerator.isSetupSuccessful = false
+            }
+        }
+
+        // Check setup success for each test
+        if !SSHCertificateGenerator.isSetupSuccessful {
+            if let error = SSHCertificateGenerator.setupError {
+                XCTFail("Certificate generation setup failed: \(error)")
+            } else {
+                XCTFail("Certificate generation setup failed")
+            }
         }
     }
 
-    override class func tearDown() {
+    override func tearDown() {
         super.tearDown()
         // Clean up generated certificates
         do {
             try TestCertificateHelper.cleanUp()
         } catch {
-            print("Failed to clean up certificates: \(error)")
+            XCTFail("Certificate cleanup failed: \(error)")
         }
     }
 
     // MARK: - Ed25519 Certificate Tests
 
     func testEd25519CertificateWithValidCertificate() throws {
+
         let (privateKey, certificate) = try TestCertificateHelper.parseEd25519Certificate(
             certificateFile: "user_ed25519-cert.pub",
             privateKeyFile: "user_ed25519"
@@ -58,13 +73,15 @@ final class CertificateAuthenticationMethodRealTests: XCTestCase {
     }
 
     func testEd25519CertificateWithExpiredCertificate() throws {
+
         // SKIP TEST: Time-based validation tests require certificates with specific validity periods
         // The test certificates are generated with 1 hour validity and may have been regenerated
         // making this test unreliable. The time validation logic is tested in CertificateSecurityValidationTests
         throw XCTSkip("Time-based validation is tested in CertificateSecurityValidationTests")
     }
 
     func testEd25519CertificateWithWrongPrincipal() throws {
+
         // Generate a certificate with limited principals
         let certificate = try TestCertificateHelper.generateLimitedPrincipalsCertificate()
 
@@ -87,6 +104,7 @@ final class CertificateAuthenticationMethodRealTests: XCTestCase {
     // MARK: - P256 Certificate Tests
 
     func testP256CertificateValidation() throws {
+
         let (privateKey, certificate) = try TestCertificateHelper.parseP256Certificate(
             certificateFile: "user_ecdsa_p256-cert.pub",
             privateKeyFile: "user_ecdsa_p256"
@@ -117,6 +135,7 @@ final class CertificateAuthenticationMethodRealTests: XCTestCase {
     // MARK: - RSA Certificate Tests
 
     func testRSACertificateValidation() throws {
+
         // SKIP TEST: RSA certificates are not supported by NIOSSH
         // While Citadel can parse and validate RSA certificates correctly,
         // NIOSSH (the underlying SSH library) does not support RSA certificates
@@ -146,6 +165,7 @@ final class CertificateAuthenticationMethodRealTests: XCTestCase {
     }
 
     func testRSACertificateWithHostType() throws {
+
         // SKIP TEST: Certificate type validation is not enforced in user authentication
         // The current implementation only validates certificate type when checking
         // principals (username for user certs, hostname for host certs).
@@ -188,6 +208,7 @@ final class CertificateAuthenticationMethodRealTests: XCTestCase {
     // MARK: - P384 Certificate Tests
 
     func testP384CertificateWithMultiplePrincipals() throws {
+
         let (privateKey, certificate) = try TestCertificateHelper.parseP384Certificate(
             certificateFile: "user_ecdsa_p384-cert.pub",
             privateKeyFile: "user_ecdsa_p384"
@@ -214,6 +235,7 @@ final class CertificateAuthenticationMethodRealTests: XCTestCase {
     // MARK: - P521 Certificate Tests
 
     func testP521CertificateValidation() throws {
+
         let (privateKey, certificate) = try TestCertificateHelper.parseP521Certificate(
             certificateFile: "user_ecdsa_p521-cert.pub",
             privateKeyFile: "user_ecdsa_p521"
@@ -231,6 +253,7 @@ final class CertificateAuthenticationMethodRealTests: XCTestCase {
     // MARK: - Time-based Certificate Tests
 
     func testNotYetValidCertificate() throws {
+
         // SKIP TEST: Time-based validation tests require certificates with specific validity periods
         // The test certificates are generated with specific future timestamps that may not be reliable
         // The time validation logic is tested in CertificateSecurityValidationTests
@@ -240,6 +263,7 @@ final class CertificateAuthenticationMethodRealTests: XCTestCase {
     // MARK: - Critical Options Tests
 
     func testCertificateWithCriticalOptions() throws {
+
         // Generate a new Ed25519 private key for this test
         let privateKey = Curve25519.Signing.PrivateKey()
 
@@ -264,6 +288,7 @@ final class CertificateAuthenticationMethodRealTests: XCTestCase {
     // MARK: - Extensions Tests
 
     func testCertificateWithAllExtensions() throws {
+
         // Generate a new Ed25519 private key for this test
         let privateKey = Curve25519.Signing.PrivateKey()
 

Tests/CitadelTests/ECDSACertificateRealTests.swift

@@ -8,31 +8,45 @@ import NIOSSH
 /// Tests for ECDSA certificates using real certificates generated by ssh-keygen
 final class ECDSACertificateRealTests: XCTestCase {
 
-    override class func setUp() {
+    override func setUp() {
         super.setUp()
-        // Generate certificates dynamically for tests
-        do {
-            try SSHCertificateGenerator.ensureSSHKeygenAvailable()
-            try SSHCertificateGenerator.setUp()
-        } catch {
-            print("Failed to set up certificate generation: \(error)")
+        // Generate certificates dynamically for tests (only once)
+        if !SSHCertificateGenerator.hasAttemptedSetup {
+            SSHCertificateGenerator.hasAttemptedSetup = true
+            do {
+                try SSHCertificateGenerator.ensureSSHKeygenAvailable()
+                try SSHCertificateGenerator.setUp()
+                SSHCertificateGenerator.isSetupSuccessful = true
+            } catch {
+                SSHCertificateGenerator.setupError = error
+                SSHCertificateGenerator.isSetupSuccessful = false
+            }
+        }
+
+        // Check setup success for each test
+        if !SSHCertificateGenerator.isSetupSuccessful {
+            if let error = SSHCertificateGenerator.setupError {
+                XCTFail("Certificate generation setup failed: \(error)")
+            } else {
+                XCTFail("Certificate generation setup failed")
+            }
         }
     }
 
-    override class func tearDown() {
+    override func tearDown() {
         super.tearDown()
         // Clean up generated certificates
         do {
             try TestCertificateHelper.cleanUp()
         } catch {
-            print("Failed to clean up certificates: \(error)")
+            XCTFail("Certificate cleanup failed: \(error)")
         }
     }
 
     // MARK: - P256 Certificate Tests
 
     func testP256CertificateParsingWithRealCertificate() throws {
-        let (privateKey, certificate) = try TestCertificateHelper.parseP256Certificate(
+                let (privateKey, certificate) = try TestCertificateHelper.parseP256Certificate(
             certificateFile: "user_ecdsa_p256-cert.pub",
             privateKeyFile: "user_ecdsa_p256"
         )
@@ -50,7 +64,7 @@ final class ECDSACertificateRealTests: XCTestCase {
     }
 
     func testP256CertificateValidation() throws {
-        // Principal validation with fresh certificates
+                // Principal validation with fresh certificates
         let (_, certificate) = try TestCertificateHelper.parseP256Certificate(
             certificateFile: "user_ecdsa_p256-cert.pub",
             privateKeyFile: "user_ecdsa_p256"
@@ -79,7 +93,7 @@ final class ECDSACertificateRealTests: XCTestCase {
     // MARK: - P384 Certificate Tests
 
     func testP384CertificateParsingWithRealCertificate() throws {
-        let (privateKey, certificate) = try TestCertificateHelper.parseP384Certificate(
+                let (privateKey, certificate) = try TestCertificateHelper.parseP384Certificate(
             certificateFile: "user_ecdsa_p384-cert.pub",
             privateKeyFile: "user_ecdsa_p384"
         )
@@ -97,7 +111,7 @@ final class ECDSACertificateRealTests: XCTestCase {
     }
 
     func testP384CertificateMultiplePrincipals() throws {
-        let (_, certificate) = try TestCertificateHelper.parseP384Certificate(
+                let (_, certificate) = try TestCertificateHelper.parseP384Certificate(
             certificateFile: "user_ecdsa_p384-cert.pub",
             privateKeyFile: "user_ecdsa_p384"
         )
@@ -129,7 +143,7 @@ final class ECDSACertificateRealTests: XCTestCase {
     // MARK: - P521 Certificate Tests
 
     func testP521CertificateParsingWithRealCertificate() throws {
-        let (privateKey, certificate) = try TestCertificateHelper.parseP521Certificate(
+                let (privateKey, certificate) = try TestCertificateHelper.parseP521Certificate(
             certificateFile: "user_ecdsa_p521-cert.pub",
             privateKeyFile: "user_ecdsa_p521"
         )
@@ -149,7 +163,7 @@ final class ECDSACertificateRealTests: XCTestCase {
     // MARK: - Certificate Equality Tests
 
     func testCertificateEqualityWithRealCertificates() throws {
-        // Generate two P256 certificates with the same configuration
+                // Generate two P256 certificates with the same configuration
         let (_, cert1) = try TestCertificateHelper.parseP256Certificate(
             certificateFile: "user_ecdsa_p256-cert.pub",
             privateKeyFile: "user_ecdsa_p256"
@@ -180,7 +194,7 @@ final class ECDSACertificateRealTests: XCTestCase {
     // MARK: - Invalid Certificate Tests
 
     func testInvalidCertificateData() throws {
-        // Test with completely invalid data
+                // Test with completely invalid data
         let invalidData = Data("This is not a certificate".utf8)
         XCTAssertThrowsError(try NIOSSHCertificateLoader.loadFromBinaryData(invalidData)) { error in
             XCTAssertTrue(error is NIOSSHCertificateLoadingError)
@@ -197,7 +211,7 @@ final class ECDSACertificateRealTests: XCTestCase {
     }
 
     func testCertificateTimeValidation() throws {
-        // Generate a certificate with known validity period
+                // Generate a certificate with known validity period
         let (_, certificate) = try TestCertificateHelper.parseP256Certificate(
             certificateFile: "user_ecdsa_p256-cert.pub",
             privateKeyFile: "user_ecdsa_p256"
@@ -221,7 +235,7 @@ final class ECDSACertificateRealTests: XCTestCase {
     // MARK: - Key Size Tests
 
     func testAllCurveSizes() throws {
-        // Test that the public key sizes are correct for each curve
+                // Test that the public key sizes are correct for each curve
         let (_, p256Cert) = try TestCertificateHelper.parseP256Certificate(
             certificateFile: "user_ecdsa_p256-cert.pub",
             privateKeyFile: "user_ecdsa_p256"

Tests/CitadelTests/KeyTests.swift

@@ -355,8 +355,6 @@ final class KeyTests: XCTestCase {
         // Verify we can read it back
         let p256Parsed = try P256.Signing.PrivateKey(sshECDSA: p256SSH)
         // Check if public keys match
-        print("Original P256 public key: \(p256Key.publicKey.x963Representation.base64EncodedString())")
-        print("Parsed P256 public key: \(p256Parsed.publicKey.x963Representation.base64EncodedString())")
         XCTAssertEqual(p256Key.publicKey.x963Representation, p256Parsed.publicKey.x963Representation)
 
         // Test ECDSA P-384 key generation and export
@@ -408,20 +406,13 @@ final class KeyTests: XCTestCase {
             passphrase: passphrase
         )
 
-        // Debug: print the generated key
-        print("Generated encrypted key:")
-        print(ed25519Encrypted)
-
         // Should contain encryption markers in the base64 content, not the PEM wrapper
         let lines = ed25519Encrypted.split(separator: "\n")
         if lines.count > 2 {
             let base64Content = lines[1..<lines.count-1].joined(separator: "")
             if let decodedData = Data(base64Encoded: base64Content) {
                 // The decoded data starts with openssh-key-v1\0 and contains cipher and kdf strings
                 let decodedString = String(decoding: decodedData, as: UTF8.self)
-                print("Decoded binary contains openssh-key-v1: \(decodedString.contains("openssh-key-v1"))")
-                print("Decoded binary contains aes256-ctr: \(decodedString.contains("aes256-ctr"))")
-                print("Decoded binary contains bcrypt: \(decodedString.contains("bcrypt"))")
                 XCTAssertTrue(decodedString.contains("aes256-ctr") || decodedString.contains("aes128-ctr"))
                 XCTAssertTrue(decodedString.contains("bcrypt"))
             } else {

Tests/CitadelTests/SSHCertificateGenerator.swift

@@ -4,6 +4,15 @@ import XCTest
 /// Helper class to generate SSH certificates dynamically during test runs
 enum SSHCertificateGenerator {
 
+    /// Track whether setup was successful
+    static var isSetupSuccessful = false
+
+    /// Track setup error if any
+    static var setupError: Error?
+
+    /// Track whether setup has been attempted
+    static var hasAttemptedSetup = false
+
     /// Temporary directory for generated certificates
     static var tempDirectory: URL {
         FileManager.default.temporaryDirectory.appendingPathComponent("CitadelTestCerts-\(ProcessInfo.processInfo.processIdentifier)")
@@ -12,6 +21,7 @@ enum SSHCertificateGenerator {
     /// Setup the temporary directory
     static func setUp() throws {
         try FileManager.default.createDirectory(at: tempDirectory, withIntermediateDirectories: true)
+        isSetupSuccessful = true
     }
 
     /// Clean up the temporary directory