impl: support for configuring SSH connection timeout

[fioan89]

Starting with Toolbox 3.3 the SSH connection timeout will be enforced by passing the value to the SSH command. Up until today the Coder plugin relied only on the default value (10 seconds) which was anyway ignored by Toolbox (i.e. it was not passed to the SSH command)

On the other hand we generate the SSH config with ConnectTimeout = 0 which effectively means whatever the OS configured. Once Toolbox 3.3 is released the command parameter will take precedence over the SSH config timeout.

With his PR there is now a user configurable setting via the UI Settings page which defaults to 10 seconds, and it is passed to both the command line but also to the config file.

• resolves https://youtrack.jetbrains.com/issue/TBX-17312

[code-asher]

Makes sense to me except I thought ConnectTimeout = 0 meant to wait indefinitely. Should we default to zero to maintain that behavior?

The man page is not clear on the behavior of zero but the source code shows it blocks indefinitely:

https://github.com/openssh/openssh-portable/blob/master/misc.c#L419-L420
https://github.com/openssh/openssh-portable/blob/master/misc.c#L431-L433

[fioan89]

This is what Linux man pages say:

ConnectTimeout
Specifies the timeout (in seconds) used when connecting to the SSH server, instead of using the default system TCP timeout. This value is used only when the target is down or really unreachable, not when it refuses the connection.

https://linux.die.net/man/5/ssh_config
That is what I meant by whatever the OS configured. It really depends how the TCP layer is configured.

Should we default to zero to maintain that behavior?

That is a good question, I went in with the "default" value that Toolbox had but which it never enforced.

[code-asher]

Hrm yeah my confusion was that if you insert a timeout of zero into that text, the literal reading is "zero [is] used [...] instead of using the default system TCP timeout", which seems to suggest the default system timeout is ignored in that case. If zero is a special case, they fail to mention it in the manpage.

Apparently ConnectTimeout=none is also valid and would have the same effect? It seems semantically clearer than zero which is nice. I think we may want that or 0 so we are not accidentally terminating connections that used to go through before.