DJB's spec at http://cr.yp.to/ecdh/curve25519-20060209.pdf is clear
that we should be clearing the low 3 bits of the _LSB_ of the private
key bit string, and setting bit 6 and clearing bit 7 of the _MSB_. We
were doing the opposite, due to feeding the resulting bit string to
bignum_from_bytes() rather than bignum_from_bytes_le().
This didn't cause an interoperability issue, because the two DH
exponentiations still commute, but it goes against the Curve25519
spec, in particular the care taken to fix the position of the leading
exponent bit.
The code is now consistent with the test vectors in RFC 7748 section
6.1: if you modify the EC_MONTGOMERY branch of ssh_ecdhkex_newkey() to
replace the loop on random_byte() with a memcpy that fills bytes[]
with 77076d0a7318a57d3c16c17251b26645df4c2f87ebc0992ab177fba51db92c2a
and then print out the resulting publicKey->x, you find that it's
(byte-reversed) the expected output value given in that RFC section,
8520f0098930a754748b7ddcb43ef75a0dbf3a0d26381af4eba4a98eaa9b4e6a.
An opcode for this was recently published in
https://tools.ietf.org/html/draft-sgtatham-secsh-iutf8-00 .
The default setting is conditional on frontend_is_utf8(), which is
consistent with the pty back end's policy for setting the same flag
locally. Of course, users can override the setting either way in the
GUI configurer, the same as all other tty modes.
Previously only Unix front ends bothered to include it, on the basis
that only the pty backend needed it (to set IUTF8 in the pty). We're
about to need it everywhere else too.
Previously, the code that marshalled tty settings into the "pty-req"
request was iterating through the subkeys stored in ssh->conf, meaning
that if a session had been saved before we gained support for a
particular tty mode, the iteration wouldn't visit that mode at all and
hence wouldn't send even the default setting for it.
Now we iterate over the array of known mode identifiers in
ssh_ttymodes[] and look each one up in ssh->conf, rather than vice
versa. This means that when we add support for a new tty mode with a
nontrivial policy for choosing its default state, we should start
using the default handler immediately, rather than bizarrely waiting
for users to save a session after the change.
Also add an assertion to do_ssh2_transport to catch this.
This bug would be highly unlikely to manifest accidentally, but I
think you could trigger it by setting the data-based rekey threshold
very low.
This should avoid the possibility of the SIGWINCH handler's blocking
when trying to write to the pipe. This could only happen if we'd
somehow received PIPE_BUF SIGWINCHes without reading the pipe, which
would be difficult to achieve.
While we're at it, also set O_NONBLOCK on the reading side of the pipe,
just in case.
All calls to ssh2_add_channel_data() were followed by a call to
ssh2_try_send(), so it seems sensible to replace ssh2_add_channel_data()
with ssh2_send_channel_data(), which does both.
Specifically, don't try to unblock all channels just because we've got
something to send on the main one. It looks like the code to do that
was left over from when SSH_MSG_CHANNEL_ADJUST was handled in
do_ssh2_authconn().
Also try to upgrade the settings of people who haven't changed the
defaults; but anyone who has, or anyone who's used the pre-release
snapshots with elliptic-curve support, will have to review their
settings manually.
I've reset the baseline to be the version of mingw-w64 that comes with
Ubuntu 14.04. Right now, that means no features need to be omitted; all
you need to do is set TOOLPATH to i686-w64-mingw32- .
I've removed -mno-cygwin without comment. Toolchains which don't support
this flag have been around since at least 2012, so we can probably
assume that no-one cares about older toolchains by now.
It's really only useful with MinGW rather than a Cygwin toolchain these
days, as recent versions of the latter insist against linking with the
Cygwin DLL.
(I think it may no longer be possible to build with Cygwin out of the
box at all these days, but I'm not going to say so without having
actually checked that's the case. Settle for listing MinGW first in
various comments and docs.)
Formerly PuTTY's SFTP code would transmit (or buffer) a megabyte of data
before even starting to look for acknowledgements, but wouldn't allow
there to be more than a megabyte of unacknowledged data at a time. Now,
instead, it pays attention to whether the transmit path is blocked, and
transmits iff it isn't.
This should mean that SFTP goes faster over long fat pipes, and also
doesn't end up buffering so much over thin ones.
I practice, I tend to run into other performance limitations (such as
TCP or SSH-2 windows) before this enhancement looks particularly good,
but with an artificial lag of 250 ms on the loopback interface this
patch almost doubles my upload speed, so I think it's worthwhile.
I've just upgraded my build environment to the latest Inno Setup
(apparently fixing some DLL hijacking issues), and found that the
build script doesn't run any more because the name of the output file
has changed - it used to produce Output/setup.exe, but now it produces
Output/mysetup.exe.
Rather than just fixing the build script to expect the new name, I've
explicitly specified an output filename of my own choice in putty.iss,
so that the build script should now work with versions before and
after the change.
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.
Previously, if you tried to set the special cflags for an object file
to the empty string, mkfiles.pl would normalise that to the string
"1". I'm not entirely sure why - that line of code was added without
explanation in commit 64150a5ef which brought in that directive in the
first place - but I have to guess that it was left over from some
earlier design iteration in which I hadn't quite decided whether I was
going to need a string or a boolean to separate version.o from other
objects.
Of course, setting an object's cflags to "" is a bit of a weird thing
to want to do anyway - why not just leave them unset? But in fact I've
now thought of something useful for it to do: this commit arranges
that setting cflags="" has the effect (in the 'am' makefile type) of
separating the object out into its own little automake library but not
actually giving that library any separate cflags. And the point of
_that_, in turn, will be that then you can add cflags to it
_conditionally_ in a "!begin am" snippet, e.g. conditionalised on
something in configure.
Ahem. Cut-and-paste goof that I introduced in commit 2eb952ca3, when I
moved the application names out of separate text controls in the
resource-file dialog descriptions.
This was defined in misc.h, and also in network.h (because one
function prototype needed to refer to it in the latter), leading to a
build failure if any source file inconveniently included both those
headers.
Fixed by guarding each copy of the typedef with a #ifdef.
A side effect of commit 1f9df706b seems to have been to squash those
areas right up against the bottom of the dialog box, which is ugly. I
don't fully understand why it only happens to those drawing areas and
not to buttons placed in the fake 'action area' by other dialogs, but
anyway, adding an explicit margin-bottom attribute seems to solve it.
This is another widget that can appear in the top-level window, in
addition to the drawing area and scrollbar we put there ourselves, and
hence which needs to be accounted for when figuring out the
relationship between the drawing area size in character cells and the
full window size in pixels.
Finding the menu bar widget itself is a bit of a hassle, but having
found it, dealing with it is basically the same as dealing with the
scrollbar, only with x and y swapped.
This function, which parses the X11-style '-geometry WxH+X+Y' option
argument and automatically loads the result into the window, is also
being deprecated.
Fortunately we already had a fallback option for GTK1 (which didn't
have gtk_window_parse_geometry in the first place), calling the Xlib
geometry-parsing function and manually loading the results into GTK.
The method of loading into GTK is not the same between the two
versions, but the basic strategy is still viable.
For the sake of maintaining and testing fewer ifdef branches, I've
removed the use of gtk_window_parse_geometry _completely_, even in
GTK2 which did have it. GTK2 now uses the same strategy that I've
switched to for GTK3.
gtk_window_resize_to_geometry and gtk_window_set_default_geometry are
deprecated as of GTK 3.20, so now we do the geometry -> pixel size
conversion on our side.
This is preparation for dealing with the fact that GTK's geometry-
based API routines for setting the window size are being deprecated:
we'll no longer be able to specify a width/height in characters and
have GTK convert that into a pixel size based on the geometry hints
we'd already fed it. So we'll need to do that conversion ourselves,
and the easiest approach is to make it easy to recompute the geometry
hints on our side whenever we need them.
gdk_device_grab and all its preparatory faff are now deprecated, and
gdk_seat_grab is the new thing. Introduce yet another branch to all
the ifdefs for keyboard-grabbing. On the plus side, at least it's
slightly simpler than the GdkDevice business.
Now bugs that are still likely to come up with relatively recent
server software (because they're only a few years fixed, or because
they're the sort of mistake that new server implementors will likely
make again) are in the Bugs panel, and very old things long since
fixed are relegated to More Bugs.
In particular, More Bugs contains everything SSH-1 related.
Blocking PROCESS_QUERY_INFORMATION access to the process turned out to
stop screen readers like Microsoft Narrator from reading parts of the
PuTTY window like the System Menu.
That function is deprecated as of 3.18, on the basis that GTK doesn't
need telling any more when the adjustment's owning widget needs
updating. So we just need to condition out the call.
strcspn() returns a size_t, which is not safe to pass as the parameter
in a printf argument list corresponding to a "*" field width specifier
in the format string, because the latter should be int, which may not
be the same size as size_t.
We were calling Windows file-handling API functions GetFilesize and
SetFilePointer, each of which returns two halves of a large integer by
writing the high half through a pointer, with pointers to the wrong
integer types. Now we're always passing the exact type defined in the
API, and converting after the fact to our own uint64 type, so this
should avoid any risk of wrong-sized pointers.
These integer types are correct for the id/handle parameter to
AppendMenu / InsertMenu / DeleteMenu, and also for the return type of
dialog box procedures.
We also have the special-purpose -DUNPROTECT to disable just the ACL
changes, but if you want to compile without any Windows security API
support at all (e.g. experimentally building against winelib) then
it's easier not to have to specify both defines separately.
With all due respect to Microsoft, a cross-platform program simply
cannot switch to using MS's assorted 'secure' versions of standard C
functions if it wants to continue compiling on platforms other than
Windows. So I might as well squash the warnings, so that any other
more interesting compiler warnings can avoid being swamped in the
mess.
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.
The old README.txt instructed you to manually update PATH if you
wanted to run pscp from a command prompt. But the MSI installer can do
that automatically, so the wording needs tweaks. And now that we're
actually launching README (at least optionally) from the installer UI,
it's more important to not make it look silly.
This is a thing that the Inno Setup installer did, and that I didn't
get round to replicating when I rushed out the initial MSI in a hurry.
I've checked that this doesn't prevent unattended installation by
administrators: running 'msiexec /q /i putty-whatever.msi' as
administrator still installs silently after this change, without
popping up the README unexpectedly on anyone's desktop as a side
effect.
(I _think_ - but I'm still a long way from an MSI expert - that that's
because /q turns off the whole UI part of the MSI system, and the
loading of README is actually triggered by the transition away from
the final UI dialog box, which we now never visit in the first place.)
I rushed out the MSI in too much of a hurry to sort out this kind of
thing, but now we've got leisure to reconsider, I think it's better
behaviour not to clutter everyone's desktops unless specifically asked
to.
Simon Tatham
Ben Harris
Kylie McClain
Jacob Nevins
Owen Dunn