If values are boolean, it's confusing to use & and | in place of &&
and ||. In two of these three cases it was simply a typo and I've used
the other one; in the third, it was a deliberate avoidance of short-
circuit evaluation (and commented as such), but having seen how easy
it is to make the same typo by accident, I've decided it's clearer to
just move the LHS and RHS evaluations outside the expression.
In some compiler modes - notably the one that gtk-config selects when
GTK PuTTY is built for GTK 1 - it's an error to typedef the same thing
twice. 'mainchan' is defined in defs.h, so it doesn't need defining
again where the structure contents are specified.
x11_char_struct returns a pointer or NULL, so while returning FALSE
would have ended up doing the right thing after macro expansion and
integer->pointer conversion, it wasn't actually how I _meant_ to spell
a failure return.
In GTK 2, this function was a new and convenient way to override the
order in which the Tab key cycled through the sub-widgets of a
container, replacing the more verbose mechanism in GTK 1 where you had
to provide a custom implementation of the "focus" method in
GtkContainerClass.
GTK 3.24 has now deprecated gtk_container_set_focus_chain(),
apparently on the grounds that that old system is what they think you
_ought_ to be doing. So I've abandoned set_focus_chain completely, and
switched back to doing it by a custom focus method for _all_ versions
of GTK, with the only slight wrinkle being that between GTK 1 and 2
the method in question moved from GtkContainer to GtkWidget (my guess
is so that an individual non-container widget can still have multiple
separately focusable UI components).
After the recent Seat and LogContext revamps, _nearly_ all the
remaining uses of the type 'Frontend' were in terminal.c, which needs
all sorts of interactions with the GUI window the terminal lives in,
from the obvious (actually drawing text on the window, reading and
writing the clipboard) to the obscure (minimising, maximising and
moving the window in response to particular escape sequences).
All of those functions are now provided by an abstraction called
TermWin. The few remaining uses of Frontend after _that_ are internal
to a particular platform directory, so as to spread the implementation
of that particular kind of Frontend between multiple source files; so
I've renamed all of those so that they take a more specifically named
type that refers to the particular implementation rather than the
general abstraction.
So now the name 'Frontend' no longer exists in the code base at all,
and everywhere one used to be used, it's completely clear whether it
was operating in one of Frontend's three abstract roles (and if so,
which), or whether it was specific to a particular implementation.
Another type that's disappeared is 'Context', which used to be a
typedef defined to something different on each platform, describing
whatever short-lived resources were necessary to draw on the terminal
window: the front end would provide a ready-made one when calling
term_paint, and the terminal could request one with get_ctx/free_ctx
if it wanted to do proactive window updates. Now that drawing context
lives inside the TermWin itself, because there was never any need to
have two of those contexts live at the same time.
(Another minor API change is that the window-title functions - both
reading and writing - have had a missing 'const' added to their char *
parameters / return values.)
I don't expect this change to enable any particularly interesting new
functionality (in particular, I have no plans that need more than one
implementation of TermWin in the same application). But it completes
the tidying-up that began with the Seat and LogContext rework.
In the very old days, when PuTTY was new and computers were slow, I
tried to implement a feature where scrolling the window would be
implemented using a fast rectangle-copy GDI operation, rather than an
expensive character-by-character redraw of all the changed areas.
It never quite worked right, and I ended up conditioning it out on
Windows, and never even tried to implement it on GTK. It's now been
sitting around unused for so long that I think it's no longer worth
keeping in the code at all - if I tried to put it back in, it surely
wouldn't even compile, and would need rewriting from scratch anyway.
Disturbingly, it looks as if I _tried_ to re-enable it at one point,
in that there was a '#define OPTIMISE_IS_SCROLL 1' in putty.h - but
that never had any effect, because the macro name is misspelled. All
the #ifdefs are for 'OPTIMISE_SCROLL', without the 'IS'. So despite
appearances, it really _has_ been conditioned out all along!
Colin Harrison points out that commit c31e3cd43 was less cautious than
it should have been: when delete_callbacks_for_context nulls out the
'next' pointer in the new tail element of the callbacks list, it
should only do so if there _is_ a new tail element. If the list has
just become empty, that won't work very well!
A function containing assert(FALSE && "Should never be called")
doesn't _really_ need to return a value, but it makes compilers
happier if it pretends to.
Like the SFTP server, this is implemented in-process rather than by
invoking a separate scp server binary.
It also uses the internal SftpServer abstraction for access to the
server's filesystem, which means that when (or if) I implement an
alternative SftpServer implementing a dummy file system for test suite
purposes, this scp server should automatically start using it too.
As a bonus, the new scpserver.c contains a large comment documenting
my understanding of the SCP protocol, which I previously didn't have
even a de-facto or post-hoc spec for. I don't claim it's authoritative
- it's all reverse-engineered from my own code and observing other
implementations in action - but at least it'll make it easier to
refresh my own memory of what's going on the next time I need to do
something to either this code or PSCP.
When you don't specify -r, we now check whether the server is sending
a whole subdirectory in place of a single file, and abort if it does.
Previously we'd accept the subdirectory download regardless.
The new error message labels this as a security violation, just on the
grounds that it involves the server doing something other than what
the implicit contract suggested it ought to, but I don't think it's a
really serious violation in the same sense as letting the server cd
into ".." or overwrite files of arbitrary name would be. In this case
it can only leave a downloaded thing in the specific place you already
authorised it to put _some_ downloaded thing - it's just returned you
a directory in place of a file.
throttled_by_backlog is quite new, so it's not too surprising that I
hadn't already noticed it wasn't initialised. But the failure to null
out the final 'next' pointer in the callbacks list when it's rewritten
as a result of delete_callbacks_for_context is a great deal older, so
I'm a lot more puzzled about how it hasn't come up until now!
The main wildcard matching code now doesn't depend on having a NUL
terminator in the string to be matched; instead it works with a pair
of pointers, one working along the string as we match, and the other
identifying the end of the string, and tests that p < target_end
before dereferencing p. User-facing entry points now allow you to pass
either an ordinary ASCIZ const char * or a ptrlen, and set up
target_end accordingly.
For the moment, the _wildcard_ parameter still has to be an ordinary
null-terminated string, but who knows, maybe that will have to change
too at some later point.
Firstly, these protocols had a display heuristic - credited to OpenSSH
in the comments - in which, if the challenge string contained a
newline, it was supposed to be printed with "Response: " on the next
line, whereas if it didn't, it would be taken as a prompt in its own
right. In fact, I had got the sense of memchr backwards, so each
behaviour was applying in the opposite case.
Secondly, apparently I'd never before tested against a server that
offered _both_ those methods, because when I tried it against Uppity
just now, I found that the setup and challenge phases for both methods
ran in immediate succession before prompting the user, which confused
the server completely. This is exactly why I wanted to have a server
implementation of everything PuTTY is supposed to speak the client
side of!
This adds the server side of the SSH-2 keyboard-interactive protocol,
and the pair of very similar SSH-1 methods AUTH_TIS and AUTH_CCARD
(which basically differ only in message numbers, and each involve a
single challenge from the server and a response from the user).
We now show the --help output if invoked with no arguments, and the
help text also includes a big safety warning in the hope of stopping
anyone from mistaking this for a _secure_ SSH server implementation.
While I'm here, the errors now all use appname[] in place of
constantly repeating the program name. (Not because I anticipate a
change right now, but if nothing else, it makes things easier moving
errors out into shared source files or between applications.)
Naturally, there's one really glaring goof I find out instants after
'git push'! If Pageant starts a watchdog subprocess which will wait
until the main process terminates and then clean up the socket, then
it had better not have that subprocess keep the standard I/O handles
open, or else commands like eval $(pageant -X) won't terminate.
I've applied the same fix in the X11 socket creation, though I think
it's less critical there.
Unlike the traditional Unix SSH server organisation, the SFTP server
is built into the same process as all the rest of the code. sesschan.c
spots a subsystem request for "sftp", and responds to it by
instantiating an SftpServer object and swapping out its own vtable for
one that talks to it.
(I rather like the idea of an object swapping its own vtable for a
different one in the middle of its lifetime! This is one of those
tricks that would be absurdly hard to implement in a 'proper' OO
language, but when you're doing vtables by hand in C, it's no more
difficult than any other piece of ordinary pointer manipulation. As
long as the methods in both vtables expect the same physical structure
layout, it doesn't cause a problem.)
The SftpServer object doesn't deal directly with SFTP packet formats;
it implements the SFTP server logic in a more abstract way, by having
a vtable method for each SFTP request type with an appropriate
parameter list. It sends its replies by calling methods in another
vtable called SftpReplyBuilder, which in the normal case will write an
SFTP reply packet to send back to the client. So SftpServer can focus
more or less completely on the details of a particular filesystem API
- and hence, the implementation I've got lives in the unix source
directory, and works directly with file descriptors and struct stat
and the like.
(One purpose of this abstraction layer is that I may well want to
write a second dummy implementation, for test-suite purposes, with
completely controllable behaviour, and now I have a handy place to
plug it in in place of the live filesystem.)
In between sesschan's parsing of the byte stream into SFTP packets and
the SftpServer object, there's a layer in the new file sftpserver.c
which does the actual packet decoding and encoding: each request
packet is passed to that, which pulls the fields out of the request
packet and calls the appropriate method of SftpServer. It also
provides the default SftpReplyBuilder which makes the output packet.
I've moved some code out of the previous SFTP client implementation -
basic packet construction code, and in particular the BinarySink/
BinarySource marshalling fuinction for fxp_attrs - into sftpcommon.c,
so that the two directions can share as much as possible.
This server is NOT SECURE! If anyone is reading this commit message,
DO NOT DEPLOY IT IN A HOSTILE-FACING ENVIRONMENT! Its purpose is to
speak the server end of everything PuTTY speaks on the client side, so
that I can test that I haven't broken PuTTY when I reorganise its
code, even things like RSA key exchange or chained auth methods which
it's hard to find a server that speaks at all.
(For this reason, it's declared with [UT] in the Recipe file, so that
it falls into the same category as programs like testbn, which won't
be installed by 'make install'.)
Working title is 'Uppity', partly for 'Universal PuTTY Protocol
Interaction Test Yoke', but mostly because it looks quite like the
word 'PuTTY' with part of it reversed. (Apparently 'test yoke' is a
very rarely used term meaning something not altogether unlike 'test
harness', which is a bit of a stretch, but it'll do.)
It doesn't actually _support_ everything I want yet. At the moment,
it's a proof of concept only. But it has most of the machinery
present, and the parts it's missing - such as chained auth methods -
should be easy enough to add because I've built in the required
flexibility, in the form of an AuthPolicy object which can request
them if it wants to. However, the current AuthPolicy object is
entirely trivial, and will let in any user with the password "weasel".
(Another way in which this is not a production-ready server is that it
also has no interaction with the OS's authentication system. In
particular, it will not only let in any user with the same password,
but it won't even change uid - it will open shells and forwardings
under whatever user id you started it up as.)
Currently, the program can only speak the SSH protocol on its standard
I/O channels (using the new FdSocket facility), so if you want it to
listen on a network port, you'll have to run it from some kind of
separate listening program similar to inetd. For my own tests, I'm not
even doing that: I'm just having PuTTY spawn it as a local proxy
process, which also conveniently eliminates the risk of anyone hostile
connecting to it.
The bulk of the actual code reorganisation is already done by previous
commits, so this change is _mostly_ just dropping in a new set of
server-specific source files alongside the client-specific ones I
created recently. The remaining changes in the shared SSH code are
numerous, but all minor:
- a few extra parameters to BPP and PPL constructors (e.g. 'are you
in server mode?'), and pass both sets of SSH-1 protocol flags from
the login to the connection layer
- in server mode, unconditionally send our version string _before_
waiting for the remote one
- a new hook in the SSH-1 BPP to handle enabling compression in
server mode, where the message exchange works the other way round
- new code in the SSH-2 BPP to do _deferred_ compression the other
way round (the non-deferred version is still nicely symmetric)
- in the SSH-2 transport layer, some adjustments to do key derivation
either way round (swapping round the identifying letters in the
various hash preimages, and making sure to list the KEXINITs in the
right order)
- also in the SSH-2 transport layer, an if statement that controls
whether we send SERVICE_REQUEST and wait for SERVICE_ACCEPT, or
vice versa
- new ConnectionLayer methods for opening outgoing channels for X and
agent forwardings
- new functions in portfwd.c to establish listening sockets suitable
for remote-to-local port forwarding (i.e. not under the direction
of a Conf the way it's done on the client side).
If the child process's standard input is provided by a pipe that's
separate from its output channels, we can - and should - honour a
request to cause that process to receive input EOF, by closing the
output end of that pipe.
As usual, we do this by setting a pending-EOF flag and calling
try_write, to ensure that any buffered output data is sent before the
pipe actually closes.
Not every "session" channel in SSH allocates a pty at all, of course,
and so I'll need a way to run a subprocess without doing so. The
simplest approach seems to be to expand uxpty's remit so that the pty
is optional: now it can open either a pty or a set of pipes for
stdin/out/err, according to an option provided to pty_backend_create.
(It amuses me that without this option I'd have an SSH server which is
incapable of _not_ honouring the "pty-req" channel request. That's
normally the easy part!)
This breaks the previous one-to-one coupling between pty backend
instances and file descriptors passed to uxsel, which I was using to
look up the Pty structure in a tree234 indexed by fd when an uxsel
notification came back. So now each Pty structure contains a
collection of subobjects of a new type PtyFd, and _those_ are what's
stored in the fd-indexed tree.
Another awkward part is that uxsel_set is not incremental: the rwx
flags you pass to it completely supersede the previous set for that
file descriptor, so I had to set up the logic that decides whether
we're trying to read or write each fd in a way that can cope equally
well with the fd aliasing another one (if it's the pty master) or not
(if there are three completely separate pipes).
The SS_SIGFOO family are implemented by sending a signal directly to
the pid of the immediate child process.
I had had the vague idea that it might be more desirable to send the
specified signal to the foreground process group in the tty. That way,
you'd be able to SIGINT (say) the foreground job in a shell session,
and return to the shell _prompt_ without terminating the whole
session, and you could do this in an emergency even if the job was a
full-screen application which had configured termios so that no
keystroke generated SIGINT.
But as far as I can see there's no actual way to do that. I wasn't
able to find any ioctl or termios call to send a signal to a pty's
foreground pgrp, and you can't even do it manually via kill(2) because
first you'd have to find out what the pgrp id _is_, and according to
the man pages, you can only call tcgetpgrp on the slave end of the pty
and even then only if it's your controlling terminal.
So SS_SIGFOO goes to the child process, because that's the only place
I can find that I _can_ send it to sensibly.
SS_BRK translates to tcsendbreak, of course (though I haven't actually
seen any effect of calling this on a pty master, not even if I set
PARMRK on the slave end which by my understanding _ought_ to show me
when break events occur).
This will be applied to the pty's termios settings at creation time,
superseding the default settings uxpty has always used. It works by
including the new sshttymodes.h with TTYMODES_LOCAL_ONLY defined, so
that modes not supported by a particular Unix system are automatically
quietly ignored.
Of course, a struct ssh_ttymodes always has the option of representing
"please make no change to the defaults", and of course, that's
precisely what is done by the one that pty_init constructs for clients
that aren't calling pty_backend_create directly.
The function that does the main pty setup is now called
pty_backend_create(), and has an API better suited to uxpty in
particular than the standard backend_init() virtual constructor. It
leaves off a load of standard parameters to backend_init() which
aren't really relevant to this backend, and it adds the 'argv'
parameter to pass in a split-up command line, which is unique to it.
The old creation function still exists, as a tiny wrapper that calls
the new pty_backend_create. And that version still gets the argv
parameter from the process-global variable pty_argv[], so the call
sites in pterm haven't had to change for this.
This will make it possible to instantiate a pty backend directly from
the SSH server code, without having to do anything really excessively
cumbersome to pass in a subcommand in the form of pre-split argv. (And
I'll add a few more specialist parameters to the new function shortly.)
There was a bit of a race condition depending on whether uxpty spotted
the EOF/EIO on the process's output first, or the SIGCHLD for its
actual termination: if the former came first, it would never bother to
reap the exit code at all.
It still doesn't bother if it's closing the session immediately and
the process genuinely _hasn't_ died (say, if it detaches itself
completely from the controlling tty to run in the background like a
weird parody of an old DOS TSR). But now when we see EOF, we make an
immediate (but nonblocking) attempt to wait for the child process, in
case its exit code was already available and we just hadn't noticed
yet.
The uxpty backend is going to be reused to implement the "session"
channel type in the upcoming SSH server implementation, which puts
quite a few new requirements on it. The first of them is that when we
get EOF from the subprocess's output channel (or rather, EIO from the
pty), we should actually notify the Seat of this.
In principle we should have been doing this all along, I'm pretty
sure. It hasn't happened to matter until now because the receiving
Seats haven't done much with that notification. But it will matter
when that's what controls the sending of SSH_MSG_CHANNEL_EOF.
The word 'PuTTY' in the outgoing SSH version string has always
represented the name of the *SSH implementation* as opposed to the
name of the specific program containing it (for example, PSCP and
PSFTP don't announce themselves with a different banner). But I think
that a change from client to server merits a change in that
implementation name, so I'm removing the prefix "PuTTY" from the
constant string sshver[], and moving it to a parameter passed in
separately to ssh_verstring_new, so that the upcoming server can pass
in a different one.
This new function contains the core setup for a PortForwarding
structure, and should be reusable for any kind of forwarding that will
simply be passing data between a local socket and an SSH channel
without any tricky modifications. On the server side, X11 and agent
forwarding both work exactly like this, so they will find this
refactored function useful during setup.
The contents of the function was originally part of pfl_accepting,
which now does all that by calling the new function. pfl_accepting is
not _quite_ doing a simple unmodified forwarding, because it might
have to prefix it with a SOCKS exchange; in that situation it rewrites
a few fields of the PortForwarding to some less generic values once
portfwd_raw_new() has returned.
I've written the decryption side of the PKCS#1 encryption used in
SSH-1, and also the RSAES-OAEP system used by SSH-2 RSA kex. Also, the
RSA kex structures now each come with an 'extra' pointer giving the
minimum key length.
Lots of user-facing messages that claim that the 'server' just did
something or other unexpected will now need to be issued _by_ the
server, when the client does the same unexpected thing. So I've
reworded them all to talk about the 'remote side' instead of the
'server', and the SSH-2 key setup messages talk about initialising
inbound and outbound crypto primitives rather than client->server and
server->client.
ssh2connection.c now knows how to unmarshal the message formats for
all the channel requests we'll need to handle when we're the server
and a client sends them. Each one is translated into a call to a new
method in the Channel vtable, which is implemented by a trivial
'always fail' routine in every channel type we know about so far.
The vtable method underneath sshfwd_write now takes an is_stderr
parameter, and in SSH-2, this is implemented by having separate stdout
and stderr bufchains in each outgoing channel, and counting the size
of both for the purposes of measuring backlog and so forth.
To avoid making _most_ call sites more verbose, the usual macro
wrapper hasn't changed its API; it just sets is_stderr=FALSE. To use
the new feature, there's an sshfwd_write_ext macro that exposes the
extra parameter.
This will be used for the server side of X forwarding. It wraps up the
mechanics of listening on the right TCP port and (if possible) the
associated AF_UNIX socket, and also creates an appropriate X authority
file containing authorisation data provided by its caller.
Like the new platform_create_agent_socket, this function spawns a
watchdog subprocess to clean up the mess afterwards, in the hope of at
least _most_ of the time not leaving old sockets and authority files
lying around /tmp,
The code in Pageant that sets up the Unix socket and its containing
directory now lives in a separate file, uxagentsock.c, where it will
also be callable from the upcoming new SSH server when it wants to
create a similar socket for agent forwarding.
While I'm at it, I've also added a feature to create a watchdog
subprocess that will try to clean up the socket and directory once
Pageant itself terminates, in the hope of leaving less cruft lying
around /tmp.
This is a major code reorganisation in preparation for making this
code base into one that can build an SSH server as well as a client.
(Mostly for purposes of using the server as a regression test suite
for the client, though I have some other possible uses in mind too.
However, it's currently no part of my plan to harden the server to the
point where it can sensibly be deployed in a hostile environment.)
In this preparatory commit, I've broken up the SSH-2 transport and
connection layers, and the SSH-1 connection layer, into multiple
source files, with each layer having its own header file containing
the shared type definitions. In each case, the new source file
contains code that's specific to the client side of the protocol, so
that a new file can be swapped in in its place when building the
server.
Mostly this is just a straightforward moving of code without changing
it very much, but there are a couple of actual changes in the process:
The parsing of SSH-2 global-request and channel open-messages is now
done by a new pair of functions in the client module. For channel
opens, I've invented a new union data type to be the return value from
that function, representing either failure (plus error message),
success (plus Channel instance to manage the new channel), or an
instruction to hand the channel over to a sharing downstream (plus a
pointer to the downstream in question).
Also, the tree234 of remote port forwardings in ssh2connection is now
initialised on first use by the client-specific code, so that's where
its compare function lives. The shared ssh2connection_free() still
takes responsibility for freeing it, but now has to check if it's
non-null first.
The outer shell of the ssh2_lportfwd_open method, for making a
local-to-remote port forwarding, is still centralised in
ssh2connection.c, but the part of it that actually constructs the
outgoing channel-open message has moved into the client code, because
that will have to change depending on whether the channel-open has to
have type direct-tcpip or forwarded-tcpip.
In the SSH-1 connection layer, half the filter_queue method has moved
out into the new client-specific code, but not all of it -
bidirectional channel maintenance messages are still handled
centrally. One exception is SSH_MSG_PORT_OPEN, which can be sent in
both directions, but with subtly different semantics - from server to
client, it's referring to a previously established remote forwarding
(and must be rejected if there isn't one that matches it), but from
client to server it's just a "direct-tcpip" request with no prior
context. So that one is in the client-specific module, and when I add
the server code it will have its own different handler.
Some kinds of channel, even after they've sent EOF in both directions,
still have something to do before they initiate the CLOSE mechanism
and wind up the channel completely. For example, a session channel
with a subprocess running inside it will want to be sure to send the
"exit-status" or "exit-signal" notification, even if that happens
after bidirectional EOF of the data channels.
Previously, the SSH-2 connection layer had the standard policy that
once EOF had been both sent and received, it would start the final
close procedure. There's a method chan_want_close() by which a Channel
could vary this policy in one direction, by indicating that it wanted
the close procedure to commence after EOF was sent in only one
direction. Its parameters are a pair of booleans saying whether EOF
has been sent, and whether it's been received.
Now chan_want_close can vary the policy in the other direction as
well: if it returns FALSE even when _both_ parameters are true, the
connection layer will honour that, and not send CHANNEL_CLOSE. If it
does that, the Channel is responsible for indicating when it _does_
want close later, by calling sshfwd_initiate_close.
Previously, it returned a human-readable string suitable for log
files, which tried to say something useful about the remote end of a
socket. Now it returns a whole SocketPeerInfo structure, of which that
human-friendly log string is just one field, but also some of the same
information - remote IP address and port, in particular - is provided
in machine-readable form where it's available.
Turns out that initiation of a CHANNEL_CLOSE message before both sides
have sent EOF is not only for _unclean_ closures or emergencies; it's
actually a perfectly normal thing that some channel types want to do.
(For example, a channel with a pty at the server end of it has no real
concept of sending EOF independently in both directions: when the pty
master sends EIO, the pty is no longer functioning, and you can no
longer send to it any more than you can receive.)
That's more directly useful in uxpty.c (which is currently the only
actual client of the function), and also matches the data that SSH
clients send in "pty-req". Also, it makes that method behave more like
the GUI query function get_window_pixels used by terminal.c (with the
sole exception that unlike g_w_p it's allowed to return failure), so
it becomes even more trivial to implement in the GUI front ends.
The new FdSocket just takes an arbitrary pair of file descriptors to
read and write, optionally with an extra input fd providing the
standard error output from a command. uxproxy.c now just does the
forking and pipe setup, and once it's got all its fds, it hands off to
FdSocket to actually do the reading and writing.
This is very like the reorganisation I did on the Windows side in
commit 98a6a3553 (back in 2013, in preparation for named-pipe sockets
and connection sharing). The idea is that it should enable me to make
a thing that the PuTTY code base sees as a Socket, but which actually
connects to the standard I/O handles of the process it lives in.
The function takes the two KEXINIT packets in their string form,
together with a list of mappings from names to known algorithm
implementations, and returns the selected one of each kind, along with
all the other necessary auxiliary stuff.
This has nice effects on code tidiness (quite a few variables now
become local to the new function instead of living permanently in the
transport layer), but mostly, the idea is to add flexibility by
introducing a convenient place to change the policy for how we write
the negotiation lists in our KEXINIT.
