1
0
mirror of https://git.tartarus.org/simon/putty.git synced 2025-07-01 11:32:48 -05:00

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!)
This commit is contained in:
Simon Tatham
2021-12-28 17:52:00 +00:00
parent a82ab70b0b
commit a2ff884512
51 changed files with 648 additions and 372 deletions

View File

@ -16,6 +16,7 @@ add_sources_from_current_dir(utils
utils/load_system32_dll.c
utils/ltime.c
utils/makedlgitemborderless.c
utils/make_spr_sw_abort_winerror.c
utils/message_box.c
utils/minefield.c
utils/open_for_write_would_lose_data.c

View File

@ -32,10 +32,10 @@ void console_print_error_msg(const char *prefix, const char *msg)
fflush(stderr);
}
int console_confirm_ssh_host_key(
SeatPromptResult console_confirm_ssh_host_key(
Seat *seat, const char *host, int port, const char *keytype,
char *keystr, const char *keydisp, char **fingerprints, bool mismatch,
void (*callback)(void *ctx, int result), void *ctx)
void (*callback)(void *ctx, SeatPromptResult result), void *ctx)
{
HANDLE hin;
DWORD savemode, i;
@ -64,7 +64,7 @@ int console_confirm_ssh_host_key(
if (console_batch_mode) {
fputs(console_abandoned_msg, stderr);
return 0;
return SPR_SW_ABORT("Cannot confirm a host key in batch mode");
}
fputs(intro, stderr);
@ -102,16 +102,16 @@ int console_confirm_ssh_host_key(
line[0] != 'q' && line[0] != 'Q') {
if (line[0] == 'y' || line[0] == 'Y')
store_host_key(host, port, keytype, keystr);
return 1;
return SPR_OK;
} else {
fputs(console_abandoned_msg, stderr);
return 0;
return SPR_USER_ABORT;
}
}
int console_confirm_weak_crypto_primitive(
SeatPromptResult console_confirm_weak_crypto_primitive(
Seat *seat, const char *algtype, const char *algname,
void (*callback)(void *ctx, int result), void *ctx)
void (*callback)(void *ctx, SeatPromptResult result), void *ctx)
{
HANDLE hin;
DWORD savemode, i;
@ -122,7 +122,8 @@ int console_confirm_weak_crypto_primitive(
if (console_batch_mode) {
fputs(console_abandoned_msg, stderr);
return 0;
return SPR_SW_ABORT("Cannot confirm a weak crypto primitive "
"in batch mode");
}
fputs(console_continue_prompt, stderr);
@ -136,16 +137,16 @@ int console_confirm_weak_crypto_primitive(
SetConsoleMode(hin, savemode);
if (line[0] == 'y' || line[0] == 'Y') {
return 1;
return SPR_OK;
} else {
fputs(console_abandoned_msg, stderr);
return 0;
return SPR_USER_ABORT;
}
}
int console_confirm_weak_cached_hostkey(
SeatPromptResult console_confirm_weak_cached_hostkey(
Seat *seat, const char *algname, const char *betteralgs,
void (*callback)(void *ctx, int result), void *ctx)
void (*callback)(void *ctx, SeatPromptResult result), void *ctx)
{
HANDLE hin;
DWORD savemode, i;
@ -156,7 +157,8 @@ int console_confirm_weak_cached_hostkey(
if (console_batch_mode) {
fputs(console_abandoned_msg, stderr);
return 0;
return SPR_SW_ABORT("Cannot confirm a weak cached host key "
"in batch mode");
}
fputs(console_continue_prompt, stderr);
@ -170,10 +172,10 @@ int console_confirm_weak_cached_hostkey(
SetConsoleMode(hin, savemode);
if (line[0] == 'y' || line[0] == 'Y') {
return 1;
return SPR_OK;
} else {
fputs(console_abandoned_msg, stderr);
return 0;
return SPR_USER_ABORT;
}
}
@ -343,7 +345,7 @@ static void console_write(HANDLE hout, ptrlen data)
WriteFile(hout, data.ptr, data.len, &dummy, NULL);
}
int console_get_userpass_input(prompts_t *p)
SeatPromptResult console_get_userpass_input(prompts_t *p)
{
HANDLE hin = INVALID_HANDLE_VALUE, hout = INVALID_HANDLE_VALUE;
size_t curr_prompt;
@ -365,7 +367,8 @@ int console_get_userpass_input(prompts_t *p)
*/
if (p->n_prompts) {
if (console_batch_mode)
return 0;
return SPR_SW_ABORT("Cannot answer interactive prompts "
"in batch mode");
hin = GetStdHandle(STD_INPUT_HANDLE);
if (hin == INVALID_HANDLE_VALUE) {
fprintf(stderr, "Cannot get standard input handle\n");
@ -418,6 +421,7 @@ int console_get_userpass_input(prompts_t *p)
console_write(hout, ptrlen_from_asciz(pr->prompt));
bool failed = false;
SeatPromptResult spr;
while (1) {
/*
* Amount of data to try to read from the console in one
@ -440,8 +444,17 @@ int console_get_userpass_input(prompts_t *p)
void *ptr = strbuf_append(pr->result, toread);
DWORD ret = 0;
if (!ReadFile(hin, ptr, toread, &ret, NULL) || ret == 0) {
if (!ReadFile(hin, ptr, toread, &ret, NULL)) {
/* An OS error when reading from the console is treated as an
* unexpected error and reported to the user. */
failed = true;
spr = make_spr_sw_abort_winerror(
"Error reading from console", GetLastError());
break;
} else if (ret == 0) {
/* Regard EOF on the terminal as a deliberate user-abort */
failed = true;
spr = SPR_USER_ABORT;
break;
}
@ -457,10 +470,9 @@ int console_get_userpass_input(prompts_t *p)
if (!pr->echo)
console_write(hout, PTRLEN_LITERAL("\r\n"));
if (failed) {
return 0; /* failure due to read error */
}
if (failed)
return spr;
}
return 1; /* success */
return SPR_OK;
}

View File

@ -976,10 +976,10 @@ static INT_PTR CALLBACK HostKeyDialogProc(HWND hwnd, UINT msg,
return 0;
}
int win_seat_confirm_ssh_host_key(
SeatPromptResult win_seat_confirm_ssh_host_key(
Seat *seat, const char *host, int port, const char *keytype,
char *keystr, const char *keydisp, char **fingerprints, bool mismatch,
void (*callback)(void *ctx, int result), void *vctx)
void (*callback)(void *ctx, SeatPromptResult result), void *vctx)
{
WinGuiSeat *wgs = container_of(seat, WinGuiSeat, seat);
@ -1008,21 +1008,21 @@ int win_seat_confirm_ssh_host_key(
assert(mbret==IDC_HK_ACCEPT || mbret==IDC_HK_ONCE || mbret==IDCANCEL);
if (mbret == IDC_HK_ACCEPT) {
store_host_key(host, port, keytype, keystr);
return 1;
return SPR_OK;
} else if (mbret == IDC_HK_ONCE) {
return 1;
return SPR_OK;
}
return 0; /* abandon the connection */
return SPR_USER_ABORT;
}
/*
* Ask whether the selected algorithm is acceptable (since it was
* below the configured 'warn' threshold).
*/
int win_seat_confirm_weak_crypto_primitive(
SeatPromptResult win_seat_confirm_weak_crypto_primitive(
Seat *seat, const char *algtype, const char *algname,
void (*callback)(void *ctx, int result), void *ctx)
void (*callback)(void *ctx, SeatPromptResult result), void *ctx)
{
static const char mbtitle[] = "%s Security Alert";
static const char msg[] =
@ -1041,14 +1041,14 @@ int win_seat_confirm_weak_crypto_primitive(
sfree(message);
sfree(title);
if (mbret == IDYES)
return 1;
return SPR_OK;
else
return 0;
return SPR_USER_ABORT;
}
int win_seat_confirm_weak_cached_hostkey(
SeatPromptResult win_seat_confirm_weak_cached_hostkey(
Seat *seat, const char *algname, const char *betteralgs,
void (*callback)(void *ctx, int result), void *ctx)
void (*callback)(void *ctx, SeatPromptResult result), void *ctx)
{
static const char mbtitle[] = "%s Security Alert";
static const char msg[] =
@ -1069,9 +1069,9 @@ int win_seat_confirm_weak_cached_hostkey(
sfree(message);
sfree(title);
if (mbret == IDYES)
return 1;
return SPR_OK;
else
return 0;
return SPR_USER_ABORT;
}
/*

View File

@ -216,16 +216,16 @@ int has_embedded_chm(void); /* 1 = yes, 0 = no, -1 = N/A */
* GUI seat methods in windlg.c, so that the vtable definition in
* window.c can refer to them.
*/
int win_seat_confirm_ssh_host_key(
SeatPromptResult win_seat_confirm_ssh_host_key(
Seat *seat, const char *host, int port, const char *keytype,
char *keystr, const char *keydisp, char **key_fingerprints, bool mismatch,
void (*callback)(void *ctx, int result), void *ctx);
int win_seat_confirm_weak_crypto_primitive(
void (*callback)(void *ctx, SeatPromptResult result), void *ctx);
SeatPromptResult win_seat_confirm_weak_crypto_primitive(
Seat *seat, const char *algtype, const char *algname,
void (*callback)(void *ctx, int result), void *ctx);
int win_seat_confirm_weak_cached_hostkey(
void (*callback)(void *ctx, SeatPromptResult result), void *ctx);
SeatPromptResult win_seat_confirm_weak_cached_hostkey(
Seat *seat, const char *algname, const char *betteralgs,
void (*callback)(void *ctx, int result), void *ctx);
void (*callback)(void *ctx, SeatPromptResult result), void *ctx);
/*
* Windows-specific clipboard helper function shared with windlg.c,
@ -734,4 +734,6 @@ bool handle_special_filemapping_cmdline(char *cmdline, Conf *conf);
void plug_closing_system_error(Plug *plug, DWORD error);
void plug_closing_winsock_error(Plug *plug, DWORD error);
SeatPromptResult make_spr_sw_abort_winerror(const char *prefix, DWORD error);
#endif /* PUTTY_WINDOWS_PLATFORM_H */

View File

@ -65,13 +65,13 @@ static bool plink_eof(Seat *seat)
return false; /* do not respond to incoming EOF with outgoing */
}
static int plink_get_userpass_input(Seat *seat, prompts_t *p)
static SeatPromptResult plink_get_userpass_input(Seat *seat, prompts_t *p)
{
int ret;
ret = cmdline_get_passwd_input(p);
if (ret == -1)
ret = console_get_userpass_input(p);
return ret;
SeatPromptResult spr;
spr = cmdline_get_passwd_input(p);
if (spr.kind == SPRK_INCOMPLETE)
spr = console_get_userpass_input(p);
return spr;
}
static bool plink_seat_interactive(Seat *seat)

View File

@ -12,13 +12,13 @@
#include "ssh.h"
#include "security-api.h"
int filexfer_get_userpass_input(Seat *seat, prompts_t *p)
SeatPromptResult filexfer_get_userpass_input(Seat *seat, prompts_t *p)
{
int ret;
ret = cmdline_get_passwd_input(p);
if (ret == -1)
ret = console_get_userpass_input(p);
return ret;
SeatPromptResult spr;
spr = cmdline_get_passwd_input(p);
if (spr.kind == SPRK_INCOMPLETE)
spr = console_get_userpass_input(p);
return spr;
}
void platform_get_x11_auth(struct X11Display *display, Conf *conf)

View File

@ -0,0 +1,22 @@
/*
* Constructor function for a SeatPromptResult of the 'software abort'
* category, whose error message includes the translation of an OS
* error code.
*/
#include "putty.h"
static void spr_winerror_errfn(SeatPromptResult spr, BinarySink *bs)
{
put_fmt(bs, "%s: %s", spr.errdata_lit, win_strerror(spr.errdata_u));
}
SeatPromptResult make_spr_sw_abort_winerror(const char *prefix, DWORD error)
{
SeatPromptResult spr;
spr.kind = SPRK_SW_ABORT;
spr.errfn = spr_winerror_errfn;
spr.errdata_lit = prefix;
spr.errdata_u = error;
return spr;
}

View File

@ -320,7 +320,7 @@ static StripCtrlChars *win_seat_stripctrl_new(
static size_t win_seat_output(
Seat *seat, SeatOutputType type, const void *, size_t);
static bool win_seat_eof(Seat *seat);
static int win_seat_get_userpass_input(Seat *seat, prompts_t *p);
static SeatPromptResult win_seat_get_userpass_input(Seat *seat, prompts_t *p);
static void win_seat_notify_remote_exit(Seat *seat);
static void win_seat_connection_fatal(Seat *seat, const char *msg);
static void win_seat_update_specials_menu(Seat *seat);
@ -5787,13 +5787,13 @@ static bool win_seat_eof(Seat *seat)
return true; /* do respond to incoming EOF with outgoing */
}
static int win_seat_get_userpass_input(Seat *seat, prompts_t *p)
static SeatPromptResult win_seat_get_userpass_input(Seat *seat, prompts_t *p)
{
int ret;
ret = cmdline_get_passwd_input(p);
if (ret == -1)
ret = term_get_userpass_input(term, p);
return ret;
SeatPromptResult spr;
spr = cmdline_get_passwd_input(p);
if (spr.kind == SPRK_INCOMPLETE)
spr = term_get_userpass_input(term, p);
return spr;
}
static void win_seat_set_trust_status(Seat *seat, bool trusted)