NFC: this is a preliminary refactoring, intended to make my life
easier when I start changing around the APIs used to pass user
keyboard input around. The fewer functions even _have_ such an API,
the less I'll have to do at that point.
Thanks to Jiri Kaspar for sending this patch (apart from the new docs
section, which is in my own words), which implements a feature we've
had as a wishlist item ('utf8-plus-vt100') for a long time.
I was actually surprised it was possible to implement it in so few
lines of code! I'd forgotten, or possibly never noticed in the first
place, that even in UTF-8 mode PuTTY not only accepts but still
_processes_ all the ISO 2022 control sequences and shift characters,
and keeps running track of all the same state in term->cset and
term->cset_attrs that it tracks in IS0-2022-enabled modes. It's just
that in UTF-8 mode, at the very last minute when a character+attribute
pair is about to be written into the terminal's character buffer, it
deliberately ignores the contents of those variables.
So all that was needed was a new flag checked at that last moment
which causes it not quite to ignore them after all, and bingo,
utf8-plus-vt100 is supported. And it works no matter which ISO 2022
sequences you're using; whether you're using ESC ( 0 to select the
line drawing set directly into GL and ESC ( B to get back when you're
done, or whether you send a preliminary ESC ( B ESC ) 0 to get GL/GR
to be ASCII and line drawing respectively so you can use SI and SO as
one-byte mode switches thereafter, both work just as well.
This implementation strategy has a couple of consequences, which I
don't think matter very much one way or the other but I document them
just in case they turn out to be important later:
- if an application expecting this mode has already filled your
terminal window with lqqqqqqqqk, then enabling this mode in Change
Settings won't retroactively turn them into the line drawing
characters you wanted, because no memory is preserved in the screen
buffer of what the ISO 2022 state was when they were printed. So
the application still has to do a screen refresh.
- on the other hand, if you already sent the ESC ( 0 or whatever to
put the terminal _into_ line drawing mode, and then you turn on
this mode in Change Settings, you _will_ still be in line drawing
mode, because the system _does_ remember your current ISO 2022
state at all times, whether it's currently applying it to output
printing characters or not.
Commit d515e4f1a went through a lot of very different shapes before it
was finally pushed. In some of them, GSS kex had its own value in the
kex enumeration, but it was used in ssh.c but not in config.c
(because, as in the final version, it wasn't configured by the same
drag-list system as the rest of them). So we had to distinguish the
set of key exchange ids known to the program as a whole from the set
controllable in the configuration.
In the final version, GSS kex ended up even more separated from the
kex enumeration than that: the enum value KEX_GSS_SHA1_K5 isn't used
at all. Instead, GSS key exchange appears in the list at the point of
translation from the list of enum values into the list of pointers to
data structures full of kex methods.
But after all the changes, everyone involved forgot to revert the part
of the patch which split KEX_MAX in two and introduced the pointless
value KEX_GSS_SHA1_K5! Better late than never: I'm reverting it now,
to avoid confusion, and because I don't have any reason to think the
distinction will be useful for any other purpose.
The former has advantages in terms of keeping Kerberos credentials up
to date, but it also does something sufficiently weird to the usual
SSH host key system that I think it's worth making sure users have a
means of turning it off separately from the less intrusive GSS
userauth.
This is a heavily edited (by me) version of a patch originally due to
Nico Williams and Viktor Dukhovni. Their comments:
* Don't delegate credentials when rekeying unless there's a new TGT
or the old service ticket is nearly expired.
* Check for the above conditions more frequently (every two minutes
by default) and rekey when we would delegate credentials.
* Do not rekey with very short service ticket lifetimes; some GSSAPI
libraries may lose the race to use an almost expired ticket. Adjust
the timing of rekey checks to try to avoid this possibility.
My further comments:
The most interesting thing about this patch to me is that the use of
GSS key exchange causes a switch over to a completely different model
of what host keys are for. This comes from RFC 4462 section 2.1: the
basic idea is that when your session is mostly bidirectionally
authenticated by the GSSAPI exchanges happening in initial kex and
every rekey, host keys become more or less vestigial, and their
remaining purpose is to allow a rekey to happen if the requirements of
the SSH protocol demand it at an awkward moment when the GSS
credentials are not currently available (e.g. timed out and haven't
been renewed yet). As such, there's no need for host keys to be
_permanent_ or to be a reliable identifier of a particular host, and
RFC 4462 allows for the possibility that they might be purely
transient and only for this kind of emergency fallback purpose.
Therefore, once PuTTY has done a GSS key exchange, it disconnects
itself completely from the permanent host key cache functions in
storage.h, and instead switches to a _transient_ host key cache stored
in memory with the lifetime of just that SSH session. That cache is
populated with keys received from the server as a side effect of GSS
kex (via the optional SSH2_MSG_KEXGSS_HOSTKEY message), and used if
later in the session we have to fall back to a non-GSS key exchange.
However, in practice servers we've tested against do not send a host
key in that way, so we also have a fallback method of populating the
transient cache by triggering an immediate non-GSS rekey straight
after userauth (reusing the code path we also use to turn on OpenSSH
delayed encryption without the race condition).
This is a preliminary refactoring for an upcoming change which will
need to affect every use of schedule_timer to wait for the next rekey:
those calls to schedule_timer are now centralised into a function that
does an organised piece of thinking about when the next timer should
be.
A side effect of this change is that the translation from
CONF_ssh_rekey_time to an actual tick count is now better proofed
against integer overflow (just in case the user entered a completely
silly value).
This is a mild security measure against malicious clipboard-writing.
It's only mild, because of course there are situations in which even a
sanitised paste could be successfully malicious (imagine someone
managing to write the traditional 'rm -rf' command into your clipboard
when you were going to paste to a shell prompt); but it at least
allows pasting into typical text editors without also allowing the
control sequence that exits the editor UI and returns to the shell
prompt.
This is a configurable option, because there's no well defined line to
be drawn between acceptable and unacceptable pastes, and it's very
plausible that users will have sensible use cases for pasting things
outside the list of permitted characters, or cases in which they know
they trust the clipboard-writer. I for one certainly find it useful on
occasion to deliberately construct a paste containing control
sequences that automate a terminal-based UI.
While I'm at it, when bracketed paste mode is enabled, we also prevent
pasting of data that includes the 'end bracketed paste' sequence
somewhere in the middle. I really _hope_ nobody was treating bracketed
paste mode as a key part of their security boundary, but then again, I
also can't imagine that anyone had an actually sensible use case for
deliberately making a bracketed paste be only partly bracketed, and
it's an easy change while I'm messing about in this area anyway.
On all platforms, you can now configure which clipboard the mouse
pastes from, which clipboard Ctrl-Ins and Shift-Ins access, and which
Ctrl-Shift-C and Ctrl-Shift-V access. In each case, the options are:
- nothing at all
- a clipboard which is implicitly written by the act of mouse
selection (the PRIMARY selection on X, CLIP_LOCAL everywhere else)
- the standard clipboard written by explicit copy/paste UI actions
(CLIPBOARD on X, the unique system clipboard elsewhere).
Also, you can control whether selecting text with the mouse _also_
writes to the explicitly accessed clipboard.
The wording of the various messages changes between platforms, but the
basic UI shape is the same everywhere.
This stores the last text selected in _this_ terminal, regardless of
whether any other application has since taken back whatever system
clipboard we also copied it to. It's written unconditionally whenever
text is selected in terminal.c.
The main purpose of this will be that it's also the place that you can
go and find the data you need to write to a system clipboard in
response to an explicit Copy operation. But it can also act as a data
source for pastes in its own right, so you can use it to implement an
intra-window private extra clipboard if that's useful. (OS X Terminal
has one of those, so _someone_ at least seems to like the idea.)
This lays some groundwork for making PuTTY's cut and paste handling
more flexible in the area of which clipboard(s) it reads and writes,
if more than one is available on the system.
I've introduced a system of list macros which define an enumeration of
integer clipboard ids, some defined centrally in putty.h (at present
just a CLIP_NULL which never has any text in it, because that seems
like the sort of thing that will come in useful for configuring a
given copy or paste UI action to be ignored) and some defined per
platform. All the front end functions that copy and paste take a
clipboard id, and the Terminal structure is now configured at startup
to tell it which clipboard id it should paste from on a mouse click,
and which it should copy from on a selection.
However, I haven't actually added _real_ support for multiple X11
clipboards, in that the Unix front end supports a single CLIP_SYSTEM
regardless of whether it's in OS X or GTK mode. So this is currently a
NFC refactoring which does nothing but prepare the way for real
changes to come.
It's actually a function specific to the Windows front end, and has
been all along - but I've only just noticed that no other front end
either uses it or defines it.
Previously, both the Unix and Windows front ends would respond to a
paste action by retrieving data from the system clipboard, converting
it appropriately, _storing_ it in a persistent dynamic data block
inside the front end, and then calling term_do_paste(term), which in
turn would call back to the front end via get_clip() to retrieve the
current contents of that stored data block.
But, as far as I can tell, this was a completely pointless mechanism,
because after a data block was written into this storage area, it
would be immediately used for exactly one paste, and then never
accessed again until the next paste action caused it to be freed and
replaced with a new chunk of pasted data.
So why on earth was it stored persistently at all, and why that
callback mechanism from frontend to terminal back to frontend to
retrieve it for the actual paste action? I have no idea. This change
removes the entire system and replaces it with the completely obvious
alternative: the character-set-converted version of paste data is
allocated in a _local_ variable in the frontend paste functions,
passed directly to term_do_paste which now takes (buffer,length)
parameters, and freed immediately afterwards. get_clip() is gone.
This is another piece of long-overdue refactoring similar to the
recent commit e3796cb77. But where that one dealt with normalisation
of stuff already stored _in_ a Conf by whatever means (including, in
particular, handling a user typing 'username@host.name' into the
Hostname box of the GUI session dialog box), this one deals with
handling argv entries and putting them into the Conf.
This isn't exactly a pure no-functional-change-at-all refactoring. On
the other hand, it isn't a full-on cleanup that completely
rationalises all the user-visible behaviour as well as the code
structure. It's somewhere in between: I've preserved all the behaviour
quirks that I could imagine a reason for having intended, but taken
the opportunity to _not_ faithfully replicate anything I thought was
clearly just a bug.
So, for example, the following inconsistency is carefully preserved:
the command 'plink -load session nextword' treats 'nextword' as a host
name if the loaded session hasn't provided a hostname already, and
otherwise treats 'nextword' as the remote command to execute on the
already-specified remote host, but the same combination of arguments
to GUI PuTTY will _always_ treat 'nextword' as a hostname, overriding
a hostname (if any) in the saved session. That makes some sense to me
because of the different shapes of the overall command lines.
On the other hand, there are two behaviour changes I know of as a
result of this commit: a third argument to GUI PuTTY (after a hostname
and port) now provokes an error message instead of being silently
ignored, and in Plink, if you combine a -P option (specifying a port
number) with the historical comma-separated protocol selection prefix
on the hostname argument (which I'd completely forgotten even existed
until this piece of work), then the -P will now override the selected
protocol's default port number, whereas previously the default port
would win. For example, 'plink -P 12345 telnet,hostname' will now
connect via Telnet to port 12345 instead of to port 23.
There may be scope for removing or rethinking some of the command-
line syntax quirks in the wake of this change. If we do decide to do
anything like that, then hopefully having it all in one place will
make it easier to remove or change things consistently across the
tools.
A more or less identical piece of code to sanitise the CONF_host
string prior to session launch existed in Windows PuTTY and both
Windows and Unix Plink. It's long past time it was centralised.
While I'm here, I've added a couple of extra comments in the
centralised version, including one that - unfortunately - tries _but
fails_ to explain why a string of the form "host.name:1234" doesn't
get the suffix moved into CONF_port the way "user@host" moves the
prefix into CONF_username. Commit c1c1bc471 is the one I'm referring
to in the comment, and unfortunately it has an unexplained one-liner
log message from before I got into the habit of being usefully
verbose.
It's an incoherent concept! There should not be any such thing as an
error box that terminates the entire program but is not modal. If it's
bad enough to terminate the whole program, i.e. _all_ currently live
connections, then there's no point in permitting progress to continue
in windows other than the affected one, because all windows are
affected anyway.
So all previous uses of fatalbox() have become modalfatalbox(), except
those which looked to me as if they shouldn't have been fatal in the
first place, e.g. lingering pieces of error handling in winnet.c which
ought to have had the severity of 'give up on this particular Socket
and close it' rather than 'give up on the ENTIRE UNIVERSE'.
This is used when you're about to destroy an object that is
(potentially) the context parameter for some still-pending toplevel
callback. It causes callbacks.c to go through its pending list and
delete any callback records referring to that context parameter, so
that when you destroy the object those callbacks aren't still waiting
to cause stale-pointer dereferences.
I've done this on a 'where possible' basis: in Windows paletted mode
(in case anyone is still using an old enough graphics card to need
that!) I simply haven't bothered, and will completely ignore the dim
flag.
Markus Gans points out that some applications which (not at all
unreasonably) don't trust $TERM to tell them the full capabilities of
their terminal will use the sequence "OSC 4 ; nn ; ? BEL" to ask for
the colour-palette value in position nn, and they may not particularly
care _what_ the results are but they will use them to decide whether
the right number of colour palette entries even exist.
I know some users don't like any colour _at all_, and we have a
separate option to turn off xterm-style 256-colour sequences, so it
seems remiss not to have an option to disable true colour as well.
This is a heavily rewritten version of a patch originally by Lorenz
Diener; it was tidied up somewhat by Christian Brabandt, and then
tidied up more by me. The basic idea is to add to the termchar
structure a pair of small structs encoding 24-bit RGB values, each
with a flag indicating whether it's turned on; if it is, it overrides
any other specification of fg or bg colour for that character cell.
I've added a test line to colours.txt containing a few example colours
from /usr/share/X11/rgb.txt. In fact it makes quite a good demo to run
the whole of rgb.txt through this treatment, with a command such as
perl -pe 's!^\s*(\d+)\s+(\d+)\s+(\d+).*$!\e[38;2;$1;$2;$3m$&\e[m!' rgb.txt
It's a function that exists on all platforms, not just on Unix - it's
used in ldisc.c - so it shouldn't have been declared only in unix.h.
Score another for clang's warnings.
Now, instead of returning a boolean indicating whether the query has
completed or is still pending, agent_query() returns NULL to indicate
that the query _has_ completed, and if it hasn't, it returns a pointer
to a context structure representing the pending query, so that the
latter can be used to cancel the query if (for example) you later
decide you need to free the thing its callback was using as a context.
This should fix a potential race-condition segfault if you overload an
agent forwarding channel and then close it abruptly. (Which nobody
will be doing for sensible purposes, of course! But I ran across this
while stress-testing other aspects of agent forwarding.)
I can't believe this codebase is around 20 years old and has had
multiple giant const-fixing patches, and yet there are _still_ things
that should have been const for years and aren't.
The UI now only has "1" and "2" options for SSH protocol version, which
behave like the old "1 only" and "2 only" options; old
SSH-N-with-fallback settings are interpreted as SSH-N-only.
This prevents any attempt at a protocol downgrade attack.
Most users should see no difference; those poor souls who still have to
work with SSH-1 equipment now have to explicitly opt in.
A compiler warning drew my attention to the fact that 'next' in
pinger_schedule() was an int, not the unsigned long it should have
been. And looking at the code that handles it, it was also taking no
care with integer wraparound when checking whether an existing
scheduled ping should be moved forward.
So now I do something a bit more robust, by remembering what time it
_was_ when we set pinger->next, and checking if the new time value
falls in the interval between those two times.
E.g. you might pass '--random-device=/dev/urandom'.
Mostly because I got sick of waiting for /dev/random to finish
blocking while I was trying to generate throwaway keys for testing bug
fixes in cmdgen itself. But it might also be useful on systems that
call their random device by a different name that we haven't
encountered.
(Since cmdgen also reads the saved PuTTY random seed file, setting
this option to /dev/zero will not render key generation deterministic.
It's tempting to provide _some_ way to do that, for testing purposes
and clearly marked as dangerous of course, but I think it would take
more faff than this.)
If you're connecting to a new server and it _only_ provides host key
types you've configured to be below the warning threshold, it's OK to
give the standard askalg() message. But if you've newly demoted a host
key type and now reconnect to some server for which that type was the
best key you had cached, the askalg() wording isn't really appropriate
(it's not that the key we've settled on is the first type _supported
by the server_, it's that it's the first type _cached by us_), and
also it's potentially helpful to list the better algorithms so that
the user can pick one to cross-certify.
Now we actually have enough of them to worry about, and especially
since some of the types we support are approved by organisations that
people might make their own decisions about whether to trust, it seems
worth having a config list for host keys the same way we have one for
kex types and ciphers.
To make room for this, I've created an SSH > Host Keys config panel,
and moved the existing host-key related configuration (manually
specified fingerprints) into there from the Kex panel.
It has three settings: on, off, and 'only until session starts'. The
idea of the last one is that if you use something like 'ssh -v' as
your proxy command, you probably wanted to see the initial SSH
connection-setup messages while you were waiting to see if the
connection would be set up successfully at all, but probably _didn't_
want a slew of diagnostics from rekeys disrupting your terminal in
mid-emacs once the session had got properly under way.
Default is off, to avoid startling people used to the old behaviour. I
wonder if I should have set it more aggressively, though.
This saves the need to fork and exec "cat", which should speed things
up. It also ensures that the network output goes to /dev/null, which
should avoid problems with blocking when writing to a full pipe.
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.
On Windows, colons are illegal in filenames, because they're part of
the path syntax. But colons can appear in automatically constructed
log file names, if an IPv6 address is expanded from the &H placeholder.
Now we coerce any such illegal characters to '.', which is a bit of a
bodge but should at least cause a log file to be generated.
Personally I like using Command as the Esc-prefixing Meta key in
terminal sessions, because it occupies the same physical keyboard
position as the Alt key that I'm used to using on non-Macs. OS X
Terminal uses Option for that purpose (freeing up Command for the
conventional Mac keyboard shortcuts, of course), so I anticipate
differences of opinion.
Hence, here's a pair of OSX-specific config options which permit a
user to set either, or neither, or both of those modifier keys to
function as the terminal Meta key.
The general plan is that if PuTTY knows a host key for a server, it
should preferentially ask for the same type of key so that there's some
chance of actually getting the same key again. This should mean that
when a server (or PuTTY) adds a new host key type, PuTTY doesn't
gratuitously switch to that key type and then warn the user about an
unrecognised key.
Having found a lot of unfixed constness issues in recent development,
I thought perhaps it was time to get proactive, so I compiled the
whole codebase with -Wwrite-strings. That turned up a huge load of
const problems, which I've fixed in this commit: the Unix build now
goes cleanly through with -Wwrite-strings, and the Windows build is as
close as I could get it (there are some lingering issues due to
occasional Windows API functions like AcquireCredentialsHandle not
having the right constness).
Notable fallout beyond the purely mechanical changing of types:
- the stuff saved by cmdline_save_param() is now explicitly
dupstr()ed, and freed in cmdline_run_saved.
- I couldn't make both string arguments to cmdline_process_param()
const, because it intentionally writes to one of them in the case
where it's the argument to -pw (in the vain hope of being at least
slightly friendly to 'ps'), so elsewhere I had to temporarily
dupstr() something for the sake of passing it to that function
- I had to invent a silly parallel version of const_cmp() so I could
pass const string literals in to lookup functions.
- stripslashes() in pscp.c and psftp.c has the annoying strchr nature
All the name strings in ssh_cipher, ssh_mac, ssh_hash, ssh_signkey
point to compile-time string literals, hence should obviously be const
char *.
Most of these const-correctness patches are just a mechanical job of
adding a 'const' in the one place you need it right now, and then
chasing the implications through the code adding further consts until
it compiles. But this one has actually shown up a bug: the 'algorithm'
output parameter in ssh2_userkey_loadpub was sometimes returning a
pointer to a string literal, and sometimes a pointer to dynamically
allocated memory, so callers were forced to either sometimes leak
memory or sometimes free a bad thing. Now it's consistently
dynamically allocated, and should be freed everywhere too.
PuTTY now uses the updated version of Diffie-Hellman group exchange,
except for a few old OpenSSH versions which Darren Tucker reports only
support the old version.
FIXME: this needs further work because the Bugs config panel has now
overflowed.
I'm not actually sure why we've always had back ends notify ldisc of
changes to echo/edit settings by giving ldisc_send(ldisc,NULL,0,0) a
special meaning, instead of by having a separate dedicated notify
function with its own prototype and parameter set. Coverity's recent
observation that the two kinds of call don't even have the same
requirements on the ldisc (particularly, whether ldisc->term can be
NULL) makes me realise that it's really high time I separated the two
conceptually different operations into actually different functions.
While I'm here, I've renamed the confusing ldisc_update() function
which that special operation ends up feeding to, because it's not
actually a function applying to an ldisc - it applies to a front end.
So ldisc_send(ldisc,NULL,0,0) is now ldisc_echoedit_update(ldisc), and
that in turn figures out the current echo/edit settings before passing
them on to frontend_echoedit_update(). I think that should be clearer.
