Feat/support passphrase and certificate authentication method (RSA excluded)#7
Merged
Feat/support passphrase and certificate authentication method (RSA excluded)#7
Conversation
- Updated RSA.PrivateKey to include prime factors (p, q) and iqmp for improved performance and security. - Implemented CRT parameters calculation for RSA private keys. - Enhanced the OpenSSHPrivateKey protocol to support public key retrieval and optional wrapping in composite strings. - Added methods for reading and writing SSH formatted data, including handling of optional parameters. - Improved key generation and representation for various key types (Ed25519, ECDSA, RSA) with support for passphrase encryption. - Added comprehensive tests for key generation, representation, and encryption/decryption processes across different key types.
- Implemented Ed25519 certificate public key handling in Ed25519.swift. - Created SSHCertificate.swift to define the structure and parsing logic for SSH certificates. - Updated OpenSSHKey.swift and SSHKeyTypeDetection.swift to include Ed25519 certificate types. - Added Ed25519CertificateTests.swift to validate parsing, serialization, and equality of Ed25519 certificates. - Enhanced KeyTests.swift to include Ed25519 certificate type detection and descriptions. - Ensured compatibility with OpenSSH certificate format in tests.
…omments, passphrases, and custom ciphers
- Implemented PEM representation for Insecure.RSA.PrivateKey and Curve25519.Signing.PrivateKey. - Added DER representation for Insecure.RSA.PrivateKey. - Enhanced SSHKeyGenerator to support PEM export for RSA and Ed25519 keys. - Created tests for PEM and DER round-trip conversions for Ed25519 and RSA keys. - Updated existing tests to reflect new PEM support in SSHKeyGenerator.
…pes and validation
…d ECDSA key types
…d real certificate tests
…ndom generation - Wrapped Security framework import with conditional compilation - Replaced SecRandomCopyBytes with Swift's built-in random API - Ensures RSA certificate functionality works on Linux in CI/CD
…rm compatibility - Removed Security framework import from RSA.swift and OpenSSHKey.swift - Replaced all SecRandomCopyBytes calls with Swift's built-in UInt8.random(in:) - Ensures the library works on all platforms including Linux - All tests pass with the cross-platform implementation
…compliance) - Added 'any' keyword to existential protocol type casts in SSHCert.swift - Fixes Swift 6 error: 'use of protocol as a type must be written any'
- Updated BigInt to version 5.7.0 and ColorizeSwift to version 1.7.0. - Added swift-nio-ssh package with version 0.3.5. - Updated swift-collections, swift-crypto, swift-log, swift-nio, and swift-system to their latest versions. - Enhanced ECDSACertificate, Ed25519, RSA, and other certificate handling to include original certificate data for serialization. - Refactored SSHAuthenticationMethod to support direct authentication without custom delegates. - Improved CertificateLoader to handle OpenSSH format certificates. - Added unit tests for new authentication methods and certificate handling.
* Add test certificate generation and loading utilities - Introduced `TestCertificateHelper` class for loading and parsing SSH certificates and private keys. - Implemented methods to parse Ed25519, P256, P384, P521, and RSA certificates. - Added a script `generate_test_certificates.sh` to automate the generation of test SSH certificates. - Created a `.gitignore` file for the test certificates directory to exclude private keys. - Added various test certificate files for different algorithms and scenarios, including expired and not yet valid certificates. * Update test certificates for various algorithms - Updated ECDSA P256 certificates with new public keys. - Updated ECDSA P384 certificates with new public keys. - Updated ECDSA P521 certificates with new public keys. - Updated Ed25519 certificates with new public keys. - Updated expired certificates with new public keys. - Updated limited principals certificates with new public keys. - Updated not yet valid certificates with new public keys. - Updated RSA certificates with new public keys. * feat: implement ASN.1 DER encoding for ECDSA signatures * feat: enhance SSH certificate constraints validation and error handling * feat: implement CA key verification and enhance certificate signature tests * Add CIDRMatcher for IPv4 and IPv6 support, implement PatternMatcher for wildcard matching - Enhanced CIDRMatcher to support both IPv4 and IPv6 address matching. - Introduced PatternMatcher for OpenSSH-compatible wildcard pattern matching. - Added comprehensive tests for CIDR matching, pattern matching, and validation. - Implemented user and hostname matching with support for negation and group patterns. - Validated CIDR lists and patterns to ensure compliance with expected formats. * feat: enhance principal validation to allow optional empty principals * feat: read nonce as the first field after key type in SSH certificate parsing * feat: add signature type extraction and validation for SSH certificates * fix: update original buffer handling for signature verification in SSHCertificate * feat: add RSA key length validation and tests for SSH certificates * feat: implement principal limit and no-touch-required extension handling in SSH certificates * Refactor SSH Certificate Tests to Use NIOSSH for Certificate Parsing - Updated RealCertificateTests to skip deprecated certificate parsing tests and utilize NIOSSH's native support for certificate handling. - Removed shell command execution for certificate generation and replaced with NIOSSHCertificateLoader for parsing certificates. - Adjusted TestCertificateHelper methods to return NIOSSHCertifiedPublicKey instead of specific key types. - Modified SSHCertificateRealTests to skip tests that rely on expired certificates and updated assertions to align with NIOSSH's structure. - Ensured all Ed25519, ECDSA, and RSA certificate parsing methods in TestCertificateHelper now utilize NIOSSH for consistency and reliability. * Remove test certificates and associated scripts - Deleted the .gitignore file for test certificates. - Removed public key files for various algorithms (ECDSA P256, P384, P521; Ed25519; RSA). - Deleted the script for generating test certificates. - Removed host and user certificate files, including those with special conditions (expired, not yet valid, limited principals). * refactor: make IP address validation cross-platform and improve code clarity - Remove dependency on Apple's Network framework for Linux compatibility - Implement custom IPv4/IPv6 parsing and validation using pure Swift - Replace magic numbers with named constants: - INET6_ADDRSTRLEN (46) for IPv6 address max length - MAX_CIDR_PREFIX_LENGTH (4) for CIDR notation - Test constants (MATCH, NO_MATCH, NEGATED_MATCH, ERROR) - Improve defensive programming with explicit switch cases for bit masking - Add comprehensive cross-platform IP validation tests - Support IPv6 short forms, IPv4-mapped addresses, and zone IDs This change enables the library to build and run on Linux and other non-Apple platforms while maintaining full SSH certificate validation functionality. * build: remove TestCertificates resource reference from Package.swift Remove the TestCertificates resource copy declaration as test certificates and associated scripts have been removed from the project. * refactor: improve IP address validation using getaddrinfo for robustness * refactor: remove deprecated certificate tests and update to NIOSSH integration
There was a problem hiding this comment.
Pull Request Overview
This pull request introduces significant enhancements to SSH key handling, testing infrastructure, and documentation in the Citadel project. The changes focus on adding comprehensive support for passphrase-protected keys, improving ECDSA key serialization, implementing cross-platform pattern matching utilities, and establishing a robust testing framework using dynamically generated SSH certificates.
- Addition of pattern matching utilities and address validation for OpenSSH-compatible matching
- Implementation of comprehensive testing infrastructure with dynamic certificate generation
- Enhancement of ECDSA key serialization with proper padding and SSH string handling
- Introduction of high-level SSH key generation API with passphrase support
Reviewed Changes
Copilot reviewed 38 out of 40 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| Tests/CitadelTests/TestCertificateHelper.swift | Helper class for loading and parsing real SSH certificates generated by ssh-keygen |
| Tests/CitadelTests/SSHKeyGeneratorTests.swift | Comprehensive tests for SSH key generation API with RSA, Ed25519, and ECDSA support |
| Tests/CitadelTests/SSHCertificateRealTests.swift | Tests using real SSH certificates generated by ssh-keygen for certificate parsing validation |
| Tests/CitadelTests/SSHCertificateGenerator.swift | Certificate generation utility for creating test certificates dynamically during test runs |
| Tests/CitadelTests/RealCertificateTests.swift | Basic certificate authentication tests using generated certificates |
| Tests/CitadelTests/PublicKeyPEMTests.swift | Tests for public key PEM format export functionality |
| Tests/CitadelTests/PatternMatcherTests.swift | Comprehensive tests for OpenSSH-compatible pattern matching functionality |
| Tests/CitadelTests/NonceFixTest.swift | Simple test for Ed25519 certificate parsing through NIOSSH |
| Tests/CitadelTests/NIOSSHCertificateAuthTests.swift | Tests for NIOSSH certificate authentication methods |
| Tests/CitadelTests/KeyTests.swift | Enhanced key tests with passphrase support and certificate key type detection |
| Tests/CitadelTests/Ed25519PEMTests.swift | Tests for Ed25519 PEM format support with private/public key round-trip validation |
| Tests/CitadelTests/ECDSAKeyTests.swift | Enhanced ECDSA key tests with PEM support and format conversion |
| Tests/CitadelTests/ECDSACertificateRealTests.swift | Tests for ECDSA certificates using real certificates generated by ssh-keygen |
| Tests/CitadelTests/CrossPlatformIPTests.swift | Tests for cross-platform IP address parsing and CIDR matching |
| Tests/CitadelTests/CertificateAuthenticationMethodRealTests.swift | Tests for certificate authentication methods using real SSH certificates |
| Tests/CitadelTests/AddressValidatorTests.swift | Tests for OpenSSH-compatible address matching functionality |
| Sources/Citadel/Utilities/PatternMatcher.swift | OpenSSH-compatible pattern matching implementation with wildcard support |
| Sources/Citadel/Utilities/CIDRMatcher.swift | Cross-platform CIDR matching utility supporting both IPv4 and IPv6 |
| Sources/Citadel/Utilities/AddressValidator.swift | Enhanced address validation matching OpenSSH's addr_match_list() behavior |
| Sources/Citadel/SignatureVerification+NIOSSH.swift | Signature verification extensions for NIOSSH integration |
| Sources/Citadel/SSHKeyTypeDetection.swift | Enhanced SSH key type detection with certificate type support |
Comments suppressed due to low confidence (4)
Tests/CitadelTests/SSHCertificateRealTests.swift:47
- [nitpick] The underscore '_' for the unused privateKey parameter makes the code less readable. Consider using a descriptive name like 'privateKey' with a comment or explicit discard.
let (_, certificate) = try TestCertificateHelper.parseEd25519Certificate(
Tests/CitadelTests/PatternMatcherTests.swift:57
- This test expects false but the comment suggests it should fail because there's nothing between dots. This assertion might be incorrect - '192.168.1' should not match '192.168..' because it has insufficient segments.
XCTAssertFalse(PatternMatcher.match("192.168.1", pattern: "192.168.*.*"))
nedithgar
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request introduces enhancements to SSH key handling, testing, and documentation in the Citadel project. Key updates include the addition of certificate-based SSH testing in CI workflows, improvements to ECDSA key serialization, and a new high-level API for SSH key generation. These changes improve testing coverage, usability, and adherence to OpenSSH standards.
CI Workflow Enhancements:
2223for certificate-based SSH testing in thetest.ymlworkflow and configured the SSH server to trust a generated CA key. This includes generating various SSH key types and certificates for testing [1] [2] [3].openssh-client) and added steps to automate SSH certificate generation and server configuration.SSH Key Handling Improvements:
New API for SSH Key Generation:
Documentation Updates:
README.mdto document the SSH key generation API, including examples for generating and exporting keys.These changes collectively enhance the project's functionality, testing robustness, and developer experience.