Merge branch 'main' into jo/updated-shell

.github/workflows/test.yml

@@ -0,0 +1,39 @@
+name: test
+concurrency:
+    group: ${{ github.workflow }}-${{ github.ref }}
+    cancel-in-progress: true
+on:
+    pull_request: { types: [opened, reopened, synchronize, ready_for_review] }
+    push: { branches: [main] }
+
+env:
+    LOG_LEVEL: info
+    SWIFT_DETERMINISTIC_HASHING: 1
+jobs:
+    test:
+        services:
+            ssh-server:
+                image: lscr.io/linuxserver/openssh-server:latest
+                ports:
+                    - 2222:2222
+                env:
+                    USER_NAME: citadel
+                    USER_PASSWORD: hunter2
+                    PASSWORD_ACCESS: true
+        runs-on: ubuntu-latest
+        container: 
+          image: swift:5.10-jammy
+          env:
+            SWIFT_DETERMINISTIC_HASHING: 1
+        steps:
+            - uses: actions/checkout@v4
+            - name: Resolve
+              run: swift package resolve
+            - name: Run tests
+              run: swift test
+              env:
+                SWIFT_DETERMINISTIC_HASHING: 1
+                SSH_HOST: ssh-server
+                SSH_PORT: 2222
+                SSH_USERNAME: citadel
+                SSH_PASSWORD: hunter2

.gitignore

@@ -6,3 +6,4 @@
 xcuserdata/
 Package.resolved
 citadel_host_key_ed25519
+.vscode/launch.json

Examples/OpenSSH/Dockerfile

@@ -0,0 +1,12 @@
+
+FROM ubuntu:latest
+
+RUN apt update && apt install openssh-server sudo -y
+
+RUN echo "ubuntu:test" | chpasswd
+
+RUN service ssh start
+
+EXPOSE 22
+
+CMD ["/usr/sbin/sshd","-D"]

Package.resolved

@@ -51,8 +51,8 @@
         "repositoryURL": "https://github.com/apple/swift-log.git",
         "state": {
           "branch": null,
-          "revision": "32e8d724467f8fe623624570367e3d50c5638e46",
-          "version": "1.5.2"
+          "revision": "e97a6fcb1ab07462881ac165fdbb37f067e205d5",
+          "version": "1.5.4"
         }
       },
       {

Sources/Citadel/Client.swift

@@ -1,5 +1,5 @@
 import NIO
-import CryptoKit
+import Crypto
 import Logging
 import NIOSSH
 

Sources/Citadel/ClientSession.swift

@@ -1,23 +1,24 @@
 import NIO
 import NIOSSH
+import Logging
 
 final class ClientHandshakeHandler: ChannelInboundHandler {
     typealias InboundIn = Any
 
     private let promise: EventLoopPromise<Void>
+    let logger = Logger(label: "nl.orlandos.citadel.handshake")
 
     /// A future that will be fulfilled when the handshake is complete.
     public var authenticated: EventLoopFuture<Void> {
         promise.futureResult
     }
 
-    init(eventLoop: EventLoop) {
+    init(eventLoop: EventLoop, loginTimeout: TimeAmount) {
         let promise = eventLoop.makePromise(of: Void.self)
         self.promise = promise
     }
 
     func userInboundEventTriggered(context: ChannelHandlerContext, event: Any) {
-        print(event)
         if event is UserAuthSuccessEvent {
             self.promise.succeed(())
         }
@@ -55,7 +56,10 @@ final class SSHClientSession {
         algorithms: SSHAlgorithms = SSHAlgorithms(),
         protocolOptions: Set<SSHProtocolOption> = []
     ) async throws -> SSHClientSession {
-        let handshakeHandler = ClientHandshakeHandler(eventLoop: channel.eventLoop)
+        let handshakeHandler = ClientHandshakeHandler(
+            eventLoop: channel.eventLoop,
+            loginTimeout: .seconds(10)
+        )
         var clientConfiguration = SSHClientConfiguration(
             userAuthDelegate: authenticationMethod(),
             serverAuthDelegate: hostKeyValidator
@@ -102,7 +106,10 @@ final class SSHClientSession {
         group: EventLoopGroup = MultiThreadedEventLoopGroup(numberOfThreads: 1),
         connectTimeout: TimeAmount = .seconds(30)
     ) async throws -> SSHClientSession {
-        let handshakeHandler = ClientHandshakeHandler(eventLoop: group.next())
+        let handshakeHandler = ClientHandshakeHandler(
+            eventLoop: group.next(),
+            loginTimeout: .seconds(10)
+        )
         var clientConfiguration = SSHClientConfiguration(
             userAuthDelegate: authenticationMethod(),
             serverAuthDelegate: hostKeyValidator

Sources/Citadel/DirectTCPIP/Server/DirectTCPIP+Server.swift

@@ -0,0 +1,62 @@
+import NIO
+import NIOSSH
+
+fileprivate final class ProxyChannelHandler: ChannelOutboundHandler {
+    typealias OutboundIn = ByteBuffer
+
+    private let write: (ByteBuffer, EventLoopPromise<Void>?) -> Void
+
+    init(write: @escaping (ByteBuffer, EventLoopPromise<Void>?) -> Void) {
+        self.write = write
+    }
+
+    func write(context: ChannelHandlerContext, data: NIOAny, promise: EventLoopPromise<Void>?) {
+        let data = self.unwrapOutboundIn(data)
+        write(data, promise)
+    }
+}
+
+public protocol DirectTCPIPDelegate {
+    func initializeDirectTCPIPChannel(_ channel: Channel, request: SSHChannelType.DirectTCPIP, context: SSHContext) -> EventLoopFuture<Void>
+}
+
+public struct DirectTCPIPForwardingDelegate: DirectTCPIPDelegate {
+    internal enum Error: Swift.Error {
+        case forbidden
+    }
+
+    public var whitelistedHosts: [String]?
+    public var whitelistedPorts: [Int]?
+
+    public init() {}
+
+    public func initializeDirectTCPIPChannel(_ channel: Channel, request: SSHChannelType.DirectTCPIP, context: SSHContext) -> EventLoopFuture<Void> {
+        if let whitelistedHosts, !whitelistedHosts.contains(request.targetHost) {
+            return channel.eventLoop.makeFailedFuture(Error.forbidden)
+        }
+
+        if let whitelistedPorts, !whitelistedPorts.contains(request.targetPort) {
+            return channel.eventLoop.makeFailedFuture(Error.forbidden)
+        }
+
+        return ClientBootstrap(group: channel.eventLoop)
+            .connect(host: request.targetHost, port: request.targetPort)
+            .flatMap { remote in
+                channel.pipeline.addHandlers([
+                    DataToBufferCodec()
+                ]).flatMap {
+                    channel.pipeline.addHandler(ProxyChannelHandler { data, promise in
+                        remote.writeAndFlush(data, promise: promise)
+                    })
+                }.flatMap {
+                    remote.pipeline.addHandler(ProxyChannelHandler { [weak channel] data, promise in
+                        guard let channel else {
+                            promise?.fail(ChannelError.ioOnClosedChannel)
+                            return
+                        }
+                        channel.writeAndFlush(data, promise: promise)
+                    })
+                }
+            }
+    }
+}

Sources/Citadel/SFTP/Client/SFTPFile.swift

@@ -40,7 +40,9 @@ public final class SFTPFile {
     public var logger: Logging.Logger { self.client.logger }
 
     deinit {
-        assert(!self.client.isActive || !self.isActive, "SFTPFile deallocated without being closed first")
+        if client.isActive && self.isActive {
+            self.logger.warning("SFTPFile deallocated without being closed first")
+        }
     }
 
     /// Read the attributes of the file. This is equivalent to the `stat()` system call.

Sources/Citadel/SFTP/SFTPFileFlags.swift

@@ -1,7 +1,7 @@
 import Foundation
 
 // pflags
-public struct SFTPOpenFileFlags: OptionSet, CustomDebugStringConvertible {
+public struct SFTPOpenFileFlags: OptionSet, CustomDebugStringConvertible, Sendable {
     public var rawValue: UInt32
 
     public init(rawValue: UInt32) {

Sources/Citadel/SSHConnectionSettings.swift

@@ -24,7 +24,7 @@ internal struct _SSHReconnectMode {
     internal static let never = _SSHReconnectMode(mode: .never)
 }
 
-public struct SSHReconnectMode: Equatable {
+public struct SSHReconnectMode: Equatable, Sendable {
     internal enum _Mode {
         case once, always, never
     }

Sources/Citadel/Server.swift

@@ -127,6 +127,7 @@ final class CitadelServerDelegate {
     var sftp: SFTPDelegate?
     var exec: ExecDelegate?
     var shell: ShellDelegate?
+    var directTCPIP: DirectTCPIPDelegate?
 
     fileprivate init() {}
 
@@ -144,7 +145,19 @@ final class CitadelServerDelegate {
             handlers.append(ExecHandler(delegate: exec, username: username))
 
             return channel.pipeline.addHandlers(handlers)
-        case .directTCPIP, .forwardedTCPIP:
+        case .directTCPIP(let request):
+            guard let delegate = directTCPIP else {
+                return channel.eventLoop.makeFailedFuture(CitadelError.unsupported)
+            }
+
+            return channel.pipeline.addHandler(DataToBufferCodec()).flatMap {
+                return delegate.initializeDirectTCPIPChannel(
+                    channel,
+                    request: request,
+                    context: SSHContext(username: username)
+                )
+            }
+        case .forwardedTCPIP:
             return channel.eventLoop.makeFailedFuture(CitadelError.unsupported)
         }
     }
@@ -183,6 +196,10 @@ public final class SSHServer {
     public func enableExec(withDelegate delegate: ExecDelegate) {
         self.delegate.exec = delegate
     }
+
+    public func enableDirectTCPIP(withDelegate delegate: DirectTCPIPDelegate) {
+        self.delegate.directTCPIP = delegate
+    }
 
     public func enableShell(withDelegate delegate: ShellDelegate) {
         self.delegate.shell = delegate
@@ -207,6 +224,8 @@ public final class SSHServer {
         let delegate = CitadelServerDelegate()
 
         let bootstrap = ServerBootstrap(group: group)
+            .serverChannelOption(ChannelOptions.socketOption(.so_reuseaddr), value: 1)
+            .childChannelOption(ChannelOptions.socketOption(.so_reuseaddr), value: 1)
             .childChannelInitializer { channel in
                 var server = SSHServerConfiguration(
                     hostKeys: hostKeys,

Sources/Citadel/TTY/Client/TTY.swift

@@ -58,9 +58,12 @@ final class ExecCommandHandler: ChannelDuplexHandler {
     typealias OutboundOut = SSHChannelData
 
     let logger: Logger
-    let onOutput: (Channel, Output) -> ()
-
-    init(logger: Logger, onOutput: @escaping (Channel, Output) -> ()) {
+    let onOutput: (Channel, Output) -> Void
+
+    init(
+        logger: Logger,
+        onOutput: @escaping (Channel, Output) -> Void
+    ) {
         self.logger = logger
         self.onOutput = onOutput
     }
@@ -160,6 +163,7 @@ extension SSHClient {
         }
 
         var hasReceivedChannelSuccess = false
+        var exitCode: Int?
 
         let handler = ExecCommandHandler(logger: logger) { channel, output in
             switch output {
@@ -168,7 +172,13 @@ extension SSHClient {
             case .stderr(let stderr):
                 streamContinuation.yield(.stderr(stderr))
             case .eof(let error):
-                streamContinuation.finish(throwing: error)
+                if let error {
+                    streamContinuation.finish(throwing: error)
+                } else if let exitCode, exitCode != 0 {
+                    streamContinuation.finish(throwing: CommandFailed(exitCode: exitCode))
+                } else {
+                    streamContinuation.finish()
+                }
             case .channelSuccess:
                 if inShell, !hasReceivedChannelSuccess {
                     let commandData = SSHChannelData(type: .channel,
@@ -177,11 +187,7 @@ extension SSHClient {
                     hasReceivedChannelSuccess = true
                 }
             case .exit(let status):
-                if status == 0 {
-                    streamContinuation.finish()
-                } else {
-                    streamContinuation.finish(throwing: CommandFailed(exitCode: status))
-                }
+                exitCode = status
             }
         }
 

Tests/CitadelTests/Citadel2Tests.swift

@@ -253,5 +253,31 @@ final class Citadel2Tests: XCTestCase {
             _ = try await file.read(from: UInt64(i * 32_768), length: 32_768)
             i += 1
         }
+        try await file.close()
+    }
+
+    func testConnectToOpenSSHServer() async throws {
+        guard
+            let host = ProcessInfo.processInfo.environment["SSH_HOST"],
+            let _port = ProcessInfo.processInfo.environment["SSH_PORT"],
+            let port = Int(_port),
+            let username = ProcessInfo.processInfo.environment["SSH_USERNAME"],
+            let password = ProcessInfo.processInfo.environment["SSH_PASSWORD"]
+        else {
+            throw XCTSkip()
+        }
+
+        let client = try await SSHClient.connect(
+            host: host,
+            port: port,
+            authenticationMethod: .passwordBased(username: username, password: password),
+            hostKeyValidator: .acceptAnything(),
+            reconnect: .never
+        )
+
+        let output = try await client.executeCommand("ls /")
+        XCTAssertFalse(String(buffer: output).isEmpty)
+
+        try await client.close()
     }
 }

Tests/CitadelTests/EndToEndTests.swift

@@ -21,8 +21,6 @@ final class AuthDelegate: NIOSSHServerUserAuthenticationDelegate {
     }
 }
 
-struct InvalidCredentials: Error, Equatable {}
-
 final class EndToEndTests: XCTestCase {
     func runTest<ExpectedError: Error & Equatable>(
         credentials: SSHAuthenticationMethod,
@@ -59,7 +57,7 @@ final class EndToEndTests: XCTestCase {
             case .password(.init(password: "test")) where request.username == "citadel":
                 promise.succeed(.success)
             default:
-                promise.fail(InvalidCredentials())
+                promise.succeed(.failure)
             }
         }
         let server = try await SSHServer.host(
@@ -104,7 +102,7 @@ final class EndToEndTests: XCTestCase {
                 password: "wrong"
             ),
             hostKeyValidator: .acceptAnything(),
-            expectedError: AuthenticationFailed()
+            expectedError: SSHClientError.allAuthenticationOptionsFailed
         )
     }
 
@@ -115,7 +113,7 @@ final class EndToEndTests: XCTestCase {
                 password: "test"
             ),
             hostKeyValidator: .acceptAnything(),
-            expectedError: AuthenticationFailed()
+            expectedError: SSHClientError.allAuthenticationOptionsFailed
         )
     }