orlandos-nl / Citadel

SSH Client & Server in Swift
MIT License
230 stars 37 forks source link

Memory/Thread Leak #74

Open christianh104 opened 18 hours ago

christianh104 commented 18 hours ago

Describe the bug When a connection is closed, a thread remains open (NIO-ELT-X-#0, where X increases with each connection). If a connection is opened and closed over and over, the thread count and memory usage just keep increasing, until eventually a DNS error occurs on the 249th connection attempt.

Reproducer Sample main.swift (must be run outside of sandbox to read private key):

import CryptoKit
import Foundation

let username = "christian"
let host = "REDACTED"
let port = 22
let privateKey = "/Users/christian/.ssh/id_rsa"
var counter = 1

while true {
    print("attempt #\(counter)")
    counter += 1

    let privateKeyFileContents = try! String(contentsOf: URL(fileURLWithPath: privateKey), encoding: .utf8)
    let authenticationMethod:SSHAuthenticationMethod

    if let privateKey = try? Insecure.RSA.PrivateKey(sshRsa: privateKeyFileContents) {
        authenticationMethod = .rsa(username: username, privateKey: privateKey)
        print("Loaded RSA key")
    } else if let privateKey = try? Curve25519.Signing.PrivateKey(sshEd25519: privateKeyFileContents) {
        authenticationMethod = .ed25519(username: username, privateKey: privateKey)
        print("Loaded Ed25519 key")
    } else if let privateKey = try? P256.Signing.PrivateKey(pemRepresentation: privateKeyFileContents) {
        authenticationMethod = .p256(username: username, privateKey: privateKey)
        print("Loaded P256 key")
    } else if let privateKey = try? P384.Signing.PrivateKey(pemRepresentation: privateKeyFileContents) {
        authenticationMethod = .p384(username: username, privateKey: privateKey)
        print("Loaded P384 key")
    } else if let privateKey = try? P521.Signing.PrivateKey(pemRepresentation: privateKeyFileContents) {
        authenticationMethod = .p521(username: username, privateKey: privateKey)
        print("Loaded P521 key")
    } else {
        fatalError("Failed to read private key: Invalid key type")
    }

    let ssh = try! await SSHClient.connect(
        host: host,
        port: port,
        authenticationMethod: authenticationMethod,
        hostKeyValidator: .acceptAnything(),
        reconnect: .never
    )
    let sftp = try await ssh.openSFTP()
    // try? await sftp.close()
    try? await ssh.close()

    sleep(1) // Give the runtime a chance to clean up?
}

Client:

Server:

Additional context I've never touched Swift before this week, so I may be doing something wrong, but the repro seems pretty straightforward. I wondered if the SFTP channel wasn't cleaning up, so I tried adding this to SFTPClient.swift: public func close () async throws { try await self.channel.close() } and then uncommenting try? await sftp.close() in main.swift. No luck.

Side note: If there is a better way to determine the private key type than brute force, I am interested to hear it.

Joannis commented 9 hours ago

Hey, thanks for the issue. Let me answer them in order:

  1. I'll run the reproducer and get back. I'm half-assuming the bug is that RSA isn't cleaned up - but not certain yet.
  2. There is a way for me to give you a "neutral" OpenSSH key, that you can parse the key out of. That sounds very helpful for many scenarios - so consider it on the agenda
  3. SFTP does close, but it's currently an open issue #70 that I'm resolving with #75, to get a warning gone. Please do try that PR out