Fix SSH ECDSA signature mpint encoding (#772)

* Fix SSH ECDSA signature mpint encoding

OpenSSHSignatureWriter was emitting non-canonical mpints (keeping
fixed-width leading 0x00 bytes), which breaks strict parsers.
Canonicalize r/s mpints and add a regression test.

* Cleanup and consolidation

---------

Co-authored-by: Max Goedjen <max.goedjen@gmail.com>

Sources/Packages/Package.swift

@@ -46,7 +46,7 @@ let package = Package(
         ),
         .testTarget(
             name: "SecretKitTests",
-            dependencies: ["SecretKit", "SecureEnclaveSecretKit", "SmartCardSecretKit"],
+            dependencies: ["SecretKit", "SecretAgentKit", "SecureEnclaveSecretKit", "SmartCardSecretKit"],
             swiftSettings: swiftSettings,
         ),
         .target(

Sources/Packages/Sources/SSHProtocolKit/OpenSSHSignatureWriter.swift

@@ -30,19 +30,28 @@ public struct OpenSSHSignatureWriter: Sendable {
 
 extension OpenSSHSignatureWriter {
 
+    /// Converts a fixed-width big-endian integer (e.g. r/s from CryptoKit rawRepresentation) into an SSH mpint.
+    /// Strips unnecessary leading zeros and prefixes `0x00` if needed to keep the value positive.
+    private func mpint(fromFixedWidthPositiveBytes bytes: Data) -> Data {
+        // mpint zero is encoded as a string with zero bytes of data.
+        guard let firstNonZeroIndex = bytes.firstIndex(where: { $0 != 0x00 }) else {
+            return Data()
+        }
+
+        let trimmed = Data(bytes[firstNonZeroIndex...])
+
+        if let first = trimmed.first, first >= 0x80 {
+            var prefixed = Data([0x00])
+            prefixed.append(trimmed)
+            return prefixed
+        }
+        return trimmed
+    }
+
     func ecdsaSignature(_ rawRepresentation: Data, keyType: KeyType) -> Data {
         let rawLength = rawRepresentation.count/2
-        // Check if we need to pad with 0x00 to prevent certain
-        // ssh servers from thinking r or s is negative
-        let paddingRange: ClosedRange<UInt8> = 0x80...0xFF
-        var r = Data(rawRepresentation[0..<rawLength])
-        if paddingRange ~= r.first! {
-            r.insert(0x00, at: 0)
-        }
-        var s = Data(rawRepresentation[rawLength...])
-        if paddingRange ~= s.first! {
-            s.insert(0x00, at: 0)
-        }
+        let r = mpint(fromFixedWidthPositiveBytes: Data(rawRepresentation[0..<rawLength]))
+        let s = mpint(fromFixedWidthPositiveBytes: Data(rawRepresentation[rawLength...]))
 
         var signatureChunk = Data()
         signatureChunk.append(r.lengthAndData)

Sources/Packages/Tests/SSHProtocolKitTests/OpenSSHSignatureWriterTests.swift

@@ -0,0 +1,83 @@
+import Foundation
+import Testing
+import SSHProtocolKit
+@testable import SecretKit
+
+@Suite struct OpenSSHSignatureWriterTests {
+
+    private let writer = OpenSSHSignatureWriter()
+
+    @Test func ecdsaMpintStripsUnnecessaryLeadingZeros() throws {
+        let secret = Constants.ecdsa256Secret
+
+        // r has a leading 0x00 followed by 0x01 (< 0x80): the mpint must not keep the leading zero.
+        let rBytes: [UInt8] = [0x00] + (1...31).map { UInt8($0) }
+        let r = Data(rBytes)
+        // s has two leading 0x00 bytes followed by 0x7f (< 0x80): the mpint must not keep the leading zeros.
+        let sBytes: [UInt8] = [0x00, 0x00, 0x7f] + Array(repeating: UInt8(0x01), count: 29)
+        let s = Data(sBytes)
+        let rawRepresentation = r + s
+
+        let response = writer.data(secret: secret, signature: rawRepresentation)
+        let (parsedR, parsedS) = try parseEcdsaSignatureMpints(from: response)
+
+        #expect(parsedR == Data((1...31).map { UInt8($0) }))
+        #expect(parsedS == Data([0x7f] + Array(repeating: UInt8(0x01), count: 29)))
+    }
+
+    @Test func ecdsaMpintPrefixesZeroWhenHighBitSet() throws {
+        let secret = Constants.ecdsa256Secret
+
+        // r starts with 0x80 (high bit set): mpint must be prefixed with 0x00.
+        let r = Data([UInt8(0x80)] + Array(repeating: UInt8(0x01), count: 31))
+        let s = Data([UInt8(0x01)] + Array(repeating: UInt8(0x02), count: 31))
+        let rawRepresentation = r + s
+
+        let response = writer.data(secret: secret, signature: rawRepresentation)
+        let (parsedR, parsedS) = try parseEcdsaSignatureMpints(from: response)
+
+        #expect(parsedR == Data([0x00, 0x80] + Array(repeating: UInt8(0x01), count: 31)))
+        #expect(parsedS == Data([0x01] + Array(repeating: UInt8(0x02), count: 31)))
+    }
+
+}
+
+private extension OpenSSHSignatureWriterTests {
+
+    enum Constants {
+        static let ecdsa256Secret = TestSecret(
+            id: Data(),
+            name: "Test Key (ECDSA 256)",
+            publicKey: Data(repeating: 0x01, count: 65),
+            attributes: Attributes(
+                keyType: KeyType(algorithm: .ecdsa, size: 256),
+                authentication: .notRequired,
+                publicKeyAttribution: "test@example.com"
+            )
+        )
+    }
+
+    enum ParseError: Error {
+        case eof
+        case invalidAlgorithm
+    }
+
+    func parseEcdsaSignatureMpints(from openSSHSignedData: Data) throws -> (r: Data, s: Data) {
+        let reader = OpenSSHReader(data: openSSHSignedData)
+
+        // Prefix
+        _ = try reader.readNextBytes(as: UInt32.self)
+
+        let algorithm = try reader.readNextChunkAsString()
+        guard algorithm == "ecdsa-sha2-nistp256" else {
+            throw ParseError.invalidAlgorithm
+        }
+
+        let sigReader = try reader.readNextChunkAsSubReader()
+        let r = try sigReader.readNextChunk()
+        let s = try sigReader.readNextChunk()
+        return (r, s)
+    }
+
+}
+