Support SSH tunnels with dynamic free port allocation

[gschaffner]

Hi! This adds two new features:

• Reverse (Open)SSH tunnels with dynamically allocated remote ports. E.g.

  > tun = SshMachine(...).tunnel(1234, 0, reverse=True)
  > tun.dport
  '43385'

  Dynamic remote port allocation is done by OpenSSH (ssh -R).

• Non-reverse and reverse (Open)SSH tunnels with dynamically allocated local ports. E.g.

  > tun = SshMachine(...).tunnel(0, 1234)
  > tun.lport
  37689

  Dynamic local port allocation is done by rpyc.utils.factory._get_free_port, which I've duplicated into Plumbum. (Minor point: license-wise, rpyc.utils.factory._get_free_port is released by @tomerfiliba under the MIT license already. In theory Plumbum should add attribution to satisfy RPyC's license here, but I suspect that Tomer probably would not mind as they authored both projects.)

This also adds a reverse property to SshTunnel. lhost and dhost properties could also be added if deemed useful—I imagine the use primary cases there would be to check what address the tunnel was bound to or if the tunnel is for ports or sockets.

This PR probably should also update SshMachine().tunnel's docs. Note that #562 did not update docs for reverse.

[coveralls]

Coverage increased (+0.09%) to 83.758% when pulling fc968c5 on gschaffner:dynamic-ssh-tunnel-ports into 70380dd on tomerfiliba:master.

[henryiii]

What's the use for this? Would a bool property "started" or something like that be better and less tied to the implementation?

[gschaffner]

The stderr saved in ShellSession().startup_result gets parsed by SshTunnel().__init__() to get the port number than OpenSSH has bound on the remote end of a reverse tunnel.

For example, for SshMachine().tunnel(1234, 0, reverse=True), the underlying ssh -R 0:localhost:1234 ... call will have OpenSSH allocate a free remote port and report it on the stderr of the ssh -R command. This command is executed by self.run("") during ShellSession.__init__.

I agree that this is tied to the implementation in a confusing way, but I don't think that ssh -R ...'s stderr can be captured other than by saving the return value of that initial ShellSession().run(""). The alternative would be to have SshTunnel().__init__() or SshMachine().tunnel() run the OpenSSH command that initializes the tunneled session, but that would break ShellSessions that aren't created by SshMachine().tunnel().

[henryiii]

Okay, I'd think you could do the parsing here and store the interesting information rather than the raw stderr. I'd assume this at least is not valid for Paramiko machine?

[gschaffner]

Yep, ParamikoMachine doesn't support tunnel. (At a glance it does look possible to add support for ParamikoMachine tunnels in the future though; see https://github.com/paramiko/paramiko/blob/1824a27c644132e5d46f2294c1e2fa131c523559/demos/forward.py and https://github.com/paramiko/paramiko/blob/1824a27c644132e5d46f2294c1e2fa131c523559/demos/rforward.py.)

[gschaffner]

We could have ShellSession().__init__ do the parsing and save a reverse_tunnel_detected_rport property rather than the current startup_result property. I didn't do that initially since I think it pushes the port detection logic even further away from SshMachine().tunnel and SshTunnel().__init__ and is convoluted, but I can move it if you prefer.

[henryiii]

Would it be okay to leave this with hidden only access for now? _startup_result? Just to give us the option to do this in the future?

[gschaffner]

Yeah! Definitely makes sense to not make it public API. Addressed with a fixup: https://github.com/tomerfiliba/plumbum/compare/fd767673b90f48682369a307b10fd1cbcd92a5e9..f55d299b335ba3d4842d7da24593d08a943c0795

[henryiii]

Would you mind adding some typing? I know we don't fully type yet (or have much at all), but it's nice on new API and I think this one would be better with a -> bool on it.

(Not a blocker for the PR, though)

[gschaffner]

I would love to see Plumbum get more typing.

I had held off on typing this since the types of these new SshTunnel() properties need to match the types for SshMachine().tunnel's arguments, which are currently untyped. My thought was that these properties should not be typed until the SshMachine().tunnel args that they come from get typed. If you want I can add typing to both SshMachine().tunnel and these properties. If so, a couple questions:

Should e.g. ports be typed as int | str or as some Port type that Plumbum can reuse elsewhere (plumbum.lib.ptyping.Port?)? If some TypeAlias Port = int | str is added, should lport be Port or (redundantly) str | Port to hint that lport can be a socket string too, not just a port string?

[henryiii]

I'd be fine to have a plumbum.typing module with some good reusable types, though I think this one is simple enough (and actually helps in reading) to leave Union[int, str] (or "int | str"; sadly we are still Python 3.6 compatible).

Than can be a followup PR if you'd like to help with it. :) I've been using monkeytype to try to get a head start on typing, but I haven't had much time to work on it (and won't till after SciPy & Snowmass, so mostly out all this month).

[gschaffner]

I would like to help with this, but I also won't have time for a while :/