Now there are lines in the checksum lists that don't correspond
directly to files we uploaded as part of the main release process,
release.pl needs to ignore them to avoid embarrassing upload failures.
After all the side-channel removal work I've done, I'm demoting
Blowfish and Arcfour right to the bottom of the pile, because they're
the ciphers that still have cache and timing leaks in them. (Neither
one can be sanitised without making it _extremely_ slow - the factor
of 4.5 slowdown in DES would be small by comparison).
Single-DES is also still below the warning threshold for simply being
too weak (irrespective of side channels), but it's above the other two.
At the point when we change over the seat's trust status to untrusted
for the last time, to finish authentication, Plink will now present a
final interactive prompt saying 'Press Return to begin session'. This
is a hint that anything after that that resembles an auth prompt
should be treated with suspicion, because _PuTTY_ thinks it's finished
authenticating.
This is of course an annoying inconvenience for interactive users, so
I've tried to reduce its impact as much as I can. It doesn't happen in
GUI PuTTY at all (because the trust sigil system is used instead); it
doesn't happen if you use plink -batch (because then the user already
knows that they _never_ expect an interactive prompt); and it doesn't
happen if Plink's standard input is being redirected from anywhere
other than the terminal / console (because then it would be pointless
for the server to try to scam passphrases out of the user anyway,
since the user isn't in a position to enter one in response to a spoof
prompt). So it should only happen to people who are using Plink in a
terminal for interactive login purposes, and that's not _really_ what
I ever intended Plink to be used for (which is why it's never had any
out-of-band control UI like OpenSSH's ~ system).
If anyone _still_ doesn't like this new prompt, it can also be turned
off using the new -no-antispoof flag, if the user is willing to
knowingly assume the risk.
In terminal-based GUI applications, this is passed through to
term_set_trust_status, to toggle whether lines are prefixed with the
new trust sigil. In console applications, the function returns false,
indicating to the backend that it should employ some other technique
for spoofing protection.
This indicates that a line contains trusted information (originated by
PuTTY) or untrusted (from the server). Trusted lines are prefixed by a
three-column signature consisting of the trust sigil (i.e. PuTTY icon)
and a separating space.
To protect against a server using escape sequences to move the cursor
back up to a trusted line and overwrite its contents, any attempt to
write to a termline is preceded by a call to check_trust_status(),
which clears the line completely if the terminal's current trust
status is different from the previous state of that line.
In the terminal data structures, the trust sigil is represented by
0xDFFE (an otherwise unused value, because it's in the surrogate
space). For bidi purposes I've arranged to treat that value as
direction-neutral, so that it will appear on the right if a terminal
line needs it to. (Not that that's currently likely to happen, with
PuTTY not being properly localised, but it's a bit of futureproofing.)
The bidi system is also where I actually insert the trust sigil: the
_logical_ terminal data structures don't include it. term_bidi_line
was a convenient place to add it, because that function was already
transforming a logical terminal line into a physical one in a way that
also generates a logical<->physical mapping table for handling mouse
clicks and cursor positioning; so that function now adds the trust
sigil as well as running the bidi algorithm.
(A knock-on effect of _that_ is that the log<->phys position map now
has to have a value for 'no correspondence', because if the user does
click on the trust sigil, there's no logical terminal position
corresponding to that. So the map can now contain the special value
BIDI_CHAR_INDEX_NONE, and anyone looking things up in it has to be
prepared to receive that as an answer.)
Of course, this terminal-data transformation can't be kept _wholly_
within term_bidi_line, because unlike proper bidi, it actually reduces
the number of visible columns on the line. So the wrapping code
(during glyph display and also copy and paste) has to take account of
the trusted status and use it to ignore the last 3 columns of the
line. This is probably not done absolutely perfectly, but then, it
doesn't need to be - trusted lines will be filled with well-controlled
data generated from the SSH code, which won't be doing every trick in
the book with escape sequences. Only untrusted terminal lines will be
using all the terminal's capabilities, and they don't have this sigil
getting in the way.
This is not yet used by anything, but the idea is that it'll be a
graphic in the terminal window that can't be replicated by a server
sending escape sequences, and hence can be used as a reliable
indication that the text on a particular terminal line is generated by
PuTTY itself and not passed through from the server. This will make it
possible to detect a malicious server trying to mimic local prompts to
trick you out of information that shouldn't be sent over the wire
(such as private-key passphrases).
The trust sigil I've picked is a small copy of the PuTTY icon, which
is thematically nice (it can be read as if the PuTTY icon is the name
of the speaker in a dialogue) and also convenient because we had that
graphic available already on all platforms. (Though the contortions I
had to go through to make the GTK 1 code draw it were quite annoying.)
The trust sigil has the same dimensions as a CJK double-width
character, i.e. it's 2 character cells wide by 1 high.
Now, instead of each seat's prompt-handling function doing the
control-char sanitisation of prompt text, the SSH code does it. This
means we can do it differently depending on the prompt.
In particular, prompts _we_ generate (e.g. a genuine request for your
private key's passphrase) are not sanitised; but prompts coming from
the server (in keyboard-interactive mode, or its more restricted SSH-1
analogues, TIS and CryptoCard) are not only sanitised but also
line-length limited and surrounded by uncounterfeitable headers, like
I've just done to the authentication banners.
This should mean that if a malicious server tries to fake the local
passphrase prompt (perhaps because it's somehow already got a copy of
your _encrypted_ private key), you can tell the difference.
This goes with the existing 'to_server' flag (indicating whether the
values typed by the user are going to be sent over the wire or remain
local), to indicate whether the _text of the prompts_ has come over
the wire or is originated locally.
Like to_server, nothing yet uses this. It's a hedge against the
possibility of maybe having an option for all the auth prompts to work
via GUI dialog boxes.
The banner text sent by the server was already being run through a
StripCtrlChars. Now it's run through one in line-limiting mode, and
surrounded by header and footer lines long enough that the line-length
limit wouldn't allow the server to counterfeit one. So it should now
be reliably possible to tell what is banner text sent by the server,
and what is not.
Now it can optionally check that output lines don't go beyond a
certain length (measured in terminal columns, via wcwidth, rather than
bytes or characters). In this mode, lines are prefixed with a
distinctive character (namely '|'), and if a line is too long, then it
is broken and the continuation line gets a different prefix ('>').
When StripCtrlChars is targeting a terminal, it asks the terminal to
call wcwidth on its behalf, so it can be sure to use the same idea as
the real terminal about which characters are wide (i.e. depending on
the configuration of ambiguous characters).
This mode isn't yet used anywhere.
The previous unlimited system was nicely general, but unfortunately
meant you could easily DoS a PuTTY-based terminal by sending a
printing character followed by an endless stream of identical
combining chars. (In fact, due to accidentally-quadratic linked list
management, you'd DoS it by using up all the CPU even before you got
the point of making it allocate all the RAM.)
The new limit is chosen to be 32, more or less arbitrarily. Overlong
sequences of combining characters are signalled by turning the whole
character cell into U+FFFD REPLACEMENT CHARACTER.
In both the Weierstrass and Montgomery forms, we now check that the
provided curve point isn't a silly one, like the identity or a torsion
point, which will give little or no variation in the possible outputs
of key exchange.
This checks that the public Diffie-Hellman value sent by the server is
not an obviously silly one like 1 or -1 (mod p). We already had the
validation function, and were using it in standard DH key exchange,
but the parallel code in the GSSAPI case missed it out.
If the terminal is one column wide, it's not possible to print a
double-width CJK character at all - it won't fit. Replace it with
U+FFFD to indicate that impossibility.
The previous behaviour was to notice that we're in the rightmost
column of the terminal, and invoke the LATTR_WRAPPED2 special case to
wrap to the leftmost column on the next line. But in a width-1
terminal, the rightmost column _is_ the leftmost column, so this would
leave us no better off, and we would have fallen through into the next
case while in exactly the situation we'd tried to rule out.
When we're displaying double-width text as a result of the VT100 ESC#6
escape sequence or its friends, and the terminal width is an odd
number of columns, we divide by 2 the number of characters we'll even
try to display, and round _down_: if there's a rightmost odd column,
it stays blank, and doesn't show the left half of a double-width char.
In the GTK redraw function, that rounding-down can set the 'len'
variable to zero. But when we're displaying a character with Unicode
combining chars on top, that fails an assertion that len == 1, because
at the top of the function we set it to 1.
The fix is just to return early if len is reduced to zero by that
rounding: if we're not displaying any characters, then we don't have
to do anything at all.
The REP escape (ESC [ nnn b) causes the previously printed graphic
character to be repeated another nnn times. So if it's sent as the
very first thing in a terminal session, when there _is_ no previously
printed graphic character, there's nothing sensible it can do.
In fact, in that situation, it does something decidedly _not_
sensible: it takes the uninitialised value term->last_graphic_char and
sends it directly to term_display_graphic_char, with undesirable
results if it's not actually a printing character. In particular, the
value 0 is treated as a combining char (because it has zero wcwidth),
leading to a knock-on assertion failure when compressing the
scrollback lines (which uses \0 as a terminating value for sequences
of combining characters, precisely because it expects it never to show
up in an actual cc slot!).
I completely forgot to check that the server had actually sent a key
of at least MINKLEN bits, as RFC 4432 clearly says that it MUST.
Without this restriction, not only can a server trick the client into
using a shared secret with inadequate entropy, but it can send a key
so short that the client attempts to generate a secret integer of
negative length, with integer-overflowing results.
I've always thought poll was more hassle to set up, because if you
want to reuse part of your pollfds list between calls then you have to
index every fd by its position in the list as well as the fd number
itself, which gives you twice as many indices to keep track of than if
the fd is always its own key.
But the problem is that select is fundamentally limited to the range
of fds that can fit in an fd_set, which is not the range of fds that
can _exist_, so I've had a change of heart and now have to go with
poll.
For the moment, I've surrounded it with a 'pollwrapper' structure that
lets me treat it more or less like select, containing a tree234 that
maps each fd to its location in the list, and also translating between
the simple select r/w/x classification and the richer poll flags.
That's let me do the migration with minimal disruption to the call
sites.
In future perhaps I can start using poll more directly, and/or using
the richer flag system (though the latter might be fiddly because of
sometimes being constrained to use the glib event loop). But this will
do for now.
Those hashes aren't directly needed for authenticating downloaded
files (the installer itself is checksummed, which covers all the files
it will unpack from itself). But they'll surely come in useful for
other purposes sooner or later, so we should arrange to keep them
somewhere easy to find.
The executables were already ignoring it.
This is a minimal change; PUTTY.HLP can still be built, and there's
still all the context IDs lying around.
Buildscr changes are untested.
With this change, we stop expecting to find putty.chm alongside the
executable file. That was a security hazard comparable to DLL
hijacking, because of the risk that a malicious CHM file could be
dropped into the same directory as putty.exe (e.g. if someone ran
PuTTY from their browser's download dir)..
Instead, the standalone putty.exe (and other binaries needing help)
embed the proper CHM file within themselves, as a Windows resource,
and if called on to display the help then they write the file out to a
temporary location. This has the advantage that if you download and
run the standalone putty.exe then you actually _get_ help, which
previously didn't happen!
The versions of the binaries in the installer don't each contain a
copy of the help file; that would be extravagant. Instead, the
installer itself writes a registry entry pointing at the proper help
file, and the executables will look there.
Another effect of this commit is that I've withdrawn support for the
older .HLP format completely. It's now entirely outdated, and
supporting it through this security fix would have been a huge pain.
My previous dodge to make the GTK 1 headers work with modern compilers
was to manually reset to -std=gnu89, which changed the semantics of
'inline' back to what glib.h was expecting. But that doesn't work now
the PuTTY code base expects to be able to use the rest of C99, so
instead I have to manually override the specific #defines that glib.h
uses to know how 'inline' works.
Also, moved that code in configure.ac out of the fallback branch that
manually detects GTK1, so that it will fire even if autoconf is run on
a system that still has the genuine GTK1 detection code. (Amazingly,
one still exists that I have access to!)
With that fixed, there's one more problem: the nethack_mode and
app_keypad_mode flags in gtkwin.c's key_event() are only used in the
GTK >= 2 branch of the ifdefs, so they should only be declared and set
in that branch as well, on pain of a -Wunused complaint.
A user reports that under OpenWatcom these are defined in <winnls.h>,
in which case it's redundant to redefine them ourselves and provokes a
compiler diagnostic.
I carefully made it return a bool to indicate that the whole PPL had
been freed, and then never actually checked that return value, so any
kind of connection-fatal event inside filter_queue (such as reporting
a DISCONNECT message) would cause a reference to freed memory on
return.
There was a race between toplevel callbacks: if we read enough data to
receive an SSH_MSG_DISCONNECT, and then returned, then whether we
reported the DISCONNECT message or the followup EOF would depend on
whether the BPP or the master PPL got called back first. Now the BPP
politely waits its turn (i.e. waits to see if it even gets called back
at all) before reporting EOF.
Turns out that my assertion that term->cols == line->cols can
sometimes fail, because if the window is shrunk, scrlineptr()
deliberately _doesn't_ shrink the line (so that the columns on the
right can be recovered if the window is then resized larger again). So
clear_line() should _make_ the line the right width, instead of
asserting that it already is.
I've factored out clear_line() (wipe out everything on a terminal line
including its line attrs) and also line_cols() (determine how many
columns are on this particular line, taking into account
LATTR_WRAPPED2 which reduces it by one).
Also, newline() and freeline() were badly named. Now they're called
newtermline() and freetermline(), which include the full actual type
name they deal with, and also means that now neither of them is named
the same as a control character!
Now instead of taking raw arguments to configure the output
StripCtrlChars with, it takes an enumerated value giving the context
of what's being sanitised, and allows the seat to decide what the
output parameters for that context should be.
The only context currently used is SIC_BANNER (SSH login banners).
I've also added a not-yet-used one for keyboard-interactive prompts.
Now instead of making a StripCtrlChars just for that function call, it
uses an existing one, pointing it at the output strbuf via
stripctrl_retarget.
This adds flexibility (now you can use the same convenient string-
sanitising function with a StripCtrl configured in any way you like)
and also saves pointless setting-up and tearing-down of identical sccs
all the time.
The existing call sites in PSCP and PSFTP now use a static
StripCtrlChars instance that was made at program startup.
stripctrl_retarget() points the StripCtrlChars at a new BinarySink, to
avoid having to pointlessly throw it away and make a new one all the
time.
Since that probably means the same scc is going to be reused for
processing a fresh data stream, we also don't want any character-set
conversion state hanging over from the previous stream, so we also
reset the state in the process. Just in case it's needed,
stripctrl_reset() is also provided to do that operation on its own.
Now if a pathname ends with a slash already, we detect that (using the
shiny new ptrlen_endswith), and don't bother putting another one in.
No functional change, but this should improve the occasional error
message, e.g. 'pscp remote:some.filename /' will now say it can't
create /some.filename instead of //some.filename.
