Add RSA key support (ssh-rsa, rsa-sha2-256, rsa-sha2-512)#219
Add RSA key support (ssh-rsa, rsa-sha2-256, rsa-sha2-512)#219daiimus wants to merge 1 commit intoapple:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds RSA key support to SwiftNIO-SSH, implementing three RSA signature algorithms (ssh-rsa, rsa-sha2-256, rsa-sha2-512) per RFC 8332. The implementation enables both host key verification and user authentication using RSA keys with different hash algorithms, prioritizing modern SHA-512 and SHA-256 variants while maintaining SHA-1 fallback for legacy compatibility.
Key changes:
- Adds
RSASignatureAlgorithmenum to represent the three RSA signature algorithm variants - Updates
SSHMessages.PublicKeyAuthTypeto includersaSignatureAlgorithmfield for algorithm negotiation - Adds comprehensive RSA key tests covering signing, verification, and wire format round-tripping
- Raises minimum swift-crypto version from 1.0.0 to 3.0.0 to support _CryptoExtras RSA functionality
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| Package.swift | Bumps swift-crypto minimum version to 3.0.0 and adds _CryptoExtras dependency for RSA support |
| Sources/NIOSSH/RSASignatureAlgorithm.swift | Defines RSA signature algorithm enum (sha512, sha256, sha1) with fallback chain and wire protocol names |
| Sources/NIOSSH/SSHMessages.swift | Updates PublicKeyAuthType to include rsaSignatureAlgorithm field; modifies write logic to use algorithm-specific names |
| Tests/NIOSSHTests/RSAKeyTests.swift | Adds 21 comprehensive tests for RSA signing, verification, algorithm selection, and wire format |
| Tests/NIOSSHTests/HostKeyTests.swift | Updates unrecognized key/signature tests from ssh-rsa to ssh-dss since RSA is now supported |
| Tests/NIOSSHTests/UserAuthenticationStateMachineTests.swift | Updates pattern matching to handle new rsaSignatureAlgorithm field in PublicKeyAuthType |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -38,7 +38,7 @@ let package = Package( | |||
| ], | |||
| dependencies: [ | |||
| .package(url: "https://github.com/apple/swift-nio.git", from: "2.81.0"), | |||
There was a problem hiding this comment.
This change raises the minimum swift-crypto version from 1.0.0 to 3.0.0, which is a breaking change for consumers who may be using swift-crypto 1.x or 2.x. While this is necessary for _CryptoExtras RSA support, this breaking change should be clearly documented in the package release notes, changelog, or migration guide. Consider whether this warrants a major version bump for SwiftNIO-SSH.
| .package(url: "https://github.com/apple/swift-nio.git", from: "2.81.0"), | |
| .package(url: "https://github.com/apple/swift-nio.git", from: "2.81.0"), | |
| // NOTE: The minimum swift-crypto version was raised to 3.x to enable _CryptoExtras | |
| // (e.g. RSA) support. This is a breaking change for users pinned to swift-crypto | |
| // 1.x or 2.x and must be called out in release notes / migration guides and | |
| // considered when deciding whether to perform a major version bump of swift-nio-ssh. |
There was a problem hiding this comment.
Added comment about swift-crypto 3.x being a breaking change.
// NOTE: Minimum swift-crypto raised to 3.x for _CryptoExtras (RSA) support.
// This is a breaking change for users on swift-crypto 1.x or 2.x.
.package(url: "https://github.com/apple/swift-crypto.git", "3.0.0"..<"4.0.0"),| /// RSA signature using SHA-1 (deprecated, legacy compatibility only) | ||
| case sha1 = "ssh-rsa" |
There was a problem hiding this comment.
The RSASignatureAlgorithm.sha1 case introduces support for the legacy ssh-rsa signature scheme, which elsewhere in this module is implemented using Insecure.SHA1.hash for RSA signatures. SHA-1 is considered cryptographically broken for signature schemes, so allowing ssh-rsa to be used for user authentication (and any other RSA signatures) can enable attackers with sufficient resources to forge signatures and impersonate clients or hosts. To avoid this, remove or hard-disable the .sha1 option (or gate it behind an explicit, strongly-worded opt-in) and prefer rsa-sha2-256/rsa-sha2-512 everywhere signatures are generated or verified.
There was a problem hiding this comment.
This is debatable but worth discussion.
bcc7aca to
43a2a84
Compare
| if case .some(.publicKey(.known(let expectedKey, _))) = expectedMessage.map({ $0.method }), | ||
| case .some(.publicKey(.known(let actualKey, let actualSignature))) = request.value.map({ $0.method }), | ||
| if case .some(.publicKey(.known(key: let expectedKey, signature: _, rsaSignatureAlgorithm: _))) = expectedMessage.map({ $0.method }), | ||
| case .some(.publicKey(.known(key: let actualKey, signature: let actualSignature, rsaSignatureAlgorithm: _))) = request.value.map({ $0.method }), |
There was a problem hiding this comment.
These changes seem unrelated.
There was a problem hiding this comment.
Reverted to minimal positional style.
if case .some(.publicKey(.known(let expectedKey, _, _))) = expectedMessage.map({ $0.method }),
case .some(.publicKey(.known(let actualKey, let actualSignature, _))) = request.value.map({ $0.method }),| extension NIOSSHPrivateKey { | ||
| /// The various key types that can be used with NIOSSH. | ||
| internal enum BackingKey { | ||
| internal enum BackingKey: @unchecked Sendable { |
There was a problem hiding this comment.
Is this change actually necessary?
There was a problem hiding this comment.
The _RSA.Signing types are already Sendable, so Swift synthesizes conformance. Removed.
| case .rsa(let key): | ||
| // For RSA, we sign the digest using SHA-512 (rsa-sha2-512) | ||
| let signature = try key.signature(for: digest, padding: .insecurePKCS1v1_5) | ||
| return NIOSSHSignature(backingSignature: .rsaSHA512(signature)) |
There was a problem hiding this comment.
This code looks incorrect. The digest used here is controlled by the digest parameter. We unilaterally assert that it's SHA512 here, but it isn't. It seems this code is only used for computing the digest hash when using the key as the server host key, so I think this is currently untested code: please add tests for it. Those tests should start to fail when using any ECDH algorithm except for P521, because all the others do not use SHA512.
In this case, it's probably sufficient to use runtime checks on the digest type here to see if it's known. But again, please do add the tests first and confirm they fail, for all 4 key exchange algorithms using RSA host keys.
There was a problem hiding this comment.
Good callout. Tests have been added. Fixed using runtime check on DigestBytes.byteCount to detect signature correctly.
case .rsa(let key):
let signature = try key.signature(for: digest, padding: .insecurePKCS1v1_5)
switch DigestBytes.byteCount {
case SHA256.byteCount:
return NIOSSHSignature(backingSignature: .rsaSHA256(signature))
case SHA384.byteCount:
// SHA-384 has no SSH algorithm; use strongest available (rsa-sha2-512)
return NIOSSHSignature(backingSignature: .rsaSHA512(signature))
case SHA512.byteCount:
return NIOSSHSignature(backingSignature: .rsaSHA512(signature))
default:
return NIOSSHSignature(backingSignature: .rsaSHA512(signature))
}| return NIOSSHSignature(backingSignature: .ecdsaP521(signature)) | ||
| case .rsa(let key): | ||
| // Sign using the specified RSA algorithm (RFC 8332) | ||
| let data = Data(payload.bytes.readableBytesView) |
There was a problem hiding this comment.
Extracting to a Data shouldn't be necessary: add a NIOFoundationCompat dependency, which should bring along a conformance to DataProtocol to ByteBufferView. That should allow us to pass payload.bytes.readableBytesView directly to the individual hash functions.
There was a problem hiding this comment.
Added import NIOFoundationCompat and now pass bytesView directly.
|
|
||
| switch request.offer { | ||
| case .privateKey(let privateKeyRequest): | ||
| let rsaAlgorithm = privateKeyRequest.rsaSignatureAlgorithm ?? .sha512 |
There was a problem hiding this comment.
Let's move this defaulting to a computed property so that it exists in only one place. Of, even better, make the property non-nil and when left unset, set it to SHA512.
There was a problem hiding this comment.
Made non-optional with default to SHA512 in init.
| /// Per RFC 8332, servers may support different RSA signature algorithms. | ||
| /// Modern servers prefer rsa-sha2-512, but older servers may only support ssh-rsa. | ||
| /// When connecting to a server, try algorithms in order of preference until one succeeds. | ||
| public enum RSASignatureAlgorithm: String, Sendable, CaseIterable { |
There was a problem hiding this comment.
Let's avoid making this a String-backed enum, they behave a bit weirdly. We can put the algorithm names into the var algorithmName instead.
Similarly, do we need CaseIterable? Is there any reason this shouldn't be Hashable?
There was a problem hiding this comment.
String removed and placed in var algorithmName. CaseIterable removed and Hashable added.
public enum RSASignatureAlgorithm: Hashable, Sendable {
case sha512
case sha256
case sha1
public var algorithmName: String {
switch self {
case .sha512: return "rsa-sha2-512"
case .sha256: return "rsa-sha2-256"
case .sha1: return "ssh-rsa"
}
}
}|
|
||
| /// Returns the next algorithm to try if this one fails. | ||
| /// Used for automatic fallback when server rejects an algorithm. | ||
| public var fallback: RSASignatureAlgorithm? { |
There was a problem hiding this comment.
I can't find any evidence that this is used, please remove it.
Sources/NIOSSH/SSHMessages.swift
Outdated
|
|
||
| enum PublicKeyAuthType: Equatable { | ||
| case known(key: NIOSSHPublicKey, signature: NIOSSHSignature?) | ||
| case known(key: NIOSSHPublicKey, signature: NIOSSHSignature?, rsaSignatureAlgorithm: RSASignatureAlgorithm = .sha512) |
There was a problem hiding this comment.
Let's remove the default value here too.
Sources/NIOSSH/SSHMessages.swift
Outdated
| } else { | ||
| // Default for non-RSA keys (value won't be used) | ||
| rsaAlgorithm = .sha512 | ||
| } |
There was a problem hiding this comment.
Please make this an initializer on the RSASignatureAlgorithm type.
There was a problem hiding this comment.
Added init?(algorithmName:).
public init?<Bytes: Collection>(algorithmName bytes: Bytes) where Bytes.Element == UInt8 {
if bytes.elementsEqual("rsa-sha2-512".utf8) {
self = .sha512
} else if bytes.elementsEqual("rsa-sha2-256".utf8) {
self = .sha256
} else if bytes.elementsEqual("ssh-rsa".utf8) {
self = .sha1
} else {
return nil
}
}| public var rsaSignatureAlgorithm: RSASignatureAlgorithm? | ||
|
|
||
| public init(privateKey: NIOSSHPrivateKey) { | ||
| public init(privateKey: NIOSSHPrivateKey, rsaSignatureAlgorithm: RSASignatureAlgorithm? = nil) { |
There was a problem hiding this comment.
Added default value in initializer so existing callers don't break.
public init(privateKey: NIOSSHPrivateKey, rsaSignatureAlgorithm: RSASignatureAlgorithm = .sha512) {
self.privateKey = privateKey
self.publicKey = privateKey.publicKey
self.rsaSignatureAlgorithm = rsaSignatureAlgorithm
}|
@Lukasa I'm in favour of changes like these; but I'm curious what happened to your objection to adding RSA support directly in NIOSSH? |
|
CryptoExtras got promoted up from being API-unstable. 😄 |
43a2a84 to
50b2cae
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| case known(key: NIOSSHPublicKey, signature: NIOSSHSignature?, rsaSignatureAlgorithm: RSASignatureAlgorithm) | ||
| case unknown |
There was a problem hiding this comment.
The rsaSignatureAlgorithm parameter is now required for all public key auth types, even non-RSA keys. While this works functionally (non-RSA keys ignore the parameter), it creates a confusing API where users must specify an RSA-specific parameter for Ed25519 and ECDSA keys.
Consider one of these alternatives:
- Make the parameter optional with a default value (e.g.,
rsaSignatureAlgorithm: RSASignatureAlgorithm = .sha512) - Create separate cases for RSA vs non-RSA keys (e.g.,
.knownRSA(key:signature:algorithm:)and.known(key:signature:)) - Store the algorithm within the signature itself rather than in the message structure
This would make the API clearer and prevent confusion about when this parameter is actually used.
| //===----------------------------------------------------------------------===// | ||
| // | ||
| // This source file is part of the SwiftNIO open source project | ||
| // | ||
| // Copyright (c) 2019-2025 Apple Inc. and the SwiftNIO project authors | ||
| // Licensed under Apache License v2.0 | ||
| // | ||
| // See LICENSE.txt for license information | ||
| // See CONTRIBUTORS.txt for the list of SwiftNIO project authors | ||
| // | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
| // | ||
| //===----------------------------------------------------------------------===// | ||
|
|
||
| import Crypto | ||
| @_spi(CryptoExtras) import _CryptoExtras | ||
| import NIOCore | ||
| import NIOFoundationCompat | ||
| import XCTest | ||
|
|
||
| @testable import NIOSSH | ||
|
|
||
| final class RSAKeyTests: XCTestCase { | ||
| // MARK: - Basic Signing Flow Tests (matching pattern from HostKeyTests) | ||
|
|
||
| func testBasicRSASHA512SigningFlow() throws { | ||
| let rsaKey = try _RSA.Signing.PrivateKey(keySize: .bits2048) | ||
| let sshKey = NIOSSHPrivateKey(rsaKey: rsaKey) | ||
|
|
||
| let digest = SHA512.hash(data: Array("hello, world!".utf8)) | ||
| let signature = try assertNoThrowWithValue(sshKey.sign(digest: digest)) | ||
|
|
||
| // Naturally, this should verify. | ||
| XCTAssertNoThrow(XCTAssertTrue(sshKey.publicKey.isValidSignature(signature, for: digest))) | ||
|
|
||
| // Now let's try round-tripping through bytebuffer. | ||
| var buffer = ByteBufferAllocator().buffer(capacity: 1024) | ||
| buffer.writeSSHSignature(signature) | ||
|
|
||
| let newSignature = try assertNoThrowWithValue(buffer.readSSHSignature()!) | ||
| XCTAssertNoThrow(XCTAssertTrue(sshKey.publicKey.isValidSignature(newSignature, for: digest))) | ||
| } | ||
|
|
||
| func testBasicRSASHA256SigningFlow() throws { | ||
| let rsaKey = try _RSA.Signing.PrivateKey(keySize: .bits2048) | ||
| let sshKey = NIOSSHPrivateKey(rsaKey: rsaKey) | ||
|
|
||
| let digest = SHA256.hash(data: Array("hello, world!".utf8)) | ||
| let signature = try assertNoThrowWithValue(sshKey.sign(digest: digest)) | ||
|
|
||
| // Verify that a signature over a SHA256 digest can be validated (algorithm selection is tested below) | ||
| XCTAssertNoThrow(XCTAssertTrue(sshKey.publicKey.isValidSignature(signature, for: digest))) | ||
| } | ||
|
|
||
| // MARK: - RSA Host Key Signing with Different Digest Types | ||
| // | ||
| // These tests verify that RSA keys used as host keys during key exchange produce | ||
| // signatures with the correct algorithm tag based on the digest type. | ||
| // Per RFC 8332, the signature algorithm identifier should match the hash used. | ||
|
|
||
| func testRSAHostKeySignsWithSHA256Digest() throws { | ||
| // When RSA is used as a host key with P-256 or Curve25519 key exchange, | ||
| // the exchange hash is SHA-256. The signature should be tagged as rsa-sha2-256. | ||
| let rsaKey = try _RSA.Signing.PrivateKey(keySize: .bits2048) | ||
| let sshKey = NIOSSHPrivateKey(rsaKey: rsaKey) | ||
|
|
||
| let digest = SHA256.hash(data: Array("simulated exchange hash".utf8)) | ||
| let signature = try sshKey.sign(digest: digest) | ||
|
|
||
| // Verify the signature type is rsaSHA256, not rsaSHA512 | ||
| switch signature.backingSignature { | ||
| case .rsaSHA256: | ||
| break // Expected | ||
| case .rsaSHA512: | ||
| XCTFail("SHA-256 digest should produce rsaSHA256 signature, not rsaSHA512") | ||
| case .rsaSHA1: | ||
| XCTFail("SHA-256 digest should produce rsaSHA256 signature, not rsaSHA1") | ||
| default: | ||
| XCTFail("Expected RSA signature type") | ||
| } | ||
|
|
||
| // Verify round-trip through wire format preserves the correct algorithm | ||
| var buffer = ByteBufferAllocator().buffer(capacity: 1024) | ||
| buffer.writeSSHSignature(signature) | ||
|
|
||
| // Check the wire format has the correct algorithm prefix | ||
| guard let prefixLength = buffer.getInteger(at: buffer.readerIndex, as: UInt32.self), | ||
| let prefixBytes = buffer.getBytes(at: buffer.readerIndex + 4, length: Int(prefixLength)) else { | ||
| XCTFail("Failed to read signature prefix") | ||
| return | ||
| } | ||
| XCTAssertEqual(String(bytes: prefixBytes, encoding: .utf8), "rsa-sha2-256", | ||
| "Wire format should use rsa-sha2-256 for SHA-256 digest") | ||
| } | ||
|
|
||
| func testRSAHostKeySignsWithSHA384Digest() throws { | ||
| // When RSA is used as a host key with P-384 key exchange, | ||
| // the exchange hash is SHA-384. Since SSH has no rsa-sha2-384, | ||
| // we should use rsa-sha2-512 (the strongest available). | ||
| let rsaKey = try _RSA.Signing.PrivateKey(keySize: .bits2048) | ||
| let sshKey = NIOSSHPrivateKey(rsaKey: rsaKey) | ||
|
|
||
| let digest = SHA384.hash(data: Array("simulated exchange hash".utf8)) | ||
| let signature = try sshKey.sign(digest: digest) | ||
|
|
||
| // SHA-384 should map to rsaSHA512 (closest stronger algorithm) | ||
| switch signature.backingSignature { | ||
| case .rsaSHA512: | ||
| break // Expected - no SHA-384 in SSH, use strongest available | ||
| case .rsaSHA256: | ||
| XCTFail("SHA-384 digest should produce rsaSHA512 signature (strongest available)") | ||
| case .rsaSHA1: | ||
| XCTFail("SHA-384 digest should not produce rsaSHA1 signature") | ||
| default: | ||
| XCTFail("Expected RSA signature type") | ||
| } | ||
| } | ||
|
|
||
| func testRSAHostKeySignsWithSHA512Digest() throws { | ||
| // When RSA is used as a host key with P-521 key exchange, | ||
| // the exchange hash is SHA-512. The signature should be tagged as rsa-sha2-512. | ||
| let rsaKey = try _RSA.Signing.PrivateKey(keySize: .bits2048) | ||
| let sshKey = NIOSSHPrivateKey(rsaKey: rsaKey) | ||
|
|
||
| let digest = SHA512.hash(data: Array("simulated exchange hash".utf8)) | ||
| let signature = try sshKey.sign(digest: digest) | ||
|
|
||
| // Verify the signature type is rsaSHA512 | ||
| switch signature.backingSignature { | ||
| case .rsaSHA512: | ||
| break // Expected | ||
| case .rsaSHA256: | ||
| XCTFail("SHA-512 digest should produce rsaSHA512 signature, not rsaSHA256") | ||
| case .rsaSHA1: | ||
| XCTFail("SHA-512 digest should produce rsaSHA512 signature, not rsaSHA1") | ||
| default: | ||
| XCTFail("Expected RSA signature type") | ||
| } | ||
|
|
||
| // Verify round-trip through wire format preserves the correct algorithm | ||
| var buffer = ByteBufferAllocator().buffer(capacity: 1024) | ||
| buffer.writeSSHSignature(signature) | ||
|
|
||
| // Check the wire format has the correct algorithm prefix | ||
| guard let prefixLength = buffer.getInteger(at: buffer.readerIndex, as: UInt32.self), | ||
| let prefixBytes = buffer.getBytes(at: buffer.readerIndex + 4, length: Int(prefixLength)) else { | ||
| XCTFail("Failed to read signature prefix") | ||
| return | ||
| } | ||
| XCTAssertEqual(String(bytes: prefixBytes, encoding: .utf8), "rsa-sha2-512", | ||
| "Wire format should use rsa-sha2-512 for SHA-512 digest") | ||
| } | ||
|
|
||
| // MARK: - RSA Signature Algorithm Selection Tests | ||
|
|
||
| func testRSASignatureAlgorithmSHA512() throws { | ||
| let rsaKey = try _RSA.Signing.PrivateKey(keySize: .bits2048) | ||
| let sshKey = NIOSSHPrivateKey(rsaKey: rsaKey) | ||
|
|
||
| var sessionID = ByteBufferAllocator().buffer(capacity: 32) | ||
| sessionID.writeBytes(0..<32) | ||
|
|
||
| let payload = UserAuthSignablePayload( | ||
| sessionIdentifier: sessionID, | ||
| userName: "testuser", | ||
| serviceName: "ssh-connection", | ||
| publicKey: sshKey.publicKey, | ||
| rsaSignatureAlgorithm: .sha512 | ||
| ) | ||
|
|
||
| let signature = try assertNoThrowWithValue(sshKey.sign(payload, rsaSignatureAlgorithm: .sha512)) | ||
| XCTAssertNoThrow(XCTAssertTrue(sshKey.publicKey.isValidSignature(signature, for: payload))) | ||
|
|
||
| // Verify round-trip | ||
| var buffer = ByteBufferAllocator().buffer(capacity: 1024) | ||
| buffer.writeSSHSignature(signature) | ||
| let roundTripped = try assertNoThrowWithValue(buffer.readSSHSignature()!) | ||
| XCTAssertNoThrow(XCTAssertTrue(sshKey.publicKey.isValidSignature(roundTripped, for: payload))) | ||
| } | ||
|
|
||
| func testRSASignatureAlgorithmSHA256() throws { | ||
| let rsaKey = try _RSA.Signing.PrivateKey(keySize: .bits2048) | ||
| let sshKey = NIOSSHPrivateKey(rsaKey: rsaKey) | ||
|
|
||
| var sessionID = ByteBufferAllocator().buffer(capacity: 32) | ||
| sessionID.writeBytes(0..<32) | ||
|
|
||
| let payload = UserAuthSignablePayload( | ||
| sessionIdentifier: sessionID, | ||
| userName: "testuser", | ||
| serviceName: "ssh-connection", | ||
| publicKey: sshKey.publicKey, | ||
| rsaSignatureAlgorithm: .sha256 | ||
| ) | ||
|
|
||
| let signature = try assertNoThrowWithValue(sshKey.sign(payload, rsaSignatureAlgorithm: .sha256)) | ||
| XCTAssertNoThrow(XCTAssertTrue(sshKey.publicKey.isValidSignature(signature, for: payload))) | ||
|
|
||
| // Verify round-trip | ||
| var buffer = ByteBufferAllocator().buffer(capacity: 1024) | ||
| buffer.writeSSHSignature(signature) | ||
| let roundTripped = try assertNoThrowWithValue(buffer.readSSHSignature()!) | ||
| XCTAssertNoThrow(XCTAssertTrue(sshKey.publicKey.isValidSignature(roundTripped, for: payload))) | ||
| } | ||
|
|
||
| func testRSASignatureAlgorithmSHA1() throws { | ||
| let rsaKey = try _RSA.Signing.PrivateKey(keySize: .bits2048) | ||
| let sshKey = NIOSSHPrivateKey(rsaKey: rsaKey) | ||
|
|
||
| var sessionID = ByteBufferAllocator().buffer(capacity: 32) | ||
| sessionID.writeBytes(0..<32) | ||
|
|
||
| let payload = UserAuthSignablePayload( | ||
| sessionIdentifier: sessionID, | ||
| userName: "testuser", | ||
| serviceName: "ssh-connection", | ||
| publicKey: sshKey.publicKey, | ||
| rsaSignatureAlgorithm: .sha1 | ||
| ) | ||
|
|
||
| let signature = try assertNoThrowWithValue(sshKey.sign(payload, rsaSignatureAlgorithm: .sha1)) | ||
| XCTAssertNoThrow(XCTAssertTrue(sshKey.publicKey.isValidSignature(signature, for: payload))) | ||
|
|
||
| // Verify round-trip | ||
| var buffer = ByteBufferAllocator().buffer(capacity: 1024) | ||
| buffer.writeSSHSignature(signature) | ||
| let roundTripped = try assertNoThrowWithValue(buffer.readSSHSignature()!) | ||
| XCTAssertNoThrow(XCTAssertTrue(sshKey.publicKey.isValidSignature(roundTripped, for: payload))) | ||
| } | ||
|
|
||
| // MARK: - Verification Failure Tests | ||
|
|
||
| func testRSAFailsVerificationWithDifferentKeys() throws { | ||
| let rsaKey = try _RSA.Signing.PrivateKey(keySize: .bits2048) | ||
| let sshKey = NIOSSHPrivateKey(rsaKey: rsaKey) | ||
|
|
||
| let digest = SHA512.hash(data: Array("hello, world!".utf8)) | ||
| let signature = try assertNoThrowWithValue(sshKey.sign(digest: digest)) | ||
|
|
||
| let otherRSAKey = try _RSA.Signing.PrivateKey(keySize: .bits2048) | ||
| let otherSSHKey = NIOSSHPrivateKey(rsaKey: otherRSAKey) | ||
|
|
||
| // Naturally, this should not verify. | ||
| XCTAssertNoThrow(XCTAssertFalse(otherSSHKey.publicKey.isValidSignature(signature, for: digest))) | ||
|
|
||
| // Now let's try round-tripping through bytebuffer. | ||
| var buffer = ByteBufferAllocator().buffer(capacity: 1024) | ||
| buffer.writeSSHSignature(signature) | ||
|
|
||
| let newSignature = try assertNoThrowWithValue(buffer.readSSHSignature()!) | ||
| XCTAssertNoThrow(XCTAssertFalse(otherSSHKey.publicKey.isValidSignature(newSignature, for: digest))) | ||
| } | ||
|
|
||
| func testRSAFailsVerificationWithWrongAlgorithmKeys() throws { | ||
| let rsaKey = try _RSA.Signing.PrivateKey(keySize: .bits2048) | ||
| let sshKey = NIOSSHPrivateKey(rsaKey: rsaKey) | ||
|
|
||
| let digest = SHA512.hash(data: Array("hello, world!".utf8)) | ||
| let signature = try assertNoThrowWithValue(sshKey.sign(digest: digest)) | ||
|
|
||
| // Try verifying with an Ed25519 key | ||
| let otherSSHKey = NIOSSHPrivateKey(ed25519Key: .init()) | ||
|
|
||
| XCTAssertNoThrow(XCTAssertFalse(otherSSHKey.publicKey.isValidSignature(signature, for: digest))) | ||
| } | ||
|
|
||
| func testEd25519FailsVerificationWithRSASignature() throws { | ||
| let edKey = Curve25519.Signing.PrivateKey() | ||
| let sshKey = NIOSSHPrivateKey(ed25519Key: edKey) | ||
|
|
||
| let digest = SHA256.hash(data: Array("hello, world!".utf8)) | ||
| let signature = try assertNoThrowWithValue(sshKey.sign(digest: digest)) | ||
|
|
||
| let rsaKey = try _RSA.Signing.PrivateKey(keySize: .bits2048) | ||
| let rsaSSHKey = NIOSSHPrivateKey(rsaKey: rsaKey) | ||
|
|
||
| // RSA key should not verify Ed25519 signature | ||
| XCTAssertNoThrow(XCTAssertFalse(rsaSSHKey.publicKey.isValidSignature(signature, for: digest))) | ||
| } | ||
|
|
||
| // MARK: - Public Key Wire Format Tests | ||
|
|
||
| func testRSAPublicKeyRoundTrip() throws { | ||
| let rsaKey = try _RSA.Signing.PrivateKey(keySize: .bits2048) | ||
| let sshKey = NIOSSHPrivateKey(rsaKey: rsaKey) | ||
| let publicKey = sshKey.publicKey | ||
|
|
||
| // Write to buffer | ||
| var buffer = ByteBufferAllocator().buffer(capacity: 1024) | ||
| buffer.writeSSHHostKey(publicKey) | ||
|
|
||
| // Read back | ||
| let readKey = try assertNoThrowWithValue(buffer.readSSHHostKey()!) | ||
|
|
||
| // Keys should be equal | ||
| XCTAssertEqual(publicKey, readKey) | ||
| } | ||
|
|
||
| func testRSAPublicKeyPrefix() throws { | ||
| let rsaKey = try _RSA.Signing.PrivateKey(keySize: .bits2048) | ||
| let sshKey = NIOSSHPrivateKey(rsaKey: rsaKey) | ||
|
|
||
| XCTAssertTrue(sshKey.publicKey.keyPrefix.elementsEqual("ssh-rsa".utf8)) | ||
| } | ||
|
|
||
| // MARK: - Host Key Algorithm Tests | ||
|
|
||
| func testRSAHostKeyAlgorithms() throws { | ||
| let rsaKey = try _RSA.Signing.PrivateKey(keySize: .bits2048) | ||
| let sshKey = NIOSSHPrivateKey(rsaKey: rsaKey) | ||
|
|
||
| let algorithms = sshKey.hostKeyAlgorithms | ||
| XCTAssertEqual(algorithms.count, 3) | ||
| XCTAssertTrue(algorithms.contains("rsa-sha2-512")) | ||
| XCTAssertTrue(algorithms.contains("rsa-sha2-256")) | ||
| XCTAssertTrue(algorithms.contains("ssh-rsa")) | ||
| } | ||
|
|
||
| // MARK: - RSASignatureAlgorithm Enum Tests | ||
|
|
||
| func testRSASignatureAlgorithmInitFromWireName() { | ||
| // Valid algorithm names | ||
| XCTAssertEqual(RSASignatureAlgorithm(algorithmName: "rsa-sha2-512".utf8), .sha512) | ||
| XCTAssertEqual(RSASignatureAlgorithm(algorithmName: "rsa-sha2-256".utf8), .sha256) | ||
| XCTAssertEqual(RSASignatureAlgorithm(algorithmName: "ssh-rsa".utf8), .sha1) | ||
|
|
||
| // Invalid algorithm names | ||
| XCTAssertNil(RSASignatureAlgorithm(algorithmName: "unknown".utf8)) | ||
| XCTAssertNil(RSASignatureAlgorithm(algorithmName: "".utf8)) | ||
| XCTAssertNil(RSASignatureAlgorithm(algorithmName: "RSA-SHA2-512".utf8)) // Case-sensitive | ||
| } | ||
|
|
||
| func testRSASignatureAlgorithmWireNames() { | ||
| XCTAssertEqual(RSASignatureAlgorithm.sha512.algorithmName, "rsa-sha2-512") | ||
| XCTAssertEqual(RSASignatureAlgorithm.sha256.algorithmName, "rsa-sha2-256") | ||
| XCTAssertEqual(RSASignatureAlgorithm.sha1.algorithmName, "ssh-rsa") | ||
| } | ||
|
|
||
| // MARK: - Different Key Sizes | ||
|
|
||
| func testRSA2048KeyWorks() throws { | ||
| let rsaKey = try _RSA.Signing.PrivateKey(keySize: .bits2048) | ||
| let sshKey = NIOSSHPrivateKey(rsaKey: rsaKey) | ||
|
|
||
| let digest = SHA512.hash(data: Array("test".utf8)) | ||
| let signature = try sshKey.sign(digest: digest) | ||
|
|
||
| XCTAssertTrue(sshKey.publicKey.isValidSignature(signature, for: digest)) | ||
| } | ||
|
|
||
| func testRSA3072KeyWorks() throws { | ||
| let rsaKey = try _RSA.Signing.PrivateKey(keySize: .bits3072) | ||
| let sshKey = NIOSSHPrivateKey(rsaKey: rsaKey) | ||
|
|
||
| let digest = SHA512.hash(data: Array("test".utf8)) | ||
| let signature = try sshKey.sign(digest: digest) | ||
|
|
||
| XCTAssertTrue(sshKey.publicKey.isValidSignature(signature, for: digest)) | ||
| } | ||
|
|
||
| func testRSA4096KeyWorks() throws { | ||
| let rsaKey = try _RSA.Signing.PrivateKey(keySize: .bits4096) | ||
| let sshKey = NIOSSHPrivateKey(rsaKey: rsaKey) | ||
|
|
||
| let digest = SHA512.hash(data: Array("test".utf8)) | ||
| let signature = try sshKey.sign(digest: digest) | ||
|
|
||
| XCTAssertTrue(sshKey.publicKey.isValidSignature(signature, for: digest)) | ||
| } | ||
|
|
||
| // MARK: - Signature Wire Format Tests | ||
|
|
||
| func testRSASHA512SignaturePrefix() throws { | ||
| let rsaKey = try _RSA.Signing.PrivateKey(keySize: .bits2048) | ||
| let sshKey = NIOSSHPrivateKey(rsaKey: rsaKey) | ||
|
|
||
| var sessionID = ByteBufferAllocator().buffer(capacity: 32) | ||
| sessionID.writeBytes(0..<32) | ||
|
|
||
| let payload = UserAuthSignablePayload( | ||
| sessionIdentifier: sessionID, | ||
| userName: "testuser", | ||
| serviceName: "ssh-connection", | ||
| publicKey: sshKey.publicKey, | ||
| rsaSignatureAlgorithm: .sha512 | ||
| ) | ||
|
|
||
| let signature = try sshKey.sign(payload, rsaSignatureAlgorithm: .sha512) | ||
|
|
||
| var buffer = ByteBufferAllocator().buffer(capacity: 1024) | ||
| buffer.writeSSHSignature(signature) | ||
|
|
||
| // Read back the algorithm prefix | ||
| guard let prefixLength = buffer.readInteger(as: UInt32.self), | ||
| let prefixBytes = buffer.readBytes(length: Int(prefixLength)) else { | ||
| XCTFail("Failed to read signature prefix") | ||
| return | ||
| } | ||
|
|
||
| XCTAssertEqual(String(bytes: prefixBytes, encoding: .utf8), "rsa-sha2-512") | ||
| } | ||
|
|
||
| func testRSASHA256SignaturePrefix() throws { | ||
| let rsaKey = try _RSA.Signing.PrivateKey(keySize: .bits2048) | ||
| let sshKey = NIOSSHPrivateKey(rsaKey: rsaKey) | ||
|
|
||
| var sessionID = ByteBufferAllocator().buffer(capacity: 32) | ||
| sessionID.writeBytes(0..<32) | ||
|
|
||
| let payload = UserAuthSignablePayload( | ||
| sessionIdentifier: sessionID, | ||
| userName: "testuser", | ||
| serviceName: "ssh-connection", | ||
| publicKey: sshKey.publicKey, | ||
| rsaSignatureAlgorithm: .sha256 | ||
| ) | ||
|
|
||
| let signature = try sshKey.sign(payload, rsaSignatureAlgorithm: .sha256) | ||
|
|
||
| var buffer = ByteBufferAllocator().buffer(capacity: 1024) | ||
| buffer.writeSSHSignature(signature) | ||
|
|
||
| // Read back the algorithm prefix | ||
| guard let prefixLength = buffer.readInteger(as: UInt32.self), | ||
| let prefixBytes = buffer.readBytes(length: Int(prefixLength)) else { | ||
| XCTFail("Failed to read signature prefix") | ||
| return | ||
| } | ||
|
|
||
| XCTAssertEqual(String(bytes: prefixBytes, encoding: .utf8), "rsa-sha2-256") | ||
| } | ||
|
|
||
| func testRSASHA1SignaturePrefix() throws { | ||
| let rsaKey = try _RSA.Signing.PrivateKey(keySize: .bits2048) | ||
| let sshKey = NIOSSHPrivateKey(rsaKey: rsaKey) | ||
|
|
||
| var sessionID = ByteBufferAllocator().buffer(capacity: 32) | ||
| sessionID.writeBytes(0..<32) | ||
|
|
||
| let payload = UserAuthSignablePayload( | ||
| sessionIdentifier: sessionID, | ||
| userName: "testuser", | ||
| serviceName: "ssh-connection", | ||
| publicKey: sshKey.publicKey, | ||
| rsaSignatureAlgorithm: .sha1 | ||
| ) | ||
|
|
||
| let signature = try sshKey.sign(payload, rsaSignatureAlgorithm: .sha1) | ||
|
|
||
| var buffer = ByteBufferAllocator().buffer(capacity: 1024) | ||
| buffer.writeSSHSignature(signature) | ||
|
|
||
| // Read back the algorithm prefix | ||
| guard let prefixLength = buffer.readInteger(as: UInt32.self), | ||
| let prefixBytes = buffer.readBytes(length: Int(prefixLength)) else { | ||
| XCTFail("Failed to read signature prefix") | ||
| return | ||
| } | ||
|
|
||
| XCTAssertEqual(String(bytes: prefixBytes, encoding: .utf8), "ssh-rsa") | ||
| } | ||
|
|
||
| // MARK: - Hashable/Equatable Tests | ||
|
|
||
| func testRSAPublicKeyEquatable() throws { | ||
| let rsaKey = try _RSA.Signing.PrivateKey(keySize: .bits2048) | ||
| let sshKey = NIOSSHPrivateKey(rsaKey: rsaKey) | ||
|
|
||
| // Same key should be equal | ||
| XCTAssertEqual(sshKey.publicKey, sshKey.publicKey) | ||
|
|
||
| // Different key should not be equal | ||
| let otherRSAKey = try _RSA.Signing.PrivateKey(keySize: .bits2048) | ||
| let otherSSHKey = NIOSSHPrivateKey(rsaKey: otherRSAKey) | ||
| XCTAssertNotEqual(sshKey.publicKey, otherSSHKey.publicKey) | ||
| } | ||
|
|
||
| func testRSAPublicKeyHashable() throws { | ||
| let rsaKey = try _RSA.Signing.PrivateKey(keySize: .bits2048) | ||
| let sshKey = NIOSSHPrivateKey(rsaKey: rsaKey) | ||
|
|
||
| var set = Set<NIOSSHPublicKey>() | ||
| set.insert(sshKey.publicKey) | ||
|
|
||
| XCTAssertTrue(set.contains(sshKey.publicKey)) | ||
|
|
||
| let otherRSAKey = try _RSA.Signing.PrivateKey(keySize: .bits2048) | ||
| let otherSSHKey = NIOSSHPrivateKey(rsaKey: otherRSAKey) | ||
| XCTAssertFalse(set.contains(otherSSHKey.publicKey)) | ||
| } | ||
| } |
There was a problem hiding this comment.
The RSAKeyTests test suite is missing critical integration tests for round-tripping SSH messages with RSA keys, particularly UserAuthRequest messages with different RSA signature algorithms (rsa-sha2-256, rsa-sha2-512). These tests would have caught the bug where knownAlgorithms doesn't include the modern RSA algorithm names, causing auth requests with rsa-sha2-256 or rsa-sha2-512 to be rejected as unknown. Consider adding tests similar to testUserAuthRequestWithKeysAndSignature in SSHMessagesTests.swift but using RSA keys with all three algorithm variants.
| /// RSA signature using SHA-1 (deprecated, legacy compatibility only) | ||
| case sha1 | ||
|
|
||
| /// The algorithm name as used in SSH wire protocol | ||
| public var algorithmName: String { | ||
| switch self { | ||
| case .sha512: return "rsa-sha2-512" | ||
| case .sha256: return "rsa-sha2-256" | ||
| case .sha1: return "ssh-rsa" | ||
| } | ||
| } | ||
|
|
||
| /// The algorithm name as UTF8 bytes for wire protocol | ||
| internal var wireBytes: String.UTF8View { self.algorithmName.utf8 } | ||
|
|
||
| /// Initialize from wire protocol algorithm name. | ||
| /// Returns nil for unrecognized algorithm names. | ||
| public init?<Bytes: Collection>(algorithmName bytes: Bytes) where Bytes.Element == UInt8 { | ||
| if bytes.elementsEqual("rsa-sha2-512".utf8) { | ||
| self = .sha512 | ||
| } else if bytes.elementsEqual("rsa-sha2-256".utf8) { | ||
| self = .sha256 | ||
| } else if bytes.elementsEqual("ssh-rsa".utf8) { | ||
| self = .sha1 |
There was a problem hiding this comment.
This enum explicitly exposes the legacy ssh-rsa algorithm via the .sha1 case, which relies on the weak SHA-1 hash and is considered cryptographically broken for authentication purposes. Weak algorithm SHA-1 is used (through the ssh-rsa mapping), consider restricting RSA signatures to the rsa-sha2-256/rsa-sha2-512 variants and only allowing ssh-rsa behind a very explicit, opt-in compatibility switch (or removing it entirely). Leaving RSASignatureAlgorithm.sha1 available for normal user authentication increases the risk that deployments will inadvertently negotiate RSA+SHA1 and degrade the security of SSH authentication.
Motivation: RSA keys are still widely used in SSH deployments, especially on older servers and enterprise environments. The current SwiftNIO-SSH implementation only supports Ed25519 and ECDSA keys, which limits compatibility. RFC 8332 deprecates the original ssh-rsa signature scheme (which uses SHA-1) in favor of rsa-sha2-256 and rsa-sha2-512, but many servers still require fallback to ssh-rsa for compatibility. Modifications: - Add NIOSSHPrivateKey(rsaKey:) initializer using _CryptoExtras - Add RSASignatureAlgorithm enum with sha512/sha256/sha1 cases and fallback chain - Implement rsa-sha2-512, rsa-sha2-256, and ssh-rsa signature algorithms per RFC 8332 - Add RSA public key wire format read/write support (exponent + modulus) - Add RSA signature wire format with proper algorithm prefixes - Support RSA user authentication with algorithm negotiation - Add BackingKey.rsa case to NIOSSHPrivateKey and NIOSSHPublicKey - Add .rsaSHA512, .rsaSHA256, .rsaSHA1 cases to NIOSSHSignature - Update SSHMessages to parse rsaSignatureAlgorithm field Tests: - Add RSAKeyTests.swift with 21 tests covering signing, verification, round-trip, cross-algorithm failures, key sizes (2048/3072/4096), and wire format - Update HostKeyTests to use ssh-dss for 'unrecognised' tests (RSA now supported) - Update UserAuthenticationStateMachineTests pattern matching for new signature field Result: SwiftNIO-SSH now supports RSA keys for both host key verification and user authentication. The implementation follows RFC 8332 by preferring rsa-sha2-512, falling back to rsa-sha2-256, and finally ssh-rsa for maximum compatibility.
Motivation
RSA keys are still widely used in SSH deployments, especially on older servers and enterprise environments. The current SwiftNIO-SSH implementation only supports Ed25519 and ECDSA keys, which limits compatibility.
RFC 8332 deprecates the original ssh-rsa signature scheme (which uses SHA-1) in favor of rsa-sha2-256 and rsa-sha2-512, but many servers still require fallback to ssh-rsa for compatibility.
Modifications
NIOSSHPrivateKey(rsaKey:)initializer using_CryptoExtrasRSASignatureAlgorithmenum with sha512/sha256/sha1 cases and fallback chainBackingKey.rsacase toNIOSSHPrivateKeyandNIOSSHPublicKey.rsaSHA512,.rsaSHA256,.rsaSHA1cases toNIOSSHSignatureSSHMessagesto parsersaSignatureAlgorithmfieldTests
RSAKeyTests.swiftwith 21 tests covering signing, verification, round-trip, cross-algorithm failures, key sizes (2048/3072/4096), and wire formatHostKeyTeststo use ssh-dss for 'unrecognised' tests (RSA now supported)UserAuthenticationStateMachineTestspattern matching for new signature fieldResult
SwiftNIO-SSH now supports RSA keys for both host key verification and user authentication. The implementation follows RFC 8332 by preferring rsa-sha2-512, falling back to rsa-sha2-256, and finally ssh-rsa for maximum compatibility.