This paves the way for me to reorganise ssh.c in a way that will mean
I don't have a ConnectionLayer available yet at the time I have to
create the connshare. The constructor function now takes a mere
Frontend, for generating setup-time Event Log messages, and there's a
separate ssh_connshare_provide_connlayer() function I can call later
once I have the ConnectionLayer to provide.
NFC for the moment: the new provide_connlayer function is called
immediately after ssh_connection_sharing_init.
This is a vtable that wraps up all the functionality required from the
SSH connection layer by associated modules like port forwarding and
connection sharing. This extra layer of indirection adds nothing
useful right now, but when I later separate the SSH-1 and SSH-2
connection layer implementations, it will be convenient for each one
to be able to implement this vtable in terms of its own internal data
structures.
To simplify this vtable, I've moved a lot of the logging duties
relating to connection sharing out of ssh.c into sshshare.c: now it
handles nearly all the logging itself relating to setting up
connection sharing in the first place and downstreams connecting and
disconnecting. The only exception is the 'Reusing a shared connection'
announcement in the console window, which is now done in ssh.c by
detecting downstream status immediately after setup.
The tree234 storing currently active port forwardings - both local and
remote - now lives in portfwd.c, as does the complicated function that
updates it based on a Conf listing the new set of desired forwardings.
Local port forwardings are passed to ssh.c via the same route as
before - once the listening port receives a connection and portfwd.c
knows where it should be directed to (in particular, after the SOCKS
exchange, if any), it calls ssh_send_port_open.
Remote forwardings are now initiated by calling ssh_rportfwd_alloc,
which adds an entry to the rportfwds tree (which _is_ still in ssh.c,
and still confusingly sorted by a different criterion depending on SSH
protocol version) and sends out the appropriate protocol request.
ssh_rportfwd_remove cancels one again, sending a protocol request too.
Those functions look enough like ssh_{alloc,remove}_sharing_rportfwd
that I've merged those into the new pair as well - now allocating an
rportfwd allows you to specify either a destination host/port or a
sharing context, and returns a handy pointer you can use to cancel the
forwarding later.
This was a particularly confusing piece of type-danger, because three
different types were passed outside sshshare.c as 'void *' and only
human vigilance prevented one coming back as the wrong one. Now they
all keep their opaque structure tags when they move through other
parts of the code.
It was horrible - even if harmless in practice - that it wrote the
NATed channel id over its input buffer, and I think it's worth the
extra memory management to avoid doing that.
This formalises my occasional habit of using a single malloc to make a
block that contains a header structure and a data buffer that a field
of the structure will point to, allowing it to be freed in one go
later. Previously I had to do this by hand, losing the type-checking
advantages of snew; now I've written an snew-style macro to do the
job, plus an accessor macro to cleanly get the auxiliary buffer
pointer afterwards, and switched existing instances of the pattern
over to using that.
Another piece of half-finished machinery that I can't have tested
properly when I set up connection sharing: I had the function
ssh_alloc_sharing_rportfwd which is how sshshare.c asks ssh.c to start
sending it channel-open requests for a given remote forwarded port,
but I had no companion function that removes one of those requests
again when a downstream remote port forwarding goes away (either by
mid-session cancel-tcpip-forward or by the whole downstream
disconnecting).
As a result, the _second_ attempt to set up the same remote port
forwarding, after a sharing downstream had done so once and then
stopped, would quietly fail.
This is another bug that must have been around since connection
sharing was introduced, and nobody noticed until I did some unusually
thorough testing yesterday.
When a sharing downstream asks to set up a remote port forwarding, we
pass through the "tcpip-forward" global request, and we also intercept
the reply so that we know that the forwarding has been set up (and
hence that we should be passing "forwarded-tcpip" channel opens for
that port to this downstream). To do that, we set the want-reply flag
in the version of the packet we pass to the server, even if it was
clear in downstream's version; and we also put an item on a queue
local to sshshare.c which reminds us what to do about the reply when
it comes back.
But when the downstream _cancels_ one of those forwardings, I wrote
the code for all parts of that process except adding that queue item.
I even wrote the code to _consume_ the queue item, but somehow I
completely forgot to generate one in the first place! So the enum
value GLOBREQ_CANCEL_TCPIP_FORWARD was declared, tested for, but never
actually assigned to anything.
Another set of localised decoding routines get thrown away here. Also,
I've changed the APIs of a couple of helper functions in x11fwd.c to
take ptrlens in place of zero-terminated C strings, because that's the
format in which they come back from the decode, and it saves mallocing
a zero-terminated version of each one just to pass to those helpers.
In the course of reworking the socket vtable system, I noticed that
both sshshare.c and x11fwd.c independently invented the idea of a Plug
none of whose methods do anything. We don't need more than one of
those, so let's centralise the idea to somewhere it can be easily
reused.
Now I've got FROMFIELD, I can rework it so that structures providing
an implementation of the Socket or Plug trait no longer have to have
the vtable pointer as the very first thing in the structure. In
particular, this means that the ProxySocket structure can now directly
implement _both_ the Socket and Plug traits, which is always
_logically_ how it's worked, but previously it had to be implemented
via two separate structs linked to each other.
Another big pile of packet-construction now looks simpler and nicer,
although - as with the agent messages - I've done that tiny cheat of
filling in the length field at the start of the packet frame at the
very end of processing.
Forgot to remove this after debugging a development-time problem with
the new EPIPE special case. One of these days I'm going to have to set
up an automated way to protect against this kind of mistake...
If you use the new 'plink -shareexists' feature, then on Unix at least
it's possible for the upstream to receive EPIPE, because the
downstream makes a test connection and immediately closes it, so that
upstream fails to write its version string.
This looks a bit ugly in the upstream's Event Log, so I'm making a
special case: an error of 'broken pipe' type, which occurs on a socket
from a connection sharing downstream, before we've received a version
string from that downstream, is treated as an unusual kind of normal
connection termination and not logged as an error.
A Plink invocation of the form 'plink -shareexists <session>' tests
for a currently live connection-sharing upstream for the session in
question. <session> can be any syntax you'd use with Plink to make the
actual connection (a host/port number, a bare saved session name,
-load, whatever).
I envisage this being useful for things like adaptive proxying - e.g.
if you want to connect to host A which you can't route to directly,
and you might already have a connection to either of hosts B or C
which are viable proxies, then you could write a proxy shell script
which checks whether you already have an upstream for B or C and goes
via whichever one is currently active.
Testing for the upstream's existence has to be done by actually
connecting to its socket, because on Unix the mere existence of a
Unix-domain socket file doesn't guarantee that there's a process
listening to it. So we make a test connection, and then immediately
disconnect; hence, that shows up in the upstream's event log.
For each connection to a downstream I had a flag indicating that we'd
sent a version string to that downstream, and one indicating that we'd
received one in return. But I never actually set the latter to TRUE -
which was OK, as it turned out, because I never used it for anything
either.
Now I do want to use it, so I'd better actually set it :-)
This is the part of ssh_connection_sharing_init() which decides on the
identifying string to pass to the platform sharing setup. I'm about to
want to use it for another purpose, so it needs to be moved into a
separate function.
If the real SSH connection goes away and we call sharestate_free with
downstreams still active, then that in turn calls share_connstate_free
on all those downstreams, freeing the things their sockets are using
as Plugs but not actually closing the sockets, so further data coming
in from downstream gives rise to a use-after-free bug.
(Thanks to Timothe Litt for a great deal of help debugging this.)
When anyone connects to a PuTTY tool's listening socket - whether it's
a user of a local->remote port forwarding, a connection-sharing
downstream or a client of Pageant - we'd like to log as much
information as we can find out about where the connection came from.
To that end, I've implemented a function sk_peer_info() in the socket
abstraction, which returns a freeform text string as best it can (or
NULL, if it can't get anything at all) describing the thing at the
other end of the connection. For TCP connections, this is done using
getpeername() to get an IP address and port in the obvious way; for
Unix-domain sockets, we attempt SO_PEERCRED (conditionalised on some
moderately hairy autoconfery) to get the pid and owner of the peer. I
haven't implemented anything for Windows named pipes, but I will if I
hear of anything useful.
If a sharing downstream asks for an auth method we don't understand,
we should send them CHANNEL_FAILURE *and then stop processing*. Ahem.
(Spotted while examining this code in the course of Coverity-related
fixes, but not itself a Coverity-found problem.)
Without this, doing 'Restart Session' on Windows in a session with
sharing enabled but no actual sharing being done would crash, because
the first incarnation of the session would become an upstream and
establish a listening named pipe, which then wouldn't get cleaned up
when the session closed, so the restarted session would try to connect
to it, triggering a call to plug_accepting on a freed sharestate.
[originally from svn r10216]
The basic strategy is described at the top of the new source file
sshshare.c. In very brief: an 'upstream' PuTTY opens a Unix-domain
socket or Windows named pipe, and listens for connections from other
PuTTYs wanting to run sessions on the same server. The protocol spoken
down that socket/pipe is essentially the bare ssh-connection protocol,
using a trivial binary packet protocol with no encryption, and the
upstream has to do some fiddly transformations that I've been
referring to as 'channel-number NAT' to avoid resource clashes between
the sessions it's managing.
This is quite different from OpenSSH's approach of using the Unix-
domain socket as a means of passing file descriptors around; the main
reason for that is that fd-passing is Unix-specific but this system
has to work on Windows too. However, there are additional advantages,
such as making it easy for each downstream PuTTY to run its own
independent set of port and X11 forwardings (though the method for
making the latter work is quite painful).
Sharing is off by default, but configuration is intended to be very
easy in the normal case - just tick one box in the SSH config panel
and everything else happens automatically.
[originally from svn r10083]
Simon Tatham
Ben Harris