feat: ECDSA key handling and ByteBuffer conversions for OpenSSH format#6
Merged
feat: ECDSA key handling and ByteBuffer conversions for OpenSSH format#6
Conversation
…mprove SSH bignum format handling
There was a problem hiding this comment.
Pull Request Overview
This pull request adds comprehensive ECDSA key support for OpenSSH format, enabling parsing, serialization, and validation of P-256, P-384, and P-521 elliptic curve keys. The implementation provides a complete ECDSA key handling solution with proper SSH format compliance.
Key changes include:
- ECDSA key type support with ByteBuffer serialization/deserialization for all three curve types
- SSH bignum handling methods for proper numeric data encoding/decoding
- Public API extensions for creating ECDSA keys from OpenSSH formatted strings
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| Sources/Citadel/Algorithms/ECDSA.swift | Implements ByteBufferConvertible and OpenSSHPrivateKey conformances for P256/P384/P521 keys with SSH format support |
| Sources/Citadel/ByteBufferHelpers.swift | Adds SSH bignum read/write methods and string helper functions for key serialization |
| Sources/Citadel/OpenSSHKey.swift | Makes OpenSSH types public and adds ECDSA key type enumerations |
| Sources/Citadel/SSHCert.swift | Provides convenient initializers for creating ECDSA keys from OpenSSH strings |
| Tests/CitadelTests/ECDSAKeyTests.swift | Comprehensive test suite covering key parsing, validation, and error handling scenarios |
Comments suppressed due to low confidence (1)
Tests/CitadelTests/ECDSAKeyTests.swift:90
- This test uses an empty string which doesn't properly test encrypted key parsing. Consider using a real encrypted key or testing the specific error type that should be thrown for invalid formats.
XCTAssertThrowsError(try P256.Signing.PrivateKey(sshECDSA: "", decryptionKey: passphrase.data(using: .utf8)))
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 support for handling ECDSA keys (P-256, P-384, and P-521) in OpenSSH format by adding parsing, serialization, and validation functionalities. It also includes test coverage for these features to ensure correctness. Below is a summary of the most important changes:
ECDSA Key Support
ByteBufferConvertibleconformances forP256,P384, andP521private and public keys to enable reading and writing keys in OpenSSH format. This includes handling curve-specific constraints and key serialization requirements. (Sources/Citadel/Algorithms/ECDSA.swift, Sources/Citadel/Algorithms/ECDSA.swiftR1-R265)OpenSSHPrivateKeyconformances forP256,P384, andP521private keys, defining key prefixes and types for OpenSSH integration. (Sources/Citadel/Algorithms/ECDSA.swift, Sources/Citadel/Algorithms/ECDSA.swiftR1-R265)ByteBuffer Enhancements
readSSHBignumandwriteSSHBignummethods toByteBufferfor reading and writing SSH bignum values, which are used in ECDSA key serialization. (Sources/Citadel/ByteBufferHelpers.swift, Sources/Citadel/ByteBufferHelpers.swiftR152-R171)BigIntto support SSH bignum operations. (Sources/Citadel/ByteBufferHelpers.swift, Sources/Citadel/ByteBufferHelpers.swiftR3)OpenSSH Key Type Updates
OpenSSH.KeyTypeandOpenSSHenums public and added new key types for ECDSA (ecdsaP256,ecdsaP384,ecdsaP521). (Sources/Citadel/OpenSSHKey.swift, [1] [2]ECDSA Key Parsing APIs
P256,P384, andP521private keys for creating keys from OpenSSH private key strings or data, with optional decryption support. (Sources/Citadel/SSHCert.swift, Sources/Citadel/SSHCert.swiftR124-R192)Unit Tests
ECDSAKeyTeststo validate parsing, serialization, and signing functionality for P-256, P-384, and P-521 private keys. Tests include scenarios for valid keys, encrypted keys, invalid formats, and curve mismatches. (Tests/CitadelTests/ECDSAKeyTests.swift, Tests/CitadelTests/ECDSAKeyTests.swiftR1-R122)