`SshChannel.Close` methods use `ExitSignal` instead of `Signal`

[cmbrose]

Prefacing this with - I'm not sure if this is actually a bug or just an oddity of the SDK

The SshChannel.Close methods (C# and TS) have options to pass a signal. In that case, the signal is sent as an exit-signal message before doing the main close operations.

I interpret this parameter as a way to send a signal to the remote process as a way to ask it to stop, otherwise the Close will just break the connection, but the remote process will continue running. If that is the correct interpretation, then this should be sent as a signal message instead of an exit-signal.

Regardless, if I read the relevant portion of the SSH RFC correctly, it seems like exit-signal should only be coming from the remote side, not the client. So it's misleading to have the signal parameter as an option because it doesn't really do anything it seems.

[jasongin]

I interpret this parameter as a way to send a signal to the remote process as a way to ask it to stop,

This is not the correct interpretation. But I can understand the confusion since it is not clarified in the doc-comments.

The purpose of that API is for the server side to close the channel with a signal, where that signal indicates something that prevented the channel from closing normally (with an exit code). For example, an exec command that crashes with a seg-fault could close the channel with a SIGSEGV signal.

If the client intends to send a signal to the server to cause the channel to close, then it should send a separate ChannelSignalMessage, then either close the channel or wait for the server to close it.

[cmbrose]

Thanks for the clarification @jasongin, that makes a lot of sense. It sounds like the issue stems from the same SshChannel being used by both client and server sides, but it's interface has the ability to try to send messages which are only valid for one those.

That could be improved with docs to explain that client apps shouldn't use .close('<signal>'), but I wonder if it would be better to be more defensive in the code itself? When the exit-signal message is sent by the SshChannel (here) it actually routes it directly through its session, rather than itself. I wonder if either SshChannel at that point could check the session to see if it's an SshClientSession and either throw or ignore the signal value? Alternatively SshClientSession could check that someone tried to send an exit-signal message through it and block it there instead.

WDYT?

[jasongin]

check the session to see if it's an SshClientSession and either throw or ignore the signal value

I prefer to avoid doing things like that because actually almost everything in the SSH protocol (outside of the key-exchange) is designed to be bi-directional. Theoretically, a client could allow a server to open a channel to exec a command on the client, and then the client would need to be able to send exit signals. In practice I don't know of any application that works that way, but that doesn't mean it should be blocked in the library. (It's not a security issue since such a request form a server would be denied unless the client application specifically handles it.)

[cmbrose]

Makes sense, I think it makes sense to close this one out as by design in that case since all the tools are already there.