simon-git: putty (main): Simon Tatham
Commits to Tartarus hosted VCS
tartarus-commits at lists.tartarus.org
Tue Dec 28 18:11:24 GMT 2021
TL;DR:
60e55757 Cosmetic fix: silly parameter name caused by copy-paste.
88d5bb2a Cosmetic fix: double indentation in an if statement.
a82ab70b Fix error handling when command-line password fails.
a2ff8845 Richer data type for interactive prompt results.
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-12-28 18:11:24
commit 60e557575efc6857ade4985db9b70bee15bee98d
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=60e557575efc6857ade4985db9b70bee15bee98d;hp=831accb2a90b305422742ed2bb3eab5d927cee0e
Author: Simon Tatham <anakin at pobox.com>
Date: Mon Dec 27 11:43:31 2021 +0000
Cosmetic fix: silly parameter name caused by copy-paste.
In SSH-1 there's a function that takes a void * that it casts to the
state of the login layer. The corresponding function in SSH-2 casts it
to the state of a differently named layer, but I had still called the
parameter 'loginv'.
ssh/transport2.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
commit 88d5bb2a222470f1f615d96384f3701f1406790d
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=88d5bb2a222470f1f615d96384f3701f1406790d;hp=60e557575efc6857ade4985db9b70bee15bee98d
Author: Simon Tatham <anakin at pobox.com>
Date: Mon Dec 27 13:02:31 2021 +0000
Cosmetic fix: double indentation in an if statement.
Not sure how that happened, but since I've spotted it, let's fix it.
ssh/transport2.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
commit a82ab70b0bf418a7c18d07e0090bbf194f795cbe
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=a82ab70b0bf418a7c18d07e0090bbf194f795cbe;hp=88d5bb2a222470f1f615d96384f3701f1406790d
Author: Simon Tatham <anakin at pobox.com>
Date: Tue Dec 28 15:15:53 2021 +0000
Fix error handling when command-line password fails.
In cmdline_get_passwd_input(), there's a boolean 'tried_once' which is
set the first time the function returns the password set by -pw or
-pwfile. The idea, as clearly commented in the code, was that if
cmdline_get_password_input asks for that password _again_, we return
failure, as if the user had refused to make a second attempt.
But that wasn't actually what happened, because when we set tried_once
to true, we also set cmdline_password to NULL, which causes the second
call to the function to behave as if no password was ever provided at
all. So after the -pw password failed, we'd fall back to asking
interactively.
This change moves the check of cmdline_password to after the check of
tried_once, restoring the originally intended behaviour: password
authentication will now _only_ be done via the pre-set password, if
there is one.
This seems like an xkcd #1172 kind of change: now that it's been wrong
for a while, _someone_ has probably found the unintended behaviour
useful, and started relying on it. So it may become necessary to add
an option to set the behaviour either way. But for the moment, let's
try it the way I originally intended it.
cmdline.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
commit a2ff884512be0cb0ef62aceee5428ba42ca64d9b
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=a2ff884512be0cb0ef62aceee5428ba42ca64d9b;hp=a82ab70b0bf418a7c18d07e0090bbf194f795cbe
Author: Simon Tatham <anakin at pobox.com>
Date: Tue Dec 28 17:52:00 2021 +0000
Richer data type for interactive prompt results.
All the seat functions that request an interactive prompt of some kind
to the user - both the main seat_get_userpass_input and the various
confirmation dialogs for things like host keys - were using a simple
int return value, with the general semantics of 0 = "fail", 1 =
"proceed" (and in the case of seat_get_userpass_input, answers to the
prompts were provided), and -1 = "request in progress, wait for a
callback".
In this commit I change all those functions' return types to a new
struct called SeatPromptResult, whose primary field is an enum
replacing those simple integer values.
The main purpose is that the enum has not three but _four_ values: the
"fail" result has been split into 'user abort' and 'software abort'.
The distinction is that a user abort occurs as a result of an
interactive UI action, such as the user clicking 'cancel' in a dialog
box or hitting ^D or ^C at a terminal password prompt - and therefore,
there's no need to display an error message telling the user that the
interactive operation has failed, because the user already knows,
because they _did_ it. 'Software abort' is from any other cause, where
PuTTY is the first to know there was a problem, and has to tell the
user.
We already had this 'user abort' vs 'software abort' distinction in
other parts of the code - the SSH backend has separate termination
functions which protocol layers can call. But we assumed that any
failure from an interactive prompt request fell into the 'user abort'
category, which is not true. A couple of examples: if you configure a
host key fingerprint in your saved session via the SSH > Host keys
pane, and the server presents a host key that doesn't match it, then
verify_ssh_host_key would report that the user had aborted the
connection, and feel no need to tell the user what had gone wrong!
Similarly, if a password provided on the command line was not
accepted, then (after I fixed the semantics of that in the previous
commit) the same wrong handling would occur.
So now, those Seat prompt functions too can communicate whether the
user or the software originated a connection abort. And in the latter
case, we also provide an error message to present to the user. Result:
in those two example cases (and others), error messages should no
longer go missing.
Implementation note: to avoid the hassle of having the error message
in a SeatPromptResult being a dynamically allocated string (and hence,
every recipient of one must always check whether it's non-NULL and
free it on every exit path, plus being careful about copying the
struct around), I've instead arranged that the structure contains a
function pointer and a couple of parameters, so that the string form
of the message can be constructed on demand. That way, the only users
who need to free it are the ones who actually _asked_ for it in the
first place, which is a much smaller set.
(This is one of the rare occasions that I regret not having C++'s
extra features available in this code base - a unique_ptr or
shared_ptr to a string would have been just the thing here, and the
compiler would have done all the hard work for me of remembering where
to insert the frees!)
cgtest.c | 6 +-
cmdgen.c | 29 ++++--
cmdline.c | 10 +-
defs.h | 1 +
otherbackends/rlogin.c | 22 +++--
proxy/http.c | 8 +-
proxy/local.c | 12 ++-
proxy/proxy.c | 10 ++
proxy/proxy.h | 5 +-
proxy/socks5.c | 8 +-
proxy/sshproxy.c | 25 ++---
proxy/telnet.c | 8 +-
putty.h | 152 +++++++++++++++++++++--------
ssh.h | 6 +-
ssh/common.c | 35 +++++--
ssh/connection1.c | 2 +-
ssh/connection1.h | 2 +-
ssh/connection2.c | 2 +-
ssh/connection2.h | 2 +-
ssh/kex2-client.c | 11 +--
ssh/login1.c | 59 ++++++-----
ssh/server.c | 10 +-
ssh/transport2.c | 44 ++++-----
ssh/transport2.h | 4 +-
ssh/userauth2-client.c | 59 ++++++-----
stubs/nocmdline.c | 4 +-
stubs/noterm.c | 4 +-
terminal/terminal.c | 24 ++---
unix/CMakeLists.txt | 1 +
unix/console.c | 55 +++++++----
unix/dialog.c | 87 +++++++++++------
unix/pageant.c | 14 ++-
unix/platform.h | 14 +--
unix/plink.c | 12 +--
unix/sftp.c | 12 +--
unix/utils/make_spr_sw_abort_errno.c | 22 +++++
unix/window.c | 12 +--
utils/CMakeLists.txt | 2 +
utils/make_spr_sw_abort_static.c | 21 ++++
utils/nullseat.c | 18 ++--
utils/prompts.c | 2 +-
utils/spr_get_error_message.c | 13 +++
utils/tempseat.c | 14 +--
windows/CMakeLists.txt | 1 +
windows/console.c | 56 ++++++-----
windows/dialog.c | 26 ++---
windows/platform.h | 14 +--
windows/plink.c | 12 +--
windows/sftp.c | 12 +--
windows/utils/make_spr_sw_abort_winerror.c | 22 +++++
windows/window.c | 14 +--
51 files changed, 648 insertions(+), 372 deletions(-)
More information about the tartarus-commits
mailing list