The return value of term_data() is used as the return value from the
GUI-terminal versions of the Seat output method, which means backends
will take it to be the amount of standard-output data currently
buffered, and exert back-pressure on the remote peer if it gets too
big (e.g. by ceasing to extend the window in that particular SSH-2
channel).
Historically, as a comment in term_data() explained, we always just
returned 0 from that function, on the basis that we were processing
all the terminal data through our terminal emulation code immediately,
and never retained any of it in the buffer at all. If the terminal
emulation code were to start running slowly, then it would slow down
the _whole_ PuTTY system, due to single-threadedness, and
back-pressure of a sort would be exerted on the remote by it simply
failing to get round to reading from the network socket. But by the
time we got back to the top level of term_data(), we'd have finished
reading all the data we had, so it was still appropriate to return 0.
That comment is still correct if you're thinking about the limiting
factor on terminal data processing being the CPU usage in term_out().
But now that's no longer the whole story, because sometimes we leave
data in term->inbuf without having processed it: during drag-selects
in the terminal window, and (just introduced) while waiting for the
response to a pending window resize request. For both those reasons,
we _don't_ always have a buffer size of zero when we return from
term_data().
So now that hole in our buffer size management is filled in:
term_data() returns the true size of the remaining unprocessed
terminal output, so that back-pressure will be exerted if the terminal
is currently not consuming it. And when processing resumes and we
start to clear our backlog, we call backend_unthrottle to let the
backend know it can relax the back-pressure if necessary.
I just spotted this in passing, and it's out of date! The prompts_t
system no longer works by consuming user input that had already been
handed to a backend, and it now has a callback that it can use to
proactively notify a backend (or other Interactor) that its prompts
have been answered. So if we did ever want to use a separate GUI
dialog box for prompts (as the comment suggested), then we do now have
the means.
(TL;DR: to suppress redundant 'Press Return to begin session' prompts
in between hops of a jump-host configuration, in Plink.)
This new query method directly asks the Seat the question: is the same
stream of input used to provide responses to interactive login
prompts, and the session input provided after login concludes?
It's used to suppress the last-ditch anti-spoofing defence in Plink of
interactively asking 'Access granted. Press Return to begin session',
on the basis that any such spoofing attack works by confusing the user
about what's a legit login prompt before the session begins and what's
sent by the server after the main session begins - so if those two
things take input from different places, the user can't be confused.
This doesn't change the existing behaviour of Plink, which was already
suppressing the antispoof prompt in cases where its standard input was
redirected from something other than a terminal. But previously it was
doing it within the can_set_trust_status() seat query, and I've now
moved it out into a separate query function.
The reason why these need to be separate is for SshProxy, which needs
to give an unusual combination of answers when run inside Plink. For
can_set_trust_status(), it needs to return whatever the parent Seat
returns, so that all the login prompts for a string of proxy
connections in session will be antispoofed the same way. But you only
want that final 'Access granted' prompt to happen _once_, after all
the proxy connection setup phases are done, because up until then
you're still in the safe hands of PuTTY itself presenting an unbroken
sequence of legit login prompts (even if they come from a succession
of different servers). Hence, SshProxy unconditionally returns 'no' to
the query of whether it has a single mixed input stream, because
indeed, it never does - for purposes of session input it behaves like
an always-redirected Plink, no matter what kind of real Seat it ends
up sending its pre-session login prompts to.
Finally, the payoff from all of this refactoring: now, when a proxy
prompts interactively during connection setup, you get a message in
advance telling you which Interactor is originating the following
messages.
To achieve this, I've arranged to link Interactors together into a
list, so that any Interactor created by a proxy has a 'parent' pointer
pointing to the Interactor its client passed to new_connection().
This allows interactor_announce() to follow the links back up the
chain and count them, so that it knows whether it's a primary
connection, or a proxy, or a proxy-for-a-proxy, or more generally an
nth-order proxy, and can include that in its announcement.
And secondly, once interactor_announce() reaches the top of the chain,
it can use that as a storage location agreed on by all Interactors in
the whole setup, to tell each other which one of them was the last to
do anything interactive. Then, whenever there's a change of
Interactor, a message can be printed to indicate it to the user; and
when the same Interactor does multiple things in succession, you don't
get a slew of pointless messages in between them all.
Previously, SshProxy dealt with creating a TempSeat to wrap the one it
was borrowing from its client, and then each client in turn dealt with
detecting when it had had its seat borrowed and finishing up with the
TempSeat. The latter involved a lot of code duplication; the former
didn't involve code duplication _yet_ (since SshProxy was the only
thing doing this job), but would have once we started wanting to do
interactive password prompting for other types of network proxy.
Now all of that functionality is centralised into two new Interactor
helper functions: interactor_borrow_seat and interactor_return_seat.
All this Interactor business has been gradually working towards being
able to inform the user _which_ network connection is currently
presenting them with a password prompt (or whatever), in situations
where more than one of them might be, such as an SSH connection being
used as a proxy for another SSH connection when neither one has
one-touch login configured.
At some point, we have to arrange that any attempt to do a user
interaction during connection setup - be it a password prompt, a host
key confirmation dialog, or just displaying an SSH login banner -
makes it clear which host it's come from. That's going to mean calling
some kind of announcement function before doing any of those things.
But there are several of those functions in the Seat API, and calls to
them are scattered far and wide across the SSH backend. (And not even
just there - the Rlogin backend also uses seat_get_userpass_input).
How can we possibly make sure we don't forget a vital call site on
some obscure little-tested code path, and leave the user confused in
just that one case which nobody might notice for years?
Today I thought of a trick to solve that problem. We can use the C
type system to enforce it for us!
The plan is: we invent a new struct type which contains nothing but a
'Seat *'. Then, for every Seat method which does a thing that ought to
be clearly identified as relating to a particular Interactor, we
adjust the API for that function to take the new struct type where it
previously took a plain 'Seat *'. Or rather - doing less violence to
the existing code - we only need to adjust the API of the dispatch
functions inline in putty.h.
How does that help? Because the way you _get_ one of these
struct-wrapped Seat pointers is by calling interactor_announce() on
your Interactor, which will in turn call interactor_get_seat(), and
wrap the returned pointer into one of these structs.
The effect is that whenever the SSH (or Rlogin) code wants to call one
of those particular Seat methods, it _has_ to call
interactor_announce() just beforehand, which (once I finish all of
this) will make sure the user is aware of who is presenting the prompt
or banner or whatever. And you can't forget to call it, because if you
don't call it, then you just don't have a struct of the right type to
give to the Seat method you wanted to call!
(Of course, there's nothing stopping code from _deliberately_ taking a
Seat * it already has and wrapping it into the new struct. In fact
SshProxy has to do that, in order to forward these requests up the
chain of Seats. But the point is that you can't do it _by accident_,
just by forgetting to make a vital function call - when you do that,
you _know_ you're doing it on purpose.)
No functional change: the new interactor_announce() function exists,
and the type-system trick ensures it's called in all the right places,
but it doesn't actually _do_ anything yet.
This trait will be implemented by anything that wants to display
interactive prompts or notifications to the user in the course of
setting up a network connection, _or_ anything that wants to make a
network connection whose proxy setup might in turn need to do that.
To begin with, that means every Backend that makes network connections
at all must be an Interactor, because any of those network connections
might be proxied via an SSH jump host which might need to interact
with the user.
I'll fill in the contents of this trait over the next few commits, to
keep the patches comprehensible. For the moment, I've just introduced
the trait, set up implementations of it in the five network backends,
and given it a single 'description' method.
The previous 'description' methods of Backend and Plug are now
removed, and their work is done by the new Interactor method instead.
(I changed my mind since last week about where that should best live.)
This isn't too much of an upheaval, fortunately, because I hadn't got
round yet to committing anything that used those methods!
Previously, SSH authentication banners were displayed by calling the
ordinary seat_output function, and passing it a special value in the
SeatOutputType enumeration indicating an auth banner.
The awkwardness of this was already showing a little in SshProxy's
implementation of seat_output, where it had to check for that special
value and do totally different things for SEAT_OUTPUT_AUTH_BANNER and
everything else. Further work in that area is going to make it more
and more awkward if I keep the two output systems unified.
So let's split them up. Now, Seat has separate output() and banner()
methods, which each implementation can override differently if it
wants to.
All the 'end user' Seat implementations use the centralised
implementation function nullseat_banner_to_stderr(), which turns
banner text straight back into SEAT_OUTPUT_STDERR and passes it on to
seat_output. So I didn't have to tediously implement a boring version
of this function in GTK, Windows GUI, consoles, file transfer etc.
Previously, checking the host key against the persistent cache managed
by the storage.h API was done as part of the seat_verify_ssh_host_key
method, i.e. separately by each Seat.
Now that check is done by verify_ssh_host_key(), which is a new
function in ssh/common.c that centralises all the parts of host key
checking that don't need an interactive prompt. It subsumes the
previous verify_ssh_manual_host_key() that checked against the Conf,
and it does the check against the storage API that each Seat was
previously doing separately. If it can't confirm or definitively
reject the host key by itself, _then_ it calls out to the Seat, once
an interactive prompt is definitely needed.
The main point of doing this is so that when SshProxy forwards a Seat
call from the proxy SSH connection to the primary Seat, it won't print
an announcement of which connection is involved unless it's actually
going to do something interactive. (Not that we're printing those
announcements _yet_ anyway, but this is a piece of groundwork that
works towards doing so.)
But while I'm at it, I've also taken the opportunity to clean things
up a bit by renaming functions sensibly. Previously we had three very
similarly named functions verify_ssh_manual_host_key(), SeatVtable's
'verify_ssh_host_key' method, and verify_host_key() in storage.h. Now
the Seat method is called 'confirm' rather than 'verify' (since its
job is now always to print an interactive prompt, so it looks more
like the other confirm_foo methods), and the storage.h function is
called check_stored_host_key(), which goes better with store_host_key
and avoids having too many functions with similar names. And the
'manual' function is subsumed into the new centralised code, so
there's now just *one* host key function with 'verify' in the name.
Several functions are reindented in this commit. Best viewed with
whitespace changes ignored.
These will typically be implemented by objects that are both a Backend
*and* a Plug, and the two methods will deliver the same results to any
caller, regardless of which facet of the object is known to that
caller.
Their purpose is to deliver a user-oriented natural-language
description of what network connection the object is handling, so that
it can appear in diagnostic messages.
The messages I specifically have in mind are going to appear in cases
where proxies require interactive authentication: when PuTTY prompts
interactively for a password, it will need to explain which *thing*
it's asking for the password for, and these descriptions are what it
will use to describe the thing in question.
Each backend is allowed to compose these messages however it thinks
best. In all cases at present, the description string is constructed
by the new centralised default_description() function, which takes a
host name and port number and combines them with the backend's display
name. But the SSH backend does things a bit differently, because it
uses the _logical_ host name (the one that goes with the SSH host key)
rather than the physical destination of the network connection. That
seems more appropriate when the question it's really helping the user
to answer is "What host am I supposed to be entering the password for?"
In this commit, no clients of the new methods are introduced. I have a
draft implementation of actually using it for the purpose I describe
above, but it needs polishing.
The current 'displayname' field is designed for presenting in the
config UI, so it starts with a capital letter even when it's not a
proper noun. If I want to name the backend in the middle of a
sentence, I'll need a version that starts with lowercase where
appropriate.
The old field is renamed displayname_tc, to avoid ambiguity.
This is the same as the previous FUNKY_XTERM mode if you don't press
any modifier keys, but now Shift or Ctrl or Alt with function keys
adds an extra bitmap parameter. The bitmaps are the same as the ones
used by the new SHARROW_BITMAP arrow key mode.
As well as affecting the bitmap field in the escape sequence, it was
_also_ having its otherwise standard effect of prefixing Esc to the
whole sequence. It shouldn't do both.
This commit introduces a new config option for how to handle shifted
arrow keys.
In the default mode (SHARROW_APPLICATION), we do what we've always
done: Ctrl flips the arrow keys between sending their most usual
escape sequences (ESC [ A ... ESC [ D) and sending the 'application
cursor keys' sequences (ESC O A ... ESC O D). Whichever of those modes
is currently configured, Ctrl+arrow sends the other one.
In the new mode (SHARROW_BITMAP), application cursor key mode is
unaffected by any shift keys, but the default sequences acquire two
numeric arguments. The first argument is 1 (reflecting the fact that a
shifted arrow key still notionally moves just 1 character cell); the
second is the bitmap (1 for Shift) + (2 for Alt) + (4 for Ctrl),
offset by 1. (Except that if _none_ of those modifiers is pressed,
both numeric arguments are simply omitted.)
The new bitmap mode is what current xterm generates, and also what
Windows ConPTY seems to expect. If you start an ordinary Command
Prompt and launch into WSL, those are the sequences it will generate
for shifted arrow keys; conversely, if you run a Command Prompt within
a ConPTY, then these sequences for Ctrl+arrow will have the effect you
expect in cmd.exe command-line editing (going backward or forward a
word). For that reason, I enable this mode unconditionally when
launching Windows pterm.
While fixing the previous commit I noticed that window titles don't
actually _work_ properly if you change the terminal character set,
because the text accumulated in the OSC string buffer is sent to the
TermWin as raw bytes, with no indication of what character set it
should interpret them as. You might get lucky if you happened to
choose the right charset (in particular, UTF-8 is a common default),
but if you change the charset half way through a run, then there's
certainly no way the frontend will know to interpret two window titles
sent before and after the change in two different charsets.
So, now win_set_title() and win_set_icon_title() both include a
codepage parameter along with the byte string, and it's up to them to
translate the provided window title from that encoding to whatever the
local window system expects to receive.
On Windows, that's wide-string Unicode, so we can just use the
existing dup_mb_to_wc utility function. But in GTK, it's UTF-8, so I
had to write an extra utility function to encode a wide string as
UTF-8.
The input length field is now a size_t rather than an int, on general
principles. The return value is now void (we weren't using the
previous return value at all). And we now require the client to have
previously allocated a BidiContext, which will allow allocated storage
to be reused between runs, saving a lot of churn on malloc.
(However, the current BidiContext doesn't contain anything
interesting. I could have moved the existing mallocs into it, but
there's no point, since I'm about to rewrite the whole thing anyway.)
This contains terminal.c, bidi.c (formerly minibidi.c), and
terminal.h. I'm about to make a couple more bidi-related source files,
so it seems worth starting by making a place to put them that won't be
cluttering up the top level.
It doesn't actually do anything specific to the userauth layer; it's
just a helper function that deals with the mechanics of printing an
unspoofable message on various kinds of front end, and the only
parameters it needs are a Seat and a message.
Currently, it's used for 'here is the start/end of the server banner'
only. But it's also got all the right functionality to be used for the
(still missing) messages about which proxy SSH server the next set of
login prompts are going to refer to.
The jump host system ought really to be treating SSH authentication
banners as a distinct thing from the standard-error session output, so
that the former can be presented to the user in the same way as the
auth banner for the main session.
This change converts the 'bool is_stderr' parameter of seat_output()
into an enumerated type with three values. For the moment, stderr and
banners are treated the same, but the plan is for that to change.
In a GUI app, when interactive userpass input begins, the Ldisc
acquires a reference to a prompts_t. If something bad happens to the
SSH connection (e.g. unexpected server-side closure), then all the SSH
layers will be destroyed, including freeing that prompts_t. So the
Ldisc will have a stale reference to it, which it might potentially
use.
To fix that, I've arranged a back-pointer so that prompts_t itself can
find the Ldisc's reference to it, and NULL it out on free. So now,
whichever of a prompts_t and an Ldisc is freed first, the link between
them should be cleanly broken.
(I'm not 100% sure this is absolutely necessary, in the sense of
whether a sequence of events can _actually_ happen that causes a stale
pointer dereference. But I don't want to take the chance!)
The system for handling seat_get_userpass_input has always been
structured differently between GUI PuTTY and CLI tools like Plink.
In the CLI tools, password input is read directly from the OS
terminal/console device by console_get_userpass_input; this means that
you need to ensure the same terminal input data _hasn't_ already been
consumed by the main event loop and sent on to the backend. This is
achieved by the backend_sendok() method, which tells the event loop
when the backend has finished issuing password prompts, and hence,
when it's safe to start passing standard input to backend_send().
But in the GUI tools, input generated by the terminal window has
always been sent straight to backend_send(), regardless of whether
backend_sendok() says it wants it. So the terminal-based
implementation of username and password prompts has to work by
consuming input data that had _already_ been passed to the backend -
hence, any backend that needs to do that must keep its input on a
bufchain, and pass that bufchain to seat_get_userpass_input.
It's awkward that these two totally different systems coexist in the
first place. And now that SSH proxying needs to present interactive
prompts of its own, it's clear which one should win: the CLI style is
the Right Thing. So this change reworks the GUI side of the mechanism
to be more similar: terminal data now goes into a queue in the Ldisc,
and is not sent on to the backend until the backend says it's ready
for it via backend_sendok(). So terminal-based userpass prompts can
now consume data directly from that queue during the connection setup
stage.
As a result, the 'bufchain *' parameter has vanished from all the
userpass_input functions (both the official implementations of the
Seat trait method, and term_get_userpass_input() to which some of
those implementations delegate). The only function that actually used
that bufchain, namely term_get_userpass_input(), now instead reads
from the ldisc's input queue via a couple of new Ldisc functions.
(Not _trivial_ functions, since input buffered by Ldisc can be a
mixture of raw bytes and session specials like SS_EOL! The input queue
inside Ldisc is a bufchain containing a fiddly binary encoding that
can represent an arbitrary interleaving of those things.)
This greatly simplifies the calls to seat_get_userpass_input in
backends, which now don't have to mess about with passing their own
user_input bufchain around, or toggling their want_user_input flag
back and forth to request data to put on to that bufchain.
But the flip side is that now there has to be some _other_ method for
notifying the terminal when there's more input to be consumed during
an interactive prompt, and for notifying the backend when prompt input
has finished so that it can proceed to the next stage of the protocol.
This is done by a pair of extra callbacks: when more data is put on to
Ldisc's input queue, it triggers a call to term_get_userpass_input,
and when term_get_userpass_input finishes, it calls a callback
function provided in the prompts_t.
Therefore, any use of a prompts_t which *might* be asynchronous must
fill in the latter callback when setting up the prompts_t. In SSH, the
callback is centralised into a common PPL helper function, which
reinvokes the same PPL's process_queue coroutine; in rlogin we have to
set it up ourselves.
I'm sorry for this large and sprawling patch: I tried fairly hard to
break it up into individually comprehensible sub-patches, but I just
couldn't tease out any part of it that would stand sensibly alone.
I've introduced a function ldisc_notify_sendok(), which backends
should call on their ldisc (if they have one) when anything changes
that might cause backend_sendok() to start returning true.
At the moment, the function does nothing. But in future, I'm going to
make ldisc start buffering typed-ahead input data not yet sent to the
backend, and then the effect of this function will be to trigger
flushing all that data into the backend.
Backends only have to call this function if sendok was previously
false: backends requiring no network connection stage (like pty and
serial) can safely return true from sendok, and in that case, they
don't also have to immediately call this function.
This is working towards allowing the subsidiary SSH connection in an
SshProxy to share the main user-facing Seat, so as to be able to pass
through interactive prompts.
This is more difficult than the similar change with LogPolicy, because
Seats are stateful. In particular, the trust-sigil status will need to
be controlled by the SshProxy until it's ready to pass over control to
the main SSH (or whatever) connection.
To make this work, I've introduced a thing called a TempSeat, which is
(yet) another Seat implementation. When a backend hands its Seat to
new_connection(), it does it in a way that allows new_connection() to
borrow it completely, and replace it in the main backend structure
with a TempSeat, which acts as a temporary placeholder. If the main
backend tries to do things like changing trust status or sending
output, the TempSeat will buffer them; later on, when the connection
is established, TempSeat will replay the changes into the real Seat.
So, in each backend, I've made the following changes:
- pass &foo->seat to new_connection, which may overwrite it with a
TempSeat.
- if it has done so (which we can tell via the is_tempseat() query
function), then we have to free the TempSeat and reinstate our main
Seat. The signal that we can do so is the PLUGLOG_CONNECT_SUCCESS
notification, which indicates that SshProxy has finished all its
connection setup work.
- we also have to remember to free the TempSeat if our backend is
disposed of without that having happened (e.g. because the
connection _doesn't_ succeed).
- in backends which have no local auth phase to worry about, ensure
we don't call seat_set_trust_status on the main Seat _before_ it
gets potentially replaced with a TempSeat. Moved some calls of
seat_set_trust_status to just after new_connection(), so that now
the initial trust status setup will go into the TempSeat (if
appropriate) and be buffered until that seat is relinquished.
In all other uses of new_connection, where we don't have a Seat
available at all, we just pass NULL.
This is NFC, because neither new_connection() nor any of its delegates
will _actually_ do this replacement yet. We're just setting up the
framework to enable it to do so in the next commit.
Now new_connection() takes an optional LogPolicy * argument, and
passes it on to the SshProxy setup. This means that SshProxy's
implementation of the LogPolicy trait can answer queries like
askappend() and logging_error() by passing them on to the same
LogPolicy used by the main backend.
Not all callers of new_connection have a LogPolicy, so we still have
to fall back to the previous conservative default behaviour if
SshProxy doesn't have a LogPolicy it can ask.
The main backend implementations didn't _quite_ have access to a
LogPolicy already, but they do have a LogContext, which has a
LogPolicy vtable pointer inside it; so I've added a query function
log_get_policy() which allows them to extract that pointer to pass to
new_connection.
This is the first step of fixing the non-interactivity limitations of
SshProxy. But it's also the easiest step: the next ones will be more
involved.
All four of the other network-protocol backends (Raw, Telnet, rlogin
and SUPDUP) now have a 'socket_connected' flag, which starts off false
and is set to true when (if) their Socket signals that the connection
attempt has succeeded.
This field is used to tell backend_socket_log whether the session has
started yet (hence, whether it should still be logging messages from
the proxy). This replaces various ad-hoc answers to that question in
each backend, which were the best I could do when sockets didn't
notify connection success. Now they do, we can do it properly.
Also, the new flag controls the answer to each backend's sendok()
method, which makes them all satisfy a new policy rule: no backend
shall return true from sendok() while its network connection attempt
is still ongoing.
(Rationale: the network connection attempt may in future involve a
proxy implementation interacting with the user via the terminal, and
it can't do that if the backend is already consuming all the terminal
input.)
This is called by the backend to notify the Seat that the connection
has progressed to the point where the main session channel (i.e. the
thing that would typically correspond to the client's stdin/stdout)
has been successfully set up.
The only Seat that implements this method nontrivially is the one in
SshProxy, which uses it as an indication that the proxied connection
to the remote host has succeeded, and sends the
PLUGLOG_CONNECT_SUCCESS notification to its own Plug.
Hence, the only backends that need to implement it at the moment are
the two SSH-shaped backends (SSH proper and bare-connection / psusan).
For other backends, it's not always obvious what 'main session
channel' would even mean, or whether it means anything very useful; so
I've also introduced a backend flag indicating whether the backend is
expecting to call that method at all, so as not to have to spend
pointless effort on defining an arbitrary meaning for it in other
contexts.
So a lot of this patch is just introducing the new method and putting
its trivial do-nothing implementation into all the existing Seat
methods. The interesting parts happen in ssh/mainchan.c (which
actually calls it), and sshproxy.c (which does something useful in
response).
On a similar theme of separating the query operation from the
attempted change, backend_send() now no longer has the side effect of
returning the current size of the send buffer. Instead, you have to
call backend_sendbuffer() every time you want to know that.
This complicates the API in one sense (more separate functions), but
in another sense, simplifies it (each function does something
simpler). When I start putting one Seat in front of another during SSH
proxying, the latter will be more important - in particular, it means
you can find out _whether_ a seat can support changing trust status
without having to actually attempt a destructive modification.
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().
