Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support an SSH (SFTP) server #16

Merged
merged 25 commits into from
Nov 11, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
12fa76c
Support an SSH (SFTP) server
Joannis Sep 20, 2022
d24936d
Add Exec command handling in SSHServer, move into async/await more. A…
Joannis Sep 22, 2022
fd83a94
Don't generate hostkeys
Joannis Sep 23, 2022
d9716ae
remove unused .map
JaapWijnen Sep 28, 2022
f8c7566
remove double switched on case
JaapWijnen Sep 28, 2022
f40d5f1
update tests so they compile again
JaapWijnen Sep 28, 2022
2a22932
Accept opening a channel
Joannis Oct 7, 2022
ad137ae
Improved SFTP file reading, much more SFTP server features
Joannis Oct 9, 2022
d9ad237
Fix compiler inferrence error for older Xcodes
Joannis Oct 9, 2022
eb45f7e
Add final SFTPDelegate features
Joannis Oct 10, 2022
90ffb2d
Add open/readdir functions
Joannis Oct 21, 2022
19a5715
Make types explicit, as Linux Swift 5.7 fails
Joannis Oct 21, 2022
2eb9763
Remove inout that fails to compile on Linux
Joannis Oct 21, 2022
e5ad069
Separate SFTPInboundHandler into separate functions
Joannis Oct 21, 2022
d5f9f65
Try enforcing inline(never)
Joannis Oct 21, 2022
48c43e4
Extreme verbosity
Joannis Oct 21, 2022
5d5b3fb
Revert "Extreme verbosity"
Joannis Oct 21, 2022
85e2069
Revert "Try enforcing inline(never)"
Joannis Oct 21, 2022
2756bd1
Fix UID/GID serialization
Joannis Oct 26, 2022
366e359
Remove extra print statements
Joannis Oct 28, 2022
47f41e1
Emit channel errors as logs
Joannis Nov 2, 2022
98c9dca
Support DH-Group14-SHA256
Joannis Nov 2, 2022
d485a38
Log inbound TCP
Joannis Nov 2, 2022
dfdc513
Implement server-side Kexinits
Joannis Nov 9, 2022
13f7f9e
SFTP 3 as minimum requirement
Joannis Nov 9, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions Sources/Citadel/Algorithms/RSA.swift
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,7 @@ extension Insecure.RSA {
//
// let result = message.power(publicExponent, modulus: modulus)
// return EncryptedMessage(rawRepresentation: result.serialize())
// throw CitadelError.unsupported
fatalError()
throw CitadelError.unsupported
}

public func isValidSignature<D: DataProtocol>(_ signature: Signature, for digest: D) -> Bool {
Expand Down Expand Up @@ -266,8 +265,7 @@ extension Insecure.RSA {
//
// return signature.power(privateExponent, modulus: modulus).serialize()
// }
// throw CitadelError.unsupported
fatalError()
throw CitadelError.unsupported
}

internal func generatedSharedSecret(with publicKey: PublicKey, modulus: BigUInt) -> Data {
Expand Down
1 change: 1 addition & 0 deletions Sources/Citadel/Errors.swift
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,6 @@ enum SFTPError: Error {
case connectionClosed
case missingResponse
case fileHandleInvalid
case errorStatus(SFTPMessage.Status)
case unsupportedVersion(SFTPProtocolVersion)
}
47 changes: 1 addition & 46 deletions Sources/Citadel/SFTP/SFTPClient.swift
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ public final class SFTPClient {

let deserializeHandler = ByteToMessageHandler(SFTPMessageParser())
let serializeHandler = MessageToByteHandler(SFTPMessageSerializer())
let sftpInboundHandler = SFTPInboundHandler(responses: responses, logger: logger)
let sftpInboundHandler = SFTPClientInboundHandler(responses: responses, logger: logger)

return channel.pipeline.addHandlers(
SSHChannelDataUnwrapper(),
Expand Down Expand Up @@ -276,48 +276,3 @@ final class SFTPResponses {
}
}
}

final class SFTPInboundHandler: ChannelInboundHandler {
typealias InboundIn = SFTPMessage

let responses: SFTPResponses
let logger: Logger

init(responses: SFTPResponses, logger: Logger) {
self.responses = responses
self.logger = logger
}

func channelRead(context: ChannelHandlerContext, data: NIOAny) {
let message = unwrapInboundIn(data)

self.logger.trace("SFTP IN: \(message.debugDescription)")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this logging line no longer needed in the renamed SFTPClientInboundHandler? (It seems like a copy and rename and this is the only line that is missing in that new file)

//self.logger.trace("SFTP IN: \(message.debugRawBytesRepresentation)")

if !self.responses.isInitialized, case .version(let version) = message {
if version.version != .v3 {
logger.warning("SFTP ERROR: Server version is unrecognized or incompatible: \(version.version.rawValue)")
context.fireErrorCaught(SFTPError.unsupportedVersion(version.version))
} else {
responses.initialized.succeed(version)
}
} else if let response = SFTPResponse(message: message) {
if let promise = responses.responses.removeValue(forKey: response.requestId) {
if case .status(let status) = response, status.errorCode != .ok {
// logged as debug rather than warning because there are many cases in which a protocol error is
// not only nonfatal, but even expected (such as SSH_FX_EOF).
self.logger.debug("SFTP error received: \(status)")
promise.fail(status)
} else {
promise.succeed(response)
}
} else {
self.logger.warning("SFTP response received for nonexistent request, this is a protocol error")
context.fireErrorCaught(SFTPError.noResponseTarget)
}
} else {
self.logger.warning("SFTP received unrecognized response message, this is a protocol error")
context.fireErrorCaught(SFTPError.invalidResponse)
}
}
}
46 changes: 46 additions & 0 deletions Sources/Citadel/SFTP/SFTPClientInboundHandler.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
import Foundation
import NIO
import NIOSSH
import Logging

final class SFTPClientInboundHandler: ChannelInboundHandler {
typealias InboundIn = SFTPMessage

let responses: SFTPResponses
let logger: Logger

init(responses: SFTPResponses, logger: Logger) {
self.responses = responses
self.logger = logger
}

func channelRead(context: ChannelHandlerContext, data: NIOAny) {
let message = unwrapInboundIn(data)

if !self.responses.isInitialized, case .version(let version) = message {
if version.version != .v3 {
logger.warning("SFTP ERROR: Server version is unrecognized or incompatible: \(version.version.rawValue)")
context.fireErrorCaught(SFTPError.unsupportedVersion(version.version))
} else {
responses.initialized.succeed(version)
}
} else if let response = SFTPResponse(message: message) {
if let promise = responses.responses.removeValue(forKey: response.requestId) {
if case .status(let status) = response, status.errorCode != .ok {
// logged as debug rather than warning because there are many cases in which a protocol error is
// not only nonfatal, but even expected (such as SSH_FX_EOF).
self.logger.debug("SFTP error received: \(status)")
promise.fail(status)
} else {
promise.succeed(response)
}
} else {
self.logger.warning("SFTP response received for nonexistent request, this is a protocol error")
context.fireErrorCaught(SFTPError.noResponseTarget)
}
} else {
self.logger.warning("SFTP received unrecognized response message, this is a protocol error")
context.fireErrorCaught(SFTPError.invalidResponse)
}
}
}
23 changes: 21 additions & 2 deletions Sources/Citadel/SFTP/SFTPFile.swift
Original file line number Diff line number Diff line change
Expand Up @@ -97,10 +97,19 @@ public final class SFTPFile {
let sliceLength = 32_000 // https://github.com/apple/swift-nio-ssh/issues/99

while data.readableBytes > 0, let slice = data.readSlice(length: Swift.min(sliceLength, data.readableBytes)) {
_ = try await self.client.sendRequest(.write(.init(
let result = try await self.client.sendRequest(.write(.init(
requestId: self.client.allocateRequestId(),
handle: self.handle, offset: offset + UInt64(data.readerIndex) - UInt64(slice.readableBytes), data: slice
)))

guard case .status(let status) = result else {
throw SFTPError.invalidResponse
}

guard status.errorCode == .ok else {
throw SFTPError.errorStatus(status)
}

self.logger.debug("SFTP wrote \(slice.readableBytes) @ \(Int(offset) + data.readerIndex - slice.readableBytes) to file \(self.handle.sftpHandleDebugDescription)")
}

Expand All @@ -120,8 +129,18 @@ public final class SFTPFile {
}

self.logger.debug("SFTP closing and invalidating file \(self.handle.sftpHandleDebugDescription)")

self.isActive = false
_ = try await self.client.sendRequest(.closeFile(.init(requestId: self.client.allocateRequestId(), handle: self.handle)))
let result = try await self.client.sendRequest(.closeFile(.init(requestId: self.client.allocateRequestId(), handle: self.handle)))

guard case .status(let status) = result else {
throw SFTPError.invalidResponse
}

guard status.errorCode == .ok else {
throw SFTPError.errorStatus(status)
}

self.logger.debug("SFTP closed file \(self.handle.sftpHandleDebugDescription)")
}
}
Expand Down
5 changes: 5 additions & 0 deletions Sources/Citadel/SFTP/SFTPFileFlags.swift
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,11 @@ public struct SFTPFileAttributes: CustomDebugStringConvertible {
// let extended_count: UInt32?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two TODO's above are already public api?


public static let none = SFTPFileAttributes()
public static let all: SFTPFileAttributes = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer a more descriptive name since SFTPFileAttributes holds more than readwrite permissions so .all could be a little confusing in that case.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should really be implementing this for permissions: http://learn.leighcotnoir.com/wp-content/uploads/2016/08/POSIX-file-permissions.pdf as optionset . Then we don't need this .all

var attr = SFTPFileAttributes()
attr.permissions = 777
return attr
}()

public var debugDescription: String { "unimplemented" }
}
Expand Down
66 changes: 59 additions & 7 deletions Sources/Citadel/SFTP/SFTPMessage.swift
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ enum SFTPResponse {
self = .data(message)
case .mkdir(let message):
self = .mkdir(message)
case .openFile, .closeFile, .read, .write, .initialize, .version:
case .attributes, .openFile, .closeFile, .read, .write, .initialize, .version, .stat, .lstat:
return nil
}
}
Expand Down Expand Up @@ -234,6 +234,40 @@ public enum SFTPMessage {
fileprivate var debugVariantWithoutLargeData: Self { self }
}

public struct Stat: SFTPMessageContent {
public static let id = SFTPMessageType.stat

public let requestId: UInt32
public let path: String

public var debugDescription: String { "{\(self.requestId)}('\(self.path)'" }

fileprivate var debugVariantWithoutLargeData: Self { self }
}

public struct LStat: SFTPMessageContent {
public static let id = SFTPMessageType.lstat

public let requestId: UInt32
public let path: String

public var debugDescription: String { "{\(self.requestId)}('\(self.path)'" }

fileprivate var debugVariantWithoutLargeData: Self { self }
}

public struct Attributes: SFTPMessageContent {
public static let id = SFTPMessageType.attributes

public let requestId: UInt32
public let attributes: SFTPFileAttributes

public var debugDescription: String { "{\(self.requestId)}('\(self.attributes)'" }

fileprivate var debugVariantWithoutLargeData: Self { self }
}


/// Client.
///
/// Starts SFTP session and indicates client version.
Expand Down Expand Up @@ -286,13 +320,26 @@ public enum SFTPMessage {
/// No response, directory gets created or an error is thrown.
case mkdir(MkDir)

case stat(Stat)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally some nice comments like the other enums :) (Since I don't immediately know what stat and lstat are or what attributes we're talking about here)

case lstat(LStat)
case attributes(Attributes)

public var messageType: SFTPMessageType {
switch self {
case .initialize(let message as SFTPMessageContent), .version(let message as SFTPMessageContent),
.openFile(let message as SFTPMessageContent), .closeFile(let message as SFTPMessageContent),
.read(let message as SFTPMessageContent), .write(let message as SFTPMessageContent),
.handle(let message as SFTPMessageContent), .status(let message as SFTPMessageContent),
.data(let message as SFTPMessageContent), .mkdir(let message as SFTPMessageContent):
case
.initialize(let message as SFTPMessageContent),
Joannis marked this conversation as resolved.
Show resolved Hide resolved
.version(let message as SFTPMessageContent),
.openFile(let message as SFTPMessageContent),
.closeFile(let message as SFTPMessageContent),
.read(let message as SFTPMessageContent),
.write(let message as SFTPMessageContent),
.handle(let message as SFTPMessageContent),
.status(let message as SFTPMessageContent),
.data(let message as SFTPMessageContent),
.mkdir(let message as SFTPMessageContent),
.stat(let message as SFTPMessageContent),
.lstat(let message as SFTPMessageContent),
.attributes(let message as SFTPMessageContent):
return message.id
}
}
Expand All @@ -303,7 +350,9 @@ public enum SFTPMessage {
.openFile(let message as SFTPMessageContent), .closeFile(let message as SFTPMessageContent),
.read(let message as SFTPMessageContent), .write(let message as SFTPMessageContent),
.handle(let message as SFTPMessageContent), .status(let message as SFTPMessageContent),
.data(let message as SFTPMessageContent), .mkdir(let message as SFTPMessageContent):
.data(let message as SFTPMessageContent), .mkdir(let message as SFTPMessageContent),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer these on their own lines as well same as above.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me know if you agree on style things like that btw

.stat(let message as SFTPMessageContent), .lstat(let message as SFTPMessageContent),
.attributes(let message as SFTPMessageContent):
return "\(message.id)\(message.debugDescription)"
}
}
Expand All @@ -320,6 +369,9 @@ public enum SFTPMessage {
case .status(let message): return Self.status(message.debugVariantWithoutLargeData)
case .data(let message): return Self.data(message.debugVariantWithoutLargeData)
case .mkdir(let message): return Self.mkdir(message.debugVariantWithoutLargeData)
case .stat(let message): return Self.stat(message.debugVariantWithoutLargeData)
case .lstat(let message): return Self.lstat(message.debugVariantWithoutLargeData)
case .attributes(let message): return Self.attributes(message.debugVariantWithoutLargeData)
}
}

Expand Down
51 changes: 48 additions & 3 deletions Sources/Citadel/SFTP/SFTPMessageParser.swift
Original file line number Diff line number Diff line change
Expand Up @@ -166,9 +166,54 @@ struct SFTPMessageParser: ByteToMessageDecoder {
data: data.slice()
)
)
case .name, .attributes, .lstat, .fstat, .setstat, .fsetstat, .opendir, .readdir, .remove, .mkdir, .rmdir,
.realpath, .stat, .readlink, .symlink, .extended, .extendedReply:
fatalError("TODO")
case .stat:
guard
let requestId = payload.readInteger(as: UInt32.self),
let path = payload.readSSHString()
else {
throw SFTPError.invalidPayload(type: type)
}

message = .stat(
.init(
requestId: requestId,
path: path
)
)
case .mkdir:
guard
let requestId = payload.readInteger(as: UInt32.self),
let path = payload.readSSHString(),
let attributes = payload.readSFTPFileAttributes()
else {
throw SFTPError.invalidPayload(type: type)
}

message = .mkdir(
.init(
requestId: requestId,
filePath: path,
attributes: attributes
)
)
case .lstat:
guard
let requestId = payload.readInteger(as: UInt32.self),
let path = payload.readSSHString()
else {
throw SFTPError.invalidPayload(type: type)
}

message = .lstat(
.init(
requestId: requestId,
path: path
)
)
case .name, .attributes, .fstat, .setstat, .fsetstat, .opendir, .readdir, .remove, .rmdir,
.realpath, .readlink, .symlink, .extended, .extendedReply:
print(type)
throw SFTPError.invalidPayload(type: type)
}

context.fireChannelRead(wrapInboundOut(message))
Expand Down
12 changes: 12 additions & 0 deletions Sources/Citadel/SFTP/SFTPSerializer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,18 @@ final class SFTPMessageSerializer: MessageToByteEncoder {
out.writeInteger(mkdir.requestId)
out.writeSSHString(mkdir.filePath)
out.writeSFTPFileAttribues(mkdir.attributes)
case .stat(let stat):
out.writeInteger(SFTPMessage.Stat.id.rawValue)
out.writeInteger(stat.requestId)
out.writeSSHString(stat.path)
case .lstat(let lstat):
out.writeInteger(SFTPMessage.LStat.id.rawValue)
out.writeInteger(lstat.requestId)
out.writeSSHString(lstat.path)
case .attributes(let fstat):
out.writeInteger(SFTPMessage.Attributes.id.rawValue)
out.writeInteger(fstat.requestId)
out.writeSFTPFileAttribues(fstat.attributes)
}

let length = out.writerIndex - lengthIndex - 4
Expand Down
Loading