From a6261ead33663c5295ad1477bd9a7b457dec1355 Mon Sep 17 00:00:00 2001 From: Nedithgar Amirka <150447520+nedithgar@users.noreply.github.com> Date: Tue, 29 Jul 2025 22:58:05 +0800 Subject: [PATCH 1/5] feat: implement ECDSA key handling and ByteBuffer conversions for OpenSSH format --- Sources/Citadel/Algorithms/ECDSA.swift | 265 ++++++++++++++++++++++++ Sources/Citadel/ByteBufferHelpers.swift | 21 ++ Sources/Citadel/OpenSSHKey.swift | 7 +- Sources/Citadel/SSHCert.swift | 69 ++++++ Tests/CitadelTests/ECDSAKeyTests.swift | 122 +++++++++++ 5 files changed, 482 insertions(+), 2 deletions(-) create mode 100644 Sources/Citadel/Algorithms/ECDSA.swift create mode 100644 Tests/CitadelTests/ECDSAKeyTests.swift diff --git a/Sources/Citadel/Algorithms/ECDSA.swift b/Sources/Citadel/Algorithms/ECDSA.swift new file mode 100644 index 0000000..cd6f1f5 --- /dev/null +++ b/Sources/Citadel/Algorithms/ECDSA.swift @@ -0,0 +1,265 @@ +import Foundation +import Crypto +import _CryptoExtras +import NIOCore +import BigInt + +extension P256.Signing.PrivateKey: ByteBufferConvertible { + public static func read(consuming buffer: inout ByteBuffer) throws -> Self { + guard + let curveName = buffer.readSSHString(), + let _ = buffer.readSSHBuffer(), // public key - we don't need it for reconstruction + let privateKeyData = buffer.readSSHBignum() + else { + throw InvalidOpenSSHKey.invalidLayout + } + + guard curveName == "nistp256" else { + throw InvalidOpenSSHKey.invalidLayout + } + + // ECDSA private keys are stored as bignums in OpenSSH format + // P256 private keys should be exactly 32 bytes (may have leading zero) + guard privateKeyData.count >= 32 && privateKeyData.count <= 33 else { + throw InvalidOpenSSHKey.invalidLayout + } + + // Remove leading zero if present + let keyData = privateKeyData.count == 33 && privateKeyData[0] == 0 ? + privateKeyData.dropFirst() : privateKeyData + + return try P256.Signing.PrivateKey(rawRepresentation: keyData) + } + + public func write(to buffer: inout ByteBuffer) -> Int { + let start = buffer.writerIndex + buffer.writeSSHString("nistp256") + + let publicKey = self.publicKey + var publicKeyBuffer = ByteBuffer() + publicKeyBuffer.writeBytes(publicKey.x963Representation) + buffer.writeSSHString(&publicKeyBuffer) + + // Write private key as bignum (matching OpenSSH format) + let privateKeyData = self.rawRepresentation + let bignum = BigInt(privateKeyData) + buffer.writeSSHBignum(bignum) + + return buffer.writerIndex - start + } +} + +extension P384.Signing.PrivateKey: ByteBufferConvertible { + public static func read(consuming buffer: inout ByteBuffer) throws -> Self { + guard + let curveName = buffer.readSSHString(), + let _ = buffer.readSSHBuffer(), // public key - we don't need it for reconstruction + let privateKeyData = buffer.readSSHBignum() + else { + throw InvalidOpenSSHKey.invalidLayout + } + + guard curveName == "nistp384" else { + throw InvalidOpenSSHKey.invalidLayout + } + + // ECDSA private keys are stored as bignums in OpenSSH format + // P384 private keys should be exactly 48 bytes (may have leading zero) + guard privateKeyData.count >= 48 && privateKeyData.count <= 49 else { + throw InvalidOpenSSHKey.invalidLayout + } + + // Remove leading zero if present + let keyData = privateKeyData.count == 49 && privateKeyData[0] == 0 ? + privateKeyData.dropFirst() : privateKeyData + + return try P384.Signing.PrivateKey(rawRepresentation: Data(keyData)) + } + + public func write(to buffer: inout ByteBuffer) -> Int { + let start = buffer.writerIndex + buffer.writeSSHString("nistp384") + + let publicKey = self.publicKey + var publicKeyBuffer = ByteBuffer() + publicKeyBuffer.writeBytes(publicKey.x963Representation) + buffer.writeSSHString(&publicKeyBuffer) + + // Write private key as bignum (matching OpenSSH format) + let privateKeyData = self.rawRepresentation + let bignum = BigInt(privateKeyData) + buffer.writeSSHBignum(bignum) + + return buffer.writerIndex - start + } +} + +extension P521.Signing.PrivateKey: ByteBufferConvertible { + public static func read(consuming buffer: inout ByteBuffer) throws -> Self { + guard + let curveName = buffer.readSSHString(), + let _ = buffer.readSSHBuffer(), // public key - we don't need it for reconstruction + let privateKeyData = buffer.readSSHBignum() + else { + throw InvalidOpenSSHKey.invalidLayout + } + + guard curveName == "nistp521" else { + throw InvalidOpenSSHKey.invalidLayout + } + + // ECDSA private keys are stored as bignums in OpenSSH format + // P521 private keys should be exactly 66 bytes (may have leading zero) + guard privateKeyData.count >= 66 && privateKeyData.count <= 67 else { + throw InvalidOpenSSHKey.invalidLayout + } + + // Remove leading zero if present + let keyData = privateKeyData.count == 67 && privateKeyData[0] == 0 ? + privateKeyData.dropFirst() : privateKeyData + + return try P521.Signing.PrivateKey(rawRepresentation: Data(keyData)) + } + + public func write(to buffer: inout ByteBuffer) -> Int { + let start = buffer.writerIndex + buffer.writeSSHString("nistp521") + + let publicKey = self.publicKey + var publicKeyBuffer = ByteBuffer() + publicKeyBuffer.writeBytes(publicKey.x963Representation) + buffer.writeSSHString(&publicKeyBuffer) + + // Write private key as bignum (matching OpenSSH format) + let privateKeyData = self.rawRepresentation + let bignum = BigInt(privateKeyData) + buffer.writeSSHBignum(bignum) + + return buffer.writerIndex - start + } +} + +// Public key types for ECDSA +extension P256.Signing.PublicKey: ByteBufferConvertible { + public static func read(consuming buffer: inout ByteBuffer) throws -> Self { + // First read the curve name + guard let curveName = buffer.readSSHString() else { + throw InvalidOpenSSHKey.invalidLayout + } + + guard curveName == "nistp256" else { + throw InvalidOpenSSHKey.invalidLayout + } + + // Then read the EC point data + guard let pointData = buffer.readSSHBuffer() else { + throw InvalidOpenSSHKey.invalidLayout + } + + let pointBytes = pointData.getBytes(at: 0, length: pointData.readableBytes) ?? [] + guard pointBytes.first == 0x04 else { // Uncompressed point + throw InvalidOpenSSHKey.invalidLayout + } + + return try P256.Signing.PublicKey(x963Representation: pointBytes) + } + + public func write(to buffer: inout ByteBuffer) -> Int { + let start = buffer.writerIndex + var publicKeyBuffer = ByteBuffer() + publicKeyBuffer.writeBytes(self.x963Representation) + buffer.writeSSHString(&publicKeyBuffer) + return buffer.writerIndex - start + } +} + +extension P384.Signing.PublicKey: ByteBufferConvertible { + public static func read(consuming buffer: inout ByteBuffer) throws -> Self { + // First read the curve name + guard let curveName = buffer.readSSHString() else { + throw InvalidOpenSSHKey.invalidLayout + } + + guard curveName == "nistp384" else { + throw InvalidOpenSSHKey.invalidLayout + } + + // Then read the EC point data + guard let pointData = buffer.readSSHBuffer() else { + throw InvalidOpenSSHKey.invalidLayout + } + + let pointBytes = pointData.getBytes(at: 0, length: pointData.readableBytes) ?? [] + guard pointBytes.first == 0x04 else { // Uncompressed point + throw InvalidOpenSSHKey.invalidLayout + } + + return try P384.Signing.PublicKey(x963Representation: pointBytes) + } + + public func write(to buffer: inout ByteBuffer) -> Int { + let start = buffer.writerIndex + var publicKeyBuffer = ByteBuffer() + publicKeyBuffer.writeBytes(self.x963Representation) + buffer.writeSSHString(&publicKeyBuffer) + return buffer.writerIndex - start + } +} + +extension P521.Signing.PublicKey: ByteBufferConvertible { + public static func read(consuming buffer: inout ByteBuffer) throws -> Self { + // First read the curve name + guard let curveName = buffer.readSSHString() else { + throw InvalidOpenSSHKey.invalidLayout + } + + guard curveName == "nistp521" else { + throw InvalidOpenSSHKey.invalidLayout + } + + // Then read the EC point data + guard let pointData = buffer.readSSHBuffer() else { + throw InvalidOpenSSHKey.invalidLayout + } + + let pointBytes = pointData.getBytes(at: 0, length: pointData.readableBytes) ?? [] + guard pointBytes.first == 0x04 else { // Uncompressed point + throw InvalidOpenSSHKey.invalidLayout + } + + return try P521.Signing.PublicKey(x963Representation: pointBytes) + } + + public func write(to buffer: inout ByteBuffer) -> Int { + let start = buffer.writerIndex + var publicKeyBuffer = ByteBuffer() + publicKeyBuffer.writeBytes(self.x963Representation) + buffer.writeSSHString(&publicKeyBuffer) + return buffer.writerIndex - start + } +} + +// OpenSSHPrivateKey conformances +extension P256.Signing.PrivateKey: OpenSSHPrivateKey { + public typealias PublicKey = P256.Signing.PublicKey + + public static var publicKeyPrefix: String { "ecdsa-sha2-nistp256" } + public static var privateKeyPrefix: String { "ecdsa-sha2-nistp256" } + public static var keyType: OpenSSH.KeyType { .ecdsaP256 } +} + +extension P384.Signing.PrivateKey: OpenSSHPrivateKey { + public typealias PublicKey = P384.Signing.PublicKey + + public static var publicKeyPrefix: String { "ecdsa-sha2-nistp384" } + public static var privateKeyPrefix: String { "ecdsa-sha2-nistp384" } + public static var keyType: OpenSSH.KeyType { .ecdsaP384 } +} + +extension P521.Signing.PrivateKey: OpenSSHPrivateKey { + public typealias PublicKey = P521.Signing.PublicKey + + public static var publicKeyPrefix: String { "ecdsa-sha2-nistp521" } + public static var privateKeyPrefix: String { "ecdsa-sha2-nistp521" } + public static var keyType: OpenSSH.KeyType { .ecdsaP521 } +} \ No newline at end of file diff --git a/Sources/Citadel/ByteBufferHelpers.swift b/Sources/Citadel/ByteBufferHelpers.swift index 355388a..96e28a7 100644 --- a/Sources/Citadel/ByteBufferHelpers.swift +++ b/Sources/Citadel/ByteBufferHelpers.swift @@ -1,5 +1,6 @@ import NIO import Foundation +import BigInt extension ByteBuffer { mutating func writeSFTPDate(_ date: Date) { @@ -148,4 +149,24 @@ extension ByteBuffer { moveReaderIndex(forwardBy: 4 + Int(length)) return slice } + + mutating func readSSHBignum() -> Data? { + guard let buffer = readSSHBuffer() else { + return nil + } + + return buffer.getData(at: 0, length: buffer.readableBytes) + } + + mutating func writeSSHBignum(_ bignum: BigInt) { + var data = bignum.serialize() + + // SSH bignum format: prepend zero byte if MSB is set + if !data.isEmpty && (data[0] & 0x80) != 0 { + data.insert(0, at: 0) + } + + writeInteger(UInt32(data.count)) + writeBytes(data) + } } diff --git a/Sources/Citadel/OpenSSHKey.swift b/Sources/Citadel/OpenSSHKey.swift index ae263a9..e12d134 100644 --- a/Sources/Citadel/OpenSSHKey.swift +++ b/Sources/Citadel/OpenSSHKey.swift @@ -183,7 +183,7 @@ extension ByteBuffer { } } -enum OpenSSH { +public enum OpenSSH { enum KeyError: Error { case missingDecryptionKey, cryptoError } @@ -284,9 +284,12 @@ enum OpenSSH { } } - enum KeyType: String { + public enum KeyType: String { case sshRSA = "ssh-rsa" case sshED25519 = "ssh-ed25519" + case ecdsaP256 = "ecdsa-sha2-nistp256" + case ecdsaP384 = "ecdsa-sha2-nistp384" + case ecdsaP521 = "ecdsa-sha2-nistp521" } struct PrivateKey { diff --git a/Sources/Citadel/SSHCert.swift b/Sources/Citadel/SSHCert.swift index 881862a..314317f 100644 --- a/Sources/Citadel/SSHCert.swift +++ b/Sources/Citadel/SSHCert.swift @@ -121,3 +121,72 @@ extension Insecure.RSA.PrivateKey: OpenSSHPrivateKey { self.init(privateExponent: privateExponent, publicExponent: publicExponent, modulus: modulus) } } + +extension P256.Signing.PrivateKey { + /// Creates a new P256 private key from an OpenSSH private key string. + /// - Parameters: + /// - key: The OpenSSH private key string. + /// - decryptionKey: The key to decrypt the private key with, if any. + public init(sshECDSA data: Data, decryptionKey: Data? = nil) throws { + if let string = String(data: data, encoding: .utf8) { + try self.init(sshECDSA: string, decryptionKey: decryptionKey) + } else { + throw InvalidOpenSSHKey.invalidUTF8String + } + } + + /// Creates a new P256 private key from an OpenSSH private key string. + /// - Parameters: + /// - key: The OpenSSH private key string. + /// - decryptionKey: The key to decrypt the private key with, if any. + public init(sshECDSA key: String, decryptionKey: Data? = nil) throws { + let privateKey = try OpenSSH.PrivateKey(string: key, decryptionKey: decryptionKey).privateKey + try self.init(rawRepresentation: privateKey.rawRepresentation) + } +} + +extension P384.Signing.PrivateKey { + /// Creates a new P384 private key from an OpenSSH private key string. + /// - Parameters: + /// - key: The OpenSSH private key string. + /// - decryptionKey: The key to decrypt the private key with, if any. + public init(sshECDSA data: Data, decryptionKey: Data? = nil) throws { + if let string = String(data: data, encoding: .utf8) { + try self.init(sshECDSA: string, decryptionKey: decryptionKey) + } else { + throw InvalidOpenSSHKey.invalidUTF8String + } + } + + /// Creates a new P384 private key from an OpenSSH private key string. + /// - Parameters: + /// - key: The OpenSSH private key string. + /// - decryptionKey: The key to decrypt the private key with, if any. + public init(sshECDSA key: String, decryptionKey: Data? = nil) throws { + let privateKey = try OpenSSH.PrivateKey(string: key, decryptionKey: decryptionKey).privateKey + try self.init(rawRepresentation: privateKey.rawRepresentation) + } +} + +extension P521.Signing.PrivateKey { + /// Creates a new P521 private key from an OpenSSH private key string. + /// - Parameters: + /// - key: The OpenSSH private key string. + /// - decryptionKey: The key to decrypt the private key with, if any. + public init(sshECDSA data: Data, decryptionKey: Data? = nil) throws { + if let string = String(data: data, encoding: .utf8) { + try self.init(sshECDSA: string, decryptionKey: decryptionKey) + } else { + throw InvalidOpenSSHKey.invalidUTF8String + } + } + + /// Creates a new P521 private key from an OpenSSH private key string. + /// - Parameters: + /// - key: The OpenSSH private key string. + /// - decryptionKey: The key to decrypt the private key with, if any. + public init(sshECDSA key: String, decryptionKey: Data? = nil) throws { + let privateKey = try OpenSSH.PrivateKey(string: key, decryptionKey: decryptionKey).privateKey + try self.init(rawRepresentation: privateKey.rawRepresentation) + } +} diff --git a/Tests/CitadelTests/ECDSAKeyTests.swift b/Tests/CitadelTests/ECDSAKeyTests.swift new file mode 100644 index 0000000..a6c4c0d --- /dev/null +++ b/Tests/CitadelTests/ECDSAKeyTests.swift @@ -0,0 +1,122 @@ +import XCTest +@testable import Citadel +import Crypto +import NIO + +final class ECDSAKeyTests: XCTestCase { + func testParseP256PrivateKey() throws { + // Real key generated with: ssh-keygen -t ecdsa -b 256 -f test_p256 -N "" -C "test@example.com" + let ecdsaP256PrivateKey = """ + -----BEGIN OPENSSH PRIVATE KEY----- + b3BlbnNzaC1rZXktdjEAAAAABG5vbmUAAAAEbm9uZQAAAAAAAAABAAAAaAAAABNlY2RzYS + 1zaGEyLW5pc3RwMjU2AAAACG5pc3RwMjU2AAAAQQRb9jp43IDOWYynle225gPMBkJ9rHil + TMAT7B215TmfXDVz/8OlZWInToGcipnuqZsixNtSgz5i4LvRInWV9DpPAAAAsLckTg+3JE + 4PAAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBFv2OnjcgM5ZjKeV + 7bbmA8wGQn2seKVMwBPsHbXlOZ9cNXP/w6VlYidOgZyKme6pmyLE21KDPmLgu9EidZX0Ok + 8AAAAhAKRCzvqPb3JF0UL2cUef8JaW8Hehgppaw/FFDcpJjfAEAAAAEHRlc3RAZXhhbXBs + ZS5jb20BAgMEBQYH + -----END OPENSSH PRIVATE KEY----- + """ + + let privateKey = try P256.Signing.PrivateKey(sshECDSA: ecdsaP256PrivateKey) + + // Verify we can create a public key from it + let publicKey = privateKey.publicKey + + // Verify key can be used for signing + let signature = try privateKey.signature(for: "test".data(using: .utf8)!) + XCTAssertTrue(publicKey.isValidSignature(signature, for: "test".data(using: .utf8)!)) + } + + func testParseP384PrivateKey() throws { + // Real key generated with: ssh-keygen -t ecdsa -b 384 -f test_p384 -N "" -C "test@example.com" + let ecdsaP384PrivateKey = """ + -----BEGIN OPENSSH PRIVATE KEY----- + b3BlbnNzaC1rZXktdjEAAAAABG5vbmUAAAAEbm9uZQAAAAAAAAABAAAAiAAAABNlY2RzYS + 1zaGEyLW5pc3RwMzg0AAAACG5pc3RwMzg0AAAAYQSYhaUzBlqml5TxqOQd6iOoXqC1tnej + LDoUBk9NH7KtZGB7RQb9ygcdpxNO4MRPG4/HXq9XkP/jex6y4epbLsIAIGUb+5+BFKV2qZ + aBGhajKAqm4cZdISWluLOiVbIAi6kAAADgdSrYt3Uq2LcAAAATZWNkc2Etc2hhMi1uaXN0 + cDM4NAAAAAhuaXN0cDM4NAAAAGEEmIWlMwZappeU8ajkHeojqF6gtbZ3oyw6FAZPTR+yrW + Rge0UG/coHHacTTuDETxuPx16vV5D/43sesuHqWy7CACBlG/ufgRSldqmWgRoWoygKpuHG + XSElpbizolWyAIupAAAAMQD2L6H07VKNLNRJE/N0Gi8xCSfHHmNCbAPMl2om+p/gonjod7 + m25VLSmR/qCCfnrBcAAAAQdGVzdEBleGFtcGxlLmNvbQECAwQFBgc= + -----END OPENSSH PRIVATE KEY----- + """ + + let privateKey = try P384.Signing.PrivateKey(sshECDSA: ecdsaP384PrivateKey) + + // Verify we can create a public key from it + let publicKey = privateKey.publicKey + + // Verify key can be used for signing + let signature = try privateKey.signature(for: "test".data(using: .utf8)!) + XCTAssertTrue(publicKey.isValidSignature(signature, for: "test".data(using: .utf8)!)) + } + + func testParseP521PrivateKey() throws { + // Real key generated with: ssh-keygen -t ecdsa -b 521 -f test_p521 -N "" -C "test@example.com" + let ecdsaP521PrivateKey = """ + -----BEGIN OPENSSH PRIVATE KEY----- + b3BlbnNzaC1rZXktdjEAAAAABG5vbmUAAAAEbm9uZQAAAAAAAAABAAAArAAAABNlY2RzYS + 1zaGEyLW5pc3RwNTIxAAAACG5pc3RwNTIxAAAAhQQALEP7/ff53UCXKnQ8bA7WbdUog93Z + 5jNVLMERhnh9ZNH3ceUbzSE48vHvC/ojRUa+KIt+QFl98oEHQ5/MjeKgWtEBABElKi5JYD + EYVSbc1po7l7fEjsYWhmBKVKr2l486sQQJbWJRF1qNxmMDDhUgc/MoGnSvwrGjTInZWKle + 0Lc42LIAAAEQHn2sUR59rFEAAAATZWNkc2Etc2hhMi1uaXN0cDUyMQAAAAhuaXN0cDUyMQ + AAAIUEACxD+/33+d1Alyp0PGwO1m3VKIPd2eYzVSzBEYZ4fWTR93HlG80hOPLx7wv6I0VG + viiLfkBZffKBB0OfzI3ioFrRAQARJSouSWAxGFUm3NaaO5e3xI7GFoZgSlSq9pePOrEECW + 1iURdajcZjAw4VIHPzKBp0r8Kxo0yJ2VipXtC3ONiyAAAAQgHDUj3BKxYlZPbb7qPlhrJF + 0yHeOiyKWeLg+Qr543AXtuGKYWmnq/ENUmgvjzFlkuN+2Y0qm4mNSpUtDelbkyZmwwAAAB + B0ZXN0QGV4YW1wbGUuY29tAQI= + -----END OPENSSH PRIVATE KEY----- + """ + + let privateKey = try P521.Signing.PrivateKey(sshECDSA: ecdsaP521PrivateKey) + + // Verify we can create a public key from it + let publicKey = privateKey.publicKey + + // Verify key can be used for signing + let signature = try privateKey.signature(for: "test".data(using: .utf8)!) + XCTAssertTrue(publicKey.isValidSignature(signature, for: "test".data(using: .utf8)!)) + } + + func testParseEncryptedP256PrivateKey() throws { + // Create a test encrypted key by generating one + let originalKey = P256.Signing.PrivateKey() + let passphrase = "testpassphrase" + + // We would need to implement key serialization to test encrypted keys + // For now, we'll test that the API exists + XCTAssertThrowsError(try P256.Signing.PrivateKey(sshECDSA: "", decryptionKey: passphrase.data(using: .utf8))) + } + + func testInvalidKeyFormat() throws { + let invalidKey = """ + -----BEGIN OPENSSH PRIVATE KEY----- + aW52YWxpZCBrZXkgZGF0YQ== + -----END OPENSSH PRIVATE KEY----- + """ + + XCTAssertThrowsError(try P256.Signing.PrivateKey(sshECDSA: invalidKey)) + } + + func testWrongCurveKey() throws { + // P-384 key attempting to be parsed as P-256 + // Real key generated with: ssh-keygen -t ecdsa -b 384 + let ecdsaP384PrivateKey = """ + -----BEGIN OPENSSH PRIVATE KEY----- + b3BlbnNzaC1rZXktdjEAAAAABG5vbmUAAAAEbm9uZQAAAAAAAAABAAAAiAAAABNlY2RzYS + 1zaGEyLW5pc3RwMzg0AAAACG5pc3RwMzg0AAAAYQSYhaUzBlqml5TxqOQd6iOoXqC1tnej + LDoUBk9NH7KtZGB7RQb9ygcdpxNO4MRPG4/HXq9XkP/jex6y4epbLsIAIGUb+5+BFKV2qZ + aBGhajKAqm4cZdISWluLOiVbIAi6kAAADgdSrYt3Uq2LcAAAATZWNkc2Etc2hhMi1uaXN0 + cDM4NAAAAAhuaXN0cDM4NAAAAGEEmIWlMwZappeU8ajkHeojqF6gtbZ3oyw6FAZPTR+yrW + Rge0UG/coHHacTTuDETxuPx16vV5D/43sesuHqWy7CACBlG/ufgRSldqmWgRoWoygKpuHG + XSElpbizolWyAIupAAAAMQD2L6H07VKNLNRJE/N0Gi8xCSfHHmNCbAPMl2om+p/gonjod7 + m25VLSmR/qCCfnrBcAAAAQdGVzdEBleGFtcGxlLmNvbQECAwQFBgc= + -----END OPENSSH PRIVATE KEY----- + """ + + // This should fail because the key is P-384 but we're trying to parse as P-256 + XCTAssertThrowsError(try P256.Signing.PrivateKey(sshECDSA: ecdsaP384PrivateKey)) + } +} \ No newline at end of file From f10a3faf62200faceaba31faf1da6fd0c7270ce5 Mon Sep 17 00:00:00 2001 From: Nedithgar Amirka <150447520+nedithgar@users.noreply.github.com> Date: Tue, 29 Jul 2025 23:17:13 +0800 Subject: [PATCH 2/5] feat: add writeSSHString methods to ByteBuffer for improved SSH data handling --- Sources/Citadel/Algorithms/ECDSA.swift | 49 +++++++++++-------------- Sources/Citadel/ByteBufferHelpers.swift | 23 +++++++++++- 2 files changed, 44 insertions(+), 28 deletions(-) diff --git a/Sources/Citadel/Algorithms/ECDSA.swift b/Sources/Citadel/Algorithms/ECDSA.swift index cd6f1f5..4be7b5f 100644 --- a/Sources/Citadel/Algorithms/ECDSA.swift +++ b/Sources/Citadel/Algorithms/ECDSA.swift @@ -4,6 +4,19 @@ import _CryptoExtras import NIOCore import BigInt +// MARK: - Helper Functions + +/// Writes ECDSA public key data to a buffer in SSH format +/// - Parameters: +/// - buffer: The buffer to write to +/// - publicKeyData: The public key data in x963 representation +/// - Returns: The number of bytes written +private func writeECDSAPublicKey(to buffer: inout ByteBuffer, publicKeyData: Data) -> Int { + let start = buffer.writerIndex + buffer.writeSSHString(publicKeyData) + return buffer.writerIndex - start +} + extension P256.Signing.PrivateKey: ByteBufferConvertible { public static func read(consuming buffer: inout ByteBuffer) throws -> Self { guard @@ -35,10 +48,8 @@ extension P256.Signing.PrivateKey: ByteBufferConvertible { let start = buffer.writerIndex buffer.writeSSHString("nistp256") - let publicKey = self.publicKey - var publicKeyBuffer = ByteBuffer() - publicKeyBuffer.writeBytes(publicKey.x963Representation) - buffer.writeSSHString(&publicKeyBuffer) + // Write public key directly as SSH string + buffer.writeSSHString(publicKey.x963Representation) // Write private key as bignum (matching OpenSSH format) let privateKeyData = self.rawRepresentation @@ -80,10 +91,8 @@ extension P384.Signing.PrivateKey: ByteBufferConvertible { let start = buffer.writerIndex buffer.writeSSHString("nistp384") - let publicKey = self.publicKey - var publicKeyBuffer = ByteBuffer() - publicKeyBuffer.writeBytes(publicKey.x963Representation) - buffer.writeSSHString(&publicKeyBuffer) + // Write public key directly as SSH string + buffer.writeSSHString(publicKey.x963Representation) // Write private key as bignum (matching OpenSSH format) let privateKeyData = self.rawRepresentation @@ -125,10 +134,8 @@ extension P521.Signing.PrivateKey: ByteBufferConvertible { let start = buffer.writerIndex buffer.writeSSHString("nistp521") - let publicKey = self.publicKey - var publicKeyBuffer = ByteBuffer() - publicKeyBuffer.writeBytes(publicKey.x963Representation) - buffer.writeSSHString(&publicKeyBuffer) + // Write public key directly as SSH string + buffer.writeSSHString(publicKey.x963Representation) // Write private key as bignum (matching OpenSSH format) let privateKeyData = self.rawRepresentation @@ -165,11 +172,7 @@ extension P256.Signing.PublicKey: ByteBufferConvertible { } public func write(to buffer: inout ByteBuffer) -> Int { - let start = buffer.writerIndex - var publicKeyBuffer = ByteBuffer() - publicKeyBuffer.writeBytes(self.x963Representation) - buffer.writeSSHString(&publicKeyBuffer) - return buffer.writerIndex - start + return writeECDSAPublicKey(to: &buffer, publicKeyData: self.x963Representation) } } @@ -198,11 +201,7 @@ extension P384.Signing.PublicKey: ByteBufferConvertible { } public func write(to buffer: inout ByteBuffer) -> Int { - let start = buffer.writerIndex - var publicKeyBuffer = ByteBuffer() - publicKeyBuffer.writeBytes(self.x963Representation) - buffer.writeSSHString(&publicKeyBuffer) - return buffer.writerIndex - start + return writeECDSAPublicKey(to: &buffer, publicKeyData: self.x963Representation) } } @@ -231,11 +230,7 @@ extension P521.Signing.PublicKey: ByteBufferConvertible { } public func write(to buffer: inout ByteBuffer) -> Int { - let start = buffer.writerIndex - var publicKeyBuffer = ByteBuffer() - publicKeyBuffer.writeBytes(self.x963Representation) - buffer.writeSSHString(&publicKeyBuffer) - return buffer.writerIndex - start + return writeECDSAPublicKey(to: &buffer, publicKeyData: self.x963Representation) } } diff --git a/Sources/Citadel/ByteBufferHelpers.swift b/Sources/Citadel/ByteBufferHelpers.swift index 96e28a7..358f1a3 100644 --- a/Sources/Citadel/ByteBufferHelpers.swift +++ b/Sources/Citadel/ByteBufferHelpers.swift @@ -126,6 +126,20 @@ extension ByteBuffer { setInteger(UInt32(writerIndex - oldWriterIndex - 4), at: oldWriterIndex) } + @discardableResult + mutating func writeSSHString(_ data: Data) -> Int { + let oldWriterIndex = writerIndex + writeInteger(UInt32(data.count)) + writeBytes(data) + return writerIndex - oldWriterIndex + } + + @discardableResult + mutating func writeSSHString(_ bytes: S) -> Int where S.Element == UInt8 { + let data = Data(bytes) + return writeSSHString(data) + } + mutating func readSSHString() -> String? { guard let length = getInteger(at: self.readerIndex, as: UInt32.self), @@ -151,6 +165,9 @@ extension ByteBuffer { } mutating func readSSHBignum() -> Data? { + // SSH bignum format: a 4-byte length field followed by the bignum data. + // The data may have a leading zero byte if it was added during serialization + // to ensure the number is interpreted as unsigned (preventing MSB issues). guard let buffer = readSSHBuffer() else { return nil } @@ -161,11 +178,15 @@ extension ByteBuffer { mutating func writeSSHBignum(_ bignum: BigInt) { var data = bignum.serialize() - // SSH bignum format: prepend zero byte if MSB is set + // SSH bignum format requires that the number is always interpreted as unsigned. + // If the most significant bit (MSB) of the first byte is set, the number could + // be misinterpreted as negative in two's complement representation. To prevent + // this, a zero byte is prepended to the data, ensuring the MSB is not set. if !data.isEmpty && (data[0] & 0x80) != 0 { data.insert(0, at: 0) } + // Write the length of the bignum data as a 4-byte unsigned integer, followed by the data itself writeInteger(UInt32(data.count)) writeBytes(data) } From b6c6dd1477b875936fdb6e6fee903e1a2713e5bb Mon Sep 17 00:00:00 2001 From: Nedithgar Amirka <150447520+nedithgar@users.noreply.github.com> Date: Tue, 29 Jul 2025 23:27:17 +0800 Subject: [PATCH 3/5] feat: enhance ECDSA public key writing with optional curve name and improve SSH bignum format handling --- Sources/Citadel/Algorithms/ECDSA.swift | 22 +++++++++--------- Sources/Citadel/ByteBufferHelpers.swift | 30 ++++++++++++++++++------- 2 files changed, 34 insertions(+), 18 deletions(-) diff --git a/Sources/Citadel/Algorithms/ECDSA.swift b/Sources/Citadel/Algorithms/ECDSA.swift index 4be7b5f..7417d6a 100644 --- a/Sources/Citadel/Algorithms/ECDSA.swift +++ b/Sources/Citadel/Algorithms/ECDSA.swift @@ -9,10 +9,15 @@ import BigInt /// Writes ECDSA public key data to a buffer in SSH format /// - Parameters: /// - buffer: The buffer to write to +/// - curveName: The curve name (e.g., "nistp256", "nistp384", "nistp521"), if provided /// - publicKeyData: The public key data in x963 representation /// - Returns: The number of bytes written -private func writeECDSAPublicKey(to buffer: inout ByteBuffer, publicKeyData: Data) -> Int { +@discardableResult +private func writeECDSAPublicKey(to buffer: inout ByteBuffer, curveName: String? = nil, publicKeyData: Data) -> Int { let start = buffer.writerIndex + if let curveName = curveName { + buffer.writeSSHString(curveName) + } buffer.writeSSHString(publicKeyData) return buffer.writerIndex - start } @@ -46,10 +51,9 @@ extension P256.Signing.PrivateKey: ByteBufferConvertible { public func write(to buffer: inout ByteBuffer) -> Int { let start = buffer.writerIndex - buffer.writeSSHString("nistp256") - // Write public key directly as SSH string - buffer.writeSSHString(publicKey.x963Representation) + // Write curve name and public key + writeECDSAPublicKey(to: &buffer, curveName: "nistp256", publicKeyData: publicKey.x963Representation) // Write private key as bignum (matching OpenSSH format) let privateKeyData = self.rawRepresentation @@ -89,10 +93,9 @@ extension P384.Signing.PrivateKey: ByteBufferConvertible { public func write(to buffer: inout ByteBuffer) -> Int { let start = buffer.writerIndex - buffer.writeSSHString("nistp384") - // Write public key directly as SSH string - buffer.writeSSHString(publicKey.x963Representation) + // Write curve name and public key + writeECDSAPublicKey(to: &buffer, curveName: "nistp384", publicKeyData: publicKey.x963Representation) // Write private key as bignum (matching OpenSSH format) let privateKeyData = self.rawRepresentation @@ -132,10 +135,9 @@ extension P521.Signing.PrivateKey: ByteBufferConvertible { public func write(to buffer: inout ByteBuffer) -> Int { let start = buffer.writerIndex - buffer.writeSSHString("nistp521") - // Write public key directly as SSH string - buffer.writeSSHString(publicKey.x963Representation) + // Write curve name and public key + writeECDSAPublicKey(to: &buffer, curveName: "nistp521", publicKeyData: publicKey.x963Representation) // Write private key as bignum (matching OpenSSH format) let privateKeyData = self.rawRepresentation diff --git a/Sources/Citadel/ByteBufferHelpers.swift b/Sources/Citadel/ByteBufferHelpers.swift index 358f1a3..6fd5398 100644 --- a/Sources/Citadel/ByteBufferHelpers.swift +++ b/Sources/Citadel/ByteBufferHelpers.swift @@ -164,10 +164,17 @@ extension ByteBuffer { return slice } + /// Reads a BigInt from the buffer in SSH bignum format. + /// + /// The SSH bignum format consists of: + /// 1. A 4-byte unsigned integer indicating the length of the bignum data + /// 2. The bignum data itself, as a big-endian byte array + /// + /// The data may include a leading zero byte that was added during serialization + /// to ensure the number is interpreted as unsigned (when MSB was set). + /// + /// - Returns: The bignum data, or nil if reading fails mutating func readSSHBignum() -> Data? { - // SSH bignum format: a 4-byte length field followed by the bignum data. - // The data may have a leading zero byte if it was added during serialization - // to ensure the number is interpreted as unsigned (preventing MSB issues). guard let buffer = readSSHBuffer() else { return nil } @@ -175,18 +182,25 @@ extension ByteBuffer { return buffer.getData(at: 0, length: buffer.readableBytes) } + /// Writes a BigInt to the buffer in SSH bignum format. + /// + /// The SSH bignum format consists of: + /// 1. A 4-byte unsigned integer indicating the length of the bignum data + /// 2. The bignum data itself, serialized as a big-endian byte array + /// + /// SSH bignums must always be interpreted as unsigned. If the most significant bit (MSB) + /// of the first byte is set, the number could be misinterpreted as negative in two's + /// complement representation. To prevent this, a zero byte is prepended when necessary. + /// + /// - Parameter bignum: The BigInt value to write in SSH format mutating func writeSSHBignum(_ bignum: BigInt) { var data = bignum.serialize() - // SSH bignum format requires that the number is always interpreted as unsigned. - // If the most significant bit (MSB) of the first byte is set, the number could - // be misinterpreted as negative in two's complement representation. To prevent - // this, a zero byte is prepended to the data, ensuring the MSB is not set. + // Prepend zero byte if MSB is set to ensure unsigned interpretation if !data.isEmpty && (data[0] & 0x80) != 0 { data.insert(0, at: 0) } - // Write the length of the bignum data as a 4-byte unsigned integer, followed by the data itself writeInteger(UInt32(data.count)) writeBytes(data) } From a67b3862e077b96e83211215e6cca70f97fbada5 Mon Sep 17 00:00:00 2001 From: Nedithgar Amirka <150447520+nedithgar@users.noreply.github.com> Date: Tue, 29 Jul 2025 23:35:16 +0800 Subject: [PATCH 4/5] feat: improve ECDSA private key processing and unify point format handling --- Sources/Citadel/Algorithms/ECDSA.swift | 74 +++++++++++++++----------- 1 file changed, 42 insertions(+), 32 deletions(-) diff --git a/Sources/Citadel/Algorithms/ECDSA.swift b/Sources/Citadel/Algorithms/ECDSA.swift index 7417d6a..7609505 100644 --- a/Sources/Citadel/Algorithms/ECDSA.swift +++ b/Sources/Citadel/Algorithms/ECDSA.swift @@ -4,6 +4,12 @@ import _CryptoExtras import NIOCore import BigInt +// MARK: - Constants + +/// ECDSA point format identifier for uncompressed points +/// In the x963 representation, uncompressed points start with 0x04 +private let uncompressedPointPrefix: UInt8 = 0x04 + // MARK: - Helper Functions /// Writes ECDSA public key data to a buffer in SSH format @@ -22,6 +28,31 @@ private func writeECDSAPublicKey(to buffer: inout ByteBuffer, curveName: String? return buffer.writerIndex - start } +/// Processes ECDSA private key data by validating its size and removing the leading zero byte if present. +/// +/// SSH bignum format may include a leading zero byte to ensure the number is interpreted as unsigned. +/// This function removes that zero byte if present and validates that the resulting data matches +/// the expected key size for the curve. +/// +/// - Parameters: +/// - privateKeyData: The raw private key data from SSH format +/// - expectedKeySize: The expected size in bytes for the specific curve (32 for P-256, 48 for P-384, 66 for P-521) +/// - Returns: The processed private key data with the correct size +/// - Throws: `InvalidOpenSSHKey.invalidLayout` if the data size is invalid +private func processECDSAPrivateKeyData(_ privateKeyData: Data, expectedKeySize: Int) throws -> Data { + // Check if we have the expected size with a leading zero byte + if privateKeyData.count == expectedKeySize + 1 && privateKeyData[0] == 0 { + // Remove the leading zero byte + return privateKeyData.dropFirst() + } else if privateKeyData.count == expectedKeySize { + // Already the correct size + return privateKeyData + } else { + // Invalid size + throw InvalidOpenSSHKey.invalidLayout + } +} + extension P256.Signing.PrivateKey: ByteBufferConvertible { public static func read(consuming buffer: inout ByteBuffer) throws -> Self { guard @@ -36,15 +67,8 @@ extension P256.Signing.PrivateKey: ByteBufferConvertible { throw InvalidOpenSSHKey.invalidLayout } - // ECDSA private keys are stored as bignums in OpenSSH format - // P256 private keys should be exactly 32 bytes (may have leading zero) - guard privateKeyData.count >= 32 && privateKeyData.count <= 33 else { - throw InvalidOpenSSHKey.invalidLayout - } - - // Remove leading zero if present - let keyData = privateKeyData.count == 33 && privateKeyData[0] == 0 ? - privateKeyData.dropFirst() : privateKeyData + // Process the private key data to validate size and remove leading zero if present + let keyData = try processECDSAPrivateKeyData(privateKeyData, expectedKeySize: 32) return try P256.Signing.PrivateKey(rawRepresentation: keyData) } @@ -78,17 +102,10 @@ extension P384.Signing.PrivateKey: ByteBufferConvertible { throw InvalidOpenSSHKey.invalidLayout } - // ECDSA private keys are stored as bignums in OpenSSH format - // P384 private keys should be exactly 48 bytes (may have leading zero) - guard privateKeyData.count >= 48 && privateKeyData.count <= 49 else { - throw InvalidOpenSSHKey.invalidLayout - } - - // Remove leading zero if present - let keyData = privateKeyData.count == 49 && privateKeyData[0] == 0 ? - privateKeyData.dropFirst() : privateKeyData + // Process the private key data to validate size and remove leading zero if present + let keyData = try processECDSAPrivateKeyData(privateKeyData, expectedKeySize: 48) - return try P384.Signing.PrivateKey(rawRepresentation: Data(keyData)) + return try P384.Signing.PrivateKey(rawRepresentation: keyData) } public func write(to buffer: inout ByteBuffer) -> Int { @@ -120,17 +137,10 @@ extension P521.Signing.PrivateKey: ByteBufferConvertible { throw InvalidOpenSSHKey.invalidLayout } - // ECDSA private keys are stored as bignums in OpenSSH format - // P521 private keys should be exactly 66 bytes (may have leading zero) - guard privateKeyData.count >= 66 && privateKeyData.count <= 67 else { - throw InvalidOpenSSHKey.invalidLayout - } - - // Remove leading zero if present - let keyData = privateKeyData.count == 67 && privateKeyData[0] == 0 ? - privateKeyData.dropFirst() : privateKeyData + // Process the private key data to validate size and remove leading zero if present + let keyData = try processECDSAPrivateKeyData(privateKeyData, expectedKeySize: 66) - return try P521.Signing.PrivateKey(rawRepresentation: Data(keyData)) + return try P521.Signing.PrivateKey(rawRepresentation: keyData) } public func write(to buffer: inout ByteBuffer) -> Int { @@ -166,7 +176,7 @@ extension P256.Signing.PublicKey: ByteBufferConvertible { } let pointBytes = pointData.getBytes(at: 0, length: pointData.readableBytes) ?? [] - guard pointBytes.first == 0x04 else { // Uncompressed point + guard pointBytes.first == uncompressedPointPrefix else { // Uncompressed point throw InvalidOpenSSHKey.invalidLayout } @@ -195,7 +205,7 @@ extension P384.Signing.PublicKey: ByteBufferConvertible { } let pointBytes = pointData.getBytes(at: 0, length: pointData.readableBytes) ?? [] - guard pointBytes.first == 0x04 else { // Uncompressed point + guard pointBytes.first == uncompressedPointPrefix else { // Uncompressed point throw InvalidOpenSSHKey.invalidLayout } @@ -224,7 +234,7 @@ extension P521.Signing.PublicKey: ByteBufferConvertible { } let pointBytes = pointData.getBytes(at: 0, length: pointData.readableBytes) ?? [] - guard pointBytes.first == 0x04 else { // Uncompressed point + guard pointBytes.first == uncompressedPointPrefix else { // Uncompressed point throw InvalidOpenSSHKey.invalidLayout } From 5c585760a0a1168d0737a7c5089b5ceb70e272d0 Mon Sep 17 00:00:00 2001 From: Nedithgar Amirka <150447520+nedithgar@users.noreply.github.com> Date: Tue, 29 Jul 2025 23:41:44 +0800 Subject: [PATCH 5/5] fix: clarify documentation for readSSHBignum and writeSSHBignum methods --- Sources/Citadel/ByteBufferHelpers.swift | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Sources/Citadel/ByteBufferHelpers.swift b/Sources/Citadel/ByteBufferHelpers.swift index 6fd5398..4810f3f 100644 --- a/Sources/Citadel/ByteBufferHelpers.swift +++ b/Sources/Citadel/ByteBufferHelpers.swift @@ -173,7 +173,7 @@ extension ByteBuffer { /// The data may include a leading zero byte that was added during serialization /// to ensure the number is interpreted as unsigned (when MSB was set). /// - /// - Returns: The bignum data, or nil if reading fails + /// - Returns: The raw bignum data as `Data`, or nil if reading fails mutating func readSSHBignum() -> Data? { guard let buffer = readSSHBuffer() else { return nil @@ -192,7 +192,9 @@ extension ByteBuffer { /// of the first byte is set, the number could be misinterpreted as negative in two's /// complement representation. To prevent this, a zero byte is prepended when necessary. /// - /// - Parameter bignum: The BigInt value to write in SSH format + /// - Parameter bignum: The BigInt value to write in SSH format. The function handles + /// the SSH requirement of prepending zero bytes for unsigned interpretation when + /// necessary. mutating func writeSSHBignum(_ bignum: BigInt) { var data = bignum.serialize()