Ian Jackson recently tried to use the recipe in the psusan manpage for
talking to UML, and found that the connection was not successfully set
up, because at some point during startup, UML read the SSH greeting
(ok, the bare-ssh-connection greeting) from its input fd and threw it
away. So by the time psusan was run by the guest init process, the
greeting wasn't there to be read.
Ian's report: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=991958
I was also able to reproduce this locally, which makes me wonder why I
_didn't_ notice it when I originally wrote that part of the psusan man
page. It worked for me before, honest! But now it doesn't.
Anyway. The ssh verstring module already has a mode switch to decide
whether we ought to send our greeting before or after waiting for the
other side's greeting (because that decision varies between client and
server, and between SSH-1 and SSH-2). So it's easy to implement an
override that forces it to 'wait for the server greeting first'.
I've added this as yet another bug workaround flag. But unlike all the
others, it can't be autodetected from the server's version string,
because, of course, we have to act on it _before_ seeing the server's
greeting and version string! So it's a manual-only flag.
However, I've mentioned it in the UML section of the psusan man page,
since that's the place where I _know_ people are likely to need to use
this flag.
Following the same pattern as the previous one (commit 6c924ba862),
except that this time, I don't have to _set up_ the pattern in the
front-end code of presenting the current and previous key details -
just change over the actual string literals in putty.h.
But the rest is the same: new keys at the top of pgpkeys.but, old ones
relegated to the historical appendix, key ids in sign.sh switched over.
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.
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.
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.
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.
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 clears up another large pile of clutter at the top level, and in
the process, allows me to rename source files to things that don't all
have that annoying 'ssh' prefix at the top.
It was there because of a limitation of mkfiles.pl, which had a single
list of include directories that it used on all platforms. CMake does
not. So now there's an easier and more sensible way to have a
different header file included on Windows and Unix: call it the same
name in the two subdirectories, and rely on CMake having put the right
one of those subdirs on the include path.
This is a module that I'd noticed in the past was too monolithic.
There's a big pile of stub functions in uxpgnt.c that only have to be
there because the implementation of true X11 _forwarding_ (i.e.
actually managing a channel within an SSH connection), which Pageant
doesn't need, was in the same module as more general X11-related
utility functions which Pageant does need.
So I've broken up this awkward monolith. Now x11fwd.c contains only
the code that really does all go together for dealing with SSH X
forwarding: the management of an X forwarding channel (including the
vtables to make it behave as Channel at the SSH end and a Plug at the
end that connects to the local X server), and the management of
authorisation for those channels, including maintaining a tree234 of
possible auth values and verifying the one we received.
Most of the functions removed from this file have moved into the utils
subdir, and also into the utils library (i.e. further down the link
order), because they were basically just string and data processing.
One exception is x11_setup_display, which parses a display string and
returns a struct telling you everything about how to connect to it.
That talks to the networking code (it does name lookups and makes a
SockAddr), so it has to live in the network library rather than utils,
and therefore it's not in the utils subdirectory either.
The other exception is x11_get_screen_number, which it turned out
nothing called at all! Apparently the job it used to do is now done as
part of x11_setup_display. So I've just removed it completely.
This replaces the pure radio-button setup that we've always had on the
Session config panel.
Since the last release, that set of radio buttons has been getting out
of hand. We've added two new protocols (SUPDUP, and the 'bare
ssh-connection' aka psusan protocol), neither of which is mainstream
enough to be a sensible thing to wave at all users on the front page
of the config GUI, so that they perhaps start wondering if that's the
protocol they want to use, or get sidetracked by going and looking it
up.
The replacement UI still has radio buttons, but only for the most
common protocols, which will typically be SSH and serial. Everything
else is relegated to a drop-down list sitting next to a third radio
button labelled "Other".
In every be_* module providing a backends[] list, there's also a
variable n_ui_backends which indicates how many of the backends ought
to appear as first-level radio buttons.
(Credit where due: this patch is a joint effort between Jacob and me,
and is one of those rare cases where it would be nice to be able to
put both our names into the Author field of the commit. Failing that,
I can at least mention it here.)
Now we pass the whole set of fingerprints, and also a displayable
format for the full host public key.
NFC: this commit doesn't modify any of the host key prompts to _use_
any of the new information. That's coming next.
Each of these #defines represents a block of 256 code values that are
used, internally to the terminal code, to indicate that a character is
in one of the currently selected single-byte character sets. One
effect of this is that reconfiguring the character set in mid-session
causes all the text already on screen to be redrawn.
Unfortunately, those 512 code points were allocated at 0xF000-0xF1FF,
which is inside the Unicode private-use area. So if a font uses that
area to define actually useful glyphs, then those glyphs won't be
displayed correctly by PuTTY; instead, outputting 0xF000+'A' (for
example) will display as 'A'. A user recently reported this problem
with the 'Hack' font from https://github.com/ryanoasis/nerd-fonts .
RDB's comment next to the #defines suggested that this was done on
purpose for consistency with Linux (though it's not clear what part of
Linux; perhaps the virtual console driver used to work this way?). But
now it's getting in the way of actually useful Unicode characters,
that consistency doesn't seem like the most important thing. (Also, it
just seems wrong to me that you even _can_ cause PuTTY's terminal
emulator to use these special internal character representations by
sending legal UTF-8.)
So I've moved this block of 512 characters to 0xDC00, which is in the
Unicode surrogate space, and hence can't be stored in the terminal by
sending UTF-8 at all (since our UTF-8 decoder rejects that range, as
per spec). That's where we were already keeping other magic blocks
like CSET_LINEDRW and CSET_SCOACS, and there's still room for two
more.
The net effect should be that in Windows PuTTY, U+F000 to U+F1FF are
now displayed as whatever your font wants to show.
Similarly to other recent changes, the frontend now proactively keeps
Terminal up to date with the current position and size of the terminal
window, so that escape-sequence queries can be answered immediately
from the Terminal's own internal data structures without needing a
call back to the frontend.
Mostly this has let me remove explicit window-system API calls that
retrieve the window position and size, in favour of having the front
ends listen for WM_MOVE / WM_SIZE / ConfigureNotify events and track
the position and size that way. One exception is that the window pixel
size is still requested by Seat via a callback, to put in the
wire-encoded termios settings. That won't be happening very much, so
I'm leaving it this way round for the moment.
Now terminal.c makes nearly all the decisions about what the colour
palette should actually contain: it does the job of reading the
GUI-configurable colours out of Conf, and also the job of making up
the rest of the xterm-256 palette. The only exception is that TermWin
can provide a method to override some of the default colours, which on
Windows is used to implement the 'Use system colours' config option.
This saves code overall, partly because the front ends don't have to
be able to send palette data back to the Terminal any more (the
Terminal keeps the master copy and can answer palette-query escape
sequences from its own knowledge), and also because now there's only
one copy of the xterm-256 palette setup code (previously gtkwin.c and
window.c each had their own version of it).
In this rewrite, I've also introduced a multi-layered storage system
for the palette data in Terminal. One layer contains the palette
information derived from Conf; the next contains platform overrides
(currently just Windows's 'Use system colours'); the last one contains
overrides set by escape sequences in the middle of the session. The
topmost two layers can each _conditionally_ override the ones below.
As a result, if a server-side application manually resets (say) the
default fg and bg colours in mid-session to something that works well
in a particular application, those changes won't be wiped out by a
change in the Windows system colours or the Conf, which they would
have been before. Instead, changes in Conf or the system colours alter
the lower layers of the structure, but then when palette_rebuild is
called, the upper layer continues to override them, until a palette
reset (ESC]R) or terminal reset (e.g. ESC c) removes those upper-layer
changes. This seems like a more consistent strategy, in that the same
set of configuration settings will produce the same end result
regardless of what order they were applied in.
The palette-related methods in TermWin have had a total rework.
palette_get and palette_reset are both gone; palette_set can now set a
contiguous range of colours in one go; and the new
palette_get_overrides replaces window.c's old systopalette().
There are three separate indexing schemes in use by various bits of
the PuTTY front ends, and _none_ of them was clearly documented, let
alone all in the same place. Worse, functions that looked obviously
related, like win_palette_set and win_palette_get, used different
encodings.
Now all the encodings are defined together in putty.h, with
explanation of why there are three in the first place and clear
documentation of where each one is used; terminal.c provides mapping
tables that convert between them; the terminology is consistent
throughout; and win_palette_set has been converted to use the sensible
encoding.
Again, I've replaced it with a push-based notification going in the
other direction, so that when the terminal output stream includes a
query for 'is the window minimised?', the Terminal doesn't have to
consult the TermWin, because it already knows the answer.
The GTK API I'm using here (getting a GdkEventWindowState via
GtkWidget's window-state-event) is not present in GTK 1. The API I was
previously using (gdk_window_is_viewable) _is_, but it turns out that
that API doesn't reliably give the right answer: it only checks
visibility of GDK window ancestors, not X window ancestors. So in fact
GTK 1 PuTTY/pterm was only ever _pretending_ to reliably support the
'am I minimised' terminal query. Now it won't pretend any more.
Previously, window title management happened in a bipartisan sort of
way: front ends would choose their initial window title once they knew
what host name they were connecting to, but then Terminal would
override that later if the server set the window title by escape
sequences.
Now it's all done the same way round: the Terminal object is always
where titles are invented, and they only propagate in one direction,
from the Terminal to the TermWin.
This allows us to avoid duplicating in multiple front ends the logic
for what the initial window title should be. The frontend just has to
make one initial call to term_setup_window_titles, to tell the
terminal what hostname should go in the default title (if the Conf
doesn't override even that). Thereafter, all it has to do is respond
to the TermWin title-setting methods.
Similarly, the logic that handles window-title changes as a result of
the Change Settings dialog is also centralised into terminal.c. This
involved introducing an extra term_pre_reconfig() call that each
frontend can call to modify the Conf that will be used for the GUI
configurer; that's where the code now lives that copies the current
window title into there. (This also means that GTK PuTTY now behaves
consistently with Windows PuTTY on that point; GTK's previous
behaviour was less well thought out.)
It also means there's no longer any need for Terminal to talk to the
front end when a remote query wants to _find out_ the window title:
the Terminal knows the answer already. So TermWin's get_title method
can go.
All implementations of it work by checking the line_codepage field in
the ucsdata structure that the terminal itself already has a pointer
to. Therefore, it's a totally unnecessary query function: the terminal
can check the same thing directly by inspecting that structure!
(In fact, it already _does_ do that, for the purpose of actually
deciding how to decode terminal output data. It only uses this query
function at all for the auxiliary purpose of inventing useful tty
modes to pass to the backend.)
The idea of the especially large RNG that we use in key generation is
that it should have as much actual entropy as possible. The reason I
based it on SHA-512 previously was that that was the hash function in
our collection with the largest output. But that's no longer true!
Among the SHA-3 family that I added for Ed448 purposes, we have a
ready-made variant of SHAKE-256 that outputs a whopping 114 bytes of
hash. I see no reason not to upgrade to that from SHA-512's 64 bytes.
(I could probably extend it even further by manually making another
SHA-3 variant specially for the purpose, but I don't know that it
would be worth it. This is a one-line change which I think is already
a positive step.)
Rather like some of the tricks I did in mpint.h, this replaces the
unparametrised function random_setup_special() with one called
random_setup_custom() taking a hash-algorithm parameter.
The old syntax random_setup_special() still exists, and is a macro
wrapper on random_setup_custom() that passes ssh_sha512 as an
argument. This means I can keep the choice of hash function consistent
between the key generation front ends.
This adds potential flexibility: now, anyone wanting a different kind
of special RNG can make it out of whatever primitive they like. But a
more immediate point is to remove an inter-module dependency:
sshrand.c now doesn't need to be linked against the SHA-512 code.
This is mostly easy: it's just like drawing an underline, except that
you put it at a different height in the character cell. The only
question is _where_ in the character cell.
Pango, and Windows GetOutlineTextMetrics, will tell you exactly where
the font wants to have it. Following xterm, I fall back to 3/8 of the
font's ascent (above the baseline) if either of those is unavailable.
Two minor memory-leak fixes on 0.74 seem not to be needed on master:
the fix in an early exit path of pageant_add_keyfile is done already
on master in a different way, and the missing sfree(fdlist) in
uxsftp.c is in code that's been completely rewritten in the uxcliloop
refactoring.
Other minor conflicts: the rework in commit b52641644905 of
ssh1login.c collided with the change from FLAG_VERBOSE to
seat_verbose(), and master and 0.74 each added an unrelated extra
field to the end of struct SshServerConfig.
This mitigates CVE-2020-14002: if you're in the habit of clicking OK
to unknown host keys (the TOFU policy - trust on first use), then an
active attacker looking to exploit that policy to substitute their own
host key in your first connection to a server can use the host key
algorithm order in your KEXINIT to (not wholly reliably) detect
whether you have a key already stored for this host, and if so, abort
their attack to avoid giving themself away.
However, for users who _don't_ use the TOFU policy and instead check
new host keys out of band, the dynamic policy is more useful. So it's
provided as a configurable option.
Now, instead of a 'const char *' in the static data segment, error
messages returned from backend setup are dynamically allocated and
freed by the caller.
This will allow me to make the messages much more specific (including
errno values and the like). However, this commit is pure refactoring:
I've _just_ changed the allocation policy, and left all the messages
alone.
In commit 1f399bec58 I had the idea of generating the protocol radio
buttons in the GUI configurer by looping over the backends[] array,
which gets the reliably correct list of available backends for a given
binary rather than having to second-guess. That's given me an idea: we
can do the same for the per-backend config panels too.
Now the GUI config panel for every backend is guarded by a check of
backend_vt_from_proto, and we won't display the config for that
backend unless it's present.
In particular, this allows me to move the serial-port configuration
back into config.c from the separate file sercfg.c: we find out
whether to apply it by querying backend_vt_from_proto(PROT_SERIAL),
the same as any other backend.
In _particular_ particular, that also makes it much easier for me to
move the serial config up the pecking order, so that it's now second
only to SSH in the list of per-protocol config panes, which I think is
now where it deserves to be.
(A side effect of that is that I now have to come up with a different
method of having each serial backend specify the subset of parity and
flow control schemes it supports. I've done it by adding an extra pair
of serial-port specific bitmask fields to BackendVtable, taking
advantage of the new vtable definition idiom to avoid having to
boringly declare them as zero in all the other backends.)
The motivation is for the SUPDUP protocol. The server may send a
signal for the terminal to reset any input buffers. After this, the
server will not know the state of the terminal, so it is required to
send its cursor position back.
This is standardised by RFC 8709 at SHOULD level, and for us it's not
too difficult (because we use general-purpose elliptic-curve code). So
let's be up to date for a change, and add it.
This implementation uses all the formats defined in the RFC. But we
also have to choose a wire format for the public+private key blob sent
to an agent, and since the OpenSSH agent protocol is the de facto
standard but not (yet?) handled by the IETF, OpenSSH themselves get to
say what the format for a key should or shouldn't be. So if they don't
support a particular key method, what do you do?
I checked with them, and they agreed that there's an obviously right
format for Ed448 keys, which is to do them exactly like Ed25519 except
that you have a 57-byte string everywhere Ed25519 had a 32-byte
string. So I've done that.
This is the same protocol that PuTTY's connection sharing has been
using for years, to communicate between the downstream and upstream
PuTTYs. I'm now promoting it to be a first-class member of the
protocols list: if you have a server for it, you can select it in the
GUI or on the command line, and write out a saved session that
specifies it.
This would be completely insecure if you used it as an ordinary
network protocol, of course. Not only is it non-cryptographic and wide
open to eavesdropping and hijacking, but it's not even _authenticated_
- it begins after the userauth phase of SSH. So there isn't even the
mild security theatre of entering an easy-to-eavesdrop password, as
there is with, say, Telnet.
However, that's not what I want to use it for. My aim is to use it for
various specialist and niche purposes, all of which involve speaking
it over an 8-bit-clean data channel that is already set up, secured
and authenticated by other methods. There are lots of examples of such
channels:
- a userv(1) invocation
- the console of a UML kernel
- the stdio channels into other kinds of container, such as Docker
- the 'adb shell' channel (although it seems quite hard to run a
custom binary at the far end of that)
- a pair of pipes between PuTTY and a Cygwin helper process
- and so on.
So this protocol is intended as a convenient way to get a client at
one end of any those to run a shell session at the other end. Unlike
other approaches, it will give you all the SSH-flavoured amenities
you're already used to, like forwarding your SSH agent into the
container, or forwarding selected network ports in or out of it, or
letting it open a window on your X server, or doing SCP/SFTP style
file transfer.
Of course another way to get all those amenities would be to run an
ordinary SSH server over the same channel - but this approach avoids
having to manage a phony password or authentication key, or taking up
your CPU time with pointless crypto.
The previous 'name' field was awkwardly serving both purposes: it was
a machine-readable identifier for the backend used in the saved
session format, and it was also used in error messages when Plink
wanted to complain that it didn't support a particular backend. Now
there are two separate name fields for those purposes.
In commit b4c8fd9d8 which introduced the Seat trait, I got a bit
confused about the prototype of new_prompts(). Previously it took a
'Frontend *' parameter; I edited the call sites to pass a 'Seat *'
instead, but the actual function definition takes no parameters at all
- and rightly so, because the 'Frontend *' inside the prompts_t has
been removed and _not_ replaced with a 'Seat *', so the constructor
would have nothing to do with such a thing anyway.
But I wrote the function declaration in putty.h with '()' rather than
'(void)' (too much time spent in C++), and so the compiler never
spotted the mismatch.
Now new_prompts() is consistently nullary everywhere it appears: the
prototype in the header is a proper (void) one, and the call sites
have been modified to not pointlessly give it a Seat or null pointer.
(cherry picked from commit d183484742)
I've added the gcc-style attribute("printf") to a lot of printf-shaped
functions in this code base that didn't have it. To make that easier,
I moved the wrapping macro into defs.h, and also enabled it if we
detect the __clang__ macro as well as __GNU__ (hence, it will be used
when building for Windows using clang-cl).
The result is that a great many format strings in the code are now
checked by the compiler, where they were previously not. This causes
build failures, which I'll fix in the next commit.
(cherry picked from commit cbfba7a0e9)
UBsan pointed out another memcpy from NULL (again with length 0) in
the prompts_t system. When I looked at it, I realised that firstly
prompt_ensure_result_size was an early not-so-good implementation of
sgrowarray_nm that would benefit from being replaced with a call to
the real one, and secondly, the whole system for storing prompt
results should really have been replaced with strbufs with the no-move
option, because that's doing all the same jobs better.
So, now each prompt_t holds a strbuf in place of its previous manually
managed string. prompt_ensure_result_size is gone (the console
prompt-reading functions use strbuf_append, and everything else just
adds to the strbuf in the usual marshal.c way). New functions exist to
retrieve a prompt_t's result, either by reference or copied.
(cherry picked from commit cd6bc14f04)
These global variables are only ever used by load_settings, which uses
them to vary the default protocol and port number in the absence of
any specification elsewhere. So there's no real need for them to be
universally accessible via the awkward GLOBAL mechanism: they can be
statics inside settings.c, with accessor functions that can set them.
That was the last GLOBAL in putty.h, so I've removed the definition of
the macro GLOBAL itself as well. There are still some GLOBALs in the
Windows subdirectory, though.
I haven't managed to make this one _not_ be a mutable variable, but at
least it's not global across all tools any more: it lives in cmdline.c
along with the code that decides what to set it to, and cmdline.c
exports a query method to ask for its value.
This mutable global was never actually used at all, _even_ in history!
Commit 1a03fa929 introduced it, as part of the jump list support, but
even that commit never _read_ from the variable - it only assigned to
it. I have to guess that it was part of an earlier draft of the jump
lists patch and ended up orphaned in the final version.
Another ugly mutable global variable gone: now, instead of this
variable being defined in cmdline.c and written to by everyone's
main(), it's defined _alongside_ everyone's main() as a constant, and
cmdline.c just refers to it.
A bonus is that now nocmdline.c doesn't have to define it anyway for
tools that don't use cmdline.c. But mostly, it didn't need to be
mutable, so better for it not to be.
While I'm at it, I've also fiddled with the bit flags that go in it,
to define their values automatically using a list macro instead of
manually specifying each one to be a different power of 2.
