A user reports that if you have MIT KfW loaded, and your PuTTY session
terminates without the PuTTY process exiting, and you select 'Restart
Session' from the menu, then a crash occurs inside the Kerberos
library itself. Scuttlebutt on the Internet suggested this might be to
do with unloading and then reloading the DLL within the process
lifetime, which indeed we were doing.
Now we avoid doing that for the KfW library in particular, by keeping
a tree234 of module handles marked 'never unload this'.
This is a workaround at best, but it seems to stop the problem
happening in my own tests.
If you don't, they are permanently leaked. A user points out that this
is particularly bad in Pageant, with the new named-pipe-based IPC,
since it will spawn an input and output I/O thread per named pipe
connection, leading to two handles being leaked every time.
In commit f69cf86a61, I added a call to term_update that happens
when we receive WM_VSCROLL / SB_THUMBPOSITION in the subsidiary
message loop that Windows creates during the handling of WM_SYSCOMMAND
/ SC_VSCROLL. The effect was that interactive dragging of the
scrollbar now redraws the window at every step, whereas previously it
didn't.
A user just pointed out that if you click on one of the scrollbar end
buttons and hold it down until it begins emulating key repeat, the
same bug occurs: the window isn't redrawn until you release the mouse
button and the subsidiary message loop ends.
This commit extends the previous fix to cover all of the WM_VSCROLL
subtypes, instead of just SB_THUMBPOSITION and SB_THUMBTRACK. Redraws
while holding down those scrollbar buttons now work again.
This is used to notify the Seat that some data has been cleared from
the backend's outgoing data buffer. In other words, it notifies the
Seat that it might be worth calling backend_sendbuffer() again.
We've never needed this before, because until now, Seats have always
been the 'main program' part of the application, meaning they were
also in control of the event loop. So they've been able to call
backend_sendbuffer() proactively, every time they go round the event
loop, instead of having to wait for a callback.
But now, the SSH proxy is the first example of a Seat without
privileged access to the event loop, so it has no way to find out that
the backend's sendbuffer has got smaller. And without that, it can't
pass that notification on to plug_sent, to unblock in turn whatever
the proxied connection might have been waiting to send.
In fact, before this commit, sshproxy.c never called plug_sent at all.
As a result, large data uploads over an SSH jump host would hang
forever as soon as the outgoing buffer filled up for the first time:
the main backend (to which sshproxy.c was acting as a Socket) would
carefully stop filling up the buffer, and then never receive the call
to plug_sent that would cause it to start again.
The new callback is ignored everywhere except in sshproxy.c. It might
be a good idea to remove backend_sendbuffer() entirely and convert all
previous uses of it into non-empty implementations of this callback,
so that we've only got one system; but for the moment, I haven't done
that.
This allows the 'no trivial auth' option introduced by the previous
commit to be tested. Uppity has grown three new options to make it
accept "none" authentication, keyboard-interactive involving no
prompts, and the perverse sending of USERAUTH_SUCCESS after a
signatureless public-key offer.
The first of those options also enables the analogue in SSH-1; the
other two have no SSH-1 analogues in the first place. (SSH-1 public
key authentication has a challenge-response structure that doesn't
contain any way to terminate the exchange early with success. And the
TIS and CryptoCard methods, which are its closest analogue of k-i,
have a fixed number of prompts, which is not 0.)
Suggested by Manfred Kaiser, who also wrote most of this patch
(although outlying parts, like documentation and SSH-1 support, are by
me).
This is a second line of defence against the kind of spoofing attacks
in which a malicious or compromised SSH server rushes the client
through the userauth phase of SSH without actually requiring any auth
inputs (passwords or signatures or whatever), and then at the start of
the connection phase it presents something like a spoof prompt,
intended to be taken for part of userauth by the user but in fact with
some more sinister purpose.
Our existing line of defence against this is the trust sigil system,
and as far as I know, that's still working. This option allows a bit of
extra defence in depth: if you don't expect your SSH server to
trivially accept authentication in the first place, then enabling this
option will cause PuTTY to disconnect if it unexpectedly does so,
without the user having to spot the presence or absence of a fiddly
little sigil anywhere.
Several types of authentication count as 'trivial'. The obvious one is
the SSH-2 "none" method, which clients always try first so that the
failure message will tell them what else they can try, and which a
server can instead accept in order to authenticate you unconditionally.
But there are two other ways to do it that we know of: one is to run
keyboard-interactive authentication and send an empty INFO_REQUEST
packet containing no actual prompts for the user, and another even
weirder one is to send USERAUTH_SUCCESS in response to the user's
preliminary *offer* of a public key (instead of sending the usual PK_OK
to request an actual signature from the key).
This new option detects all of those, by clearing the 'is_trivial_auth'
flag only when we send some kind of substantive authentication response
(be it a password, a k-i prompt response, a signature, or a GSSAPI
token). So even if there's a further path through the userauth maze we
haven't spotted, that somehow avoids sending anything substantive, this
strategy should still pick it up.
This generates primality certificates for numbers, in the form of
Python / testcrypt code that calls Pockle methods. It factors p-1 by
calling out to the 'yafu' utility, which is a moderately sophisticated
integer factoring tool (including ECC and quadratic sieve methods)
that runs as a standalone command-line program.
Also added a Pockle test generated as output from this script, which
verifies the primality of the three NIST curves' moduli and their
generators' orders. I already had Pockle certificates for the moduli
and orders used in EdDSA, so this completes the set, and it does it
without me having had to do a lot of manual work.
Enthusiastic copy-paste: in commit 17c57e1078 I added the same
precautionary call to ensure_handlewaits_tree_exists() everywhere,
even in functions that didn't actually need to use the tree.
This script makes 128 connections to your SSH agent at once, and then
sends requests down them in random order to check that the agent is
correctly selecting between all its incoming sockets / named pipes /
whatever.
128 is bigger than MAXIMUM_WAIT_OBJECTS, so a successful run of this
script inside a Windows PuTTY agent-forwarding to a Pageant indicates
that both the PuTTY and the Pageant are managing to handle >64 I/O
subthreads without overloading their event loop.
Before commit 6e69223dc2, Pageant would stop working after a
certain number of PuTTYs were active at the same time. (At most about
60, but maybe fewer - see below.)
This was because of two separate bugs. The easy one, fixed in
6e69223dc2 itself, was that PuTTY left each named-pipe connection
to Pageant open for the rest of its lifetime. So the real problem was
that Pageant had too many active connections at once. (And since a
given PuTTY might make multiple connections during userauth - one to
list keys, and maybe another to actually make a signature - that was
why the number of _PuTTYs_ might vary.)
It was clearly a bug that PuTTY was leaving connections to Pageant
needlessly open. But it was _also_ a bug that Pageant couldn't handle
more than about 60 at once. In this commit, I fix that secondary bug.
The cause of the bug is that the WaitForMultipleObjects function
family in the Windows API have a limit on the number of HANDLE objects
they can select between. The limit is MAXIMUM_WAIT_OBJECTS, defined to
be 64. And handle-io.c was using a separate event object for each I/O
subthread to communicate back to the main thread, so as soon as all
those event objects (plus a handful of other HANDLEs) added up to more
than 64, we'd start passing an overlarge handle array to
WaitForMultipleObjects, and it would start not doing what we wanted.
To fix this, I've reorganised handle-io.c so that all its subthreads
share just _one_ event object to signal readiness back to the main
thread. There's now a linked list of 'struct handle' objects that are
ready to be processed, protected by a CRITICAL_SECTION. Each subthread
signals readiness by adding itself to the linked list, and setting the
event object to indicate that the list is now non-empty. When the main
thread receives the event, it iterates over the whole list processing
all the ready handles.
(Each 'struct handle' still has a separate event object for the main
thread to use to communicate _to_ the subthread. That's OK, because no
thread is ever waiting on all those events at once: each subthread
only waits on its own.)
The previous HT_FOREIGN system didn't really fit into this framework.
So I've moved it out into its own system. There's now a handle-wait.c
which deals with the relatively simple job of managing a list of
handles that need to be waited for, each with a callback function;
that's what communicates a list of HANDLEs to event loops, and
receives the notification when the event loop notices that one of them
has done something. And handle-io.c is now just one client of
handle-wait.c, providing a single HANDLE to the event loop, and
dealing internally with everything that needs to be done when that
handle fires.
The new top-level handle-wait.c system *still* can't deal with more
than MAXIMUM_WAIT_OBJECTS. At the moment, I'm reasonably convinced it
doesn't need to: the only kind of HANDLE that any of our tools could
previously have needed to wait on more than one of was the one in
handle-io.c that I've just removed. But I've left some assertions and
a TODO comment in there just in case we need to change that in future.
Commit 0d3bb73608 inserted PROXY_SSH just before PROXY_CMD, so
that it got the numerical value that PROXY_CMD had before. But that
meant that any saved session configured as PROXY_CMD by an older build
of PuTTY would be regarded as PROXY_SSH by the new version - even
after the previous commit fixed the unconditional use of SSH proxying
regardless of the type setting.
Now PROXY_CMD has its old value back, and PROXY_SSH is inserted at the
_end_ of the enum. (Or rather, just before the extra-weird PROXY_FUZZ,
which is allowed to have an unstable numerical value because it's
never stored in a saved session at all.)
This is all rather unsatisfactory, and makes me wish I'd got round to
reworking the saved data format to use keywords in place of integers.
In commit 0d3bb73608, I introduced the new SSH / jump-host proxy
type, which should be invoked by proxy.c when CONF_proxy_type is set
to PROXY_SSH. In fact, I left out the check, so it's invoked by
proxy.c _unconditionally_, after the check to see whether proxying is
required at all. So any saved session configured with any other proxy
type (other than PROXY_NONE) would be treated as PROXY_SSH by mistake.
How embarrassing. I did remember at one point that I needed to fix
this, but it fell out of my head before I pushed!
In commit 9cc586e605 I changed the low-level key-file reading
routines like read_header and read_body so that they read from a
BinarySource via get_byte(), rather than from a FILE * via fgetc. But
I forgot that the two functions don't signal end-of-file the same way,
so testing the return value of get_byte() against EOF is pointless and
will never match, and conversely, real EOF won't be spotted unless you
also examine the error indicator in the BinarySource.
As a result, a key file that ends without a trailing newline will
cause a tight loop in one of those low-level read routines.
This introduces a new entry to the radio-button list of proxy types,
in which the 'Proxy host' box is taken to be the name of an SSH server
or saved session. We make an entire subsidiary SSH connection to that
host, open a direct-tcpip channel through it, and use that as the
connection over which to run the primary network connection.
The result is basically the same as if you used a local proxy
subprocess, with a command along the lines of 'plink -batch %proxyhost
-nc %host:%port'. But it's all done in-process, by having an SshProxy
object implement the Socket trait to talk to the main connection, and
implement Seat and LogPolicy to talk to its subsidiary SSH backend.
All the refactoring in recent years has got us to the point where we
can do that without both SSH instances fighting over some global
variable or unique piece of infrastructure.
From an end user perspective, doing SSH proxying in-process like this
is a little bit easier to set up: it doesn't require you to bake the
full pathname of Plink into your saved session (or to have it on the
system PATH), and the SshProxy setup function automatically turns off
SSH features that would be inappropriate in this context, such as
additional port forwardings, or acting as a connection-sharing
upstream. And it has minor advantages like getting the Event Log for
the subsidiary connection interleaved in the main Event Log, as if it
were stderr output from a proxy subcommand, without having to
deliberately configure the subsidiary Plink into verbose mode.
However, this is an initial implementation only, and it doesn't yet
support the _big_ payoff for doing this in-process, which (I hope)
will be the ability to handle interactive prompts from the subsidiary
SSH connection via the same user interface as the primary one. For
example, you might need to answer two password prompts in succession,
or (the first time you use a session configured this way) confirm the
host keys for both proxy and destination SSH servers. Comments in the
new source file discuss some design thoughts on filling in this gap.
For the moment, if the proxy SSH connection encounters any situation
where an interactive prompt is needed, it will make the safe
assumption, the same way 'plink -batch' would do. So it's at least no
_worse_ than the existing technique of putting the proxy connection in
a subprocess.
This notifies the Seat that the entire backend session has finished
and closed its network connection - or rather, that it _might_ have
done, and that the frontend should check backend_connected() if it
wasn't planning to do so already.
The existing Seat implementations haven't needed this: the GUI ones
don't actually need to do anything specific when the network
connection goes away, and the CLI ones deal with it by being in charge
of their own event loop so that they can easily check
backend_connected() at every possible opportunity in any case. But I'm
about to introduce a new Seat implementation that does need to know
this, and doesn't have any other way to get notified of it.
This flag is set in backends which can be used programmatically to
proxy a network connection in place of running a shell session. That
is true of both SSH proper, and the psusan ssh-connection protocol.
Nothing yet uses this flag, but something is about to.
Since ca9cd983e1, changing colour config mid-session had no effect
(until the palette was reset for some other reason). Now it does take
effect immediately (provided that the palette has not been overridden by
escape sequence -- this is new with ca9cd983e1).
This changes the semantics of palette_reset(): the only important
parameter when doing that is whether we keep escape sequence overrides
-- there's no harm in re-fetching config and platform colours whether or
not they've changed -- so that's what the parameter becomes (with a
sense that doesn't require changing the call sites). The other part of
this change is actually remembering to trigger this when the
configuration is changed.
A user pointed out that once we've identified the key algorithm from
an apparent public-key blob, we call ssh_key_new_pub on the blob data
and assume it will succeed. But there are plenty of ways it could
still fail, and ssh_key_new_pub could return NULL.
I was cleaning up the 'struct handle', but not the underlying HANDLE.
As a result, any PuTTY process that makes a request to Pageant keeps
the named pipe connection open until the end of the process's
lifetime.
I was checking a HANDLE against INVALID_HANDLE_VALUE to decide whether
it should be closed. But ten lines further up, I was setting it
manually to NULL to suppress the close. Oops.
Gives a quick and easy report of which HW-accelerated crypto
implementations are (a) compiled in to testcrypt, (b) actually
instantiable at testcrypt run time.
Less than 12 hours after 0.75 went out of the door, a user pointed out
that enabling the 'Use system colours' config option causes an
immediate NULL-dereference crash. The reason is because a chain of
calls from term_init() ends up calling back to the Windows
implementation of the palette_get_overrides() method, which responds
by trying to call functions on the static variable 'term' in window.c,
which won't be initialised until term_init() has returned.
Simple fix: palette_get_overrides() is now given a pointer to the
Terminal that it should be updating, because it can't find it out any
other way.
This fulfills our long-standing Mayhem-difficulty wishlist item
'win-command-prompt': this is a Windows pterm in the sense that when
you run it you get a local cmd.exe running inside a PuTTY-style window.
Advantages of this: you get the same free choice of fonts as PuTTY has
(no restriction to a strange subset of the system's available fonts);
you get the same copy-paste gestures as PuTTY (no mental gear-shifting
when you have command prompts and SSH sessions open on the same
desktop); you get scrollback with the PuTTY semantics (scrolling to
the bottom gets you to where the action is, as opposed to the way you
could accidentally find yourself 500 lines past the end of the action
in a real console).
'win-command-prompt' was at Mayhem difficulty ('Probably impossible')
basically on the grounds that with Windows's old APIs for accessing
the contents of consoles, there was no way I could find to get this to
work sensibly. What was needed to make it feasible was a major piece
of re-engineering work inside Windows itself.
But, of course, that's exactly what happened! In 2019, the new ConPTY
API arrived, which lets you create an object that behaves like a
Windows console at one end, and round the back, emits a stream of
VT-style escape sequences as the screen contents evolve, and accepts a
VT-style input stream in return which it will parse function and arrow
keys out of in the usual way.
So now it's actually _easy_ to get this to basically work. The new
backend, in conpty.c, has to do a handful of magic Windows API calls
to set up the pseudo-console and its feeder pipes and start a
subprocess running in it, a further magic call every time the PuTTY
window is resized, and detect the end of the session by watching for
the subprocess terminating. But apart from that, all it has to do is
pass data back and forth unmodified between those pipes and the
backend's associated Seat!
That said, this is new and experimental, and there will undoubtedly be
issues. One that I already know about is that you can't copy and paste
a word that has wrapped between lines without getting an annoying
newline in the middle of it. As far as I can see this is a fundamental
limitation: the ConPTY system sends the _same_ escape sequence stream
for a line that wrapped as it would send for a line that had a logical
\n at what would have been the wrap point. Probably the best we can do
to mitigate this is to adopt a different heuristic for newline elision
that's right more often than it's wrong.
For the moment, that experimental-ness is indicated by the fact that
Buildscr will build, sign and deliver a copy of pterm.exe for each
flavour of Windows, but won't include it in the .zip file or in the
installer. (In fact, that puts it in exactly the same ad-hoc category
as PuTTYtel, although for completely different reasons.)
icons/Makefile will now rebuild them, but also, as per this code
base's usual policy with Windows icons, they're committed directly in
the windows subdir.
Now they're done by putty.rc and puttytel.rc, before including
putty-common.rc2. So another user of putty-common.rc2 can disagree on
what icons to use.
This prepares the ground for a second essentially similarly-shaped
program reusing most of window.c but handling its command line and
startup differently. A couple of large parts of WinMain() to do with
backend selection and command-line handling are now subfunctions in a
separate file putty.c.
Also, our custom AppUserModelId is defined in that file, so that it
can vary with the client application.
The code to find out the location of the c:\windows\system32 directory
was already present, in load_system32_dll(). Now it's moved out into a
function of its own, so it can be called in other contexts.
It's silly to require all the time-consuming cmake configuration for
the source code, if all you want to do is to build the documentation.
My own website update script will like this optimisation, and so will
Buildscr.
In order to make doc/CMakeLists.txt work standalone, I had to add a
'project' header (citing no languages, so that cmake won't even bother
looking for a C compiler); include FindGit, which cmake/setup.cmake
now won't be doing for it; change all references to CMAKE_SOURCE_DIR
to CMAKE_CURRENT_SOURCE_DIR/.. (since now the former will be defined
differently in a nested or standalone doc build); and spot whether
we're nested or not in order to conditionalise things designed to
interoperate with the parent CMakeLists.
It's always the same as the cwd when the script is invoked, and by
having the script get it _from_ its own cwd, we arrange a bit of
automatic normalisation in situations where you need to invoke it with
some non-canonical path like one ending in "/.." - which I'll do in
the next commit.
When building against the Mac Homebrew installation of GTK, you find
that GTK exists, libX11 exists, but the integration between the two
(in the form of the header file gdk/gdkx.h) doesn't exist. In that
situation, we need to compile out X11 support.
doc/CMakeLists.txt now sets a variable indicating that we either have,
or can build, each individual man page. And when we call our
installed_program() function to mark a program as official enough to
put in 'make install', that function also installs the man page
similarly if it exists, and warns if not.
For the convenience of people building-and-installing from the .tar.gz
we ship, I've arranged that they can still get the man pages installed
without needing Halibut: the previous commit ensured that the prebuilt
man pages are still in the tarball, and this one arranges that if we
don't have Halibut but we do have prebuilt man pages, then we can
'build' them by copying from the prebuilt versions.
The standalone separate doc/Makefile is gone, replaced by a
CMakeLists.txt that makes 'doc' function as a subdirectory of the main
CMake build system. This auto-detects Halibut, and if it's present,
uses it to build the man pages and the various forms of the main
manual, including the Windows CHM help file in particular.
One awkward thing I had to do was to move just one config directive in
blurb.but into its own file: the one that cites a relative path to the
stylesheet file to put into the CHM. CMake builds often like to be
out-of-tree, so there's no longer a fixed relative path between the
build directory and chm.css. And Halibut has no concept of an include
path to search for files cited by other files, so I can't fix that
with an -I option on the Halibut command line. So I moved that single
config directive into its own file, and had CMake write out a custom
version of that file in the build directory citing the right path.
(Perhaps in the longer term I should fix that omission in Halibut;
out-of-tree friendliness seems like a useful feature. But even if I
do, I still need this build to work now.)
I'm about to want to embed the current git commit into a Halibut
source file, for which I'll need to add a second output mode to the
existing script that finds it out.
Since the previous commit is causing an RC2 build of 0.75 anyway,
let's take the opportunity to bring in updates to the docs from main,
so that the release will have the most up-to-date version available.
This is a combined cherry-pick of:
f6142ba29b7c1bea59a3f5d1d4ce4b
Jacob spots that on Windows, current PuTTY is not compatible with
0.74, if one of them acts as a connection sharing upstream and the
other as a downstream. That's because commit 1344d4d1cd
accidentally changed the hash preimage in capi_obfuscate_string() so
that it no longer had an SSH-like string length field at the front. So
the two versions of PuTTY will expect the named pipe to have a
different pathname, and so they won't be able to find each other.
Interoperation between PuTTY versions is not the most important use
case of connection sharing - surely the typical user will invoke it by
activating the same session twice, or by using Duplicate Session. But
it was never intended to deliberately _not_ work, so let's fix it
before 0.75 goes out, so that at least the incompatible behaviour will
only ever have appeared in development snapshots.
I've recently been coming round in general to the idea that -Werror is
fine for developers and centralised binary builds, but has too many
unanticipated failure modes in the field (with everyone's different
versions of compilers, headers etc) to leave turned on for the 'just
download and build' source tarball that's supposed to work everywhere.
On main, I've already made the change to hide it behind a cmake
'strict' setting.
In particular, I've just done pre-release build tests with various
versions of GTK, which reminded me that the GTK 2 installation on
Ubuntu 20.04 fails to build at -Werror, because GTK's own header files
have a warning-generating inconsistency. (glib/gtypes.h declares
GTimeVal as deprecated, and then gtk/gtktooltips.h uses it anyway.)
Clearly this is the kind of thing that ought not to break the build of
a client application!
Now that the main source file of Plink in each platform directory has
the same name, we can put centralise the main definition of the
program in the main CMakeLists.txt, and in the platform directory,
just add the few extra modules needed to clear up platform-specific
details.
The same goes for psocks. And PSCP and PSFTP could have been moved to
the top level already - I just hadn't done it in the initial setup.
This gets rid of all those annoying 'win', 'ux' and 'gtk' prefixes
which made filenames annoying to type and to tab-complete. Also, as
with my other recent renaming sprees, I've taken the opportunity to
expand and clarify some of the names so that they're not such cryptic
abbreviations.
