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.
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.
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.
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.
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.
It's only a warning; Windows PuTTYgen puts it up as a message box, and
will still generate the key if you click yes, and Unix PuTTYgen just
prints the warning and gets on with generation anyway. But it might
help encourage people to move away from 1024-bit keys, if they're
still using them.
Protecting our processes from outside interference need not be limited
to just PuTTY: there's no reason why the other SSH-speaking tools
shouldn't have the same treatment (PSFTP, PSCP, Plink), and PuTTYgen
and Pageant which handle private key material.
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.
This is an absolutely horrible piece of code, relying not only on font
metrics but also on an observed correlation between the length of a
key algorithm name and whether or not it needs a separate key size
displayed. But it'll do for the moment, and it's less effort than
writing a custom piece of Windows API code to display the list box
entries in a properly robust way :-(
Jacob pointed out that a free-text field for entering a key size in
bits is all very well for key types where we actually _can_ generate a
key to a size of your choice, but less useful for key types where
there are only three (or one) legal values for the field, especially
if we don't _say_ what they are.
So I've revamped the UI a bit: now, in ECDSA mode, you get a dropdown
list selector showing the available elliptic curves (and they're even
named, rather than just given by bit count), and in ED25519 mode even
that disappears. The curve selector for ECDSA and the bits selector
for RSA/DSA are independent controls, so each one remembers its last
known value even while temporarily hidden in favour of the other.
The actual generation function still expects a bit count rather than
an actual curve or algorithm ID, so the easiest way to actually
arrange to populate the drop-down list was to have an array of bit
counts exposed by sshecc.c. That's a bit ugly, but there we go.
One small functional change: if you enter an absurdly low value into
the RSA/DSA bit count box (under 256), PuTTYgen used to give a warning
and reset it to 256. Now it resets it to the default key length of
2048, basically because I was touching that code anyway to change a
variable name and just couldn't bring myself to leave it in a state
where it intentionally chose such an utterly useless key size. Of
course this doesn't prevent generation of 256-bit keys if someone
still really wants one - it just means they don't get one selected as
the result of a typo.
It would be nicer if we could also make this show up as the icon for
the .msi file itself when viewed in Explorer, but apparently nothing
can change that. But at least this still gives us _some_ use for the
cardboard-box icon :-)
Mostly this is a reaction to the reports of Inno Setup having a DLL
hijacking vulnerability. But also, the new installer has several other
nice features that our Inno Setup one didn't provide: it can put the
PuTTY install directory on PATH automatically, and it supports
completely automatic and silent install/uninstall via 'msiexec /q'
which should make it easier for sysadmins to roll out installation in
large organisations. Also, it just seems like good sense to be using
Windows's own native packaging system (or closest equivalent) rather
than going it alone.
(And on the developer side, I have to say I like the fact that WiX
lets me pass in the version number as a set of command-line #define-
equivalents, whereas for Inno Setup I had to have Buildscr apply Perl
rewriting to the source file.)
For the moment, I'm still building the old Inno Setup installer
alongside this one, but I expect to retire it once the WiX one has
survived in the wild for a while and proven itself more or less
stable.
I've found both MSI and WiX to be confusing and difficult
technologies, so this installer has some noticeable pieces missing
(e.g. retrospective reconfiguration of the installed feature set, and
per-user vs systemwide installation) simply because I couldn't get
them to work. I've commented the new installer source code heavily, in
the hope that a passing WiX expert can give me a hand!
A user reported in January that locking down our process ACL causes
get_user_sid's call to OpenProcessToken to fail with a permissions
error. This _shouldn't_ be important, because we'll already have found
and cached the user SID before getting that far - but unfortunately
the call to get_user_sid in winnpc.c was bypassing the cache and
trying the whole process again.
This fix changes the memory ownership semantics of get_user_sid():
it's now an error to free the value it gives you, or else the *next*
call to get_user_sid() will return a stale pointer. Hence, also
removed those frees everywhere they appear.
Now all the uses of the licence text or the short copyright notice get
it from a new header "licence.h", which in turn is built by a Perl
script licence.pl invoked by mkfiles.pl, using LICENCE itself as the
source.
Hence, I can completely remove a whole section from the list of
licence locations in CHECKLST.txt :-)
(cherry picked from commit 9ddd071ec2)
Conflicts:
unix/gtkdlg.c
windows/winpgnt.c
(cherry-picker's notes: one conflict was just changed context, the
other was deleting a copy of the licence that wasn't quite the same
between branches)
This makes the About and Licence boxes copy-and-pasteable, similarly
to what I've just done on Unix.
(But unlike on the Unix side, here I haven't touched the host key
prompt dialog, because that's a standard Windows MessageBox and not
easy to mess around with. Plus, in any case, you can already hit ^C to
copy the whole text out of a MessageBox. Same goes for the PGP
fingerprints dialog.)
As a side effect, several copies of the copyright notice and licence
text have moved from .rc files into C source. I've updated
CHECKLST.txt, but they won't stay there for long.
(cherry picked from commit 2eb952ca31)
Conflicts:
windows/pageant.rc
windows/puttygen.rc
windows/win_res.rc2
(cherry-picker's notes: the conflict was just because several copies
of the licence text were deleted, and they weren't quite the same
between branches)
logevent() doesn't do printf-style formatting (though the logeventf
wrapper in ssh.c does), so if you need to format a message, it has to
be done separately with dupprintf.
(cherry picked from commit 1659cf3f14)
By default Windows processes have wide open ACLs which allow interference
by other processes running as the same user. Adjust our ACL to make this
a bit harder.
Because it's useful to protect PuTTYtel as well, carve winsecur.c into
advapi functions and wincapi.c for crypt32 functions.
(cherry picked from commit 48db456801)
Conflicts:
Recipe
(cherry-picker's note: the conflict was just some context not looking
quite the same)
make_private_security_descriptor and a new function protectprocess().
protectprocess() opens the running PuTTY process and adjusts the
Everyone and user access control entries in its ACL to deny a
selection of permissions which malicious processes running as the same
user could use to hijack PuTTY.
(cherry picked from commit aba7234bc1)
TOOLTYPE_NONNETWORK (i.e. pterm) already has "-log" (as does Unix
PuTTY), so there's no sense suppressing the synonym "-sessionlog".
Undocumented lacunae that remain:
plink accepts -sessionlog, but does nothing with it. Arguably it should.
puttytel accepts -sshlog/-sshrawlog (and happily logs e.g. Telnet
negotiation, as does PuTTY proper).
(cherry picked from commit a454399ec8)
Conflicts:
unix/uxplink.c
windows/winplink.c
(cherry-picker's notes: the conflict was only contextual, in the Plink
help output)
Now all the uses of the licence text or the short copyright notice get
it from a new header "licence.h", which in turn is built by a Perl
script licence.pl invoked by mkfiles.pl, using LICENCE itself as the
source.
Hence, I can completely remove a whole section from the list of
licence locations in CHECKLST.txt :-)
This makes the About and Licence boxes copy-and-pasteable, similarly
to what I've just done on Unix.
(But unlike on the Unix side, here I haven't touched the host key
prompt dialog, because that's a standard Windows MessageBox and not
easy to mess around with. Plus, in any case, you can already hit ^C to
copy the whole text out of a MessageBox. Same goes for the PGP
fingerprints dialog.)
As a side effect, several copies of the copyright notice and licence
text have moved from .rc files into C source. I've updated
CHECKLST.txt, but they won't stay there for long.
logevent() doesn't do printf-style formatting (though the logeventf
wrapper in ssh.c does), so if you need to format a message, it has to
be done separately with dupprintf.
By default Windows processes have wide open ACLs which allow interference
by other processes running as the same user. Adjust our ACL to make this
a bit harder.
Because it's useful to protect PuTTYtel as well, carve winsecur.c into
advapi functions and wincapi.c for crypt32 functions.
Thanks to Colin Harrison for spotting it very quickly. No thanks to
Visual Studio for only giving me a _warning_ when I prototyped a
function with four parameters and called it with five!
On both Unix and Windows, we now redirect the local proxy command's
standard error into a third pipe; data received from that pipe is
broken up at newlines and logged in the Event Log. So if the proxy
command emits any error messages in the course of failing to connect
to something, you now have a fighting chance of finding out what went
wrong.
This feature is disabled in command-line tools like PSFTP and Plink,
on the basis that in that situation it seems more likely that the user
would expect standard-error output to go to the ordinary standard
error in the ordinary way. Only GUI PuTTY catches it and logs it like
this, because it either doesn't have a standard error at all (on
Windows) or is likely to be pointing it at some completely unhelpful
session log file (under X).
I've defined a new value for the 'int type' parameter passed to
plug_log(), which proxy sockets will use to pass their backend
information on how the setup of their proxied connections are going.
I've implemented support for the new type code in all _nontrivial_
plug log functions (which, conveniently, are precisely the ones I just
refactored into backend_socket_log); the ones which just throw all
their log data away anyway will do that to the new code as well.
We use the new type code to log the DNS lookup and connection setup
for connecting to a networked proxy, and also to log the exact command
string sent down Telnet proxy connections (so the user can easily
debug mistakes in the configured format string) and the exact command
executed when spawning a local proxy process. (The latter was already
supported on Windows by a bodgy logging call taking advantage of
Windows in particular having no front end pointer; I've converted that
into a sensible use of the new plug_log facility, and done the same
thing on Unix.)
make_private_security_descriptor and a new function protectprocess().
protectprocess() opens the running PuTTY process and adjusts the
Everyone and user access control entries in its ACL to deny a
selection of permissions which malicious processes running as the same
user could use to hijack PuTTY.
TOOLTYPE_NONNETWORK (i.e. pterm) already has "-log" (as does Unix
PuTTY), so there's no sense suppressing the synonym "-sessionlog".
Undocumented lacunae that remain:
plink accepts -sessionlog, but does nothing with it. Arguably it should.
puttytel accepts -sshlog/-sshrawlog (and happily logs e.g. Telnet
negotiation, as does PuTTY proper).
This brings in the rest of the 0.66 branch, including some changes new
on master.
Conflicts:
doc/plink.but
sshrsa.c
(The conflicts were both trivial: in one, the addition of an extra
parameter to rsa2_newkey on master happened on the line next to 0.66's
addition of a check for NULL return value, and in the other, I'd got
the version number in the plink -h transcript messed up on master.)
Handles managed by winhandl.c have a 'busy' flag, which is used to
mean two things: (a) is a subthread currently blocked on this handle
so various operations in the main thread have to be deferred until it
finishes? And (b) is this handle currently one that should be returned
to the main loop to be waited for?
For HT_INPUT and HT_OUTPUT, those things are either both true or both
false, so a single flag covering both of them is fine. But HT_FOREIGN
handles have the property that they should always be waited for in the
main loop, but no subthread is blocked on them. The latter means that
operations done on them in the main thread should not be deferred; the
only such operation is cleaning them up in handle_free().
handle_free() was failing to spot this, and was deferring freeing
HT_FOREIGN handles until their subthread terminated - which of course
never happened. As a result, when a named pipe server was closed, its
actual Windows event object got destroyed, but winhandl.c still kept
passing it back to the main thread, leading to a tight loop because
MsgWaitForMultipleObjects would return ERROR_INVALID_HANDLE and never
block.
(cherry picked from commit 431f8db862)
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.
(cherry picked from commit 64ec5e03d5)
For the moment we're also retaining the old ones. Not sure when will
be the best time to get rid of those; after the next release, perhaps?
(cherry picked from commit e88b8d21f2)
We've had several reports that launching saved sessions from the
Windows 10 jump list fails; Changyu Li reports that this is because we
create those IShellLink objects with a command line string starting
with @, and in Windows 10 that causes the SetArguments method to
silently do the wrong thing.
(cherry picked from commit 8bf5c1b31f)
This is generated in response to the SendInput() Windows API call, if
that in turn is passed an KEYBDINPUT structure with KEYEVENTF_UNICODE
set. That method of input generation is used by programs such as
'WinCompose' to send an arbitrary Unicode character as if it had been
typed at the keyboard, even if the keyboard doesn't actually provide a
key for it.
Like VK_PROCESSKEY, this key code is an exception to our usual policy
of manually translating keystrokes: we handle it by calling
TranslateMessage, to get back the Unicode character it contains as a
WM_CHAR message.
(If that Unicode character in turn is outside the BMP, it may come
back as a pair of WM_CHARs in succession containing UTF-16 surrogates;
if so, that's OK, because the new Unicode WM_CHAR handler can cope.)
(cherry picked from commit 65f3500906)
This causes WM_CHAR messages sent to us to have a wParam containing a
16-bit value encoded in UTF-16, rather than an 8-bit value encoded in
the system code page.
As far as I can tell, there aren't many other knock-on effects - e.g.
you can still interact with the window using ordinary char-based API
functions such as SetWindowText, and the Windows API will do the
necessary conversions behind the scenes. However, even so, I'm half
expecting some sort of unforeseen bug to show up as a result of this.
(cherry picked from commit 67e5ceb9a8)
Commit f2e61275f introduced the use of uintptr_t, without adding an
include of <stdint.h> which is where the C standard says that type
should be defined. This didn't cause a build failure, because Visual
Studio also defines it in <stddef.h> which we do include. But a user
points out that other Windows toolchains - e.g. MinGW - don't
necessarily do the same.
I can't add an unconditional include of <stdint.h>, because the VS I
use for the current official builds doesn't have that header at all.
So I conditionalise it out for old VS; if it needs throwing out for
any other toolchain, I'll add further conditions as reports come in.
Handles managed by winhandl.c have a 'busy' flag, which is used to
mean two things: (a) is a subthread currently blocked on this handle
so various operations in the main thread have to be deferred until it
finishes? And (b) is this handle currently one that should be returned
to the main loop to be waited for?
For HT_INPUT and HT_OUTPUT, those things are either both true or both
false, so a single flag covering both of them is fine. But HT_FOREIGN
handles have the property that they should always be waited for in the
main loop, but no subthread is blocked on them. The latter means that
operations done on them in the main thread should not be deferred; the
only such operation is cleaning them up in handle_free().
handle_free() was failing to spot this, and was deferring freeing
HT_FOREIGN handles until their subthread terminated - which of course
never happened. As a result, when a named pipe server was closed, its
actual Windows event object got destroyed, but winhandl.c still kept
passing it back to the main thread, leading to a tight loop because
MsgWaitForMultipleObjects would return ERROR_INVALID_HANDLE and never
block.
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.
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.
I noticed that Unix PSCP was unwantedly renaming downloaded files
which had a backslash in their names, because pscp.c's stripslashes()
treated \ as a path component separator, since it hadn't been modified
since PSCP ran on Windows only.
It also turns out that pscp.c, psftp.c and winsftp.c all had a
stripslashes(), and they didn't all have quite the same prototype. So
now there's one in winsftp.c and one in uxsftp.c, with appropriate
OS-dependent behaviour, and the ones in pscp.c and psftp.c are gone.
We are passing pointers as third argument to AppendMenu. Do not
truncate them to UINT, use UINT_PTR instead which has the required
size on 64bit Windows.
We're passing a pointer as 4th argument to WinHelp. Do not cast it to
DWORD which would truncate the pointer. Instead use UINT_PTR as that
is what WinHelp expects.
We've had several reports that launching saved sessions from the
Windows 10 jump list fails; Changyu Li reports that this is because we
create those IShellLink objects with a command line string starting
with @, and in Windows 10 that causes the SetArguments method to
silently do the wrong thing.
This is generated in response to the SendInput() Windows API call, if
that in turn is passed an KEYBDINPUT structure with KEYEVENTF_UNICODE
set. That method of input generation is used by programs such as
'WinCompose' to send an arbitrary Unicode character as if it had been
typed at the keyboard, even if the keyboard doesn't actually provide a
key for it.
Like VK_PROCESSKEY, this key code is an exception to our usual policy
of manually translating keystrokes: we handle it by calling
TranslateMessage, to get back the Unicode character it contains as a
WM_CHAR message.
(If that Unicode character in turn is outside the BMP, it may come
back as a pair of WM_CHARs in succession containing UTF-16 surrogates;
if so, that's OK, because the new Unicode WM_CHAR handler can cope.)
This causes WM_CHAR messages sent to us to have a wParam containing a
16-bit value encoded in UTF-16, rather than an 8-bit value encoded in
the system code page.
As far as I can tell, there aren't many other knock-on effects - e.g.
you can still interact with the window using ordinary char-based API
functions such as SetWindowText, and the Windows API will do the
necessary conversions behind the scenes. However, even so, I'm half
expecting some sort of unforeseen bug to show up as a result of this.
Coverity complained that some paths through the loop in the
WM_INITDIALOG handler might leave firstpath==NULL. In fact this can't
happen because the input data to that loop is largely static and we
know what it looks like, but it doesn't seem unreasonable to add an
assertion anyway, to keep static checkers happy and as an explanatory
quasi-comment for humans.
Our config boxes are constructed using the CreateDialog() API
function, rather than the modal DialogBox(). CreateDialog() is not
that different from CreateWindow(), so windows created with it don't
appear on the screen automatically; MSDN says that they must be shown
via ShowWindow(), just like non-dialog windows have to be. But we
weren't doing that at any point!
So how was our config box ever getting displayed at all? Apparently by
sheer chance, it turns out. The handler for a selection change in the
tree view, which has to delete a whole panel of controls and creates a
different set, surrounds that procedure with some WM_SETREDRAW calls
and an InvalidateRect(), to prevent flicker while lots of changes were
being made. And the creation of the _first_ panelful of controls, at
dialog box setup, was done by simply selecting an item in the treeview
and expecting that handler to be recursively called. And it appears
that calling WM_SETREDRAW(TRUE) and then InvalidateRect was
undocumentedly having an effect equivalent to the ShowWindow() we
should have called, so that we never noticed the latter was missing.
But a recent Vista update (all reports implicate KB3057839) has caused
that not to work any more: on an updated Vista machine, in some
desktop configurations, it seems that any attempt to fiddle with
WM_SETREDRAW during dialog setup can leave the dialog box in a really
unhelpful invisible state - the window is _physically there_ (you can
see its taskbar entry, and the mouse pointer changes as you move over
where its edit boxes are), but 100% transparent.
So now we're doing something a bit more sensible. The first panelful
of controls is created directly by the WM_INITDIALOG handler, rather
than recursing into code that wasn't really designed to run at setup
time. To be on the safe side, that handler for treeview selection
change is also disabled until the WM_INITDIALOG handler has finished
(like we already did with the WM_COMMAND handler), so that we can be
sure of not accidentally messing about with WM_SETREDRAW at all during
setup. And at the end of setup, we show the window in the sensible
way, by a docs-approved call to ShowWindow().
This appears (on the one machine I've so far tested it on) to fix the
Vista invisible-window issue, and also it should be more API-compliant
and hence safer in future.
(cherry picked from commit 6163710f04)
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.
(cherry picked from commit c8f83979a3)
Conflicts:
pageant.c
Cherry-picker's notes: the conflict was because the original commit
also added a use of the same feature in the centralised Pageant code,
which doesn't exist on this branch. Also I had to remove 'const' from
the type of the second parameter to wrap_send_port_open(), since this
branch hasn't had the same extensive const-fixing as master.
The last use of it, to store the contents of the saved session name
edit box, was removed nearly two years ago in svn r9923 and replaced
by ctrl_alloc_with_free. The mechanism has been unused ever since
then, and I suspect any further uses of it would be a bad idea for the
same reasons, so let's get rid of it.
(cherry picked from commit 42c592c4ef)
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.
(cherry picked from commit 62a1bce7cb)
When a winhandl.c input thread returns EOF to the main thread, the
latter might immediately delete the input thread's context. I
carefully wrote in a comment that in that case we had to not touch ctx
ever again after signalling to the main thread - but the test for
whether that was true, which also touched ctx, itself came _after_ the
SetEvent which sent that signal. Ahem.
Spotted by Minefield, which it looks as if I haven't run for a while.
(cherry picked from commit 9fec2e7738)
I had set up an event object for signalling incoming connections to
the named pipe, and then called handle_add_foreign_event to get that
event object watched for connections - but when I closed down the
listening pipe, I deleted the event object without also cancelling
that foreign-event handle, so that winhandl.c would potentially call
the callback for a destroyed object.
(cherry picked from commit 6f241cef2c)
This was an old bug, fixed around 0.59, which apparently regressed
when I rewrote the main event loop using the toplevel_callback
mechanism.
Investigation just now suggests that it has to do with my faulty
assumption that Windows PeekMessage would deliver messages in its
message queue in FIFO order (i.e. that the thing calling itself a
message queue is actually a _queue_). In fact my WM_NETEVENT seems to
like to jump the queue, so that once a steady stream of them starts
arriving, we never do anything else in the main event loop (except
deal with handles).
Worked around in a simple and slightly bodgy way, namely, we don't
stop looping on PeekMessage and run our toplevel callbacks until we've
either run out of messages completely or else seen at least one that
_isn't_ a WM_NETEVENT. That way we should reliably interleave NETEVENT
processing with processing of other stuff.
(cherry picked from commit 7d97c2a8fd)
To understand the handle leak bug that I fixed in git commit
7549f2da40, I had to think fairly hard
to remind myself what all this code was doing, which means the
comments weren't good enough. Expanded and rewritten some of them in
the hope that things will be clearer next time.
(cherry picked from commit a87a14ae0f)
Cherry-picker's notes: this apparently pointless commit is required on
this branch because it's a dependency of the rather less pointless
9fec2e7738.