SFTP subsystem support — architecture proposal

[GlassOnTin]

Background

Haven (an Android SSH/SFTP client) currently uses JSch for SFTP and I'd like to contribute SFTP support to ssh-proto. Per our discussion in GlassHaven/Haven#58, opening this issue first to discuss architecture before starting work.

Proposed architecture

Layer 1: Subsystem channel request

Add requestSubsystem(name: String): Boolean to SshSession, following the same pattern as requestShell() / requestExec(). The Kaitai definition (channel_request_subsystem.ksy) already exists. This is a small, independent piece that could be a separate PR.

Layer 2: SFTP state machine

An SftpClient class that drives the SFTP protocol over an SSH session channel:

SshSession (channel with "sftp" subsystem)
    └── SftpClient (state machine, packet framing, request/response correlation)
            ├── SftpPacketIO (reads/writes SFTP packets: length-prefixed type+id+payload)
            └── Request/response correlation (maps request IDs to pending completions)

Key design decisions:

1. Packet framing: SFTP messages are length-prefixed (uint32 length + type + request_id + payload). This sits on top of SshSession.write()/read() — the SFTP client reassembles SSH channel data into complete SFTP packets.

2. Request/response correlation: SFTP is multiplexed — multiple requests can be in flight. Each request carries a unique id, and the response echoes it. A ConcurrentHashMap<UInt, CompletableDeferred<SftpResponse>> or similar structure maps request IDs to pending completions.

3. State machine: Minimal states — Init (version negotiation), Ready (accepting requests), Closed. Version negotiation happens on SSH_FXP_INIT / SSH_FXP_VERSION exchange.

4. Version support: Start with SFTPv3 (RFC draft-ietf-secsh-filexfer-02) as the baseline — it's what OpenSSH implements. Version negotiation picks min(client_version, server_version). I'd structure the code so v4/v5/v6 extensions can be added later without breaking the v3 API.

Layer 3: High-level API

interface SftpClient : AutoCloseable {
    val protocolVersion: Int

    suspend fun stat(path: String): SftpFileAttributes
    suspend fun lstat(path: String): SftpFileAttributes
    suspend fun fstat(handle: SftpHandle): SftpFileAttributes

    suspend fun opendir(path: String): SftpHandle
    suspend fun readdir(handle: SftpHandle): List<SftpDirEntry>?
    suspend fun listdir(path: String): List<SftpDirEntry>  // convenience

    suspend fun open(path: String, flags: Int, attrs: SftpFileAttributes = SftpFileAttributes.EMPTY): SftpHandle
    suspend fun read(handle: SftpHandle, offset: Long, length: Int): ByteArray?
    suspend fun write(handle: SftpHandle, offset: Long, data: ByteArray)
    suspend fun close(handle: SftpHandle)

    suspend fun remove(path: String)
    suspend fun mkdir(path: String, attrs: SftpFileAttributes = SftpFileAttributes.EMPTY)
    suspend fun rmdir(path: String)
    suspend fun rename(oldPath: String, newPath: String)

    suspend fun realpath(path: String): String
    suspend fun readlink(path: String): String
    suspend fun symlink(linkPath: String, targetPath: String)

    suspend fun setstat(path: String, attrs: SftpFileAttributes)
    suspend fun fsetstat(handle: SftpHandle, attrs: SftpFileAttributes)
}

Protocol definitions

I'd add Kaitai .ksy files for the SFTP packet types (init, version, open, close, read, write, stat, etc.) following the existing pattern. These would live in the protocol module.

Module placement

SFTP could either:

• A) Live in the sshlib module alongside the SSH channel code (since it's tightly coupled to SshSession)
• B) Be a separate sftp module that depends on sshlib

I'd lean toward (A) for simplicity, unless you prefer the separation.

Implementation plan

1. PR: subsystem channel request — requestSubsystem() on SshSession
2. PR: SFTP packet framing + version negotiation — SftpPacketIO, init/version exchange
3. PR: SFTP operations — file operations, directory listing, stat, etc.

Happy to adjust any of this based on your preferences. What are your thoughts on the state machine approach and module placement?

[kruton]

That looks like a good plan. I think integrating it with the main module makes sense as well. I don't think this will cause an unreasonable increase in the size of the compiled library. Kotlin's Dead Code Elimination pass can remove it for users that don't use the functionality.

I'd also pay attention to the API surface added just to make sure only things useful to library users are added to the publicly visible API surface. You can monitor this with the Metalava tasks (i.e., see if ./gradlew metalavaGenerateSignature adds anything unexpected to sshlib/api.txt).

It's not explicitly stated in your plan, but make sure there are tests for the newly added code. Separating the code to make it easily unit testable was a goal in using KStateMachine. Creating SFTP tests as a separate OpenSSH integration test would make it more manageable at the expense of spinning up the Docker container once more per test run. If there is a popular SFTP server besides OpenSSH, consider trying to make another integration test with Test Containers using a Dockerfile definition to build the server.

[GlassOnTin]

Thanks for the review — that's all clear.

Plan of action:

1. PR: requestSubsystem() — small addition to SshSession/SessionChannel, same pattern as exec (PR Add exec channel request support (RFC 4254 §6.5) #95). This unblocks SFTP without adding bulk.
2. PR: SFTP packet framing + version negotiation — SftpClient in sshlib module, KStateMachine for the SFTP state machine (Init → Ready → Closed), request/response correlation via ConcurrentHashMap<UInt, CompletableDeferred>. Internal classes package-private, only the SftpClient interface in the public API.
3. PR: SFTP operations — file ops, directory listing, stat, etc.

Will use metalavaGenerateSignature to keep the API surface clean. Integration tests against OpenSSH testcontainer, and I'll look at AsyncSSH as a second server for cross-implementation testing.

Starting with the subsystem PR once #95 is merged.