simon-git: putty (main): Simon Tatham
Commits to Tartarus hosted VCS
tartarus-commits at lists.tartarus.org
Tue Sep 14 13:23:14 BST 2021
TL;DR:
2fd2f471 Squash shift warnings in ssh2_bpp_check_unimplemented.
9f0e7d29 Backends: notify ldisc when sendok becomes true. (NFC)
cd8a7181 Complete rework of terminal userpass input system.
d1374c58 SshProxy: pass through userpass input prompts.
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-09-14 13:23:14
commit 2fd2f4715d55b5009620b6c03de2e939712cc249
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=2fd2f4715d55b5009620b6c03de2e939712cc249;hp=8e35e6eeae986f1c5575790e444533c3c1269035
Author: Simon Tatham <anakin at pobox.com>
Date: Tue Sep 14 11:16:49 2021 +0100
Squash shift warnings in ssh2_bpp_check_unimplemented.
I did a horrible thing with a list macro which builds up a 256-bit
bitmap of known SSH-2 message types at compile time, by means of
evaluating a conditional expression per known message type and per
bitmap word which boils down to (in pseudocode)
(shift count in range ? 1 << shift count : 0)
I think this is perfectly valid C. If the shift count is out of range,
then the use of the << operator in the true branch of the ?: would
have undefined behaviour if it were executed - but that's OK, because
in that situation, the safe false branch is executed instead.
But when the whole thing is a compile-time evaluated constant
expression, the compiler can prove statically that the << in the true
branch is an out-of-range shift, and at least some compilers will warn
about it verbosely. The same compiler *could* also prove statically
that that branch isn't taken, and use that to suppress the warning -
but at least clang does not.
The solution is the same one I used in shift_right_by_one_word and
shift_left_by_one_word in mpint.c: inside the true branch, nest a
second conditional expression which coerces the shift count to always
be in range, by setting it to 0 if it's not. This doesn't affect the
output, because the only cases in which the output of the true branch
is altered by this transformation are the ones in which the true
branch wasn't taken anyway.
So this change should make no difference to the output of this macro
construction, but it suppresses about 350 pointless warnings from
clang.
ssh/common.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
commit 9f0e7d291558989cc44f98cf8603d5cf2787ce3a
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=9f0e7d291558989cc44f98cf8603d5cf2787ce3a;hp=2fd2f4715d55b5009620b6c03de2e939712cc249
Author: Simon Tatham <anakin at pobox.com>
Date: Tue Sep 14 10:13:28 2021 +0100
Backends: notify ldisc when sendok becomes true. (NFC)
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.
ldisc.c | 4 ++++
otherbackends/raw.c | 6 +++++-
otherbackends/rlogin.c | 6 +++++-
otherbackends/supdup.c | 5 +++++
otherbackends/telnet.c | 2 ++
pscp.c | 1 +
psftp.c | 1 +
putty.h | 1 +
ssh.h | 1 +
ssh/connection1.c | 2 ++
ssh/connection2.c | 2 ++
ssh/server.c | 2 ++
ssh/ssh.c | 8 ++++++++
13 files changed, 39 insertions(+), 2 deletions(-)
commit cd8a7181fddf42e14c44f024de71732bc2a6c715
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=cd8a7181fddf42e14c44f024de71732bc2a6c715;hp=9f0e7d291558989cc44f98cf8603d5cf2787ce3a
Author: Simon Tatham <anakin at pobox.com>
Date: Tue Sep 14 11:57:21 2021 +0100
Complete rework of terminal userpass input system.
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.
fuzzterm.c | 5 +
ldisc.c | 250 +++++++++++++++++++++++++++++++++++++++++++++----
noterm.c | 5 +
otherbackends/rlogin.c | 69 ++++++--------
putty.h | 60 +++++++++---
ssh/common.c | 13 +++
ssh/connection1.c | 18 +---
ssh/connection2.c | 18 +---
ssh/login1.c | 56 +++--------
ssh/ppl.h | 5 +
ssh/userauth2-client.c | 92 +++++-------------
sshproxy.c | 3 +-
terminal.c | 54 +++++++++--
unix/plink.c | 2 +-
unix/sftp.c | 2 +-
unix/window.c | 5 +-
utils/nullseat.c | 3 +-
utils/prompts.c | 2 +
utils/tempseat.c | 3 +-
windows/plink.c | 2 +-
windows/sftp.c | 2 +-
windows/window.c | 8 +-
22 files changed, 445 insertions(+), 232 deletions(-)
commit d1374c5890c945dc50cbeedca502b8d4100845d6
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=d1374c5890c945dc50cbeedca502b8d4100845d6;hp=cd8a7181fddf42e14c44f024de71732bc2a6c715
Author: Simon Tatham <anakin at pobox.com>
Date: Tue Sep 14 12:14:37 2021 +0100
SshProxy: pass through userpass input prompts.
This is the big payoff from the huge refactoring in the previous
commit: now it's possible for proxy implementations to present their
own interactive prompts via the seat_get_userpass_input system,
because the input data that those prompts will need to consume is now
always somewhere sensible (and hasn't, for example, already been put
on to the main backend's input queue where the proxy can't get at it).
Like the GUI dialog prompts, this isn't yet fully polished, because
the login and password prompts are very unclear about which SSH server
they're talking about. But at least you now _can_ log in manually with
a username and password to two SSH servers in succession (if you know
which server(s) you're expecting to see prompts from), and that was
the really hard part.
sshproxy.c | 65 ++++++++++++++++++++++++++++++--------------------------------
1 file changed, 31 insertions(+), 34 deletions(-)
More information about the tartarus-commits
mailing list