If this occurs before cmdline_run_saved, then the latter will use its
saved pointers to arguments in the freed CmdlineArgList.
Affects uses of PuTTY without a saved session (like 'putty -ssh
foohost'), and a very small number of pterm options, in particular
-sessionlog.
This is the simplest possible fix: just remove the free completely,
so that the parsed command-line arguments leak. There's at most one
instance of them per process, so it doesn't matter.
I had allocated a string, advanced a pointer along it, and then freed
that pointer instead of the pointer to the start of the string.
I'd already applied the correct fix in tolocal() in commit
841bf321d4, but it needed applying in get_dir_list() too.
This reverts the change to is_interactive() by commit 80aed96286,
which switched it to using the new conio system. Now we're back to
doing it the same way as we used to: we check if stdin is a console.
The only use of is_interactive() on Windows is deciding whether to
present the console antispoof prompt. (On Unix it has an additional
use in cmdgen, for deciding whether to emit progress reports, but on
Windows that doesn't come up).
On Unix this is based on stdin being a tty, which means that a
command such as "plink host do stuff </dev/null" omits the antispoof
prompt. That's deliberate: the prompt is to defend against attacks
where the user sends interactive input to the SSH session channel
believing it to be directed at userauth, but if the input _isn't_
coming from the interactive terminal where the user is answering
userauth prompts, then they can't do that even if they are fooled.
On Windows, I think the same argument applies, now that we're reading
userauth prompts from the console in the same way as Unix. So
is_interactive() now does the analogous thing on Windows.
Conveniently, this _also_ means is_interactive() is back to exactly
how it was before the conio rewrite, which means it's one fewer thing
that can unexpectedly change and break someone's workflow. (Otherwise
I might also have wanted to change its behaviour based on
-legacy-stdio-handling, which would be extra ugly.)
In 0.81, some prompts (such as host-key prompts) went to stderr, while
others (such as username and password prompts) went to stdout.
With -legacy-stdio-prompts (or if we otherwise couldn't get console
handles), we were sending all such prompts to stdout, which might have
messed up someone's workflow if they were interacting with the
non-username/password prompts programmatically.
The fix in commit 31ab5b8e30 was incomplete. It works when you
maximise the window by pressing the maximise button in the title bar,
but not when you drag the window to the very top of the screen. In the
latter case, the resize at the instant of maximisation works right,
but then we get a WM_EXITSIZEMOVE when the drag is released,
triggering one final resize which ignored the window border again.
That was because wm_size_resize_term() was being given a flag on
purpose telling it to ignore the border: apparently at some point in
the past, ignoring the border for even a normally maximised
window (not even full-screen) was done on purpose. But for the reasons
in the previous commit, I'm changing that.
Just as with other recent changes like usernames, this allows the
remote command line to include characters outside the system code
page, encoding as UTF-8 on the wire (as the SSH protocol has wanted
all along).
When term_input_data_from_charset receives data in a specified
character set (that isn't a negative number indicating "just send
binary"), it was translating from that character set into wide-
character Unicode, but then forgetting to translate back again into
the terminal's configured character set.
Introduced by the rewrite in commit 4f756d2a4d. Affected the
answerback string sent in response to ^E, and I think potentially some
paths through term_keyinput too, although I haven't quite worked out
which ones would have been affected.
The UCS-2 translation of the access-mode string ("r", "wb" etc) was
not being freed, because the free call appeared after an unconditional
return statement.
Spotted by Coverity, although now I wonder why an ordinary compiler
warning hadn't long since pointed this one out!
They weren't being enacted if you ran psftp without a hostname and
later issued an 'open' command, because in that code path,
cmdline_run_saved() was never called. Without SAVEABLE, the options
are processed immediately instead of being deferred for later.
Also, it's pointless. The purpose of marking command-line options as
SAVEABLE is so that their processing can be correctly ordered with
respect to loading a saved session, so that they can reliably override
settings that the saved session might have defined another way. (E.g.
"plink -A sessionname": processing those options in strict
left-to-right order, the saved session's opinion about agent
forwarding would be used, regardless of the -A option.) So anything
that's not stored in Conf (and hence the saved session) at all doesn't
have any reason to be SAVEABLE.
I think I must have put the SAVEABLE in by thoughtless clone-and-hack.
Typing just 'pscp' for help doesn't work since ecfa6b2734 (and is a
particularly infuriating suggestion if that's what you did to elicit
this message).
When Windows console prompting moved away from stdin/stderr by default
(80aed96286), 0.78 was the most recent release; but we've made several
more releases off branches since then.
draft-ietf-sshm-ntruprime-ssh-00 asserts that it's identical to the
@openssh.com version we already implement:
'[sntrup761x25519-sha512@openssh.com] became the default key exchange
algorithm in OpenSSH during 2022. That is identical to the
"sntrup761x25519-sha512" mechanism described in this document.'
Literally speaking, it's not true that PuTTY only connects to the
server you told it to. It typically has to connect to a DNS server
first to find out where that server _is_. (If you've provided a
hostname, and if that hostname isn't in /etc/hosts or equivalent.)
Of course, if you're concerned about people _in your organisation's
network_ finding out where you've been connecting to, you have bigger
problems, because whether you did a DNS lookup or not they can
certainly see your IP-layer headers. But that really is outside the
scope of this document. I only mention DNS out of pedantry, because
not doing so made "does not connect to any other site" technically
inaccurate. (Perhaps even: only inaccurate if the DNS lookup happens
over TCP :-)
Re-reading the wording, I think I was a bit cavalier about "if you
don't like the host key cache recording where you've been, check host
keys yourself." It should be more like "check host keys yourself,
SERIOUSLY, WE REALLY MEAN IT, DO NOT LEAVE THIS STEP OUT."
In the original HTML-only version of the privacy document, there were
two major sections at <h2> level, "stuff stored locally" and "stuff
sent over the network", each with subsections at <h3> level describing
individual aspects. But somehow when I translated it into Halibut to
put it into the manual, they all became \H and the nesting was lost.
If we're setting ssh->s to NULL, we ought to free the thing it
previously pointed to (having extracted the error message first).
At the very least this is a memory leak.
But in fact it's worse, because not freeing it also means not
cancelling its toplevel callbacks. And if you don't do that, then a
failure to set up an SSH connection proxied over another SSH
connection will generate two error dialog boxes in succession, the
second one from the callback that should have been cancelled here.
On Windows that callback never gets called, because we exit the whole
process before getting into the main message loop which might run the
callback. But on Unix, we do go to the main message loop (we don't
have a separate one for the error box), which causes an assertion
failure in register_dialog() when the second box finds the
DIALOG_SLOT_CONNECTION_FATAL slot already occupied.
During startup, calculate the top/left position of the window so that
the whole window is visible. Without this change, sometimes the window
is clipped vertically and you can't see the bottom.
Thanks to Bruno Kraychete da Costa for the original version of this
patch; I've tweaked it slightly, but the basic strategy of adjusting
the output of GetWindowRect against the monitor's working area is all
his.
Debian #1084583 points out that Python 3.13 is going to get rid of the
'pipes' module completely. shlex.quote has been available as a
replacement for ages.
(Not that Debian actually cares, since they don't re-run our wobbly
edifice of MSI build bodges! But thanks to some bug reporter for
pointing it out anyway.)
Jacob reminds me that that's another piece of saved data, less obvious
than the ones I'd already documented.
I'd actually forgotten myself _exactly_ which actions cause data to be
added to the jump list. But it's easy to check. The only functions
exported from jump-list.c are add_session_to_jumplist(),
remove_session_from_jumplist() and clear_jumplist(); the only one of
those that _records_ information (rather than removing it) is
add_session_to_jumplist(); and that is called in exactly one place,
namely at the end of load_session() in settings.c.
On most keyboard modes the escape-sequence for Alt-Fn is the same as
for Fn alone but with an additional ESC prepended.
However, this additional ESC should not be sent for keyboard modes
that sends a different escape-sequence when Alt is pressed, like e.g.
Xterm216+, as this is not expected by the server-side application.
Signed-off-by: Anders Larsen <al@alarsen.net>
During the 0.81 release process, I found out that the Windows Store
now requires applications to provide a privacy policy, so I had to
write one in order to get 0.81 into the Store.
This initially seemed like makework (especially having to do it in a
hurry as a prerequisite to get a really important security fix
distributed!). But after I started writing it, I found there was
actually quite a lot to say. It's easy to think "PuTTY doesn't phone
home to the developers, that's all, we're done". But of course it
_does_ store information on your machine (host key cache, saved
sessions, etc). And it does send information to servers on the
network (only the ones you ask it to, but even so). And it's not 100%
obvious in every case what is and isn't stored, and what a privacy-
conscious individual might be revealing about themself by doing this
or that thing.
So I think the web page I hastily put up at the time of the 0.81
release deserves to be promoted into part of the documentation. Here's
a (very lightly) copy-edited version in the form of a docs appendix.
(Once this is committed and built, I expect I'll turn the privacy web
page into a mirror of this docs appendix, in the same way as the
website FAQ and feedback pages.)
The immediate usefulness of this is in pterm.exe: when the user uses
-e to specify a command to run in the pterm, we retrieve the command
in Unicode, store it in CONF_remote_cmd as UTF-8, and then in conpty.c
we can extract it in the same form and convert it back to Unicode to
pass losslessly to CreateProcessW. So now non-ACP Unicode works in
that part of the pterm command line.
While testing the new command-line handling, I tried actually using
that option for the first time in a long time, and saw
These are the fingerprints of the PuTTY PGP Master Keys. They
can
be used to establish a trust path from this executable to another
one. See the manual for more information.
because Windows MessageBox() had done its own wrapping on the text, at
a slightly narrower width than the one implied by the hard newlines in
the input string. Removed the mid-paragraph newlines, so that
Windows's wrapping will be the only wrapping.
That's not a failure outcome. The user asked for some information; we
printed it; nothing went wrong. Mission successful, so exit(0)!
I noticed this because it was sitting right next to some of the
usage() calls modified in the previous commit. Those also had the
misfeature of exiting with failure after successfully printing the
help, possibly due to confusion arising from the way that usage() was
_sometimes_ printed on error as well. But pgp_fingerprints() has no
such excuse. That one's just silly.
In the course of debugging the command-line argument refactoring in
previous commits, I found I wasn't quite sure whether PSCP thought I'd
given it too many arguments, or too few, because it didn't print an
error message saying which: it just printed its giant usage message.
Over the last few years I've come to the belief that this is Just
Wrong anyway. Printing the whole of a giant help message should only
be done when the user asked for it: otherwise, print a short and
to-the-point error, and maybe _suggest_ how to get help, but scrolling
everything else off the user's screen is not a good response to a
typo. I wrote this thought up more fully last year:
https://www.chiark.greenend.org.uk/~sgtatham/quasiblog/stop-helping/
So, time to practise what I preach! The PuTTY tools now follow the
'Stop helping!' principle. You can get full help by saying --help.
Also, when we do print the help, we now exit(0) rather than exit(1),
because there's no reason to report failure: we successfully did what
the user asked us for.
Converting a CmdlineArg straight to a Filename allows us to make the
filename out of the wide-character version of the string on Windows.
So now filenames specified on the command line should generally be
able to handle pathnames containing Unicode characters not in the
system code page.
This change also involves making some char pointers _into_ Filename
structs where they weren't previously: for example, the
'openssh_config_file' variable in Windows Pageant's WinMain().
This is the pathfinding change that proves it's possible for _one_
Conf setting to become Unicode-capable.
That seems like quite a small reward for all the refactoring in the
previous patches this week! But changing over one configuration
setting is enough to get started with: once all the bugs are out of
this one, we can try switching over some more.
Changing the type to CONF_TYPE_STR_AMBI is enough by itself to make
the configuration dialog box write it into Conf as UTF-8, because
conf_editbox_handler automatically checks whether that possibility is
available. However, setting the same Conf entry from the command line
isn't automatic: I had to add code in the handler for the -l
command-line option in cmdline.c.
This commit also doesn't yet handle the _other_ way to specify a
username on the command line: including it as part of the hostname
argument via "putty user@host" or similar. That's more difficult,
because it also requires deciding what to do about UTF-8 in the actual
hostname.
(That looks as if it ought to be possible: Windows should be able to
handle looking up Unicode hostnames if you use GetAddrInfoW() in place
of getaddrinfo(). But plumbing it through everything in between
cmdline.c and windows/network.c is a bigger job than I'm prepared to
do in this proof-of-concept commit.)
This begins the process of enabling our Windows applications to handle
Unicode characters on their command lines which don't fit in the
system code page.
Instead of passing plain strings to cmdline_process_param, we now pass
a partially opaque and platform-specific thing called a CmdlineArg.
This has a method that extracts the argument word as a default-encoded
string, and another one that tries to extract it as UTF-8 (though it
may fail if the UTF-8 isn't available).
On Windows, the command line is now constructed by calling
split_into_argv_w on the Unicode command line returned by
GetCommandLineW(), and the UTF-8 method returns text converted
directly from that wide-character form, not going via the system code
page. So it _can_ include UTF-8 characters that wouldn't have
round-tripped via CP_ACP.
This commit introduces the abstraction and switches over the
cross-platform and Windows argv-handling code to use it, with minimal
functional change. Nothing yet tries to call cmdline_arg_get_utf8().
I say 'cross-platform and Windows' because on the Unix side there's
still a lot of use of plain old argv which I haven't converted. That
would be a much larger project, and isn't currently needed: the
_current_ aim of this abstraction is to get the right things to happen
relating to Unicode on Windows, so for code that doesn't run on
Windows anyway, it's not adding value. (Also there's a tension with
GTK, which wants to talk to standard argv and extract arguments _it_
knows about, so at the very least we'd have to let it munge argv
before importing it into this new system.)
Even though I wrote them, I keep forgetting their semantics. In
particular I can never quite remember off the top of my head whether
they modify their input command line, or allocate it fresh. To make
that clearer, I've made it a const parameter.
(That means the output argstart pointers have the same awkward const
-> mutable conversion as strchr - but then, strchr is itself precedent
for that being the usual way to not-quite-handle that annoyance in C!)
Now you can use an ordinary edit box in the config setup to talk to a
string-valued Conf setting typed as any of CONF_TYPE_STR,
CONF_TYPE_UTF8 or CONF_TYPE_STR_AMBI.
There's no difficulty with implementing these, on either platform.
Windows has native Unicode support for its edit boxes: we can set and
retrieve the text as a wide-character string, and then converting it
to and from UTF-8 is easy. And GTK has specified its edit boxes as
being UTF-8 all along, no matter what the system locale.
This begins the process of making PuTTY more able to handle Unicode
strings as a first-class type in its configuration. One of the new
types, CONF_TYPE_UTF8, looks physically just like CONF_TYPE_STR but
the semantics are that it's definitely encoded in UTF-8, instead of
'shrug, whatever the system locale's encoding is'.
Unfortunately, we can't yet switch over any Conf items to having that
type, because our data representations in saved configuration (both on
Unix and Windows) store char strings in the system encoding. So we'll
have to change that representation at the same time, which risks
breaking backwards compatibility with old PuTTYs reading the same
configuration.
So the other new type, CONF_TYPE_STR_AMBI, is intended as a
transitional form, recording a configuration setting that _might_ be
explicitly UTF-8 or might have the legacy 'shrug, whatever' semantics,
depending on where we got it from.
My general migration plan is that first I _enable_ Unicode support in
a Conf item, by turning it into STR_AMBI; the Unicode version of the
string (if any) is saved in a new location, and a best-effort
local-charset version is saved where it's always been. That way new
PuTTY can read the Unicode version, and old PuTTY reading that
configuration will behave no worse than it would have done already.
It would be nice to think that in the far future we've migrated
everything to STR_AMBI and can move them all to mandatory UTF-8,
obsoleting the old configuration. I think it's more likely we'll never
get there. But at least _new_ Conf items, with no backwards
compatibility requirement in the first place, can be CONF_TYPE_UTF8
where appropriate.
(In conf_get_str_ambi(), I considered making it mandatory via assert()
to pass the 'utf8' output pointer as non-NULL, to defend against lazy
adaptation of existing code by just changing the function call. But in
fact I think there's a legitimate use case for not caring if the
output is UTF-8 or not, because some of the existing SSH code
currently just shoves strings like usernames directly on to the wire
whether they're in the right encoding or not; so if you want to do the
correct UTF-8 thing where possible and preserve legacy behaviour if
not, then treating both classes of string the same _is_ the right
thing to do.)
This also requires linking the Unicode support into many Unix
applications that hadn't previously needed it.
The previous mb_to_wc and wc_to_mb had horrible and also buggy APIs.
This commit introduces a fresh pair of functions to replace them,
which generate output by writing to a BinarySink. So it's now up to
the caller to decide whether it wants the output written to a
fixed-size buffer with overflow checking (via buffer_sink), or
dynamically allocated, or even written directly to some other output
channel.
Nothing uses the new functions yet. I plan to migrate things over in
upcoming commits.
What was wrong with the old APIs: they had that awkward undocumented
Windows-specific 'flags' parameter that I described in the previous
commit and took out of the dup_X_to_Y wrappers. But much worse, the
semantics for buffer overflow were not just undocumented but actually
inconsistent. dup_wc_to_mb() in utils assumed that the underlying
wc_to_mb would fill the buffer nearly full and return the size of data
it wrote. In fact, this was untrue in the case where wc_to_mb called
WideCharToMultiByte: that returns straight-up failure, setting the
Windows error code to ERROR_INSUFFICIENT_BUFFER. It _does_ partially
fill the output buffer, but doesn't tell you how much it wrote!
What's wrong with the new API: it's a bit awkward to write a sequence
of wchar_t in native byte order to a byte-oriented BinarySink, so
people using put_mb_to_wc directly have to do some annoying pointer
casting. But I think that's less horrible than the previous APIs.
Another change: in the new API for wc_to_mb, defchr can be "", but not
NULL.
This #if completely replaced the actually useful version of clipme(),
so I think it must have been used early in development before the
useful version was even written.
Since then it's bit-rotted to the point where it doesn't even make
sense: the "#endif" is nested inside a while loop that the "#if 0" and
"#else" are outside, so that if the condition were changed to evaluate
true, you'd get syntactic nonsense out of the preprocessor.
And I can't see any use for it now, even if it did compile! Get rid of
it.
This parameter was undocumented, and Windows-specific: its semantics
date from before PuTTY was cross-platform, and are "Pass this flags
parameter straight through to the Win32 API's conversion functions".
So in Windows platform code you can pass flags like MB_USEGLYPHCHARS,
but in cross-platform code, you dare not pass anything nonzero at all
because the Unix frontend won't recognise it (or, likely, even
compile).
I've kept the flag for now in the underlying mb_to_wc / wc_to_mb
functions. Partly that's because there's one place in the Windows code
where the parameter _is_ used; mostly, it's because I'm about to
replace those functions anyway, so there's no point in editing all the
call sites twice.
term_do_paste and term_input_data_from_* were still taking int, which
should be size_t. Not that I expect those functions to get >2^31 bytes
of input in one go, but I noticed it while about to modify the same
functions for another reason.
When dumping out the contents of a Conf, it's useful not to have to
guess what the integer indices mean.
By putting these identifiers in a separate array in its own library
module, I should avoid them getting linked in to production binaries
to take up space, as long as conf_id() is only called from inside
debug() statements. And to enforce _that_, it isn't even declared in a
header file unless you #define DEBUG.
The code in the 'if (IsZoomed)' statement in reset_window() was
failing to take account of the user-configured gap between the text
and the window edge, so that the requested border was lost. Now it
does take that into account.
In this commit, this change of behaviour applies to both a normally
maximised window (with the window frame still visible round the edge)
and to a full-screen window (nothing visible on the whole monitor
except PuTTY).
I'm not 100% sure whether that's the right behaviour: perhaps the
purpose of this configurable border is to space the text away from the
window furniture, so that there's no need for it if there isn't any
furniture? But on the other hand, one thing _I_ use this border for is
to make space round the edge of a terminal window for the green border
Zoom superimposes when sharing the window. And that's a use case that
would still make sense when the window is full-screened.
This is a small refinement of my own to Marco Ricci's new mode
introduced by the previous commit. If Pageant is being run by a parent
process intending to make requests to it, then it's probably put a
pipe on Pageant's stdout, and will be reading from that pipe to
retrieve the environment setup commands. So it needs to know when it's
read enough.
Closing stdout immediately makes this as easy as possible, freeing the
parent process of the need to count lines of output (and also know how
many lines to expect): it can simply read until there's no more data.
This also means there's no need to make stdout line-buffered, of
course – the fclose will flush it anyway.
