simon-git: putty (main): Simon Tatham
Commits to Tartarus hosted VCS
tartarus-commits at lists.tartarus.org
Sat Oct 30 18:26:32 BST 2021
TL;DR:
27f00038 Fix trust-sigil handling when scrolling the terminal.
76dc2855 Add memsets after allocation of all Backend implementors.
971c70e6 Move proxy-related source files into a subdirectory.
74a0be9c Split seat_banner from seat_output.
44db74ec Introduce a new 'Interactor' trait.
aac5e096 Add Interactor methods to get/set LogPolicy and Seat.
89a390bd Pass an Interactor to new_connection().
f00c72cc Framework for announcing which Interactor is talking.
74605944 Move TempSeat creation/destruction into Interactor.
215b9d17 Actually print announcements of Interactors' identity.
Repository: https://git.tartarus.org/simon/putty.git
On the web: https://git.tartarus.org/?p=simon/putty.git
Branch updated: main
Committer: Simon Tatham <anakin at pobox.com>
Date: 2021-10-30 18:26:32
commit 27f00038e1e22b9aba6cc287471ff2dda41fc8d4
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=27f00038e1e22b9aba6cc287471ff2dda41fc8d4;hp=5eee8ca648c8453ff5308f7a009277eccfa80d16
Author: Simon Tatham <anakin at pobox.com>
Date: Sat Oct 30 15:32:34 2021 +0100
Fix trust-sigil handling when scrolling the terminal.
Previously, when we scrolled the terminal, the newly exposed line at
the bottom would be immediately allocated a trust status corresponding
to the current state of the terminal. So if you're in trusted mode and
you print a newline, then the line scrolled on at the bottom
immediately gets a trust sigil, whether you subsequently print
anything on it or not.
Up until now, that hasn't mattered, because we always _do_ print
something on it. But if you don't - if you send \r\n\r\n to
deliberately leave a blank line - then it turns out that's not what we
want after all, because if the screen _doesn't_ scroll, the
passed-over line remains completely blank, whereas if it does scroll
the blank line gets a trust sigil, which is inconsistent.
Now, terminal lines newly exposed by a scroll have untrusted status,
just the same as terminal lines that were present in the initial blank
screen. They only become trusted if you actually print at least one
character on them (whereupon check_trust_status will re-clear them
just in case). And this is now independent of whether the terminal has
scrolled or not.
terminal/terminal.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
commit 76dc28552c401a4cd77d2bdd8b2a756c52334c06
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=76dc28552c401a4cd77d2bdd8b2a756c52334c06;hp=27f00038e1e22b9aba6cc287471ff2dda41fc8d4
Author: Simon Tatham <anakin at pobox.com>
Date: Sat Oct 30 14:51:24 2021 +0100
Add memsets after allocation of all Backend implementors.
Now every struct that implements the Backend trait is completely
cleared before we start initialising any of its fields. This will mean
I can add new fields that default to 0 or NULL, without having to mess
around initialising them explicitly everywhere.
otherbackends/raw.c | 1 +
otherbackends/rlogin.c | 1 +
otherbackends/supdup.c | 1 +
otherbackends/telnet.c | 1 +
unix/pty.c | 1 +
unix/serial.c | 1 +
windows/conpty.c | 1 +
windows/serial.c | 1 +
8 files changed, 8 insertions(+)
commit 971c70e6031f3fcfd043c59a75ed886b1b1df905
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=971c70e6031f3fcfd043c59a75ed886b1b1df905;hp=76dc28552c401a4cd77d2bdd8b2a756c52334c06
Author: Simon Tatham <anakin at pobox.com>
Date: Sat Oct 30 11:02:28 2021 +0100
Move proxy-related source files into a subdirectory.
There are quite a few of them already, and I'm about to make another
one, so let's start with a bit of tidying up.
The CMake build organisation is unchanged: I haven't put the proxy
object files into a separate library, just moved the locations of the
source files. (Organising proxying as a library would be tricky
anyway, because of the various overrides for tools that want to avoid
cryptography.)
CMakeLists.txt | 8 ++++----
cproxy.c => proxy/cproxy.c | 0
nocproxy.c => proxy/nocproxy.c | 0
noproxy.c => proxy/noproxy.c | 0
nosshproxy.c => proxy/nosshproxy.c | 0
pproxy.c => proxy/pproxy.c | 0
proxy.c => proxy/proxy.c | 0
proxy.h => proxy/proxy.h | 0
sshproxy.c => proxy/sshproxy.c | 0
unix/CMakeLists.txt | 10 +++++-----
unix/local-proxy.c | 2 +-
unix/sharing.c | 2 +-
windows/CMakeLists.txt | 6 +++---
windows/local-proxy.c | 2 +-
windows/named-pipe-client.c | 2 +-
windows/named-pipe-server.c | 2 +-
windows/sharing.c | 2 +-
17 files changed, 18 insertions(+), 18 deletions(-)
commit 74a0be9c5647ed033b7ef7b58d074a3510bcc071
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=74a0be9c5647ed033b7ef7b58d074a3510bcc071;hp=971c70e6031f3fcfd043c59a75ed886b1b1df905
Author: Simon Tatham <anakin at pobox.com>
Date: Sat Oct 30 17:06:00 2021 +0100
Split seat_banner from seat_output.
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.
proxy/sshproxy.c | 29 ++++++++++++++++-------------
pscp.c | 1 +
psftp.c | 1 +
putty.h | 29 ++++++++++++++++++++---------
ssh/server.c | 1 +
ssh/sesschan.c | 6 +-----
unix/plink.c | 1 +
unix/window.c | 1 +
utils/nullseat.c | 3 +++
utils/tempseat.c | 6 ++++++
windows/plink.c | 1 +
windows/window.c | 1 +
12 files changed, 53 insertions(+), 27 deletions(-)
commit 44db74ec513d36b95d28bb718f85e594b18c49de
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=44db74ec513d36b95d28bb718f85e594b18c49de;hp=74a0be9c5647ed033b7ef7b58d074a3510bcc071
Author: Simon Tatham <anakin at pobox.com>
Date: Sat Oct 30 17:16:08 2021 +0100
Introduce a new 'Interactor' trait.
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!
defs.h | 2 ++
network.h | 22 ---------------
otherbackends/raw.c | 27 +++++++++---------
otherbackends/rlogin.c | 27 +++++++++---------
otherbackends/supdup.c | 17 ++++++------
otherbackends/telnet.c | 27 +++++++++---------
putty.h | 75 ++++++++++++++++++++++++++++++++------------------
ssh/ssh.c | 28 +++++++++----------
8 files changed, 110 insertions(+), 115 deletions(-)
commit aac5e096fa0579c754776167c1486bdb693c065d
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=aac5e096fa0579c754776167c1486bdb693c065d;hp=44db74ec513d36b95d28bb718f85e594b18c49de
Author: Simon Tatham <anakin at pobox.com>
Date: Sat Oct 30 17:34:53 2021 +0100
Add Interactor methods to get/set LogPolicy and Seat.
Nothing uses this yet, but the next commit will.
otherbackends/raw.c | 21 +++++++++++++++++++++
otherbackends/rlogin.c | 21 +++++++++++++++++++++
otherbackends/supdup.c | 21 +++++++++++++++++++++
otherbackends/telnet.c | 21 +++++++++++++++++++++
putty.h | 22 ++++++++++++++++++++++
ssh/ssh.c | 21 +++++++++++++++++++++
6 files changed, 127 insertions(+)
commit 89a390bdeb5b624e17789518efb843e3a88b5417
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=89a390bdeb5b624e17789518efb843e3a88b5417;hp=aac5e096fa0579c754776167c1486bdb693c065d
Author: Simon Tatham <anakin at pobox.com>
Date: Sat Oct 30 17:36:52 2021 +0100
Pass an Interactor to new_connection().
Thanks to the previous commit, this new parameter can replace two of
the existing ones: instead of passing a LogPolicy and a Seat, we now
pass just an Interactor, from which any proxy implementation can
extract the LogPolicy and the Seat anyway if they need it.
network.h | 24 +++++++++---------------
otherbackends/raw.c | 3 +--
otherbackends/rlogin.c | 2 +-
otherbackends/supdup.c | 2 +-
otherbackends/telnet.c | 2 +-
proxy/noproxy.c | 2 +-
proxy/nosshproxy.c | 3 +--
proxy/proxy.c | 4 ++--
proxy/sshproxy.c | 39 +++++++++++++++++++++------------------
ssh/portfwd.c | 2 +-
ssh/ssh.c | 3 +--
ssh/x11fwd.c | 2 +-
unix/pageant.c | 2 +-
unix/sharing.c | 2 +-
14 files changed, 43 insertions(+), 49 deletions(-)
commit f00c72cc2a92ce2c49e6951a05860ddca551e1cd
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=f00c72cc2a92ce2c49e6951a05860ddca551e1cd;hp=89a390bdeb5b624e17789518efb843e3a88b5417
Author: Simon Tatham <anakin at pobox.com>
Date: Sat Oct 30 18:05:36 2021 +0100
Framework for announcing which Interactor is talking.
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.
CMakeLists.txt | 3 ++-
defs.h | 1 +
otherbackends/rlogin.c | 3 ++-
proxy/interactor.c | 18 +++++++++++++++++
proxy/sshproxy.c | 17 +++++++++++-----
putty.h | 54 ++++++++++++++++++++++++++++++++++++--------------
ssh.h | 4 ++--
ssh/common.c | 6 +++---
ssh/connection1.c | 4 ++--
ssh/connection2.c | 4 ++--
ssh/kex2-client.c | 4 ++--
ssh/login1.c | 18 +++++++++--------
ssh/ppl.h | 4 ++++
ssh/server.c | 1 +
ssh/ssh.c | 1 +
ssh/transport2.c | 4 ++--
ssh/userauth2-client.c | 35 ++++++++++++++++----------------
utils/antispoof.c | 8 ++++----
18 files changed, 125 insertions(+), 64 deletions(-)
commit 746059443373b8a4cd73d85f67fe473a2c56667f
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=746059443373b8a4cd73d85f67fe473a2c56667f;hp=f00c72cc2a92ce2c49e6951a05860ddca551e1cd
Author: Simon Tatham <anakin at pobox.com>
Date: Sat Oct 30 17:45:38 2021 +0100
Move TempSeat creation/destruction into Interactor.
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.
otherbackends/raw.c | 6 ------
otherbackends/rlogin.c | 6 ------
otherbackends/supdup.c | 6 ------
otherbackends/telnet.c | 6 ------
proxy/interactor.c | 37 +++++++++++++++++++++++++++++++++++++
proxy/sshproxy.c | 34 ++++++++++++++--------------------
putty.h | 8 +++++---
ssh/ssh.c | 9 ---------
8 files changed, 56 insertions(+), 56 deletions(-)
commit 215b9d17759ccbde068f5af617f12ce45bf3f9fc
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=215b9d17759ccbde068f5af617f12ce45bf3f9fc;hp=746059443373b8a4cd73d85f67fe473a2c56667f
Author: Simon Tatham <anakin at pobox.com>
Date: Sat Oct 30 18:08:02 2021 +0100
Actually print announcements of Interactors' identity.
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.
proxy/interactor.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++++++---
proxy/sshproxy.c | 9 +--------
putty.h | 15 +++++++++++++++
3 files changed, 69 insertions(+), 11 deletions(-)
More information about the tartarus-commits
mailing list