simon-git: putty (main): Simon Tatham
Commits to Tartarus hosted VCS
tartarus-commits at lists.tartarus.org
Sat Nov 6 14:50:57 GMT 2021
TL;DR:
1811f51b sshproxy: improve a couple of comments.
1fd27e64 Remove unnecessary interactor_announce() calls.
5fdce31e sshproxy: fix handling of connection closure.
364e1aa3 Convenience wrappers on plug_closing().
0fe41294 New API for plug_closing() with a custom type enum.
2ae338b4 New plug_closing error type for 'user abort'.
028714d0 Fix Plink's handling of interactor_announce() blank lines.
7eb7d5e2 New Seat query, has_mixed_input_stream().
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-11-06 14:50:57
commit 1811f51b947c894a137a7abf3e8f1e3909fda30b
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=1811f51b947c894a137a7abf3e8f1e3909fda30b;hp=aca339d189b2665f7f1f100a71896f4eb4593081
Author: Simon Tatham <anakin at pobox.com>
Date: Sat Nov 6 11:31:40 2021 +0000
sshproxy: improve a couple of comments.
Removed the FIXMEs in various Seat passthrough functions that were
there to remind me to say which SSH connection they referred to: that
is now done by the interactor_announce mechanism, as of commit
215b9d17759ccbd. (Though not by modifying the actual passthrough
functions, as it turned out, which is how I didn't find and remove the
FIXMEs when I did all that.)
Also, added a comment in wrap() explaining *why* it's allowed to (in
fact, must) cheat by making an InteractionReadySeat without going
through interactor_announce().
proxy/sshproxy.c | 22 ++++++++++++++++++----
1 file changed, 18 insertions(+), 4 deletions(-)
commit 1fd27e649a281a1e67aea1bc875165b2816cde4a
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=1fd27e649a281a1e67aea1bc875165b2816cde4a;hp=1811f51b947c894a137a7abf3e8f1e3909fda30b
Author: Simon Tatham <anakin at pobox.com>
Date: Sat Nov 6 11:32:51 2021 +0000
Remove unnecessary interactor_announce() calls.
In interactor_return_seat, I wrote a comment saying that we should
call interactor_announce when handing over to the next Interactor in
the chain, *if* any Interactor had already made any kind of
announcement.
But, having written that comment, I didn't actually *implement* the
'if' clause, and called interactor_announce unconditionally! Now
fixed.
proxy/interactor.c | 38 +++++++++++++++++++++++++-------------
1 file changed, 25 insertions(+), 13 deletions(-)
commit 5fdce31ecafc63e78afec2e2bb5155d40edc6566
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=5fdce31ecafc63e78afec2e2bb5155d40edc6566;hp=1fd27e649a281a1e67aea1bc875165b2816cde4a
Author: Simon Tatham <anakin at pobox.com>
Date: Sat Nov 6 11:48:33 2021 +0000
sshproxy: fix handling of connection closure.
Now we always respond to backend disconnection or connection_fatal by
calling plug_closing. And we always do it in a toplevel callback, so
that when the Plug responds by calling our Socket close method (which
frees us), nothing re-entrant happens.
Also, the handling of notify_remote_disconnect is brought into line
with the spec in putty.h, which says it can be sent redundantly (when
already disconnected) or spuriously (when not even disconnected at
all), so the toplevel callback queued by that method will check first.
After this change, failures during connection_setup are now handled
_mostly_ sensibly: if the proxy connection fails, then the main
connection gets enough information to pass a sensible connection_fatal
on to the real front end.
This also fixes the assertion failure mentioned in the TODO comment,
replacing it with a reasonably sensible connection_fatal() - although
I still think that in that situation it might be better not to have a
dialog box at all.
proxy/sshproxy.c | 45 +++++++++++++++++++++++++++++++++------------
1 file changed, 33 insertions(+), 12 deletions(-)
commit 364e1aa3f39b4c0dd9a04e487fe5f188d4f2f59b
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=364e1aa3f39b4c0dd9a04e487fe5f188d4f2f59b;hp=5fdce31ecafc63e78afec2e2bb5155d40edc6566
Author: Simon Tatham <anakin at pobox.com>
Date: Sat Nov 6 13:25:42 2021 +0000
Convenience wrappers on plug_closing().
Having a single plug_closing() function covering various kinds of
closure is reasonably convenient from the point of view of Plug
implementations, but it's annoying for callers, who all have to fill
in pointless NULL and 0 parameters in the cases where they're not
used.
Added some inline helper functions in network.h alongside the main
plug_closing() dispatch wrappers, so that each kind of connection
closure can present a separate API for the Socket side of the
interface, without complicating the vtable for the Plug side.
Also, added OS-specific extra helpers in the Unix and Windows
directories, which centralise the job of taking an OS error code (of
whatever kind) and translating it into its error message.
In passing, this removes the horrible ad-hoc made-up error codes in
proxy.h, which is OK, because nothing checked for them anyway, and
also I'm about to do an API change to plug_closing proper that removes
the need for them.
network.h | 4 +++
proxy/cproxy.c | 27 +++++++----------
proxy/nocproxy.c | 13 ++++-----
proxy/proxy.c | 78 ++++++++++++++++++++++---------------------------
proxy/proxy.h | 3 --
proxy/sshproxy.c | 7 +++--
unix/fd-socket.c | 6 ++--
unix/network.c | 21 ++++++++-----
unix/platform.h | 3 ++
windows/handle-socket.c | 6 ++--
windows/network.c | 25 +++++++++++-----
windows/platform.h | 4 +++
12 files changed, 103 insertions(+), 94 deletions(-)
commit 0fe41294e62546db74a631928c535e50315cf87c
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=0fe41294e62546db74a631928c535e50315cf87c;hp=364e1aa3f39b4c0dd9a04e487fe5f188d4f2f59b
Author: Simon Tatham <anakin at pobox.com>
Date: Sat Nov 6 13:28:32 2021 +0000
New API for plug_closing() with a custom type enum.
Passing an operating-system-specific error code to plug_closing(),
such as errno or GetLastError(), was always a bit weird, given that it
generally had to be handled by cross-platform receiving code in
backends. I had the platform.h implementations #define any error
values that the cross-platform code would have to handle specially,
but that's still not a great system, because it also doesn't leave
freedom to invent error representations of my own that don't
correspond to any OS code. (For example, the ones I just removed from
proxy.h.)
So now, the OS error code is gone from the plug_closing API, and in
its place is a custom enumeration of closure types: normal, error, and
the special case BROKEN_PIPE which is the only OS error code we have
so far needed to handle specially. (All others just mean 'abandon the
connection and print the textual message'.)
Having already centralised the handling of OS error codes in the
previous commit, we've now got a convenient place to add any further
type codes for errors needing special handling: each of Unix
plug_closing_errno(), Windows plug_closing_system_error(), and Windows
plug_closing_winsock_error() can easily grow extra special cases if
need be, and each one will only have to live in one place.
network.h | 42 ++++++++++++++++++++++++++++--------------
nullplug.c | 2 +-
otherbackends/raw.c | 4 ++--
otherbackends/rlogin.c | 8 +++++---
otherbackends/supdup.c | 5 +++--
otherbackends/telnet.c | 5 +++--
pageant.c | 12 ++++++------
proxy/proxy.c | 15 ++++++++-------
proxy/proxy.h | 2 +-
psocks.c | 7 +++----
ssh/portfwd.c | 6 +++---
ssh/server.c | 5 +++--
ssh/sesschan.c | 2 +-
ssh/sharing.c | 38 ++++++++++++++++++--------------------
ssh/ssh.c | 4 ++--
ssh/x11fwd.c | 4 ++--
unix/network.c | 5 ++++-
unix/pageant.c | 2 +-
unix/psusan.c | 6 ++++--
unix/uppity.c | 6 ++++--
windows/network.c | 7 +++++--
windows/platform.h | 2 --
22 files changed, 107 insertions(+), 82 deletions(-)
commit 2ae338b407ab88edeb850a17065abfb5a8a95276
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=2ae338b407ab88edeb850a17065abfb5a8a95276;hp=0fe41294e62546db74a631928c535e50315cf87c
Author: Simon Tatham <anakin at pobox.com>
Date: Sat Nov 6 13:31:09 2021 +0000
New plug_closing error type for 'user abort'.
This is generated when setup of a network connection is cancelled by
deliberate user action, namely, pressing ^C or ^D or the like at a
get_userpass_input prompt presented during proxy setup.
It's handled just like normal socket setup errors, except that it
omits the call to seat_connection_fatal, on the grounds that in this
one case of connection-setup failure, the user doesn't need to be
_informed_ that the connection failed - they already know, because
they failed it themself on purpose.
network.h | 13 +++++++++++++
otherbackends/raw.c | 3 ++-
otherbackends/rlogin.c | 3 ++-
otherbackends/supdup.c | 3 ++-
otherbackends/telnet.c | 3 ++-
proxy/sshproxy.c | 14 ++------------
ssh/ssh.c | 4 +++-
7 files changed, 26 insertions(+), 17 deletions(-)
commit 028714d02ab8e181b93ea2096fcfc12fd1133ea7
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=028714d02ab8e181b93ea2096fcfc12fd1133ea7;hp=2ae338b407ab88edeb850a17065abfb5a8a95276
Author: Simon Tatham <anakin at pobox.com>
Date: Sat Nov 6 14:01:18 2021 +0000
Fix Plink's handling of interactor_announce() blank lines.
I'd forgotten that the text-only branch of seat_antispoof_msg()
constructs a string from its input in the expectation that it's a
one-line message. So it was a mistake to put a \n at the start of the
string in interactor_announce() to get a blank line first.
Now interactor_announce() makes an extra call to seat_antispoof_msg to
show its blank line, and seat_antispoof_msg itself handles the
blank-line case specially.
proxy/interactor.c | 2 +-
utils/antispoof.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
commit 7eb7d5e2e9117132940f6bc865a23aa3cc1b60ff
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=7eb7d5e2e9117132940f6bc865a23aa3cc1b60ff;hp=028714d02ab8e181b93ea2096fcfc12fd1133ea7
Author: Simon Tatham <anakin at pobox.com>
Date: Sat Nov 6 14:33:03 2021 +0000
New Seat query, has_mixed_input_stream().
(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.
proxy/sshproxy.c | 1 +
pscp.c | 1 +
psftp.c | 1 +
putty.h | 17 +++++++++++++++++
ssh/connection1-client.c | 6 +++++-
ssh/connection2-client.c | 9 +++++++--
ssh/server.c | 1 +
ssh/sesschan.c | 1 +
unix/console.c | 27 ++++++++++++++++++---------
unix/plink.c | 1 +
unix/window.c | 1 +
utils/nullseat.c | 2 ++
utils/tempseat.c | 7 +++++++
windows/console.c | 27 ++++++++++++++++++---------
windows/plink.c | 1 +
windows/window.c | 1 +
16 files changed, 83 insertions(+), 21 deletions(-)
More information about the tartarus-commits
mailing list